RE: [PATCH v3 00/10] kvm/arm: Introduce a customizable aarch64 KVM host model

[Shameerali Kolothum Thodi via]

> -----Original Message-----
> From: Cornelia Huck <coh...@redhat.com>
> Sent: Tuesday, June 3, 2025 4:15 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.th...@huawei.com>; eric.auger....@gmail.com;
> eric.au...@redhat.com; qemu-devel@nongnu.org; qemu-...@nongnu.org;
> kvm...@lists.linux.dev; peter.mayd...@linaro.org;
> richard.hender...@linaro.org; alex.ben...@linaro.org; m...@kernel.org;
> oliver.up...@linux.dev; seb...@redhat.com; arm...@redhat.com;
> berra...@redhat.com; abolo...@redhat.com; jdene...@redhat.com
> Cc: ag...@csgraf.de; shahu...@redhat.com; mark.rutl...@arm.com;
> phi...@linaro.org; pbonz...@redhat.com
> Subject: RE: [PATCH v3 00/10] kvm/arm: Introduce a customizable aarch64
> KVM host model
> 
> On Tue, May 27 2025, Cornelia Huck <coh...@redhat.com> wrote:
> 
> > On Mon, May 26 2025, Cornelia Huck <coh...@redhat.com> wrote:
> >
> >> On Fri, May 23 2025, Shameerali Kolothum Thodi
> <shameerali.kolothum.th...@huawei.com> wrote:
> >>
> >>> Hi,
> >>>
> >>>> -----Original Message-----
> >>>> From: Cornelia Huck <coh...@redhat.com>
> >>>> Sent: Monday, April 14, 2025 5:39 PM
> >>>> To: eric.auger....@gmail.com; eric.au...@redhat.com; qemu-
> >>>> de...@nongnu.org; qemu-...@nongnu.org; kvm...@lists.linux.dev;
> >>>> peter.mayd...@linaro.org; richard.hender...@linaro.org;
> >>>> alex.ben...@linaro.org; m...@kernel.org; oliver.up...@linux.dev;
> >>>> seb...@redhat.com; Shameerali Kolothum Thodi
> >>>> <shameerali.kolothum.th...@huawei.com>; arm...@redhat.com;
> >>>> berra...@redhat.com; abolo...@redhat.com;
> jdene...@redhat.com
> >>>> Cc: ag...@csgraf.de; shahu...@redhat.com; mark.rutl...@arm.com;
> >>>> phi...@linaro.org; pbonz...@redhat.com; Cornelia Huck
> >>>> <coh...@redhat.com>
> >>>> Subject: [PATCH v3 00/10] kvm/arm: Introduce a customizable aarch64
> >>>> KVM host model
> >>>
> >>> [..]
> >>>
> >>> )
> >>>>
> >>>> Code also available at
> >>>> https://gitlab.com/cohuck/qemu/-/tree/arm-cpu-model-
> >>>> rfcv3?ref_type=heads
> >>>
> >>> I had a spin with the above branch, but Qemu boot fails,
> >>>
> >>> ERROR:../target/arm/cpu64.c:57:get_sysreg_idx: code should not be
> >>> reached Bail out! ERROR:../target/arm/cpu64.c:57:get_sysreg_idx:
> >>> code should not be reached
> >>>
> >>> From a quick debug, it looks like the below path results in an invalid ID
> idx.
> >>>
> >>> kvm_arm_expose_idreg_properties()
> >>>  kvm_idx_to_idregs_idx(0)
> >>>   get_sysreg_idx(0xc000)  --> id_register seems to start at 0xc008
> >>>
> >>> Haven't debugged further.
> >>>
> >>> I am running against a 6.15-rc1 kernel after updating the Qemu
> >>> branch by, ./update-aarch64-sysreg-code.sh  path_to_6.15-rc1
> >>>
> >>> Not sure I am  missing anything. Please check and let me know.
> >>
> >> Thanks for trying this out; I'll try to re-create this here.
> >> (I think I've messed up those conversion functions often enough...)
> >
> > The conversion functions are not at fault here, but we're missing
> > registers. If we have MIDR and friends writable, they show up in the
> > masks returned by the kernel, but they are not present in the kernel's
> > sysreg file where we generate our definitions from, and
> > kvm_idx_to_idregs_idx() asserts instead of returning an error, which
> > is kind of suboptimal...
> >
> > So I see two possible ways to fix this:
> > - add MIDR and friends to the kernel's sysreg file
> > - add MIDR and friends in QEMU's cpu-sysregs.h.inc file, and only append
> >   generated definitions there
> >
> > First option means one more round trip, second options has more
> > potential for messing things up if we keep stuff local to QEMU.
> 
> With the patch below, things work for me with a 6.15+ kernel. It's a bit

Yes works for me too now. Thanks.

> messy, though, and raises questions (how do we want to handle those regs
> across accelerators, for example, or how we can make sure that the code is
> more robust when registers are added.)
> 
> My biggest question, however, is how this interacts with the framework to
> provide lists of MIDR/REVIDR/AIDR for errata management. The hack below
> adds properties to configure those regs, I guess we'd want to suppress
> adding the props in order to avoid conflicts.

Not sure how this impacts the errata management though. My initial take on
this was, user will provide a list of target CPU ids through command line and
that will be used to set the target CPUs for errata management(if kernel
supports it).

Eg:
-machine virt,.., x-target-impl-cpus=0xMIDR1:0xREVIDR1-0xMIDR2:REVIDR2

And these will be stored in,

#define MAX_TARGET_IMPL_CPUS    4
typedef struct TargetImplCpu {
     uint32_t midr;
     uint32_t revidr;
} TargetImplCpu;


Please see the initial (a hack for testing kernel) implementation here,
https://github.com/hisilicon/qemu/commit/a393c1180274c73d34f32eaab66764a874a9ad31

Please let me know if there is a better/preferred way of obtaining this
target CPU list from user.

Thanks,
Shameer