[SRU,Bionic] Revert "KVM: x86: Manually calculate reserved bits when loading PDPTRS"
diff mbox series

Message ID 20191111222432.150741-1-cascardo@canonical.com
State New
Headers show
Series
  • [SRU,Bionic] Revert "KVM: x86: Manually calculate reserved bits when loading PDPTRS"
Related show

Commit Message

Thadeu Lima de Souza Cascardo Nov. 11, 2019, 10:24 p.m. UTC
This reverts commit c5d574d63996442c80e886c60278a790230f4d2b.

This commit has caused failure to launch 64-bit linux VMs when kvm-intel
ept module parameter is off. It is off by default on old Intel CPUs.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
---
 arch/x86/kvm/x86.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

Comments

Tyler Hicks Nov. 11, 2019, 10:43 p.m. UTC | #1
On 2019-11-11 19:24:32, Thadeu Lima de Souza Cascardo wrote:
> This reverts commit c5d574d63996442c80e886c60278a790230f4d2b.
> 
> This commit has caused failure to launch 64-bit linux VMs when kvm-intel
> ept module parameter is off. It is off by default on old Intel CPUs.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>

Acked-by: Tyler Hicks <tyhicks@canonical.com>

Thanks!

Tyler

> ---
>  arch/x86/kvm/x86.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ddbd7578ae25..d59095ab1f1c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -566,14 +566,8 @@ static int kvm_read_nested_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn,
>  				       data, offset, len, access);
>  }
>  
> -static inline u64 pdptr_rsvd_bits(struct kvm_vcpu *vcpu)
> -{
> -	return rsvd_bits(cpuid_maxphyaddr(vcpu), 63) | rsvd_bits(5, 8) |
> -	       rsvd_bits(1, 2);
> -}
> -
>  /*
> - * Load the pae pdptrs.  Return 1 if they are all valid, 0 otherwise.
> + * Load the pae pdptrs.  Return true is they are all valid.
>   */
>  int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
>  {
> @@ -592,7 +586,8 @@ int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
>  	}
>  	for (i = 0; i < ARRAY_SIZE(pdpte); ++i) {
>  		if ((pdpte[i] & PT_PRESENT_MASK) &&
> -		    (pdpte[i] & pdptr_rsvd_bits(vcpu))) {
> +		    (pdpte[i] &
> +		     vcpu->arch.mmu.guest_rsvd_check.rsvd_bits_mask[0][2])) {
>  			ret = 0;
>  			goto out;
>  		}
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Khaled Elmously Nov. 12, 2019, 4:13 a.m. UTC | #2
Was there a bug link for this regression?


On 2019-11-11 19:24:32 , Thadeu Lima de Souza Cascardo wrote:
> This reverts commit c5d574d63996442c80e886c60278a790230f4d2b.
> 
> This commit has caused failure to launch 64-bit linux VMs when kvm-intel
> ept module parameter is off. It is off by default on old Intel CPUs.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> ---
>  arch/x86/kvm/x86.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ddbd7578ae25..d59095ab1f1c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -566,14 +566,8 @@ static int kvm_read_nested_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn,
>  				       data, offset, len, access);
>  }
>  
> -static inline u64 pdptr_rsvd_bits(struct kvm_vcpu *vcpu)
> -{
> -	return rsvd_bits(cpuid_maxphyaddr(vcpu), 63) | rsvd_bits(5, 8) |
> -	       rsvd_bits(1, 2);
> -}
> -
>  /*
> - * Load the pae pdptrs.  Return 1 if they are all valid, 0 otherwise.
> + * Load the pae pdptrs.  Return true is they are all valid.
>   */
>  int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
>  {
> @@ -592,7 +586,8 @@ int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
>  	}
>  	for (i = 0; i < ARRAY_SIZE(pdpte); ++i) {
>  		if ((pdpte[i] & PT_PRESENT_MASK) &&
> -		    (pdpte[i] & pdptr_rsvd_bits(vcpu))) {
> +		    (pdpte[i] &
> +		     vcpu->arch.mmu.guest_rsvd_check.rsvd_bits_mask[0][2])) {
>  			ret = 0;
>  			goto out;
>  		}
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Po-Hsu Lin Nov. 12, 2019, 4:17 a.m. UTC | #3
I think this one can be used:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1851709

On Tue, Nov 12, 2019 at 12:13 PM Khaled Elmously
<khalid.elmously@canonical.com> wrote:
>
> Was there a bug link for this regression?
>
>
> On 2019-11-11 19:24:32 , Thadeu Lima de Souza Cascardo wrote:
> > This reverts commit c5d574d63996442c80e886c60278a790230f4d2b.
> >
> > This commit has caused failure to launch 64-bit linux VMs when kvm-intel
> > ept module parameter is off. It is off by default on old Intel CPUs.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> > ---
> >  arch/x86/kvm/x86.c | 11 +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ddbd7578ae25..d59095ab1f1c 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -566,14 +566,8 @@ static int kvm_read_nested_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn,
> >                                      data, offset, len, access);
> >  }
> >
> > -static inline u64 pdptr_rsvd_bits(struct kvm_vcpu *vcpu)
> > -{
> > -     return rsvd_bits(cpuid_maxphyaddr(vcpu), 63) | rsvd_bits(5, 8) |
> > -            rsvd_bits(1, 2);
> > -}
> > -
> >  /*
> > - * Load the pae pdptrs.  Return 1 if they are all valid, 0 otherwise.
> > + * Load the pae pdptrs.  Return true is they are all valid.
> >   */
> >  int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
> >  {
> > @@ -592,7 +586,8 @@ int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
> >       }
> >       for (i = 0; i < ARRAY_SIZE(pdpte); ++i) {
> >               if ((pdpte[i] & PT_PRESENT_MASK) &&
> > -                 (pdpte[i] & pdptr_rsvd_bits(vcpu))) {
> > +                 (pdpte[i] &
> > +                  vcpu->arch.mmu.guest_rsvd_check.rsvd_bits_mask[0][2])) {
> >                       ret = 0;
> >                       goto out;
> >               }
> > --
> > 2.20.1
> >
> >
> > --
> > kernel-team mailing list
> > kernel-team@lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Thadeu Lima de Souza Cascardo Nov. 12, 2019, 8:58 a.m. UTC | #4
On Tue, Nov 12, 2019 at 12:17:36PM +0800, Po-Hsu Lin wrote:
> I think this one can be used:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1851709

Sorry, I meant to use the other one, which has been marked as duplicated of this one.

LP: #1851709 is good enough. Notice that this has been tested to fix the issue.

I will send again including the fix for disco.

Cascardo.


> 
> On Tue, Nov 12, 2019 at 12:13 PM Khaled Elmously
> <khalid.elmously@canonical.com> wrote:
> >
> > Was there a bug link for this regression?
> >
> >
> > On 2019-11-11 19:24:32 , Thadeu Lima de Souza Cascardo wrote:
> > > This reverts commit c5d574d63996442c80e886c60278a790230f4d2b.
> > >
> > > This commit has caused failure to launch 64-bit linux VMs when kvm-intel
> > > ept module parameter is off. It is off by default on old Intel CPUs.
> > >
> > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> > > ---
> > >  arch/x86/kvm/x86.c | 11 +++--------
> > >  1 file changed, 3 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index ddbd7578ae25..d59095ab1f1c 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -566,14 +566,8 @@ static int kvm_read_nested_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn,
> > >                                      data, offset, len, access);
> > >  }
> > >
> > > -static inline u64 pdptr_rsvd_bits(struct kvm_vcpu *vcpu)
> > > -{
> > > -     return rsvd_bits(cpuid_maxphyaddr(vcpu), 63) | rsvd_bits(5, 8) |
> > > -            rsvd_bits(1, 2);
> > > -}
> > > -
> > >  /*
> > > - * Load the pae pdptrs.  Return 1 if they are all valid, 0 otherwise.
> > > + * Load the pae pdptrs.  Return true is they are all valid.
> > >   */
> > >  int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
> > >  {
> > > @@ -592,7 +586,8 @@ int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
> > >       }
> > >       for (i = 0; i < ARRAY_SIZE(pdpte); ++i) {
> > >               if ((pdpte[i] & PT_PRESENT_MASK) &&
> > > -                 (pdpte[i] & pdptr_rsvd_bits(vcpu))) {
> > > +                 (pdpte[i] &
> > > +                  vcpu->arch.mmu.guest_rsvd_check.rsvd_bits_mask[0][2])) {
> > >                       ret = 0;
> > >                       goto out;
> > >               }
> > > --
> > > 2.20.1
> > >
> > >
> > > --
> > > kernel-team mailing list
> > > kernel-team@lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
> >
> > --
> > kernel-team mailing list
> > kernel-team@lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
Stefan Bader Nov. 12, 2019, 9:29 a.m. UTC | #5
This was re-submitted in a different thread.

Patch
diff mbox series

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ddbd7578ae25..d59095ab1f1c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -566,14 +566,8 @@  static int kvm_read_nested_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn,
 				       data, offset, len, access);
 }
 
-static inline u64 pdptr_rsvd_bits(struct kvm_vcpu *vcpu)
-{
-	return rsvd_bits(cpuid_maxphyaddr(vcpu), 63) | rsvd_bits(5, 8) |
-	       rsvd_bits(1, 2);
-}
-
 /*
- * Load the pae pdptrs.  Return 1 if they are all valid, 0 otherwise.
+ * Load the pae pdptrs.  Return true is they are all valid.
  */
 int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
 {
@@ -592,7 +586,8 @@  int load_pdptrs(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, unsigned long cr3)
 	}
 	for (i = 0; i < ARRAY_SIZE(pdpte); ++i) {
 		if ((pdpte[i] & PT_PRESENT_MASK) &&
-		    (pdpte[i] & pdptr_rsvd_bits(vcpu))) {
+		    (pdpte[i] &
+		     vcpu->arch.mmu.guest_rsvd_check.rsvd_bits_mask[0][2])) {
 			ret = 0;
 			goto out;
 		}