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 >