Re: [Xen-devel] [PATCH v2 1/3] xen/arm: fix buf size in make_cpus_node

[Julien Grall <Julien.Grall@arm.com>]

Hi Stefano,

On 08/10/2019 22:18, Stefano Stabellini wrote:
> On Tue, 8 Oct 2019, Julien Grall wrote:
>> On 10/8/19 2:14 AM, Stefano Stabellini wrote:
>>> The size of buf is calculated wrongly: the number is 64bit, not 32bit.
>>
>> While the variable mpdir_aff is 64-bit, we only write the first 32-bit in the
>> property reg (#address-cells == 1 and fdt_property_cell()). So what needs to
>> be modified is the format here.
>>
>> Also, looking the CPU bindings (see
>> linux/Documentation/devicetree/bindings/arm/cpus.yaml), technically only the
>> bits [23:0] of the mpidr should be used. The rest is zeroed.
>>
>> This is ok because vcpuid_to_vaffinity() is always returning a value following
>> the requirements above. However, for correctness, this may want to be fixed.
> 
> It looks like it would be best to change mpdir_aff to uint32_t and
> change vcpuid_to_vaffinity to return a uint32_t.

vcpuid_to_vaffinity is meant to return the AFFx bits of the MIDR (so 
32-bit on Arm32 and 64-bit on Arm64). We are only using AFF0 and AFF1, 
so the rest is zeroed. But this does not mean we should switch to 32-bit.

If we want to change the interface then, it should be register_t and not 
32-bit.

However, I didn't suggest to switch to 32-bit but to transfer the bits 
[23:0] to a variable and possibly check that the rest is 0.

Maybe something like:

uint32_t reg;

reg = mpidr_aff & GENMASK(23, 0);
/* We only are able to deal with AFF{0, 1, 2} stored in bits [23:0] at 
the moment */
if ( reg != mpidr_aff )
{
   printk(XENLOG_ERR "Unable to handle MPIDR AFFINITY 0x%"PRIx64"\n", 
mpidr_aff);
   return -EINVAL;
}

Cheers,

-- 
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel