| Message ID | 157262962352.2838.15656190309312238595.stgit@naples-babu.amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Emulate and enable UMIP feature on AMD | expand |
On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: > > AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction > Prevention) feature. The UMIP feature prevents the execution of certain > instructions if the Current Privilege Level (CPL) is greater than 0. > If any of these instructions are executed with CPL > 0 and UMIP > is enabled, then kernel reports a #GP exception. > > The idea is taken from articles: > https://lwn.net/Articles/738209/ > https://lwn.net/Articles/694385/ > > Enable the feature if supported on bare metal and emulate instructions > to return dummy values for certain cases. What are these cases?
On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: > > AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction > Prevention) feature. The UMIP feature prevents the execution of certain > instructions if the Current Privilege Level (CPL) is greater than 0. > If any of these instructions are executed with CPL > 0 and UMIP > is enabled, then kernel reports a #GP exception. > > The idea is taken from articles: > https://lwn.net/Articles/738209/ > https://lwn.net/Articles/694385/ > > Enable the feature if supported on bare metal and emulate instructions > to return dummy values for certain cases. > > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > arch/x86/kvm/svm.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 4153ca8cddb7..79abbdeca148 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) > { > } > > +static bool svm_umip_emulated(void) > +{ > + return boot_cpu_has(X86_FEATURE_UMIP); > +} This makes no sense to me. If the hardware actually supports UMIP, then it doesn't have to be emulated. To the extent that kvm emulates UMIP on Intel CPUs without hardware UMIP (i.e. smsw is still allowed at CPL>0), we can always do the same emulation on AMD, because SVM has always offered intercepts of sgdt, sidt, sldt, and str. So, if you really want to offer this emulation on pre-EPYC 2 CPUs, this function should just return true. But, I have to ask, "why?" *Virtualization* of UMIP on EPYC 2 already works without any of these changes. > static void update_cr0_intercept(struct vcpu_svm *svm) > { > ulong gcr0 = svm->vcpu.arch.cr0; > @@ -4438,6 +4443,13 @@ static int interrupt_window_interception(struct vcpu_svm *svm) > return 1; > } > > +static int umip_interception(struct vcpu_svm *svm) > +{ > + struct kvm_vcpu *vcpu = &svm->vcpu; > + > + return kvm_emulate_instruction(vcpu, 0); > +} > + > static int pause_interception(struct vcpu_svm *svm) > { > struct kvm_vcpu *vcpu = &svm->vcpu; > @@ -4775,6 +4787,10 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = { > [SVM_EXIT_SMI] = nop_on_interception, > [SVM_EXIT_INIT] = nop_on_interception, > [SVM_EXIT_VINTR] = interrupt_window_interception, > + [SVM_EXIT_IDTR_READ] = umip_interception, > + [SVM_EXIT_GDTR_READ] = umip_interception, > + [SVM_EXIT_LDTR_READ] = umip_interception, > + [SVM_EXIT_TR_READ] = umip_interception, > [SVM_EXIT_RDPMC] = rdpmc_interception, > [SVM_EXIT_CPUID] = cpuid_interception, > [SVM_EXIT_IRET] = iret_interception, > @@ -5976,11 +5992,6 @@ static bool svm_xsaves_supported(void) > return boot_cpu_has(X86_FEATURE_XSAVES); > } > > -static bool svm_umip_emulated(void) > -{ > - return false; > -} > - > static bool svm_pt_supported(void) > { > return false; >
On 11/1/19 1:24 PM, Andy Lutomirski wrote: > On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: >> >> AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction >> Prevention) feature. The UMIP feature prevents the execution of certain >> instructions if the Current Privilege Level (CPL) is greater than 0. >> If any of these instructions are executed with CPL > 0 and UMIP >> is enabled, then kernel reports a #GP exception. >> >> The idea is taken from articles: >> https://lwn.net/Articles/738209/ >> https://lwn.net/Articles/694385/ >> >> Enable the feature if supported on bare metal and emulate instructions >> to return dummy values for certain cases. > > What are these cases? It is mentioned in the article https://lwn.net/Articles/738209/ === How does it impact applications? When enabled, however, UMIP will change the behavior that certain applications expect from the operating system. For instance, programs running on WineHQ and DOSEMU2 rely on some of these instructions to function. Stas Sergeev found that Microsoft Windows 3.1 and dos4gw use the instruction SMSW when running in virtual-8086 mode [4]. SGDT and SIDT can also be used on virtual-8086 mode. I have not tested these cases. I just followed the same code that was done for Intel UMIP.
On Fri, Nov 1, 2019 at 11:38 AM Moger, Babu <Babu.Moger@amd.com> wrote: > > > > On 11/1/19 1:24 PM, Andy Lutomirski wrote: > > On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: > >> > >> AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction > >> Prevention) feature. The UMIP feature prevents the execution of certain > >> instructions if the Current Privilege Level (CPL) is greater than 0. > >> If any of these instructions are executed with CPL > 0 and UMIP > >> is enabled, then kernel reports a #GP exception. > >> > >> The idea is taken from articles: > >> https://lwn.net/Articles/738209/ > >> https://lwn.net/Articles/694385/ > >> > >> Enable the feature if supported on bare metal and emulate instructions > >> to return dummy values for certain cases. > > > > What are these cases? > > It is mentioned in the article https://lwn.net/Articles/738209/ > > === How does it impact applications? > > When enabled, however, UMIP will change the behavior that certain > applications expect from the operating system. For instance, programs > running on WineHQ and DOSEMU2 rely on some of these instructions to > function. Stas Sergeev found that Microsoft Windows 3.1 and dos4gw use the > instruction SMSW when running in virtual-8086 mode [4]. SGDT and SIDT can > also be used on virtual-8086 mode. > What does that have to do with your series? Your series is about enabling UMIP (or emulating UMIP -- your descriptions are quite unclear) on AMD hardware, and the hypervisor should *not* be emulating instructions to return dummy values. The *guest kernel* already knows how to emulate userspace instructions as needed. --Andy
On 11/1/19 1:29 PM, Jim Mattson wrote: > On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: >> >> AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction >> Prevention) feature. The UMIP feature prevents the execution of certain >> instructions if the Current Privilege Level (CPL) is greater than 0. >> If any of these instructions are executed with CPL > 0 and UMIP >> is enabled, then kernel reports a #GP exception. >> >> The idea is taken from articles: >> https://lwn.net/Articles/738209/ >> https://lwn.net/Articles/694385/ >> >> Enable the feature if supported on bare metal and emulate instructions >> to return dummy values for certain cases. >> >> Signed-off-by: Babu Moger <babu.moger@amd.com> >> --- >> arch/x86/kvm/svm.c | 21 ++++++++++++++++----- >> 1 file changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 4153ca8cddb7..79abbdeca148 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) >> { >> } >> >> +static bool svm_umip_emulated(void) >> +{ >> + return boot_cpu_has(X86_FEATURE_UMIP); >> +} > > This makes no sense to me. If the hardware actually supports UMIP, > then it doesn't have to be emulated. My understanding.. If the hardware supports the UMIP, it will generate the #GP fault when these instructions are executed at CPL > 0. Purpose of the emulation is to trap the GP and return a dummy value. Seems like this required in certain legacy OSes running in protected and virtual-8086 modes. In long mode no need to emulate. Here is the bit explanation https://lwn.net/Articles/738209/ If we don't care about those legacy cases we don't need to emulate. > > To the extent that kvm emulates UMIP on Intel CPUs without hardware > UMIP (i.e. smsw is still allowed at CPL>0), we can always do the same > emulation on AMD, because SVM has always offered intercepts of sgdt, > sidt, sldt, and str. So, if you really want to offer this emulation on > pre-EPYC 2 CPUs, this function should just return true. But, I have to > ask, "why?" Trying to support UMIP feature only on EPYC 2 hardware. No intention to support pre-EPYC 2. > > *Virtualization* of UMIP on EPYC 2 already works without any of these changes. > >> static void update_cr0_intercept(struct vcpu_svm *svm) >> { >> ulong gcr0 = svm->vcpu.arch.cr0; >> @@ -4438,6 +4443,13 @@ static int interrupt_window_interception(struct vcpu_svm *svm) >> return 1; >> } >> >> +static int umip_interception(struct vcpu_svm *svm) >> +{ >> + struct kvm_vcpu *vcpu = &svm->vcpu; >> + >> + return kvm_emulate_instruction(vcpu, 0); >> +} >> + >> static int pause_interception(struct vcpu_svm *svm) >> { >> struct kvm_vcpu *vcpu = &svm->vcpu; >> @@ -4775,6 +4787,10 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = { >> [SVM_EXIT_SMI] = nop_on_interception, >> [SVM_EXIT_INIT] = nop_on_interception, >> [SVM_EXIT_VINTR] = interrupt_window_interception, >> + [SVM_EXIT_IDTR_READ] = umip_interception, >> + [SVM_EXIT_GDTR_READ] = umip_interception, >> + [SVM_EXIT_LDTR_READ] = umip_interception, >> + [SVM_EXIT_TR_READ] = umip_interception, >> [SVM_EXIT_RDPMC] = rdpmc_interception, >> [SVM_EXIT_CPUID] = cpuid_interception, >> [SVM_EXIT_IRET] = iret_interception, >> @@ -5976,11 +5992,6 @@ static bool svm_xsaves_supported(void) >> return boot_cpu_has(X86_FEATURE_XSAVES); >> } >> >> -static bool svm_umip_emulated(void) >> -{ >> - return false; >> -} >> - >> static bool svm_pt_supported(void) >> { >> return false; >>
On Fri, Nov 1, 2019 at 12:20 PM Moger, Babu <Babu.Moger@amd.com> wrote: > > > > On 11/1/19 1:29 PM, Jim Mattson wrote: > > On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: > >> > >> AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction > >> Prevention) feature. The UMIP feature prevents the execution of certain > >> instructions if the Current Privilege Level (CPL) is greater than 0. > >> If any of these instructions are executed with CPL > 0 and UMIP > >> is enabled, then kernel reports a #GP exception. > >> > >> The idea is taken from articles: > >> https://lwn.net/Articles/738209/ > >> https://lwn.net/Articles/694385/ > >> > >> Enable the feature if supported on bare metal and emulate instructions > >> to return dummy values for certain cases. > >> > >> Signed-off-by: Babu Moger <babu.moger@amd.com> > >> --- > >> arch/x86/kvm/svm.c | 21 ++++++++++++++++----- > >> 1 file changed, 16 insertions(+), 5 deletions(-) > >> > >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > >> index 4153ca8cddb7..79abbdeca148 100644 > >> --- a/arch/x86/kvm/svm.c > >> +++ b/arch/x86/kvm/svm.c > >> @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) > >> { > >> } > >> > >> +static bool svm_umip_emulated(void) > >> +{ > >> + return boot_cpu_has(X86_FEATURE_UMIP); > >> +} > > > > This makes no sense to me. If the hardware actually supports UMIP, > > then it doesn't have to be emulated. > My understanding.. > > If the hardware supports the UMIP, it will generate the #GP fault when > these instructions are executed at CPL > 0. Purpose of the emulation is to > trap the GP and return a dummy value. Seems like this required in certain > legacy OSes running in protected and virtual-8086 modes. In long mode no > need to emulate. Here is the bit explanation https://lwn.net/Articles/738209/ > Indeed. Again, what does this have to do with your patch? > > > > > To the extent that kvm emulates UMIP on Intel CPUs without hardware > > UMIP (i.e. smsw is still allowed at CPL>0), we can always do the same > > emulation on AMD, because SVM has always offered intercepts of sgdt, > > sidt, sldt, and str. So, if you really want to offer this emulation on > > pre-EPYC 2 CPUs, this function should just return true. But, I have to > > ask, "why?" > > > Trying to support UMIP feature only on EPYC 2 hardware. No intention to > support pre-EPYC 2. > I think you need to totally rewrite your changelog to explain what you are doing. As I understand it, there are a couple of things KVM can do: 1. If the underlying hardware supports UMIP, KVM can expose UMIP to the guest. SEV should be irrelevant here. 2. Regardless of whether the underlying hardware supports UMIP, KVM can try to emulate UMIP in the guest. This may be impossible if SEV is enabled. Which of these are you doing?
On 11/1/19 2:24 PM, Andy Lutomirski wrote: > On Fri, Nov 1, 2019 at 12:20 PM Moger, Babu <Babu.Moger@amd.com> wrote: >> >> >> >> On 11/1/19 1:29 PM, Jim Mattson wrote: >>> On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: >>>> >>>> AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction >>>> Prevention) feature. The UMIP feature prevents the execution of certain >>>> instructions if the Current Privilege Level (CPL) is greater than 0. >>>> If any of these instructions are executed with CPL > 0 and UMIP >>>> is enabled, then kernel reports a #GP exception. >>>> >>>> The idea is taken from articles: >>>> https://lwn.net/Articles/738209/ >>>> https://lwn.net/Articles/694385/ >>>> >>>> Enable the feature if supported on bare metal and emulate instructions >>>> to return dummy values for certain cases. >>>> >>>> Signed-off-by: Babu Moger <babu.moger@amd.com> >>>> --- >>>> arch/x86/kvm/svm.c | 21 ++++++++++++++++----- >>>> 1 file changed, 16 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >>>> index 4153ca8cddb7..79abbdeca148 100644 >>>> --- a/arch/x86/kvm/svm.c >>>> +++ b/arch/x86/kvm/svm.c >>>> @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) >>>> { >>>> } >>>> >>>> +static bool svm_umip_emulated(void) >>>> +{ >>>> + return boot_cpu_has(X86_FEATURE_UMIP); >>>> +} >>> >>> This makes no sense to me. If the hardware actually supports UMIP, >>> then it doesn't have to be emulated. >> My understanding.. >> >> If the hardware supports the UMIP, it will generate the #GP fault when >> these instructions are executed at CPL > 0. Purpose of the emulation is to >> trap the GP and return a dummy value. Seems like this required in certain >> legacy OSes running in protected and virtual-8086 modes. In long mode no >> need to emulate. Here is the bit explanation https://lwn.net/Articles/738209/ >> > > Indeed. Again, what does this have to do with your patch? > >> >>> >>> To the extent that kvm emulates UMIP on Intel CPUs without hardware >>> UMIP (i.e. smsw is still allowed at CPL>0), we can always do the same >>> emulation on AMD, because SVM has always offered intercepts of sgdt, >>> sidt, sldt, and str. So, if you really want to offer this emulation on >>> pre-EPYC 2 CPUs, this function should just return true. But, I have to >>> ask, "why?" >> >> >> Trying to support UMIP feature only on EPYC 2 hardware. No intention to >> support pre-EPYC 2. >> > > I think you need to totally rewrite your changelog to explain what you > are doing. > > As I understand it, there are a couple of things KVM can do: > > 1. If the underlying hardware supports UMIP, KVM can expose UMIP to > the guest. SEV should be irrelevant here. > > 2. Regardless of whether the underlying hardware supports UMIP, KVM > can try to emulate UMIP in the guest. This may be impossible if SEV > is enabled. > > Which of these are you doing? > My intention was to do 1. Let me go back and think about this again.
On Fri, Nov 1, 2019 at 1:04 PM Moger, Babu <Babu.Moger@amd.com> wrote: > > > > On 11/1/19 2:24 PM, Andy Lutomirski wrote: > > On Fri, Nov 1, 2019 at 12:20 PM Moger, Babu <Babu.Moger@amd.com> wrote: > >> > >> > >> > >> On 11/1/19 1:29 PM, Jim Mattson wrote: > >>> On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> wrote: > >>>> > >>>> AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction > >>>> Prevention) feature. The UMIP feature prevents the execution of certain > >>>> instructions if the Current Privilege Level (CPL) is greater than 0. > >>>> If any of these instructions are executed with CPL > 0 and UMIP > >>>> is enabled, then kernel reports a #GP exception. > >>>> > >>>> The idea is taken from articles: > >>>> https://lwn.net/Articles/738209/ > >>>> https://lwn.net/Articles/694385/ > >>>> > >>>> Enable the feature if supported on bare metal and emulate instructions > >>>> to return dummy values for certain cases. > >>>> > >>>> Signed-off-by: Babu Moger <babu.moger@amd.com> > >>>> --- > >>>> arch/x86/kvm/svm.c | 21 ++++++++++++++++----- > >>>> 1 file changed, 16 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > >>>> index 4153ca8cddb7..79abbdeca148 100644 > >>>> --- a/arch/x86/kvm/svm.c > >>>> +++ b/arch/x86/kvm/svm.c > >>>> @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) > >>>> { > >>>> } > >>>> > >>>> +static bool svm_umip_emulated(void) > >>>> +{ > >>>> + return boot_cpu_has(X86_FEATURE_UMIP); > >>>> +} > >>> > >>> This makes no sense to me. If the hardware actually supports UMIP, > >>> then it doesn't have to be emulated. > >> My understanding.. > >> > >> If the hardware supports the UMIP, it will generate the #GP fault when > >> these instructions are executed at CPL > 0. Purpose of the emulation is to > >> trap the GP and return a dummy value. Seems like this required in certain > >> legacy OSes running in protected and virtual-8086 modes. In long mode no > >> need to emulate. Here is the bit explanation https://lwn.net/Articles/738209/ > >> > > > > Indeed. Again, what does this have to do with your patch? > > > >> > >>> > >>> To the extent that kvm emulates UMIP on Intel CPUs without hardware > >>> UMIP (i.e. smsw is still allowed at CPL>0), we can always do the same > >>> emulation on AMD, because SVM has always offered intercepts of sgdt, > >>> sidt, sldt, and str. So, if you really want to offer this emulation on > >>> pre-EPYC 2 CPUs, this function should just return true. But, I have to > >>> ask, "why?" > >> > >> > >> Trying to support UMIP feature only on EPYC 2 hardware. No intention to > >> support pre-EPYC 2. > >> > > > > I think you need to totally rewrite your changelog to explain what you > > are doing. > > > > As I understand it, there are a couple of things KVM can do: > > > > 1. If the underlying hardware supports UMIP, KVM can expose UMIP to > > the guest. SEV should be irrelevant here. > > > > 2. Regardless of whether the underlying hardware supports UMIP, KVM > > can try to emulate UMIP in the guest. This may be impossible if SEV > > is enabled. > > > > Which of these are you doing? > > > My intention was to do 1. Let me go back and think about this again. (1) already works.
> -----Original Message----- > From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf > Of Jim Mattson > Sent: Friday, November 1, 2019 3:08 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: Andy Lutomirski <luto@kernel.org>; tglx@linutronix.de; > mingo@redhat.com; bp@alien8.de; hpa@zytor.com; pbonzini@redhat.com; > rkrcmar@redhat.com; sean.j.christopherson@intel.com; vkuznets@redhat.com; > wanpengli@tencent.com; x86@kernel.org; joro@8bytes.org; > zohar@linux.ibm.com; yamada.masahiro@socionext.com; > nayna@linux.ibm.com; linux-kernel@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH 2/4] kvm: svm: Enable UMIP feature on AMD > > On Fri, Nov 1, 2019 at 1:04 PM Moger, Babu <Babu.Moger@amd.com> wrote: > > > > > > > > On 11/1/19 2:24 PM, Andy Lutomirski wrote: > > > On Fri, Nov 1, 2019 at 12:20 PM Moger, Babu <Babu.Moger@amd.com> > wrote: > > >> > > >> > > >> > > >> On 11/1/19 1:29 PM, Jim Mattson wrote: > > >>> On Fri, Nov 1, 2019 at 10:33 AM Moger, Babu <Babu.Moger@amd.com> > wrote: > > >>>> > > >>>> AMD 2nd generation EPYC processors support UMIP (User-Mode > Instruction > > >>>> Prevention) feature. The UMIP feature prevents the execution of certain > > >>>> instructions if the Current Privilege Level (CPL) is greater than 0. > > >>>> If any of these instructions are executed with CPL > 0 and UMIP > > >>>> is enabled, then kernel reports a #GP exception. > > >>>> > > >>>> The idea is taken from articles: > > >>>> https://lwn.net/Articles/738209/ > > >>>> https://lwn.net/Articles/694385/ > > >>>> > > >>>> Enable the feature if supported on bare metal and emulate instructions > > >>>> to return dummy values for certain cases. > > >>>> > > >>>> Signed-off-by: Babu Moger <babu.moger@amd.com> > > >>>> --- > > >>>> arch/x86/kvm/svm.c | 21 ++++++++++++++++----- > > >>>> 1 file changed, 16 insertions(+), 5 deletions(-) > > >>>> > > >>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > > >>>> index 4153ca8cddb7..79abbdeca148 100644 > > >>>> --- a/arch/x86/kvm/svm.c > > >>>> +++ b/arch/x86/kvm/svm.c > > >>>> @@ -2533,6 +2533,11 @@ static void > svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) > > >>>> { > > >>>> } > > >>>> > > >>>> +static bool svm_umip_emulated(void) > > >>>> +{ > > >>>> + return boot_cpu_has(X86_FEATURE_UMIP); > > >>>> +} > > >>> > > >>> This makes no sense to me. If the hardware actually supports UMIP, > > >>> then it doesn't have to be emulated. > > >> My understanding.. > > >> > > >> If the hardware supports the UMIP, it will generate the #GP fault when > > >> these instructions are executed at CPL > 0. Purpose of the emulation is to > > >> trap the GP and return a dummy value. Seems like this required in certain > > >> legacy OSes running in protected and virtual-8086 modes. In long mode no > > >> need to emulate. Here is the bit explanation > https://lwn.net/Articles/738209/ > > >> > > > > > > Indeed. Again, what does this have to do with your patch? > > > > > >> > > >>> > > >>> To the extent that kvm emulates UMIP on Intel CPUs without hardware > > >>> UMIP (i.e. smsw is still allowed at CPL>0), we can always do the same > > >>> emulation on AMD, because SVM has always offered intercepts of sgdt, > > >>> sidt, sldt, and str. So, if you really want to offer this emulation on > > >>> pre-EPYC 2 CPUs, this function should just return true. But, I have to > > >>> ask, "why?" > > >> > > >> > > >> Trying to support UMIP feature only on EPYC 2 hardware. No intention to > > >> support pre-EPYC 2. > > >> > > > > > > I think you need to totally rewrite your changelog to explain what you > > > are doing. > > > > > > As I understand it, there are a couple of things KVM can do: > > > > > > 1. If the underlying hardware supports UMIP, KVM can expose UMIP to > > > the guest. SEV should be irrelevant here. > > > > > > 2. Regardless of whether the underlying hardware supports UMIP, KVM > > > can try to emulate UMIP in the guest. This may be impossible if SEV > > > is enabled. > > > > > > Which of these are you doing? > > > > > My intention was to do 1. Let me go back and think about this again. > > (1) already works. That’s right. Thanks Jim and Andy. How about updating the Kconfig (patch #4) and update it to CONFIG_X86_UMIP (instead of CONFIG_X86_INTEL_UMIP). Right now, it appears it is supported on only Intel.
On Sat, Nov 02, 2019 at 07:23:45PM +0000, Moger, Babu wrote: > How about updating the Kconfig (patch #4) and update it to > CONFIG_X86_UMIP (instead of CONFIG_X86_INTEL_UMIP). Yes, pls do that and make it depend on CPU_SUP_AMD too. Thx.
On 01/11/19 18:33, Moger, Babu wrote: > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 4153ca8cddb7..79abbdeca148 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) > { > } > > +static bool svm_umip_emulated(void) > +{ > + return boot_cpu_has(X86_FEATURE_UMIP); > +} For hardware that supports UMIP, this is only needed because of your patch 1. Without it, X86_FEATURE_UMIP should already be enabled on processors that natively support UMIP. If you want UMIP *emulation* instead, this should become "return true". > static void update_cr0_intercept(struct vcpu_svm *svm) > { > ulong gcr0 = svm->vcpu.arch.cr0; > @@ -4438,6 +4443,13 @@ static int interrupt_window_interception(struct vcpu_svm *svm) > return 1; > } > > +static int umip_interception(struct vcpu_svm *svm) > +{ > + struct kvm_vcpu *vcpu = &svm->vcpu; > + > + return kvm_emulate_instruction(vcpu, 0); > +} > + > static int pause_interception(struct vcpu_svm *svm) > { > struct kvm_vcpu *vcpu = &svm->vcpu; > @@ -4775,6 +4787,10 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = { > [SVM_EXIT_SMI] = nop_on_interception, > [SVM_EXIT_INIT] = nop_on_interception, > [SVM_EXIT_VINTR] = interrupt_window_interception, > + [SVM_EXIT_IDTR_READ] = umip_interception, > + [SVM_EXIT_GDTR_READ] = umip_interception, > + [SVM_EXIT_LDTR_READ] = umip_interception, > + [SVM_EXIT_TR_READ] = umip_interception, This is missing enabling the intercepts. Also, this can be just emulate_on_interception instead of a new function. Paolo > [SVM_EXIT_RDPMC] = rdpmc_interception, > [SVM_EXIT_CPUID] = cpuid_interception, > [SVM_EXIT_IRET] = iret_interception, > @@ -5976,11 +5992,6 @@ static bool svm_xsaves_supported(void) > return boot_cpu_has(X86_FEATURE_XSAVES); > } > > -static bool svm_umip_emulated(void) > -{ > - return false; > -} > - > static bool svm_pt_supported(void) > { > return false; >
On 11/4/19 5:54 AM, Paolo Bonzini wrote: > On 01/11/19 18:33, Moger, Babu wrote: >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 4153ca8cddb7..79abbdeca148 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) >> { >> } >> >> +static bool svm_umip_emulated(void) >> +{ >> + return boot_cpu_has(X86_FEATURE_UMIP); >> +} > > For hardware that supports UMIP, this is only needed because of your > patch 1. Without it, X86_FEATURE_UMIP should already be enabled on > processors that natively support UMIP. Yes, That is correct. Will remove the patch #1. Intention was to enable UMIP for the hardware that supports it. Will send out only the config changes(Patch #4). Also there is a complexity with supporting emulation on SEV guest. > > If you want UMIP *emulation* instead, this should become "return true". > >> static void update_cr0_intercept(struct vcpu_svm *svm) >> { >> ulong gcr0 = svm->vcpu.arch.cr0; >> @@ -4438,6 +4443,13 @@ static int interrupt_window_interception(struct vcpu_svm *svm) >> return 1; >> } >> >> +static int umip_interception(struct vcpu_svm *svm) >> +{ >> + struct kvm_vcpu *vcpu = &svm->vcpu; >> + >> + return kvm_emulate_instruction(vcpu, 0); >> +} >> + >> static int pause_interception(struct vcpu_svm *svm) >> { >> struct kvm_vcpu *vcpu = &svm->vcpu; >> @@ -4775,6 +4787,10 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = { >> [SVM_EXIT_SMI] = nop_on_interception, >> [SVM_EXIT_INIT] = nop_on_interception, >> [SVM_EXIT_VINTR] = interrupt_window_interception, >> + [SVM_EXIT_IDTR_READ] = umip_interception, >> + [SVM_EXIT_GDTR_READ] = umip_interception, >> + [SVM_EXIT_LDTR_READ] = umip_interception, >> + [SVM_EXIT_TR_READ] = umip_interception, > > This is missing enabling the intercepts. Also, this can be just > emulate_on_interception instead of a new function. > > Paolo > >> [SVM_EXIT_RDPMC] = rdpmc_interception, >> [SVM_EXIT_CPUID] = cpuid_interception, >> [SVM_EXIT_IRET] = iret_interception, >> @@ -5976,11 +5992,6 @@ static bool svm_xsaves_supported(void) >> return boot_cpu_has(X86_FEATURE_XSAVES); >> } >> >> -static bool svm_umip_emulated(void) >> -{ >> - return false; >> -} >> - >> static bool svm_pt_supported(void) >> { >> return false; >> >
On 11/3/19 5:45 AM, Borislav Petkov wrote: > On Sat, Nov 02, 2019 at 07:23:45PM +0000, Moger, Babu wrote: >> How about updating the Kconfig (patch #4) and update it to >> CONFIG_X86_UMIP (instead of CONFIG_X86_INTEL_UMIP). > > Yes, pls do that and make it depend on CPU_SUP_AMD too. Sure. Will send the patch soon. Thanks > > Thx. >
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 4153ca8cddb7..79abbdeca148 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2533,6 +2533,11 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) { } +static bool svm_umip_emulated(void) +{ + return boot_cpu_has(X86_FEATURE_UMIP); +} + static void update_cr0_intercept(struct vcpu_svm *svm) { ulong gcr0 = svm->vcpu.arch.cr0; @@ -4438,6 +4443,13 @@ static int interrupt_window_interception(struct vcpu_svm *svm) return 1; } +static int umip_interception(struct vcpu_svm *svm) +{ + struct kvm_vcpu *vcpu = &svm->vcpu; + + return kvm_emulate_instruction(vcpu, 0); +} + static int pause_interception(struct vcpu_svm *svm) { struct kvm_vcpu *vcpu = &svm->vcpu; @@ -4775,6 +4787,10 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = { [SVM_EXIT_SMI] = nop_on_interception, [SVM_EXIT_INIT] = nop_on_interception, [SVM_EXIT_VINTR] = interrupt_window_interception, + [SVM_EXIT_IDTR_READ] = umip_interception, + [SVM_EXIT_GDTR_READ] = umip_interception, + [SVM_EXIT_LDTR_READ] = umip_interception, + [SVM_EXIT_TR_READ] = umip_interception, [SVM_EXIT_RDPMC] = rdpmc_interception, [SVM_EXIT_CPUID] = cpuid_interception, [SVM_EXIT_IRET] = iret_interception, @@ -5976,11 +5992,6 @@ static bool svm_xsaves_supported(void) return boot_cpu_has(X86_FEATURE_XSAVES); } -static bool svm_umip_emulated(void) -{ - return false; -} - static bool svm_pt_supported(void) { return false;
AMD 2nd generation EPYC processors support UMIP (User-Mode Instruction Prevention) feature. The UMIP feature prevents the execution of certain instructions if the Current Privilege Level (CPL) is greater than 0. If any of these instructions are executed with CPL > 0 and UMIP is enabled, then kernel reports a #GP exception. The idea is taken from articles: https://lwn.net/Articles/738209/ https://lwn.net/Articles/694385/ Enable the feature if supported on bare metal and emulate instructions to return dummy values for certain cases. Signed-off-by: Babu Moger <babu.moger@amd.com> --- arch/x86/kvm/svm.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)