| Message ID | 20210413080745.33004-4-wangyanan55@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/virt: Introduce cpu topology support | expand |
On Tue, Apr 13, 2021 at 04:07:42PM +0800, Yanan Wang wrote: > From: Ying Fang <fangying1@huawei.com> > > When building ACPI tables regarding CPUs we should always build > them for the number of possible CPUs, not the number of present > CPUs. We then ensure only the present CPUs are enabled in MADT. > Furthermore, it is also needed if we are going to support CPU > hotplug in the future. > > This patch is a rework based on Andrew Jones's contribution at > https://lists.gnu.org/archive/html/qemu-arm/2018-07/msg00076.html > > Signed-off-by: Ying Fang <fangying1@huawei.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/arm/virt-acpi-build.c | 14 ++++++++++---- > hw/arm/virt.c | 3 +++ > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index f5a2b2d4cb..2ad5dad1bf 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -61,13 +61,16 @@ > > static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) > { > - MachineState *ms = MACHINE(vms); > + CPUArchIdList *possible_cpus = MACHINE(vms)->possible_cpus; > uint16_t i; > > - for (i = 0; i < ms->smp.cpus; i++) { > + for (i = 0; i < possible_cpus->len; i++) { > Aml *dev = aml_device("C%.03X", i); > aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007"))); > aml_append(dev, aml_name_decl("_UID", aml_int(i))); > + if (possible_cpus->cpus[i].cpu == NULL) { > + aml_append(dev, aml_name_decl("_STA", aml_int(0))); > + } > aml_append(scope, dev); > } > } > @@ -479,6 +482,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > const int *irqmap = vms->irqmap; > AcpiMadtGenericDistributor *gicd; > AcpiMadtGenericMsiFrame *gic_msi; > + CPUArchIdList *possible_cpus = MACHINE(vms)->possible_cpus; > int i; > > acpi_data_push(table_data, sizeof(AcpiMultipleApicTable)); > @@ -489,7 +493,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base); > gicd->version = vms->gic_version; > > - for (i = 0; i < MACHINE(vms)->smp.cpus; i++) { > + for (i = 0; i < possible_cpus->len; i++) { > AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data, > sizeof(*gicc)); > ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i)); > @@ -504,7 +508,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > gicc->cpu_interface_number = cpu_to_le32(i); > gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity); > gicc->uid = cpu_to_le32(i); > - gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED); > + if (possible_cpus->cpus[i].cpu != NULL) { > + gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED); > + } > > if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) { > gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ)); > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index f4ae60ded9..3e5d9b6f26 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -2063,6 +2063,9 @@ static void machvirt_init(MachineState *machine) > } > > qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > + > + /* Initialize cpu member here since cpu hotplug is not supported yet */ > + machine->possible_cpus->cpus[n].cpu = cpuobj; Can drop the 'machine->' as possible_cpus is already set to the pointer. > object_unref(cpuobj); > } > fdt_add_timer_nodes(vms); > -- > 2.19.1 > Thanks, drew
On Tue, Apr 13, 2021 at 04:07:42PM +0800, Yanan Wang wrote: > From: Ying Fang <fangying1@huawei.com> > > When building ACPI tables regarding CPUs we should always build > them for the number of possible CPUs, not the number of present > CPUs. We then ensure only the present CPUs are enabled in MADT. > Furthermore, it is also needed if we are going to support CPU > hotplug in the future. > > This patch is a rework based on Andrew Jones's contribution at > https://lists.gnu.org/archive/html/qemu-arm/2018-07/msg00076.html Thank you for this credit, but I think I'd prefer a Co-developed-by tag instead, if you don't mind. Thanks, drew
On 2021/4/27 21:18, Andrew Jones wrote: > On Tue, Apr 13, 2021 at 04:07:42PM +0800, Yanan Wang wrote: >> From: Ying Fang <fangying1@huawei.com> >> >> When building ACPI tables regarding CPUs we should always build >> them for the number of possible CPUs, not the number of present >> CPUs. We then ensure only the present CPUs are enabled in MADT. >> Furthermore, it is also needed if we are going to support CPU >> hotplug in the future. >> >> This patch is a rework based on Andrew Jones's contribution at >> https://lists.gnu.org/archive/html/qemu-arm/2018-07/msg00076.html >> >> Signed-off-by: Ying Fang <fangying1@huawei.com> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> --- >> hw/arm/virt-acpi-build.c | 14 ++++++++++---- >> hw/arm/virt.c | 3 +++ >> 2 files changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >> index f5a2b2d4cb..2ad5dad1bf 100644 >> --- a/hw/arm/virt-acpi-build.c >> +++ b/hw/arm/virt-acpi-build.c >> @@ -61,13 +61,16 @@ >> >> static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) >> { >> - MachineState *ms = MACHINE(vms); >> + CPUArchIdList *possible_cpus = MACHINE(vms)->possible_cpus; >> uint16_t i; >> >> - for (i = 0; i < ms->smp.cpus; i++) { >> + for (i = 0; i < possible_cpus->len; i++) { >> Aml *dev = aml_device("C%.03X", i); >> aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007"))); >> aml_append(dev, aml_name_decl("_UID", aml_int(i))); >> + if (possible_cpus->cpus[i].cpu == NULL) { >> + aml_append(dev, aml_name_decl("_STA", aml_int(0))); >> + } >> aml_append(scope, dev); >> } >> } >> @@ -479,6 +482,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >> const int *irqmap = vms->irqmap; >> AcpiMadtGenericDistributor *gicd; >> AcpiMadtGenericMsiFrame *gic_msi; >> + CPUArchIdList *possible_cpus = MACHINE(vms)->possible_cpus; >> int i; >> >> acpi_data_push(table_data, sizeof(AcpiMultipleApicTable)); >> @@ -489,7 +493,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >> gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base); >> gicd->version = vms->gic_version; >> >> - for (i = 0; i < MACHINE(vms)->smp.cpus; i++) { >> + for (i = 0; i < possible_cpus->len; i++) { >> AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data, >> sizeof(*gicc)); >> ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i)); >> @@ -504,7 +508,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >> gicc->cpu_interface_number = cpu_to_le32(i); >> gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity); >> gicc->uid = cpu_to_le32(i); >> - gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED); >> + if (possible_cpus->cpus[i].cpu != NULL) { >> + gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED); >> + } >> >> if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) { >> gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ)); >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index f4ae60ded9..3e5d9b6f26 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -2063,6 +2063,9 @@ static void machvirt_init(MachineState *machine) >> } >> >> qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); >> + >> + /* Initialize cpu member here since cpu hotplug is not supported yet */ >> + machine->possible_cpus->cpus[n].cpu = cpuobj; > Can drop the 'machine->' as possible_cpus is already set to the pointer. Right, I will drop it. Thanks, Yanan >> object_unref(cpuobj); >> } >> fdt_add_timer_nodes(vms); >> -- >> 2.19.1 >> > Thanks, > drew > > > .
Hi Drew, On 2021/4/27 22:50, Andrew Jones wrote: > On Tue, Apr 13, 2021 at 04:07:42PM +0800, Yanan Wang wrote: >> From: Ying Fang <fangying1@huawei.com> >> >> When building ACPI tables regarding CPUs we should always build >> them for the number of possible CPUs, not the number of present >> CPUs. We then ensure only the present CPUs are enabled in MADT. >> Furthermore, it is also needed if we are going to support CPU >> hotplug in the future. >> >> This patch is a rework based on Andrew Jones's contribution at >> https://lists.gnu.org/archive/html/qemu-arm/2018-07/msg00076.html > Thank you for this credit, but I think I'd prefer a Co-developed-by > tag instead, if you don't mind. No problem. I will add your Co-developed-by tag and you deserve that. There may still be something inappropriate about credit in this series, but I will try to make things all right. Thanks, Yanan > Thanks, > drew > > .
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index f5a2b2d4cb..2ad5dad1bf 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -61,13 +61,16 @@ static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) { - MachineState *ms = MACHINE(vms); + CPUArchIdList *possible_cpus = MACHINE(vms)->possible_cpus; uint16_t i; - for (i = 0; i < ms->smp.cpus; i++) { + for (i = 0; i < possible_cpus->len; i++) { Aml *dev = aml_device("C%.03X", i); aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007"))); aml_append(dev, aml_name_decl("_UID", aml_int(i))); + if (possible_cpus->cpus[i].cpu == NULL) { + aml_append(dev, aml_name_decl("_STA", aml_int(0))); + } aml_append(scope, dev); } } @@ -479,6 +482,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) const int *irqmap = vms->irqmap; AcpiMadtGenericDistributor *gicd; AcpiMadtGenericMsiFrame *gic_msi; + CPUArchIdList *possible_cpus = MACHINE(vms)->possible_cpus; int i; acpi_data_push(table_data, sizeof(AcpiMultipleApicTable)); @@ -489,7 +493,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base); gicd->version = vms->gic_version; - for (i = 0; i < MACHINE(vms)->smp.cpus; i++) { + for (i = 0; i < possible_cpus->len; i++) { AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data, sizeof(*gicc)); ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i)); @@ -504,7 +508,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) gicc->cpu_interface_number = cpu_to_le32(i); gicc->arm_mpidr = cpu_to_le64(armcpu->mp_affinity); gicc->uid = cpu_to_le32(i); - gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED); + if (possible_cpus->cpus[i].cpu != NULL) { + gicc->flags = cpu_to_le32(ACPI_MADT_GICC_ENABLED); + } if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) { gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ)); diff --git a/hw/arm/virt.c b/hw/arm/virt.c index f4ae60ded9..3e5d9b6f26 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2063,6 +2063,9 @@ static void machvirt_init(MachineState *machine) } qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); + + /* Initialize cpu member here since cpu hotplug is not supported yet */ + machine->possible_cpus->cpus[n].cpu = cpuobj; object_unref(cpuobj); } fdt_add_timer_nodes(vms);