2022-12-10 16:21:36

by Zhang, Chen

[permalink] [raw]
Subject: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES

Add the 63 bit in MSR_IA32_ARCH_CAPABILITIES for enable the virtual MSRs.
Virtual MSRs can allow guests to notify VMM whether or not
they are using specific software mitigation, allowing a VMM
to enable there hardware control only where necessary.
As Intel spec defination, expose virtual MSR for guest.
Make guest have ability to check virtual MSR 0x50000000.

Signed-off-by: Zhang Chen <[email protected]>
---
arch/x86/kvm/vmx/vmx.c | 15 +++++++++++++++
arch/x86/kvm/vmx/vmx.h | 1 +
arch/x86/kvm/x86.c | 16 +++++++++++++++-
3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 407061b369b4..6ed6b743be0e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2001,6 +2001,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_IA32_DEBUGCTLMSR:
msr_info->data = vmcs_read64(GUEST_IA32_DEBUGCTL);
break;
+ case MSR_VIRTUAL_ENUMERATION:
+ if (!msr_info->host_initiated &&
+ !(vcpu->arch.arch_capabilities & ARCH_CAP_VIRTUAL_ENUM))
+ return 1;
+ msr_info->data = vmx->msr_virtual_enumeration;
+ break;
default:
find_uret_msr:
msr = vmx_find_uret_msr(vmx, msr_info->index);
@@ -2375,6 +2381,15 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
}
ret = kvm_set_msr_common(vcpu, msr_info);
break;
+ case MSR_VIRTUAL_ENUMERATION:
+ if (msr_info->host_initiated &&
+ !(vcpu->arch.arch_capabilities & ARCH_CAP_VIRTUAL_ENUM))
+ return 1;
+ if (data & ~VIRT_ENUM_MITIGATION_CTRL_SUPPORT)
+ return 1;
+ vmx->msr_virtual_enumeration = data &
+ VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
+ break;

default:
find_uret_msr:
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index c5a41ae14237..fc873cf45f70 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -298,6 +298,7 @@ struct vcpu_vmx {
* IA32_SPEC_CTRL_MSR.
*/
u64 spec_ctrl_mask;
+ u64 msr_virtual_enumeration;
u32 msr_ia32_umwait_control;

/*
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2835bd796639..6be0a3f1281f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1532,6 +1532,8 @@ static const u32 emulated_msrs_all[] = {
MSR_IA32_VMX_EPT_VPID_CAP,
MSR_IA32_VMX_VMFUNC,

+ MSR_VIRTUAL_ENUMERATION,
+
MSR_K7_HWCR,
MSR_KVM_POLL_CONTROL,
};
@@ -1567,6 +1569,7 @@ static const u32 msr_based_features_all[] = {
MSR_IA32_UCODE_REV,
MSR_IA32_ARCH_CAPABILITIES,
MSR_IA32_PERF_CAPABILITIES,
+ MSR_VIRTUAL_ENUMERATION,
};

static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all)];
@@ -1588,7 +1591,8 @@ static unsigned int num_msr_based_features;
ARCH_CAP_SKIP_VMENTRY_L1DFLUSH | ARCH_CAP_SSB_NO | ARCH_CAP_MDS_NO | \
ARCH_CAP_PSCHANGE_MC_NO | ARCH_CAP_TSX_CTRL_MSR | ARCH_CAP_TAA_NO | \
ARCH_CAP_SBDR_SSDP_NO | ARCH_CAP_FBSDP_NO | ARCH_CAP_PSDP_NO | \
- ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO)
+ ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO | \
+ ARCH_CAP_VIRTUAL_ENUM)

static u64 kvm_get_arch_capabilities(void)
{
@@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
*/
data |= ARCH_CAP_PSCHANGE_MC_NO;

+ /*
+ * Virtual MSRs can allow guests to notify VMM whether or not
+ * they are using specific software mitigation, allowing a VMM
+ * to enable there hardware control only where necessary.
+ */
+ data |= ARCH_CAP_VIRTUAL_ENUM;
+
/*
* If we're doing cache flushes (either "always" or "cond")
* we will do one whenever the guest does a vmlaunch/vmresume.
@@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct kvm_msr_entry *msr)
case MSR_IA32_UCODE_REV:
rdmsrl_safe(msr->index, &msr->data);
break;
+ case MSR_VIRTUAL_ENUMERATION:
+ msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
+ break;
default:
return static_call(kvm_x86_get_msr_feature)(msr);
}
--
2.25.1


2022-12-21 04:57:48

by Yang, Weijiang

[permalink] [raw]
Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES


On 12/11/2022 12:00 AM, Zhang Chen wrote:
> Add the 63 bit in MSR_IA32_ARCH_CAPABILITIES for enable the virtual MSRs.
> Virtual MSRs can allow guests to notify VMM whether or not
> they are using specific software mitigation, allowing a VMM
> to enable there hardware control only where necessary.
> As Intel spec defination, expose virtual MSR for guest.
> Make guest have ability to check virtual MSR 0x50000000.
>
> Signed-off-by: Zhang Chen <[email protected]>
> ---
> arch/x86/kvm/vmx/vmx.c | 15 +++++++++++++++
> arch/x86/kvm/vmx/vmx.h | 1 +
> arch/x86/kvm/x86.c | 16 +++++++++++++++-
> 3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 407061b369b4..6ed6b743be0e 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2001,6 +2001,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> case MSR_IA32_DEBUGCTLMSR:
> msr_info->data = vmcs_read64(GUEST_IA32_DEBUGCTL);
> break;
> + case MSR_VIRTUAL_ENUMERATION:
> + if (!msr_info->host_initiated &&
> + !(vcpu->arch.arch_capabilities & ARCH_CAP_VIRTUAL_ENUM))
> + return 1;
> + msr_info->data = vmx->msr_virtual_enumeration;
> + break;
> default:
> find_uret_msr:
> msr = vmx_find_uret_msr(vmx, msr_info->index);
> @@ -2375,6 +2381,15 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> }
> ret = kvm_set_msr_common(vcpu, msr_info);
> break;
> + case MSR_VIRTUAL_ENUMERATION:
> + if (msr_info->host_initiated &&
> + !(vcpu->arch.arch_capabilities & ARCH_CAP_VIRTUAL_ENUM))
> + return 1;
> + if (data & ~VIRT_ENUM_MITIGATION_CTRL_SUPPORT)
> + return 1;
> + vmx->msr_virtual_enumeration = data &
> + VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
> + break;
>
> default:
> find_uret_msr:
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index c5a41ae14237..fc873cf45f70 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -298,6 +298,7 @@ struct vcpu_vmx {
> * IA32_SPEC_CTRL_MSR.
> */
> u64 spec_ctrl_mask;
> + u64 msr_virtual_enumeration;
> u32 msr_ia32_umwait_control;
>
> /*
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2835bd796639..6be0a3f1281f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1532,6 +1532,8 @@ static const u32 emulated_msrs_all[] = {
> MSR_IA32_VMX_EPT_VPID_CAP,
> MSR_IA32_VMX_VMFUNC,
>
> + MSR_VIRTUAL_ENUMERATION,
> +
> MSR_K7_HWCR,
> MSR_KVM_POLL_CONTROL,
> };
> @@ -1567,6 +1569,7 @@ static const u32 msr_based_features_all[] = {
> MSR_IA32_UCODE_REV,
> MSR_IA32_ARCH_CAPABILITIES,
> MSR_IA32_PERF_CAPABILITIES,
> + MSR_VIRTUAL_ENUMERATION,
> };
>
> static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all)];
> @@ -1588,7 +1591,8 @@ static unsigned int num_msr_based_features;
> ARCH_CAP_SKIP_VMENTRY_L1DFLUSH | ARCH_CAP_SSB_NO | ARCH_CAP_MDS_NO | \
> ARCH_CAP_PSCHANGE_MC_NO | ARCH_CAP_TSX_CTRL_MSR | ARCH_CAP_TAA_NO | \
> ARCH_CAP_SBDR_SSDP_NO | ARCH_CAP_FBSDP_NO | ARCH_CAP_PSDP_NO | \
> - ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO)
> + ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO | \
> + ARCH_CAP_VIRTUAL_ENUM)
>
> static u64 kvm_get_arch_capabilities(void)
> {
> @@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
> */
> data |= ARCH_CAP_PSCHANGE_MC_NO;
>
> + /*
> + * Virtual MSRs can allow guests to notify VMM whether or not
> + * they are using specific software mitigation, allowing a VMM
> + * to enable there hardware control only where necessary.
> + */
> + data |= ARCH_CAP_VIRTUAL_ENUM;


IMO, this is:  data &= ARCH_CAP_VIRTUAL_ENUM; because it requires
platform support.


> +
> /*
> * If we're doing cache flushes (either "always" or "cond")
> * we will do one whenever the guest does a vmlaunch/vmresume.
> @@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct kvm_msr_entry *msr)
> case MSR_IA32_UCODE_REV:
> rdmsrl_safe(msr->index, &msr->data);
> break;
> + case MSR_VIRTUAL_ENUMERATION:
> + msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;


Need to check bit 63 of host MSR_ARCH_CAPABILITIES before expose the
feature.


> + break;
> default:
> return static_call(kvm_x86_get_msr_feature)(msr);
> }

2022-12-29 03:08:23

by Zhang, Chen

[permalink] [raw]
Subject: RE: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES



> -----Original Message-----
> From: Yang, Weijiang <[email protected]>
> Sent: Wednesday, December 21, 2022 12:04 PM
> To: Zhang, Chen <[email protected]>
> Cc: Gao, Chao <[email protected]>; Pawan Gupta
> <[email protected]>; Paolo Bonzini
> <[email protected]>; Christopherson,, Sean <[email protected]>; H.
> Peter Anvin <[email protected]>; Dave Hansen <[email protected]>;
> Borislav Petkov <[email protected]>; Ingo Molnar <[email protected]>; Thomas
> Gleixner <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for
> guest MSR_IA32_ARCH_CAPABILITIES
>
>
> On 12/11/2022 12:00 AM, Zhang Chen wrote:
> > Add the 63 bit in MSR_IA32_ARCH_CAPABILITIES for enable the virtual MSRs.
> > Virtual MSRs can allow guests to notify VMM whether or not they are
> > using specific software mitigation, allowing a VMM to enable there
> > hardware control only where necessary.
> > As Intel spec defination, expose virtual MSR for guest.
> > Make guest have ability to check virtual MSR 0x50000000.
> >
> > Signed-off-by: Zhang Chen <[email protected]>
> > ---
> > arch/x86/kvm/vmx/vmx.c | 15 +++++++++++++++
> > arch/x86/kvm/vmx/vmx.h | 1 +
> > arch/x86/kvm/x86.c | 16 +++++++++++++++-
> > 3 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index
> > 407061b369b4..6ed6b743be0e 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -2001,6 +2001,12 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu,
> struct msr_data *msr_info)
> > case MSR_IA32_DEBUGCTLMSR:
> > msr_info->data = vmcs_read64(GUEST_IA32_DEBUGCTL);
> > break;
> > + case MSR_VIRTUAL_ENUMERATION:
> > + if (!msr_info->host_initiated &&
> > + !(vcpu->arch.arch_capabilities &
> ARCH_CAP_VIRTUAL_ENUM))
> > + return 1;
> > + msr_info->data = vmx->msr_virtual_enumeration;
> > + break;
> > default:
> > find_uret_msr:
> > msr = vmx_find_uret_msr(vmx, msr_info->index); @@ -2375,6
> +2381,15
> > @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data
> *msr_info)
> > }
> > ret = kvm_set_msr_common(vcpu, msr_info);
> > break;
> > + case MSR_VIRTUAL_ENUMERATION:
> > + if (msr_info->host_initiated &&
> > + !(vcpu->arch.arch_capabilities &
> ARCH_CAP_VIRTUAL_ENUM))
> > + return 1;
> > + if (data & ~VIRT_ENUM_MITIGATION_CTRL_SUPPORT)
> > + return 1;
> > + vmx->msr_virtual_enumeration = data &
> > +
> VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
> > + break;
> >
> > default:
> > find_uret_msr:
> > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h index
> > c5a41ae14237..fc873cf45f70 100644
> > --- a/arch/x86/kvm/vmx/vmx.h
> > +++ b/arch/x86/kvm/vmx/vmx.h
> > @@ -298,6 +298,7 @@ struct vcpu_vmx {
> > * IA32_SPEC_CTRL_MSR.
> > */
> > u64 spec_ctrl_mask;
> > + u64 msr_virtual_enumeration;
> > u32 msr_ia32_umwait_control;
> >
> > /*
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index
> > 2835bd796639..6be0a3f1281f 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1532,6 +1532,8 @@ static const u32 emulated_msrs_all[] = {
> > MSR_IA32_VMX_EPT_VPID_CAP,
> > MSR_IA32_VMX_VMFUNC,
> >
> > + MSR_VIRTUAL_ENUMERATION,
> > +
> > MSR_K7_HWCR,
> > MSR_KVM_POLL_CONTROL,
> > };
> > @@ -1567,6 +1569,7 @@ static const u32 msr_based_features_all[] = {
> > MSR_IA32_UCODE_REV,
> > MSR_IA32_ARCH_CAPABILITIES,
> > MSR_IA32_PERF_CAPABILITIES,
> > + MSR_VIRTUAL_ENUMERATION,
> > };
> >
> > static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all)];
> > @@ -1588,7 +1591,8 @@ static unsigned int num_msr_based_features;
> > ARCH_CAP_SKIP_VMENTRY_L1DFLUSH | ARCH_CAP_SSB_NO |
> ARCH_CAP_MDS_NO | \
> > ARCH_CAP_PSCHANGE_MC_NO | ARCH_CAP_TSX_CTRL_MSR |
> ARCH_CAP_TAA_NO | \
> > ARCH_CAP_SBDR_SSDP_NO | ARCH_CAP_FBSDP_NO |
> ARCH_CAP_PSDP_NO | \
> > - ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO)
> > + ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO |
> \
> > + ARCH_CAP_VIRTUAL_ENUM)
> >
> > static u64 kvm_get_arch_capabilities(void)
> > {
> > @@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
> > */
> > data |= ARCH_CAP_PSCHANGE_MC_NO;
> >
> > + /*
> > + * Virtual MSRs can allow guests to notify VMM whether or not
> > + * they are using specific software mitigation, allowing a VMM
> > + * to enable there hardware control only where necessary.
> > + */
> > + data |= ARCH_CAP_VIRTUAL_ENUM;
>
>
> IMO, this is:  data &= ARCH_CAP_VIRTUAL_ENUM; because it requires
> platform support.

Intel defined the virtual MSRs for software mitigations for all platforms.
KVM should be unconditionally opened it for the software mitigation in migration pools.
For example migration from the old platform to the new platform.
Please check the Software Mitigations in Migration Pools section in documents:
https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/branch-history-injection.html

>
>
> > +
> > /*
> > * If we're doing cache flushes (either "always" or "cond")
> > * we will do one whenever the guest does a vmlaunch/vmresume.
> > @@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct
> kvm_msr_entry *msr)
> > case MSR_IA32_UCODE_REV:
> > rdmsrl_safe(msr->index, &msr->data);
> > break;
> > + case MSR_VIRTUAL_ENUMERATION:
> > + msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
>
>
> Need to check bit 63 of host MSR_ARCH_CAPABILITIES before expose the
> feature.

Refer to the above comments.

Thanks
Chen

>
>
> > + break;
> > default:
> > return static_call(kvm_x86_get_msr_feature)(msr);
> > }

2022-12-29 07:21:41

by Yang, Weijiang

[permalink] [raw]
Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES


On 12/29/2022 10:58 AM, Zhang, Chen wrote:

[...]

>> \
>>> + ARCH_CAP_VIRTUAL_ENUM)
>>>
>>> static u64 kvm_get_arch_capabilities(void)
>>> {
>>> @@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
>>> */
>>> data |= ARCH_CAP_PSCHANGE_MC_NO;
>>>
>>> + /*
>>> + * Virtual MSRs can allow guests to notify VMM whether or not
>>> + * they are using specific software mitigation, allowing a VMM
>>> + * to enable there hardware control only where necessary.
>>> + */
>>> + data |= ARCH_CAP_VIRTUAL_ENUM;
>>
>> IMO, this is:  data &= ARCH_CAP_VIRTUAL_ENUM; because it requires
>> platform support.
> Intel defined the virtual MSRs for software mitigations for all platforms.
> KVM should be unconditionally opened it for the software mitigation in migration pools.
> For example migration from the old platform to the new platform.
> Please check the Software Mitigations in Migration Pools section in documents:
> https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/branch-history-injection.html

If this series running on old platforms, how VMM can set specific vmcs
fields,

e.g., “virtualize IA32_SPEC_CTRL” VM-execution control, to mitigate
guest issues?


>
>>
>>> +
>>> /*
>>> * If we're doing cache flushes (either "always" or "cond")
>>> * we will do one whenever the guest does a vmlaunch/vmresume.
>>> @@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct
>> kvm_msr_entry *msr)
>>> case MSR_IA32_UCODE_REV:
>>> rdmsrl_safe(msr->index, &msr->data);
>>> break;
>>> + case MSR_VIRTUAL_ENUMERATION:
>>> + msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
>>
>> Need to check bit 63 of host MSR_ARCH_CAPABILITIES before expose the
>> feature.
> Refer to the above comments.
>
> Thanks
> Chen
>
>>
>>> + break;
>>> default:
>>> return static_call(kvm_x86_get_msr_feature)(msr);
>>> }

2022-12-29 07:48:00

by Zhang, Chen

[permalink] [raw]
Subject: RE: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES



> -----Original Message-----
> From: Yang, Weijiang <[email protected]>
> Sent: Thursday, December 29, 2022 3:03 PM
> To: Zhang, Chen <[email protected]>
> Cc: Gao, Chao <[email protected]>; Pawan Gupta
> <[email protected]>; Paolo Bonzini
> <[email protected]>; Christopherson,, Sean <[email protected]>; H.
> Peter Anvin <[email protected]>; Dave Hansen <[email protected]>;
> Borislav Petkov <[email protected]>; Ingo Molnar <[email protected]>; Thomas
> Gleixner <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for
> guest MSR_IA32_ARCH_CAPABILITIES
>
>
> On 12/29/2022 10:58 AM, Zhang, Chen wrote:
>
> [...]
>
> >> \
> >>> + ARCH_CAP_VIRTUAL_ENUM)
> >>>
> >>> static u64 kvm_get_arch_capabilities(void)
> >>> {
> >>> @@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
> >>> */
> >>> data |= ARCH_CAP_PSCHANGE_MC_NO;
> >>>
> >>> + /*
> >>> + * Virtual MSRs can allow guests to notify VMM whether or not
> >>> + * they are using specific software mitigation, allowing a VMM
> >>> + * to enable there hardware control only where necessary.
> >>> + */
> >>> + data |= ARCH_CAP_VIRTUAL_ENUM;
> >>
> >> IMO, this is:  data &= ARCH_CAP_VIRTUAL_ENUM; because it requires
> >> platform support.
> > Intel defined the virtual MSRs for software mitigations for all platforms.
> > KVM should be unconditionally opened it for the software mitigation in
> migration pools.
> > For example migration from the old platform to the new platform.
> > Please check the Software Mitigations in Migration Pools section in
> documents:
> > https://www.intel.com/content/www/us/en/developer/articles/technical/s
> > oftware-security-guidance/technical-documentation/branch-history-injec
> > tion.html
>
> If this series running on old platforms, how VMM can set specific vmcs fields,
>
> e.g., “virtualize IA32_SPEC_CTRL” VM-execution control, to mitigate guest
> issues?

Enable the virtual MSRs does not means to enable the “virtualize IA32_SPEC_CTRL”.
KVM will check "cpu_has_virt_spec_ctrl()" before set specific VMCS.

Thanks
Chen

>
>
> >
> >>
> >>> +
> >>> /*
> >>> * If we're doing cache flushes (either "always" or "cond")
> >>> * we will do one whenever the guest does a vmlaunch/vmresume.
> >>> @@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct
> >> kvm_msr_entry *msr)
> >>> case MSR_IA32_UCODE_REV:
> >>> rdmsrl_safe(msr->index, &msr->data);
> >>> break;
> >>> + case MSR_VIRTUAL_ENUMERATION:
> >>> + msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
> >>
> >> Need to check bit 63 of host MSR_ARCH_CAPABILITIES before expose the
> >> feature.
> > Refer to the above comments.
> >
> > Thanks
> > Chen
> >
> >>
> >>> + break;
> >>> default:
> >>> return static_call(kvm_x86_get_msr_feature)(msr);
> >>> }

2022-12-29 09:30:25

by Yang, Weijiang

[permalink] [raw]
Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES


On 12/29/2022 3:41 PM, Zhang, Chen wrote:
>
>> -----Original Message-----
>> From: Yang, Weijiang <[email protected]>
>> Sent: Thursday, December 29, 2022 3:03 PM
>> To: Zhang, Chen <[email protected]>
>> Cc: Gao, Chao <[email protected]>; Pawan Gupta
>> <[email protected]>; Paolo Bonzini
>> <[email protected]>; Christopherson,, Sean <[email protected]>; H.
>> Peter Anvin <[email protected]>; Dave Hansen <[email protected]>;
>> Borislav Petkov <[email protected]>; Ingo Molnar <[email protected]>; Thomas
>> Gleixner <[email protected]>; [email protected]; [email protected];
>> [email protected]
>> Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for
>> guest MSR_IA32_ARCH_CAPABILITIES
>>
>>
>> On 12/29/2022 10:58 AM, Zhang, Chen wrote:
>>
>> [...]
>>
>>>> \
>>>>> + ARCH_CAP_VIRTUAL_ENUM)
>>>>>
>>>>> static u64 kvm_get_arch_capabilities(void)
>>>>> {
>>>>> @@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
>>>>> */
>>>>> data |= ARCH_CAP_PSCHANGE_MC_NO;
>>>>>
>>>>> + /*
>>>>> + * Virtual MSRs can allow guests to notify VMM whether or not
>>>>> + * they are using specific software mitigation, allowing a VMM
>>>>> + * to enable there hardware control only where necessary.
>>>>> + */
>>>>> + data |= ARCH_CAP_VIRTUAL_ENUM;
>>>> IMO, this is:  data &= ARCH_CAP_VIRTUAL_ENUM; because it requires
>>>> platform support.
>>> Intel defined the virtual MSRs for software mitigations for all platforms.
>>> KVM should be unconditionally opened it for the software mitigation in
>> migration pools.
>>> For example migration from the old platform to the new platform.
>>> Please check the Software Mitigations in Migration Pools section in
>> documents:
>>> https://www.intel.com/content/www/us/en/developer/articles/technical/s
>>> oftware-security-guidance/technical-documentation/branch-history-injec
>>> tion.html
>> If this series running on old platforms, how VMM can set specific vmcs fields,
>>
>> e.g., “virtualize IA32_SPEC_CTRL” VM-execution control, to mitigate guest
>> issues?
> Enable the virtual MSRs does not means to enable the “virtualize IA32_SPEC_CTRL”.
> KVM will check "cpu_has_virt_spec_ctrl()" before set specific VMCS.


What I'm concerned is, if the feature is not supported on some
platforms, unconditionally

exposing to guest would make it wrongly "think"  the feature is
supported on HW, but actually

WMM can do nothing to mitigate issues. Read through the patch set, looks
like guest cannot

even get any errors during feature configuration in this case. So I
suggest add more

sanity checks. Did I  miss something?


>
> Thanks
> Chen
>
>>
>>>>> +
>>>>> /*
>>>>> * If we're doing cache flushes (either "always" or "cond")
>>>>> * we will do one whenever the guest does a vmlaunch/vmresume.
>>>>> @@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct
>>>> kvm_msr_entry *msr)
>>>>> case MSR_IA32_UCODE_REV:
>>>>> rdmsrl_safe(msr->index, &msr->data);
>>>>> break;
>>>>> + case MSR_VIRTUAL_ENUMERATION:
>>>>> + msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
>>>> Need to check bit 63 of host MSR_ARCH_CAPABILITIES before expose the
>>>> feature.
>>> Refer to the above comments.
>>>
>>> Thanks
>>> Chen
>>>
>>>>> + break;
>>>>> default:
>>>>> return static_call(kvm_x86_get_msr_feature)(msr);
>>>>> }

2022-12-29 10:02:17

by Zhang, Chen

[permalink] [raw]
Subject: RE: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for guest MSR_IA32_ARCH_CAPABILITIES



> -----Original Message-----
> From: Yang, Weijiang <[email protected]>
> Sent: Thursday, December 29, 2022 4:39 PM
> To: Zhang, Chen <[email protected]>
> Cc: Gao, Chao <[email protected]>; Pawan Gupta
> <[email protected]>; Paolo Bonzini
> <[email protected]>; Christopherson,, Sean <[email protected]>; H.
> Peter Anvin <[email protected]>; Dave Hansen <[email protected]>;
> Borislav Petkov <[email protected]>; Ingo Molnar <[email protected]>; Thomas
> Gleixner <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for
> guest MSR_IA32_ARCH_CAPABILITIES
>
>
> On 12/29/2022 3:41 PM, Zhang, Chen wrote:
> >
> >> -----Original Message-----
> >> From: Yang, Weijiang <[email protected]>
> >> Sent: Thursday, December 29, 2022 3:03 PM
> >> To: Zhang, Chen <[email protected]>
> >> Cc: Gao, Chao <[email protected]>; Pawan Gupta
> >> <[email protected]>; Paolo Bonzini
> >> <[email protected]>; Christopherson,, Sean <[email protected]>; H.
> >> Peter Anvin <[email protected]>; Dave Hansen
> >> <[email protected]>; Borislav Petkov <[email protected]>; Ingo
> >> Molnar <[email protected]>; Thomas Gleixner <[email protected]>;
> >> [email protected]; [email protected]; [email protected]
> >> Subject: Re: [RFC PATCH 6/9] kvm/x86: Add ARCH_CAP_VIRTUAL_ENUM for
> >> guest MSR_IA32_ARCH_CAPABILITIES
> >>
> >>
> >> On 12/29/2022 10:58 AM, Zhang, Chen wrote:
> >>
> >> [...]
> >>
> >>>> \
> >>>>> + ARCH_CAP_VIRTUAL_ENUM)
> >>>>>
> >>>>> static u64 kvm_get_arch_capabilities(void)
> >>>>> {
> >>>>> @@ -1607,6 +1611,13 @@ static u64 kvm_get_arch_capabilities(void)
> >>>>> */
> >>>>> data |= ARCH_CAP_PSCHANGE_MC_NO;
> >>>>>
> >>>>> + /*
> >>>>> + * Virtual MSRs can allow guests to notify VMM whether or not
> >>>>> + * they are using specific software mitigation, allowing a VMM
> >>>>> + * to enable there hardware control only where necessary.
> >>>>> + */
> >>>>> + data |= ARCH_CAP_VIRTUAL_ENUM;
> >>>> IMO, this is:  data &= ARCH_CAP_VIRTUAL_ENUM; because it requires
> >>>> platform support.
> >>> Intel defined the virtual MSRs for software mitigations for all platforms.
> >>> KVM should be unconditionally opened it for the software mitigation
> >>> in
> >> migration pools.
> >>> For example migration from the old platform to the new platform.
> >>> Please check the Software Mitigations in Migration Pools section in
> >> documents:
> >>> https://www.intel.com/content/www/us/en/developer/articles/technical
> >>> /s
> >>> oftware-security-guidance/technical-documentation/branch-history-inj
> >>> ec
> >>> tion.html
> >> If this series running on old platforms, how VMM can set specific
> >> vmcs fields,
> >>
> >> e.g., “virtualize IA32_SPEC_CTRL” VM-execution control, to mitigate
> >> guest issues?
> > Enable the virtual MSRs does not means to enable the “virtualize
> IA32_SPEC_CTRL”.
> > KVM will check "cpu_has_virt_spec_ctrl()" before set specific VMCS.
>
>
> What I'm concerned is, if the feature is not supported on some platforms,
> unconditionally
>
> exposing to guest would make it wrongly "think"  the feature is supported on
> HW, but actually
>
> WMM can do nothing to mitigate issues. Read through the patch set, looks like
> guest cannot
>
> even get any errors during feature configuration in this case. So I suggest add
> more
>
> sanity checks. Did I  miss something?

Understand your concern, but I think no need to concern about it.
Although we have unconditionally turned on virtual MSRs in all platforms,
The detailed virtual MSR bits still can tell guest what feature is supported on HW.
QEMU will setup the MSR_VIRTUAL_MITIGATION_ENUM(0x50000001) bits
Like the BHB_CLEAR_SEQ_S_SUPPORT/RETPOLINE_S_SUPPORT before boot VMs.

Thanks
Chen

>
>
> >
> > Thanks
> > Chen
> >
> >>
> >>>>> +
> >>>>> /*
> >>>>> * If we're doing cache flushes (either "always" or "cond")
> >>>>> * we will do one whenever the guest does a
> vmlaunch/vmresume.
> >>>>> @@ -1657,6 +1668,9 @@ static int kvm_get_msr_feature(struct
> >>>> kvm_msr_entry *msr)
> >>>>> case MSR_IA32_UCODE_REV:
> >>>>> rdmsrl_safe(msr->index, &msr->data);
> >>>>> break;
> >>>>> + case MSR_VIRTUAL_ENUMERATION:
> >>>>> + msr->data = VIRT_ENUM_MITIGATION_CTRL_SUPPORT;
> >>>> Need to check bit 63 of host MSR_ARCH_CAPABILITIES before expose
> >>>> the feature.
> >>> Refer to the above comments.
> >>>
> >>> Thanks
> >>> Chen
> >>>
> >>>>> + break;
> >>>>> default:
> >>>>> return static_call(kvm_x86_get_msr_feature)(msr);
> >>>>> }