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
>>>> 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
>>>> Signed-off-by: Sergej Proskurin <prosku...@sec.in.tum.de>
>>>> Acked-by: Julien Grall <julien.gr...@arm.com>
>>> 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.
Xen-devel mailing list