[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/6] x86/mem-paging: use guest handle for XENMEM_paging_op_prep

On 20.04.2020 14:20, Julien Grall wrote:
> Hi,
> On 20/04/2020 13:12, Jan Beulich wrote:
>> On 20.04.2020 14:08, Julien Grall wrote:
>>> On 20/04/2020 08:26, Jan Beulich wrote:
>>>> On 17.04.2020 19:13, Julien Grall wrote:
>>>>> FWIW, the different matters on Arm. Although, it looks like the
>>>>> compiler will not warn you if you are using the wrong handler :(.
>>>> I find this highly suspicious, but can't check myself until back
>>>> in the office - these are distinct compound types after all, so
>>>> this shouldn't just be a warning, but an error. Or did you merely
>>>> mean there's no warning on x86?
>>> I mean on Arm 32-bit. I have changed one of the function to use 
>>> XEN_GUEST_HANDLE_PARAM() rather than XEN_GUEST_HANDLE() but not changing 
>>> the caller.
>>> It is probably because they are both defined using an union. Interestly, 
>>> the type will also not be checked, so the code a function will happily 
>>> accept a XEN_GUEST_HANDLE_PARAM(uint8) even if the prototype requested 
>>> This looks rather messy, maybe we should use a structure (and some 
>>> alignment) to add more safety.
>> Are the unions plain ones? I could see room for behavior like
>> the one you describe with transparent unions, albeit still
>> not quite like you describe it. Getting handle types to be
>> properly type-checked by the compiler is pretty imperative imo.
> It looks like x86 is using structure, but arm is using plain union:
> #define ___DEFINE_XEN_GUEST_HANDLE(name, type)                  \
>     typedef union { type *p; unsigned long q; }                 \
>         __guest_handle_ ## name;                                \
>     typedef union { type *p; uint64_aligned_t q; }              \
>         __guest_handle_64_ ## name

I don't see how this would make a difference, and hence ...

> I will look at introducing a union on Arm.

... how this would help. I must be missing something, or there
must be a very curious bug in only the Arm gcc.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.