* [PATCH 01/11] KVM: x86: Introduce vm_type to differentiate normal VMs from confidential VMs
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 16:47 ` Sean Christopherson
2021-11-12 15:37 ` [PATCH 02/11] KVM: x86: Disable direct IRQ injection for TDX Xiaoyao Li
` (9 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Sean Christopherson <sean.j.christopherson@intel.com>
Unlike normal VMs, confidential VMs (Intel TDX and AMD SEV-ES) don't
allow some operations (e.g., memory read/write, register state acces, etc).
Introduce vm_type to track the type of the VM. Further, different policy
can be made based on vm_type.
Define KVM_X86_NORMAL_VM for normal VM as default and define
KVM_X86_TDX_VM for Intel TDX VM.
Add a capability KVM_CAP_VM_TYPES to effectively allow userspace to query
what VM types are supported by KVM.
Co-developed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
Documentation/virt/kvm/api.rst | 15 +++++++++++++++
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 2 ++
arch/x86/include/uapi/asm/kvm.h | 3 +++
arch/x86/kvm/svm/svm.c | 6 ++++++
arch/x86/kvm/vmx/vmx.c | 6 ++++++
arch/x86/kvm/x86.c | 9 ++++++++-
include/uapi/linux/kvm.h | 1 +
tools/arch/x86/include/uapi/asm/kvm.h | 3 +++
tools/include/uapi/linux/kvm.h | 1 +
10 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 3b093d6dbe22..79b64cdb603b 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -147,15 +147,30 @@ described as 'basic' will be available.
The new VM has no virtual cpus and no memory.
You probably want to use 0 as machine type.
+X86:
+^^^^
+
+Supported vm type can be queried from KVM_CAP_VM_TYPES, which returns the
+bitmap of supported vm types. The 1-setting of bit @n means vm type with
+value @n is supported.
+
+S390:
+^^^^^
+
In order to create user controlled virtual machines on S390, check
KVM_CAP_S390_UCONTROL and use the flag KVM_VM_S390_UCONTROL as
privileged user (CAP_SYS_ADMIN).
+MIPS:
+^^^^^
+
To use hardware assisted virtualization on MIPS (VZ ASE) rather than
the default trap & emulate implementation (which changes the virtual
memory layout to fit in user mode), check KVM_CAP_MIPS_VZ and use the
flag KVM_VM_MIPS_VZ.
+ARM64:
+^^^^^^
On arm64, the physical address size for a VM (IPA Size limit) is limited
to 40bits by default. The limit can be configured if the host supports the
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index cefe1d81e2e8..4aa0a1c10b84 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -18,6 +18,7 @@ KVM_X86_OP_NULL(hardware_unsetup)
KVM_X86_OP_NULL(cpu_has_accelerated_tpr)
KVM_X86_OP(has_emulated_msr)
KVM_X86_OP(vcpu_after_set_cpuid)
+KVM_X86_OP(is_vm_type_supported)
KVM_X86_OP(vm_init)
KVM_X86_OP_NULL(vm_destroy)
KVM_X86_OP(vcpu_create)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2acf37cc1991..a5e27111b198 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1036,6 +1036,7 @@ struct kvm_x86_msr_filter {
#define APICV_INHIBIT_REASON_X2APIC 5
struct kvm_arch {
+ unsigned long vm_type;
unsigned long n_used_mmu_pages;
unsigned long n_requested_mmu_pages;
unsigned long n_max_mmu_pages;
@@ -1310,6 +1311,7 @@ struct kvm_x86_ops {
bool (*has_emulated_msr)(struct kvm *kvm, u32 index);
void (*vcpu_after_set_cpuid)(struct kvm_vcpu *vcpu);
+ bool (*is_vm_type_supported)(unsigned long vm_type);
unsigned int vm_size;
int (*vm_init)(struct kvm *kvm);
void (*vm_destroy)(struct kvm *kvm);
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 5a776a08f78c..102b71229f87 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -508,4 +508,7 @@ struct kvm_pmu_event_filter {
#define KVM_VCPU_TSC_CTRL 0 /* control group for the timestamp counter (TSC) */
#define KVM_VCPU_TSC_OFFSET 0 /* attribute for the TSC offset */
+#define KVM_X86_NORMAL_VM 0
+#define KVM_X86_TDX_VM 1
+
#endif /* _ASM_X86_KVM_H */
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index b36ca4e476c2..1c7a7e1f56f8 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4567,6 +4567,11 @@ static void svm_vm_destroy(struct kvm *kvm)
sev_vm_destroy(kvm);
}
+static bool svm_is_vm_type_supported(unsigned long type)
+{
+ return type == KVM_X86_NORMAL_VM;
+}
+
static int svm_vm_init(struct kvm *kvm)
{
if (!pause_filter_count || !pause_filter_thresh)
@@ -4594,6 +4599,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
.vcpu_free = svm_free_vcpu,
.vcpu_reset = svm_vcpu_reset,
+ .is_vm_type_supported = svm_is_vm_type_supported,
.vm_size = sizeof(struct kvm_svm),
.vm_init = svm_vm_init,
.vm_destroy = svm_vm_destroy,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 76861b66bbcf..41513ae18f22 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6925,6 +6925,11 @@ static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
return err;
}
+static bool vmx_is_vm_type_supported(unsigned long type)
+{
+ return type == KVM_X86_NORMAL_VM;
+}
+
#define L1TF_MSG_SMT "L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n"
#define L1TF_MSG_L1D "L1TF CPU bug present and virtualization mitigation disabled, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n"
@@ -7578,6 +7583,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
.cpu_has_accelerated_tpr = report_flexpriority,
.has_emulated_msr = vmx_has_emulated_msr,
+ .is_vm_type_supported = vmx_is_vm_type_supported,
.vm_size = sizeof(struct kvm_vmx),
.vm_init = vmx_vm_init,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c1c4e2b05a63..c080c68e4386 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4183,6 +4183,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
else
r = 0;
break;
+ case KVM_CAP_VM_TYPES:
+ r = BIT(KVM_X86_NORMAL_VM);
+ if (static_call(kvm_x86_is_vm_type_supported)(KVM_X86_TDX_VM))
+ r |= BIT(KVM_X86_TDX_VM);
+ break;
default:
break;
}
@@ -11231,9 +11236,11 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
int ret;
unsigned long flags;
- if (type)
+ if (!static_call(kvm_x86_is_vm_type_supported)(type))
return -EINVAL;
+ kvm->arch.vm_type = type;
+
ret = kvm_page_track_init(kvm);
if (ret)
return ret;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 78f0719cc2a3..91f75704ef6a 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1130,6 +1130,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_BINARY_STATS_FD 203
#define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204
#define KVM_CAP_ARM_MTE 205
+#define KVM_CAP_VM_TYPES 206
#ifdef KVM_CAP_IRQ_ROUTING
diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
index 2ef1f6513c68..69851dfd5085 100644
--- a/tools/arch/x86/include/uapi/asm/kvm.h
+++ b/tools/arch/x86/include/uapi/asm/kvm.h
@@ -504,4 +504,7 @@ struct kvm_pmu_event_filter {
#define KVM_PMU_EVENT_ALLOW 0
#define KVM_PMU_EVENT_DENY 1
+#define KVM_X86_NORMAL_VM 0
+#define KVM_X86_TDX_VM 1
+
#endif /* _ASM_X86_KVM_H */
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index a067410ebea5..e7616e64fb4e 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -1112,6 +1112,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_BINARY_STATS_FD 203
#define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204
#define KVM_CAP_ARM_MTE 205
+#define KVM_CAP_VM_TYPES 206
#ifdef KVM_CAP_IRQ_ROUTING
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 01/11] KVM: x86: Introduce vm_type to differentiate normal VMs from confidential VMs
2021-11-12 15:37 ` [PATCH 01/11] KVM: x86: Introduce vm_type to differentiate normal VMs from confidential VMs Xiaoyao Li
@ 2021-11-12 16:47 ` Sean Christopherson
2021-11-15 15:37 ` Xiaoyao Li
0 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2021-11-12 16:47 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On Fri, Nov 12, 2021, Xiaoyao Li wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> Unlike normal VMs, confidential VMs (Intel TDX and AMD SEV-ES) don't
> allow some operations (e.g., memory read/write, register state acces, etc).
>
> Introduce vm_type to track the type of the VM. Further, different policy
> can be made based on vm_type.
>
> Define KVM_X86_NORMAL_VM for normal VM as default and define
> KVM_X86_TDX_VM for Intel TDX VM.
I still don't like the "normal" terminology, I would much prefer we use "auto"
or "default".
https://lkml.kernel.org/r/YQsjQ5aJokV1HZ8N@google.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 01/11] KVM: x86: Introduce vm_type to differentiate normal VMs from confidential VMs
2021-11-12 16:47 ` Sean Christopherson
@ 2021-11-15 15:37 ` Xiaoyao Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-15 15:37 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On 11/13/2021 12:47 AM, Sean Christopherson wrote:
> On Fri, Nov 12, 2021, Xiaoyao Li wrote:
>> From: Sean Christopherson <sean.j.christopherson@intel.com>
>>
>> Unlike normal VMs, confidential VMs (Intel TDX and AMD SEV-ES) don't
>> allow some operations (e.g., memory read/write, register state acces, etc).
>>
>> Introduce vm_type to track the type of the VM. Further, different policy
>> can be made based on vm_type.
>>
>> Define KVM_X86_NORMAL_VM for normal VM as default and define
>> KVM_X86_TDX_VM for Intel TDX VM.
>
> I still don't like the "normal" terminology, I would much prefer we use "auto"
> or "default".
>
> https://lkml.kernel.org/r/YQsjQ5aJokV1HZ8N@google.com
>
Apparently I missed this. I'll use KVM_X86_DEFAULT_VM in next submission
if no better option appears.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 02/11] KVM: x86: Disable direct IRQ injection for TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
2021-11-12 15:37 ` [PATCH 01/11] KVM: x86: Introduce vm_type to differentiate normal VMs from confidential VMs Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 15:37 ` [PATCH 03/11] KVM: x86: Clean up kvm_vcpu_ioctl_x86_setup_mce() Xiaoyao Li
` (8 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Sean Christopherson <sean.j.christopherson@intel.com>
For TDX VM, direct IRQ injection is not supported.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/x86.c | 3 ++-
arch/x86/kvm/x86.h | 5 +++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c080c68e4386..23617582712d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4455,7 +4455,7 @@ static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu)
static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
struct kvm_interrupt *irq)
{
- if (irq->irq >= KVM_NR_INTERRUPTS)
+ if (irq->irq >= KVM_NR_INTERRUPTS || kvm_irq_injection_disallowed(vcpu))
return -EINVAL;
if (!irqchip_in_kernel(vcpu->kvm)) {
@@ -8891,6 +8891,7 @@ static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
static int dm_request_for_irq_injection(struct kvm_vcpu *vcpu)
{
return vcpu->run->request_interrupt_window &&
+ !kvm_irq_injection_disallowed(vcpu) &&
likely(!pic_in_kernel(vcpu->kvm));
}
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index ea264c4502e4..a2813892740d 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -436,6 +436,11 @@ static inline void kvm_machine_check(void)
#endif
}
+static __always_inline bool kvm_irq_injection_disallowed(struct kvm_vcpu *vcpu)
+{
+ return vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
int kvm_spec_ctrl_test_value(u64 value);
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 03/11] KVM: x86: Clean up kvm_vcpu_ioctl_x86_setup_mce()
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
2021-11-12 15:37 ` [PATCH 01/11] KVM: x86: Introduce vm_type to differentiate normal VMs from confidential VMs Xiaoyao Li
2021-11-12 15:37 ` [PATCH 02/11] KVM: x86: Disable direct IRQ injection for TDX Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-12-02 1:19 ` Xiaoyao Li
2021-11-12 15:37 ` [PATCH 04/11] KVM: x86: Disable MCE related stuff for TDX Xiaoyao Li
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
No need to use goto.
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/x86.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 23617582712d..b02088343d80 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4505,15 +4505,13 @@ static int vcpu_ioctl_tpr_access_reporting(struct kvm_vcpu *vcpu,
static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu,
u64 mcg_cap)
{
- int r;
unsigned bank_num = mcg_cap & 0xff, bank;
- r = -EINVAL;
if (!bank_num || bank_num > KVM_MAX_MCE_BANKS)
- goto out;
+ return -EINVAL;
if (mcg_cap & ~(kvm_mce_cap_supported | 0xff | 0xff0000))
- goto out;
- r = 0;
+ return -EINVAL;
+
vcpu->arch.mcg_cap = mcg_cap;
/* Init IA32_MCG_CTL to all 1s */
if (mcg_cap & MCG_CTL_P)
@@ -4523,8 +4521,8 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu,
vcpu->arch.mce_banks[bank*4] = ~(u64)0;
static_call(kvm_x86_setup_mce)(vcpu);
-out:
- return r;
+
+ return 0;
}
static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu,
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 03/11] KVM: x86: Clean up kvm_vcpu_ioctl_x86_setup_mce()
2021-11-12 15:37 ` [PATCH 03/11] KVM: x86: Clean up kvm_vcpu_ioctl_x86_setup_mce() Xiaoyao Li
@ 2021-12-02 1:19 ` Xiaoyao Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-12-02 1:19 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: erdemaktas, Connor Kuehl, x86, linux-kernel, kvm, isaku.yamahata,
Kai Huang
Paolo,
Isaku is going to re-organize TDX KVM series based on your and tglx's
comments. I'm not sure if it's good/correct to make this series
separate. Maybe introduction of vm_type is better to sit at the very
beginning of Isaku's next series. In that case, I think you can pick
this cleanup patch separately?
Thanks,
-Xiaoyao
On 11/12/2021 11:37 PM, Xiaoyao Li wrote:
> No need to use goto.
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/kvm/x86.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 23617582712d..b02088343d80 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4505,15 +4505,13 @@ static int vcpu_ioctl_tpr_access_reporting(struct kvm_vcpu *vcpu,
> static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu,
> u64 mcg_cap)
> {
> - int r;
> unsigned bank_num = mcg_cap & 0xff, bank;
>
> - r = -EINVAL;
> if (!bank_num || bank_num > KVM_MAX_MCE_BANKS)
> - goto out;
> + return -EINVAL;
> if (mcg_cap & ~(kvm_mce_cap_supported | 0xff | 0xff0000))
> - goto out;
> - r = 0;
> + return -EINVAL;
> +
> vcpu->arch.mcg_cap = mcg_cap;
> /* Init IA32_MCG_CTL to all 1s */
> if (mcg_cap & MCG_CTL_P)
> @@ -4523,8 +4521,8 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu,
> vcpu->arch.mce_banks[bank*4] = ~(u64)0;
>
> static_call(kvm_x86_setup_mce)(vcpu);
> -out:
> - return r;
> +
> + return 0;
> }
>
> static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu,
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 04/11] KVM: x86: Disable MCE related stuff for TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (2 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 03/11] KVM: x86: Clean up kvm_vcpu_ioctl_x86_setup_mce() Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 17:01 ` Sean Christopherson
2021-11-12 15:37 ` [PATCH 05/11] KVM: x86: Disallow tsc manipulation " Xiaoyao Li
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Sean Christopherson <sean.j.christopherson@intel.com>
MCE is not supported for TDX VM and KVM cannot inject #MC to TDX VM.
Introduce kvm_guest_mce_disallowed() which actually reports the MCE
availability based on vm_type. And use it to guard all the MCE related
CAPs and IOCTLs.
Note: KVM_X86_GET_MCE_CAP_SUPPORTED is KVM scope so that what it reports
may not match the behavior of specific VM (e.g., here for TDX VM). The
same for KVM_CAP_MCE when queried from /dev/kvm. To qeuery the precise
KVM_CAP_MCE of the VM, it should use VM's fd.
[ Xiaoyao: Guard MCE related CAPs ]
Co-developed-by: Kai Huang <kai.huang@linux.intel.com>
Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/x86.c | 10 ++++++++++
arch/x86/kvm/x86.h | 5 +++++
2 files changed, 15 insertions(+)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b02088343d80..2b21c5169f32 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4150,6 +4150,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
break;
case KVM_CAP_MCE:
r = KVM_MAX_MCE_BANKS;
+ if (kvm)
+ r = kvm_guest_mce_disallowed(kvm) ? 0 : r;
break;
case KVM_CAP_XCRS:
r = boot_cpu_has(X86_FEATURE_XSAVE);
@@ -5155,6 +5157,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
case KVM_X86_SETUP_MCE: {
u64 mcg_cap;
+ r = EINVAL;
+ if (kvm_guest_mce_disallowed(vcpu->kvm))
+ goto out;
+
r = -EFAULT;
if (copy_from_user(&mcg_cap, argp, sizeof(mcg_cap)))
goto out;
@@ -5164,6 +5170,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
case KVM_X86_SET_MCE: {
struct kvm_x86_mce mce;
+ r = EINVAL;
+ if (kvm_guest_mce_disallowed(vcpu->kvm))
+ goto out;
+
r = -EFAULT;
if (copy_from_user(&mce, argp, sizeof(mce)))
goto out;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index a2813892740d..69c60297bef2 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -441,6 +441,11 @@ static __always_inline bool kvm_irq_injection_disallowed(struct kvm_vcpu *vcpu)
return vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM;
}
+static __always_inline bool kvm_guest_mce_disallowed(struct kvm *kvm)
+{
+ return kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
int kvm_spec_ctrl_test_value(u64 value);
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 04/11] KVM: x86: Disable MCE related stuff for TDX
2021-11-12 15:37 ` [PATCH 04/11] KVM: x86: Disable MCE related stuff for TDX Xiaoyao Li
@ 2021-11-12 17:01 ` Sean Christopherson
2021-11-15 15:39 ` Xiaoyao Li
0 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2021-11-12 17:01 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On Fri, Nov 12, 2021, Xiaoyao Li wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> MCE is not supported for TDX VM and KVM cannot inject #MC to TDX VM.
>
> Introduce kvm_guest_mce_disallowed() which actually reports the MCE
> availability based on vm_type. And use it to guard all the MCE related
> CAPs and IOCTLs.
>
> Note: KVM_X86_GET_MCE_CAP_SUPPORTED is KVM scope so that what it reports
> may not match the behavior of specific VM (e.g., here for TDX VM). The
> same for KVM_CAP_MCE when queried from /dev/kvm. To qeuery the precise
> KVM_CAP_MCE of the VM, it should use VM's fd.
>
> [ Xiaoyao: Guard MCE related CAPs ]
>
> Co-developed-by: Kai Huang <kai.huang@linux.intel.com>
> Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/kvm/x86.c | 10 ++++++++++
> arch/x86/kvm/x86.h | 5 +++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b02088343d80..2b21c5169f32 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4150,6 +4150,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> break;
> case KVM_CAP_MCE:
> r = KVM_MAX_MCE_BANKS;
> + if (kvm)
> + r = kvm_guest_mce_disallowed(kvm) ? 0 : r;
r = KVM_MAX_MCE_BANKS;
if (kvm && kvm_guest_mce_disallowed(kvm))
r = 0;
or
r = (kvm && kvm_guest_mce_disallowed(kvm)) ? 0 : KVM_MAX_MCE_BANKS;
> break;
> case KVM_CAP_XCRS:
> r = boot_cpu_has(X86_FEATURE_XSAVE);
> @@ -5155,6 +5157,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> case KVM_X86_SETUP_MCE: {
> u64 mcg_cap;
>
> + r = EINVAL;
> + if (kvm_guest_mce_disallowed(vcpu->kvm))
> + goto out;
> +
> r = -EFAULT;
> if (copy_from_user(&mcg_cap, argp, sizeof(mcg_cap)))
> goto out;
> @@ -5164,6 +5170,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> case KVM_X86_SET_MCE: {
> struct kvm_x86_mce mce;
>
> + r = EINVAL;
> + if (kvm_guest_mce_disallowed(vcpu->kvm))
> + goto out;
> +
> r = -EFAULT;
> if (copy_from_user(&mce, argp, sizeof(mce)))
> goto out;
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index a2813892740d..69c60297bef2 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -441,6 +441,11 @@ static __always_inline bool kvm_irq_injection_disallowed(struct kvm_vcpu *vcpu)
> return vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM;
> }
>
> +static __always_inline bool kvm_guest_mce_disallowed(struct kvm *kvm)
The "guest" part is potentially confusing and incosistent with e.g.
kvm_irq_injection_disallowed. And given the current ridiculous spec, CR4.MCE=1
is _required_, so saying "mce disallowed" is arguably wrong from that perspective.
kvm_mce_injection_disallowed() would be more appropriate.
> +{
> + return kvm->arch.vm_type == KVM_X86_TDX_VM;
> +}
> +
> void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
> void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
> int kvm_spec_ctrl_test_value(u64 value);
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 04/11] KVM: x86: Disable MCE related stuff for TDX
2021-11-12 17:01 ` Sean Christopherson
@ 2021-11-15 15:39 ` Xiaoyao Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-15 15:39 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On 11/13/2021 1:01 AM, Sean Christopherson wrote:
> On Fri, Nov 12, 2021, Xiaoyao Li wrote:
>> From: Sean Christopherson <sean.j.christopherson@intel.com>
>>
>> MCE is not supported for TDX VM and KVM cannot inject #MC to TDX VM.
>>
>> Introduce kvm_guest_mce_disallowed() which actually reports the MCE
>> availability based on vm_type. And use it to guard all the MCE related
>> CAPs and IOCTLs.
>>
>> Note: KVM_X86_GET_MCE_CAP_SUPPORTED is KVM scope so that what it reports
>> may not match the behavior of specific VM (e.g., here for TDX VM). The
>> same for KVM_CAP_MCE when queried from /dev/kvm. To qeuery the precise
>> KVM_CAP_MCE of the VM, it should use VM's fd.
>>
>> [ Xiaoyao: Guard MCE related CAPs ]
>>
>> Co-developed-by: Kai Huang <kai.huang@linux.intel.com>
>> Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
>> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>> arch/x86/kvm/x86.c | 10 ++++++++++
>> arch/x86/kvm/x86.h | 5 +++++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index b02088343d80..2b21c5169f32 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4150,6 +4150,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> break;
>> case KVM_CAP_MCE:
>> r = KVM_MAX_MCE_BANKS;
>> + if (kvm)
>> + r = kvm_guest_mce_disallowed(kvm) ? 0 : r;
>
> r = KVM_MAX_MCE_BANKS;
> if (kvm && kvm_guest_mce_disallowed(kvm))
> r = 0;
>
> or
>
> r = (kvm && kvm_guest_mce_disallowed(kvm)) ? 0 : KVM_MAX_MCE_BANKS;
I will use this one in next submission.
>> break;
>> case KVM_CAP_XCRS:
>> r = boot_cpu_has(X86_FEATURE_XSAVE);
>> @@ -5155,6 +5157,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>> case KVM_X86_SETUP_MCE: {
>> u64 mcg_cap;
>>
>> + r = EINVAL;
>> + if (kvm_guest_mce_disallowed(vcpu->kvm))
>> + goto out;
>> +
>> r = -EFAULT;
>> if (copy_from_user(&mcg_cap, argp, sizeof(mcg_cap)))
>> goto out;
>> @@ -5164,6 +5170,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>> case KVM_X86_SET_MCE: {
>> struct kvm_x86_mce mce;
>>
>> + r = EINVAL;
>> + if (kvm_guest_mce_disallowed(vcpu->kvm))
>> + goto out;
>> +
>> r = -EFAULT;
>> if (copy_from_user(&mce, argp, sizeof(mce)))
>> goto out;
>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
>> index a2813892740d..69c60297bef2 100644
>> --- a/arch/x86/kvm/x86.h
>> +++ b/arch/x86/kvm/x86.h
>> @@ -441,6 +441,11 @@ static __always_inline bool kvm_irq_injection_disallowed(struct kvm_vcpu *vcpu)
>> return vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM;
>> }
>>
>> +static __always_inline bool kvm_guest_mce_disallowed(struct kvm *kvm)
>
> The "guest" part is potentially confusing and incosistent with e.g.
> kvm_irq_injection_disallowed. And given the current ridiculous spec, CR4.MCE=1
> is _required_, so saying "mce disallowed" is arguably wrong from that perspective.
>
> kvm_mce_injection_disallowed() would be more appropriate.
Good advice, I'll rename to it.
>> +{
>> + return kvm->arch.vm_type == KVM_X86_TDX_VM;
>> +}
>> +
>> void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
>> void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
>> int kvm_spec_ctrl_test_value(u64 value);
>> --
>> 2.27.0
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 05/11] KVM: x86: Disallow tsc manipulation for TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (3 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 04/11] KVM: x86: Disable MCE related stuff for TDX Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 15:37 ` [PATCH 06/11] KVM: x86: Disable in-kernel I/O APIC and level routes " Xiaoyao Li
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Sean Christopherson <sean.j.christopherson@intel.com>
The TSC for TDX guest is fixed at TD creation time.
Introduce kvm_tsc_immutable() to tell if TSC is immutable or not.
If immutable, short circuit all paths that lead to one of the myriad TSC
adjustment flows.
Suggested-by: Kai Huang <kai.huang@linux.intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/x86.c | 34 ++++++++++++++++++++++++++--------
arch/x86/kvm/x86.h | 5 +++++
2 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2b21c5169f32..34dd93b29932 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2247,7 +2247,9 @@ static int set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
u64 ratio;
/* Guest TSC same frequency as host TSC? */
- if (!scale) {
+ if (!scale || kvm_tsc_immutable(vcpu)) {
+ if (scale)
+ pr_warn_ratelimited("Guest TSC immutable, scaling not supported\n");
kvm_vcpu_write_tsc_multiplier(vcpu, kvm_default_tsc_scaling_ratio);
return 0;
}
@@ -2534,6 +2536,9 @@ static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 data)
bool matched = false;
bool synchronizing = false;
+ if (WARN_ON_ONCE(kvm_tsc_immutable(vcpu)))
+ return;
+
raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
offset = kvm_compute_l1_tsc_offset(vcpu, data);
ns = get_kvmclock_base_ns();
@@ -2960,6 +2965,10 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
u8 pvclock_flags;
bool use_master_clock;
+ /* Unable to update guest time if the TSC is immutable. */
+ if (kvm_tsc_immutable(v))
+ return 0;
+
kernel_ns = 0;
host_tsc = 0;
@@ -4332,7 +4341,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
if (tsc_delta < 0)
mark_tsc_unstable("KVM discovered backwards TSC");
- if (kvm_check_tsc_unstable()) {
+ if (kvm_check_tsc_unstable() && !kvm_tsc_immutable(vcpu)) {
u64 offset = kvm_compute_l1_tsc_offset(vcpu,
vcpu->arch.last_guest_tsc);
kvm_vcpu_write_tsc_offset(vcpu, offset);
@@ -4346,7 +4355,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
* On a host with synchronized TSC, there is no need to update
* kvmclock on vcpu->cpu migration
*/
- if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1)
+ if ((!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1) &&
+ !kvm_tsc_immutable(vcpu))
kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu);
if (vcpu->cpu != cpu)
kvm_make_request(KVM_REQ_MIGRATE_TIMER, vcpu);
@@ -5274,10 +5284,11 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
break;
}
case KVM_SET_TSC_KHZ: {
- u32 user_tsc_khz;
+ u32 user_tsc_khz = (u32)arg;
r = -EINVAL;
- user_tsc_khz = (u32)arg;
+ if (kvm_tsc_immutable(vcpu))
+ goto out;
if (kvm_has_tsc_control &&
user_tsc_khz >= kvm_max_guest_tsc_khz)
@@ -10857,9 +10868,12 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
if (mutex_lock_killable(&vcpu->mutex))
return;
- vcpu_load(vcpu);
- kvm_synchronize_tsc(vcpu, 0);
- vcpu_put(vcpu);
+
+ if (!kvm_tsc_immutable(vcpu)) {
+ vcpu_load(vcpu);
+ kvm_synchronize_tsc(vcpu, 0);
+ vcpu_put(vcpu);
+ }
/* poll control enabled by default */
vcpu->arch.msr_kvm_poll_control = 1;
@@ -11119,6 +11133,10 @@ int kvm_arch_hardware_enable(void)
if (backwards_tsc) {
u64 delta_cyc = max_tsc - local_tsc;
list_for_each_entry(kvm, &vm_list, vm_list) {
+ if (kvm_tsc_immutable(vcpu)) {
+ pr_warn_ratelimited("Backwards TSC observed and guest with immutable TSC active\n");
+ continue;
+ }
kvm->arch.backwards_tsc_observed = true;
kvm_for_each_vcpu(i, vcpu, kvm) {
vcpu->arch.tsc_offset_adjustment += delta_cyc;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 69c60297bef2..0d8435b32bf5 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -446,6 +446,11 @@ static __always_inline bool kvm_guest_mce_disallowed(struct kvm *kvm)
return kvm->arch.vm_type == KVM_X86_TDX_VM;
}
+static __always_inline bool kvm_tsc_immutable(struct kvm_vcpu *vcpu)
+{
+ return vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
int kvm_spec_ctrl_test_value(u64 value);
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 06/11] KVM: x86: Disable in-kernel I/O APIC and level routes for TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (4 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 05/11] KVM: x86: Disallow tsc manipulation " Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 15:37 ` [PATCH 07/11] KVM: x86: Disable SMM " Xiaoyao Li
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Kai Huang <kai.huang@linux.intel.com>
Introduce kvm_eoi_intercept_disallowed() to disallow the in-kernel
I/O APIC, level triggered routes for a userspace I/O APIC, and anything
else that relies on being able to intercept EOIs. It's currently for
TDX, since TDX module does not allow intercepting EOI.
Note, technically KVM could partially emulate the I/O APIC by allowing
only edge triggered interrupts, but that adds a lot of complexity for
basically zero benefit. Ideally KVM wouldn't even allow I/O APIC route
reservation, but disabling that is a train wreck for Qemu.
Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/ioapic.c | 5 +++++
arch/x86/kvm/irq_comm.c | 9 +++++++--
arch/x86/kvm/lapic.c | 3 ++-
arch/x86/kvm/x86.c | 6 ++++++
arch/x86/kvm/x86.h | 5 +++++
5 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 816a82515dcd..f9fb2c694c83 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -45,6 +45,7 @@
#include "ioapic.h"
#include "lapic.h"
#include "irq.h"
+#include "x86.h"
static int ioapic_service(struct kvm_ioapic *vioapic, int irq,
bool line_status);
@@ -311,6 +312,10 @@ void kvm_arch_post_irq_ack_notifier_list_update(struct kvm *kvm)
{
if (!ioapic_in_kernel(kvm))
return;
+
+ if (WARN_ON_ONCE(kvm_eoi_intercept_disallowed(kvm)))
+ return;
+
kvm_make_scan_ioapic_request(kvm);
}
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index d5b72a08e566..f9f643e31893 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -123,7 +123,12 @@ EXPORT_SYMBOL_GPL(kvm_set_msi_irq);
static inline bool kvm_msi_route_invalid(struct kvm *kvm,
struct kvm_kernel_irq_routing_entry *e)
{
- return kvm->arch.x2apic_format && (e->msi.address_hi & 0xff);
+ struct msi_msg msg = { .address_lo = e->msi.address_lo,
+ .address_hi = e->msi.address_hi,
+ .data = e->msi.data };
+ return (kvm_eoi_intercept_disallowed(kvm) &&
+ msg.arch_data.is_level) ||
+ (kvm->arch.x2apic_format && (msg.address_hi & 0xff));
}
int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
@@ -385,7 +390,7 @@ int kvm_setup_empty_irq_routing(struct kvm *kvm)
void kvm_arch_post_irq_routing_update(struct kvm *kvm)
{
- if (!irqchip_split(kvm))
+ if (!irqchip_split(kvm) || kvm_eoi_intercept_disallowed(kvm))
return;
kvm_make_scan_ioapic_request(kvm);
}
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d6ac32f3f650..235971c016d9 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -281,7 +281,8 @@ void kvm_recalculate_apic_map(struct kvm *kvm)
if (old)
call_rcu(&old->rcu, kvm_apic_map_free);
- kvm_make_scan_ioapic_request(kvm);
+ if (!kvm_eoi_intercept_disallowed(kvm))
+ kvm_make_scan_ioapic_request(kvm);
}
static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 34dd93b29932..113ed9aa5c82 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6023,6 +6023,9 @@ long kvm_arch_vm_ioctl(struct file *filp,
goto create_irqchip_unlock;
r = -EINVAL;
+ if (kvm_eoi_intercept_disallowed(kvm))
+ goto create_irqchip_unlock;
+
if (kvm->created_vcpus)
goto create_irqchip_unlock;
@@ -6053,6 +6056,9 @@ long kvm_arch_vm_ioctl(struct file *filp,
u.pit_config.flags = KVM_PIT_SPEAKER_DUMMY;
goto create_pit;
case KVM_CREATE_PIT2:
+ r = -EINVAL;
+ if (kvm_eoi_intercept_disallowed(kvm))
+ goto out;
r = -EFAULT;
if (copy_from_user(&u.pit_config, argp,
sizeof(struct kvm_pit_config)))
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 0d8435b32bf5..65c8c77e507b 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -451,6 +451,11 @@ static __always_inline bool kvm_tsc_immutable(struct kvm_vcpu *vcpu)
return vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM;
}
+static __always_inline bool kvm_eoi_intercept_disallowed(struct kvm *kvm)
+{
+ return kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
int kvm_spec_ctrl_test_value(u64 value);
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 07/11] KVM: x86: Disable SMM for TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (5 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 06/11] KVM: x86: Disable in-kernel I/O APIC and level routes " Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 18:04 ` Sean Christopherson
2021-11-12 15:37 ` [PATCH 08/11] KVM: x86: Disable INIT/SIPI " Xiaoyao Li
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
SMM is not supported for TDX VM, nor can KVM emulate it for TDX VM.
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/irq_comm.c | 2 ++
arch/x86/kvm/x86.c | 6 ++++++
arch/x86/kvm/x86.h | 5 +++++
3 files changed, 13 insertions(+)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index f9f643e31893..705fc0dc0272 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -128,6 +128,8 @@ static inline bool kvm_msi_route_invalid(struct kvm *kvm,
.data = e->msi.data };
return (kvm_eoi_intercept_disallowed(kvm) &&
msg.arch_data.is_level) ||
+ (kvm_smm_unsupported(kvm) &&
+ msg.arch_data.delivery_mode == APIC_DELIVERY_MODE_SMI) ||
(kvm->arch.x2apic_format && (msg.address_hi & 0xff));
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 113ed9aa5c82..1f3cc2a2d844 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4132,6 +4132,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
r |= KVM_X86_DISABLE_EXITS_MWAIT;
break;
case KVM_CAP_X86_SMM:
+ if (kvm && kvm_smm_unsupported(kvm))
+ break;
+
/* SMBASE is usually relocated above 1M on modern chipsets,
* and SMM handlers might indeed rely on 4G segment limits,
* so do not report SMM to be available if real mode is
@@ -4500,6 +4503,9 @@ static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
static int kvm_vcpu_ioctl_smi(struct kvm_vcpu *vcpu)
{
+ if (kvm_smm_unsupported(vcpu->kvm))
+ return -EINVAL;
+
kvm_make_request(KVM_REQ_SMI, vcpu);
return 0;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 65c8c77e507b..ab7c91ca2478 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -456,6 +456,11 @@ static __always_inline bool kvm_eoi_intercept_disallowed(struct kvm *kvm)
return kvm->arch.vm_type == KVM_X86_TDX_VM;
}
+static __always_inline bool kvm_smm_unsupported(struct kvm *kvm)
+{
+ return kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
int kvm_spec_ctrl_test_value(u64 value);
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 07/11] KVM: x86: Disable SMM for TDX
2021-11-12 15:37 ` [PATCH 07/11] KVM: x86: Disable SMM " Xiaoyao Li
@ 2021-11-12 18:04 ` Sean Christopherson
2021-11-12 18:35 ` Sean Christopherson
2021-12-01 6:29 ` Xiaoyao Li
0 siblings, 2 replies; 22+ messages in thread
From: Sean Christopherson @ 2021-11-12 18:04 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On Fri, Nov 12, 2021, Xiaoyao Li wrote:
> SMM is not supported for TDX VM, nor can KVM emulate it for TDX VM.
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/kvm/irq_comm.c | 2 ++
> arch/x86/kvm/x86.c | 6 ++++++
> arch/x86/kvm/x86.h | 5 +++++
> 3 files changed, 13 insertions(+)
>
> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> index f9f643e31893..705fc0dc0272 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -128,6 +128,8 @@ static inline bool kvm_msi_route_invalid(struct kvm *kvm,
> .data = e->msi.data };
> return (kvm_eoi_intercept_disallowed(kvm) &&
> msg.arch_data.is_level) ||
> + (kvm_smm_unsupported(kvm) &&
> + msg.arch_data.delivery_mode == APIC_DELIVERY_MODE_SMI) ||
This patch neglects to disallow SMI via IPI. Ditto for INIT+SIPI in the next
patch. And no small part of me thinks we shouldn't even bother handling the
delivery mode in the MSI routing. If we reject MSI configuration, then to be
consistent we should also technically reject guest attempts to configure LVT
entries. Sadly, KVM doesn't handle any of that stuff correctly as there are
assumptions left and right about how the guest will configure things like LVTT,
but from an architctural perspective it is legal to configure LVT0, LVT1, LVTT,
etc... to send SMI, NMI, INIT, etc...
The kvm_eoi_intercept_disallowed() part is a little different, since KVM can
deliver the interrupt, it just can handle the backend correctly. Dropping an
event on the floor is a bit gross, but on the other hand I really don't want to
sign up for a game of whack-a-mole for all the paths that can get to
__apic_accept_irq().
E.g. I'm thinking:
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 76fb00921203..33364d3e4d02 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1112,6 +1112,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
break;
case APIC_DM_SMI:
+ if (kvm_smi_disallowed(vcpu->kvm))
+ break;
+
result = 1;
kvm_make_request(KVM_REQ_SMI, vcpu);
kvm_vcpu_kick(vcpu);
@@ -1124,6 +1127,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
break;
case APIC_DM_INIT:
+ if (kvm_init_disallowed(vcpu->kvm))
+ break;
+
if (!trig_mode || level) {
result = 1;
/* assumes that there are only KVM_APIC_INIT/SIPI */
@@ -1134,6 +1140,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
break;
case APIC_DM_STARTUP:
+ if (kvm_sipi_disallowed(vcpu->kvm))
+ break;
+
result = 1;
apic->sipi_vector = vector;
/* make sure sipi_vector is visible for the receiver */
> (kvm->arch.x2apic_format && (msg.address_hi & 0xff));
> }
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 113ed9aa5c82..1f3cc2a2d844 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4132,6 +4132,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> r |= KVM_X86_DISABLE_EXITS_MWAIT;
> break;
> case KVM_CAP_X86_SMM:
> + if (kvm && kvm_smm_unsupported(kvm))
> + break;
> +
> /* SMBASE is usually relocated above 1M on modern chipsets,
> * and SMM handlers might indeed rely on 4G segment limits,
> * so do not report SMM to be available if real mode is
> @@ -4500,6 +4503,9 @@ static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
>
> static int kvm_vcpu_ioctl_smi(struct kvm_vcpu *vcpu)
> {
> + if (kvm_smm_unsupported(vcpu->kvm))
> + return -EINVAL;
> +
> kvm_make_request(KVM_REQ_SMI, vcpu);
>
> return 0;
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 65c8c77e507b..ab7c91ca2478 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -456,6 +456,11 @@ static __always_inline bool kvm_eoi_intercept_disallowed(struct kvm *kvm)
> return kvm->arch.vm_type == KVM_X86_TDX_VM;
> }
>
> +static __always_inline bool kvm_smm_unsupported(struct kvm *kvm)
This should be "kvm_smi_disallowed" to be consistent with the other helpers. Also,
why are these all __always_inline? Generally speaking, __always_inline should
really only be used if there is a hard dependency on the function being inlined.
I would be extremely surprised if it actually changed anything in this case, but
it's odd and unnecessary.
> +{
> + return kvm->arch.vm_type == KVM_X86_TDX_VM;
There really needs to be a helper for this:
static inline bool is_tdx_guest(struct kvm *kvm*)
{
return kvm->arch.vm_type == KVM_X86_TDX_VM;
}
And I think we should bite the bullet and expose SEV-ES status in x86. Ideally,
we would not have had to do that, but TDX and SEV diverge just enough that a single
guest_state_protected doesn't suffice :-( Whining aside, exposing TDX in x86 but
not SEV-ES will create a weird split where some things are handled in common x86
and others are deferred to vendor code.
And I think it would make sense to tie the "smi disallowed" part to whether or
not KVM can emulate an instruction, because that's really the issue. E.g.
static inline bool kvm_smi_disallowed(struct kvm *kvm)
{
/* SMM requires emulation in KVM. */
return __kvm_can_emulate_instruction(kvm);
}
And then the existing kvm_x86_ops.can_emulation_instruction() can be folded into
a helper that checks both the "can this VM emulating _anything_" as well as the
"can this specific instruction be emulated".
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 21bb81710e0f..7af4393ccecd 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4465,12 +4465,6 @@ static bool svm_can_emulate_instruction(struct kvm_vcpu *vcpu, void *insn, int i
bool smep, smap, is_user;
unsigned long cr4;
- /*
- * When the guest is an SEV-ES guest, emulation is not possible.
- */
- if (sev_es_guest(vcpu->kvm))
- return false;
-
/*
* Detect and workaround Errata 1096 Fam_17h_00_0Fh.
*
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9a0440e22ede..c34f653e2546 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6717,6 +6717,18 @@ int kvm_write_guest_virt_system(struct kvm_vcpu *vcpu, gva_t addr, void *val,
}
EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
+static bool __kvm_can_emulate_instruction(struct kvm *kvm)
+{
+ return !is_sev_guest(kvm) && !is_tdx_guest(kvm);
+}
+
+static bool kvm_can_emulate_instruction(struct kvm_vcpu *vcpu,
+ void *insn, int insn_len)
+{
+ return __kvm_can_emulate_instruction(vcpu->kvm) &&
+ static_call(kvm_x86_can_emulate_instruction)(vcpu, NULL, 0);
+}
+
int handle_ud(struct kvm_vcpu *vcpu)
{
static const char kvm_emulate_prefix[] = { __KVM_EMULATE_PREFIX };
@@ -6724,7 +6736,7 @@ int handle_ud(struct kvm_vcpu *vcpu)
char sig[5]; /* ud2; .ascii "kvm" */
struct x86_exception e;
- if (unlikely(!static_call(kvm_x86_can_emulate_instruction)(vcpu, NULL, 0)))
+ if (unlikely(!kvm_can_emulate_instruction(vcpu, NULL, 0)))
return 1;
if (force_emulation_prefix &&
@@ -8071,7 +8083,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
bool writeback = true;
bool write_fault_to_spt;
- if (unlikely(!static_call(kvm_x86_can_emulate_instruction)(vcpu, insn, insn_len)))
+ if (unlikely(!kvm_can_emulate_instruction(vcpu, insn, insn_len)))
return 1;
vcpu->arch.l1tf_flush_l1d = true;
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 07/11] KVM: x86: Disable SMM for TDX
2021-11-12 18:04 ` Sean Christopherson
@ 2021-11-12 18:35 ` Sean Christopherson
2021-12-01 6:29 ` Xiaoyao Li
1 sibling, 0 replies; 22+ messages in thread
From: Sean Christopherson @ 2021-11-12 18:35 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On Fri, Nov 12, 2021, Sean Christopherson wrote:
> On Fri, Nov 12, 2021, Xiaoyao Li wrote:
> This patch neglects to disallow SMI via IPI. Ditto for INIT+SIPI in the next
> patch. And no small part of me thinks we shouldn't even bother handling the
> delivery mode in the MSI routing. If we reject MSI configuration, then to be
> consistent we should also technically reject guest attempts to configure LVT
> entries. Sadly, KVM doesn't handle any of that stuff correctly as there are
> assumptions left and right about how the guest will configure things like LVTT,
> but from an architctural perspective it is legal to configure LVT0, LVT1, LVTT,
> etc... to send SMI, NMI, INIT, etc...
>
> The kvm_eoi_intercept_disallowed() part is a little different, since KVM can
> deliver the interrupt, it just can handle the backend correctly. Dropping an
> event on the floor is a bit gross, but on the other hand I really don't want to
> sign up for a game of whack-a-mole for all the paths that can get to
> __apic_accept_irq().
...
> > static int kvm_vcpu_ioctl_smi(struct kvm_vcpu *vcpu)
> > {
> > + if (kvm_smm_unsupported(vcpu->kvm))
> > + return -EINVAL;
> > +
Oh, and despite saying we should "allow" the MSI routing, I do think we should
reject this ioctl(). The difference in my mind is that for the ioctl(), it's KVM's
"architecture" and we can define it as we see fit, whereas things like APIC delivery
mode are x86 architecture and we should do our best to avoid making things up.
We're obviously still making up behavior to some extent, but it's at least somewhat
aligned with hardware since I believe the architcture just says "undefined behavior"
for invalid modes/combinations, and AFAIK configuring invalid/reserved delivery modes
will "succeed" in the sense that the CPU won't explode on the write and the APIC
won't catch fire.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 07/11] KVM: x86: Disable SMM for TDX
2021-11-12 18:04 ` Sean Christopherson
2021-11-12 18:35 ` Sean Christopherson
@ 2021-12-01 6:29 ` Xiaoyao Li
1 sibling, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-12-01 6:29 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On 11/13/2021 2:04 AM, Sean Christopherson wrote:
> On Fri, Nov 12, 2021, Xiaoyao Li wrote:
>> SMM is not supported for TDX VM, nor can KVM emulate it for TDX VM.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>> arch/x86/kvm/irq_comm.c | 2 ++
>> arch/x86/kvm/x86.c | 6 ++++++
>> arch/x86/kvm/x86.h | 5 +++++
>> 3 files changed, 13 insertions(+)
>>
>> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
>> index f9f643e31893..705fc0dc0272 100644
>> --- a/arch/x86/kvm/irq_comm.c
>> +++ b/arch/x86/kvm/irq_comm.c
>> @@ -128,6 +128,8 @@ static inline bool kvm_msi_route_invalid(struct kvm *kvm,
>> .data = e->msi.data };
>> return (kvm_eoi_intercept_disallowed(kvm) &&
>> msg.arch_data.is_level) ||
>> + (kvm_smm_unsupported(kvm) &&
>> + msg.arch_data.delivery_mode == APIC_DELIVERY_MODE_SMI) ||
>
> This patch neglects to disallow SMI via IPI. Ditto for INIT+SIPI in the next
> patch. And no small part of me thinks we shouldn't even bother handling the
> delivery mode in the MSI routing. If we reject MSI configuration, then to be
> consistent we should also technically reject guest attempts to configure LVT
> entries. Sadly, KVM doesn't handle any of that stuff correctly as there are
> assumptions left and right about how the guest will configure things like LVTT,
> but from an architctural perspective it is legal to configure LVT0, LVT1, LVTT,
> etc... to send SMI, NMI, INIT, etc...
>
> The kvm_eoi_intercept_disallowed() part is a little different, since KVM can
> deliver the interrupt, it just can handle the backend correctly. Dropping an
> event on the floor is a bit gross, but on the other hand I really don't want to
> sign up for a game of whack-a-mole for all the paths that can get to
> __apic_accept_irq().
>
> E.g. I'm thinking:
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 76fb00921203..33364d3e4d02 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1112,6 +1112,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> break;
>
> case APIC_DM_SMI:
> + if (kvm_smi_disallowed(vcpu->kvm))
> + break;
> +
> result = 1;
> kvm_make_request(KVM_REQ_SMI, vcpu);
> kvm_vcpu_kick(vcpu);
> @@ -1124,6 +1127,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> break;
>
> case APIC_DM_INIT:
> + if (kvm_init_disallowed(vcpu->kvm))
> + break;
> +
> if (!trig_mode || level) {
> result = 1;
> /* assumes that there are only KVM_APIC_INIT/SIPI */
> @@ -1134,6 +1140,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> break;
>
> case APIC_DM_STARTUP:
> + if (kvm_sipi_disallowed(vcpu->kvm))
> + break;
> +
> result = 1;
> apic->sipi_vector = vector;
> /* make sure sipi_vector is visible for the receiver */
>
>
This looks better. We'll use this.
>> (kvm->arch.x2apic_format && (msg.address_hi & 0xff));
>> }
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 113ed9aa5c82..1f3cc2a2d844 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4132,6 +4132,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> r |= KVM_X86_DISABLE_EXITS_MWAIT;
>> break;
>> case KVM_CAP_X86_SMM:
>> + if (kvm && kvm_smm_unsupported(kvm))
>> + break;
>> +
>> /* SMBASE is usually relocated above 1M on modern chipsets,
>> * and SMM handlers might indeed rely on 4G segment limits,
>> * so do not report SMM to be available if real mode is
>> @@ -4500,6 +4503,9 @@ static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
>>
>> static int kvm_vcpu_ioctl_smi(struct kvm_vcpu *vcpu)
>> {
>> + if (kvm_smm_unsupported(vcpu->kvm))
>> + return -EINVAL;
>> +
>> kvm_make_request(KVM_REQ_SMI, vcpu);
>>
>> return 0;
>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
>> index 65c8c77e507b..ab7c91ca2478 100644
>> --- a/arch/x86/kvm/x86.h
>> +++ b/arch/x86/kvm/x86.h
>> @@ -456,6 +456,11 @@ static __always_inline bool kvm_eoi_intercept_disallowed(struct kvm *kvm)
>> return kvm->arch.vm_type == KVM_X86_TDX_VM;
>> }
>>
>> +static __always_inline bool kvm_smm_unsupported(struct kvm *kvm)
>
> This should be "kvm_smi_disallowed" to be consistent with the other helpers.
Yah, will rename to it.
> Also,
> why are these all __always_inline? Generally speaking, __always_inline should
> really only be used if there is a hard dependency on the function being inlined.
> I would be extremely surprised if it actually changed anything in this case, but
> it's odd and unnecessary.
will switch to use inline
>> +{
>> + return kvm->arch.vm_type == KVM_X86_TDX_VM;
>
> There really needs to be a helper for this:
>
> static inline bool is_tdx_guest(struct kvm *kvm*)
> {
> return kvm->arch.vm_type == KVM_X86_TDX_VM;
> }
>
> And I think we should bite the bullet and expose SEV-ES status in x86. Ideally,
> we would not have had to do that, but TDX and SEV diverge just enough that a single
> guest_state_protected doesn't suffice :-( Whining aside, exposing TDX in x86 but
> not SEV-ES will create a weird split where some things are handled in common x86
> and others are deferred to vendor code.
>
> And I think it would make sense to tie the "smi disallowed" part to whether or
> not KVM can emulate an instruction, because that's really the issue. E.g.
good idea, but I would leave it to another patch after people agree with
the 3 original helper {smi,init,sipi}_disallowed()
> static inline bool kvm_smi_disallowed(struct kvm *kvm)
> {
> /* SMM requires emulation in KVM. */
> return __kvm_can_emulate_instruction(kvm);
> }
>
>
> And then the existing kvm_x86_ops.can_emulation_instruction() can be folded into
> a helper that checks both the "can this VM emulating _anything_" as well as the
> "can this specific instruction be emulated".
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 21bb81710e0f..7af4393ccecd 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4465,12 +4465,6 @@ static bool svm_can_emulate_instruction(struct kvm_vcpu *vcpu, void *insn, int i
> bool smep, smap, is_user;
> unsigned long cr4;
>
> - /*
> - * When the guest is an SEV-ES guest, emulation is not possible.
> - */
> - if (sev_es_guest(vcpu->kvm))
> - return false;
> -
> /*
> * Detect and workaround Errata 1096 Fam_17h_00_0Fh.
> *
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 9a0440e22ede..c34f653e2546 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6717,6 +6717,18 @@ int kvm_write_guest_virt_system(struct kvm_vcpu *vcpu, gva_t addr, void *val,
> }
> EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
>
> +static bool __kvm_can_emulate_instruction(struct kvm *kvm)
> +{
> + return !is_sev_guest(kvm) && !is_tdx_guest(kvm);
> +}
> +
> +static bool kvm_can_emulate_instruction(struct kvm_vcpu *vcpu,
> + void *insn, int insn_len)
> +{
> + return __kvm_can_emulate_instruction(vcpu->kvm) &&
> + static_call(kvm_x86_can_emulate_instruction)(vcpu, NULL, 0);
> +}
> +
> int handle_ud(struct kvm_vcpu *vcpu)
> {
> static const char kvm_emulate_prefix[] = { __KVM_EMULATE_PREFIX };
> @@ -6724,7 +6736,7 @@ int handle_ud(struct kvm_vcpu *vcpu)
> char sig[5]; /* ud2; .ascii "kvm" */
> struct x86_exception e;
>
> - if (unlikely(!static_call(kvm_x86_can_emulate_instruction)(vcpu, NULL, 0)))
> + if (unlikely(!kvm_can_emulate_instruction(vcpu, NULL, 0)))
> return 1;
>
> if (force_emulation_prefix &&
> @@ -8071,7 +8083,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> bool writeback = true;
> bool write_fault_to_spt;
>
> - if (unlikely(!static_call(kvm_x86_can_emulate_instruction)(vcpu, insn, insn_len)))
> + if (unlikely(!kvm_can_emulate_instruction(vcpu, insn, insn_len)))
> return 1;
>
> vcpu->arch.l1tf_flush_l1d = true;
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 08/11] KVM: x86: Disable INIT/SIPI for TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (6 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 07/11] KVM: x86: Disable SMM " Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 15:37 ` [PATCH 09/11] KVM: x86: Block ioctls to access guest state " Xiaoyao Li
` (2 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
INIT/SIPI is not supportd for TDX guest. Disallow injecting interrupt with
delivery mode of INIT/SIPI, and add a check to reject INIT/SIPI
interrupt delivery mode.
Co-developed-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/irq_comm.c | 2 ++
arch/x86/kvm/x86.h | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 705fc0dc0272..c51faa621aa4 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -130,6 +130,8 @@ static inline bool kvm_msi_route_invalid(struct kvm *kvm,
msg.arch_data.is_level) ||
(kvm_smm_unsupported(kvm) &&
msg.arch_data.delivery_mode == APIC_DELIVERY_MODE_SMI) ||
+ (kvm_init_sipi_unsupported(kvm) &&
+ msg.arch_data.delivery_mode == APIC_DELIVERY_MODE_INIT) ||
(kvm->arch.x2apic_format && (msg.address_hi & 0xff));
}
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index ab7c91ca2478..2203cc283e04 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -461,6 +461,11 @@ static __always_inline bool kvm_smm_unsupported(struct kvm *kvm)
return kvm->arch.vm_type == KVM_X86_TDX_VM;
}
+static __always_inline bool kvm_init_sipi_unsupported(struct kvm *kvm)
+{
+ return kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
int kvm_spec_ctrl_test_value(u64 value);
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 09/11] KVM: x86: Block ioctls to access guest state for TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (7 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 08/11] KVM: x86: Disable INIT/SIPI " Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 15:37 ` [PATCH 10/11] KVM: Disallow read-only memory for x86 TDX Xiaoyao Li
2021-11-12 15:37 ` [PATCH 11/11] KVM: Disallow dirty logging " Xiaoyao Li
10 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Sean Christopherson <sean.j.christopherson@intel.com>
For non-debug TDX guest, its states (REGS, SREGS, FPU, XSAVE, XCRS, etc)
cannot be accessed (neither get nor set). Return an error if userspace
attempts to get/set register state for a TDX guest. KVM can't provide
sane data, it's userspace's responsibility to avoid attempting to read
guest state when it's known to be inaccessible.
Retrieving vCPU events is the one exception, as the userspace VMM is
allowed to inject NMIs.
Note, for debug TD, most guest state will become accesible. It's future
work when enabling debug TD support
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/x86.c | 105 +++++++++++++++++++++++++++++++++++++--------
arch/x86/kvm/x86.h | 5 +++
2 files changed, 93 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1f3cc2a2d844..d06ee07bd486 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4645,7 +4645,8 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
vcpu->arch.interrupt.injected && !vcpu->arch.interrupt.soft;
events->interrupt.nr = vcpu->arch.interrupt.nr;
events->interrupt.soft = 0;
- events->interrupt.shadow = static_call(kvm_x86_get_interrupt_shadow)(vcpu);
+ if (!kvm_guest_state_inaccesible(vcpu))
+ events->interrupt.shadow = static_call(kvm_x86_get_interrupt_shadow)(vcpu);
events->nmi.injected = vcpu->arch.nmi_injected;
events->nmi.pending = vcpu->arch.nmi_pending != 0;
@@ -4671,14 +4672,24 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
static void kvm_smm_changed(struct kvm_vcpu *vcpu, bool entering_smm);
+static inline u32 kvm_get_allowed_vcpu_event_flags(struct kvm_vcpu *vcpu)
+{
+ if (vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM)
+ return KVM_VCPUEVENT_VALID_NMI_PENDING;
+ else
+ return KVM_VCPUEVENT_VALID_NMI_PENDING |
+ KVM_VCPUEVENT_VALID_SIPI_VECTOR |
+ KVM_VCPUEVENT_VALID_SHADOW |
+ KVM_VCPUEVENT_VALID_SMM |
+ KVM_VCPUEVENT_VALID_PAYLOAD;
+}
+
static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
struct kvm_vcpu_events *events)
{
- if (events->flags & ~(KVM_VCPUEVENT_VALID_NMI_PENDING
- | KVM_VCPUEVENT_VALID_SIPI_VECTOR
- | KVM_VCPUEVENT_VALID_SHADOW
- | KVM_VCPUEVENT_VALID_SMM
- | KVM_VCPUEVENT_VALID_PAYLOAD))
+ u32 allowed_flags = kvm_get_allowed_vcpu_event_flags(vcpu);
+
+ if (events->flags & ~allowed_flags)
return -EINVAL;
if (events->flags & KVM_VCPUEVENT_VALID_PAYLOAD) {
@@ -4754,17 +4765,22 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
return 0;
}
-static void kvm_vcpu_ioctl_x86_get_debugregs(struct kvm_vcpu *vcpu,
- struct kvm_debugregs *dbgregs)
+static int kvm_vcpu_ioctl_x86_get_debugregs(struct kvm_vcpu *vcpu,
+ struct kvm_debugregs *dbgregs)
{
unsigned long val;
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
memcpy(dbgregs->db, vcpu->arch.db, sizeof(vcpu->arch.db));
kvm_get_dr(vcpu, 6, &val);
dbgregs->dr6 = val;
dbgregs->dr7 = vcpu->arch.dr7;
dbgregs->flags = 0;
memset(&dbgregs->reserved, 0, sizeof(dbgregs->reserved));
+
+ return 0;
}
static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
@@ -4778,6 +4794,9 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
if (!kvm_dr7_valid(dbgregs->dr7))
return -EINVAL;
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
memcpy(vcpu->arch.db, dbgregs->db, sizeof(vcpu->arch.db));
kvm_update_dr0123(vcpu);
vcpu->arch.dr6 = dbgregs->dr6;
@@ -4787,21 +4806,28 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
return 0;
}
-static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
+static int kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
struct kvm_xsave *guest_xsave)
{
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
- return;
+ return 0;
fpu_copy_guest_fpstate_to_uabi(&vcpu->arch.guest_fpu,
guest_xsave->region,
sizeof(guest_xsave->region),
vcpu->arch.pkru);
+ return 0;
}
static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
struct kvm_xsave *guest_xsave)
{
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
return 0;
@@ -4810,18 +4836,22 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
supported_xcr0, &vcpu->arch.pkru);
}
-static void kvm_vcpu_ioctl_x86_get_xcrs(struct kvm_vcpu *vcpu,
- struct kvm_xcrs *guest_xcrs)
+static int kvm_vcpu_ioctl_x86_get_xcrs(struct kvm_vcpu *vcpu,
+ struct kvm_xcrs *guest_xcrs)
{
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
if (!boot_cpu_has(X86_FEATURE_XSAVE)) {
guest_xcrs->nr_xcrs = 0;
- return;
+ return 0;
}
guest_xcrs->nr_xcrs = 1;
guest_xcrs->flags = 0;
guest_xcrs->xcrs[0].xcr = XCR_XFEATURE_ENABLED_MASK;
guest_xcrs->xcrs[0].value = vcpu->arch.xcr0;
+ return 0;
}
static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
@@ -4829,6 +4859,9 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
{
int i, r = 0;
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
if (!boot_cpu_has(X86_FEATURE_XSAVE))
return -EINVAL;
@@ -5220,7 +5253,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
case KVM_GET_DEBUGREGS: {
struct kvm_debugregs dbgregs;
- kvm_vcpu_ioctl_x86_get_debugregs(vcpu, &dbgregs);
+ r = kvm_vcpu_ioctl_x86_get_debugregs(vcpu, &dbgregs);
+ if (r)
+ break;
r = -EFAULT;
if (copy_to_user(argp, &dbgregs,
@@ -5246,7 +5281,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
if (!u.xsave)
break;
- kvm_vcpu_ioctl_x86_get_xsave(vcpu, u.xsave);
+ r = kvm_vcpu_ioctl_x86_get_xsave(vcpu, u.xsave);
+ if (r)
+ break;
r = -EFAULT;
if (copy_to_user(argp, u.xsave, sizeof(struct kvm_xsave)))
@@ -5270,7 +5307,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
if (!u.xcrs)
break;
- kvm_vcpu_ioctl_x86_get_xcrs(vcpu, u.xcrs);
+ r = kvm_vcpu_ioctl_x86_get_xcrs(vcpu, u.xcrs);
+ if (r)
+ break;
r = -EFAULT;
if (copy_to_user(argp, u.xcrs,
@@ -5413,6 +5452,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
}
#endif
case KVM_GET_SREGS2: {
+ r = -EINVAL;
+ if (kvm_guest_state_inaccesible(vcpu))
+ goto out;
+
u.sregs2 = kzalloc(sizeof(struct kvm_sregs2), GFP_KERNEL);
r = -ENOMEM;
if (!u.sregs2)
@@ -5425,6 +5468,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
break;
}
case KVM_SET_SREGS2: {
+ r = -EINVAL;
+ if (kvm_guest_state_inaccesible(vcpu))
+ goto out;
+
u.sregs2 = memdup_user(argp, sizeof(struct kvm_sregs2));
if (IS_ERR(u.sregs2)) {
r = PTR_ERR(u.sregs2);
@@ -10148,6 +10195,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
goto out;
}
+ if (kvm_guest_state_inaccesible(vcpu) &&
+ (kvm_run->kvm_valid_regs || kvm_run->kvm_dirty_regs)) {
+ r = -EINVAL;
+ goto out;
+ }
+
if (kvm_run->kvm_dirty_regs) {
r = sync_regs(vcpu);
if (r != 0)
@@ -10178,7 +10231,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
out:
kvm_put_guest_fpu(vcpu);
- if (kvm_run->kvm_valid_regs)
+ if (kvm_run->kvm_valid_regs && !kvm_guest_state_inaccesible(vcpu))
store_regs(vcpu);
post_kvm_run_save(vcpu);
kvm_sigset_deactivate(vcpu);
@@ -10225,6 +10278,9 @@ static void __get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
{
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
vcpu_load(vcpu);
__get_regs(vcpu, regs);
vcpu_put(vcpu);
@@ -10265,6 +10321,9 @@ static void __set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
{
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
vcpu_load(vcpu);
__set_regs(vcpu, regs);
vcpu_put(vcpu);
@@ -10347,6 +10406,9 @@ static void __get_sregs2(struct kvm_vcpu *vcpu, struct kvm_sregs2 *sregs2)
int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,
struct kvm_sregs *sregs)
{
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
vcpu_load(vcpu);
__get_sregs(vcpu, sregs);
vcpu_put(vcpu);
@@ -10595,6 +10657,9 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
{
int ret;
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
vcpu_load(vcpu);
ret = __set_sregs(vcpu, sregs);
vcpu_put(vcpu);
@@ -10688,6 +10753,9 @@ int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
{
struct fxregs_state *fxsave;
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
return 0;
@@ -10711,6 +10779,9 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
{
struct fxregs_state *fxsave;
+ if (kvm_guest_state_inaccesible(vcpu))
+ return -EINVAL;
+
if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
return 0;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 2203cc283e04..6b3cf1fdcf08 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -466,6 +466,11 @@ static __always_inline bool kvm_init_sipi_unsupported(struct kvm *kvm)
return kvm->arch.vm_type == KVM_X86_TDX_VM;
}
+static __always_inline bool kvm_guest_state_inaccesible(struct kvm_vcpu *vcpu)
+{
+ return vcpu->kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
int kvm_spec_ctrl_test_value(u64 value);
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 10/11] KVM: Disallow read-only memory for x86 TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (8 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 09/11] KVM: x86: Block ioctls to access guest state " Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
2021-11-12 16:52 ` Sean Christopherson
2021-11-12 15:37 ` [PATCH 11/11] KVM: Disallow dirty logging " Xiaoyao Li
10 siblings, 1 reply; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Isaku Yamahata <isaku.yamahata@intel.com>
TDX doesn't expose permission bits to the VMM in the SEPT tables, i.e.,
doesn't support read-only private memory.
Introduce kvm_arch_support_readonly_mem(), which returns true except for
x86. x86 has its own implementation based on vm_type that returns faluse
for TDX VM.
Propagate it to KVM_CAP_READONLY_MEM to allow reporting on a per-VM
basis.
Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/x86.c | 9 ++++++++-
include/linux/kvm_host.h | 1 +
virt/kvm/kvm_main.c | 19 ++++++++++++++-----
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d06ee07bd486..f091a4d3c8f2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4079,7 +4079,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_ASYNC_PF_INT:
case KVM_CAP_GET_TSC_KHZ:
case KVM_CAP_KVMCLOCK_CTRL:
- case KVM_CAP_READONLY_MEM:
case KVM_CAP_HYPERV_TIME:
case KVM_CAP_IOAPIC_POLARITY_IGNORED:
case KVM_CAP_TSC_DEADLINE_TIMER:
@@ -4202,6 +4201,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
if (static_call(kvm_x86_is_vm_type_supported)(KVM_X86_TDX_VM))
r |= BIT(KVM_X86_TDX_VM);
break;
+ case KVM_CAP_READONLY_MEM:
+ r = kvm && !kvm_arch_support_readonly_mem(kvm) ? 0 : 1;
+ break;
default:
break;
}
@@ -12609,6 +12611,11 @@ int kvm_sev_es_string_io(struct kvm_vcpu *vcpu, unsigned int size,
}
EXPORT_SYMBOL_GPL(kvm_sev_es_string_io);
+bool kvm_arch_support_readonly_mem(struct kvm *kvm)
+{
+ return kvm->arch.vm_type != KVM_X86_TDX_VM;
+}
+
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_entry);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 60a35d9fe259..1dbabf199c13 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1071,6 +1071,7 @@ bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
int kvm_arch_post_init_vm(struct kvm *kvm);
void kvm_arch_pre_destroy_vm(struct kvm *kvm);
int kvm_arch_create_vm_debugfs(struct kvm *kvm);
+bool kvm_arch_support_readonly_mem(struct kvm *kvm);
#ifndef __KVM_HAVE_ARCH_VM_ALLOC
/*
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 3f6d450355f0..c288c92a9c82 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1421,13 +1421,22 @@ static void update_memslots(struct kvm_memslots *slots,
}
}
-static int check_memory_region_flags(const struct kvm_userspace_memory_region *mem)
+bool __weak kvm_arch_support_readonly_mem(struct kvm *kvm)
{
- u32 valid_flags = KVM_MEM_LOG_DIRTY_PAGES;
-
#ifdef __KVM_HAVE_READONLY_MEM
- valid_flags |= KVM_MEM_READONLY;
+ return true;
+#else
+ return false;
#endif
+}
+
+static int check_memory_region_flags(struct kvm *kvm,
+ const struct kvm_userspace_memory_region *mem)
+{
+ u32 valid_flags = KVM_MEM_LOG_DIRTY_PAGES;
+
+ if (kvm_arch_support_readonly_mem(kvm))
+ valid_flags |= KVM_MEM_READONLY;
if (mem->flags & ~valid_flags)
return -EINVAL;
@@ -1664,7 +1673,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
int as_id, id;
int r;
- r = check_memory_region_flags(mem);
+ r = check_memory_region_flags(kvm, mem);
if (r)
return r;
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 10/11] KVM: Disallow read-only memory for x86 TDX
2021-11-12 15:37 ` [PATCH 10/11] KVM: Disallow read-only memory for x86 TDX Xiaoyao Li
@ 2021-11-12 16:52 ` Sean Christopherson
2021-11-14 3:43 ` Xiaoyao Li
0 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2021-11-12 16:52 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On Fri, Nov 12, 2021, Xiaoyao Li wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> TDX doesn't expose permission bits to the VMM in the SEPT tables, i.e.,
> doesn't support read-only private memory.
>
> Introduce kvm_arch_support_readonly_mem(), which returns true except for
> x86. x86 has its own implementation based on vm_type that returns faluse
> for TDX VM.
>
> Propagate it to KVM_CAP_READONLY_MEM to allow reporting on a per-VM
> basis.
Assuming KVM gains support for private memslots (or memslots that _may_ be mapped
private), this is incorrect, the restriction on read-only memory only applies to
private memory. Userspace should still be allowed to create read-only shared memory.
Ditto for dirty-logging in the next patch.
When this patch was originally created, it was "correct" because there was no
(proposed) concept of a private memslot or of a memslot that can be mapped private.
So these two patches at least need to wait until KVM has a defind ABI for managing
guest private memory.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 10/11] KVM: Disallow read-only memory for x86 TDX
2021-11-12 16:52 ` Sean Christopherson
@ 2021-11-14 3:43 ` Xiaoyao Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-14 3:43 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
On 11/13/2021 12:52 AM, Sean Christopherson wrote:
> On Fri, Nov 12, 2021, Xiaoyao Li wrote:
>> From: Isaku Yamahata <isaku.yamahata@intel.com>
>>
>> TDX doesn't expose permission bits to the VMM in the SEPT tables, i.e.,
>> doesn't support read-only private memory.
>>
>> Introduce kvm_arch_support_readonly_mem(), which returns true except for
>> x86. x86 has its own implementation based on vm_type that returns faluse
>> for TDX VM.
>>
>> Propagate it to KVM_CAP_READONLY_MEM to allow reporting on a per-VM
>> basis.
>
> Assuming KVM gains support for private memslots (or memslots that _may_ be mapped
> private), this is incorrect, the restriction on read-only memory only applies to
> private memory. Userspace should still be allowed to create read-only shared memory.
> Ditto for dirty-logging in the next patch.
Yes. I had the same concern before sending it out. :)
But I forgot to mention it.
> When this patch was originally created, it was "correct" because there was no
> (proposed) concept of a private memslot or of a memslot that can be mapped private.
>
> So these two patches at least need to wait until KVM has a defind ABI for managing
> guest private memory.
>
I'll drop the two patches for next submission.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 11/11] KVM: Disallow dirty logging for x86 TDX
2021-11-12 15:37 [PATCH 00/11] KVM: x86: TDX preparation of introducing vm_type and blocking ioctls based on vm_type Xiaoyao Li
` (9 preceding siblings ...)
2021-11-12 15:37 ` [PATCH 10/11] KVM: Disallow read-only memory for x86 TDX Xiaoyao Li
@ 2021-11-12 15:37 ` Xiaoyao Li
10 siblings, 0 replies; 22+ messages in thread
From: Xiaoyao Li @ 2021-11-12 15:37 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel
Cc: xiaoyao.li, erdemaktas, Connor Kuehl, x86, linux-kernel, kvm,
isaku.yamahata, Kai Huang
From: Sean Christopherson <sean.j.christopherson@intel.com>
TDX doesn't support dirty logging for now.
Suggested-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Do we need to introduce a VM-scope CAP to report if dirty log is supported?
since in the future, next generation of TDX will support live migration.
---
arch/x86/kvm/x86.c | 5 +++++
include/linux/kvm_host.h | 1 +
virt/kvm/kvm_main.c | 10 +++++++++-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f091a4d3c8f2..c30933289b54 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12611,6 +12611,11 @@ int kvm_sev_es_string_io(struct kvm_vcpu *vcpu, unsigned int size,
}
EXPORT_SYMBOL_GPL(kvm_sev_es_string_io);
+bool kvm_arch_dirty_log_supported(struct kvm *kvm)
+{
+ return kvm->arch.vm_type != KVM_X86_TDX_VM;
+}
+
bool kvm_arch_support_readonly_mem(struct kvm *kvm)
{
return kvm->arch.vm_type != KVM_X86_TDX_VM;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1dbabf199c13..5f3debcd4295 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1071,6 +1071,7 @@ bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
int kvm_arch_post_init_vm(struct kvm *kvm);
void kvm_arch_pre_destroy_vm(struct kvm *kvm);
int kvm_arch_create_vm_debugfs(struct kvm *kvm);
+bool kvm_arch_dirty_log_supported(struct kvm *kvm);
bool kvm_arch_support_readonly_mem(struct kvm *kvm);
#ifndef __KVM_HAVE_ARCH_VM_ALLOC
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c288c92a9c82..4c9d1e7834eb 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1421,6 +1421,11 @@ static void update_memslots(struct kvm_memslots *slots,
}
}
+bool __weak kvm_arch_dirty_log_supported(struct kvm *kvm)
+{
+ return true;
+}
+
bool __weak kvm_arch_support_readonly_mem(struct kvm *kvm)
{
#ifdef __KVM_HAVE_READONLY_MEM
@@ -1433,7 +1438,10 @@ bool __weak kvm_arch_support_readonly_mem(struct kvm *kvm)
static int check_memory_region_flags(struct kvm *kvm,
const struct kvm_userspace_memory_region *mem)
{
- u32 valid_flags = KVM_MEM_LOG_DIRTY_PAGES;
+ u32 valid_flags = 0;
+
+ if (kvm_arch_dirty_log_supported(kvm))
+ valid_flags |= KVM_MEM_LOG_DIRTY_PAGES;
if (kvm_arch_support_readonly_mem(kvm))
valid_flags |= KVM_MEM_READONLY;
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread