| Message ID | 1446285001-7316-1-git-send-email-zhaoshenglong@huawei.com |
|---|---|
| State | New |
| Headers | show |
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? Regards, Peter > gicc->uid = i; > if (test_bit(i, cpuinfo->found_cpus)) { > gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED); > -- > 2.0.4 > > >
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.) thanks -- PMM
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
On 3 November 2015 at 04:33, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: > 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. The difficulty with that is that to support KVM we need to let KVM (ie the CPU object) override the board's ideas about mpidr, because the kernel doesn't yet support letting the board model inform it about what mpidr values to use. So we probably need to have 'board model sets cpu property, everything else reads cpu property which might or might not be what the board hoped for'. thanks -- PMM
On 31 October 2015 at 09:50, 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 > Applied to target-arm.next, thanks. -- PMM
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; gicc->uid = i; if (test_bit(i, cpuinfo->found_cpus)) { gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED);