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.
Xen-devel mailing list