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...
> +
> + 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);
> + error_setg(errp,
> + "Invalid cache properties: %s. "
> + "The cache properties are duplicated",
> + CacheLevelAndType_str(node->value->cache));
> + return false;
> + }
> +
> + machine_set_cache_topo_level(ms, node->value->cache,
> + node->value->topology);
> + set_bit(node->value->cache, caches_bitmap);
> + }
> +
> + return true;
> +}
thanks
-- PMM
