xen/arch/x86/include/asm/x86-defns.h | 1 + xen/arch/x86/pv/dom0_build.c | 18 ++++++++++-------- xen/include/public/arch-x86/cpufeatureset.h | 1 + 3 files changed, 12 insertions(+), 8 deletions(-)
The logic would be more robust disabling SMAP based on its precense in CR4,
rather than on certain features.
A forthcoming feature, LASS, needs the same treatment here. Introduce minimum
enumeration information, although it will take a bit more work to get LASS
fully usable in guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
I know LASS can't be used with traditional PV guests, but I have some PV-lite
plans. The problem is the PV kernel, in CPL3, accessing addresses in the high
canonincal half.
---
xen/arch/x86/include/asm/x86-defns.h | 1 +
xen/arch/x86/pv/dom0_build.c | 18 ++++++++++--------
xen/include/public/arch-x86/cpufeatureset.h | 1 +
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/include/asm/x86-defns.h b/xen/arch/x86/include/asm/x86-defns.h
index caa92829eaa9..8f97fb1e6a12 100644
--- a/xen/arch/x86/include/asm/x86-defns.h
+++ b/xen/arch/x86/include/asm/x86-defns.h
@@ -75,6 +75,7 @@
#define X86_CR4_PKE 0x00400000 /* enable PKE */
#define X86_CR4_CET 0x00800000 /* Control-flow Enforcement Technology */
#define X86_CR4_PKS 0x01000000 /* Protection Key Supervisor */
+#define X86_CR4_LASS 0x08000000 /* Linear Address Space Separation */
/*
* XSTATE component flags in XCR0 | MSR_XSS
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 262edb6bf2f0..f5c868df384f 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -1057,29 +1057,31 @@ int __init dom0_construct_pv(struct domain *d,
module_t *initrd,
const char *cmdline)
{
+ unsigned long cr4 = read_cr4();
+ unsigned long mask = X86_CR4_SMAP | X86_CR4_LASS;
int rc;
/*
- * Clear SMAP in CR4 to allow user-accesses in construct_dom0(). This
- * prevents us needing to write construct_dom0() in terms of
+ * Clear SMAP/LASS in CR4 to allow user-accesses in construct_dom0().
+ * This prevents us needing to write construct_dom0() in terms of
* copy_{to,from}_user().
*/
- if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
+ if ( cr4 & mask )
{
if ( IS_ENABLED(CONFIG_PV32) )
- cr4_pv32_mask &= ~X86_CR4_SMAP;
+ cr4_pv32_mask &= ~mask;
- write_cr4(read_cr4() & ~X86_CR4_SMAP);
+ write_cr4(cr4 & ~mask);
}
rc = dom0_construct(d, image, image_headroom, initrd, cmdline);
- if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
+ if ( cr4 & mask )
{
- write_cr4(read_cr4() | X86_CR4_SMAP);
+ write_cr4(cr4);
if ( IS_ENABLED(CONFIG_PV32) )
- cr4_pv32_mask |= X86_CR4_SMAP;
+ cr4_pv32_mask |= mask;
}
return rc;
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 8fa3fb711a8d..cbc0a3a8aa2b 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -303,6 +303,7 @@ XEN_CPUFEATURE(SM3, 10*32+ 1) /*A SM3 Instructions */
XEN_CPUFEATURE(SM4, 10*32+ 2) /*A SM4 Instructions */
XEN_CPUFEATURE(AVX_VNNI, 10*32+ 4) /*A AVX-VNNI Instructions */
XEN_CPUFEATURE(AVX512_BF16, 10*32+ 5) /*A AVX512 BFloat16 Instructions */
+XEN_CPUFEATURE(LASS, 10*32+ 6) /* Linear Address Space Separation */
XEN_CPUFEATURE(CMPCCXADD, 10*32+ 7) /*a CMPccXADD Instructions */
XEN_CPUFEATURE(FZRM, 10*32+10) /*A Fast Zero-length REP MOVSB */
XEN_CPUFEATURE(FSRS, 10*32+11) /*A Fast Short REP STOSB */
--
2.39.5
On 24.09.2024 13:23, Andrew Cooper wrote:
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -1057,29 +1057,31 @@ int __init dom0_construct_pv(struct domain *d,
> module_t *initrd,
> const char *cmdline)
> {
> + unsigned long cr4 = read_cr4();
> + unsigned long mask = X86_CR4_SMAP | X86_CR4_LASS;
> int rc;
>
> /*
> - * Clear SMAP in CR4 to allow user-accesses in construct_dom0(). This
> - * prevents us needing to write construct_dom0() in terms of
> + * Clear SMAP/LASS in CR4 to allow user-accesses in construct_dom0().
> + * This prevents us needing to write construct_dom0() in terms of
> * copy_{to,from}_user().
> */
> - if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
> + if ( cr4 & mask )
> {
> if ( IS_ENABLED(CONFIG_PV32) )
> - cr4_pv32_mask &= ~X86_CR4_SMAP;
> + cr4_pv32_mask &= ~mask;
>
> - write_cr4(read_cr4() & ~X86_CR4_SMAP);
> + write_cr4(cr4 & ~mask);
> }
>
> rc = dom0_construct(d, image, image_headroom, initrd, cmdline);
>
> - if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
> + if ( cr4 & mask )
> {
> - write_cr4(read_cr4() | X86_CR4_SMAP);
> + write_cr4(cr4);
>
> if ( IS_ENABLED(CONFIG_PV32) )
> - cr4_pv32_mask |= X86_CR4_SMAP;
> + cr4_pv32_mask |= mask;
You may end up setting a bit here which wasn't previously set, and which
might then fault when cr4_pv32_restore tries to OR this into %cr4. Aiui
you must have tested this on LASS-capable hardware, for it to have worked.
Jan
On Tue, Sep 24, 2024 at 03:44:07PM +0200, Jan Beulich wrote:
> On 24.09.2024 13:23, Andrew Cooper wrote:
> > --- a/xen/arch/x86/pv/dom0_build.c
> > +++ b/xen/arch/x86/pv/dom0_build.c
> > @@ -1057,29 +1057,31 @@ int __init dom0_construct_pv(struct domain *d,
> > module_t *initrd,
> > const char *cmdline)
> > {
> > + unsigned long cr4 = read_cr4();
> > + unsigned long mask = X86_CR4_SMAP | X86_CR4_LASS;
> > int rc;
> >
> > /*
> > - * Clear SMAP in CR4 to allow user-accesses in construct_dom0(). This
> > - * prevents us needing to write construct_dom0() in terms of
> > + * Clear SMAP/LASS in CR4 to allow user-accesses in construct_dom0().
> > + * This prevents us needing to write construct_dom0() in terms of
> > * copy_{to,from}_user().
> > */
> > - if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
> > + if ( cr4 & mask )
> > {
> > if ( IS_ENABLED(CONFIG_PV32) )
> > - cr4_pv32_mask &= ~X86_CR4_SMAP;
> > + cr4_pv32_mask &= ~mask;
> >
> > - write_cr4(read_cr4() & ~X86_CR4_SMAP);
> > + write_cr4(cr4 & ~mask);
> > }
> >
> > rc = dom0_construct(d, image, image_headroom, initrd, cmdline);
> >
> > - if ( boot_cpu_has(X86_FEATURE_XEN_SMAP) )
> > + if ( cr4 & mask )
> > {
> > - write_cr4(read_cr4() | X86_CR4_SMAP);
> > + write_cr4(cr4);
> >
> > if ( IS_ENABLED(CONFIG_PV32) )
> > - cr4_pv32_mask |= X86_CR4_SMAP;
> > + cr4_pv32_mask |= mask;
>
> You may end up setting a bit here which wasn't previously set, and which
> might then fault when cr4_pv32_restore tries to OR this into %cr4. Aiui
> you must have tested this on LASS-capable hardware, for it to have worked.
Possibly also needs X86_CR4_LASS adding to the XEN_CR4_PV32_BITS
define, as otherwise it won't end up in cr4_pv32_mask in the first
place AFAICT.
Thanks, Roger.
On Tue, Sep 24, 2024 at 12:23:43PM +0100, Andrew Cooper wrote:
> The logic would be more robust disabling SMAP based on its precense in CR4,
> rather than on certain features.
>
> A forthcoming feature, LASS, needs the same treatment here. Introduce minimum
> enumeration information, although it will take a bit more work to get LASS
> fully usable in guests.
Reading the ISA, doesn't LASS require SMAP to be enabled in %cr4, and
hence disabling SMAP already disables LASS? (without having to toggle
the LASS %cr4 bit)
"A supervisor-mode data access causes a LASS violation only if
supervisor-mode access protection is enabled (because CR4.SMAP = 1)
and either RFLAGS.AC = 0 or the access implicitly accesses a system
data structure."
We can consider also disabling it, but I think it would need to be
noted that such disabling is not strictly necessary, as disabling SMAP
already disables LASS.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
>
> I know LASS can't be used with traditional PV guests, but I have some PV-lite
> plans. The problem is the PV kernel, in CPL3, accessing addresses in the high
> canonincal half.
> ---
> xen/arch/x86/include/asm/x86-defns.h | 1 +
> xen/arch/x86/pv/dom0_build.c | 18 ++++++++++--------
> xen/include/public/arch-x86/cpufeatureset.h | 1 +
> 3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/x86-defns.h b/xen/arch/x86/include/asm/x86-defns.h
> index caa92829eaa9..8f97fb1e6a12 100644
> --- a/xen/arch/x86/include/asm/x86-defns.h
> +++ b/xen/arch/x86/include/asm/x86-defns.h
> @@ -75,6 +75,7 @@
> #define X86_CR4_PKE 0x00400000 /* enable PKE */
> #define X86_CR4_CET 0x00800000 /* Control-flow Enforcement Technology */
> #define X86_CR4_PKS 0x01000000 /* Protection Key Supervisor */
> +#define X86_CR4_LASS 0x08000000 /* Linear Address Space Separation */
>
> /*
> * XSTATE component flags in XCR0 | MSR_XSS
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index 262edb6bf2f0..f5c868df384f 100644
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -1057,29 +1057,31 @@ int __init dom0_construct_pv(struct domain *d,
> module_t *initrd,
> const char *cmdline)
> {
> + unsigned long cr4 = read_cr4();
> + unsigned long mask = X86_CR4_SMAP | X86_CR4_LASS;
const maybe? Seeing as it is read-only.
Thanks, Roger.
On 24/09/2024 1:30 pm, Roger Pau Monné wrote: > On Tue, Sep 24, 2024 at 12:23:43PM +0100, Andrew Cooper wrote: >> The logic would be more robust disabling SMAP based on its precense in CR4, >> rather than on certain features. >> >> A forthcoming feature, LASS, needs the same treatment here. Introduce minimum >> enumeration information, although it will take a bit more work to get LASS >> fully usable in guests. > Reading the ISA, doesn't LASS require SMAP to be enabled in %cr4, and > hence disabling SMAP already disables LASS? (without having to toggle > the LASS %cr4 bit) > > "A supervisor-mode data access causes a LASS violation only if > supervisor-mode access protection is enabled (because CR4.SMAP = 1) > and either RFLAGS.AC = 0 or the access implicitly accesses a system > data structure." > > We can consider also disabling it, but I think it would need to be > noted that such disabling is not strictly necessary, as disabling SMAP > already disables LASS. Hmm. LASS looks to have no CR4 dependencies on SMAP or SMEP, and the ISE does suggest they can be used independently. However, I see no connection to paging (beyond LMA), and that is going to become a problem in due course. Anyway - I'll drop the LASS aspect for now. It can be left to whomever gets some working real hardware first. ~Andrew
© 2016 - 2025 Red Hat, Inc.