| Message ID | 2fb62e93734163d2f367fd44e3335cd8a2bf2995.1649219184.git.kai.huang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TDX host kernel support | expand |
On 4/5/22 9:49 PM, Kai Huang wrote:
> + * Intel Trusted Domain CPU Architecture Extension spec:
In TDX guest code, we have been using TDX as "Intel Trust Domain
Extensions". It also aligns with spec. Maybe you should change
your patch set to use the same.
On 4/5/22 9:49 PM, Kai Huang wrote: > detect_seam(c); > + detect_tdx_keyids(c); Do you want to add some return value to detect_seam() and not proceed if it fails? In case if this function is going to be extended by future patch set, maybe do the same for detect_tdx_keyids()?
On Mon, 2022-04-18 at 22:39 -0700, Sathyanarayanan Kuppuswamy wrote: > > On 4/5/22 9:49 PM, Kai Huang wrote: > > + * Intel Trusted Domain CPU Architecture Extension spec: > > In TDX guest code, we have been using TDX as "Intel Trust Domain > Extensions". It also aligns with spec. Maybe you should change > your patch set to use the same. > Yeah will change to use "Intel Trust Domain ...". Thanks.
On Mon, 2022-04-18 at 22:42 -0700, Sathyanarayanan Kuppuswamy wrote: > > On 4/5/22 9:49 PM, Kai Huang wrote: > > detect_seam(c); > > + detect_tdx_keyids(c); > > Do you want to add some return value to detect_seam() and not > proceed if it fails? I don't think this function should return. However it may make sense to stop detecting TDX KeyIDs when on some cpu SEAMRR is detected as not enabled (i.e. on BSP when SEAMRR is not enabled by BIOS, or on any AP when there's BIOS bug that BIOS doesn't configure SEAMRR consistently on all cpus). The reason is TDX KeyIDs can only be accessed by software runs in SEAM mode. So if SEAMRR configuration is broken, TDX KeyID configuration probably is broken too. However detect_tdx_keyids() essentially only uses rdmsr_safe() to read some MSR, so if there's any problem, rdmsr_safe() will catch it. And SEAMRR is always checked before doing any TDX related staff later, therefore in practice there will be no problem. But anyway I guess there's no harm to add additional SEAMRR check in detect_tdx_keyids(). I'll think more on this. Thanks. > > In case if this function is going to be extended by future > patch set, maybe do the same for detect_tdx_keyids()? > I'd prefer to leaving this in current way until there's a real need.
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c index 03f35c75f439..ba2210001ea8 100644 --- a/arch/x86/virt/vmx/tdx/tdx.c +++ b/arch/x86/virt/vmx/tdx/tdx.c @@ -29,9 +29,28 @@ #define SEAMRR_ENABLED_BITS \ (SEAMRR_PHYS_MASK_ENABLED | SEAMRR_PHYS_MASK_LOCKED) +/* + * Intel Trusted Domain CPU Architecture Extension spec: + * + * IA32_MKTME_KEYID_PARTIONING: + * + * Bit [31:0]: number of MKTME KeyIDs. + * Bit [63:32]: number of TDX private KeyIDs. + * + * TDX private KeyIDs start after the last MKTME KeyID. + */ +#define MSR_IA32_MKTME_KEYID_PARTITIONING 0x00000087 + +#define TDX_KEYID_START(_keyid_part) \ + ((u32)(((_keyid_part) & 0xffffffffull) + 1)) +#define TDX_KEYID_NUM(_keyid_part) ((u32)((_keyid_part) >> 32)) + /* BIOS must configure SEAMRR registers for all cores consistently */ static u64 seamrr_base, seamrr_mask; +static u32 tdx_keyid_start; +static u32 tdx_keyid_num; + static bool __seamrr_enabled(void) { return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS; @@ -96,7 +115,60 @@ static void detect_seam(struct cpuinfo_x86 *c) detect_seam_ap(c); } +static void detect_tdx_keyids_bsp(struct cpuinfo_x86 *c) +{ + u64 keyid_part; + + /* TDX is built on MKTME, which is based on TME */ + if (!boot_cpu_has(X86_FEATURE_TME)) + return; + + if (rdmsrl_safe(MSR_IA32_MKTME_KEYID_PARTITIONING, &keyid_part)) + return; + + /* If MSR value is 0, TDX is not enabled by BIOS. */ + if (!keyid_part) + return; + + tdx_keyid_num = TDX_KEYID_NUM(keyid_part); + if (!tdx_keyid_num) + return; + + tdx_keyid_start = TDX_KEYID_START(keyid_part); +} + +static void detect_tdx_keyids_ap(struct cpuinfo_x86 *c) +{ + u64 keyid_part; + + /* + * Don't bother to detect this AP if TDX KeyIDs are + * not detected or cleared after earlier detections. + */ + if (!tdx_keyid_num) + return; + + rdmsrl(MSR_IA32_MKTME_KEYID_PARTITIONING, keyid_part); + + if ((tdx_keyid_start == TDX_KEYID_START(keyid_part)) && + (tdx_keyid_num == TDX_KEYID_NUM(keyid_part))) + return; + + pr_err("Inconsistent TDX KeyID configuration among packages by BIOS\n"); + tdx_keyid_start = 0; + tdx_keyid_num = 0; +} + +static void detect_tdx_keyids(struct cpuinfo_x86 *c) +{ + if (c == &boot_cpu_data) + detect_tdx_keyids_bsp(c); + else + detect_tdx_keyids_ap(c); +} + void tdx_detect_cpu(struct cpuinfo_x86 *c) { detect_seam(c); + detect_tdx_keyids(c); }
Pre-TDX Intel hardware has support for a memory encryption architecture called MKTME. The memory encryption hardware underpinning MKTME is also used for Intel TDX. TDX ends up "stealing" some of the physical address space from the MKTME architecture for crypto-protection to VMs. A new MSR (IA32_MKTME_KEYID_PARTITIONING) helps to enumerate how MKTME- enumerated "KeyID" space is distributed between TDX and legacy MKTME. KeyIDs reserved for TDX are called 'TDX private KeyIDs' or 'TDX KeyIDs' for short. The new MSR is per package and BIOS is responsible for partitioning MKTME KeyIDs and TDX KeyIDs consistently among all packages. Detect TDX private KeyIDs as a preparation to initialize TDX. Similar to detecting SEAMRR, detect on all cpus to detect any potential BIOS misconfiguration among packages. Signed-off-by: Kai Huang <kai.huang@intel.com> --- arch/x86/virt/vmx/tdx/tdx.c | 72 +++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)