On 08/08/2017 06:20 PM, Andrew Cooper wrote:
> 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.
I absolutely agree :)
Julien, would that be ok for you if I changed the type of the base field
in short_desc_* structs accordingly? Or shall I remove your Acked-by for
Xen-devel mailing list