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 <[email protected]>
---
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__
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,
base-commit: 2df0193e62cf887f373995fb8a91068562784adc
--
2.39.3
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 <[email protected]>
> ---
> 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
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
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 <[email protected]>
>> ---
>> 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 <[email protected]>
>>> ---
>>> 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 <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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 <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> 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
>>>>
>>>
>>
>>