Re: [PATCH v7 13/14] arm/mem_access: Add short-descriptor based gpt

[Sergej Proskurin <proskurin@sec.in.tum.de>]

Hi Andrew,


>>> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
>>> index b258248322..7f34a2b1d3 100644
>>> --- a/xen/arch/arm/guest_walk.c
>>> +++ b/xen/arch/arm/guest_walk.c
>>> @@ -112,7 +112,12 @@ static int guest_walk_sd(const struct vcpu *v,
>>>           * level translation table does not need to be page aligned.
>>>           */
>>>          mask = GENMASK(19, 12);
>>> -        paddr = (pte.walk.base << 10) | ((gva & mask) >> 10);
>>> +        /*
>>> +         * Cast pte.walk.base to paddr_t to cope with C type promotion
>>> of types
>>> +         * smaller than int. Otherwise pte.walk.base would be casted to
>>> int and
>>> +         * subsequently sign extended, thus leading to a wrong value.
>>> +         */
>>> +        paddr = ((paddr_t)pte.walk.base << 10) | ((gva & mask) >> 10);
>> Why not change the bitfield type from unsigned int to paddr_t ?
>>
>> The result is 100% less liable to go wrong in this way.
>>

Actually, AFAICT we would get into same troubles as before. Because of
the fact that the bitfield is smaller than an int (22 bit), it would be
first promoted to int and then we would face the same issues as we
already had.

If that is ok for you, I will resubmit the next patch without changing
the type of the bitfield. If you should not agree with me, I would
gladly discuss this issue in v8 :)

Thanks,
~Sergej


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel