Re: [Qemu-devel] [Qemu-arm] [PATCH] ARM: ACPI: Fix MPIDR value in ACPI table

[Peter Crosthwaite <crosthwaitepeter@gmail.com>]

On Sat, Oct 31, 2015 at 5:04 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 31 October 2015 at 18:53, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sat, Oct 31, 2015 at 2:50 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Use mp_affinity of ARMCPU as the CPU MPIDR instead of the CPU index.
>>>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> ---
>>> This patch is based on below patch.
>>> http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg06919.html
>>>
>>>  hw/arm/virt-acpi-build.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>> index 29a1980..1c621cb 100644
>>> --- a/hw/arm/virt-acpi-build.c
>>> +++ b/hw/arm/virt-acpi-build.c
>>> @@ -451,13 +451,15 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
>>>      for (i = 0; i < guest_info->smp_cpus; i++) {
>>>          AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data,
>>>                                                       sizeof *gicc);
>>> +        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
>>> +
>>>          gicc->type = ACPI_APIC_GENERIC_INTERRUPT;
>>>          gicc->length = sizeof(*gicc);
>>>          if (guest_info->gic_version == 2) {
>>>              gicc->base_address = memmap[VIRT_GIC_CPU].base;
>>>          }
>>>          gicc->cpu_interface_number = i;
>>> -        gicc->arm_mpidr = i;
>>> +        gicc->arm_mpidr = armcpu->mp_affinity;
>>
>> As a general rule, hw/ should not be reaching into the CPU state
>> struct like this. What is the real HW equivalent of this query?
>
> This is just doing the same thing the hw/arm/virt.c code does
> to populate the dt... (In real firmware it would presumably
> either (a) have a fixed ACPI table or (b) maybe read the MPIDR,
> but neither of those really fits here I think.)

So, I think this is just another case of the MPIDR information flow
going the wrong way. It should go from board to all of CPU, DT and now
this. I guess we can just fix this incrementally when we fix the
implicit setting of MPIDR in mach-virt.

Regards,
Peter

>
> thanks
> -- PMM