Re: [PATCH 4/8] hw/core: Check smp cache topology support for machine

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu,  4 Jul 2024 11:15:59 +0800
Zhao Liu <zhao1.liu@intel.com> wrote:

> Add cache_supported flags in SMPCompatProps to allow machines to
> configure various caches support.
> 
> And implement check() method for machine's "smp-cache" link property,
> which will check the compatibility of the cache properties with the
> machine support.

One question inline.

> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes since RFC v2:
>  * Split as a separate commit to just include compatibility checking and
>    topology checking.
>  * Allow setting "default" topology level even though the cache
>    isn't supported by machine. (Daniel)
> ---
>  hw/core/machine-smp.c | 80 +++++++++++++++++++++++++++++++++++++++++++
>  hw/core/machine.c     | 17 +++++++--
>  include/hw/boards.h   |  6 ++++
>  3 files changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> index 88a73743eb1c..bf6f2f91070d 100644
> --- a/hw/core/machine-smp.c
> +++ b/hw/core/machine-smp.c
> @@ -276,3 +276,83 @@ CpuTopologyLevel machine_get_cache_topo_level(const MachineState *ms,
>  {
>      return ms->smp_cache->props[cache].topo;
>  }
> +
> +static bool machine_check_topo_support(MachineState *ms,
> +                                       CpuTopologyLevel topo,
> +                                       Error **errp)
> +{
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> +    if ((topo == CPU_TOPO_LEVEL_MODULE && !mc->smp_props.modules_supported) ||

I can sort of understand why they will always be supported but why not include
core and socket in here just to avoid the complexity of wondering if they are?

> +        (topo == CPU_TOPO_LEVEL_CLUSTER && !mc->smp_props.clusters_supported) ||
> +        (topo == CPU_TOPO_LEVEL_DIE && !mc->smp_props.dies_supported) ||
> +        (topo == CPU_TOPO_LEVEL_BOOK && !mc->smp_props.books_supported) ||
> +        (topo == CPU_TOPO_LEVEL_DRAWER && !mc->smp_props.drawers_supported)) {
> +        error_setg(errp,
> +                   "Invalid topology level: %s. "
> +                   "The topology level is not supported by this machine",
> +                   CpuTopologyLevel_str(topo));
> +        return false;
> +    }
> +
> +    return true;
> +}