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/[email protected]/
- 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
From: Sean Christopherson <[email protected]>
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 <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Queued, thanks.
Paolo
On Tue, Mar 29, 2022 at 11:01:07AM +0800,
SU Hang <[email protected]> wrote:
> From: Sean Christopherson <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> 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 <[email protected]>
On Fri, Apr 08, 2022, Isaku Yamahata wrote:
> On Tue, Mar 29, 2022 at 11:01:07AM +0800,
> SU Hang <[email protected]> wrote:
>
> > From: Sean Christopherson <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Sean Christopherson <[email protected]>
> > ---
> > 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/[email protected]