Re: [PATCH 4/4] X86: Use KVM CR pin MSRs

[Dave Hansen <dave.hansen@intel.com>]

On 6/18/20 8:26 AM, Andersen, John wrote:
> On Thu, Jun 18, 2020 at 07:41:04AM -0700, Dave Hansen wrote:
>> Let's say kexec is config'd off.  This feature is enabled by default and
>> crashes the kernel in early boot.  I have no way to disable this fancy
>> new feature.  Is that what we want?
>>
>> I also think that instead of having to *enable* this explicitly when
>> kexec is present, maybe we should have a "disable_kexec" parameter.  If
>> kexec is configured out or disabled on the command-line, then you can
>> turn CR pinning on.
>>
>> If someone fails to kexec() because of this feature, there's no way in
>> hell they'll ever track down "pv_cr_pin" on the command-line as the
>> cause.  The might have a chance of finding disable_kexec, though.
>>
>> Wouldn't it also be nice to add a single printk() the first time a kexec
>> fails because of this feature being present?
> 
> That sounds like a good plan. I'll change pv_cr_pin to disable_kexec, and add a
> disable_pv_cr_pin option in case it's being on by default via the compile time
> option breaks a users workflow at runtime.
> 
> In this case, I'm assuming we can do away with the kconfig option then.
> 
> Just have it enabled by default. If kexec is present, it's disabled by default,
> unless kexec is disabled, in which case, pinning is enabled unless
> disable_pv_cr_pin is set.

Yes, that sounds good to me.

...
>>> +config PARAVIRT_CR_PIN
>>> +       bool "Paravirtual bit pinning for CR0 and CR4"
>>> +       depends on KVM_GUEST
>>> +       help
>>> +         Select this option to have the virtualised guest request that the
>>> +         hypervisor disallow it from disabling protections set in control
>>> +         registers. The hypervisor will prevent exploits from disabling
>>> +         features such as SMEP, SMAP, UMIP, and WP.
>>
>> I'm confused.  Does this add support for ""Paravirtual bit pinning", or
>> actually tell the guest to request pinning by default?
>>
>> It says "Select this option to have the virtualised guest request...",
>> which makes it sound like it affects the default rather than the
>> availability of the option.
> 
> How about this
> 
> Select this option to request protection of SMEP, SMAP, UMIP, and WP
> control register bits when running paravirtualized under KVM. Protection will
> be active provided the feature is available host side and kexec is disabled via
> kconfig or the command line for the guest requesting protection.

It still isn't very clear to me.

Let's pull the config option out of this patch.  Enable the feature by
default and do the command-line processing in this patch.

If you still think a Kconfig option is helpful, add it in a separate
patch calling out the deficiencies with the boot-time options.