Hi Eric,
On Thu, Oct 29, 2026 at 10:20:21AM +0100, Eric Auger wrote:
> In KVM mode, virt_get_caches() currently gets a zero value for the
> CLIDR_EL1 register. This is due to the fact CLIDR is never read from
> host. This is a bug.
yeah, as the reviewer of f669de70011 I agree :)
> Soon we will initialize all writable isar.idreg[] from host and this
> includes CLIDR_EL1. However we cannot directly fix the initialization,
^--- comman missing
> first because it would change the way the guest is exposed caches in
"the way the guest's caches are exposed"
> DT/ACPI descriptions and because some host values would be rejected
> by the current QEMU code, expecially in the case host has a unified
^--- "especially"
> L1 cache which is currently not supported and triggers the following
> errod: "Unified type is not implemented at level 1" error.
^--- "error" ?
>
> So this issue will be fixed separately.
>
> Signed-off-by: Eric Auger <[email protected]>
> Reviewed-by: Khushit Shah <[email protected]>
>
> ---
>
> v9 -> v10
> - add a comment explaining why we oveeride the clidr value
> ---
> hw/arm/virt.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 0d56197dac..ed329e76e6 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -338,6 +338,17 @@ unsigned int virt_get_caches(const VirtMachineState
> *vms, CPUCoreCaches *caches)
> ARMISARegisters *isar = &armcpu->isar;
> uint32_t clidr = GET_IDREG(isar, CLIDR);
>
> + /*
> + * Commit f669de70011 ("hw/arm/virt: add cache hierarchy to device tree")
> + * read the CLIDR value from the isar array but in KVM mode, this index
> + * was never initialized from host. Now we want to initialize all ID
I think we should mention the bug here as well and also enhance a bit the text:
"... but in KVM mode, however, due to a bug CLIDR is never initialized correctly
from the host and we read 0. Now we want to fix the bug and initialize all ID
registers, including the CLIDR register, but until this gets fixed ..."
> + * registers * from host and until this gets fixed, we need to override
^---- stray *
> + * clidr to 0 to keep the compatibility.
> + */
> + if (kvm_enabled()) {
> + clidr = 0;
> + }
> +
> for (int cache_level = 1; cache_level <= CLIDR_CTYPE_MAX_CACHE_LEVEL;
> cache_level++) {
> uint8_t ctype =
> --
> 2.53.0
>
>
Otherwise,
Reviewed-by: Gustavo Romero <[email protected]>
Cheers,
Gustavo
