| Message ID | 20240606074850.2651896-1-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LoongArch: KVM: Add feature passing from user space | expand |
Hi, On 6/6/24 15:48, Bibo Mao wrote: > Currently features defined in cpucfg CPUCFG_KVM_FEATURE comes from > kvm kernel mode only. Some features are defined in user space VMM, "come from kernel side only. But currently KVM is not aware of user-space VMM features which makes it hard to employ optimizations that are both (1) specific to the VM use case and (2) requiring cooperation from user space." > however KVM module does not know. Here interface is added to update > register CPUCFG_KVM_FEATURE from user space, only bit 24 - 31 is valid. > > Feature KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI is added from user mdoe. > FEAT_VIRT_EXTIOI is virt EXTIOI extension which can route interrupt > to 256 VCPUs rather than 4 CPUs like real hw. "A new feature bit ... is added which can be set from user space. FEAT_... indicates that the VM EXTIOI can route interrupts to 256 vCPUs, rather than 4 like with real HW." (Am I right in paraphrasing the "EXTIOI" part?) > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > --- > arch/loongarch/include/asm/kvm_host.h | 4 +++ > arch/loongarch/include/asm/loongarch.h | 5 ++++ > arch/loongarch/include/uapi/asm/kvm.h | 2 ++ > arch/loongarch/kvm/exit.c | 1 + > arch/loongarch/kvm/vcpu.c | 36 +++++++++++++++++++++++--- > 5 files changed, 44 insertions(+), 4 deletions(-) > > diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h > index 88023ab59486..8fa50d757247 100644 > --- a/arch/loongarch/include/asm/kvm_host.h > +++ b/arch/loongarch/include/asm/kvm_host.h > @@ -135,6 +135,9 @@ enum emulation_result { > #define KVM_LARCH_HWCSR_USABLE (0x1 << 4) > #define KVM_LARCH_LBT (0x1 << 5) > > +#define KVM_LOONGARCH_USR_FEAT_MASK \ > + BIT(KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI) > + > struct kvm_vcpu_arch { > /* > * Switch pointer-to-function type to unsigned long > @@ -210,6 +213,7 @@ struct kvm_vcpu_arch { > u64 last_steal; > struct gfn_to_hva_cache cache; > } st; > + unsigned int usr_features; > }; > > static inline unsigned long readl_sw_gcsr(struct loongarch_csrs *csr, int reg) > diff --git a/arch/loongarch/include/asm/loongarch.h b/arch/loongarch/include/asm/loongarch.h > index 7a4633ef284b..4d9837512c19 100644 > --- a/arch/loongarch/include/asm/loongarch.h > +++ b/arch/loongarch/include/asm/loongarch.h > @@ -167,9 +167,14 @@ > > #define CPUCFG_KVM_SIG (CPUCFG_KVM_BASE + 0) > #define KVM_SIGNATURE "KVM\0" > +/* > + * BIT 24 - 31 is features configurable by user space vmm > + */ > #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4) > #define KVM_FEATURE_IPI BIT(1) > #define KVM_FEATURE_STEAL_TIME BIT(2) > +/* With VIRT_EXTIOI feature, interrupt can route to 256 VCPUs */ > +#define KVM_FEATURE_VIRT_EXTIOI BIT(24) > > #ifndef __ASSEMBLY__ > What about assigning a new CPUCFG leaf ID for separating the two kinds of feature flags very cleanly? > @@ -896,7 +907,24 @@ static int kvm_loongarch_vcpu_get_attr(struct kvm_vcpu *vcpu, > static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu, > struct kvm_device_attr *attr) > { > - return -ENXIO; > + u64 __user *user = (u64 __user *)attr->addr; > + u64 val, valid_flags; > + > + switch (attr->attr) { > + case CPUCFG_KVM_FEATURE: > + if (get_user(val, user)) > + return -EFAULT; > + > + valid_flags = KVM_LOONGARCH_USR_FEAT_MASK; > + if (val & ~valid_flags) > + return -EINVAL; > + > + vcpu->arch.usr_features |= val; Isn't this usage of "|=" instead of "=" implying that the feature bits could not get disabled after being enabled earlier, for whatever reason? > + return 0; > + > + default: > + return -ENXIO; > + } > } > > static int kvm_loongarch_vcpu_set_attr(struct kvm_vcpu *vcpu, > > base-commit: 2df0193e62cf887f373995fb8a91068562784adc
Xuerui, Thanks for your reviewing. I reply inline. On 2024/6/6 下午7:20, WANG Xuerui wrote: > Hi, > > On 6/6/24 15:48, Bibo Mao wrote: >> Currently features defined in cpucfg CPUCFG_KVM_FEATURE comes from >> kvm kernel mode only. Some features are defined in user space VMM, > > "come from kernel side only. But currently KVM is not aware of > user-space VMM features which makes it hard to employ optimizations that > are both (1) specific to the VM use case and (2) requiring cooperation > from user space." Will modify in next version. > >> however KVM module does not know. Here interface is added to update >> register CPUCFG_KVM_FEATURE from user space, only bit 24 - 31 is valid. >> >> Feature KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI is added from user mdoe. >> FEAT_VIRT_EXTIOI is virt EXTIOI extension which can route interrupt >> to 256 VCPUs rather than 4 CPUs like real hw. > > "A new feature bit ... is added which can be set from user space. > FEAT_... indicates that the VM EXTIOI can route interrupts to 256 vCPUs, > rather than 4 like with real HW." will modify in next version. > > (Am I right in paraphrasing the "EXTIOI" part?) > >> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >> --- >> arch/loongarch/include/asm/kvm_host.h | 4 +++ >> arch/loongarch/include/asm/loongarch.h | 5 ++++ >> arch/loongarch/include/uapi/asm/kvm.h | 2 ++ >> arch/loongarch/kvm/exit.c | 1 + >> arch/loongarch/kvm/vcpu.c | 36 +++++++++++++++++++++++--- >> 5 files changed, 44 insertions(+), 4 deletions(-) >> >> diff --git a/arch/loongarch/include/asm/kvm_host.h >> b/arch/loongarch/include/asm/kvm_host.h >> index 88023ab59486..8fa50d757247 100644 >> --- a/arch/loongarch/include/asm/kvm_host.h >> +++ b/arch/loongarch/include/asm/kvm_host.h >> @@ -135,6 +135,9 @@ enum emulation_result { >> #define KVM_LARCH_HWCSR_USABLE (0x1 << 4) >> #define KVM_LARCH_LBT (0x1 << 5) >> +#define KVM_LOONGARCH_USR_FEAT_MASK \ >> + BIT(KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI) >> + >> struct kvm_vcpu_arch { >> /* >> * Switch pointer-to-function type to unsigned long >> @@ -210,6 +213,7 @@ struct kvm_vcpu_arch { >> u64 last_steal; >> struct gfn_to_hva_cache cache; >> } st; >> + unsigned int usr_features; >> }; >> static inline unsigned long readl_sw_gcsr(struct loongarch_csrs >> *csr, int reg) >> diff --git a/arch/loongarch/include/asm/loongarch.h >> b/arch/loongarch/include/asm/loongarch.h >> index 7a4633ef284b..4d9837512c19 100644 >> --- a/arch/loongarch/include/asm/loongarch.h >> +++ b/arch/loongarch/include/asm/loongarch.h >> @@ -167,9 +167,14 @@ >> #define CPUCFG_KVM_SIG (CPUCFG_KVM_BASE + 0) >> #define KVM_SIGNATURE "KVM\0" >> +/* >> + * BIT 24 - 31 is features configurable by user space vmm >> + */ >> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4) >> #define KVM_FEATURE_IPI BIT(1) >> #define KVM_FEATURE_STEAL_TIME BIT(2) >> +/* With VIRT_EXTIOI feature, interrupt can route to 256 VCPUs */ >> +#define KVM_FEATURE_VIRT_EXTIOI BIT(24) >> #ifndef __ASSEMBLY__ > > What about assigning a new CPUCFG leaf ID for separating the two kinds > of feature flags very cleanly? For compatible issue like new kernel on old KVM host, to add a new CPUCFG leaf ID, a new feature need be defined on existing CPUCFG_KVM_FEATURE register. Such as: #define KVM_FEATURE_EXTEND_CPUCFG BIT(3) VM need check feature KVM_FEATURE_EXTEND_CPUCFG at first, and then read the new CPUCFG leaf ID if feature EXTEND_CPUCFG is enabled. That maybe makes it complicated since feature bit is enough now. > >> @@ -896,7 +907,24 @@ static int kvm_loongarch_vcpu_get_attr(struct >> kvm_vcpu *vcpu, >> static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu, >> struct kvm_device_attr *attr) >> { >> - return -ENXIO; >> + u64 __user *user = (u64 __user *)attr->addr; >> + u64 val, valid_flags; >> + >> + switch (attr->attr) { >> + case CPUCFG_KVM_FEATURE: >> + if (get_user(val, user)) >> + return -EFAULT; >> + >> + valid_flags = KVM_LOONGARCH_USR_FEAT_MASK; >> + if (val & ~valid_flags) >> + return -EINVAL; >> + >> + vcpu->arch.usr_features |= val; > > Isn't this usage of "|=" instead of "=" implying that the feature bits > could not get disabled after being enabled earlier, for whatever reason? yes, "=" is better. Will modify in next version. Regards Bibo Mao > >> + return 0; >> + >> + default: >> + return -ENXIO; >> + } >> } >> static int kvm_loongarch_vcpu_set_attr(struct kvm_vcpu *vcpu, >> >> base-commit: 2df0193e62cf887f373995fb8a91068562784adc >
On 2024/6/6 下午7:54, maobibo wrote: > Xuerui, > > Thanks for your reviewing. > I reply inline. > > On 2024/6/6 下午7:20, WANG Xuerui wrote: >> Hi, >> >> On 6/6/24 15:48, Bibo Mao wrote: >>> Currently features defined in cpucfg CPUCFG_KVM_FEATURE comes from >>> kvm kernel mode only. Some features are defined in user space VMM, >> >> "come from kernel side only. But currently KVM is not aware of >> user-space VMM features which makes it hard to employ optimizations >> that are both (1) specific to the VM use case and (2) requiring >> cooperation from user space." > Will modify in next version. >> >>> however KVM module does not know. Here interface is added to update >>> register CPUCFG_KVM_FEATURE from user space, only bit 24 - 31 is valid. >>> >>> Feature KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI is added from user mdoe. >>> FEAT_VIRT_EXTIOI is virt EXTIOI extension which can route interrupt >>> to 256 VCPUs rather than 4 CPUs like real hw. >> >> "A new feature bit ... is added which can be set from user space. >> FEAT_... indicates that the VM EXTIOI can route interrupts to 256 >> vCPUs, rather than 4 like with real HW." > will modify in next version. > >> >> (Am I right in paraphrasing the "EXTIOI" part?) >> >>> >>> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >>> --- >>> arch/loongarch/include/asm/kvm_host.h | 4 +++ >>> arch/loongarch/include/asm/loongarch.h | 5 ++++ >>> arch/loongarch/include/uapi/asm/kvm.h | 2 ++ >>> arch/loongarch/kvm/exit.c | 1 + >>> arch/loongarch/kvm/vcpu.c | 36 +++++++++++++++++++++++--- >>> 5 files changed, 44 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/loongarch/include/asm/kvm_host.h >>> b/arch/loongarch/include/asm/kvm_host.h >>> index 88023ab59486..8fa50d757247 100644 >>> --- a/arch/loongarch/include/asm/kvm_host.h >>> +++ b/arch/loongarch/include/asm/kvm_host.h >>> @@ -135,6 +135,9 @@ enum emulation_result { >>> #define KVM_LARCH_HWCSR_USABLE (0x1 << 4) >>> #define KVM_LARCH_LBT (0x1 << 5) >>> +#define KVM_LOONGARCH_USR_FEAT_MASK \ >>> + BIT(KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI) >>> + >>> struct kvm_vcpu_arch { >>> /* >>> * Switch pointer-to-function type to unsigned long >>> @@ -210,6 +213,7 @@ struct kvm_vcpu_arch { >>> u64 last_steal; >>> struct gfn_to_hva_cache cache; >>> } st; >>> + unsigned int usr_features; >>> }; >>> static inline unsigned long readl_sw_gcsr(struct loongarch_csrs >>> *csr, int reg) >>> diff --git a/arch/loongarch/include/asm/loongarch.h >>> b/arch/loongarch/include/asm/loongarch.h >>> index 7a4633ef284b..4d9837512c19 100644 >>> --- a/arch/loongarch/include/asm/loongarch.h >>> +++ b/arch/loongarch/include/asm/loongarch.h >>> @@ -167,9 +167,14 @@ >>> #define CPUCFG_KVM_SIG (CPUCFG_KVM_BASE + 0) >>> #define KVM_SIGNATURE "KVM\0" >>> +/* >>> + * BIT 24 - 31 is features configurable by user space vmm >>> + */ >>> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4) >>> #define KVM_FEATURE_IPI BIT(1) >>> #define KVM_FEATURE_STEAL_TIME BIT(2) >>> +/* With VIRT_EXTIOI feature, interrupt can route to 256 VCPUs */ >>> +#define KVM_FEATURE_VIRT_EXTIOI BIT(24) >>> #ifndef __ASSEMBLY__ >> >> What about assigning a new CPUCFG leaf ID for separating the two kinds >> of feature flags very cleanly? > For compatible issue like new kernel on old KVM host, to add a new > CPUCFG leaf ID, a new feature need be defined on existing > CPUCFG_KVM_FEATURE register. Such as: > #define KVM_FEATURE_EXTEND_CPUCFG BIT(3) > > VM need check feature KVM_FEATURE_EXTEND_CPUCFG at first, and then read > the new CPUCFG leaf ID if feature EXTEND_CPUCFG is enabled. > > That maybe makes it complicated since feature bit is enough now. The default return value is zero with old kvm host, it is possible to use a new CPUCFG leaf ID. Both methods are ok for me. Huacai, What is your optnion about this? Regards Bibo Mao >> >>> @@ -896,7 +907,24 @@ static int kvm_loongarch_vcpu_get_attr(struct >>> kvm_vcpu *vcpu, >>> static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu, >>> struct kvm_device_attr *attr) >>> { >>> - return -ENXIO; >>> + u64 __user *user = (u64 __user *)attr->addr; >>> + u64 val, valid_flags; >>> + >>> + switch (attr->attr) { >>> + case CPUCFG_KVM_FEATURE: >>> + if (get_user(val, user)) >>> + return -EFAULT; >>> + >>> + valid_flags = KVM_LOONGARCH_USR_FEAT_MASK; >>> + if (val & ~valid_flags) >>> + return -EINVAL; >>> + >>> + vcpu->arch.usr_features |= val; >> >> Isn't this usage of "|=" instead of "=" implying that the feature bits >> could not get disabled after being enabled earlier, for whatever reason? > yes, "=" is better. Will modify in next version. > > Regards > Bibo Mao >> >>> + return 0; >>> + >>> + default: >>> + return -ENXIO; >>> + } >>> } >>> static int kvm_loongarch_vcpu_set_attr(struct kvm_vcpu *vcpu, >>> >>> base-commit: 2df0193e62cf887f373995fb8a91068562784adc >> >
Hi, Bibo, On Thu, Jun 6, 2024 at 8:05 PM maobibo <maobibo@loongson.cn> wrote: > > > > On 2024/6/6 下午7:54, maobibo wrote: > > Xuerui, > > > > Thanks for your reviewing. > > I reply inline. > > > > On 2024/6/6 下午7:20, WANG Xuerui wrote: > >> Hi, > >> > >> On 6/6/24 15:48, Bibo Mao wrote: > >>> Currently features defined in cpucfg CPUCFG_KVM_FEATURE comes from > >>> kvm kernel mode only. Some features are defined in user space VMM, > >> > >> "come from kernel side only. But currently KVM is not aware of > >> user-space VMM features which makes it hard to employ optimizations > >> that are both (1) specific to the VM use case and (2) requiring > >> cooperation from user space." > > Will modify in next version. > >> > >>> however KVM module does not know. Here interface is added to update > >>> register CPUCFG_KVM_FEATURE from user space, only bit 24 - 31 is valid. > >>> > >>> Feature KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI is added from user mdoe. > >>> FEAT_VIRT_EXTIOI is virt EXTIOI extension which can route interrupt > >>> to 256 VCPUs rather than 4 CPUs like real hw. > >> > >> "A new feature bit ... is added which can be set from user space. > >> FEAT_... indicates that the VM EXTIOI can route interrupts to 256 > >> vCPUs, rather than 4 like with real HW." > > will modify in next version. > > > >> > >> (Am I right in paraphrasing the "EXTIOI" part?) > >> > >>> > >>> Signed-off-by: Bibo Mao <maobibo@loongson.cn> > >>> --- > >>> arch/loongarch/include/asm/kvm_host.h | 4 +++ > >>> arch/loongarch/include/asm/loongarch.h | 5 ++++ > >>> arch/loongarch/include/uapi/asm/kvm.h | 2 ++ > >>> arch/loongarch/kvm/exit.c | 1 + > >>> arch/loongarch/kvm/vcpu.c | 36 +++++++++++++++++++++++--- > >>> 5 files changed, 44 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/arch/loongarch/include/asm/kvm_host.h > >>> b/arch/loongarch/include/asm/kvm_host.h > >>> index 88023ab59486..8fa50d757247 100644 > >>> --- a/arch/loongarch/include/asm/kvm_host.h > >>> +++ b/arch/loongarch/include/asm/kvm_host.h > >>> @@ -135,6 +135,9 @@ enum emulation_result { > >>> #define KVM_LARCH_HWCSR_USABLE (0x1 << 4) > >>> #define KVM_LARCH_LBT (0x1 << 5) > >>> +#define KVM_LOONGARCH_USR_FEAT_MASK \ > >>> + BIT(KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI) > >>> + > >>> struct kvm_vcpu_arch { > >>> /* > >>> * Switch pointer-to-function type to unsigned long > >>> @@ -210,6 +213,7 @@ struct kvm_vcpu_arch { > >>> u64 last_steal; > >>> struct gfn_to_hva_cache cache; > >>> } st; > >>> + unsigned int usr_features; > >>> }; > >>> static inline unsigned long readl_sw_gcsr(struct loongarch_csrs > >>> *csr, int reg) > >>> diff --git a/arch/loongarch/include/asm/loongarch.h > >>> b/arch/loongarch/include/asm/loongarch.h > >>> index 7a4633ef284b..4d9837512c19 100644 > >>> --- a/arch/loongarch/include/asm/loongarch.h > >>> +++ b/arch/loongarch/include/asm/loongarch.h > >>> @@ -167,9 +167,14 @@ > >>> #define CPUCFG_KVM_SIG (CPUCFG_KVM_BASE + 0) > >>> #define KVM_SIGNATURE "KVM\0" > >>> +/* > >>> + * BIT 24 - 31 is features configurable by user space vmm > >>> + */ > >>> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4) > >>> #define KVM_FEATURE_IPI BIT(1) > >>> #define KVM_FEATURE_STEAL_TIME BIT(2) > >>> +/* With VIRT_EXTIOI feature, interrupt can route to 256 VCPUs */ > >>> +#define KVM_FEATURE_VIRT_EXTIOI BIT(24) > >>> #ifndef __ASSEMBLY__ > >> > >> What about assigning a new CPUCFG leaf ID for separating the two kinds > >> of feature flags very cleanly? > > For compatible issue like new kernel on old KVM host, to add a new > > CPUCFG leaf ID, a new feature need be defined on existing > > CPUCFG_KVM_FEATURE register. Such as: > > #define KVM_FEATURE_EXTEND_CPUCFG BIT(3) > > > > VM need check feature KVM_FEATURE_EXTEND_CPUCFG at first, and then read > > the new CPUCFG leaf ID if feature EXTEND_CPUCFG is enabled. > > > > That maybe makes it complicated since feature bit is enough now. > The default return value is zero with old kvm host, it is possible to > use a new CPUCFG leaf ID. Both methods are ok for me. > > Huacai, > What is your optnion about this? I don't think we need a new leaf, but is this feature detection duplicated with EXTIOI_VIRT_FEATURES here? https://lore.kernel.org/lkml/871q5a2lo8.ffs@tglx/T/#t Huacai > > Regards > Bibo Mao > >> > >>> @@ -896,7 +907,24 @@ static int kvm_loongarch_vcpu_get_attr(struct > >>> kvm_vcpu *vcpu, > >>> static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu, > >>> struct kvm_device_attr *attr) > >>> { > >>> - return -ENXIO; > >>> + u64 __user *user = (u64 __user *)attr->addr; > >>> + u64 val, valid_flags; > >>> + > >>> + switch (attr->attr) { > >>> + case CPUCFG_KVM_FEATURE: > >>> + if (get_user(val, user)) > >>> + return -EFAULT; > >>> + > >>> + valid_flags = KVM_LOONGARCH_USR_FEAT_MASK; > >>> + if (val & ~valid_flags) > >>> + return -EINVAL; > >>> + > >>> + vcpu->arch.usr_features |= val; > >> > >> Isn't this usage of "|=" instead of "=" implying that the feature bits > >> could not get disabled after being enabled earlier, for whatever reason? > > yes, "=" is better. Will modify in next version. > > > > Regards > > Bibo Mao > >> > >>> + return 0; > >>> + > >>> + default: > >>> + return -ENXIO; > >>> + } > >>> } > >>> static int kvm_loongarch_vcpu_set_attr(struct kvm_vcpu *vcpu, > >>> > >>> base-commit: 2df0193e62cf887f373995fb8a91068562784adc > >> > > > >
On 2024/6/7 上午11:58, Huacai Chen wrote: > Hi, Bibo, > > > On Thu, Jun 6, 2024 at 8:05 PM maobibo <maobibo@loongson.cn> wrote: >> >> >> >> On 2024/6/6 下午7:54, maobibo wrote: >>> Xuerui, >>> >>> Thanks for your reviewing. >>> I reply inline. >>> >>> On 2024/6/6 下午7:20, WANG Xuerui wrote: >>>> Hi, >>>> >>>> On 6/6/24 15:48, Bibo Mao wrote: >>>>> Currently features defined in cpucfg CPUCFG_KVM_FEATURE comes from >>>>> kvm kernel mode only. Some features are defined in user space VMM, >>>> >>>> "come from kernel side only. But currently KVM is not aware of >>>> user-space VMM features which makes it hard to employ optimizations >>>> that are both (1) specific to the VM use case and (2) requiring >>>> cooperation from user space." >>> Will modify in next version. >>>> >>>>> however KVM module does not know. Here interface is added to update >>>>> register CPUCFG_KVM_FEATURE from user space, only bit 24 - 31 is valid. >>>>> >>>>> Feature KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI is added from user mdoe. >>>>> FEAT_VIRT_EXTIOI is virt EXTIOI extension which can route interrupt >>>>> to 256 VCPUs rather than 4 CPUs like real hw. >>>> >>>> "A new feature bit ... is added which can be set from user space. >>>> FEAT_... indicates that the VM EXTIOI can route interrupts to 256 >>>> vCPUs, rather than 4 like with real HW." >>> will modify in next version. >>> >>>> >>>> (Am I right in paraphrasing the "EXTIOI" part?) >>>> >>>>> >>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >>>>> --- >>>>> arch/loongarch/include/asm/kvm_host.h | 4 +++ >>>>> arch/loongarch/include/asm/loongarch.h | 5 ++++ >>>>> arch/loongarch/include/uapi/asm/kvm.h | 2 ++ >>>>> arch/loongarch/kvm/exit.c | 1 + >>>>> arch/loongarch/kvm/vcpu.c | 36 +++++++++++++++++++++++--- >>>>> 5 files changed, 44 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/arch/loongarch/include/asm/kvm_host.h >>>>> b/arch/loongarch/include/asm/kvm_host.h >>>>> index 88023ab59486..8fa50d757247 100644 >>>>> --- a/arch/loongarch/include/asm/kvm_host.h >>>>> +++ b/arch/loongarch/include/asm/kvm_host.h >>>>> @@ -135,6 +135,9 @@ enum emulation_result { >>>>> #define KVM_LARCH_HWCSR_USABLE (0x1 << 4) >>>>> #define KVM_LARCH_LBT (0x1 << 5) >>>>> +#define KVM_LOONGARCH_USR_FEAT_MASK \ >>>>> + BIT(KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI) >>>>> + >>>>> struct kvm_vcpu_arch { >>>>> /* >>>>> * Switch pointer-to-function type to unsigned long >>>>> @@ -210,6 +213,7 @@ struct kvm_vcpu_arch { >>>>> u64 last_steal; >>>>> struct gfn_to_hva_cache cache; >>>>> } st; >>>>> + unsigned int usr_features; >>>>> }; >>>>> static inline unsigned long readl_sw_gcsr(struct loongarch_csrs >>>>> *csr, int reg) >>>>> diff --git a/arch/loongarch/include/asm/loongarch.h >>>>> b/arch/loongarch/include/asm/loongarch.h >>>>> index 7a4633ef284b..4d9837512c19 100644 >>>>> --- a/arch/loongarch/include/asm/loongarch.h >>>>> +++ b/arch/loongarch/include/asm/loongarch.h >>>>> @@ -167,9 +167,14 @@ >>>>> #define CPUCFG_KVM_SIG (CPUCFG_KVM_BASE + 0) >>>>> #define KVM_SIGNATURE "KVM\0" >>>>> +/* >>>>> + * BIT 24 - 31 is features configurable by user space vmm >>>>> + */ >>>>> #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4) >>>>> #define KVM_FEATURE_IPI BIT(1) >>>>> #define KVM_FEATURE_STEAL_TIME BIT(2) >>>>> +/* With VIRT_EXTIOI feature, interrupt can route to 256 VCPUs */ >>>>> +#define KVM_FEATURE_VIRT_EXTIOI BIT(24) >>>>> #ifndef __ASSEMBLY__ >>>> >>>> What about assigning a new CPUCFG leaf ID for separating the two kinds >>>> of feature flags very cleanly? >>> For compatible issue like new kernel on old KVM host, to add a new >>> CPUCFG leaf ID, a new feature need be defined on existing >>> CPUCFG_KVM_FEATURE register. Such as: >>> #define KVM_FEATURE_EXTEND_CPUCFG BIT(3) >>> >>> VM need check feature KVM_FEATURE_EXTEND_CPUCFG at first, and then read >>> the new CPUCFG leaf ID if feature EXTEND_CPUCFG is enabled. >>> >>> That maybe makes it complicated since feature bit is enough now. >> The default return value is zero with old kvm host, it is possible to >> use a new CPUCFG leaf ID. Both methods are ok for me. >> >> Huacai, >> What is your optnion about this? > I don't think we need a new leaf, but is this feature detection > duplicated with EXTIOI_VIRT_FEATURES here? > https://lore.kernel.org/lkml/871q5a2lo8.ffs@tglx/T/#t It is for compatible consideration. The result is unknown on old hypervisor if new kernel accesses newly added IOCSR register EXTIOI_VIRT_FEATURES. For cpucfg instruction it is returns zero if it is not supported, however there is no such spec for iocsr read or write instruction. Regards Bibo > > Huacai > >> >> Regards >> Bibo Mao >>>> >>>>> @@ -896,7 +907,24 @@ static int kvm_loongarch_vcpu_get_attr(struct >>>>> kvm_vcpu *vcpu, >>>>> static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu, >>>>> struct kvm_device_attr *attr) >>>>> { >>>>> - return -ENXIO; >>>>> + u64 __user *user = (u64 __user *)attr->addr; >>>>> + u64 val, valid_flags; >>>>> + >>>>> + switch (attr->attr) { >>>>> + case CPUCFG_KVM_FEATURE: >>>>> + if (get_user(val, user)) >>>>> + return -EFAULT; >>>>> + >>>>> + valid_flags = KVM_LOONGARCH_USR_FEAT_MASK; >>>>> + if (val & ~valid_flags) >>>>> + return -EINVAL; >>>>> + >>>>> + vcpu->arch.usr_features |= val; >>>> >>>> Isn't this usage of "|=" instead of "=" implying that the feature bits >>>> could not get disabled after being enabled earlier, for whatever reason? >>> yes, "=" is better. Will modify in next version. >>> >>> Regards >>> Bibo Mao >>>> >>>>> + return 0; >>>>> + >>>>> + default: >>>>> + return -ENXIO; >>>>> + } >>>>> } >>>>> static int kvm_loongarch_vcpu_set_attr(struct kvm_vcpu *vcpu, >>>>> >>>>> base-commit: 2df0193e62cf887f373995fb8a91068562784adc >>>> >>> >> >>
diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h index 88023ab59486..8fa50d757247 100644 --- a/arch/loongarch/include/asm/kvm_host.h +++ b/arch/loongarch/include/asm/kvm_host.h @@ -135,6 +135,9 @@ enum emulation_result { #define KVM_LARCH_HWCSR_USABLE (0x1 << 4) #define KVM_LARCH_LBT (0x1 << 5) +#define KVM_LOONGARCH_USR_FEAT_MASK \ + BIT(KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI) + struct kvm_vcpu_arch { /* * Switch pointer-to-function type to unsigned long @@ -210,6 +213,7 @@ struct kvm_vcpu_arch { u64 last_steal; struct gfn_to_hva_cache cache; } st; + unsigned int usr_features; }; static inline unsigned long readl_sw_gcsr(struct loongarch_csrs *csr, int reg) diff --git a/arch/loongarch/include/asm/loongarch.h b/arch/loongarch/include/asm/loongarch.h index 7a4633ef284b..4d9837512c19 100644 --- a/arch/loongarch/include/asm/loongarch.h +++ b/arch/loongarch/include/asm/loongarch.h @@ -167,9 +167,14 @@ #define CPUCFG_KVM_SIG (CPUCFG_KVM_BASE + 0) #define KVM_SIGNATURE "KVM\0" +/* + * BIT 24 - 31 is features configurable by user space vmm + */ #define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4) #define KVM_FEATURE_IPI BIT(1) #define KVM_FEATURE_STEAL_TIME BIT(2) +/* With VIRT_EXTIOI feature, interrupt can route to 256 VCPUs */ +#define KVM_FEATURE_VIRT_EXTIOI BIT(24) #ifndef __ASSEMBLY__ diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h index ed12e509815c..dd141259de48 100644 --- a/arch/loongarch/include/uapi/asm/kvm.h +++ b/arch/loongarch/include/uapi/asm/kvm.h @@ -99,6 +99,8 @@ struct kvm_fpu { /* Device Control API on vcpu fd */ #define KVM_LOONGARCH_VCPU_CPUCFG 0 +/* For CPUCFG_KVM_FEATURE register */ +#define KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI 24 #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1 #define KVM_LOONGARCH_VCPU_PVTIME_GPA 0 diff --git a/arch/loongarch/kvm/exit.c b/arch/loongarch/kvm/exit.c index e1bd81d27fd8..ab2dcc76784a 100644 --- a/arch/loongarch/kvm/exit.c +++ b/arch/loongarch/kvm/exit.c @@ -53,6 +53,7 @@ static int kvm_emu_cpucfg(struct kvm_vcpu *vcpu, larch_inst inst) ret = KVM_FEATURE_IPI; if (sched_info_on()) ret |= KVM_FEATURE_STEAL_TIME; + ret |= vcpu->arch.usr_features; vcpu->arch.gprs[rd] = ret; break; default: diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c index 3783151fde32..26f2b22b6a62 100644 --- a/arch/loongarch/kvm/vcpu.c +++ b/arch/loongarch/kvm/vcpu.c @@ -832,6 +832,8 @@ static int kvm_loongarch_cpucfg_has_attr(struct kvm_vcpu *vcpu, switch (attr->attr) { case 2: return 0; + case CPUCFG_KVM_FEATURE: + return 0; default: return -ENXIO; } @@ -865,9 +867,18 @@ static int kvm_loongarch_get_cpucfg_attr(struct kvm_vcpu *vcpu, uint64_t val; uint64_t __user *uaddr = (uint64_t __user *)attr->addr; - ret = _kvm_get_cpucfg_mask(attr->attr, &val); - if (ret) - return ret; + switch (attr->attr) { + case 0 ... (KVM_MAX_CPUCFG_REGS - 1): + ret = _kvm_get_cpucfg_mask(attr->attr, &val); + if (ret) + return ret; + break; + case CPUCFG_KVM_FEATURE: + val = vcpu->arch.usr_features & KVM_LOONGARCH_USR_FEAT_MASK; + break; + default: + return -ENXIO; + } put_user(val, uaddr); @@ -896,7 +907,24 @@ static int kvm_loongarch_vcpu_get_attr(struct kvm_vcpu *vcpu, static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) { - return -ENXIO; + u64 __user *user = (u64 __user *)attr->addr; + u64 val, valid_flags; + + switch (attr->attr) { + case CPUCFG_KVM_FEATURE: + if (get_user(val, user)) + return -EFAULT; + + valid_flags = KVM_LOONGARCH_USR_FEAT_MASK; + if (val & ~valid_flags) + return -EINVAL; + + vcpu->arch.usr_features |= val; + return 0; + + default: + return -ENXIO; + } } static int kvm_loongarch_vcpu_set_attr(struct kvm_vcpu *vcpu,
Currently features defined in cpucfg CPUCFG_KVM_FEATURE comes from kvm kernel mode only. Some features are defined in user space VMM, however KVM module does not know. Here interface is added to update register CPUCFG_KVM_FEATURE from user space, only bit 24 - 31 is valid. Feature KVM_LOONGARCH_VCPU_FEAT_VIRT_EXTIOI is added from user mdoe. FEAT_VIRT_EXTIOI is virt EXTIOI extension which can route interrupt to 256 VCPUs rather than 4 CPUs like real hw. Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- arch/loongarch/include/asm/kvm_host.h | 4 +++ arch/loongarch/include/asm/loongarch.h | 5 ++++ arch/loongarch/include/uapi/asm/kvm.h | 2 ++ arch/loongarch/kvm/exit.c | 1 + arch/loongarch/kvm/vcpu.c | 36 +++++++++++++++++++++++--- 5 files changed, 44 insertions(+), 4 deletions(-) base-commit: 2df0193e62cf887f373995fb8a91068562784adc