| Message ID | 1495818213-345-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 26.05.17 at 19:03, <andrew.cooper3@citrix.com> wrote: > This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. > > When determining Access Rights, Protection Keys only take effect when CR4.PKE > it set, and 4-level paging is active. All other circumstances (notibly, 32bit > PAE paging) skip the Protection Key control mechanism. > > Therefore, we do not need to clear CR4.PKE behind the back of a guest which is > not using paging, as such a guest is necesserily running with EFER.LME > disabled. DYM EFER.LMA here? > The {RD,WR}PKRU instructions are specified as being legal for use in any > operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the > back of an unpaged guest, these instructions yield #UD despite the guest > seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> I would like to get clarification from Huaitong, however, on the reason for the original change: According to the reasoning here, it shouldn't have been an observed failure of some kind, but merely the thinking that something may be wrong (but really wasn't). Jan
On Fri, 2017-05-26 at 18:03 +0100, Andrew Cooper wrote: > This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. > > When determining Access Rights, Protection Keys only take effect when CR4.PKE > it set, and 4-level paging is active. All other circumstances (notibly, 32bit > PAE paging) skip the Protection Key control mechanism. > > Therefore, we do not need to clear CR4.PKE behind the back of a guest which is > not using paging, as such a guest is necesserily running with EFER.LME > disabled. Yes, if EFER.LME = 0, Protection Keys would take no effect too, so it isn't necessary to clear CR4.PKE in non-paging mode. > > The {RD,WR}PKRU instructions are specified as being legal for use in any > operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the > back of an unpaged guest, these instructions yield #UD despite the guest > seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. If CR4.PKE is cleared, OSPKE would be invisible at the same time. When guest does set CR4_PKE in non-paging mode, then CR4_PKE would be cleared in vmcs loading, so, OSPKE should be always invisible, and #UD should not be yielded too. Reviewed-by: Huaitong Han <huaitong.han@intel.com> > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Jun Nakajima <jun.nakajima@intel.com> > CC: Kevin Tian <kevin.tian@intel.com> > CC: Huaitong Han <huaitong.han@intel.com> > --- > xen/arch/x86/hvm/vmx/vmx.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index c8ef18a..58552c3 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -1673,13 +1673,12 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr) > if ( !hvm_paging_enabled(v) ) > { > /* > - * SMEP/SMAP/PKU is disabled if CPU is in non-paging mode in > - * hardware. However Xen always uses paging mode to emulate guest > - * non-paging mode. To emulate this behavior, SMEP/SMAP/PKU needs > - * to be manually disabled when guest VCPU is in non-paging mode. > + * SMEP/SMAP is disabled if CPU is in non-paging mode in hardware. > + * However Xen always uses paging mode to emulate guest non-paging > + * mode. To emulate this behavior, SMEP/SMAP needs to be manually > + * disabled when guest VCPU is in non-paging mode. > */ > - v->arch.hvm_vcpu.hw_cr[4] &= > - ~(X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE); > + v->arch.hvm_vcpu.hw_cr[4] &= ~(X86_CR4_SMEP | X86_CR4_SMAP); > } > __vmwrite(GUEST_CR4, v->arch.hvm_vcpu.hw_cr[4]); > break;
On 31/05/2017 08:09, Han, Huaitong wrote: > On Fri, 2017-05-26 at 18:03 +0100, Andrew Cooper wrote: >> This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. >> >> When determining Access Rights, Protection Keys only take effect when CR4.PKE >> it set, and 4-level paging is active. All other circumstances (notibly, 32bit >> PAE paging) skip the Protection Key control mechanism. >> >> Therefore, we do not need to clear CR4.PKE behind the back of a guest which is >> not using paging, as such a guest is necesserily running with EFER.LME >> disabled. > Yes, if EFER.LME = 0, Protection Keys would take no effect too, so it > isn't necessary to clear CR4.PKE in non-paging mode. > >> The {RD,WR}PKRU instructions are specified as being legal for use in any >> operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the >> back of an unpaged guest, these instructions yield #UD despite the guest >> seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. > If CR4.PKE is cleared, OSPKE would be invisible at the same time. When > guest does set CR4_PKE in non-paging mode, then CR4_PKE would be cleared > in vmcs loading, so, OSPKE should be always invisible, and #UD should > not be yielded too. Remember that for HVM guests, Xen calculates OSPKE in software; it never comes from hardware, as CPUID is an automatic VMEXIT. The CPUID code uses the same source of information as a read from cr4, so comes to the conclusion that OSPKE should be visible. Therefore, when the guest looks at CPUID, it sees OSPKE set even though hardware would come to the opposite conclusion. > Reviewed-by: Huaitong Han <huaitong.han@intel.com> Does this stand despite the OSPKE issue? ~Andrew
>>> On 31.05.17 at 09:44, <andrew.cooper3@citrix.com> wrote: > On 31/05/2017 08:09, Han, Huaitong wrote: >> On Fri, 2017-05-26 at 18:03 +0100, Andrew Cooper wrote: >>> This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. >>> >>> When determining Access Rights, Protection Keys only take effect when > CR4.PKE >>> it set, and 4-level paging is active. All other circumstances (notibly, > 32bit >>> PAE paging) skip the Protection Key control mechanism. >>> >>> Therefore, we do not need to clear CR4.PKE behind the back of a guest which > is >>> not using paging, as such a guest is necesserily running with EFER.LME >>> disabled. >> Yes, if EFER.LME = 0, Protection Keys would take no effect too, so it >> isn't necessary to clear CR4.PKE in non-paging mode. >> >>> The {RD,WR}PKRU instructions are specified as being legal for use in any >>> operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the >>> back of an unpaged guest, these instructions yield #UD despite the guest >>> seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. >> If CR4.PKE is cleared, OSPKE would be invisible at the same time. When >> guest does set CR4_PKE in non-paging mode, then CR4_PKE would be cleared >> in vmcs loading, so, OSPKE should be always invisible, and #UD should >> not be yielded too. > > Remember that for HVM guests, Xen calculates OSPKE in software; it never > comes from hardware, as CPUID is an automatic VMEXIT. > > The CPUID code uses the same source of information as a read from cr4, > so comes to the conclusion that OSPKE should be visible. > > Therefore, when the guest looks at CPUID, it sees OSPKE set even though > hardware would come to the opposite conclusion. Shouldn't we correct this (independent of the patch here)? Jan
On 31/05/2017 08:56, Jan Beulich wrote: >>>> On 31.05.17 at 09:44, <andrew.cooper3@citrix.com> wrote: >> On 31/05/2017 08:09, Han, Huaitong wrote: >>> On Fri, 2017-05-26 at 18:03 +0100, Andrew Cooper wrote: >>>> This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. >>>> >>>> When determining Access Rights, Protection Keys only take effect when >> CR4.PKE >>>> it set, and 4-level paging is active. All other circumstances (notibly, >> 32bit >>>> PAE paging) skip the Protection Key control mechanism. >>>> >>>> Therefore, we do not need to clear CR4.PKE behind the back of a guest which >> is >>>> not using paging, as such a guest is necesserily running with EFER.LME >>>> disabled. >>> Yes, if EFER.LME = 0, Protection Keys would take no effect too, so it >>> isn't necessary to clear CR4.PKE in non-paging mode. >>> >>>> The {RD,WR}PKRU instructions are specified as being legal for use in any >>>> operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the >>>> back of an unpaged guest, these instructions yield #UD despite the guest >>>> seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. >>> If CR4.PKE is cleared, OSPKE would be invisible at the same time. When >>> guest does set CR4_PKE in non-paging mode, then CR4_PKE would be cleared >>> in vmcs loading, so, OSPKE should be always invisible, and #UD should >>> not be yielded too. >> Remember that for HVM guests, Xen calculates OSPKE in software; it never >> comes from hardware, as CPUID is an automatic VMEXIT. >> >> The CPUID code uses the same source of information as a read from cr4, >> so comes to the conclusion that OSPKE should be visible. >> >> Therefore, when the guest looks at CPUID, it sees OSPKE set even though >> hardware would come to the opposite conclusion. > Shouldn't we correct this (independent of the patch here)? No, I don't think so. That would involve the generic cpuid code looking at GUEST_CR4 and making decisions contrary to what is described in the manuals. Besides, it very definitely should be visible in a read of CR4 (because the guest did really set it), which means OSPKE should be visible in CPUID. This corner case (along with others) will soon be regression tested in the (newly-introduced) 0-level paging pagetable-emulation XTF test, which I put together after stumbling blindly into this particular #UD while investigating a different issue. ~Andrew
>>> On 31.05.17 at 10:06, <andrew.cooper3@citrix.com> wrote: > On 31/05/2017 08:56, Jan Beulich wrote: >>>>> On 31.05.17 at 09:44, <andrew.cooper3@citrix.com> wrote: >>> On 31/05/2017 08:09, Han, Huaitong wrote: >>>> On Fri, 2017-05-26 at 18:03 +0100, Andrew Cooper wrote: >>>>> This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. >>>>> >>>>> When determining Access Rights, Protection Keys only take effect when >>> CR4.PKE >>>>> it set, and 4-level paging is active. All other circumstances (notibly, >>> 32bit >>>>> PAE paging) skip the Protection Key control mechanism. >>>>> >>>>> Therefore, we do not need to clear CR4.PKE behind the back of a guest which >>> is >>>>> not using paging, as such a guest is necesserily running with EFER.LME >>>>> disabled. >>>> Yes, if EFER.LME = 0, Protection Keys would take no effect too, so it >>>> isn't necessary to clear CR4.PKE in non-paging mode. >>>> >>>>> The {RD,WR}PKRU instructions are specified as being legal for use in any >>>>> operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the >>>>> back of an unpaged guest, these instructions yield #UD despite the guest >>>>> seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. >>>> If CR4.PKE is cleared, OSPKE would be invisible at the same time. When >>>> guest does set CR4_PKE in non-paging mode, then CR4_PKE would be cleared >>>> in vmcs loading, so, OSPKE should be always invisible, and #UD should >>>> not be yielded too. >>> Remember that for HVM guests, Xen calculates OSPKE in software; it never >>> comes from hardware, as CPUID is an automatic VMEXIT. >>> >>> The CPUID code uses the same source of information as a read from cr4, >>> so comes to the conclusion that OSPKE should be visible. >>> >>> Therefore, when the guest looks at CPUID, it sees OSPKE set even though >>> hardware would come to the opposite conclusion. >> Shouldn't we correct this (independent of the patch here)? > > No, I don't think so. That would involve the generic cpuid code looking > at GUEST_CR4 and making decisions contrary to what is described in the > manuals. > > Besides, it very definitely should be visible in a read of CR4 (because > the guest did really set it), which means OSPKE should be visible in CPUID. Oh, then I misunderstood your earlier reply, taking it that we wrongly show the flag as set to the guest. Jan
On Wed, 2017-05-31 at 08:44 +0100, Andrew Cooper wrote: > On 31/05/2017 08:09, Han, Huaitong wrote: > > On Fri, 2017-05-26 at 18:03 +0100, Andrew Cooper wrote: > >> This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. > >> > >> When determining Access Rights, Protection Keys only take effect when CR4.PKE > >> it set, and 4-level paging is active. All other circumstances (notibly, 32bit > >> PAE paging) skip the Protection Key control mechanism. > >> > >> Therefore, we do not need to clear CR4.PKE behind the back of a guest which is > >> not using paging, as such a guest is necesserily running with EFER.LME > >> disabled. > > Yes, if EFER.LME = 0, Protection Keys would take no effect too, so it > > isn't necessary to clear CR4.PKE in non-paging mode. > > > >> The {RD,WR}PKRU instructions are specified as being legal for use in any > >> operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the > >> back of an unpaged guest, these instructions yield #UD despite the guest > >> seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. > > If CR4.PKE is cleared, OSPKE would be invisible at the same time. When > > guest does set CR4_PKE in non-paging mode, then CR4_PKE would be cleared > > in vmcs loading, so, OSPKE should be always invisible, and #UD should > > not be yielded too. > > Remember that for HVM guests, Xen calculates OSPKE in software; it never > comes from hardware, as CPUID is an automatic VMEXIT. > > The CPUID code uses the same source of information as a read from cr4, > so comes to the conclusion that OSPKE should be visible. > > Therefore, when the guest looks at CPUID, it sees OSPKE set even though > hardware would come to the opposite conclusion. Yes, I get the reason: the hw_cr4 is updated, but the guest_cr4 is not updated, so the OSPKE is visible. > > > Reviewed-by: Huaitong Han <huaitong.han@intel.com> > > Does this stand despite the OSPKE issue? Yes, I have no comments now. > > ~Andrew
> From: Han, Huaitong > Sent: Wednesday, May 31, 2017 4:15 PM > > On Wed, 2017-05-31 at 08:44 +0100, Andrew Cooper wrote: > > On 31/05/2017 08:09, Han, Huaitong wrote: > > > On Fri, 2017-05-26 at 18:03 +0100, Andrew Cooper wrote: > > >> This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. > > >> > > >> When determining Access Rights, Protection Keys only take effect when > CR4.PKE > > >> it set, and 4-level paging is active. All other circumstances (notibly, 32bit > > >> PAE paging) skip the Protection Key control mechanism. > > >> > > >> Therefore, we do not need to clear CR4.PKE behind the back of a guest > which is > > >> not using paging, as such a guest is necesserily running with EFER.LME > > >> disabled. > > > Yes, if EFER.LME = 0, Protection Keys would take no effect too, so it > > > isn't necessary to clear CR4.PKE in non-paging mode. > > > > > >> The {RD,WR}PKRU instructions are specified as being legal for use in any > > >> operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind > the > > >> back of an unpaged guest, these instructions yield #UD despite the guest > > >> seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. > > > If CR4.PKE is cleared, OSPKE would be invisible at the same time. When > > > guest does set CR4_PKE in non-paging mode, then CR4_PKE would be > cleared > > > in vmcs loading, so, OSPKE should be always invisible, and #UD should > > > not be yielded too. > > > > Remember that for HVM guests, Xen calculates OSPKE in software; it never > > comes from hardware, as CPUID is an automatic VMEXIT. > > > > The CPUID code uses the same source of information as a read from cr4, > > so comes to the conclusion that OSPKE should be visible. > > > > Therefore, when the guest looks at CPUID, it sees OSPKE set even though > > hardware would come to the opposite conclusion. > > Yes, I get the reason: the hw_cr4 is updated, but the guest_cr4 is not > updated, so the OSPKE is visible. > > > > > > Reviewed-by: Huaitong Han <huaitong.han@intel.com> > > > > Does this stand despite the OSPKE issue? > Yes, I have no comments now. > Acked-by: Kevin Tian <kevin.tian@intel.com>
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index c8ef18a..58552c3 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1673,13 +1673,12 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr) if ( !hvm_paging_enabled(v) ) { /* - * SMEP/SMAP/PKU is disabled if CPU is in non-paging mode in - * hardware. However Xen always uses paging mode to emulate guest - * non-paging mode. To emulate this behavior, SMEP/SMAP/PKU needs - * to be manually disabled when guest VCPU is in non-paging mode. + * SMEP/SMAP is disabled if CPU is in non-paging mode in hardware. + * However Xen always uses paging mode to emulate guest non-paging + * mode. To emulate this behavior, SMEP/SMAP needs to be manually + * disabled when guest VCPU is in non-paging mode. */ - v->arch.hvm_vcpu.hw_cr[4] &= - ~(X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE); + v->arch.hvm_vcpu.hw_cr[4] &= ~(X86_CR4_SMEP | X86_CR4_SMAP); } __vmwrite(GUEST_CR4, v->arch.hvm_vcpu.hw_cr[4]); break;
This reverts commit c41e0266dd59ab50b7a153157e9bd2a3ad114b53. When determining Access Rights, Protection Keys only take effect when CR4.PKE it set, and 4-level paging is active. All other circumstances (notibly, 32bit PAE paging) skip the Protection Key control mechanism. Therefore, we do not need to clear CR4.PKE behind the back of a guest which is not using paging, as such a guest is necesserily running with EFER.LME disabled. The {RD,WR}PKRU instructions are specified as being legal for use in any operating mode, but only if CR4.PKE is set. By clearing CR4.PKE behind the back of an unpaged guest, these instructions yield #UD despite the guest seeing PKE set if it reads CR4, and OSPKE being visible in CPUID. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Jun Nakajima <jun.nakajima@intel.com> CC: Kevin Tian <kevin.tian@intel.com> CC: Huaitong Han <huaitong.han@intel.com> --- xen/arch/x86/hvm/vmx/vmx.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)