Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
running 32-bit PV guest inside PV shim comes with significant performance
hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
tries to perform global TLB invalidation for PV shim domain. This usually
results in eventual hang of a PV shim with at least several vCPUs.

Since the overall security risk is generally lower for shim Xen as it being
there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
it by default on AMD and Hygon unless a user chose otherwise.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
I'm a little bit on the fence with this one. We're having the same issue with
general nested virt but I'm not inclined to trade security for a user in
general case. Disabling it by default for PV shim only seems rather inocuous
due to the use case restricion. I'd like to hear more opinions.
---
 docs/misc/xen-command-line.pandoc | 10 ++++++++--
 xen/arch/x86/setup.c              | 20 ++++++++++++++------
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 981a5e2..05b2dde 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1936,19 +1936,25 @@ is 1MB.
 ### smap (x86)
 > `= <boolean> | hvm`
 
-> Default: `true`
+> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
 
 Flag to enable Supervisor Mode Access Prevention
 Use `smap=hvm` to allow SMAP use by HVM guests only.
 
+In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
+in some cases and generally lower security risk the option defaults to false.
+
 ### smep (x86)
 > `= <boolean> | hvm`
 
-> Default: `true`
+> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
 
 Flag to enable Supervisor Mode Execution Protection
 Use `smep=hvm` to allow SMEP use by HVM guests only.
 
+In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
+in some cases and generally lower security risk the option defaults to false.
+
 ### smt (x86)
 > `= <boolean>`
 
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5bdc229..8432b77 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -105,9 +105,9 @@ struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
 
 unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
 
-/* smep: Enable/disable Supervisor Mode Execution Protection (default on). */
-#define SMEP_HVM_ONLY (-1)
-static s8 __initdata opt_smep = 1;
+/* smep: Enable/disable Supervisor Mode Execution Protection */
+#define SMEP_HVM_ONLY (-2)
+static s8 __initdata opt_smep = -1;
 
 /*
  * Initial domain place holder. Needs to be global so it can be created in
@@ -142,9 +142,9 @@ static int __init parse_smep_param(const char *s)
 }
 custom_param("smep", parse_smep_param);
 
-/* smap: Enable/disable Supervisor Mode Access Prevention (default on). */
-#define SMAP_HVM_ONLY (-1)
-static s8 __initdata opt_smap = 1;
+/* smap: Enable/disable Supervisor Mode Access Prevention */
+#define SMAP_HVM_ONLY (-2)
+static s8 __initdata opt_smap = -1;
 
 static int __init parse_smap_param(const char *s)
 {
@@ -1616,6 +1616,14 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     set_in_cr4(X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT);
 
+    /* Do not enable SMEP/SMAP in PV shim on AMD and Hygon by default */
+    if ( opt_smep == -1 )
+        opt_smep = !pv_shim || !(boot_cpu_data.x86_vendor &
+                                (X86_VENDOR_AMD | X86_VENDOR_HYGON));
+    if ( opt_smap == -1 )
+        opt_smap = !pv_shim || !(boot_cpu_data.x86_vendor &
+                                (X86_VENDOR_AMD | X86_VENDOR_HYGON));
+
     if ( !opt_smep )
         setup_clear_cpu_cap(X86_FEATURE_SMEP);
     if ( cpu_has_smep && opt_smep != SMEP_HVM_ONLY )
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 16.01.2020 17:00, Igor Druzhinin wrote:
> Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
> running 32-bit PV guest inside PV shim comes with significant performance
> hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
> switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
> tries to perform global TLB invalidation for PV shim domain. This usually
> results in eventual hang of a PV shim with at least several vCPUs.
> 
> Since the overall security risk is generally lower for shim Xen as it being
> there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
> it by default on AMD and Hygon unless a user chose otherwise.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with two minor adjustments (two instances each):

> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1936,19 +1936,25 @@ is 1MB.
>  ### smap (x86)
>  > `= <boolean> | hvm`
>  
> -> Default: `true`
> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
>  
>  Flag to enable Supervisor Mode Access Prevention
>  Use `smap=hvm` to allow SMAP use by HVM guests only.
>  
> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact

Missing 'r' in performance.

> @@ -1616,6 +1616,14 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      set_in_cr4(X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT);
>  
> +    /* Do not enable SMEP/SMAP in PV shim on AMD and Hygon by default */
> +    if ( opt_smep == -1 )
> +        opt_smep = !pv_shim || !(boot_cpu_data.x86_vendor &
> +                                (X86_VENDOR_AMD | X86_VENDOR_HYGON));

One space missing on this line to cover for the still open parenthesis
from the previous one.

Both can be easily taken care of while committing.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On Thu, Jan 16, 2020 at 04:00:03PM +0000, Igor Druzhinin wrote:
> Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
> running 32-bit PV guest inside PV shim comes with significant performance
> hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
> switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
> tries to perform global TLB invalidation for PV shim domain. This usually
> results in eventual hang of a PV shim with at least several vCPUs.
> 
> Since the overall security risk is generally lower for shim Xen as it being
> there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
> it by default on AMD and Hygon unless a user chose otherwise.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> ---
> I'm a little bit on the fence with this one. We're having the same issue with
> general nested virt but I'm not inclined to trade security for a user in
> general case. Disabling it by default for PV shim only seems rather inocuous
> due to the use case restricion. I'd like to hear more opinions.
> ---
>  docs/misc/xen-command-line.pandoc | 10 ++++++++--
>  xen/arch/x86/setup.c              | 20 ++++++++++++++------
>  2 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index 981a5e2..05b2dde 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1936,19 +1936,25 @@ is 1MB.
>  ### smap (x86)
>  > `= <boolean> | hvm`
>  
> -> Default: `true`
> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
>  
>  Flag to enable Supervisor Mode Access Prevention
>  Use `smap=hvm` to allow SMAP use by HVM guests only.
>  
> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
> +in some cases and generally lower security risk the option defaults to false.
> +
>  ### smep (x86)
>  > `= <boolean> | hvm`
>  
> -> Default: `true`
> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
>  
>  Flag to enable Supervisor Mode Execution Protection
>  Use `smep=hvm` to allow SMEP use by HVM guests only.
>  
> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
> +in some cases and generally lower security risk the option defaults to false.
> +
>  ### smt (x86)
>  > `= <boolean>`
>  
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 5bdc229..8432b77 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -105,9 +105,9 @@ struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
>  
>  unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
>  
> -/* smep: Enable/disable Supervisor Mode Execution Protection (default on). */
> -#define SMEP_HVM_ONLY (-1)
> -static s8 __initdata opt_smep = 1;
> +/* smep: Enable/disable Supervisor Mode Execution Protection */
> +#define SMEP_HVM_ONLY (-2)
> +static s8 __initdata opt_smep = -1;

Could you change the type to int8_t instead of s8? (here and below,
can be done on commit with the changes requested by Jan).

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 20.01.2020 15:07, Roger Pau Monné  wrote:
> On Thu, Jan 16, 2020 at 04:00:03PM +0000, Igor Druzhinin wrote:
>> Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
>> running 32-bit PV guest inside PV shim comes with significant performance
>> hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
>> switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
>> tries to perform global TLB invalidation for PV shim domain. This usually
>> results in eventual hang of a PV shim with at least several vCPUs.
>>
>> Since the overall security risk is generally lower for shim Xen as it being
>> there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
>> it by default on AMD and Hygon unless a user chose otherwise.
>>
>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
>> ---
>> I'm a little bit on the fence with this one. We're having the same issue with
>> general nested virt but I'm not inclined to trade security for a user in
>> general case. Disabling it by default for PV shim only seems rather inocuous
>> due to the use case restricion. I'd like to hear more opinions.
>> ---
>>  docs/misc/xen-command-line.pandoc | 10 ++++++++--
>>  xen/arch/x86/setup.c              | 20 ++++++++++++++------
>>  2 files changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
>> index 981a5e2..05b2dde 100644
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -1936,19 +1936,25 @@ is 1MB.
>>  ### smap (x86)
>>  > `= <boolean> | hvm`
>>  
>> -> Default: `true`
>> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
>>  
>>  Flag to enable Supervisor Mode Access Prevention
>>  Use `smap=hvm` to allow SMAP use by HVM guests only.
>>  
>> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
>> +in some cases and generally lower security risk the option defaults to false.
>> +
>>  ### smep (x86)
>>  > `= <boolean> | hvm`
>>  
>> -> Default: `true`
>> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
>>  
>>  Flag to enable Supervisor Mode Execution Protection
>>  Use `smep=hvm` to allow SMEP use by HVM guests only.
>>  
>> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
>> +in some cases and generally lower security risk the option defaults to false.
>> +
>>  ### smt (x86)
>>  > `= <boolean>`
>>  
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 5bdc229..8432b77 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -105,9 +105,9 @@ struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
>>  
>>  unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
>>  
>> -/* smep: Enable/disable Supervisor Mode Execution Protection (default on). */
>> -#define SMEP_HVM_ONLY (-1)
>> -static s8 __initdata opt_smep = 1;
>> +/* smep: Enable/disable Supervisor Mode Execution Protection */
>> +#define SMEP_HVM_ONLY (-2)
>> +static s8 __initdata opt_smep = -1;
> 
> Could you change the type to int8_t instead of s8? (here and below,
> can be done on commit with the changes requested by Jan).

Too late, sorry, this was committed before the weekend already.
(I guess I should have noticed this myself, though.)

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On Mon, Jan 20, 2020 at 03:38:02PM +0100, Jan Beulich wrote:
> On 20.01.2020 15:07, Roger Pau Monné  wrote:
> > On Thu, Jan 16, 2020 at 04:00:03PM +0000, Igor Druzhinin wrote:
> >> Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
> >> running 32-bit PV guest inside PV shim comes with significant performance
> >> hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
> >> switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
> >> tries to perform global TLB invalidation for PV shim domain. This usually
> >> results in eventual hang of a PV shim with at least several vCPUs.
> >>
> >> Since the overall security risk is generally lower for shim Xen as it being
> >> there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
> >> it by default on AMD and Hygon unless a user chose otherwise.
> >>
> >> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> >> ---
> >> I'm a little bit on the fence with this one. We're having the same issue with
> >> general nested virt but I'm not inclined to trade security for a user in
> >> general case. Disabling it by default for PV shim only seems rather inocuous
> >> due to the use case restricion. I'd like to hear more opinions.
> >> ---
> >>  docs/misc/xen-command-line.pandoc | 10 ++++++++--
> >>  xen/arch/x86/setup.c              | 20 ++++++++++++++------
> >>  2 files changed, 22 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> >> index 981a5e2..05b2dde 100644
> >> --- a/docs/misc/xen-command-line.pandoc
> >> +++ b/docs/misc/xen-command-line.pandoc
> >> @@ -1936,19 +1936,25 @@ is 1MB.
> >>  ### smap (x86)
> >>  > `= <boolean> | hvm`
> >>  
> >> -> Default: `true`
> >> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
> >>  
> >>  Flag to enable Supervisor Mode Access Prevention
> >>  Use `smap=hvm` to allow SMAP use by HVM guests only.
> >>  
> >> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
> >> +in some cases and generally lower security risk the option defaults to false.
> >> +
> >>  ### smep (x86)
> >>  > `= <boolean> | hvm`
> >>  
> >> -> Default: `true`
> >> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
> >>  
> >>  Flag to enable Supervisor Mode Execution Protection
> >>  Use `smep=hvm` to allow SMEP use by HVM guests only.
> >>  
> >> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact
> >> +in some cases and generally lower security risk the option defaults to false.
> >> +
> >>  ### smt (x86)
> >>  > `= <boolean>`
> >>  
> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> >> index 5bdc229..8432b77 100644
> >> --- a/xen/arch/x86/setup.c
> >> +++ b/xen/arch/x86/setup.c
> >> @@ -105,9 +105,9 @@ struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
> >>  
> >>  unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
> >>  
> >> -/* smep: Enable/disable Supervisor Mode Execution Protection (default on). */
> >> -#define SMEP_HVM_ONLY (-1)
> >> -static s8 __initdata opt_smep = 1;
> >> +/* smep: Enable/disable Supervisor Mode Execution Protection */
> >> +#define SMEP_HVM_ONLY (-2)
> >> +static s8 __initdata opt_smep = -1;
> > 
> > Could you change the type to int8_t instead of s8? (here and below,
> > can be done on commit with the changes requested by Jan).
> 
> Too late, sorry, this was committed before the weekend already.
> (I guess I should have noticed this myself, though.)

Oh, sorry, didn't realize this was already committed, never mind then.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 16/01/2020 16:00, Igor Druzhinin wrote:
> Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
> running 32-bit PV guest inside PV shim comes with significant performance
> hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
> switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
> tries to perform global TLB invalidation for PV shim domain. This usually
> results in eventual hang of a PV shim with at least several vCPUs.
>
> Since the overall security risk is generally lower for shim Xen as it being
> there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
> it by default on AMD and Hygon unless a user chose otherwise.
>
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> ---
> I'm a little bit on the fence with this one. We're having the same issue with
> general nested virt but I'm not inclined to trade security for a user in
> general case. Disabling it by default for PV shim only seems rather inocuous
> due to the use case restricion. I'd like to hear more opinions.

So everyone on the list is up to date with the discussion we had IRL.

SMEP/SMAP are defence in depth measures.  We support running on hardware
without these features, and the overall result is the same, security wise.

In the PV Shim case, there is only a single guest and nothing
interesting in Xen, data wise.  We specifically do not have the risk of
sideways data leakage from other guests to be worried about.

We do however care for performance, and not taking a VMExit on every
SYSCALL/Interrupt/Exception makes a massive difference overall.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

> 
> On 16/01/2020 16:00, Igor Druzhinin wrote:
> > Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
> > running 32-bit PV guest inside PV shim comes with significant performance
> > hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
> > switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
> > tries to perform global TLB invalidation for PV shim domain. This usually
> > results in eventual hang of a PV shim with at least several vCPUs.
> >
> > Since the overall security risk is generally lower for shim Xen as it being
> > there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
> > it by default on AMD and Hygon unless a user chose otherwise.
> >
> > Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> > ---
> > I'm a little bit on the fence with this one. We're having the same issue with
> > general nested virt but I'm not inclined to trade security for a user in
> > general case. Disabling it by default for PV shim only seems rather inocuous
> > due to the use case restricion. I'd like to hear more opinions.
> 
> So everyone on the list is up to date with the discussion we had IRL.
> 
> SMEP/SMAP are defence in depth measures.  We support running on hardware
> without these features, and the overall result is the same, security wise.
> 
> In the PV Shim case, there is only a single guest and nothing
> interesting in Xen, data wise.  We specifically do not have the risk of
> sideways data leakage from other guests to be worried about.
> 
> We do however care for performance, and not taking a VMExit on every
> SYSCALL/Interrupt/Exception makes a massive difference overall.

FWIW when running the shim under KVM even on Intel, constantly frobbing
the CR4.SMEP bit still performs awfully.

(Yes, we should make KVM pass that bit through to its guest on Intel
hardware, just as Xen does when it's the HVM host. cf.
https://lore.kernel.org/kvm/7fba6d8fc3de0bcb86bf629a4f5b0217552fe999.camel@infradead.org/T/#m39a117e90f29fc862b78ec1441b761459e7be86a
)

But why does the shim even need to turn it off when switching to the
guest context? Its guest isn't running in supervisor mode so surely it
doesn't *matter* whether SMEP is enabled or not? Why not just leave it
on at all times?

On 11/10/2023 7:34 pm, David Woodhouse wrote:
>> On 16/01/2020 16:00, Igor Druzhinin wrote:
>>> Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
>>> running 32-bit PV guest inside PV shim comes with significant performance
>>> hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
>>> switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
>>> tries to perform global TLB invalidation for PV shim domain. This usually
>>> results in eventual hang of a PV shim with at least several vCPUs.
>>>
>>> Since the overall security risk is generally lower for shim Xen as it being
>>> there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
>>> it by default on AMD and Hygon unless a user chose otherwise.
>>>
>>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
>>> ---
>>> I'm a little bit on the fence with this one. We're having the same issue with
>>> general nested virt but I'm not inclined to trade security for a user in
>>> general case. Disabling it by default for PV shim only seems rather inocuous
>>> due to the use case restricion. I'd like to hear more opinions.
>> So everyone on the list is up to date with the discussion we had IRL.
>>
>> SMEP/SMAP are defence in depth measures.  We support running on hardware
>> without these features, and the overall result is the same, security wise.
>>
>> In the PV Shim case, there is only a single guest and nothing
>> interesting in Xen, data wise.  We specifically do not have the risk of
>> sideways data leakage from other guests to be worried about.
>>
>> We do however care for performance, and not taking a VMExit on every
>> SYSCALL/Interrupt/Exception makes a massive difference overall.
> FWIW when running the shim under KVM even on Intel, constantly frobbing
> the CR4.SMEP bit still performs awfully.
>
> (Yes, we should make KVM pass that bit through to its guest on Intel
> hardware, just as Xen does when it's the HVM host. cf.
> https://lore.kernel.org/kvm/7fba6d8fc3de0bcb86bf629a4f5b0217552fe999.camel@infradead.org/T/#m39a117e90f29fc862b78ec1441b761459e7be86a
> )
>
> But why does the shim even need to turn it off when switching to the
> guest context? Its guest isn't running in supervisor mode so surely it
> doesn't *matter* whether SMEP is enabled or not? Why not just leave it
> on at all times?

32bit PV kernels run in Ring1.  Which is supervisor and not user.

Some older PV kernels do execute on user pages, and don't like getting
SMEP faults when they didn't turn it on to begin with.

SMAP is far more horrible.  STAC/CLAC are CPL0 instructions despite POPF
(changing AC) having the same architectural effect, but again, the guest
kernel doesn't tolerate SMAP faults when it didn't know it was enabled.

~Andrew

On Thu, 2023-10-12 at 10:13 +0800, andrew.cooper3@citrix.com wrote:
> On 11/10/2023 7:34 pm, David Woodhouse wrote:
> > But why does the shim even need to turn it off when switching to the
> > guest context? Its guest isn't running in supervisor mode so surely it
> > doesn't *matter* whether SMEP is enabled or not? Why not just leave it
> > on at all times?
> 
> 32bit PV kernels run in Ring1.  Which is supervisor and not user.

Ah, thanks.

> Some older PV kernels do execute on user pages, and don't like getting
> SMEP faults when they didn't turn it on to begin with.

PV guests never actually had the option to turn SMEP on, did they? 

(Otherwise I may have to rethink the approach of just putting
'smep=off' onto the shim command line when running under KVM...)

On 12/10/2023 4:21 pm, David Woodhouse wrote:
> On Thu, 2023-10-12 at 10:13 +0800, andrew.cooper3@citrix.com wrote:
>> On 11/10/2023 7:34 pm, David Woodhouse wrote:
>>> But why does the shim even need to turn it off when switching to the
>>> guest context? Its guest isn't running in supervisor mode so surely it
>>> doesn't *matter* whether SMEP is enabled or not? Why not just leave it
>>> on at all times?
>>
>> 32bit PV kernels run in Ring1.  Which is supervisor and not user.
> 
> Ah, thanks.
> 
>> Some older PV kernels do execute on user pages, and don't like getting
>> SMEP faults when they didn't turn it on to begin with.
> 
> PV guests never actually had the option to turn SMEP on, did they? 
> 
> (Otherwise I may have to rethink the approach of just putting
> 'smep=off' onto the shim command line when running under KVM...)


Xen and PV guests share a set of pagetables.  There is no ability to
independently control SMEP/SMAP.

While we could in principle make SMEP an feature that PV kernels can opt
into, SMAP we really can't.  The emulation costs of STAC/CLAC are
obscene from a perf perspective.

One TODO which has yet to be done is to look at the PV kernel's
ELF32/64-ness.  For a shimmed 64bit guest, SMEP/SMAP should be on and
stay on.

TBH, it's probably best to just hide the SMEP/SMAP features, rather than
to play around with the cmdline.

~Andrew