[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

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>

[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 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 0/2] KVM: x86: Replace magic number with readable macro

[Paolo Bonzini]

Queued, thanks.

Paolo