On Tue, 5 Nov 2024 at 22:49, Philippe Mathieu-Daudé <[email protected]> wrote:
>
> From: Zhao Liu <[email protected]>
>
> Add cache_supported flags in SMPCompatProps to allow machines to
> configure various caches support.
>
> And check the compatibility of the cache properties with the
> machine support in machine_parse_smp_cache().

Hi; Coverity points out an issue in this code (CID 1565391):

>  bool machine_parse_smp_cache(MachineState *ms,
>                               const SmpCachePropertiesList *caches,
>                               Error **errp)
>  {
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
>      const SmpCachePropertiesList *node;
>      DECLARE_BITMAP(caches_bitmap, CACHE_LEVEL_AND_TYPE__MAX);
>
> @@ -283,6 +305,25 @@ bool machine_parse_smp_cache(MachineState *ms,
>          set_bit(node->value->cache, caches_bitmap);
>      }
>
> +    for (int i = 0; i < CACHE_LEVEL_AND_TYPE__MAX; i++) {
> +        const SmpCacheProperties *props = &ms->smp_cache.props[i];
> +
> +        /*
> +         * Reject non "default" topology level if the cache isn't
> +         * supported by the machine.
> +         */
> +        if (props->topology != CPU_TOPOLOGY_LEVEL_DEFAULT &&
> +            !mc->smp_props.cache_supported[props->cache]) {
> +            error_setg(errp,
> +                       "%s cache topology not supported by this machine",
> +                       CacheLevelAndType_str(node->value->cache));

This error message looks at "node", but "node" was the iteration
variable in the first loop in this function, so generally at
this point it will be NULL because that is the loop termination
condition.

The code looks like it ought to be reporting an error relating
to the 'props' variable, not 'node' ?

> +            return false;
> +        }
> +
> +        if (!machine_check_topo_support(ms, props->topology, errp)) {
> +            return false;
> +        }
> +    }
>      return true;
>  }

thanks
-- PMM

Reply via email to