Re: [PATCH v2] kvm/svm: PKU not currently supported

[John Allen <john.allen@amd.com>]

On Fri, Dec 20, 2019 at 10:25:16AM +0100, Paolo Bonzini wrote:
> On 19/12/19 21:32, Sean Christopherson wrote:
> > On Thu, Dec 19, 2019 at 02:17:59PM -0600, John Allen wrote:
> >> Current SVM implementation does not have support for handling PKU. Guests
> >> running on a host with future AMD cpus that support the feature will read
> >> garbage from the PKRU register and will hit segmentation faults on boot as
> >> memory is getting marked as protected that should not be. Ensure that cpuid
> >> from SVM does not advertise the feature.
> >>
> >> Signed-off-by: John Allen <john.allen@amd.com>
> >> ---
> >> v2:
> >>   -Introduce kvm_x86_ops->pku_supported()
> > 
> > I like the v1 approach better, it's less code to unwind when SVM gains
> > support for virtualizaing PKU.
> > 
> > The existing cases of kvm_x86_ops->*_supported() in __do_cpuid_func() are
> > necessary to handle cases where it may not be possible to expose a feature
> > even though it's supported in hardware, host and KVM, e.g. VMX's separate
> > MSR-based features and PT's software control to hide it from guest.  In
> > this case, hiding PKU is purely due to lack of support in KVM.  The SVM
> > series to enable PKU can then delete a single line of SVM code instead of
> > having to go back in and do surgery on x86 and VMX.
> > 
> 
> I sort of liked the V1 approach better, in that I liked using
> set_supported_cpuid but I didn't like *removing* features from it.
> 
> I think all *_supported() should be removed, and the code moved from
> __do_cpuid_func() to set_supported_cpuid.
> 
> For now, however, this one is consistent with other features so I am
> applying it.

Hey Paolo,

If you haven't already applied this, would it be too much trouble to add a
fixes tag? If it's already applied, don't worry about it.

...
Fixes: 0556cbdc2fbc ("x86/pkeys: Don't check if PKRU is zero before writing it")

Thanks,
John

> 
> Paolo
>