Re: [PATCH 1/3] x86/pv: Options to disable and/or compile out 32bit PV support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/04/2020 14:47, Roger Pau Monné wrote:
> On Fri, Apr 17, 2020 at 04:50:02PM +0100, Andrew Cooper wrote:
>> This is the start of some performance and security-hardening improvements,
>> based on the fact that 32bit PV guests are few and far between these days.
>>
>> Ring1 is full or architectural corner cases, such as counting as supervisor
>                 ^ of

Already fixed (I spotted it 30s after posting).

>> from a paging point of view.  This accounts for a substantial performance hit
>> on processors from the last 8 years (adjusting SMEP/SMAP on every privilege
>> transition), and the gap is only going to get bigger with new hardware
>> features.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>
>> There is a series I can't quite post yet which wants to conditionally turn
>> opt_pv32 off, which is why I've put it straight in in an int8_t form rather
> s/in in/in/

"in in" is legitimate in some cases, despite it looking awkard.   In
this case, the structure is "straight in", separate from "in an int8_t
form".

If this sentence were for inclusion in the commit message, I'd have
probably spent more effort trying to phrase it differently.

>
>> than a straight boolean form.
>> ---
>>  docs/misc/xen-command-line.pandoc | 12 +++++++++++-
>>  xen/arch/x86/Kconfig              | 16 ++++++++++++++++
>>  xen/arch/x86/pv/domain.c          | 35 +++++++++++++++++++++++++++++++++++
>>  xen/arch/x86/setup.c              |  9 +++++++--
>>  xen/include/asm-x86/pv/domain.h   |  6 ++++++
>>  5 files changed, 75 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/misc/xen-command-line.pandoc 
>> b/docs/misc/xen-command-line.pandoc
>> index acd0b3d994..ee12b0f53f 100644
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -1694,7 +1694,17 @@ The following resources are available:
>>      CDP, one COS will corespond two CBMs other than one with CAT, due to the
>>      sum of CBMs is fixed, that means actual `cos_max` in use will 
>> automatically
>>      reduce to half when CDP is enabled.
>> -    
>> +
>> +### pv
>> +    = List of [ 32=<bool> ]
>> +
>> +    Applicability: x86
>> +
>> +Controls for aspects of PV guest support.
>> +
>> +*   The `32` boolean controls whether 32bit PV guests can be created.  It
>> +    defaults to `true`, and is ignored when `CONFIG_PV32` is compiled out.
>> +
>>  ### pv-linear-pt (x86)
>>  > `= <boolean>`
>>  
>> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
>> index 8149362bde..4c52197de3 100644
>> --- a/xen/arch/x86/Kconfig
>> +++ b/xen/arch/x86/Kconfig
>> @@ -49,6 +49,22 @@ config PV
>>  
>>        If unsure, say Y.
>>  
>> +config PV32
>> +    bool "Support for 32bit PV guests"
>> +    depends on PV
>> +    default y
>> +    ---help---
>> +      The 32bit PV ABI uses Ring1, an area of the x86 architecture which
>> +      was deprecated and mostly removed in the AMD64 spec.  As a result,
>> +      it occasionally conflicts with newer x86 hardware features, causing
>> +      overheads for Xen to maintain backwards compatibility.
>> +
>> +      People may wish to disable 32bit PV guests for attack surface
>> +      reduction, or performance reasons.  Backwards compatibility can be
>> +      provided via the PV Shim mechanism.
>> +
>> +      If unsure, say Y.
>> +
>>  config PV_LINEAR_PT
>>         bool "Support for PV linear pagetables"
>>         depends on PV
>> diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
>> index 70fae43965..47a0db082f 100644
>> --- a/xen/arch/x86/pv/domain.c
>> +++ b/xen/arch/x86/pv/domain.c
>> @@ -16,6 +16,39 @@
>>  #include <asm/pv/domain.h>
>>  #include <asm/shadow.h>
>>  
>> +#ifdef CONFIG_PV32
>> +int8_t __read_mostly opt_pv32 = -1;
>> +#endif
>> +
>> +static int parse_pv(const char *s)
> __init
>
> With that:
>
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks,

~Andrew