Adjust TD setting on boot:
- Disable EPT violation #VE on private memory if TD can
control it;
- Enable virtualization of topology-related CPUID leafs
X2APIC_APICID MSR;
v2:
- Rebased;
- Allow write to TDCS_TD_CTLS to fail;
- Adjust commit messages;
Kirill A. Shutemov (4):
x86/tdx: Introduce tdg_vm_wr()
x86/tdx: Rename tdx_parse_tdinfo() to tdx_setup()
x86/tdx: Handle PENDING_EPT_VIOLATION_V2
x86/tdx: Enable ENUM_TOPOLOGY
arch/x86/coco/tdx/tdx.c | 125 ++++++++++++++++++++++++++----
arch/x86/include/asm/shared/tdx.h | 21 ++++-
2 files changed, 131 insertions(+), 15 deletions(-)
--
2.43.0
Add a helper function to write to a TD-scope metadata field and use it
to set NOTIFY_ENABLES.
The helper function will be paired with tdg_vm_rd() and will be used to
manipulate other metadata fields, not just NOTIFY_ENABLES.
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/coco/tdx/tdx.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 59776ce1c1d7..4fb36e5c4e80 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -77,6 +77,20 @@ static inline void tdcall(u64 fn, struct tdx_module_args *args)
panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
}
+static inline u64 tdg_vm_wr(u64 field, u64 value, u64 mask)
+{
+ struct tdx_module_args args = {
+ .rdx = field,
+ .r8 = value,
+ .r9 = mask,
+ };
+
+ tdcall(TDG_VM_WR, &args);
+
+ /* Old value */
+ return args.r8;
+}
+
/**
* tdx_mcall_get_report0() - Wrapper to get TDREPORT0 (a.k.a. TDREPORT
* subtype 0) using TDG.MR.REPORT TDCALL.
@@ -902,10 +916,6 @@ static void tdx_kexec_unshare_mem(void)
void __init tdx_early_init(void)
{
- struct tdx_module_args args = {
- .rdx = TDCS_NOTIFY_ENABLES,
- .r9 = -1ULL,
- };
u64 cc_mask;
u32 eax, sig[3];
@@ -924,7 +934,7 @@ void __init tdx_early_init(void)
cc_set_mask(cc_mask);
/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
- tdcall(TDG_VM_WR, &args);
+ tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
/*
* All bits above GPA width are reserved and kernel treats shared bit
--
2.43.0
Rename tdx_parse_tdinfo() to tdx_setup() and move setting NOTIFY_ENABLES
there.
The function will be extended to adjust TD configuration.
Signed-off-by: Kirill A. Shutemov <[email protected]>
Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
arch/x86/coco/tdx/tdx.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 4fb36e5c4e80..08e2bb462ce8 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -181,7 +181,7 @@ static void __noreturn tdx_panic(const char *msg)
__tdx_hypercall(&args);
}
-static void tdx_parse_tdinfo(u64 *cc_mask)
+static void tdx_setup(u64 *cc_mask)
{
struct tdx_module_args args = {};
unsigned int gpa_width;
@@ -206,6 +206,9 @@ static void tdx_parse_tdinfo(u64 *cc_mask)
gpa_width = args.rcx & GENMASK(5, 0);
*cc_mask = BIT_ULL(gpa_width - 1);
+ /* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
+ tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
+
/*
* The kernel can not handle #VE's when accessing normal kernel
* memory. Ensure that no #VE will be delivered for accesses to
@@ -930,11 +933,11 @@ void __init tdx_early_init(void)
setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
cc_vendor = CC_VENDOR_INTEL;
- tdx_parse_tdinfo(&cc_mask);
- cc_set_mask(cc_mask);
- /* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
- tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
+ /* Configure the TD */
+ tdx_setup(&cc_mask);
+
+ cc_set_mask(cc_mask);
/*
* All bits above GPA width are reserved and kernel treats shared bit
--
2.43.0
On Mon, 2024-03-25 at 12:46 +0200, Kirill A. Shutemov wrote:
> Add a helper function to write to a TD-scope metadata field and use it
> to set NOTIFY_ENABLES.
>
> The helper function will be paired with tdg_vm_rd() and will be used to
> manipulate other metadata fields, not just NOTIFY_ENABLES.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
>
Reviewed-by: Kai Huang <[email protected]>
On Mon, 2024-03-25 at 12:46 +0200, Kirill A. Shutemov wrote:
> Rename tdx_parse_tdinfo() to tdx_setup() and move setting NOTIFY_ENABLES
> there.
>
> The function will be extended to adjust TD configuration.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
>
Reviewed-by: Kai Huang <[email protected]>
On 3/25/24 3:46 AM, Kirill A. Shutemov wrote:
> Add a helper function to write to a TD-scope metadata field and use it
> to set NOTIFY_ENABLES.
>
> The helper function will be paired with tdg_vm_rd() and will be used to
> manipulate other metadata fields, not just NOTIFY_ENABLES.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> ---
Looks good to me.
Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
> arch/x86/coco/tdx/tdx.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 59776ce1c1d7..4fb36e5c4e80 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -77,6 +77,20 @@ static inline void tdcall(u64 fn, struct tdx_module_args *args)
> panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
> }
>
> +static inline u64 tdg_vm_wr(u64 field, u64 value, u64 mask)
> +{
> + struct tdx_module_args args = {
> + .rdx = field,
> + .r8 = value,
> + .r9 = mask,
> + };
> +
> + tdcall(TDG_VM_WR, &args);
> +
> + /* Old value */
> + return args.r8;
> +}
> +
> /**
> * tdx_mcall_get_report0() - Wrapper to get TDREPORT0 (a.k.a. TDREPORT
> * subtype 0) using TDG.MR.REPORT TDCALL.
> @@ -902,10 +916,6 @@ static void tdx_kexec_unshare_mem(void)
>
> void __init tdx_early_init(void)
> {
> - struct tdx_module_args args = {
> - .rdx = TDCS_NOTIFY_ENABLES,
> - .r9 = -1ULL,
> - };
> u64 cc_mask;
> u32 eax, sig[3];
>
> @@ -924,7 +934,7 @@ void __init tdx_early_init(void)
> cc_set_mask(cc_mask);
>
> /* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
> - tdcall(TDG_VM_WR, &args);
> + tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
>
> /*
> * All bits above GPA width are reserved and kernel treats shared bit
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
On Wed, 2024-04-10 at 17:37 +0300, Kirill A. Shutemov wrote:
> On Mon, Mar 25, 2024 at 12:46:03PM +0200, Kirill A. Shutemov wrote:
> > Adjust TD setting on boot:
> >
> > - Disable EPT violation #VE on private memory if TD can
> > control it;
> >
> > - Enable virtualization of topology-related CPUID leafs
> > X2APIC_APICID MSR;
>
> Any feedback?
It is missing a lot of the normal things that come in coverletters like what is
the problem and importance. It might help attract more review.
On Mon, Mar 25, 2024 at 12:46:03PM +0200, Kirill A. Shutemov wrote:
> Adjust TD setting on boot:
>
> - Disable EPT violation #VE on private memory if TD can
> control it;
>
> - Enable virtualization of topology-related CPUID leafs
> X2APIC_APICID MSR;
Any feedback?
--
Kiryl Shutsemau / Kirill A. Shutemov
On Wed, Apr 10, 2024 at 02:41:13PM +0000, Edgecombe, Rick P wrote:
> On Wed, 2024-04-10 at 17:37 +0300, Kirill A. Shutemov wrote:
> > On Mon, Mar 25, 2024 at 12:46:03PM +0200, Kirill A. Shutemov wrote:
> > > Adjust TD setting on boot:
> > >
> > > ?? - Disable EPT violation #VE on private memory if TD can
> > > ???? control it;
> > >
> > > ?? - Enable virtualization of topology-related CPUID leafs
> > > ???? X2APIC_APICID MSR;
> >
> > Any feedback?
>
> It is missing a lot of the normal things that come in coverletters like what is
> the problem and importance. It might help attract more review.
What about this:
The patchset adjusts a few TD settings on boot for the optimal functioning
of the system:
- Disable EPT violation #VE on private memory if TD can control it
The newer TDX module allows the guest to control whether it wants to
see #VE on EPT violation on private memory. The Linux kernel does not
want such #VEs and needs to disable them.
- Enable virtualization of topology-related CPUID leafs X2APIC_APICID MSR;
The ENUM_TOPOLOGY feature allows the VMM to provide topology
information to the guest. Enabling the feature eliminates
topology-related #VEs: the TDX module virtualizes accesses to the
CPUID leafs and the MSR.
It allows TDX guest to run with non-trivial topology configuration.
--
Kiryl Shutsemau / Kirill A. Shutemov
So, the text here should help me understand what is going on. What it
tries to tell me should not be a literal rephrasing of the contents of
the diff.
This patch literally introduces a function called tdg_vm_wr(). The
subject adds precisely zero to what I can get from reading the diff.
How about:
x86/tdx: Factor out TD metadata write tdcall
On 3/25/24 03:46, Kirill A. Shutemov wrote:
> Add a helper function to write to a TD-scope metadata field and use it
> to set NOTIFY_ENABLES.
>
> The helper function will be paired with tdg_vm_rd() and will be used to
> manipulate other metadata fields, not just NOTIFY_ENABLES.
Ideally this would at least tell me what problem this is solving:
The TDG_VM_WR seamcall is used to ask the TDX module to change
some TD-specific VM configuration. There is currently only one
user in the kernel of this seamcall. More will be added
shortly.
.. and then the solution:
Refactor to make way for more users of TDG_VM_WR who will need
to modify other TD configuration values.
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 59776ce1c1d7..4fb36e5c4e80 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -77,6 +77,20 @@ static inline void tdcall(u64 fn, struct tdx_module_args *args)
> panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
> }
>
> +static inline u64 tdg_vm_wr(u64 field, u64 value, u64 mask)
> +{
> + struct tdx_module_args args = {
> + .rdx = field,
> + .r8 = value,
> + .r9 = mask,
> + };
> +
> + tdcall(TDG_VM_WR, &args);
> +
> + /* Old value */
> + return args.r8;
> +}
> +
> /**
> * tdx_mcall_get_report0() - Wrapper to get TDREPORT0 (a.k.a. TDREPORT
> * subtype 0) using TDG.MR.REPORT TDCALL.
> @@ -902,10 +916,6 @@ static void tdx_kexec_unshare_mem(void)
>
> void __init tdx_early_init(void)
> {
> - struct tdx_module_args args = {
> - .rdx = TDCS_NOTIFY_ENABLES,
> - .r9 = -1ULL,
> - };
> u64 cc_mask;
> u32 eax, sig[3];
>
> @@ -924,7 +934,7 @@ void __init tdx_early_init(void)
> cc_set_mask(cc_mask);
>
> /* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
> - tdcall(TDG_VM_WR, &args);
> + tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
>
> /*
> * All bits above GPA width are reserved and kernel treats shared bit