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

From: John Allen
Date: Mon Dec 23 2019 - 10:21:14 EST


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@xxxxxxx>
> >> ---
> >> 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
>