[PATCH v2 0/2] KVM: x86: Replace magic number with readable macro

[PATCH v2 0/2] KVM: x86: Replace magic number with readable macro

[SU Hang]

Replace magic number 0x180 with `EPT_VIOLATION_GVA_IS_VALID | EPT_VIOLATION_GVA_TRANSLATED`
in arch/x86/kvm/mmu/paging_tmpl.h
Similarly, replace `(pte_access & 0x7) << 3` with
`(pte_access & VMX_EPT_RWX_MASK) << EPT_VIOLATION_RWX_SHIFT`.

v1 -> v2: https://lore.kernel.org/kvm/20220321094203.109546-1-darcy.sh@antgroup.com/
- Rename `EPT_VIOLATION_GVA_VALIDATION` to `EPT_VIOLATION_GVA_IS_VALID`. [Sean]
- Using new added `VMX_EPT_RWX_MASK` to replace magic number 0x7 and so on,
  to avoid using branch statement in hotpath. [Sean]

SU Hang (1):
  KVM: VMX:  replace 0x180 with EPT_VIOLATION_* definition

Sean Christopherson (1):
  KVM: x86/mmu: Derive EPT violation RWX bits from EPTE RWX bits

 arch/x86/include/asm/vmx.h     |  9 +++------
 arch/x86/kvm/mmu/paging_tmpl.h | 11 +++++++++--
 arch/x86/kvm/vmx/vmx.c         |  4 +---
 3 files changed, 13 insertions(+), 11 deletions(-)

--
2.32.0.3.g01195cf9f

[PATCH v2 1/2] KVM: VMX: replace 0x180 with EPT_VIOLATION_* definition

[SU Hang]

Using self-expressing macro definition EPT_VIOLATION_GVA_VALIDATION
and EPT_VIOLATION_GVA_TRANSLATED instead of 0x180
in FNAME(walk_addr_generic)().

Signed-off-by: SU Hang <darcy.sh@antgroup.com>
---
 arch/x86/include/asm/vmx.h     | 2 ++
 arch/x86/kvm/mmu/paging_tmpl.h | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 0ffaa3156a4e..3586d4aeaac7 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -546,6 +546,7 @@ enum vm_entry_failure_code {
 #define EPT_VIOLATION_READABLE_BIT	3
 #define EPT_VIOLATION_WRITABLE_BIT	4
 #define EPT_VIOLATION_EXECUTABLE_BIT	5
+#define EPT_VIOLATION_GVA_IS_VALID_BIT	7
 #define EPT_VIOLATION_GVA_TRANSLATED_BIT 8
 #define EPT_VIOLATION_ACC_READ		(1 << EPT_VIOLATION_ACC_READ_BIT)
 #define EPT_VIOLATION_ACC_WRITE		(1 << EPT_VIOLATION_ACC_WRITE_BIT)
@@ -553,6 +554,7 @@ enum vm_entry_failure_code {
 #define EPT_VIOLATION_READABLE		(1 << EPT_VIOLATION_READABLE_BIT)
 #define EPT_VIOLATION_WRITABLE		(1 << EPT_VIOLATION_WRITABLE_BIT)
 #define EPT_VIOLATION_EXECUTABLE	(1 << EPT_VIOLATION_EXECUTABLE_BIT)
+#define EPT_VIOLATION_GVA_IS_VALID	(1 << EPT_VIOLATION_GVA_IS_VALID_BIT)
 #define EPT_VIOLATION_GVA_TRANSLATED	(1 << EPT_VIOLATION_GVA_TRANSLATED_BIT)
 
 /*
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 95367f5ca998..db594366f60c 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -523,7 +523,8 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 	 * The other bits are set to 0.
 	 */
 	if (!(errcode & PFERR_RSVD_MASK)) {
-		vcpu->arch.exit_qualification &= 0x180;
+		vcpu->arch.exit_qualification &= (EPT_VIOLATION_GVA_IS_VALID |
+						  EPT_VIOLATION_GVA_TRANSLATED);
 		if (write_fault)
 			vcpu->arch.exit_qualification |= EPT_VIOLATION_ACC_WRITE;
 		if (user_fault)
-- 
2.32.0.3.g01195cf9f

[PATCH v2 2/2] KVM: x86/mmu: Derive EPT violation RWX bits from EPTE RWX bits

[SU Hang]

From: Sean Christopherson <seanjc@google.com>

Derive the mask of RWX bits reported on EPT violations from the mask of
RWX bits that are shoved into EPT entries; the layout is the same, the
EPT violation bits are simply shifted by three.  Use the new shift and a
slight copy-paste of the mask derivation instead of completely open
coding the same to convert between the EPT entry bits and the exit
qualification when synthesizing a nested EPT Violation.

No functional change intended.

Cc: SU Hang <darcy.sh@antgroup.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/vmx.h     | 7 +------
 arch/x86/kvm/mmu/paging_tmpl.h | 8 +++++++-
 arch/x86/kvm/vmx/vmx.c         | 4 +---
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 3586d4aeaac7..46bc7072f6a2 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -543,17 +543,12 @@ enum vm_entry_failure_code {
 #define EPT_VIOLATION_ACC_READ_BIT	0
 #define EPT_VIOLATION_ACC_WRITE_BIT	1
 #define EPT_VIOLATION_ACC_INSTR_BIT	2
-#define EPT_VIOLATION_READABLE_BIT	3
-#define EPT_VIOLATION_WRITABLE_BIT	4
-#define EPT_VIOLATION_EXECUTABLE_BIT	5
 #define EPT_VIOLATION_GVA_IS_VALID_BIT	7
 #define EPT_VIOLATION_GVA_TRANSLATED_BIT 8
 #define EPT_VIOLATION_ACC_READ		(1 << EPT_VIOLATION_ACC_READ_BIT)
 #define EPT_VIOLATION_ACC_WRITE		(1 << EPT_VIOLATION_ACC_WRITE_BIT)
 #define EPT_VIOLATION_ACC_INSTR		(1 << EPT_VIOLATION_ACC_INSTR_BIT)
-#define EPT_VIOLATION_READABLE		(1 << EPT_VIOLATION_READABLE_BIT)
-#define EPT_VIOLATION_WRITABLE		(1 << EPT_VIOLATION_WRITABLE_BIT)
-#define EPT_VIOLATION_EXECUTABLE	(1 << EPT_VIOLATION_EXECUTABLE_BIT)
+#define EPT_VIOLATION_RWX_MASK		(VMX_EPT_RWX_MASK << EPT_VIOLATION_RWX_SHIFT)
 #define EPT_VIOLATION_GVA_IS_VALID	(1 << EPT_VIOLATION_GVA_IS_VALID_BIT)
 #define EPT_VIOLATION_GVA_TRANSLATED	(1 << EPT_VIOLATION_GVA_TRANSLATED_BIT)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index db594366f60c..a4a9d7f2d3bd 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -531,7 +531,13 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 			vcpu->arch.exit_qualification |= EPT_VIOLATION_ACC_READ;
 		if (fetch_fault)
 			vcpu->arch.exit_qualification |= EPT_VIOLATION_ACC_INSTR;
-		vcpu->arch.exit_qualification |= (pte_access & 0x7) << 3;
+
+		/*
+		 * Note, pte_access holds the raw RWX bits from the EPTE, not
+		 * ACC_*_MASK flags!
+		 */
+		vcpu->arch.exit_qualification |= (pte_access & VMX_EPT_RWX_MASK) <<
+						 EPT_VIOLATION_RWX_SHIFT;
 	}
 #endif
 	walker->fault.address = addr;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 5b629acafa69..9c1f6d3dceef 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5410,9 +5410,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
 	error_code |= (exit_qualification & EPT_VIOLATION_ACC_INSTR)
 		      ? PFERR_FETCH_MASK : 0;
 	/* ept page table entry is present? */
-	error_code |= (exit_qualification &
-		       (EPT_VIOLATION_READABLE | EPT_VIOLATION_WRITABLE |
-			EPT_VIOLATION_EXECUTABLE))
+	error_code |= (exit_qualification & EPT_VIOLATION_RWX_MASK)
 		      ? PFERR_PRESENT_MASK : 0;

 	error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) != 0 ?
--
2.32.0.3.g01195cf9f

Re: [PATCH v2 1/2] KVM: VMX: replace 0x180 with EPT_VIOLATION_* definition

[Sean Christopherson]

On Tue, Mar 29, 2022, SU Hang wrote:
> Using self-expressing macro definition EPT_VIOLATION_GVA_VALIDATION

Nit, this still refers to the name used in v1.  With that fixed (Paolo might do
it for you :-) ),

Reviewed-by: Sean Christopherson <seanjc@google.com>

Re: [PATCH v2 0/2] KVM: x86: Replace magic number with readable macro

[Paolo Bonzini]

Queued, thanks.

Paolo

Re: [PATCH v2 2/2] KVM: x86/mmu: Derive EPT violation RWX bits from EPTE RWX bits

[Isaku Yamahata]

On Tue, Mar 29, 2022 at 11:01:07AM +0800,
SU Hang <darcy.sh@antgroup.com> wrote:

> From: Sean Christopherson <seanjc@google.com>
> 
> Derive the mask of RWX bits reported on EPT violations from the mask of
> RWX bits that are shoved into EPT entries; the layout is the same, the
> EPT violation bits are simply shifted by three.  Use the new shift and a
> slight copy-paste of the mask derivation instead of completely open
> coding the same to convert between the EPT entry bits and the exit
> qualification when synthesizing a nested EPT Violation.
> 
> No functional change intended.
> 
> Cc: SU Hang <darcy.sh@antgroup.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/vmx.h     | 7 +------
>  arch/x86/kvm/mmu/paging_tmpl.h | 8 +++++++-
>  arch/x86/kvm/vmx/vmx.c         | 4 +---
>  3 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 3586d4aeaac7..46bc7072f6a2 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -543,17 +543,12 @@ enum vm_entry_failure_code {
>  #define EPT_VIOLATION_ACC_READ_BIT	0
>  #define EPT_VIOLATION_ACC_WRITE_BIT	1
>  #define EPT_VIOLATION_ACC_INSTR_BIT	2
> -#define EPT_VIOLATION_READABLE_BIT	3
> -#define EPT_VIOLATION_WRITABLE_BIT	4
> -#define EPT_VIOLATION_EXECUTABLE_BIT	5
>  #define EPT_VIOLATION_GVA_IS_VALID_BIT	7
>  #define EPT_VIOLATION_GVA_TRANSLATED_BIT 8
>  #define EPT_VIOLATION_ACC_READ		(1 << EPT_VIOLATION_ACC_READ_BIT)
>  #define EPT_VIOLATION_ACC_WRITE		(1 << EPT_VIOLATION_ACC_WRITE_BIT)
>  #define EPT_VIOLATION_ACC_INSTR		(1 << EPT_VIOLATION_ACC_INSTR_BIT)
> -#define EPT_VIOLATION_READABLE		(1 << EPT_VIOLATION_READABLE_BIT)
> -#define EPT_VIOLATION_WRITABLE		(1 << EPT_VIOLATION_WRITABLE_BIT)
> -#define EPT_VIOLATION_EXECUTABLE	(1 << EPT_VIOLATION_EXECUTABLE_BIT)
> +#define EPT_VIOLATION_RWX_MASK		(VMX_EPT_RWX_MASK << EPT_VIOLATION_RWX_SHIFT)


"#define EPT_VIOLATION_RWX_SHIFT 3" is missing.
It fails to compile.

  CC [M]  arch/x86/kvm/vmx/vmx.o
In file included from linux/arch/x86/include/asm/virtext.h:18,
                 from /linux/arch/x86/kvm/vmx/vmx.c:49:
/linux/arch/x86/kvm/vmx/vmx.c: In function 'handle_ept_violation':
/linux/arch/x86/include/asm/vmx.h:551:54: error: 'EPT_VIOLATION_RWX_SHIFT' undeclared (first use in this function); did you mean 'EPT_VIOLATION_RWX_MASK'?
  551 | #define EPT_VIOLATION_RWX_MASK  (VMX_EPT_RWX_MASK << EPT_VIOLATION_RWX_SHIFT)
      |                                                      ^~~~~~~~~~~~~~~~~~~~~~~

Thanks,
-- 
Isaku Yamahata <isaku.yamahata@gmail.com>

Re: [PATCH v2 2/2] KVM: x86/mmu: Derive EPT violation RWX bits from EPTE RWX bits

[Sean Christopherson]

On Fri, Apr 08, 2022, Isaku Yamahata wrote:
> On Tue, Mar 29, 2022 at 11:01:07AM +0800,
> SU Hang <darcy.sh@antgroup.com> wrote:
> 
> > From: Sean Christopherson <seanjc@google.com>
> > 
> > Derive the mask of RWX bits reported on EPT violations from the mask of
> > RWX bits that are shoved into EPT entries; the layout is the same, the
> > EPT violation bits are simply shifted by three.  Use the new shift and a
> > slight copy-paste of the mask derivation instead of completely open
> > coding the same to convert between the EPT entry bits and the exit
> > qualification when synthesizing a nested EPT Violation.
> > 
> > No functional change intended.
> > 
> > Cc: SU Hang <darcy.sh@antgroup.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/include/asm/vmx.h     | 7 +------
> >  arch/x86/kvm/mmu/paging_tmpl.h | 8 +++++++-
> >  arch/x86/kvm/vmx/vmx.c         | 4 +---
> >  3 files changed, 9 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> > index 3586d4aeaac7..46bc7072f6a2 100644
> > --- a/arch/x86/include/asm/vmx.h
> > +++ b/arch/x86/include/asm/vmx.h
> > @@ -543,17 +543,12 @@ enum vm_entry_failure_code {
> >  #define EPT_VIOLATION_ACC_READ_BIT	0
> >  #define EPT_VIOLATION_ACC_WRITE_BIT	1
> >  #define EPT_VIOLATION_ACC_INSTR_BIT	2
> > -#define EPT_VIOLATION_READABLE_BIT	3
> > -#define EPT_VIOLATION_WRITABLE_BIT	4
> > -#define EPT_VIOLATION_EXECUTABLE_BIT	5
> >  #define EPT_VIOLATION_GVA_IS_VALID_BIT	7
> >  #define EPT_VIOLATION_GVA_TRANSLATED_BIT 8
> >  #define EPT_VIOLATION_ACC_READ		(1 << EPT_VIOLATION_ACC_READ_BIT)
> >  #define EPT_VIOLATION_ACC_WRITE		(1 << EPT_VIOLATION_ACC_WRITE_BIT)
> >  #define EPT_VIOLATION_ACC_INSTR		(1 << EPT_VIOLATION_ACC_INSTR_BIT)
> > -#define EPT_VIOLATION_READABLE		(1 << EPT_VIOLATION_READABLE_BIT)
> > -#define EPT_VIOLATION_WRITABLE		(1 << EPT_VIOLATION_WRITABLE_BIT)
> > -#define EPT_VIOLATION_EXECUTABLE	(1 << EPT_VIOLATION_EXECUTABLE_BIT)
> > +#define EPT_VIOLATION_RWX_MASK		(VMX_EPT_RWX_MASK << EPT_VIOLATION_RWX_SHIFT)
> 
> 
> "#define EPT_VIOLATION_RWX_SHIFT 3" is missing.
> It fails to compile.
> 
>   CC [M]  arch/x86/kvm/vmx/vmx.o
> In file included from linux/arch/x86/include/asm/virtext.h:18,
>                  from /linux/arch/x86/kvm/vmx/vmx.c:49:
> /linux/arch/x86/kvm/vmx/vmx.c: In function 'handle_ept_violation':
> /linux/arch/x86/include/asm/vmx.h:551:54: error: 'EPT_VIOLATION_RWX_SHIFT' undeclared (first use in this function); did you mean 'EPT_VIOLATION_RWX_MASK'?
>   551 | #define EPT_VIOLATION_RWX_MASK  (VMX_EPT_RWX_MASK << EPT_VIOLATION_RWX_SHIFT)
>       |                                                      ^~~~~~~~~~~~~~~~~~~~~~~

Yeah, not sure how that one line got dropped.

https://lore.kernel.org/all/20220408202815.386932-1-seanjc@google.com

Re: [PATCH v2 2/2] KVM: x86/mmu: Derive EPT violation RWX bits from EPTE RWX bits

[SU Hang]

I sincerely apologize to you and Palo for making this mistake due to my
negligence and lack of testing. Since the patches only care about macro
substitutions and no function changes, this makes me sloppy. Now I realize my
mistake.

Ashamed Hang