[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fs4n5t3ylzhboxcdrnuhlm6rdsprt7xaeeoae3cbyapw6y4cha@kqm5cwjavs3n>
Date: Tue, 27 Aug 2024 13:04:19 +0300
From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To: "Huang, Kai" <kai.huang@...el.com>
Cc: Dave Hansen <dave.hansen@...ux.intel.com>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, linux-coco@...ts.linux.dev,
linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCHv5, REBASED 3/4] x86/tdx: Dynamically disable SEPT
violations from causing #VEs
On Wed, Aug 21, 2024 at 01:52:49PM +1200, Huang, Kai wrote:
> > + * attribute is no longer reliable. It reflects the initial state of the
> > + * control for the TD, but it will not be updated if someone (e.g. bootloader)
> > + * changes it before the kernel starts. Kernel must check TDCS_TD_CTLS bit to
> > + * determine if SEPT #VEs are enabled or disabled.
> > + */
> > +static void disable_sept_ve(u64 td_attr)
> > +{
> > + const char *msg = "TD misconfiguration: SEPT #VE has to be disabled";
>
> The original msg was:
>
> "TD misconfiguration: SEPT_VE_DISABLE attribute must be set."
>
> Any reason to change?
Because the attribute is not the only way to control if #VE is going to be
injected.
>
>
> > + bool debug = td_attr & ATTR_DEBUG;
> > + u64 config, controls;
> > +
> > + /* Is this TD allowed to disable SEPT #VE */
> > + tdg_vm_rd(TDCS_CONFIG_FLAGS, &config);
> > + if (!(config & TDCS_CONFIG_FLEXIBLE_PENDING_VE)) {
>
> Does this field ID exist in TDX1.0? I.e., whether it can fail here and
> should we check the return value first?
See TDG.VM.RD definition:
R8 Contents of the field
In case of no success, as indicated by RAX, R8 returns 0.
No need in error checking here.
> > diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> > index 7e12cfa28bec..fecb2a6e864b 100644
> > --- a/arch/x86/include/asm/shared/tdx.h
> > +++ b/arch/x86/include/asm/shared/tdx.h
> > @@ -19,9 +19,17 @@
> > #define TDG_VM_RD 7
> > #define TDG_VM_WR 8
> > -/* TDCS fields. To be used by TDG.VM.WR and TDG.VM.RD module calls */
> > +/* TDX TD-Scope Metadata. To be used by TDG.VM.WR and TDG.VM.RD */
>
> I am not sure whether this change is necessary.
It is more in-line with spec json dump.
> > +#define TDCS_CONFIG_FLAGS 0x1110000300000016
> > +#define TDCS_TD_CTLS 0x1110000300000017
>
> The TDX 1.5 spec 'td_scope_metadata.json' says they are 0x9110000300000016
> and 0x9110000300000017.
The spec is broken. It is going to be fixed. I use correct values.
> I know the bit 63 is ignored by the TDX module, but since (IIUC) those two
> fields are introduced in TDX1.5, it's just better to follow what TDX1.5 spec
> says.
Newer modules will ignore this bit and both values are going to
acceptable.
--
Kiryl Shutsemau / Kirill A. Shutemov
Powered by blists - more mailing lists