Hi Peter,
On Fri, Nov 08, 2024 at 07:10:42PM +0000, Peter Maydell wrote:
> Date: Fri, 8 Nov 2024 19:10:42 +0000
> From: Peter Maydell <[email protected]>
> Subject: Re: [PULL 09/29] qapi/qom: Define cache enumeration and properties
> for machine
>
> On Tue, 5 Nov 2024 at 22:49, Philippe Mathieu-Daudé <[email protected]> wrote:
> >
> > From: Zhao Liu <[email protected]>
> >
> > The x86 and ARM need to allow user to configure cache properties
> > (current only topology):
> > * For x86, the default cache topology model (of max/host CPU) does not
> > always match the Host's real physical cache topology. Performance can
> > increase when the configured virtual topology is closer to the
> > physical topology than a default topology would be.
> > * For ARM, QEMU can't get the cache topology information from the CPU
> > registers, then user configuration is necessary. Additionally, the
> > cache information is also needed for MPAM emulation (for TCG) to
> > build the right PPTT.
> >
>
> Hi; Coverity points out an issue with this change (CID 1565389):
>
> > diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> > index 5d8d7edcbd3..c6d90cd6d41 100644
> > --- a/hw/core/machine-smp.c
> > +++ b/hw/core/machine-smp.c
> > @@ -261,6 +261,31 @@ void machine_parse_smp_config(MachineState *ms,
> > }
> > }
> >
> > +bool machine_parse_smp_cache(MachineState *ms,
> > + const SmpCachePropertiesList *caches,
> > + Error **errp)
> > +{
> > + const SmpCachePropertiesList *node;
> > + DECLARE_BITMAP(caches_bitmap, CACHE_LEVEL_AND_TYPE__MAX);
>
> DECLARE_BITMAP() defines the caches_bitmap bitmap, but it
> does not initialize it...
Yes, I missed this...
> > +
> > + for (node = caches; node; node = node->next) {
> > + /* Prohibit users from repeating settings. */
> > + if (test_bit(node->value->cache, caches_bitmap)) {
>
> ...so here we are reading the variable when it is uninitialized.
>
> If you want to zero-initialize the bitmap you can use
> bitmap_zero(caches_bitmap, CACHE_LEVEL_AND_TYPE__MAX);
Thank you for your advice! I'll submit a patch to fix this!
Regards,
Zhao
