diff mbox series

kvm/svm: PKU not currently supported

Message ID 20191219152332.28857-1-john.allen@amd.com (mailing list archive)
State New, archived
Headers show
Series kvm/svm: PKU not currently supported | expand

Commit Message

John Allen Dec. 19, 2019, 3:23 p.m. UTC
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>
---
 arch/x86/kvm/svm.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Vitaly Kuznetsov Dec. 19, 2019, 7:09 p.m. UTC | #1
John Allen <john.allen@amd.com> writes:

> 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>
> ---
>  arch/x86/kvm/svm.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 122d4ce3b1ab..f911aa1b41c8 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -5933,6 +5933,8 @@ static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
>  		if (avic)
>  			entry->ecx &= ~bit(X86_FEATURE_X2APIC);
>  		break;
> +	case 0x7:
> +		entry->ecx &= ~bit(X86_FEATURE_PKU);

Would it make more sense to introduce kvm_x86_ops->pku_supported() (and
return false for SVM and boot_cpu_has(X86_FEATURE_PKU) for vmx) so we
don't set the bit in the first place?

>  	case 0x80000001:
>  		if (nested)
>  			entry->ecx |= (1 << 2); /* Set SVM bit */
John Allen Dec. 19, 2019, 7:49 p.m. UTC | #2
On Thu, Dec 19, 2019 at 08:09:57PM +0100, Vitaly Kuznetsov wrote:
> John Allen <john.allen@amd.com> writes:
> 
> > 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>
> > ---
> >  arch/x86/kvm/svm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index 122d4ce3b1ab..f911aa1b41c8 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -5933,6 +5933,8 @@ static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
> >  		if (avic)
> >  			entry->ecx &= ~bit(X86_FEATURE_X2APIC);
> >  		break;
> > +	case 0x7:
> > +		entry->ecx &= ~bit(X86_FEATURE_PKU);
> 
> Would it make more sense to introduce kvm_x86_ops->pku_supported() (and
> return false for SVM and boot_cpu_has(X86_FEATURE_PKU) for vmx) so we
> don't set the bit in the first place?

Yes, I think you're right. I had initially planned to do it that way so I
already have a patch ready. I'll send it up pronto.

> 
> >  	case 0x80000001:
> >  		if (nested)
> >  			entry->ecx |= (1 << 2); /* Set SVM bit */
> 
> -- 
> Vitaly
>
diff mbox series

Patch

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 122d4ce3b1ab..f911aa1b41c8 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5933,6 +5933,8 @@  static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
 		if (avic)
 			entry->ecx &= ~bit(X86_FEATURE_X2APIC);
 		break;
+	case 0x7:
+		entry->ecx &= ~bit(X86_FEATURE_PKU);
 	case 0x80000001:
 		if (nested)
 			entry->ecx |= (1 << 2); /* Set SVM bit */