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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/08/17 16:28, Sergej Proskurin wrote:
>
> On 08/08/2017 05:18 PM, Julien Grall wrote:
>>
>> On 08/08/17 16:17, Sergej Proskurin wrote:
>>> Hi Julien,
>>>
>>>
>>> On 07/18/2017 02:25 PM, Sergej Proskurin wrote:
>>>> This commit adds functionality to walk the guest's page tables using
>>>> the
>>>> short-descriptor translation table format for both ARMv7 and ARMv8. The
>>>> implementation is based on ARM DDI 0487B-a J1-6002 and ARM DDI 0406C-b
>>>> B3-1506.
>>>>
>>>> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
>>>> Acked-by: Julien Grall <julien.grall@xxxxxxx>
>>> As you have already Acked this patch I would like you to ask whether I
>>> should remove your Acked-by for now as I have extended the previous
>>> patch by additional casts of the pte.*.base fields to (paddr_t) as
>>> discussed in patch 00/14.
>> I am fine with this, assuming this is the only change made.
> The changes are limited to 4 similar casts to (paddr_t) in total and an
> additional comment. Here are the only changes in this patch:
>
> 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.

~Andrew

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