| Message ID | 20230926100436.28284-23-salil.mehta@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 9/26/23 20:04, Salil Mehta wrote: > From: Jean-Philippe Brucker <jean-philippe@linaro.org> > > The GICC interface on arm64 vCPUs is statically defined in the MADT, and > doesn't require a _MAT entry. Although the GICC is indicated as present > by the MADT entry, it can only be used from vCPU sysregs, which aren't > accessible until hot-add. > > Co-developed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Co-developed-by: Jonathan Cameron <jonathan.cameron@huawei.com> > Signed-off-by: Jonathan Cameron <jonathan.cameron@huawei.com> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > --- > hw/acpi/cpu.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > With following nits addressed: Reviewed-by: Gavin Shan <gshan@redhat.com> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > index e1299696d3..217db99538 100644 > --- a/hw/acpi/cpu.c > +++ b/hw/acpi/cpu.c > @@ -715,11 +715,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > aml_append(dev, method); > > /* build _MAT object */ > - assert(adevc && adevc->madt_cpu); > - adevc->madt_cpu(i, arch_ids, madt_buf, > - true); /* set enabled flag */ > - aml_append(dev, aml_name_decl("_MAT", > - aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); > + if (adevc && adevc->madt_cpu) { > + assert(adevc && adevc->madt_cpu); > + adevc->madt_cpu(i, arch_ids, madt_buf, > + true); /* set enabled flag */ > + aml_append(dev, aml_name_decl("_MAT", > + aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); > + } > g_array_free(madt_buf, true); > > if (CPU(arch_ids->cpus[i].cpu) != first_cpu) { May be worthy to have comment to mention _MAT isn't needed on aarch64. /* Build _MAT object, which isn't needed by aarch64 */ Thanks, Gavin
Hi Gavin, > From: Gavin Shan <gshan@redhat.com> > Sent: Friday, September 29, 2023 12:50 AM > To: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org; qemu-arm@nongnu.org > Cc: maz@kernel.org; jean-philippe@linaro.org; Jonathan Cameron > <jonathan.cameron@huawei.com>; lpieralisi@kernel.org; > peter.maydell@linaro.org; richard.henderson@linaro.org; > imammedo@redhat.com; andrew.jones@linux.dev; david@redhat.com; > philmd@linaro.org; eric.auger@redhat.com; will@kernel.org; ardb@kernel.org; > oliver.upton@linux.dev; pbonzini@redhat.com; mst@redhat.com; > rafael@kernel.org; borntraeger@linux.ibm.com; alex.bennee@linaro.org; > linux@armlinux.org.uk; darren@os.amperecomputing.com; > ilkka@os.amperecomputing.com; vishnu@os.amperecomputing.com; > karl.heubaum@oracle.com; miguel.luis@oracle.com; salil.mehta@opnsrc.net; > zhukeqian <zhukeqian1@huawei.com>; wangxiongfeng (C) > <wangxiongfeng2@huawei.com>; wangyanan (Y) <wangyanan55@huawei.com>; > jiakernel2@gmail.com; maobibo@loongson.cn; lixianglai@loongson.cn > Subject: Re: [PATCH RFC V2 22/37] hw/acpi: Make _MAT method optional > > On 9/26/23 20:04, Salil Mehta wrote: > > From: Jean-Philippe Brucker <jean-philippe@linaro.org> > > > > The GICC interface on arm64 vCPUs is statically defined in the MADT, and > > doesn't require a _MAT entry. Although the GICC is indicated as present > > by the MADT entry, it can only be used from vCPU sysregs, which aren't > > accessible until hot-add. > > > > Co-developed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > Co-developed-by: Jonathan Cameron <jonathan.cameron@huawei.com> > > Signed-off-by: Jonathan Cameron <jonathan.cameron@huawei.com> > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com> > > --- > > hw/acpi/cpu.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > With following nits addressed: > > Reviewed-by: Gavin Shan <gshan@redhat.com> Thanks. > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > > index e1299696d3..217db99538 100644 > > --- a/hw/acpi/cpu.c > > +++ b/hw/acpi/cpu.c > > @@ -715,11 +715,13 @@ void build_cpus_aml(Aml *table, MachineState > *machine, CPUHotplugFeatures opts, > > aml_append(dev, method); > > > > /* build _MAT object */ > > - assert(adevc && adevc->madt_cpu); > > - adevc->madt_cpu(i, arch_ids, madt_buf, > > - true); /* set enabled flag */ > > - aml_append(dev, aml_name_decl("_MAT", > > - aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); > > + if (adevc && adevc->madt_cpu) { > > + assert(adevc && adevc->madt_cpu); > > + adevc->madt_cpu(i, arch_ids, madt_buf, > > + true); /* set enabled flag */ > > + aml_append(dev, aml_name_decl("_MAT", > > + aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); > > + } > > g_array_free(madt_buf, true); > > > > if (CPU(arch_ids->cpus[i].cpu) != first_cpu) { > > May be worthy to have comment to mention _MAT isn't needed on aarch64. > > /* Build _MAT object, which isn't needed by aarch64 */ This file is not an architecture specific file so not a good idea to mention above. Thanks Salil.
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index e1299696d3..217db99538 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -715,11 +715,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_append(dev, method); /* build _MAT object */ - assert(adevc && adevc->madt_cpu); - adevc->madt_cpu(i, arch_ids, madt_buf, - true); /* set enabled flag */ - aml_append(dev, aml_name_decl("_MAT", - aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); + if (adevc && adevc->madt_cpu) { + assert(adevc && adevc->madt_cpu); + adevc->madt_cpu(i, arch_ids, madt_buf, + true); /* set enabled flag */ + aml_append(dev, aml_name_decl("_MAT", + aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); + } g_array_free(madt_buf, true); if (CPU(arch_ids->cpus[i].cpu) != first_cpu) {