Re: [PATCH v1 01/24] LoongArch: KVM: Implement kvm module related interface

[Tianrui Zhao]

在 2023年02月14日 14:38, Greg Kroah-Hartman 写道:
> On Tue, Feb 14, 2023 at 10:56:25AM +0800, Tianrui Zhao wrote:
> > 1. Implement loongarch kvm module init, module exit interface,
> > using kvm context to save the vpid info and vcpu world switch
> > interface pointer.
> > 2. Implement kvm hardware enable, disable interface, setting
> > the guest config reg to enable virtualization features.
> > 3. Add kvm related headers.
> Shouldn't this be 3 different patches instead?
Thanks, I will separate this to 3 different patches.
> > Signed-off-by: Tianrui Zhao <zhaotianrui@xxxxxxxxxxx>
> > ---
> >   arch/loongarch/include/asm/cpu-features.h |  22 ++
> >   arch/loongarch/include/asm/kvm_host.h     | 257 ++++++++++++++++++++++
> >   arch/loongarch/include/asm/kvm_types.h    |  11 +
> >   arch/loongarch/include/uapi/asm/kvm.h     | 121 ++++++++++
> >   arch/loongarch/kvm/main.c                 | 152 +++++++++++++
> >   include/uapi/linux/kvm.h                  |  15 ++
> >   6 files changed, 578 insertions(+)
> >   create mode 100644 arch/loongarch/include/asm/kvm_host.h
> >   create mode 100644 arch/loongarch/include/asm/kvm_types.h
> >   create mode 100644 arch/loongarch/include/uapi/asm/kvm.h
> >   create mode 100644 arch/loongarch/kvm/main.c
> >
> > diff --git a/arch/loongarch/include/asm/cpu-features.h b/arch/loongarch/include/asm/cpu-features.h
> > index b07974218..23e7c3ae5 100644
> > --- a/arch/loongarch/include/asm/cpu-features.h
> > +++ b/arch/loongarch/include/asm/cpu-features.h
> > @@ -64,5 +64,27 @@
> >   #define cpu_has_guestid		cpu_opt(LOONGARCH_CPU_GUESTID)
> >   #define cpu_has_hypervisor	cpu_opt(LOONGARCH_CPU_HYPERVISOR)
> >   
> > +#define cpu_has_matc_guest	(cpu_data[0].guest_cfg & (1 << 0))
> > +#define cpu_has_matc_root	(cpu_data[0].guest_cfg & (1 << 1))
> > +#define cpu_has_matc_nest	(cpu_data[0].guest_cfg & (1 << 2))
> > +#define cpu_has_sitp		(cpu_data[0].guest_cfg & (1 << 6))
> > +#define cpu_has_titp		(cpu_data[0].guest_cfg & (1 << 8))
> > +#define cpu_has_toep		(cpu_data[0].guest_cfg & (1 << 10))
> > +#define cpu_has_topp		(cpu_data[0].guest_cfg & (1 << 12))
> > +#define cpu_has_torup		(cpu_data[0].guest_cfg & (1 << 14))
> > +#define cpu_has_gcip_all	(cpu_data[0].guest_cfg & (1 << 16))
> > +#define cpu_has_gcip_hit	(cpu_data[0].guest_cfg & (1 << 17))
> > +#define cpu_has_gcip_secure	(cpu_data[0].guest_cfg & (1 << 18))
> Why not use BIT() for all of those "<<" statements?
Ok thanks, I will use BIT() to replace those "<<" statements.
> > +#define KVM_GET_CSRS		_IOWR(KVMIO, 0xc5, struct kvm_csrs)
> > +#define KVM_SET_CSRS		_IOW(KVMIO,  0xc6, struct kvm_csrs)
> Why does this arch need new ioctls?
We want to use this ioctl to access multiple csrs at one time. If 
without this, we only access one csr.
There is another function, can we use the KVM_GET/SET_MSRS to access our 
csrs?

Thanks,
Tianrui Zhao
> thanks,
>
> greg k-h