Re: [RFC 12/52] hw/acpi: Replace MachineState.smp access with topology helpers

[Zhao Liu]

On Thu, Feb 16, 2023 at 05:31:11PM +0800, wangyanan (Y) wrote:
> Date: Thu, 16 Feb 2023 17:31:11 +0800
> From: "wangyanan (Y)" <wangyanan55@huawei.com>
> Subject: Re: [RFC 12/52] hw/acpi: Replace MachineState.smp access with
>  topology helpers
> 
> Hi Zhao,
> 
> 在 2023/2/13 17:49, Zhao Liu 写道:
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > At present, in QEMU only arm needs PPTT table to build cpu topology.
> > 
> > Before QEMU's arm supports hybrid architectures, it's enough to limit
> > the cpu topology of PPTT to smp type through the explicit smp interface
> > (machine_topo_get_smp_threads()).
> > 
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Igor Mammedov <imammedo@redhat.com>
> > Cc: Ani Sinha <ani@anisinha.ca>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >   hw/acpi/aml-build.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index ea331a20d131..693bd8833d10 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -2044,7 +2044,7 @@ void build_pptt(GArray *table_data, BIOSLinker 
> > *linker, MachineState *ms,
> >               cluster_offset = socket_offset;
> >           }
> > -        if (ms->smp.threads == 1) {
> > +        if (machine_topo_get_smp_threads(ms) == 1) {
> >               build_processor_hierarchy_node(table_data,
> >                   (1 << 1) | /* ACPI Processor ID valid */
> >                   (1 << 3),  /* Node is a Leaf */
> ACPI PPTT table is designed to also support the hybrid CPU topology
> case where nodes on the same CPU topology level can have different
> number of child nodes.
> 
> So to be general, the diff should be:
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ea331a20d1..dfded95bbc 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2044,7 +2044,7 @@ void build_pptt(GArray *table_data, BIOSLinker
> *linker, MachineState *ms,
>              cluster_offset = socket_offset;
>          }
> 
> -        if (ms->smp.threads == 1) {
> +        if (machine_topo_get_threads_by_idx(n) == 1) {
>              build_processor_hierarchy_node(table_data,
>                  (1 << 1) | /* ACPI Processor ID valid */
>                  (1 << 3),  /* Node is a Leaf */

Nice! I'll replace that.

> 
> Actually I'm recently working on ARM hmp virtualization which relys on
> PPTT for topology representation, so we will also need PPTT to be general
> for hybrid case anyway.

Good to know that you are considering hybrid support for arm.
BTW, I explained the difference between arm and x86's hybrid in previous
email [1] [2], mainly about whether the cpm model is the same.

I tentatively think that this difference can be solved by arch-specific
coretype(). Do you have any comments on this? Thanks!

[1]: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03884.html
[2]: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03789.html

> 
> Thanks,
> Yanan