Re: [PATCH 13/16] target/riscv/kvm.c: add multi-letter extension KVM properties

[Andrew Jones]

On Wed, Jun 07, 2023 at 04:59:02PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 6/7/23 08:48, Andrew Jones wrote:
> > On Tue, May 30, 2023 at 04:46:20PM -0300, Daniel Henrique Barboza wrote:
> > > Let's add KVM user properties for the multi-letter extensions that KVM
> > > currently supports: zicbom, zicboz, zihintpause, zbb, ssaia, sstc,
> > > svinval and svpbmt.
> > > 
> > > As with the recently added MISA properties we're also going to add a
> > > 'user_set' flag in each of them. The flag will be set only if the user
> > > chose an option that's different from the host and will require extra
> > > handling from the KVM driver.
> > > 
> > > However, multi-letter CPUs have more cases to cover than MISA
> > > extensions, so we're adding an extra 'supported' flag as well. This flag
> > > will reflect if a given extension is supported by KVM, i.e. KVM knows
> > > how to handle it. This is determined during KVM extension discovery in
> > > kvm_riscv_init_multiext_cfg(), where we test for EINVAL errors. Any
> > > other error different from EINVAL will cause an abort.
> > 
> > I wish that was ENOENT, but I suppose that ship sailed.
> > 
> > > 
> > > The 'supported' flag will then be used later on to give an exception for
> > > users that are disabling multi-letter extensions that are unknown to
> > > KVM.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > > ---
> > >   target/riscv/kvm.c | 136 +++++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 136 insertions(+)
> > > 
> > > diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> > > index bb1dafe263..b4193a10d8 100644
> > > --- a/target/riscv/kvm.c
> > > +++ b/target/riscv/kvm.c
> > > @@ -202,6 +202,99 @@ static void kvm_riscv_update_cpu_misa_ext(RISCVCPU 
> > > *cpu, CPUState *cs)
> > >       }
> > >   }
> > > +typedef struct RISCVCPUMultiExtConfig {
> > > +    const char *name;
> > 
> > No description? I'd prefer we use the same cfg struct for single-letter
> > and multi-letter extensions. We can use a union to overlap cpu_cfg_offset
> > and misa_bit.
> 
> multi-letter extensions don't have a 'description' field in TCG. Nothing
> prevents us from adding for KVM though.

KVM can wait for descriptions until after adding them to TCG, and then,
hopefully, figuring out how to share them with KVM.

Thanks,
drew