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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/08/17 09:18, Sergej Proskurin wrote:
> 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 :)

Oh - that's unfortunate.  Never mind then.

~Andrew

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