Hi Peter, On Fri, Nov 08, 2024 at 07:16:50PM +0000, Peter Maydell wrote: > Date: Fri, 8 Nov 2024 19:16:50 +0000 > From: Peter Maydell <[email protected]> > Subject: Re: [PULL 10/29] hw/core: Check smp cache topology support for > machine > > 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' ?
Thank you! My fault and you're right, here I should use "props->cache" instead of "node->value->cache". I'll submit a patch to fix this. Regards, Zhao
