| Message ID | 20220512031803.3315890-9-xiaoyao.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | TDX QEMU support | expand |
Hi, > - The supported XCR0 and XSS bits needs to be cap'ed by tdx_caps, because > KVM uses them to setup XFAM of TD. > + case 0xd: > + if (index == 0) { > + if (reg == R_EAX) { > + *ret &= (uint32_t)tdx_caps->xfam_fixed0 & XCR0_MASK; > + *ret |= (uint32_t)tdx_caps->xfam_fixed1 & XCR0_MASK; > + } else if (reg == R_EDX) { > + *ret &= (tdx_caps->xfam_fixed0 & XCR0_MASK) >> 32; > + *ret |= (tdx_caps->xfam_fixed1 & XCR0_MASK) >> 32; > + } > + } else if (index == 1) { > + /* TODO: Adjust XSS when it's supported. */ > + } > + break; > + default: > + /* TODO: Use tdx_caps to adjust CPUID leafs. */ > + break; Hmm, that looks all a bit messy and incomplete, also the commit message doesn't match the patch (describes XSS which isn't actually implemented). take care, Gerd
On 5/23/2022 5:01 PM, Gerd Hoffmann wrote: > Hi, > >> - The supported XCR0 and XSS bits needs to be cap'ed by tdx_caps, because >> KVM uses them to setup XFAM of TD. > >> + case 0xd: >> + if (index == 0) { >> + if (reg == R_EAX) { >> + *ret &= (uint32_t)tdx_caps->xfam_fixed0 & XCR0_MASK; >> + *ret |= (uint32_t)tdx_caps->xfam_fixed1 & XCR0_MASK; >> + } else if (reg == R_EDX) { >> + *ret &= (tdx_caps->xfam_fixed0 & XCR0_MASK) >> 32; >> + *ret |= (tdx_caps->xfam_fixed1 & XCR0_MASK) >> 32; >> + } >> + } else if (index == 1) { >> + /* TODO: Adjust XSS when it's supported. */ >> + } >> + break; > >> + default: >> + /* TODO: Use tdx_caps to adjust CPUID leafs. */ >> + break; > > Hmm, that looks all a bit messy and incomplete, also the commit > message doesn't match the patch (describes XSS which isn't actually > implemented). For XSS, QEMU recently got XSS MASK defined, I will use it in this patch. For other CPUID leaves, we have following patches (a series) to enable fine-grained feature control for TDX guest and CPU model for it. So the plan is to make it functional with no error in this basic series. Anyway I will update the commit message to describe clearly. > take care, > Gerd >
diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 9661f9fbd1c6..0c922e5a305a 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -567,6 +567,11 @@ typedef enum X86Seg { #define ESA_FEATURE_XFD_MASK (1U << ESA_FEATURE_XFD_BIT) +#define XCR0_MASK (XSTATE_FP_MASK | XSTATE_SSE_MASK | XSTATE_YMM_MASK | \ + XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK | \ + XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | \ + XSTATE_Hi16_ZMM_MASK | XSTATE_PKRU_MASK) + /* CPUID feature words */ typedef enum FeatureWord { FEAT_1_EDX, /* CPUID[1].EDX */ diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index f257ffda259d..0751e6e102cc 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -498,6 +498,10 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, ret |= 1U << KVM_HINTS_REALTIME; } + if (is_tdx_vm()) { + tdx_get_supported_cpuid(function, index, reg, &ret); + } + return ret; } diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c index 803154efdb91..6e3b15ba8a4a 100644 --- a/target/i386/kvm/tdx.c +++ b/target/i386/kvm/tdx.c @@ -14,11 +14,22 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qom/object_interfaces.h" +#include "standard-headers/asm-x86/kvm_para.h" #include "sysemu/kvm.h" #include "hw/i386/x86.h" #include "tdx.h" +#define TDX_SUPPORTED_KVM_FEATURES ((1ULL << KVM_FEATURE_NOP_IO_DELAY) | \ + (1ULL << KVM_FEATURE_STEAL_TIME) | \ + (1ULL << KVM_FEATURE_PV_EOI) | \ + (1ULL << KVM_FEATURE_PV_UNHALT) | \ + (1ULL << KVM_FEATURE_PV_TLB_FLUSH) | \ + (1ULL << KVM_FEATURE_PV_SEND_IPI) | \ + (1ULL << KVM_FEATURE_POLL_CONTROL) | \ + (1ULL << KVM_FEATURE_PV_SCHED_YIELD) | \ + (1ULL << KVM_FEATURE_MSI_EXT_DEST_ID)) + static TdxGuest *tdx_guest; /* It's valid after kvm_confidential_guest_init()->kvm_tdx_init() */ @@ -121,6 +132,39 @@ int tdx_kvm_init(MachineState *ms, Error **errp) return 0; } +void tdx_get_supported_cpuid(uint32_t function, uint32_t index, int reg, + uint32_t *ret) +{ + switch (function) { + case 1: + if (reg == R_ECX) { + *ret &= ~CPUID_EXT_VMX; + } + break; + case 0xd: + if (index == 0) { + if (reg == R_EAX) { + *ret &= (uint32_t)tdx_caps->xfam_fixed0 & XCR0_MASK; + *ret |= (uint32_t)tdx_caps->xfam_fixed1 & XCR0_MASK; + } else if (reg == R_EDX) { + *ret &= (tdx_caps->xfam_fixed0 & XCR0_MASK) >> 32; + *ret |= (tdx_caps->xfam_fixed1 & XCR0_MASK) >> 32; + } + } else if (index == 1) { + /* TODO: Adjust XSS when it's supported. */ + } + break; + case KVM_CPUID_FEATURES: + if (reg == R_EAX) { + *ret &= TDX_SUPPORTED_KVM_FEATURES; + } + break; + default: + /* TODO: Use tdx_caps to adjust CPUID leafs. */ + break; + } +} + /* tdx guest */ OBJECT_DEFINE_TYPE_WITH_INTERFACES(TdxGuest, tdx_guest, diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h index 4036ca2f3f99..06599b65b827 100644 --- a/target/i386/kvm/tdx.h +++ b/target/i386/kvm/tdx.h @@ -27,5 +27,7 @@ bool is_tdx_vm(void); #endif /* CONFIG_TDX */ int tdx_kvm_init(MachineState *ms, Error **errp); +void tdx_get_supported_cpuid(uint32_t function, uint32_t index, int reg, + uint32_t *ret); #endif /* QEMU_I386_TDX_H */
For TDX, the allowable CPUID configuration differs from what KVM reports for KVM scope via KVM_GET_SUPPORTED_CPUID. - Some CPUID bits are not supported for TDX VM while KVM reports the support. Mask them off for TDX VM. e.g., CPUID_EXT_VMX, some PV features. - The supported XCR0 and XSS bits needs to be cap'ed by tdx_caps, because KVM uses them to setup XFAM of TD. Introduce tdx_get_supported_cpuid() to adjust the kvm_arch_get_supported_cpuid() for TDX VM. Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> --- target/i386/cpu.h | 5 +++++ target/i386/kvm/kvm.c | 4 ++++ target/i386/kvm/tdx.c | 44 +++++++++++++++++++++++++++++++++++++++++++ target/i386/kvm/tdx.h | 2 ++ 4 files changed, 55 insertions(+)