Re: [PATCH 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error

[Julien Grall <julien@xen.org>]

On 28/04/2021 19:27, Costin Lupu wrote:
> Hi Julien,

Hi Costin,

> On 4/28/21 12:03 PM, Julien Grall wrote:
>> Hi Costin,
>>
>> On 27/04/2021 13:05, Costin Lupu wrote:
>>>    tools/libs/foreignmemory/private.h | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/libs/foreignmemory/private.h
>>> b/tools/libs/foreignmemory/private.h
>>> index 1ee3626dd2..f3c1ba2867 100644
>>> --- a/tools/libs/foreignmemory/private.h
>>> +++ b/tools/libs/foreignmemory/private.h
>>> @@ -10,11 +10,13 @@
>>>    #include <xen/xen.h>
>>>    #include <xen/sys/privcmd.h>
>>>    -#ifndef PAGE_SHIFT /* Mini-os, Yukk */
>>> +#ifndef PAGE_SHIFT
>>>    #define PAGE_SHIFT           12
>>>    #endif
>>> -#ifndef __MINIOS__ /* Yukk */
>>> +#ifndef PAGE_SIZE
>>>    #define PAGE_SIZE            (1UL << PAGE_SHIFT)
>>> +#endif
>>> +#ifndef PAGE_MASK
>>>    #define PAGE_MASK            (~(PAGE_SIZE-1))
>>>    #endif
>>
>> Looking at the usage, I believe PAGE_* are referring to the page
>> granularity of Xen rather than the page granularity of the control
>> domain itself.
>>
>> So it would be incorrect to use the domain's page granularity here and
>> would break dom0 using 64KB page granularity on Arm.
>>
>> Instead, we should replace PAGE_* with XC_PAGE_*. If some of them are
>> still referring to the control domain granularity, then we should use
>> getpageshift() (Or equivalent) because the userspace shouldn't rely on a
>> specific page granularity.
> 
> Yes, this makes sense. One thing I need to clarify: what does XC stand
> for? I thought for some time it stands for Xen Control.

I think it is Xen Control, which is a bit confusing for that specific 
define.

Cheers,

-- 
Julien Grall