[PATCH v3 00/11] SVM cleanup and INVPCID support for the AMD guests

[PATCH v3 00/11] SVM cleanup and INVPCID support for the AMD guests

[Babu Moger]

The following series adds the support for PCID/INVPCID on AMD guests.
While doing it re-structured the vmcb_control_area data structure to
combine all the intercept vectors into one 32 bit array. Makes it easy
for future additions.

INVPCID interceptions are added only when the guest is running with
shadow page table enabled. In this case the hypervisor needs to handle
the tlbflush based on the type of invpcid instruction.

For the guests with nested page table (NPT) support, the INVPCID feature
works as running it natively. KVM does not need to do any special handling.

AMD documentation for INVPCID feature is available at "AMD64 Architecture
Programmer’s Manual Volume 2: System Programming, Pub. 24593 Rev. 3.34(or later)"

The documentation can be obtained at the links below:
Link: https://www.amd.com/system/files/TechDocs/24593.pdf
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
---
v3:
 1. Addressing the comments from Jim Mattson. Follow the v2 link below
    for the context.
 2. Introduced the generic __set_intercept, __clr_intercept and is_intercept
    using native __set_bit, clear_bit and test_bit.
 3. Combined all the intercepts vectors into single 32 bit array.
 4. Removed set_intercept_cr, clr_intercept_cr, set_exception_intercepts,
    clr_exception_intercept etc. Used the generic set_intercept and
    clr_intercept where applicable.
 5. Tested both L1 guest and l2 nested guests. 

v2:
  https://lore.kernel.org/lkml/159234483706.6230.13753828995249423191.stgit@bmoger-ubuntu/
  - Taken care of few comments from Jim Mattson.
  - KVM interceptions added only when tdp is off. No interceptions
    when tdp is on.
  - Reverted the fault priority to original order in VMX. 
  
v1:
  https://lore.kernel.org/lkml/159191202523.31436.11959784252237488867.stgit@bmoger-ubuntu/

Babu Moger (11):
      KVM: SVM: Introduce __set_intercept, __clr_intercept and __is_intercept
      KVM: SVM: Change intercept_cr to generic intercepts
      KVM: SVM: Change intercept_dr to generic intercepts
      KVM: SVM: Modify intercept_exceptions to generic intercepts
      KVM: SVM: Modify 64 bit intercept field to two 32 bit vectors
      KVM: SVM: Add new intercept vector in vmcb_control_area
      KVM: nSVM: Cleanup nested_state data structure
      KVM: SVM: Remove set_cr_intercept, clr_cr_intercept and is_cr_intercept
      KVM: SVM: Remove set_exception_intercept and clr_exception_intercept
      KVM: X86: Move handling of INVPCID types to x86
      KVM:SVM: Enable INVPCID feature on AMD


 arch/x86/include/asm/svm.h      |  117 ++++++++++++++++++++++---------
 arch/x86/include/uapi/asm/svm.h |    2 +
 arch/x86/kvm/svm/nested.c       |   66 ++++++++----------
 arch/x86/kvm/svm/svm.c          |  146 ++++++++++++++++++++++++++++-----------
 arch/x86/kvm/svm/svm.h          |   87 ++++++++---------------
 arch/x86/kvm/trace.h            |   21 ++++--
 arch/x86/kvm/vmx/vmx.c          |   62 -----------------
 arch/x86/kvm/x86.c              |   69 ++++++++++++++++++
 arch/x86/kvm/x86.h              |    3 +
 9 files changed, 335 insertions(+), 238 deletions(-)

--

[PATCH v3 01/11] KVM: SVM: Introduce __set_intercept, __clr_intercept and __is_intercept

[Babu Moger]

This is in preparation for the future intercept vector additions.

Add new functions __set_intercept, __clr_intercept and __is_intercept
using kernel APIs __set_bit, __clear_bit and test_bit espectively.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kvm/svm/svm.h |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 6ac4c00a5d82..3b669718190a 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -217,6 +217,21 @@ static inline struct vmcb *get_host_vmcb(struct vcpu_svm *svm)
 		return svm->vmcb;
 }
 
+static inline void __set_intercept(void *addr, int bit)
+{
+	__set_bit(bit, (unsigned long *)addr);
+}
+
+static inline void __clr_intercept(void *addr, int bit)
+{
+	__clear_bit(bit, (unsigned long *)addr);
+}
+
+static inline bool __is_intercept(void *addr, int bit)
+{
+	return test_bit(bit, (unsigned long *)addr);
+}
+
 static inline void set_cr_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);

Re: [PATCH v3 01/11] KVM: SVM: Introduce __set_intercept, __clr_intercept and __is_intercept

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:37 PM Babu Moger <babu.moger@amd.com> wrote:
>
> This is in preparation for the future intercept vector additions.
>
> Add new functions __set_intercept, __clr_intercept and __is_intercept
> using kernel APIs __set_bit, __clear_bit and test_bit espectively.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Sean will probably complain about introducing unused functions, but...

Reviewed-by: Jim Mattson <jmattson@google.com>

Re: [PATCH v3 01/11] KVM: SVM: Introduce __set_intercept, __clr_intercept and __is_intercept

[Paolo Bonzini]

On 29/07/20 01:37, Babu Moger wrote:
> This is in preparation for the future intercept vector additions.
> 
> Add new functions __set_intercept, __clr_intercept and __is_intercept
> using kernel APIs __set_bit, __clear_bit and test_bit espectively.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kvm/svm/svm.h |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 6ac4c00a5d82..3b669718190a 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -217,6 +217,21 @@ static inline struct vmcb *get_host_vmcb(struct vcpu_svm *svm)
>  		return svm->vmcb;
>  }
>  
> +static inline void __set_intercept(void *addr, int bit)
> +{
> +	__set_bit(bit, (unsigned long *)addr);
> +}
> +
> +static inline void __clr_intercept(void *addr, int bit)
> +{
> +	__clear_bit(bit, (unsigned long *)addr);
> +}
> +
> +static inline bool __is_intercept(void *addr, int bit)
> +{
> +	return test_bit(bit, (unsigned long *)addr);
> +}
> +

Probably worth adding a range check?

Paolo

>  static inline void set_cr_intercept(struct vcpu_svm *svm, int bit)
>  {
>  	struct vmcb *vmcb = get_host_vmcb(svm);
> 

RE: [PATCH v3 01/11] KVM: SVM: Introduce __set_intercept, __clr_intercept and __is_intercept

[Babu Moger]

> -----Original Message-----
> From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf
> Of Paolo Bonzini
> Sent: Wednesday, July 29, 2020 6:09 PM
> To: Moger, Babu <Babu.Moger@amd.com>; vkuznets@redhat.com;
> wanpengli@tencent.com; sean.j.christopherson@intel.com;
> jmattson@google.com
> Cc: kvm@vger.kernel.org; joro@8bytes.org; x86@kernel.org; linux-
> kernel@vger.kernel.org; mingo@redhat.com; bp@alien8.de; hpa@zytor.com;
> tglx@linutronix.de
> Subject: Re: [PATCH v3 01/11] KVM: SVM: Introduce __set_intercept,
> __clr_intercept and __is_intercept
> 
> On 29/07/20 01:37, Babu Moger wrote:
> > This is in preparation for the future intercept vector additions.
> >
> > Add new functions __set_intercept, __clr_intercept and __is_intercept
> > using kernel APIs __set_bit, __clear_bit and test_bit espectively.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/kvm/svm/svm.h |   15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index
> > 6ac4c00a5d82..3b669718190a 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -217,6 +217,21 @@ static inline struct vmcb *get_host_vmcb(struct
> vcpu_svm *svm)
> >  		return svm->vmcb;
> >  }
> >
> > +static inline void __set_intercept(void *addr, int bit) {
> > +	__set_bit(bit, (unsigned long *)addr); }
> > +
> > +static inline void __clr_intercept(void *addr, int bit) {
> > +	__clear_bit(bit, (unsigned long *)addr); }
> > +
> > +static inline bool __is_intercept(void *addr, int bit) {
> > +	return test_bit(bit, (unsigned long *)addr); }
> > +
> 
> Probably worth adding a range check?

Paolo,
Not sure. It is only called with intercept bits like __set_intercept(addr,
INTERCEPT_ xx). All these intercept bits are already accounted for.
Thanks


> 
> >  static inline void set_cr_intercept(struct vcpu_svm *svm, int bit)  {
> >  	struct vmcb *vmcb = get_host_vmcb(svm);
> >

[PATCH v3 02/11] KVM: SVM: Change intercept_cr to generic intercepts

[Babu Moger]

Change intercept_cr to generic intercepts in vmcb_control_area.
Use the new __set_intercept, __clr_intercept and __is_intercept
where applicable.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/svm.h |   42 ++++++++++++++++++++++++++++++++----------
 arch/x86/kvm/svm/nested.c  |   26 +++++++++++++++++---------
 arch/x86/kvm/svm/svm.c     |    4 ++--
 arch/x86/kvm/svm/svm.h     |    6 +++---
 4 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 8a1f5382a4ea..d4739f4eae63 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -4,6 +4,37 @@
 
 #include <uapi/asm/svm.h>
 
+/*
+ * VMCB Control Area intercept bits starting
+ * at Byte offset 000h (Vector 0).
+ */
+
+enum vector_offset {
+	CR_VECTOR = 0,
+	MAX_VECTORS,
+};
+
+enum {
+	/* Byte offset 000h (Vector 0) */
+	INTERCEPT_CR0_READ = 0,
+	INTERCEPT_CR1_READ,
+	INTERCEPT_CR2_READ,
+	INTERCEPT_CR3_READ,
+	INTERCEPT_CR4_READ,
+	INTERCEPT_CR5_READ,
+	INTERCEPT_CR6_READ,
+	INTERCEPT_CR7_READ,
+	INTERCEPT_CR8_READ,
+	INTERCEPT_CR0_WRITE = 16,
+	INTERCEPT_CR1_WRITE,
+	INTERCEPT_CR2_WRITE,
+	INTERCEPT_CR3_WRITE,
+	INTERCEPT_CR4_WRITE,
+	INTERCEPT_CR5_WRITE,
+	INTERCEPT_CR6_WRITE,
+	INTERCEPT_CR7_WRITE,
+	INTERCEPT_CR8_WRITE,
+};
 
 enum {
 	INTERCEPT_INTR,
@@ -57,7 +88,7 @@ enum {
 
 
 struct __attribute__ ((__packed__)) vmcb_control_area {
-	u32 intercept_cr;
+	u32 intercepts[MAX_VECTORS];
 	u32 intercept_dr;
 	u32 intercept_exceptions;
 	u64 intercept;
@@ -240,15 +271,6 @@ struct __attribute__ ((__packed__)) vmcb {
 #define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK
 #define SVM_SELECTOR_CODE_MASK (1 << 3)
 
-#define INTERCEPT_CR0_READ	0
-#define INTERCEPT_CR3_READ	3
-#define INTERCEPT_CR4_READ	4
-#define INTERCEPT_CR8_READ	8
-#define INTERCEPT_CR0_WRITE	(16 + 0)
-#define INTERCEPT_CR3_WRITE	(16 + 3)
-#define INTERCEPT_CR4_WRITE	(16 + 4)
-#define INTERCEPT_CR8_WRITE	(16 + 8)
-
 #define INTERCEPT_DR0_READ	0
 #define INTERCEPT_DR1_READ	1
 #define INTERCEPT_DR2_READ	2
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 6bceafb19108..46f5c82d9b45 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -105,6 +105,7 @@ static void nested_svm_uninit_mmu_context(struct kvm_vcpu *vcpu)
 void recalc_intercepts(struct vcpu_svm *svm)
 {
 	struct vmcb_control_area *c, *h, *g;
+	unsigned int i;
 
 	mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
 
@@ -117,15 +118,17 @@ void recalc_intercepts(struct vcpu_svm *svm)
 
 	svm->nested.host_intercept_exceptions = h->intercept_exceptions;
 
-	c->intercept_cr = h->intercept_cr;
+	for (i = 0; i < MAX_VECTORS; i++)
+		c->intercepts[i] = h->intercepts[i];
+
 	c->intercept_dr = h->intercept_dr;
 	c->intercept_exceptions = h->intercept_exceptions;
 	c->intercept = h->intercept;
 
 	if (g->int_ctl & V_INTR_MASKING_MASK) {
 		/* We only want the cr8 intercept bits of L1 */
-		c->intercept_cr &= ~(1U << INTERCEPT_CR8_READ);
-		c->intercept_cr &= ~(1U << INTERCEPT_CR8_WRITE);
+		__clr_intercept(&c->intercepts, INTERCEPT_CR8_READ);
+		__clr_intercept(&c->intercepts, INTERCEPT_CR8_WRITE);
 
 		/*
 		 * Once running L2 with HF_VINTR_MASK, EFLAGS.IF does not
@@ -138,7 +141,9 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	/* We don't want to see VMMCALLs from a nested guest */
 	c->intercept &= ~(1ULL << INTERCEPT_VMMCALL);
 
-	c->intercept_cr |= g->intercept_cr;
+	for (i = 0; i < MAX_VECTORS; i++)
+		c->intercepts[i] |= g->intercepts[i];
+
 	c->intercept_dr |= g->intercept_dr;
 	c->intercept_exceptions |= g->intercept_exceptions;
 	c->intercept |= g->intercept;
@@ -147,7 +152,11 @@ void recalc_intercepts(struct vcpu_svm *svm)
 static void copy_vmcb_control_area(struct vmcb_control_area *dst,
 				   struct vmcb_control_area *from)
 {
-	dst->intercept_cr         = from->intercept_cr;
+	unsigned int i;
+
+	for (i = 0; i < MAX_VECTORS; i++)
+		dst->intercepts[i] = from->intercepts[i];
+
 	dst->intercept_dr         = from->intercept_dr;
 	dst->intercept_exceptions = from->intercept_exceptions;
 	dst->intercept            = from->intercept;
@@ -430,8 +439,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
 			       nested_vmcb->control.event_inj,
 			       nested_vmcb->control.nested_ctl);
 
-	trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr & 0xffff,
-				    nested_vmcb->control.intercept_cr >> 16,
+	trace_kvm_nested_intercepts(nested_vmcb->control.intercepts[CR_VECTOR] & 0xffff,
+				    nested_vmcb->control.intercepts[CR_VECTOR] >> 16,
 				    nested_vmcb->control.intercept_exceptions,
 				    nested_vmcb->control.intercept);
 
@@ -703,8 +712,7 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
 		vmexit = nested_svm_intercept_ioio(svm);
 		break;
 	case SVM_EXIT_READ_CR0 ... SVM_EXIT_WRITE_CR8: {
-		u32 bit = 1U << (exit_code - SVM_EXIT_READ_CR0);
-		if (svm->nested.ctl.intercept_cr & bit)
+		if (__is_intercept(&svm->nested.ctl.intercepts, exit_code))
 			vmexit = NESTED_EXIT_DONE;
 		break;
 	}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index c0da4dd78ac5..334bda8b31c1 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2797,8 +2797,8 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
 	}
 
 	pr_err("VMCB Control Area:\n");
-	pr_err("%-20s%04x\n", "cr_read:", control->intercept_cr & 0xffff);
-	pr_err("%-20s%04x\n", "cr_write:", control->intercept_cr >> 16);
+	pr_err("%-20s%04x\n", "cr_read:", control->intercepts[CR_VECTOR] & 0xffff);
+	pr_err("%-20s%04x\n", "cr_write:", control->intercepts[CR_VECTOR] >> 16);
 	pr_err("%-20s%04x\n", "dr_read:", control->intercept_dr & 0xffff);
 	pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
 	pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 3b669718190a..89d1d91d5bc6 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -236,7 +236,7 @@ static inline void set_cr_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept_cr |= (1U << bit);
+	__set_intercept(&vmcb->control.intercepts, bit);
 
 	recalc_intercepts(svm);
 }
@@ -245,7 +245,7 @@ static inline void clr_cr_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept_cr &= ~(1U << bit);
+	__clr_intercept(&vmcb->control.intercepts, bit);
 
 	recalc_intercepts(svm);
 }
@@ -254,7 +254,7 @@ static inline bool is_cr_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	return vmcb->control.intercept_cr & (1U << bit);
+	return __is_intercept(&vmcb->control.intercepts, bit);
 }
 
 static inline void set_dr_intercepts(struct vcpu_svm *svm)

Re: [PATCH v3 02/11] KVM: SVM: Change intercept_cr to generic intercepts

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> Change intercept_cr to generic intercepts in vmcb_control_area.
> Use the new __set_intercept, __clr_intercept and __is_intercept
> where applicable.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/include/asm/svm.h |   42 ++++++++++++++++++++++++++++++++----------
>  arch/x86/kvm/svm/nested.c  |   26 +++++++++++++++++---------
>  arch/x86/kvm/svm/svm.c     |    4 ++--
>  arch/x86/kvm/svm/svm.h     |    6 +++---
>  4 files changed, 54 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 8a1f5382a4ea..d4739f4eae63 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -4,6 +4,37 @@
>
>  #include <uapi/asm/svm.h>
>
> +/*
> + * VMCB Control Area intercept bits starting
> + * at Byte offset 000h (Vector 0).
> + */
> +
> +enum vector_offset {
> +       CR_VECTOR = 0,
> +       MAX_VECTORS,
> +};
> +
> +enum {
> +       /* Byte offset 000h (Vector 0) */
> +       INTERCEPT_CR0_READ = 0,
> +       INTERCEPT_CR1_READ,
> +       INTERCEPT_CR2_READ,
> +       INTERCEPT_CR3_READ,
> +       INTERCEPT_CR4_READ,
> +       INTERCEPT_CR5_READ,
> +       INTERCEPT_CR6_READ,
> +       INTERCEPT_CR7_READ,
> +       INTERCEPT_CR8_READ,
> +       INTERCEPT_CR0_WRITE = 16,
> +       INTERCEPT_CR1_WRITE,
> +       INTERCEPT_CR2_WRITE,
> +       INTERCEPT_CR3_WRITE,
> +       INTERCEPT_CR4_WRITE,
> +       INTERCEPT_CR5_WRITE,
> +       INTERCEPT_CR6_WRITE,
> +       INTERCEPT_CR7_WRITE,
> +       INTERCEPT_CR8_WRITE,
> +};
>
>  enum {
>         INTERCEPT_INTR,
> @@ -57,7 +88,7 @@ enum {
>
>
>  struct __attribute__ ((__packed__)) vmcb_control_area {
> -       u32 intercept_cr;
> +       u32 intercepts[MAX_VECTORS];
>         u32 intercept_dr;
>         u32 intercept_exceptions;
>         u64 intercept;
> @@ -240,15 +271,6 @@ struct __attribute__ ((__packed__)) vmcb {
>  #define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK
>  #define SVM_SELECTOR_CODE_MASK (1 << 3)
>
> -#define INTERCEPT_CR0_READ     0
> -#define INTERCEPT_CR3_READ     3
> -#define INTERCEPT_CR4_READ     4
> -#define INTERCEPT_CR8_READ     8
> -#define INTERCEPT_CR0_WRITE    (16 + 0)
> -#define INTERCEPT_CR3_WRITE    (16 + 3)
> -#define INTERCEPT_CR4_WRITE    (16 + 4)
> -#define INTERCEPT_CR8_WRITE    (16 + 8)
> -
>  #define INTERCEPT_DR0_READ     0
>  #define INTERCEPT_DR1_READ     1
>  #define INTERCEPT_DR2_READ     2
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 6bceafb19108..46f5c82d9b45 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -105,6 +105,7 @@ static void nested_svm_uninit_mmu_context(struct kvm_vcpu *vcpu)
>  void recalc_intercepts(struct vcpu_svm *svm)
>  {
>         struct vmcb_control_area *c, *h, *g;
> +       unsigned int i;
>
>         mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
>
> @@ -117,15 +118,17 @@ void recalc_intercepts(struct vcpu_svm *svm)
>
>         svm->nested.host_intercept_exceptions = h->intercept_exceptions;
>
> -       c->intercept_cr = h->intercept_cr;
> +       for (i = 0; i < MAX_VECTORS; i++)
> +               c->intercepts[i] = h->intercepts[i];
> +
>         c->intercept_dr = h->intercept_dr;
>         c->intercept_exceptions = h->intercept_exceptions;
>         c->intercept = h->intercept;
>
>         if (g->int_ctl & V_INTR_MASKING_MASK) {
>                 /* We only want the cr8 intercept bits of L1 */
> -               c->intercept_cr &= ~(1U << INTERCEPT_CR8_READ);
> -               c->intercept_cr &= ~(1U << INTERCEPT_CR8_WRITE);
> +               __clr_intercept(&c->intercepts, INTERCEPT_CR8_READ);
> +               __clr_intercept(&c->intercepts, INTERCEPT_CR8_WRITE);

Why the direct calls to the __clr_intercept worker function? Can't
these be calls to clr_cr_intercept()?
Likewise throughout.

RE: [PATCH v3 02/11] KVM: SVM: Change intercept_cr to generic intercepts

[Babu Moger]

> -----Original Message-----
> From: Jim Mattson <jmattson@google.com>
> Sent: Tuesday, July 28, 2020 6:56 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; Wanpeng Li <wanpengli@tencent.com>; Sean
> Christopherson <sean.j.christopherson@intel.com>; kvm list
> <kvm@vger.kernel.org>; Joerg Roedel <joro@8bytes.org>; the arch/x86
> maintainers <x86@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Ingo
> Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; H . Peter Anvin
> <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v3 02/11] KVM: SVM: Change intercept_cr to generic
> intercepts
> 
> On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
> >
> > Change intercept_cr to generic intercepts in vmcb_control_area.
> > Use the new __set_intercept, __clr_intercept and __is_intercept where
> > applicable.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/include/asm/svm.h |   42 ++++++++++++++++++++++++++++++++----
> ------
> >  arch/x86/kvm/svm/nested.c  |   26 +++++++++++++++++---------
> >  arch/x86/kvm/svm/svm.c     |    4 ++--
> >  arch/x86/kvm/svm/svm.h     |    6 +++---
> >  4 files changed, 54 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> > index 8a1f5382a4ea..d4739f4eae63 100644
> > --- a/arch/x86/include/asm/svm.h
> > +++ b/arch/x86/include/asm/svm.h
> > @@ -4,6 +4,37 @@
> >
> >  #include <uapi/asm/svm.h>
> >
> > +/*
> > + * VMCB Control Area intercept bits starting
> > + * at Byte offset 000h (Vector 0).
> > + */
> > +
> > +enum vector_offset {
> > +       CR_VECTOR = 0,
> > +       MAX_VECTORS,
> > +};
> > +
> > +enum {
> > +       /* Byte offset 000h (Vector 0) */
> > +       INTERCEPT_CR0_READ = 0,
> > +       INTERCEPT_CR1_READ,
> > +       INTERCEPT_CR2_READ,
> > +       INTERCEPT_CR3_READ,
> > +       INTERCEPT_CR4_READ,
> > +       INTERCEPT_CR5_READ,
> > +       INTERCEPT_CR6_READ,
> > +       INTERCEPT_CR7_READ,
> > +       INTERCEPT_CR8_READ,
> > +       INTERCEPT_CR0_WRITE = 16,
> > +       INTERCEPT_CR1_WRITE,
> > +       INTERCEPT_CR2_WRITE,
> > +       INTERCEPT_CR3_WRITE,
> > +       INTERCEPT_CR4_WRITE,
> > +       INTERCEPT_CR5_WRITE,
> > +       INTERCEPT_CR6_WRITE,
> > +       INTERCEPT_CR7_WRITE,
> > +       INTERCEPT_CR8_WRITE,
> > +};
> >
> >  enum {
> >         INTERCEPT_INTR,
> > @@ -57,7 +88,7 @@ enum {
> >
> >
> >  struct __attribute__ ((__packed__)) vmcb_control_area {
> > -       u32 intercept_cr;
> > +       u32 intercepts[MAX_VECTORS];
> >         u32 intercept_dr;
> >         u32 intercept_exceptions;
> >         u64 intercept;
> > @@ -240,15 +271,6 @@ struct __attribute__ ((__packed__)) vmcb {
> > #define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK  #define
> > SVM_SELECTOR_CODE_MASK (1 << 3)
> >
> > -#define INTERCEPT_CR0_READ     0
> > -#define INTERCEPT_CR3_READ     3
> > -#define INTERCEPT_CR4_READ     4
> > -#define INTERCEPT_CR8_READ     8
> > -#define INTERCEPT_CR0_WRITE    (16 + 0)
> > -#define INTERCEPT_CR3_WRITE    (16 + 3)
> > -#define INTERCEPT_CR4_WRITE    (16 + 4)
> > -#define INTERCEPT_CR8_WRITE    (16 + 8)
> > -
> >  #define INTERCEPT_DR0_READ     0
> >  #define INTERCEPT_DR1_READ     1
> >  #define INTERCEPT_DR2_READ     2
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 6bceafb19108..46f5c82d9b45 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -105,6 +105,7 @@ static void nested_svm_uninit_mmu_context(struct
> > kvm_vcpu *vcpu)  void recalc_intercepts(struct vcpu_svm *svm)  {
> >         struct vmcb_control_area *c, *h, *g;
> > +       unsigned int i;
> >
> >         mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
> >
> > @@ -117,15 +118,17 @@ void recalc_intercepts(struct vcpu_svm *svm)
> >
> >         svm->nested.host_intercept_exceptions =
> > h->intercept_exceptions;
> >
> > -       c->intercept_cr = h->intercept_cr;
> > +       for (i = 0; i < MAX_VECTORS; i++)
> > +               c->intercepts[i] = h->intercepts[i];
> > +
> >         c->intercept_dr = h->intercept_dr;
> >         c->intercept_exceptions = h->intercept_exceptions;
> >         c->intercept = h->intercept;
> >
> >         if (g->int_ctl & V_INTR_MASKING_MASK) {
> >                 /* We only want the cr8 intercept bits of L1 */
> > -               c->intercept_cr &= ~(1U << INTERCEPT_CR8_READ);
> > -               c->intercept_cr &= ~(1U << INTERCEPT_CR8_WRITE);
> > +               __clr_intercept(&c->intercepts, INTERCEPT_CR8_READ);
> > +               __clr_intercept(&c->intercepts, INTERCEPT_CR8_WRITE);
> 
> Why the direct calls to the __clr_intercept worker function? Can't these be calls
> to clr_cr_intercept()?
> Likewise throughout.

This code uses the address to clear the bits.  So called __clr_intercept
directly. The call clr_cr_intercept() passes the structure vcpu_svm and
then uses get_host_vmcb to get the address.

Re: [PATCH v3 02/11] KVM: SVM: Change intercept_cr to generic intercepts

[Paolo Bonzini]

On 29/07/20 18:08, Babu Moger wrote:
>>>
>>>         if (g->int_ctl & V_INTR_MASKING_MASK) {
>>>                 /* We only want the cr8 intercept bits of L1 */
>>> -               c->intercept_cr &= ~(1U << INTERCEPT_CR8_READ);
>>> -               c->intercept_cr &= ~(1U << INTERCEPT_CR8_WRITE);
>>> +               __clr_intercept(&c->intercepts, INTERCEPT_CR8_READ);
>>> +               __clr_intercept(&c->intercepts, INTERCEPT_CR8_WRITE);
>> Why the direct calls to the __clr_intercept worker function? Can't these be calls
>> to clr_cr_intercept()?
>> Likewise throughout.
> This code uses the address to clear the bits.  So called __clr_intercept
> directly. The call clr_cr_intercept() passes the structure vcpu_svm and
> then uses get_host_vmcb to get the address.

Yes, this is correct.

Paolo

[PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic intercepts

[Babu Moger]

Modify intercept_dr to generic intercepts in vmcb_control_area.
Use generic __set_intercept, __clr_intercept and __is_intercept
to set/clear/test the intercept_dr bits.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/svm.h |   36 ++++++++++++++++++------------------
 arch/x86/kvm/svm/nested.c  |    6 +-----
 arch/x86/kvm/svm/svm.c     |    4 ++--
 arch/x86/kvm/svm/svm.h     |   34 +++++++++++++++++-----------------
 4 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index d4739f4eae63..ffc89d8e4fcb 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -11,6 +11,7 @@
 
 enum vector_offset {
 	CR_VECTOR = 0,
+	DR_VECTOR,
 	MAX_VECTORS,
 };
 
@@ -34,6 +35,23 @@ enum {
 	INTERCEPT_CR6_WRITE,
 	INTERCEPT_CR7_WRITE,
 	INTERCEPT_CR8_WRITE,
+	/* Byte offset 004h (Vector 1) */
+	INTERCEPT_DR0_READ = 32,
+	INTERCEPT_DR1_READ,
+	INTERCEPT_DR2_READ,
+	INTERCEPT_DR3_READ,
+	INTERCEPT_DR4_READ,
+	INTERCEPT_DR5_READ,
+	INTERCEPT_DR6_READ,
+	INTERCEPT_DR7_READ,
+	INTERCEPT_DR0_WRITE = 48,
+	INTERCEPT_DR1_WRITE,
+	INTERCEPT_DR2_WRITE,
+	INTERCEPT_DR3_WRITE,
+	INTERCEPT_DR4_WRITE,
+	INTERCEPT_DR5_WRITE,
+	INTERCEPT_DR6_WRITE,
+	INTERCEPT_DR7_WRITE,
 };
 
 enum {
@@ -89,7 +107,6 @@ enum {
 
 struct __attribute__ ((__packed__)) vmcb_control_area {
 	u32 intercepts[MAX_VECTORS];
-	u32 intercept_dr;
 	u32 intercept_exceptions;
 	u64 intercept;
 	u8 reserved_1[40];
@@ -271,23 +288,6 @@ struct __attribute__ ((__packed__)) vmcb {
 #define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK
 #define SVM_SELECTOR_CODE_MASK (1 << 3)
 
-#define INTERCEPT_DR0_READ	0
-#define INTERCEPT_DR1_READ	1
-#define INTERCEPT_DR2_READ	2
-#define INTERCEPT_DR3_READ	3
-#define INTERCEPT_DR4_READ	4
-#define INTERCEPT_DR5_READ	5
-#define INTERCEPT_DR6_READ	6
-#define INTERCEPT_DR7_READ	7
-#define INTERCEPT_DR0_WRITE	(16 + 0)
-#define INTERCEPT_DR1_WRITE	(16 + 1)
-#define INTERCEPT_DR2_WRITE	(16 + 2)
-#define INTERCEPT_DR3_WRITE	(16 + 3)
-#define INTERCEPT_DR4_WRITE	(16 + 4)
-#define INTERCEPT_DR5_WRITE	(16 + 5)
-#define INTERCEPT_DR6_WRITE	(16 + 6)
-#define INTERCEPT_DR7_WRITE	(16 + 7)
-
 #define SVM_EVTINJ_VEC_MASK 0xff
 
 #define SVM_EVTINJ_TYPE_SHIFT 8
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 46f5c82d9b45..71ca89afb2a3 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -121,7 +121,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	for (i = 0; i < MAX_VECTORS; i++)
 		c->intercepts[i] = h->intercepts[i];
 
-	c->intercept_dr = h->intercept_dr;
 	c->intercept_exceptions = h->intercept_exceptions;
 	c->intercept = h->intercept;
 
@@ -144,7 +143,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	for (i = 0; i < MAX_VECTORS; i++)
 		c->intercepts[i] |= g->intercepts[i];
 
-	c->intercept_dr |= g->intercept_dr;
 	c->intercept_exceptions |= g->intercept_exceptions;
 	c->intercept |= g->intercept;
 }
@@ -157,7 +155,6 @@ static void copy_vmcb_control_area(struct vmcb_control_area *dst,
 	for (i = 0; i < MAX_VECTORS; i++)
 		dst->intercepts[i] = from->intercepts[i];
 
-	dst->intercept_dr         = from->intercept_dr;
 	dst->intercept_exceptions = from->intercept_exceptions;
 	dst->intercept            = from->intercept;
 	dst->iopm_base_pa         = from->iopm_base_pa;
@@ -717,8 +714,7 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
 		break;
 	}
 	case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
-		u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
-		if (svm->nested.ctl.intercept_dr & bit)
+		if (__is_intercept(&svm->nested.ctl.intercepts, exit_code))
 			vmexit = NESTED_EXIT_DONE;
 		break;
 	}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 334bda8b31c1..6d95025938d8 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2799,8 +2799,8 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
 	pr_err("VMCB Control Area:\n");
 	pr_err("%-20s%04x\n", "cr_read:", control->intercepts[CR_VECTOR] & 0xffff);
 	pr_err("%-20s%04x\n", "cr_write:", control->intercepts[CR_VECTOR] >> 16);
-	pr_err("%-20s%04x\n", "dr_read:", control->intercept_dr & 0xffff);
-	pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
+	pr_err("%-20s%04x\n", "dr_read:", control->intercepts[DR_VECTOR] & 0xffff);
+	pr_err("%-20s%04x\n", "dr_write:", control->intercepts[DR_VECTOR] >> 16);
 	pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
 	pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
 	pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 89d1d91d5bc6..f33a50f92b92 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -261,22 +261,22 @@ static inline void set_dr_intercepts(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept_dr = (1 << INTERCEPT_DR0_READ)
-		| (1 << INTERCEPT_DR1_READ)
-		| (1 << INTERCEPT_DR2_READ)
-		| (1 << INTERCEPT_DR3_READ)
-		| (1 << INTERCEPT_DR4_READ)
-		| (1 << INTERCEPT_DR5_READ)
-		| (1 << INTERCEPT_DR6_READ)
-		| (1 << INTERCEPT_DR7_READ)
-		| (1 << INTERCEPT_DR0_WRITE)
-		| (1 << INTERCEPT_DR1_WRITE)
-		| (1 << INTERCEPT_DR2_WRITE)
-		| (1 << INTERCEPT_DR3_WRITE)
-		| (1 << INTERCEPT_DR4_WRITE)
-		| (1 << INTERCEPT_DR5_WRITE)
-		| (1 << INTERCEPT_DR6_WRITE)
-		| (1 << INTERCEPT_DR7_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR0_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR1_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR2_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR3_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR4_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR5_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR6_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR7_READ);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR0_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR1_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR2_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR3_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR4_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR5_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR6_WRITE);
+	__set_intercept(&vmcb->control.intercepts, INTERCEPT_DR7_WRITE);
 
 	recalc_intercepts(svm);
 }
@@ -285,7 +285,7 @@ static inline void clr_dr_intercepts(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept_dr = 0;
+	vmcb->control.intercepts[DR_VECTOR] = 0;
 
 	recalc_intercepts(svm);
 }

Re: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic intercepts

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> Modify intercept_dr to generic intercepts in vmcb_control_area.
> Use generic __set_intercept, __clr_intercept and __is_intercept
> to set/clear/test the intercept_dr bits.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/include/asm/svm.h |   36 ++++++++++++++++++------------------
>  arch/x86/kvm/svm/nested.c  |    6 +-----
>  arch/x86/kvm/svm/svm.c     |    4 ++--
>  arch/x86/kvm/svm/svm.h     |   34 +++++++++++++++++-----------------
>  4 files changed, 38 insertions(+), 42 deletions(-)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index d4739f4eae63..ffc89d8e4fcb 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -11,6 +11,7 @@
>
>  enum vector_offset {
>         CR_VECTOR = 0,
> +       DR_VECTOR,
>         MAX_VECTORS,
>  };
>
> @@ -34,6 +35,23 @@ enum {
>         INTERCEPT_CR6_WRITE,
>         INTERCEPT_CR7_WRITE,
>         INTERCEPT_CR8_WRITE,
> +       /* Byte offset 004h (Vector 1) */
> +       INTERCEPT_DR0_READ = 32,
> +       INTERCEPT_DR1_READ,
> +       INTERCEPT_DR2_READ,
> +       INTERCEPT_DR3_READ,
> +       INTERCEPT_DR4_READ,
> +       INTERCEPT_DR5_READ,
> +       INTERCEPT_DR6_READ,
> +       INTERCEPT_DR7_READ,
> +       INTERCEPT_DR0_WRITE = 48,
> +       INTERCEPT_DR1_WRITE,
> +       INTERCEPT_DR2_WRITE,
> +       INTERCEPT_DR3_WRITE,
> +       INTERCEPT_DR4_WRITE,
> +       INTERCEPT_DR5_WRITE,
> +       INTERCEPT_DR6_WRITE,
> +       INTERCEPT_DR7_WRITE,
>  };
>
>  enum {
> @@ -89,7 +107,6 @@ enum {
>
>  struct __attribute__ ((__packed__)) vmcb_control_area {
>         u32 intercepts[MAX_VECTORS];
> -       u32 intercept_dr;
>         u32 intercept_exceptions;
>         u64 intercept;
>         u8 reserved_1[40];
> @@ -271,23 +288,6 @@ struct __attribute__ ((__packed__)) vmcb {
>  #define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK
>  #define SVM_SELECTOR_CODE_MASK (1 << 3)
>
> -#define INTERCEPT_DR0_READ     0
> -#define INTERCEPT_DR1_READ     1
> -#define INTERCEPT_DR2_READ     2
> -#define INTERCEPT_DR3_READ     3
> -#define INTERCEPT_DR4_READ     4
> -#define INTERCEPT_DR5_READ     5
> -#define INTERCEPT_DR6_READ     6
> -#define INTERCEPT_DR7_READ     7
> -#define INTERCEPT_DR0_WRITE    (16 + 0)
> -#define INTERCEPT_DR1_WRITE    (16 + 1)
> -#define INTERCEPT_DR2_WRITE    (16 + 2)
> -#define INTERCEPT_DR3_WRITE    (16 + 3)
> -#define INTERCEPT_DR4_WRITE    (16 + 4)
> -#define INTERCEPT_DR5_WRITE    (16 + 5)
> -#define INTERCEPT_DR6_WRITE    (16 + 6)
> -#define INTERCEPT_DR7_WRITE    (16 + 7)
> -
>  #define SVM_EVTINJ_VEC_MASK 0xff
>
>  #define SVM_EVTINJ_TYPE_SHIFT 8
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 46f5c82d9b45..71ca89afb2a3 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -121,7 +121,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
>         for (i = 0; i < MAX_VECTORS; i++)
>                 c->intercepts[i] = h->intercepts[i];
>
> -       c->intercept_dr = h->intercept_dr;
>         c->intercept_exceptions = h->intercept_exceptions;
>         c->intercept = h->intercept;
>
> @@ -144,7 +143,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
>         for (i = 0; i < MAX_VECTORS; i++)
>                 c->intercepts[i] |= g->intercepts[i];
>
> -       c->intercept_dr |= g->intercept_dr;
>         c->intercept_exceptions |= g->intercept_exceptions;
>         c->intercept |= g->intercept;
>  }
> @@ -157,7 +155,6 @@ static void copy_vmcb_control_area(struct vmcb_control_area *dst,
>         for (i = 0; i < MAX_VECTORS; i++)
>                 dst->intercepts[i] = from->intercepts[i];
>
> -       dst->intercept_dr         = from->intercept_dr;
>         dst->intercept_exceptions = from->intercept_exceptions;
>         dst->intercept            = from->intercept;
>         dst->iopm_base_pa         = from->iopm_base_pa;
> @@ -717,8 +714,7 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
>                 break;
>         }
>         case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
> -               u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
> -               if (svm->nested.ctl.intercept_dr & bit)
> +               if (__is_intercept(&svm->nested.ctl.intercepts, exit_code))

Can I assume that all of these __<function> calls will become
<function> calls when the grand unification is done? (Maybe I should
just look ahead.)

RE: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic intercepts

[Babu Moger]

> -----Original Message-----
> From: Jim Mattson <jmattson@google.com>
> Sent: Tuesday, July 28, 2020 6:59 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; Wanpeng Li <wanpengli@tencent.com>; Sean
> Christopherson <sean.j.christopherson@intel.com>; kvm list
> <kvm@vger.kernel.org>; Joerg Roedel <joro@8bytes.org>; the arch/x86
> maintainers <x86@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Ingo
> Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; H . Peter Anvin
> <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic
> intercepts
> 
> On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
> >
> > Modify intercept_dr to generic intercepts in vmcb_control_area.
> > Use generic __set_intercept, __clr_intercept and __is_intercept to
> > set/clear/test the intercept_dr bits.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/include/asm/svm.h |   36 ++++++++++++++++++------------------
> >  arch/x86/kvm/svm/nested.c  |    6 +-----
> >  arch/x86/kvm/svm/svm.c     |    4 ++--
> >  arch/x86/kvm/svm/svm.h     |   34 +++++++++++++++++-----------------
> >  4 files changed, 38 insertions(+), 42 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> > index d4739f4eae63..ffc89d8e4fcb 100644
> > --- a/arch/x86/include/asm/svm.h
> > +++ b/arch/x86/include/asm/svm.h
> > @@ -11,6 +11,7 @@
> >
> >  enum vector_offset {
> >         CR_VECTOR = 0,
> > +       DR_VECTOR,
> >         MAX_VECTORS,
> >  };
> >
> > @@ -34,6 +35,23 @@ enum {
> >         INTERCEPT_CR6_WRITE,
> >         INTERCEPT_CR7_WRITE,
> >         INTERCEPT_CR8_WRITE,
> > +       /* Byte offset 004h (Vector 1) */
> > +       INTERCEPT_DR0_READ = 32,
> > +       INTERCEPT_DR1_READ,
> > +       INTERCEPT_DR2_READ,
> > +       INTERCEPT_DR3_READ,
> > +       INTERCEPT_DR4_READ,
> > +       INTERCEPT_DR5_READ,
> > +       INTERCEPT_DR6_READ,
> > +       INTERCEPT_DR7_READ,
> > +       INTERCEPT_DR0_WRITE = 48,
> > +       INTERCEPT_DR1_WRITE,
> > +       INTERCEPT_DR2_WRITE,
> > +       INTERCEPT_DR3_WRITE,
> > +       INTERCEPT_DR4_WRITE,
> > +       INTERCEPT_DR5_WRITE,
> > +       INTERCEPT_DR6_WRITE,
> > +       INTERCEPT_DR7_WRITE,
> >  };
> >
> >  enum {
> > @@ -89,7 +107,6 @@ enum {
> >
> >  struct __attribute__ ((__packed__)) vmcb_control_area {
> >         u32 intercepts[MAX_VECTORS];
> > -       u32 intercept_dr;
> >         u32 intercept_exceptions;
> >         u64 intercept;
> >         u8 reserved_1[40];
> > @@ -271,23 +288,6 @@ struct __attribute__ ((__packed__)) vmcb {
> > #define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK  #define
> > SVM_SELECTOR_CODE_MASK (1 << 3)
> >
> > -#define INTERCEPT_DR0_READ     0
> > -#define INTERCEPT_DR1_READ     1
> > -#define INTERCEPT_DR2_READ     2
> > -#define INTERCEPT_DR3_READ     3
> > -#define INTERCEPT_DR4_READ     4
> > -#define INTERCEPT_DR5_READ     5
> > -#define INTERCEPT_DR6_READ     6
> > -#define INTERCEPT_DR7_READ     7
> > -#define INTERCEPT_DR0_WRITE    (16 + 0)
> > -#define INTERCEPT_DR1_WRITE    (16 + 1)
> > -#define INTERCEPT_DR2_WRITE    (16 + 2)
> > -#define INTERCEPT_DR3_WRITE    (16 + 3)
> > -#define INTERCEPT_DR4_WRITE    (16 + 4)
> > -#define INTERCEPT_DR5_WRITE    (16 + 5)
> > -#define INTERCEPT_DR6_WRITE    (16 + 6)
> > -#define INTERCEPT_DR7_WRITE    (16 + 7)
> > -
> >  #define SVM_EVTINJ_VEC_MASK 0xff
> >
> >  #define SVM_EVTINJ_TYPE_SHIFT 8
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 46f5c82d9b45..71ca89afb2a3 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -121,7 +121,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
> >         for (i = 0; i < MAX_VECTORS; i++)
> >                 c->intercepts[i] = h->intercepts[i];
> >
> > -       c->intercept_dr = h->intercept_dr;
> >         c->intercept_exceptions = h->intercept_exceptions;
> >         c->intercept = h->intercept;
> >
> > @@ -144,7 +143,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
> >         for (i = 0; i < MAX_VECTORS; i++)
> >                 c->intercepts[i] |= g->intercepts[i];
> >
> > -       c->intercept_dr |= g->intercept_dr;
> >         c->intercept_exceptions |= g->intercept_exceptions;
> >         c->intercept |= g->intercept;
> >  }
> > @@ -157,7 +155,6 @@ static void copy_vmcb_control_area(struct
> vmcb_control_area *dst,
> >         for (i = 0; i < MAX_VECTORS; i++)
> >                 dst->intercepts[i] = from->intercepts[i];
> >
> > -       dst->intercept_dr         = from->intercept_dr;
> >         dst->intercept_exceptions = from->intercept_exceptions;
> >         dst->intercept            = from->intercept;
> >         dst->iopm_base_pa         = from->iopm_base_pa;
> > @@ -717,8 +714,7 @@ static int nested_svm_intercept(struct vcpu_svm
> *svm)
> >                 break;
> >         }
> >         case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
> > -               u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
> > -               if (svm->nested.ctl.intercept_dr & bit)
> > +               if (__is_intercept(&svm->nested.ctl.intercepts,
> > + exit_code))
> 
> Can I assume that all of these __<function> calls will become <function> calls
> when the grand unification is done? (Maybe I should just look ahead.)

There are two types of calls here.

1. Calls like set_cr_intercept, clr_cr_intercept, set_dr_intercept,
clr_dr_intercept, set_exception_intercept, clr_exception_intercept.
These calls pass svm data structure. I replaced these calls with either
set_intercept or clr_intercept because we have combined all the intercept
vectors into one 32 bit array.

2. Some calls sets or clears the bit directly like
  c->intercept_cr &= ~(1U << INTERCEPT_CR8_READ);
  Replaced these with __clr_intercept or __set_intercepts.

There is a scope to make all these calls set_intercept or clr_intercept.
These calls use another call get_host_vmcb to get the address. We can take
that up as next cleanup.

Re: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic intercepts

[Paolo Bonzini]

On 29/07/20 01:59, Jim Mattson wrote:
>>         case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
>> -               u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
>> -               if (svm->nested.ctl.intercept_dr & bit)
>> +               if (__is_intercept(&svm->nested.ctl.intercepts, exit_code))
> Can I assume that all of these __<function> calls will become
> <function> calls when the grand unification is done? (Maybe I should
> just look ahead.)
> 

The <function> calls are reserved for the active VMCB while these take a
vector.  Probably it would be nicer to call them
vmcb_{set,clr,is}_intercept and make them take a struct
vmcb_control_area*, but apart from that the concept is fine

Once we do the vmcb01/vmcb02/vmcb12 work, there will not be anymore
&svm->nested.ctl (replaced by &svm->nested.vmcb12->ctl) and we will be
able to change them to take a struct vmcb*.  Then is_intercept would for
example be simply:

	return vmcb_is_intercept(svm->vmcb, nr);

as expected.

Paolo

RE: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic intercepts

[Babu Moger]

> -----Original Message-----
> From: Paolo Bonzini <pbonzini@redhat.com>
> Sent: Wednesday, July 29, 2020 6:12 PM
> To: Jim Mattson <jmattson@google.com>; Moger, Babu
> <Babu.Moger@amd.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>; Wanpeng Li
> <wanpengli@tencent.com>; Sean Christopherson
> <sean.j.christopherson@intel.com>; kvm list <kvm@vger.kernel.org>; Joerg
> Roedel <joro@8bytes.org>; the arch/x86 maintainers <x86@kernel.org>; LKML
> <linux-kernel@vger.kernel.org>; Ingo Molnar <mingo@redhat.com>; Borislav
> Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Thomas Gleixner
> <tglx@linutronix.de>
> Subject: Re: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic
> intercepts
> 
> On 29/07/20 01:59, Jim Mattson wrote:
> >>         case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
> >> -               u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
> >> -               if (svm->nested.ctl.intercept_dr & bit)
> >> +               if (__is_intercept(&svm->nested.ctl.intercepts,
> >> + exit_code))
> > Can I assume that all of these __<function> calls will become
> > <function> calls when the grand unification is done? (Maybe I should
> > just look ahead.)
> >
> 
> The <function> calls are reserved for the active VMCB while these take a vector.
> Probably it would be nicer to call them vmcb_{set,clr,is}_intercept and make
> them take a struct vmcb_control_area*, but apart from that the concept is fine
> 
> Once we do the vmcb01/vmcb02/vmcb12 work, there will not be anymore
> &svm->nested.ctl (replaced by &svm->nested.vmcb12->ctl) and we will be able
> to change them to take a struct vmcb*.  Then is_intercept would for example be
> simply:
Yea. True. It makes the code even cleaner. Also we can avoid calling
recalc_intercepts every time we set or clear a bit inside the same
function(like init_vmcb).

Let me try to understand.

vmcb01 is &svm->vmcb->control;l
vmcb02 is &svm->nested.hsave->control
vmcb12 is  &svm->nested.ctl;

The functions set_intercept and clr_intercept calls get_host_vmcb to get
the vmcb address.

static inline struct vmcb *get_host_vmcb(struct vcpu_svm *svm)
{
        if (is_guest_mode(&svm->vcpu))
                return svm->nested.hsave;
        else
                return svm->vmcb;
}

I need to study little bit when is_guest_mode Is on or off.  Let me take a
look at.

Thanks

> 
> 	return vmcb_is_intercept(svm->vmcb, nr);
> 
> as expected.
> 
> Paolo

RE: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic intercepts

[Babu Moger]

> -----Original Message-----
> From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf
> Of Babu Moger
> Sent: Thursday, July 30, 2020 11:38 AM
> To: Paolo Bonzini <pbonzini@redhat.com>; Jim Mattson
> <jmattson@google.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>; Wanpeng Li
> <wanpengli@tencent.com>; Sean Christopherson
> <sean.j.christopherson@intel.com>; kvm list <kvm@vger.kernel.org>; Joerg
> Roedel <joro@8bytes.org>; the arch/x86 maintainers <x86@kernel.org>; LKML
> <linux-kernel@vger.kernel.org>; Ingo Molnar <mingo@redhat.com>; Borislav
> Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Thomas Gleixner
> <tglx@linutronix.de>
> Subject: RE: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic
> intercepts
> 
> 
> 
> > -----Original Message-----
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Sent: Wednesday, July 29, 2020 6:12 PM
> > To: Jim Mattson <jmattson@google.com>; Moger, Babu
> > <Babu.Moger@amd.com>
> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com>; Wanpeng Li
> > <wanpengli@tencent.com>; Sean Christopherson
> > <sean.j.christopherson@intel.com>; kvm list <kvm@vger.kernel.org>;
> > Joerg Roedel <joro@8bytes.org>; the arch/x86 maintainers
> > <x86@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Ingo Molnar
> > <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; H . Peter Anvin
> > <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> > Subject: Re: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic
> > intercepts
> >
> > On 29/07/20 01:59, Jim Mattson wrote:
> > >>         case SVM_EXIT_READ_DR0 ... SVM_EXIT_WRITE_DR7: {
> > >> -               u32 bit = 1U << (exit_code - SVM_EXIT_READ_DR0);
> > >> -               if (svm->nested.ctl.intercept_dr & bit)
> > >> +               if (__is_intercept(&svm->nested.ctl.intercepts,
> > >> + exit_code))
> > > Can I assume that all of these __<function> calls will become
> > > <function> calls when the grand unification is done? (Maybe I should
> > > just look ahead.)
> > >
> >
> > The <function> calls are reserved for the active VMCB while these take a
> vector.
> > Probably it would be nicer to call them vmcb_{set,clr,is}_intercept
> > and make them take a struct vmcb_control_area*, but apart from that
> > the concept is fine
> >
> > Once we do the vmcb01/vmcb02/vmcb12 work, there will not be anymore
> > &svm->nested.ctl (replaced by &svm->nested.vmcb12->ctl) and we will be
> > able to change them to take a struct vmcb*.  Then is_intercept would
> > for example be
> > simply:
> Yea. True. It makes the code even cleaner. Also we can avoid calling
> recalc_intercepts every time we set or clear a bit inside the same function(like
> init_vmcb).
> 
> Let me try to understand.
> 
> vmcb01 is &svm->vmcb->control;l
> vmcb02 is &svm->nested.hsave->control
> vmcb12 is  &svm->nested.ctl;
> 
> The functions set_intercept and clr_intercept calls get_host_vmcb to get the
> vmcb address.

I will move the get_host_vmcb inside the caller and then call
vmcb_set_intercept/vmcb_clr_intercept/vmcb_is_intercept directly.
I will re post the series. This will change the whole series a little bit.

Jim has already reviewed some of the patches. But I probably cannot use
"Reviewed-by" if I change the patches too much. thanks

> 
> static inline struct vmcb *get_host_vmcb(struct vcpu_svm *svm) {
>         if (is_guest_mode(&svm->vcpu))
>                 return svm->nested.hsave;
>         else
>                 return svm->vmcb;
> }
> 
> I need to study little bit when is_guest_mode Is on or off.  Let me take a look at.


> 
> Thanks
> 
> >
> > 	return vmcb_is_intercept(svm->vmcb, nr);
> >
> > as expected.
> >
> > Paolo

Re: [PATCH v3 03/11] KVM: SVM: Change intercept_dr to generic intercepts

[Paolo Bonzini]

On 31/07/20 00:41, Babu Moger wrote:
>> Let me try to understand.
>>
>> vmcb01 is &svm->vmcb->control;l
>> vmcb02 is &svm->nested.hsave->control
>> vmcb12 is  &svm->nested.ctl;

Right now we don't have a separate vmcb01/vmcb02, we have the current
and hsave VMCBs.  Cathy is working on it.

Just do the refactoring by passing the control area to
vmcb_set_intercept/vmcb_clr_intercept/vmcb_is_intercept.

>> The functions set_intercept and clr_intercept calls get_host_vmcb to get the
>> vmcb address.
>
> I will move the get_host_vmcb inside the caller and then call
> vmcb_set_intercept/vmcb_clr_intercept/vmcb_is_intercept directly.

Hmm no I think set_intercept and clr_intercept should remain as is.

Paolo

> I will re post the series. This will change the whole series a little bit.

[PATCH v3 04/11] KVM: SVM: Modify intercept_exceptions to generic intercepts

[Babu Moger]

Modify intercept_exceptions to generic intercepts in vmcb_control_area.
Use the generic __set_intercept, __clr_intercept and __is_intercept to
set the intercept_exceptions bits.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/svm.h |   22 +++++++++++++++++++++-
 arch/x86/kvm/svm/nested.c  |   12 +++++-------
 arch/x86/kvm/svm/svm.c     |   22 +++++++++++-----------
 arch/x86/kvm/svm/svm.h     |    4 ++--
 4 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index ffc89d8e4fcb..751a6deb64ef 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -3,6 +3,7 @@
 #define __SVM_H
 
 #include <uapi/asm/svm.h>
+#include <uapi/asm/kvm.h>
 
 /*
  * VMCB Control Area intercept bits starting
@@ -12,6 +13,7 @@
 enum vector_offset {
 	CR_VECTOR = 0,
 	DR_VECTOR,
+	EXCEPTION_VECTOR,
 	MAX_VECTORS,
 };
 
@@ -52,6 +54,25 @@ enum {
 	INTERCEPT_DR5_WRITE,
 	INTERCEPT_DR6_WRITE,
 	INTERCEPT_DR7_WRITE,
+	/* Byte offset 008h (Vector 2) */
+	INTERCEPT_DE_VECTOR = 64 + DE_VECTOR,
+	INTERCEPT_DB_VECTOR,
+	INTERCEPT_BP_VECTOR = 64 + BP_VECTOR,
+	INTERCEPT_OF_VECTOR,
+	INTERCEPT_BR_VECTOR,
+	INTERCEPT_UD_VECTOR,
+	INTERCEPT_NM_VECTOR,
+	INTERCEPT_DF_VECTOR,
+	INTERCEPT_TS_VECTOR = 64 + TS_VECTOR,
+	INTERCEPT_NP_VECTOR,
+	INTERCEPT_SS_VECTOR,
+	INTERCEPT_GP_VECTOR,
+	INTERCEPT_PF_VECTOR,
+	INTERCEPT_MF_VECTOR = 64 + MF_VECTOR,
+	INTERCEPT_AC_VECTOR,
+	INTERCEPT_MC_VECTOR,
+	INTERCEPT_XM_VECTOR,
+	INTERCEPT_VE_VECTOR,
 };
 
 enum {
@@ -107,7 +128,6 @@ enum {
 
 struct __attribute__ ((__packed__)) vmcb_control_area {
 	u32 intercepts[MAX_VECTORS];
-	u32 intercept_exceptions;
 	u64 intercept;
 	u8 reserved_1[40];
 	u16 pause_filter_thresh;
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 71ca89afb2a3..ee126d5d3348 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -116,12 +116,11 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	h = &svm->nested.hsave->control;
 	g = &svm->nested.ctl;
 
-	svm->nested.host_intercept_exceptions = h->intercept_exceptions;
+	svm->nested.host_intercept_exceptions = h->intercepts[EXCEPTION_VECTOR];
 
 	for (i = 0; i < MAX_VECTORS; i++)
 		c->intercepts[i] = h->intercepts[i];
 
-	c->intercept_exceptions = h->intercept_exceptions;
 	c->intercept = h->intercept;
 
 	if (g->int_ctl & V_INTR_MASKING_MASK) {
@@ -143,7 +142,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	for (i = 0; i < MAX_VECTORS; i++)
 		c->intercepts[i] |= g->intercepts[i];
 
-	c->intercept_exceptions |= g->intercept_exceptions;
 	c->intercept |= g->intercept;
 }
 
@@ -155,7 +153,6 @@ static void copy_vmcb_control_area(struct vmcb_control_area *dst,
 	for (i = 0; i < MAX_VECTORS; i++)
 		dst->intercepts[i] = from->intercepts[i];
 
-	dst->intercept_exceptions = from->intercept_exceptions;
 	dst->intercept            = from->intercept;
 	dst->iopm_base_pa         = from->iopm_base_pa;
 	dst->msrpm_base_pa        = from->msrpm_base_pa;
@@ -438,7 +435,7 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
 
 	trace_kvm_nested_intercepts(nested_vmcb->control.intercepts[CR_VECTOR] & 0xffff,
 				    nested_vmcb->control.intercepts[CR_VECTOR] >> 16,
-				    nested_vmcb->control.intercept_exceptions,
+				    nested_vmcb->control.intercepts[EXCEPTION_VECTOR],
 				    nested_vmcb->control.intercept);
 
 	/* Clear internal status */
@@ -773,7 +770,7 @@ static bool nested_exit_on_exception(struct vcpu_svm *svm)
 {
 	unsigned int nr = svm->vcpu.arch.exception.nr;
 
-	return (svm->nested.ctl.intercept_exceptions & (1 << nr));
+	return (svm->nested.ctl.intercepts[EXCEPTION_VECTOR] & (1 << nr));
 }
 
 static void nested_svm_inject_exception_vmexit(struct vcpu_svm *svm)
@@ -922,7 +919,8 @@ int nested_svm_exit_special(struct vcpu_svm *svm)
 	case SVM_EXIT_EXCP_BASE ... SVM_EXIT_EXCP_BASE + 0x1f: {
 		u32 excp_bits = 1 << (exit_code - SVM_EXIT_EXCP_BASE);
 
-		if (get_host_vmcb(svm)->control.intercept_exceptions & excp_bits)
+		if (get_host_vmcb(svm)->control.intercepts[EXCEPTION_VECTOR] &
+				excp_bits)
 			return NESTED_EXIT_HOST;
 		else if (exit_code == SVM_EXIT_EXCP_BASE + PF_VECTOR &&
 			 svm->vcpu.arch.apf.host_apf_flags)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 6d95025938d8..d4ac2c5bb365 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -988,11 +988,11 @@ static void init_vmcb(struct vcpu_svm *svm)
 
 	set_dr_intercepts(svm);
 
-	set_exception_intercept(svm, PF_VECTOR);
-	set_exception_intercept(svm, UD_VECTOR);
-	set_exception_intercept(svm, MC_VECTOR);
-	set_exception_intercept(svm, AC_VECTOR);
-	set_exception_intercept(svm, DB_VECTOR);
+	set_exception_intercept(svm, INTERCEPT_PF_VECTOR);
+	set_exception_intercept(svm, INTERCEPT_UD_VECTOR);
+	set_exception_intercept(svm, INTERCEPT_MC_VECTOR);
+	set_exception_intercept(svm, INTERCEPT_AC_VECTOR);
+	set_exception_intercept(svm, INTERCEPT_DB_VECTOR);
 	/*
 	 * Guest access to VMware backdoor ports could legitimately
 	 * trigger #GP because of TSS I/O permission bitmap.
@@ -1000,7 +1000,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 	 * as VMware does.
 	 */
 	if (enable_vmware_backdoor)
-		set_exception_intercept(svm, GP_VECTOR);
+		set_exception_intercept(svm, INTERCEPT_GP_VECTOR);
 
 	set_intercept(svm, INTERCEPT_INTR);
 	set_intercept(svm, INTERCEPT_NMI);
@@ -1078,7 +1078,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 		/* Setup VMCB for Nested Paging */
 		control->nested_ctl |= SVM_NESTED_CTL_NP_ENABLE;
 		clr_intercept(svm, INTERCEPT_INVLPG);
-		clr_exception_intercept(svm, PF_VECTOR);
+		clr_exception_intercept(svm, INTERCEPT_PF_VECTOR);
 		clr_cr_intercept(svm, INTERCEPT_CR3_READ);
 		clr_cr_intercept(svm, INTERCEPT_CR3_WRITE);
 		save->g_pat = svm->vcpu.arch.pat;
@@ -1120,7 +1120,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 
 	if (sev_guest(svm->vcpu.kvm)) {
 		svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ENABLE;
-		clr_exception_intercept(svm, UD_VECTOR);
+		clr_exception_intercept(svm, INTERCEPT_UD_VECTOR);
 	}
 
 	mark_all_dirty(svm->vmcb);
@@ -1631,11 +1631,11 @@ static void update_bp_intercept(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	clr_exception_intercept(svm, BP_VECTOR);
+	clr_exception_intercept(svm, INTERCEPT_BP_VECTOR);
 
 	if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) {
 		if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
-			set_exception_intercept(svm, BP_VECTOR);
+			set_exception_intercept(svm, INTERCEPT_BP_VECTOR);
 	} else
 		vcpu->guest_debug = 0;
 }
@@ -2801,7 +2801,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
 	pr_err("%-20s%04x\n", "cr_write:", control->intercepts[CR_VECTOR] >> 16);
 	pr_err("%-20s%04x\n", "dr_read:", control->intercepts[DR_VECTOR] & 0xffff);
 	pr_err("%-20s%04x\n", "dr_write:", control->intercepts[DR_VECTOR] >> 16);
-	pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
+	pr_err("%-20s%08x\n", "exceptions:", control->intercepts[EXCEPTION_VECTOR]);
 	pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
 	pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
 	pr_err("%-20s%d\n", "pause filter threshold:",
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index f33a50f92b92..9c798781172d 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -294,7 +294,7 @@ static inline void set_exception_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept_exceptions |= (1U << bit);
+	__set_intercept(&vmcb->control.intercepts, bit);
 
 	recalc_intercepts(svm);
 }
@@ -303,7 +303,7 @@ static inline void clr_exception_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept_exceptions &= ~(1U << bit);
+	__clr_intercept(&vmcb->control.intercepts, bit);
 
 	recalc_intercepts(svm);
 }

Re: [PATCH v3 04/11] KVM: SVM: Modify intercept_exceptions to generic intercepts

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> Modify intercept_exceptions to generic intercepts in vmcb_control_area.
> Use the generic __set_intercept, __clr_intercept and __is_intercept to
> set the intercept_exceptions bits.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

> @@ -52,6 +54,25 @@ enum {
>         INTERCEPT_DR5_WRITE,
>         INTERCEPT_DR6_WRITE,
>         INTERCEPT_DR7_WRITE,
> +       /* Byte offset 008h (Vector 2) */
> +       INTERCEPT_DE_VECTOR = 64 + DE_VECTOR,
> +       INTERCEPT_DB_VECTOR,
> +       INTERCEPT_BP_VECTOR = 64 + BP_VECTOR,
> +       INTERCEPT_OF_VECTOR,
> +       INTERCEPT_BR_VECTOR,
> +       INTERCEPT_UD_VECTOR,
> +       INTERCEPT_NM_VECTOR,
> +       INTERCEPT_DF_VECTOR,
> +       INTERCEPT_TS_VECTOR = 64 + TS_VECTOR,
> +       INTERCEPT_NP_VECTOR,
> +       INTERCEPT_SS_VECTOR,
> +       INTERCEPT_GP_VECTOR,
> +       INTERCEPT_PF_VECTOR,
> +       INTERCEPT_MF_VECTOR = 64 + MF_VECTOR,
> +       INTERCEPT_AC_VECTOR,
> +       INTERCEPT_MC_VECTOR,
> +       INTERCEPT_XM_VECTOR,
> +       INTERCEPT_VE_VECTOR,
>  };

I think it's demanding a lot of the reader to know where there are and
are not gaps in the allocated hardware exception vectors. Perhaps all
of the above enumeration definitions could have initializers? Either
way...

Reviewed-by: Jim Mattson <jmattson@google.com>

RE: [PATCH v3 04/11] KVM: SVM: Modify intercept_exceptions to generic intercepts

[Babu Moger]

> -----Original Message-----
> From: Jim Mattson <jmattson@google.com>
> Sent: Wednesday, July 29, 2020 3:48 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; Wanpeng Li <wanpengli@tencent.com>; Sean
> Christopherson <sean.j.christopherson@intel.com>; kvm list
> <kvm@vger.kernel.org>; Joerg Roedel <joro@8bytes.org>; the arch/x86
> maintainers <x86@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Ingo
> Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; H . Peter Anvin
> <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v3 04/11] KVM: SVM: Modify intercept_exceptions to
> generic intercepts
> 
> On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
> >
> > Modify intercept_exceptions to generic intercepts in vmcb_control_area.
> > Use the generic __set_intercept, __clr_intercept and __is_intercept to
> > set the intercept_exceptions bits.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> 
> > @@ -52,6 +54,25 @@ enum {
> >         INTERCEPT_DR5_WRITE,
> >         INTERCEPT_DR6_WRITE,
> >         INTERCEPT_DR7_WRITE,
> > +       /* Byte offset 008h (Vector 2) */
> > +       INTERCEPT_DE_VECTOR = 64 + DE_VECTOR,
> > +       INTERCEPT_DB_VECTOR,
> > +       INTERCEPT_BP_VECTOR = 64 + BP_VECTOR,
> > +       INTERCEPT_OF_VECTOR,
> > +       INTERCEPT_BR_VECTOR,
> > +       INTERCEPT_UD_VECTOR,
> > +       INTERCEPT_NM_VECTOR,
> > +       INTERCEPT_DF_VECTOR,
> > +       INTERCEPT_TS_VECTOR = 64 + TS_VECTOR,
> > +       INTERCEPT_NP_VECTOR,
> > +       INTERCEPT_SS_VECTOR,
> > +       INTERCEPT_GP_VECTOR,
> > +       INTERCEPT_PF_VECTOR,
> > +       INTERCEPT_MF_VECTOR = 64 + MF_VECTOR,
> > +       INTERCEPT_AC_VECTOR,
> > +       INTERCEPT_MC_VECTOR,
> > +       INTERCEPT_XM_VECTOR,
> > +       INTERCEPT_VE_VECTOR,
> >  };
> 
> I think it's demanding a lot of the reader to know where there are and are not
> gaps in the allocated hardware exception vectors. Perhaps all of the above
> enumeration definitions could have initializers? Either way...

Sure. Will add all the initializers here. Thanks

> 
> Reviewed-by: Jim Mattson <jmattson@google.com>

[PATCH v3 05/11] KVM: SVM: Modify 64 bit intercept field to two 32 bit vectors

[Babu Moger]

Convert all the intercepts to one array of 32 bit vectors in
vmcb_control_area. This makes it easy for future intercept vector
additions.  Also update trace functions.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/svm.h |   14 +++++++-------
 arch/x86/kvm/svm/nested.c  |   25 ++++++++++---------------
 arch/x86/kvm/svm/svm.c     |   18 ++++++++----------
 arch/x86/kvm/svm/svm.h     |   12 ++++++------
 arch/x86/kvm/trace.h       |   18 +++++++++++-------
 5 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 751a6deb64ef..aa9f1d62db29 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -14,6 +14,8 @@ enum vector_offset {
 	CR_VECTOR = 0,
 	DR_VECTOR,
 	EXCEPTION_VECTOR,
+	INTERCEPT_VECTOR_3,
+	INTERCEPT_VECTOR_4,
 	MAX_VECTORS,
 };
 
@@ -73,10 +75,8 @@ enum {
 	INTERCEPT_MC_VECTOR,
 	INTERCEPT_XM_VECTOR,
 	INTERCEPT_VE_VECTOR,
-};
-
-enum {
-	INTERCEPT_INTR,
+	/* Byte offset 00Ch (Vector 3) */
+	INTERCEPT_INTR = 96,
 	INTERCEPT_NMI,
 	INTERCEPT_SMI,
 	INTERCEPT_INIT,
@@ -108,7 +108,8 @@ enum {
 	INTERCEPT_TASK_SWITCH,
 	INTERCEPT_FERR_FREEZE,
 	INTERCEPT_SHUTDOWN,
-	INTERCEPT_VMRUN,
+	/* Byte offset 010h (Vector 4) */
+	INTERCEPT_VMRUN = 128,
 	INTERCEPT_VMMCALL,
 	INTERCEPT_VMLOAD,
 	INTERCEPT_VMSAVE,
@@ -128,8 +129,7 @@ enum {
 
 struct __attribute__ ((__packed__)) vmcb_control_area {
 	u32 intercepts[MAX_VECTORS];
-	u64 intercept;
-	u8 reserved_1[40];
+	u8 reserved_1[60 - (MAX_VECTORS * 4)];
 	u16 pause_filter_thresh;
 	u16 pause_filter_count;
 	u64 iopm_base_pa;
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index ee126d5d3348..d2552de42fb1 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -121,8 +121,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	for (i = 0; i < MAX_VECTORS; i++)
 		c->intercepts[i] = h->intercepts[i];
 
-	c->intercept = h->intercept;
-
 	if (g->int_ctl & V_INTR_MASKING_MASK) {
 		/* We only want the cr8 intercept bits of L1 */
 		__clr_intercept(&c->intercepts, INTERCEPT_CR8_READ);
@@ -133,16 +131,14 @@ void recalc_intercepts(struct vcpu_svm *svm)
 		 * affect any interrupt we may want to inject; therefore,
 		 * interrupt window vmexits are irrelevant to L0.
 		 */
-		c->intercept &= ~(1ULL << INTERCEPT_VINTR);
+		__clr_intercept(&c->intercepts, INTERCEPT_VINTR);
 	}
 
 	/* We don't want to see VMMCALLs from a nested guest */
-	c->intercept &= ~(1ULL << INTERCEPT_VMMCALL);
+	__clr_intercept(&c->intercepts, INTERCEPT_VMMCALL);
 
 	for (i = 0; i < MAX_VECTORS; i++)
 		c->intercepts[i] |= g->intercepts[i];
-
-	c->intercept |= g->intercept;
 }
 
 static void copy_vmcb_control_area(struct vmcb_control_area *dst,
@@ -153,7 +149,6 @@ static void copy_vmcb_control_area(struct vmcb_control_area *dst,
 	for (i = 0; i < MAX_VECTORS; i++)
 		dst->intercepts[i] = from->intercepts[i];
 
-	dst->intercept            = from->intercept;
 	dst->iopm_base_pa         = from->iopm_base_pa;
 	dst->msrpm_base_pa        = from->msrpm_base_pa;
 	dst->tsc_offset           = from->tsc_offset;
@@ -186,7 +181,7 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
 	 */
 	int i;
 
-	if (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
+	if (!(__is_intercept(&svm->nested.ctl.intercepts, INTERCEPT_MSR_PROT)))
 		return true;
 
 	for (i = 0; i < MSRPM_OFFSETS; i++) {
@@ -212,7 +207,7 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
 
 static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
 {
-	if ((control->intercept & (1ULL << INTERCEPT_VMRUN)) == 0)
+	if ((__is_intercept(&control->intercepts, INTERCEPT_VMRUN)) == 0)
 		return false;
 
 	if (control->asid == 0)
@@ -436,7 +431,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
 	trace_kvm_nested_intercepts(nested_vmcb->control.intercepts[CR_VECTOR] & 0xffff,
 				    nested_vmcb->control.intercepts[CR_VECTOR] >> 16,
 				    nested_vmcb->control.intercepts[EXCEPTION_VECTOR],
-				    nested_vmcb->control.intercept);
+				    nested_vmcb->control.intercepts[INTERCEPT_VECTOR_3],
+				    nested_vmcb->control.intercepts[INTERCEPT_VECTOR_4]);
 
 	/* Clear internal status */
 	kvm_clear_exception_queue(&svm->vcpu);
@@ -648,7 +644,7 @@ static int nested_svm_exit_handled_msr(struct vcpu_svm *svm)
 	u32 offset, msr, value;
 	int write, mask;
 
-	if (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
+	if (!(__is_intercept(&svm->nested.ctl.intercepts, INTERCEPT_MSR_PROT)))
 		return NESTED_EXIT_HOST;
 
 	msr    = svm->vcpu.arch.regs[VCPU_REGS_RCX];
@@ -675,7 +671,7 @@ static int nested_svm_intercept_ioio(struct vcpu_svm *svm)
 	u8 start_bit;
 	u64 gpa;
 
-	if (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_IOIO_PROT)))
+	if (!(__is_intercept(&svm->nested.ctl.intercepts, INTERCEPT_IOIO_PROT)))
 		return NESTED_EXIT_HOST;
 
 	port = svm->vmcb->control.exit_info_1 >> 16;
@@ -729,8 +725,7 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
 		break;
 	}
 	default: {
-		u64 exit_bits = 1ULL << (exit_code - SVM_EXIT_INTR);
-		if (svm->nested.ctl.intercept & exit_bits)
+		if (__is_intercept(&svm->nested.ctl.intercepts, exit_code))
 			vmexit = NESTED_EXIT_DONE;
 	}
 	}
@@ -838,7 +833,7 @@ static void nested_svm_intr(struct vcpu_svm *svm)
 
 static inline bool nested_exit_on_init(struct vcpu_svm *svm)
 {
-	return (svm->nested.ctl.intercept & (1ULL << INTERCEPT_INIT));
+	return __is_intercept(&svm->nested.ctl.intercepts, INTERCEPT_INIT);
 }
 
 static void nested_svm_init(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d4ac2c5bb365..1db783435a8a 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2204,12 +2204,10 @@ static bool check_selective_cr0_intercepted(struct vcpu_svm *svm,
 {
 	unsigned long cr0 = svm->vcpu.arch.cr0;
 	bool ret = false;
-	u64 intercept;
-
-	intercept = svm->nested.ctl.intercept;
 
 	if (!is_guest_mode(&svm->vcpu) ||
-	    (!(intercept & (1ULL << INTERCEPT_SELECTIVE_CR0))))
+	    (!(__is_intercept(&svm->nested.ctl.intercepts,
+			      INTERCEPT_SELECTIVE_CR0))))
 		return false;
 
 	cr0 &= ~SVM_CR0_SELECTIVE_MASK;
@@ -2802,7 +2800,8 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
 	pr_err("%-20s%04x\n", "dr_read:", control->intercepts[DR_VECTOR] & 0xffff);
 	pr_err("%-20s%04x\n", "dr_write:", control->intercepts[DR_VECTOR] >> 16);
 	pr_err("%-20s%08x\n", "exceptions:", control->intercepts[EXCEPTION_VECTOR]);
-	pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
+	pr_err("%-20s%08x\n", "intercept1:", control->intercepts[INTERCEPT_VECTOR_3]);
+	pr_err("%-20s%08x\n", "intercept2:", control->intercepts[INTERCEPT_VECTOR_4]);
 	pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
 	pr_err("%-20s%d\n", "pause filter threshold:",
 	       control->pause_filter_thresh);
@@ -3677,7 +3676,6 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 		break;
 	case SVM_EXIT_WRITE_CR0: {
 		unsigned long cr0, val;
-		u64 intercept;
 
 		if (info->intercept == x86_intercept_cr_write)
 			icpt_info.exit_code += info->modrm_reg;
@@ -3686,9 +3684,8 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 		    info->intercept == x86_intercept_clts)
 			break;
 
-		intercept = svm->nested.ctl.intercept;
-
-		if (!(intercept & (1ULL << INTERCEPT_SELECTIVE_CR0)))
+		if (!(__is_intercept(&svm->nested.ctl.intercepts,
+				     INTERCEPT_SELECTIVE_CR0)))
 			break;
 
 		cr0 = vcpu->arch.cr0 & ~SVM_CR0_SELECTIVE_MASK;
@@ -3947,7 +3944,8 @@ static bool svm_apic_init_signal_blocked(struct kvm_vcpu *vcpu)
 	 * if an INIT signal is pending.
 	 */
 	return !gif_set(svm) ||
-		   (svm->vmcb->control.intercept & (1ULL << INTERCEPT_INIT));
+		   (__is_intercept(&svm->vmcb->control.intercepts,
+				   INTERCEPT_INIT));
 }
 
 static void svm_vm_destroy(struct kvm *kvm)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 9c798781172d..cf0cfd57a972 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -312,7 +312,7 @@ static inline void set_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept |= (1ULL << bit);
+	__set_intercept(&vmcb->control.intercepts, bit);
 
 	recalc_intercepts(svm);
 }
@@ -321,14 +321,14 @@ static inline void clr_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);
 
-	vmcb->control.intercept &= ~(1ULL << bit);
+	__clr_intercept(&vmcb->control.intercepts, bit);
 
 	recalc_intercepts(svm);
 }
 
 static inline bool is_intercept(struct vcpu_svm *svm, int bit)
 {
-	return (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
+	return __is_intercept(&svm->vmcb->control.intercepts, bit);
 }
 
 static inline bool vgif_enabled(struct vcpu_svm *svm)
@@ -389,17 +389,17 @@ static inline bool svm_nested_virtualize_tpr(struct kvm_vcpu *vcpu)
 
 static inline bool nested_exit_on_smi(struct vcpu_svm *svm)
 {
-	return (svm->nested.ctl.intercept & (1ULL << INTERCEPT_SMI));
+	return __is_intercept(&svm->nested.ctl.intercepts, INTERCEPT_SMI);
 }
 
 static inline bool nested_exit_on_intr(struct vcpu_svm *svm)
 {
-	return (svm->nested.ctl.intercept & (1ULL << INTERCEPT_INTR));
+	return __is_intercept(&svm->nested.ctl.intercepts, INTERCEPT_INTR);
 }
 
 static inline bool nested_exit_on_nmi(struct vcpu_svm *svm)
 {
-	return (svm->nested.ctl.intercept & (1ULL << INTERCEPT_NMI));
+	return __is_intercept(&svm->nested.ctl.intercepts, INTERCEPT_NMI);
 }
 
 void enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index b66432b015d2..6e7262229e6a 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -544,26 +544,30 @@ TRACE_EVENT(kvm_nested_vmrun,
 );
 
 TRACE_EVENT(kvm_nested_intercepts,
-	    TP_PROTO(__u16 cr_read, __u16 cr_write, __u32 exceptions, __u64 intercept),
-	    TP_ARGS(cr_read, cr_write, exceptions, intercept),
+	    TP_PROTO(__u16 cr_read, __u16 cr_write, __u32 exceptions, __u32 intercept1,
+		     __u32 intercept2),
+	    TP_ARGS(cr_read, cr_write, exceptions, intercept1, intercept2),
 
 	TP_STRUCT__entry(
 		__field(	__u16,		cr_read		)
 		__field(	__u16,		cr_write	)
 		__field(	__u32,		exceptions	)
-		__field(	__u64,		intercept	)
+		__field(	__u32,		intercept1	)
+		__field(	__u32,		intercept2	)
 	),
 
 	TP_fast_assign(
 		__entry->cr_read	= cr_read;
 		__entry->cr_write	= cr_write;
 		__entry->exceptions	= exceptions;
-		__entry->intercept	= intercept;
+		__entry->intercept1	= intercept1;
+		__entry->intercept2	= intercept2;
 	),
 
-	TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx",
-		__entry->cr_read, __entry->cr_write, __entry->exceptions,
-		__entry->intercept)
+	TP_printk("cr_read: %04x cr_write: %04x excp: %08x "
+		  "intercept1: %08x intercept2: %08x",
+		  __entry->cr_read, __entry->cr_write, __entry->exceptions,
+		  __entry->intercept1, __entry->intercept2)
 );
 /*
  * Tracepoint for #VMEXIT while nested

Re: [PATCH v3 05/11] KVM: SVM: Modify 64 bit intercept field to two 32 bit vectors

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> Convert all the intercepts to one array of 32 bit vectors in
> vmcb_control_area. This makes it easy for future intercept vector
> additions.  Also update trace functions.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

> @@ -128,8 +129,7 @@ enum {
>
>  struct __attribute__ ((__packed__)) vmcb_control_area {
>         u32 intercepts[MAX_VECTORS];
> -       u64 intercept;
> -       u8 reserved_1[40];
> +       u8 reserved_1[60 - (MAX_VECTORS * 4)];

Perhaps this could be simplified to 'u32 reserved_1[15 - MAX_VECTORS];'

>         u16 pause_filter_thresh;
>         u16 pause_filter_count;
>         u64 iopm_base_pa;

Reviewed-by: Jim Mattson <jmattson@google.com>

RE: [PATCH v3 05/11] KVM: SVM: Modify 64 bit intercept field to two 32 bit vectors

[Babu Moger]

> -----Original Message-----
> From: Jim Mattson <jmattson@google.com>
> Sent: Wednesday, July 29, 2020 4:06 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; Wanpeng Li <wanpengli@tencent.com>; Sean
> Christopherson <sean.j.christopherson@intel.com>; kvm list
> <kvm@vger.kernel.org>; Joerg Roedel <joro@8bytes.org>; the arch/x86
> maintainers <x86@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Ingo
> Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; H . Peter Anvin
> <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v3 05/11] KVM: SVM: Modify 64 bit intercept field to two 32
> bit vectors
> 
> On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
> >
> > Convert all the intercepts to one array of 32 bit vectors in
> > vmcb_control_area. This makes it easy for future intercept vector
> > additions.  Also update trace functions.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> 
> > @@ -128,8 +129,7 @@ enum {
> >
> >  struct __attribute__ ((__packed__)) vmcb_control_area {
> >         u32 intercepts[MAX_VECTORS];
> > -       u64 intercept;
> > -       u8 reserved_1[40];
> > +       u8 reserved_1[60 - (MAX_VECTORS * 4)];
> 
> Perhaps this could be simplified to 'u32 reserved_1[15 - MAX_VECTORS];'

Ok. Will change it. thanks

> 
> >         u16 pause_filter_thresh;
> >         u16 pause_filter_count;
> >         u64 iopm_base_pa;
> 
> Reviewed-by: Jim Mattson <jmattson@google.com>

[PATCH v3 06/11] KVM: SVM: Add new intercept vector in vmcb_control_area

[Babu Moger]

The new intercept bits have been added in vmcb control area to support
few more interceptions. Here are the some of them.
 - INTERCEPT_INVLPGB,
 - INTERCEPT_INVLPGB_ILLEGAL,
 - INTERCEPT_INVPCID,
 - INTERCEPT_MCOMMIT,
 - INTERCEPT_TLBSYNC,

Add new intercept vector in vmcb_control_area to support these instructions.
Also update kvm_nested_vmrun trace function to support the new addition.

AMD documentation for these instructions is available at "AMD64
Architecture Programmer’s Manual Volume 2: System Programming, Pub. 24593
Rev. 3.34(or later)"

The documentation can be obtained at the links below:
Link: https://www.amd.com/system/files/TechDocs/24593.pdf
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/svm.h |    7 +++++++
 arch/x86/kvm/svm/nested.c  |    3 ++-
 arch/x86/kvm/trace.h       |   13 ++++++++-----
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index aa9f1d62db29..75cbcfb81332 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -16,6 +16,7 @@ enum vector_offset {
 	EXCEPTION_VECTOR,
 	INTERCEPT_VECTOR_3,
 	INTERCEPT_VECTOR_4,
+	INTERCEPT_VECTOR_5,
 	MAX_VECTORS,
 };
 
@@ -124,6 +125,12 @@ enum {
 	INTERCEPT_MWAIT_COND,
 	INTERCEPT_XSETBV,
 	INTERCEPT_RDPRU,
+	/* Byte offset 014h (Vector 5) */
+	INTERCEPT_INVLPGB = 160,
+	INTERCEPT_INVLPGB_ILLEGAL,
+	INTERCEPT_INVPCID,
+	INTERCEPT_MCOMMIT,
+	INTERCEPT_TLBSYNC,
 };
 
 
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index d2552de42fb1..b0e47f474bb6 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -432,7 +432,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
 				    nested_vmcb->control.intercepts[CR_VECTOR] >> 16,
 				    nested_vmcb->control.intercepts[EXCEPTION_VECTOR],
 				    nested_vmcb->control.intercepts[INTERCEPT_VECTOR_3],
-				    nested_vmcb->control.intercepts[INTERCEPT_VECTOR_4]);
+				    nested_vmcb->control.intercepts[INTERCEPT_VECTOR_4],
+				    nested_vmcb->control.intercepts[INTERCEPT_VECTOR_5]);
 
 	/* Clear internal status */
 	kvm_clear_exception_queue(&svm->vcpu);
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 6e7262229e6a..11046171b5d9 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -544,9 +544,10 @@ TRACE_EVENT(kvm_nested_vmrun,
 );
 
 TRACE_EVENT(kvm_nested_intercepts,
-	    TP_PROTO(__u16 cr_read, __u16 cr_write, __u32 exceptions, __u32 intercept1,
-		     __u32 intercept2),
-	    TP_ARGS(cr_read, cr_write, exceptions, intercept1, intercept2),
+	    TP_PROTO(__u16 cr_read, __u16 cr_write, __u32 exceptions,
+		     __u32 intercept1, __u32 intercept2, __u32 intercept3),
+	    TP_ARGS(cr_read, cr_write, exceptions, intercept1,
+		    intercept2, intercept3),
 
 	TP_STRUCT__entry(
 		__field(	__u16,		cr_read		)
@@ -554,6 +555,7 @@ TRACE_EVENT(kvm_nested_intercepts,
 		__field(	__u32,		exceptions	)
 		__field(	__u32,		intercept1	)
 		__field(	__u32,		intercept2	)
+		__field(	__u32,		intercept3	)
 	),
 
 	TP_fast_assign(
@@ -562,12 +564,13 @@ TRACE_EVENT(kvm_nested_intercepts,
 		__entry->exceptions	= exceptions;
 		__entry->intercept1	= intercept1;
 		__entry->intercept2	= intercept2;
+		__entry->intercept3	= intercept3;
 	),
 
 	TP_printk("cr_read: %04x cr_write: %04x excp: %08x "
-		  "intercept1: %08x intercept2: %08x",
+		  "intercept1: %08x intercept2: %08x  intercept3: %08x",
 		  __entry->cr_read, __entry->cr_write, __entry->exceptions,
-		  __entry->intercept1, __entry->intercept2)
+		  __entry->intercept1, __entry->intercept2, __entry->intercept3)
 );
 /*
  * Tracepoint for #VMEXIT while nested

Re: [PATCH v3 06/11] KVM: SVM: Add new intercept vector in vmcb_control_area

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> The new intercept bits have been added in vmcb control area to support
> few more interceptions. Here are the some of them.
>  - INTERCEPT_INVLPGB,
>  - INTERCEPT_INVLPGB_ILLEGAL,
>  - INTERCEPT_INVPCID,
>  - INTERCEPT_MCOMMIT,
>  - INTERCEPT_TLBSYNC,
>
> Add new intercept vector in vmcb_control_area to support these instructions.
> Also update kvm_nested_vmrun trace function to support the new addition.
>
> AMD documentation for these instructions is available at "AMD64
> Architecture Programmer’s Manual Volume 2: System Programming, Pub. 24593
> Rev. 3.34(or later)"
>
> The documentation can be obtained at the links below:
> Link: https://www.amd.com/system/files/TechDocs/24593.pdf
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

> @@ -16,6 +16,7 @@ enum vector_offset {
>         EXCEPTION_VECTOR,
>         INTERCEPT_VECTOR_3,
>         INTERCEPT_VECTOR_4,
> +       INTERCEPT_VECTOR_5,
>         MAX_VECTORS,
>  };

Is this enumeration actually adding any value?
vmcb->control.intercepts[INTERCEPT_VECTOR_5] doesn't seem in any way
"better" than just vmcb->control.intercepts[5].

Reviewed-by: Jim Mattson <jmattson@google.com>

RE: [PATCH v3 06/11] KVM: SVM: Add new intercept vector in vmcb_control_area

[Babu Moger]

> -----Original Message-----
> From: Jim Mattson <jmattson@google.com>
> Sent: Wednesday, July 29, 2020 4:24 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; Wanpeng Li <wanpengli@tencent.com>; Sean
> Christopherson <sean.j.christopherson@intel.com>; kvm list
> <kvm@vger.kernel.org>; Joerg Roedel <joro@8bytes.org>; the arch/x86
> maintainers <x86@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Ingo
> Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; H . Peter Anvin
> <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v3 06/11] KVM: SVM: Add new intercept vector in
> vmcb_control_area
> 
> On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
> >
> > The new intercept bits have been added in vmcb control area to support
> > few more interceptions. Here are the some of them.
> >  - INTERCEPT_INVLPGB,
> >  - INTERCEPT_INVLPGB_ILLEGAL,
> >  - INTERCEPT_INVPCID,
> >  - INTERCEPT_MCOMMIT,
> >  - INTERCEPT_TLBSYNC,
> >
> > Add new intercept vector in vmcb_control_area to support these instructions.
> > Also update kvm_nested_vmrun trace function to support the new addition.
> >
> > AMD documentation for these instructions is available at "AMD64
> > Architecture Programmer’s Manual Volume 2: System Programming, Pub.
> > 24593 Rev. 3.34(or later)"
> >
> > The documentation can be obtained at the links below:
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> >
> amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&amp;data=02%7C01%
> 7Cbab
> >
> u.moger%40amd.com%7C04dafd87052d4ed59f9808d83405b0a4%7C3dd8961fe
> 4884e6
> >
> 08e11a82d994e183d%7C0%7C0%7C637316547054108593&amp;sdata=2ncYK2
> NY1J3xL
> > 9ZXSdb24zq0M0ZkF0iy%2FIW7SUDoFeg%3D&amp;reserved=0
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> >
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D206537&amp;data=02%7C01%7Cbab
> u.m
> >
> oger%40amd.com%7C04dafd87052d4ed59f9808d83405b0a4%7C3dd8961fe488
> 4e608e
> >
> 11a82d994e183d%7C0%7C0%7C637316547054108593&amp;sdata=Trw3tJE1Z6
> dOTXi0
> > DbPhOUAh4Ulr7HxxoJNpM2IjbvM%3D&amp;reserved=0
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> 
> > @@ -16,6 +16,7 @@ enum vector_offset {
> >         EXCEPTION_VECTOR,
> >         INTERCEPT_VECTOR_3,
> >         INTERCEPT_VECTOR_4,
> > +       INTERCEPT_VECTOR_5,
> >         MAX_VECTORS,
> >  };
> 
> Is this enumeration actually adding any value?

Yea. It is not much of a value add. It helps readability a little bit.
That’s why I kept that way. Thanks

> vmcb->control.intercepts[INTERCEPT_VECTOR_5] doesn't seem in any way
> "better" than just vmcb->control.intercepts[5].
> 
> Reviewed-by: Jim Mattson <jmattson@google.com>

[PATCH v3 07/11] KVM: nSVM: Cleanup nested_state data structure

[Babu Moger]

host_intercept_exceptions is not used anywhere. Clean it up.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kvm/svm/nested.c |    2 --
 arch/x86/kvm/svm/svm.h    |    1 -
 2 files changed, 3 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index b0e47f474bb6..1318cf1cd0fe 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -116,8 +116,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	h = &svm->nested.hsave->control;
 	g = &svm->nested.ctl;
 
-	svm->nested.host_intercept_exceptions = h->intercepts[EXCEPTION_VECTOR];
-
 	for (i = 0; i < MAX_VECTORS; i++)
 		c->intercepts[i] = h->intercepts[i];
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index cf0cfd57a972..450d7b196efd 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -86,7 +86,6 @@ struct nested_state {
 	u64 hsave_msr;
 	u64 vm_cr_msr;
 	u64 vmcb;
-	u32 host_intercept_exceptions;
 
 	/* These are the merged vectors */
 	u32 *msrpm;

Re: [PATCH v3 07/11] KVM: nSVM: Cleanup nested_state data structure

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> host_intercept_exceptions is not used anywhere. Clean it up.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[PATCH v3 08/11] KVM: SVM: Remove set_cr_intercept, clr_cr_intercept and is_cr_intercept

[Babu Moger]

Remove set_cr_intercept, clr_cr_intercept and is_cr_intercept. Instead
call generic set_intercept, clr_intercept and is_intercept for all
cr intercepts.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kvm/svm/svm.c |   34 +++++++++++++++++-----------------
 arch/x86/kvm/svm/svm.h |   25 -------------------------
 2 files changed, 17 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 1db783435a8a..32037ed622a7 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -977,14 +977,14 @@ static void init_vmcb(struct vcpu_svm *svm)
 
 	svm->vcpu.arch.hflags = 0;
 
-	set_cr_intercept(svm, INTERCEPT_CR0_READ);
-	set_cr_intercept(svm, INTERCEPT_CR3_READ);
-	set_cr_intercept(svm, INTERCEPT_CR4_READ);
-	set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
-	set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
-	set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
+	set_intercept(svm, INTERCEPT_CR0_READ);
+	set_intercept(svm, INTERCEPT_CR3_READ);
+	set_intercept(svm, INTERCEPT_CR4_READ);
+	set_intercept(svm, INTERCEPT_CR0_WRITE);
+	set_intercept(svm, INTERCEPT_CR3_WRITE);
+	set_intercept(svm, INTERCEPT_CR4_WRITE);
 	if (!kvm_vcpu_apicv_active(&svm->vcpu))
-		set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
+		set_intercept(svm, INTERCEPT_CR8_WRITE);
 
 	set_dr_intercepts(svm);
 
@@ -1079,8 +1079,8 @@ static void init_vmcb(struct vcpu_svm *svm)
 		control->nested_ctl |= SVM_NESTED_CTL_NP_ENABLE;
 		clr_intercept(svm, INTERCEPT_INVLPG);
 		clr_exception_intercept(svm, INTERCEPT_PF_VECTOR);
-		clr_cr_intercept(svm, INTERCEPT_CR3_READ);
-		clr_cr_intercept(svm, INTERCEPT_CR3_WRITE);
+		clr_intercept(svm, INTERCEPT_CR3_READ);
+		clr_intercept(svm, INTERCEPT_CR3_WRITE);
 		save->g_pat = svm->vcpu.arch.pat;
 		save->cr3 = 0;
 		save->cr4 = 0;
@@ -1534,11 +1534,11 @@ static void update_cr0_intercept(struct vcpu_svm *svm)
 	mark_dirty(svm->vmcb, VMCB_CR);
 
 	if (gcr0 == *hcr0) {
-		clr_cr_intercept(svm, INTERCEPT_CR0_READ);
-		clr_cr_intercept(svm, INTERCEPT_CR0_WRITE);
+		clr_intercept(svm, INTERCEPT_CR0_READ);
+		clr_intercept(svm, INTERCEPT_CR0_WRITE);
 	} else {
-		set_cr_intercept(svm, INTERCEPT_CR0_READ);
-		set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
+		set_intercept(svm, INTERCEPT_CR0_READ);
+		set_intercept(svm, INTERCEPT_CR0_WRITE);
 	}
 }
 
@@ -2916,7 +2916,7 @@ static int handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
 
 	trace_kvm_exit(exit_code, vcpu, KVM_ISA_SVM);
 
-	if (!is_cr_intercept(svm, INTERCEPT_CR0_WRITE))
+	if (!is_intercept(svm, INTERCEPT_CR0_WRITE))
 		vcpu->arch.cr0 = svm->vmcb->save.cr0;
 	if (npt_enabled)
 		vcpu->arch.cr3 = svm->vmcb->save.cr3;
@@ -3042,13 +3042,13 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 	if (svm_nested_virtualize_tpr(vcpu))
 		return;
 
-	clr_cr_intercept(svm, INTERCEPT_CR8_WRITE);
+	clr_intercept(svm, INTERCEPT_CR8_WRITE);
 
 	if (irr == -1)
 		return;
 
 	if (tpr >= irr)
-		set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
+		set_intercept(svm, INTERCEPT_CR8_WRITE);
 }
 
 bool svm_nmi_blocked(struct kvm_vcpu *vcpu)
@@ -3236,7 +3236,7 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu)
 	if (svm_nested_virtualize_tpr(vcpu))
 		return;
 
-	if (!is_cr_intercept(svm, INTERCEPT_CR8_WRITE)) {
+	if (!is_intercept(svm, INTERCEPT_CR8_WRITE)) {
 		int cr8 = svm->vmcb->control.int_ctl & V_TPR_MASK;
 		kvm_set_cr8(vcpu, cr8);
 	}
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 450d7b196efd..c7fbce738337 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -231,31 +231,6 @@ static inline bool __is_intercept(void *addr, int bit)
 	return test_bit(bit, (unsigned long *)addr);
 }
 
-static inline void set_cr_intercept(struct vcpu_svm *svm, int bit)
-{
-	struct vmcb *vmcb = get_host_vmcb(svm);
-
-	__set_intercept(&vmcb->control.intercepts, bit);
-
-	recalc_intercepts(svm);
-}
-
-static inline void clr_cr_intercept(struct vcpu_svm *svm, int bit)
-{
-	struct vmcb *vmcb = get_host_vmcb(svm);
-
-	__clr_intercept(&vmcb->control.intercepts, bit);
-
-	recalc_intercepts(svm);
-}
-
-static inline bool is_cr_intercept(struct vcpu_svm *svm, int bit)
-{
-	struct vmcb *vmcb = get_host_vmcb(svm);
-
-	return __is_intercept(&vmcb->control.intercepts, bit);
-}
-
 static inline void set_dr_intercepts(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);

Re: [PATCH v3 08/11] KVM: SVM: Remove set_cr_intercept, clr_cr_intercept and is_cr_intercept

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> Remove set_cr_intercept, clr_cr_intercept and is_cr_intercept. Instead
> call generic set_intercept, clr_intercept and is_intercept for all
> cr intercepts.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[PATCH v3 09/11] KVM: SVM: Remove set_exception_intercept and clr_exception_intercept

[Babu Moger]

Remove set_exception_intercept and clr_exception_intercept.
Replace with generic set_intercept and clr_intercept for these calls.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kvm/svm/svm.c |   20 ++++++++++----------
 arch/x86/kvm/svm/svm.h |   18 ------------------
 2 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 32037ed622a7..99cc9c285fe6 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -988,11 +988,11 @@ static void init_vmcb(struct vcpu_svm *svm)
 
 	set_dr_intercepts(svm);
 
-	set_exception_intercept(svm, INTERCEPT_PF_VECTOR);
-	set_exception_intercept(svm, INTERCEPT_UD_VECTOR);
-	set_exception_intercept(svm, INTERCEPT_MC_VECTOR);
-	set_exception_intercept(svm, INTERCEPT_AC_VECTOR);
-	set_exception_intercept(svm, INTERCEPT_DB_VECTOR);
+	set_intercept(svm, INTERCEPT_PF_VECTOR);
+	set_intercept(svm, INTERCEPT_UD_VECTOR);
+	set_intercept(svm, INTERCEPT_MC_VECTOR);
+	set_intercept(svm, INTERCEPT_AC_VECTOR);
+	set_intercept(svm, INTERCEPT_DB_VECTOR);
 	/*
 	 * Guest access to VMware backdoor ports could legitimately
 	 * trigger #GP because of TSS I/O permission bitmap.
@@ -1000,7 +1000,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 	 * as VMware does.
 	 */
 	if (enable_vmware_backdoor)
-		set_exception_intercept(svm, INTERCEPT_GP_VECTOR);
+		set_intercept(svm, INTERCEPT_GP_VECTOR);
 
 	set_intercept(svm, INTERCEPT_INTR);
 	set_intercept(svm, INTERCEPT_NMI);
@@ -1078,7 +1078,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 		/* Setup VMCB for Nested Paging */
 		control->nested_ctl |= SVM_NESTED_CTL_NP_ENABLE;
 		clr_intercept(svm, INTERCEPT_INVLPG);
-		clr_exception_intercept(svm, INTERCEPT_PF_VECTOR);
+		clr_intercept(svm, INTERCEPT_PF_VECTOR);
 		clr_intercept(svm, INTERCEPT_CR3_READ);
 		clr_intercept(svm, INTERCEPT_CR3_WRITE);
 		save->g_pat = svm->vcpu.arch.pat;
@@ -1120,7 +1120,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 
 	if (sev_guest(svm->vcpu.kvm)) {
 		svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ENABLE;
-		clr_exception_intercept(svm, INTERCEPT_UD_VECTOR);
+		clr_intercept(svm, INTERCEPT_UD_VECTOR);
 	}
 
 	mark_all_dirty(svm->vmcb);
@@ -1631,11 +1631,11 @@ static void update_bp_intercept(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	clr_exception_intercept(svm, INTERCEPT_BP_VECTOR);
+	clr_intercept(svm, INTERCEPT_BP_VECTOR);
 
 	if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) {
 		if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
-			set_exception_intercept(svm, INTERCEPT_BP_VECTOR);
+			set_intercept(svm, INTERCEPT_BP_VECTOR);
 	} else
 		vcpu->guest_debug = 0;
 }
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index c7fbce738337..b95074971ea1 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -264,24 +264,6 @@ static inline void clr_dr_intercepts(struct vcpu_svm *svm)
 	recalc_intercepts(svm);
 }
 
-static inline void set_exception_intercept(struct vcpu_svm *svm, int bit)
-{
-	struct vmcb *vmcb = get_host_vmcb(svm);
-
-	__set_intercept(&vmcb->control.intercepts, bit);
-
-	recalc_intercepts(svm);
-}
-
-static inline void clr_exception_intercept(struct vcpu_svm *svm, int bit)
-{
-	struct vmcb *vmcb = get_host_vmcb(svm);
-
-	__clr_intercept(&vmcb->control.intercepts, bit);
-
-	recalc_intercepts(svm);
-}
-
 static inline void set_intercept(struct vcpu_svm *svm, int bit)
 {
 	struct vmcb *vmcb = get_host_vmcb(svm);

Re: [PATCH v3 09/11] KVM: SVM: Remove set_exception_intercept and clr_exception_intercept

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> Remove set_exception_intercept and clr_exception_intercept.
> Replace with generic set_intercept and clr_intercept for these calls.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[PATCH v3 10/11] KVM: X86: Move handling of INVPCID types to x86

[Babu Moger]

INVPCID instruction handling is mostly same across both VMX and
SVM. So, move the code to common x86.c.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kvm/vmx/vmx.c |   62 +------------------------------------------
 arch/x86/kvm/x86.c     |   69 ++++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.h     |    3 +-
 3 files changed, 72 insertions(+), 62 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 13745f2a5ecd..eb988ebedd9e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5500,11 +5500,8 @@ static int handle_invpcid(struct kvm_vcpu *vcpu)
 {
 	u32 vmx_instruction_info;
 	unsigned long type;
-	bool pcid_enabled;
 	gva_t gva;
 	struct x86_exception e;
-	unsigned i;
-	unsigned long roots_to_free = 0;
 	struct {
 		u64 pcid;
 		u64 gla;
@@ -5536,64 +5533,7 @@ static int handle_invpcid(struct kvm_vcpu *vcpu)
 	if (r != X86EMUL_CONTINUE)
 		return vmx_handle_memory_failure(vcpu, r, &e);
 
-	if (operand.pcid >> 12 != 0) {
-		kvm_inject_gp(vcpu, 0);
-		return 1;
-	}
-
-	pcid_enabled = kvm_read_cr4_bits(vcpu, X86_CR4_PCIDE);
-
-	switch (type) {
-	case INVPCID_TYPE_INDIV_ADDR:
-		if ((!pcid_enabled && (operand.pcid != 0)) ||
-		    is_noncanonical_address(operand.gla, vcpu)) {
-			kvm_inject_gp(vcpu, 0);
-			return 1;
-		}
-		kvm_mmu_invpcid_gva(vcpu, operand.gla, operand.pcid);
-		return kvm_skip_emulated_instruction(vcpu);
-
-	case INVPCID_TYPE_SINGLE_CTXT:
-		if (!pcid_enabled && (operand.pcid != 0)) {
-			kvm_inject_gp(vcpu, 0);
-			return 1;
-		}
-
-		if (kvm_get_active_pcid(vcpu) == operand.pcid) {
-			kvm_mmu_sync_roots(vcpu);
-			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
-		}
-
-		for (i = 0; i < KVM_MMU_NUM_PREV_ROOTS; i++)
-			if (kvm_get_pcid(vcpu, vcpu->arch.mmu->prev_roots[i].pgd)
-			    == operand.pcid)
-				roots_to_free |= KVM_MMU_ROOT_PREVIOUS(i);
-
-		kvm_mmu_free_roots(vcpu, vcpu->arch.mmu, roots_to_free);
-		/*
-		 * If neither the current cr3 nor any of the prev_roots use the
-		 * given PCID, then nothing needs to be done here because a
-		 * resync will happen anyway before switching to any other CR3.
-		 */
-
-		return kvm_skip_emulated_instruction(vcpu);
-
-	case INVPCID_TYPE_ALL_NON_GLOBAL:
-		/*
-		 * Currently, KVM doesn't mark global entries in the shadow
-		 * page tables, so a non-global flush just degenerates to a
-		 * global flush. If needed, we could optimize this later by
-		 * keeping track of global entries in shadow page tables.
-		 */
-
-		/* fall-through */
-	case INVPCID_TYPE_ALL_INCL_GLOBAL:
-		kvm_mmu_unload(vcpu);
-		return kvm_skip_emulated_instruction(vcpu);
-
-	default:
-		BUG(); /* We have already checked above that type <= 3 */
-	}
+	return kvm_handle_invpcid(vcpu, type, operand.pcid, operand.gla);
 }
 
 static int handle_pml_full(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 88c593f83b28..a3a3aa42b695 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -70,6 +70,7 @@
 #include <asm/irq_remapping.h>
 #include <asm/mshyperv.h>
 #include <asm/hypervisor.h>
+#include <asm/tlbflush.h>
 #include <asm/intel_pt.h>
 #include <asm/emulate_prefix.h>
 #include <clocksource/hyperv_timer.h>
@@ -10699,6 +10700,74 @@ u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
 
+int kvm_handle_invpcid(struct kvm_vcpu *vcpu, unsigned long type,
+		       u64 pcid, u64 gla)
+{
+	unsigned long roots_to_free = 0;
+	bool pcid_enabled;
+	unsigned int i;
+
+	if (pcid >> 12 != 0) {
+		kvm_inject_gp(vcpu, 0);
+		return 1;
+	}
+
+	pcid_enabled = kvm_read_cr4_bits(vcpu, X86_CR4_PCIDE);
+
+	switch (type) {
+	case INVPCID_TYPE_INDIV_ADDR:
+		if ((!pcid_enabled && (pcid != 0)) ||
+		    is_noncanonical_address(gla, vcpu)) {
+			kvm_inject_gp(vcpu, 0);
+			return 1;
+		}
+		kvm_mmu_invpcid_gva(vcpu, gla, pcid);
+		return kvm_skip_emulated_instruction(vcpu);
+
+	case INVPCID_TYPE_SINGLE_CTXT:
+		if (!pcid_enabled && (pcid != 0)) {
+			kvm_inject_gp(vcpu, 0);
+			return 1;
+		}
+
+		if (kvm_get_active_pcid(vcpu) == pcid) {
+			kvm_mmu_sync_roots(vcpu);
+			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
+		}
+
+		for (i = 0; i < KVM_MMU_NUM_PREV_ROOTS; i++)
+			if (kvm_get_pcid(vcpu, vcpu->arch.mmu->prev_roots[i].pgd)
+			    == pcid)
+				roots_to_free |= KVM_MMU_ROOT_PREVIOUS(i);
+
+		kvm_mmu_free_roots(vcpu, vcpu->arch.mmu, roots_to_free);
+		/*
+		 * If neither the current cr3 nor any of the prev_roots use the
+		 * given PCID, then nothing needs to be done here because a
+		 * resync will happen anyway before switching to any other CR3.
+		 */
+
+		return kvm_skip_emulated_instruction(vcpu);
+
+	case INVPCID_TYPE_ALL_NON_GLOBAL:
+		/*
+		 * Currently, KVM doesn't mark global entries in the shadow
+		 * page tables, so a non-global flush just degenerates to a
+		 * global flush. If needed, we could optimize this later by
+		 * keeping track of global entries in shadow page tables.
+		 */
+
+		/* fall-through */
+	case INVPCID_TYPE_ALL_INCL_GLOBAL:
+		kvm_mmu_unload(vcpu);
+		return kvm_skip_emulated_instruction(vcpu);
+
+	default:
+		BUG(); /* We have already checked above that type <= 3 */
+	}
+}
+EXPORT_SYMBOL_GPL(kvm_handle_invpcid);
+
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_inj_virq);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 6eb62e97e59f..2f2db47a1a50 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -365,5 +365,6 @@ void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
 void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
 u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu);
 bool kvm_vcpu_exit_request(struct kvm_vcpu *vcpu);
-
+int kvm_handle_invpcid(struct kvm_vcpu *vcpu, unsigned long type,
+		       u64 pcid, u64 gla);
 #endif

Re: [PATCH v3 10/11] KVM: X86: Move handling of INVPCID types to x86

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:38 PM Babu Moger <babu.moger@amd.com> wrote:
>
> INVPCID instruction handling is mostly same across both VMX and
> SVM. So, move the code to common x86.c.
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[PATCH v3 11/11] KVM:SVM: Enable INVPCID feature on AMD

[Babu Moger]

The following intercept bit has been added to support VMEXIT
for INVPCID instruction:
Code    Name            Cause
A2h     VMEXIT_INVPCID  INVPCID instruction

The following bit has been added to the VMCB layout control area
to control intercept of INVPCID:
Byte Offset     Bit(s)    Function
14h             2         intercept INVPCID

Enable the interceptions when the the guest is running with shadow
page table enabled and handle the tlbflush based on the invpcid
instruction type.

For the guests with nested page table (NPT) support, the INVPCID
feature works as running it natively. KVM does not need to do any
special handling in this case.

AMD documentation for INVPCID feature is available at "AMD64
Architecture Programmer’s Manual Volume 2: System Programming,
Pub. 24593 Rev. 3.34(or later)"

The documentation can be obtained at the links below:
Link: https://www.amd.com/system/files/TechDocs/24593.pdf
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/uapi/asm/svm.h |    2 +
 arch/x86/kvm/svm/svm.c          |   64 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
index 2e8a30f06c74..522d42dfc28c 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -76,6 +76,7 @@
 #define SVM_EXIT_MWAIT_COND    0x08c
 #define SVM_EXIT_XSETBV        0x08d
 #define SVM_EXIT_RDPRU         0x08e
+#define SVM_EXIT_INVPCID       0x0a2
 #define SVM_EXIT_NPF           0x400
 #define SVM_EXIT_AVIC_INCOMPLETE_IPI		0x401
 #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS	0x402
@@ -171,6 +172,7 @@
 	{ SVM_EXIT_MONITOR,     "monitor" }, \
 	{ SVM_EXIT_MWAIT,       "mwait" }, \
 	{ SVM_EXIT_XSETBV,      "xsetbv" }, \
+	{ SVM_EXIT_INVPCID,     "invpcid" }, \
 	{ SVM_EXIT_NPF,         "npf" }, \
 	{ SVM_EXIT_AVIC_INCOMPLETE_IPI,		"avic_incomplete_ipi" }, \
 	{ SVM_EXIT_AVIC_UNACCELERATED_ACCESS,   "avic_unaccelerated_access" }, \
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 99cc9c285fe6..6b099e0b28c0 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -813,6 +813,11 @@ static __init void svm_set_cpu_caps(void)
 	if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
 	    boot_cpu_has(X86_FEATURE_AMD_SSBD))
 		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
+
+	/* Enable INVPCID if both PCID and INVPCID enabled */
+	if (boot_cpu_has(X86_FEATURE_PCID) &&
+	    boot_cpu_has(X86_FEATURE_INVPCID))
+		kvm_cpu_cap_set(X86_FEATURE_INVPCID);
 }
 
 static __init int svm_hardware_setup(void)
@@ -1099,6 +1104,18 @@ static void init_vmcb(struct vcpu_svm *svm)
 		clr_intercept(svm, INTERCEPT_PAUSE);
 	}
 
+	/*
+	 * Intercept INVPCID instruction only if shadow page table is
+	 * enabled. Interception is not required with nested page table
+	 * enabled.
+	 */
+	if (boot_cpu_has(X86_FEATURE_INVPCID)) {
+		if (!npt_enabled)
+			set_intercept(svm, INTERCEPT_INVPCID);
+		else
+			clr_intercept(svm, INTERCEPT_INVPCID);
+	}
+
 	if (kvm_vcpu_apicv_active(&svm->vcpu))
 		avic_init_vmcb(svm);
 
@@ -2715,6 +2732,43 @@ static int mwait_interception(struct vcpu_svm *svm)
 	return nop_interception(svm);
 }
 
+static int invpcid_interception(struct vcpu_svm *svm)
+{
+	struct kvm_vcpu *vcpu = &svm->vcpu;
+	struct x86_exception e;
+	unsigned long type;
+	gva_t gva;
+	struct {
+		u64 pcid;
+		u64 gla;
+	} operand;
+
+	if (!guest_cpuid_has(vcpu, X86_FEATURE_INVPCID)) {
+		kvm_queue_exception(vcpu, UD_VECTOR);
+		return 1;
+	}
+
+	/*
+	 * For an INVPCID intercept:
+	 * EXITINFO1 provides the linear address of the memory operand.
+	 * EXITINFO2 provides the contents of the register operand.
+	 */
+	type = svm->vmcb->control.exit_info_2;
+	gva = svm->vmcb->control.exit_info_1;
+
+	if (type > 3) {
+		kvm_inject_gp(vcpu, 0);
+		return 1;
+	}
+
+	if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
+		kvm_inject_emulated_page_fault(vcpu, &e);
+		return 1;
+	}
+
+	return kvm_handle_invpcid(vcpu, type, operand.pcid, operand.gla);
+}
+
 static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
 	[SVM_EXIT_READ_CR0]			= cr_interception,
 	[SVM_EXIT_READ_CR3]			= cr_interception,
@@ -2777,6 +2831,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
 	[SVM_EXIT_MWAIT]			= mwait_interception,
 	[SVM_EXIT_XSETBV]			= xsetbv_interception,
 	[SVM_EXIT_RDPRU]			= rdpru_interception,
+	[SVM_EXIT_INVPCID]                      = invpcid_interception,
 	[SVM_EXIT_NPF]				= npf_interception,
 	[SVM_EXIT_RSM]                          = rsm_interception,
 	[SVM_EXIT_AVIC_INCOMPLETE_IPI]		= avic_incomplete_ipi_interception,
@@ -3562,6 +3617,15 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
 	svm->nrips_enabled = kvm_cpu_cap_has(X86_FEATURE_NRIPS) &&
 			     guest_cpuid_has(&svm->vcpu, X86_FEATURE_NRIPS);
 
+	/* Check again if INVPCID interception if required */
+	if (boot_cpu_has(X86_FEATURE_INVPCID) &&
+	    guest_cpuid_has(vcpu, X86_FEATURE_INVPCID)) {
+		if (!npt_enabled)
+			set_intercept(svm, INTERCEPT_INVPCID);
+		else
+			clr_intercept(svm, INTERCEPT_INVPCID);
+	}
+
 	if (!kvm_vcpu_apicv_active(vcpu))
 		return;
 

Re: [PATCH v3 11/11] KVM:SVM: Enable INVPCID feature on AMD

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:39 PM Babu Moger <babu.moger@amd.com> wrote:
>
> The following intercept bit has been added to support VMEXIT
> for INVPCID instruction:
> Code    Name            Cause
> A2h     VMEXIT_INVPCID  INVPCID instruction
>
> The following bit has been added to the VMCB layout control area
> to control intercept of INVPCID:
> Byte Offset     Bit(s)    Function
> 14h             2         intercept INVPCID
>
> Enable the interceptions when the the guest is running with shadow
> page table enabled and handle the tlbflush based on the invpcid
> instruction type.
>
> For the guests with nested page table (NPT) support, the INVPCID
> feature works as running it natively. KVM does not need to do any
> special handling in this case.
>
> AMD documentation for INVPCID feature is available at "AMD64
> Architecture Programmer’s Manual Volume 2: System Programming,
> Pub. 24593 Rev. 3.34(or later)"
>
> The documentation can be obtained at the links below:
> Link: https://www.amd.com/system/files/TechDocs/24593.pdf
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/include/uapi/asm/svm.h |    2 +
>  arch/x86/kvm/svm/svm.c          |   64 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>
> diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
> index 2e8a30f06c74..522d42dfc28c 100644
> --- a/arch/x86/include/uapi/asm/svm.h
> +++ b/arch/x86/include/uapi/asm/svm.h
> @@ -76,6 +76,7 @@
>  #define SVM_EXIT_MWAIT_COND    0x08c
>  #define SVM_EXIT_XSETBV        0x08d
>  #define SVM_EXIT_RDPRU         0x08e
> +#define SVM_EXIT_INVPCID       0x0a2
>  #define SVM_EXIT_NPF           0x400
>  #define SVM_EXIT_AVIC_INCOMPLETE_IPI           0x401
>  #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS     0x402
> @@ -171,6 +172,7 @@
>         { SVM_EXIT_MONITOR,     "monitor" }, \
>         { SVM_EXIT_MWAIT,       "mwait" }, \
>         { SVM_EXIT_XSETBV,      "xsetbv" }, \
> +       { SVM_EXIT_INVPCID,     "invpcid" }, \
>         { SVM_EXIT_NPF,         "npf" }, \
>         { SVM_EXIT_AVIC_INCOMPLETE_IPI,         "avic_incomplete_ipi" }, \
>         { SVM_EXIT_AVIC_UNACCELERATED_ACCESS,   "avic_unaccelerated_access" }, \
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 99cc9c285fe6..6b099e0b28c0 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -813,6 +813,11 @@ static __init void svm_set_cpu_caps(void)
>         if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
>             boot_cpu_has(X86_FEATURE_AMD_SSBD))
>                 kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
> +
> +       /* Enable INVPCID if both PCID and INVPCID enabled */
> +       if (boot_cpu_has(X86_FEATURE_PCID) &&
> +           boot_cpu_has(X86_FEATURE_INVPCID))
> +               kvm_cpu_cap_set(X86_FEATURE_INVPCID);
>  }

Why is PCID required? Can't this just be
'kvm_cpu_cap_check_and_set(X86_FEATURE_INVPCID);'?

>  static __init int svm_hardware_setup(void)
> @@ -1099,6 +1104,18 @@ static void init_vmcb(struct vcpu_svm *svm)
>                 clr_intercept(svm, INTERCEPT_PAUSE);
>         }
>
> +       /*
> +        * Intercept INVPCID instruction only if shadow page table is
> +        * enabled. Interception is not required with nested page table
> +        * enabled.
> +        */
> +       if (boot_cpu_has(X86_FEATURE_INVPCID)) {

Shouldn't this be 'kvm_cpu_cap_has(X86_FEATURE_INVPCID),' so that it
is consistent with the code above?

> +               if (!npt_enabled)
> +                       set_intercept(svm, INTERCEPT_INVPCID);
> +               else
> +                       clr_intercept(svm, INTERCEPT_INVPCID);
> +       }
> +
>         if (kvm_vcpu_apicv_active(&svm->vcpu))
>                 avic_init_vmcb(svm);
>
> @@ -2715,6 +2732,43 @@ static int mwait_interception(struct vcpu_svm *svm)
>         return nop_interception(svm);
>  }
>
> +static int invpcid_interception(struct vcpu_svm *svm)
> +{
> +       struct kvm_vcpu *vcpu = &svm->vcpu;
> +       struct x86_exception e;
> +       unsigned long type;
> +       gva_t gva;
> +       struct {
> +               u64 pcid;
> +               u64 gla;
> +       } operand;
> +
> +       if (!guest_cpuid_has(vcpu, X86_FEATURE_INVPCID)) {
> +               kvm_queue_exception(vcpu, UD_VECTOR);
> +               return 1;
> +       }
> +
> +       /*
> +        * For an INVPCID intercept:
> +        * EXITINFO1 provides the linear address of the memory operand.
> +        * EXITINFO2 provides the contents of the register operand.
> +        */
> +       type = svm->vmcb->control.exit_info_2;
> +       gva = svm->vmcb->control.exit_info_1;
> +
> +       if (type > 3) {
> +               kvm_inject_gp(vcpu, 0);
> +               return 1;
> +       }
> +
> +       if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
> +               kvm_inject_emulated_page_fault(vcpu, &e);
> +               return 1;
> +       }

The emulated page fault is not always correct. See commit
7a35e515a7055 ("KVM: VMX: Properly handle kvm_read/write_guest_virt*()
result"). I don't think the problems are only on the VMX side.

> +
> +       return kvm_handle_invpcid(vcpu, type, operand.pcid, operand.gla);
> +}
> +
>  static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
>         [SVM_EXIT_READ_CR0]                     = cr_interception,
>         [SVM_EXIT_READ_CR3]                     = cr_interception,
> @@ -2777,6 +2831,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
>         [SVM_EXIT_MWAIT]                        = mwait_interception,
>         [SVM_EXIT_XSETBV]                       = xsetbv_interception,
>         [SVM_EXIT_RDPRU]                        = rdpru_interception,
> +       [SVM_EXIT_INVPCID]                      = invpcid_interception,
>         [SVM_EXIT_NPF]                          = npf_interception,
>         [SVM_EXIT_RSM]                          = rsm_interception,
>         [SVM_EXIT_AVIC_INCOMPLETE_IPI]          = avic_incomplete_ipi_interception,
> @@ -3562,6 +3617,15 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
>         svm->nrips_enabled = kvm_cpu_cap_has(X86_FEATURE_NRIPS) &&
>                              guest_cpuid_has(&svm->vcpu, X86_FEATURE_NRIPS);
>
> +       /* Check again if INVPCID interception if required */
> +       if (boot_cpu_has(X86_FEATURE_INVPCID) &&

Again, shouldn't this be 'kvm_cpu_cap_has(X86_FEATURE_INVPCID)'?
(Better, perhaps, would be to extract this common block of code into a
separate function to be called from both places.)

> +           guest_cpuid_has(vcpu, X86_FEATURE_INVPCID)) {
> +               if (!npt_enabled)
> +                       set_intercept(svm, INTERCEPT_INVPCID);
> +               else
> +                       clr_intercept(svm, INTERCEPT_INVPCID);
> +       }
> +
>         if (!kvm_vcpu_apicv_active(vcpu))
>                 return;
>
>

RE: [PATCH v3 11/11] KVM:SVM: Enable INVPCID feature on AMD

[Babu Moger]

> -----Original Message-----
> From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf
> Of Jim Mattson
> Sent: Wednesday, July 29, 2020 6:01 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; Wanpeng Li <wanpengli@tencent.com>; Sean
> Christopherson <sean.j.christopherson@intel.com>; kvm list
> <kvm@vger.kernel.org>; Joerg Roedel <joro@8bytes.org>; the arch/x86
> maintainers <x86@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Ingo
> Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; H . Peter Anvin
> <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: [PATCH v3 11/11] KVM:SVM: Enable INVPCID feature on AMD
> 
> On Tue, Jul 28, 2020 at 4:39 PM Babu Moger <babu.moger@amd.com> wrote:
> >
> > The following intercept bit has been added to support VMEXIT for
> > INVPCID instruction:
> > Code    Name            Cause
> > A2h     VMEXIT_INVPCID  INVPCID instruction
> >
> > The following bit has been added to the VMCB layout control area to
> > control intercept of INVPCID:
> > Byte Offset     Bit(s)    Function
> > 14h             2         intercept INVPCID
> >
> > Enable the interceptions when the the guest is running with shadow
> > page table enabled and handle the tlbflush based on the invpcid
> > instruction type.
> >
> > For the guests with nested page table (NPT) support, the INVPCID
> > feature works as running it natively. KVM does not need to do any
> > special handling in this case.
> >
> > AMD documentation for INVPCID feature is available at "AMD64
> > Architecture Programmer’s Manual Volume 2: System Programming, Pub.
> > 24593 Rev. 3.34(or later)"
> >
> > The documentation can be obtained at the links below:
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> >
> amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&amp;data=02%7C01%
> 7CBab
> >
> u.Moger%40amd.com%7C68f9bafe44704700ac3b08d834135678%7C3dd8961fe
> 4884e6
> >
> 08e11a82d994e183d%7C0%7C0%7C637316605732430961&amp;sdata=c%2Fss1
> 2Y5Hcy
> > pwDfEIv8kHiI33XI6jtLAb5wUm96%2BY8I%3D&amp;reserved=0
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> >
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D206537&amp;data=02%7C01%7CBab
> u.M
> >
> oger%40amd.com%7C68f9bafe44704700ac3b08d834135678%7C3dd8961fe488
> 4e608e
> >
> 11a82d994e183d%7C0%7C0%7C637316605732430961&amp;sdata=wv5px6rzaT
> R8DcZl
> > CWpAkAFMkAv61XkdRv3274BJD6A%3D&amp;reserved=0
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/include/uapi/asm/svm.h |    2 +
> >  arch/x86/kvm/svm/svm.c          |   64
> +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+)
> >
> > diff --git a/arch/x86/include/uapi/asm/svm.h
> > b/arch/x86/include/uapi/asm/svm.h index 2e8a30f06c74..522d42dfc28c
> > 100644
> > --- a/arch/x86/include/uapi/asm/svm.h
> > +++ b/arch/x86/include/uapi/asm/svm.h
> > @@ -76,6 +76,7 @@
> >  #define SVM_EXIT_MWAIT_COND    0x08c
> >  #define SVM_EXIT_XSETBV        0x08d
> >  #define SVM_EXIT_RDPRU         0x08e
> > +#define SVM_EXIT_INVPCID       0x0a2
> >  #define SVM_EXIT_NPF           0x400
> >  #define SVM_EXIT_AVIC_INCOMPLETE_IPI           0x401
> >  #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS     0x402
> > @@ -171,6 +172,7 @@
> >         { SVM_EXIT_MONITOR,     "monitor" }, \
> >         { SVM_EXIT_MWAIT,       "mwait" }, \
> >         { SVM_EXIT_XSETBV,      "xsetbv" }, \
> > +       { SVM_EXIT_INVPCID,     "invpcid" }, \
> >         { SVM_EXIT_NPF,         "npf" }, \
> >         { SVM_EXIT_AVIC_INCOMPLETE_IPI,         "avic_incomplete_ipi" }, \
> >         { SVM_EXIT_AVIC_UNACCELERATED_ACCESS,
> "avic_unaccelerated_access" }, \
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index
> > 99cc9c285fe6..6b099e0b28c0 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -813,6 +813,11 @@ static __init void svm_set_cpu_caps(void)
> >         if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
> >             boot_cpu_has(X86_FEATURE_AMD_SSBD))
> >                 kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
> > +
> > +       /* Enable INVPCID if both PCID and INVPCID enabled */
> > +       if (boot_cpu_has(X86_FEATURE_PCID) &&
> > +           boot_cpu_has(X86_FEATURE_INVPCID))
> > +               kvm_cpu_cap_set(X86_FEATURE_INVPCID);
> >  }
> 
> Why is PCID required? Can't this just be
> 'kvm_cpu_cap_check_and_set(X86_FEATURE_INVPCID);'?

Ok.  Let me check.
> 
> >  static __init int svm_hardware_setup(void) @@ -1099,6 +1104,18 @@
> > static void init_vmcb(struct vcpu_svm *svm)
> >                 clr_intercept(svm, INTERCEPT_PAUSE);
> >         }
> >
> > +       /*
> > +        * Intercept INVPCID instruction only if shadow page table is
> > +        * enabled. Interception is not required with nested page table
> > +        * enabled.
> > +        */
> > +       if (boot_cpu_has(X86_FEATURE_INVPCID)) {
> 
> Shouldn't this be 'kvm_cpu_cap_has(X86_FEATURE_INVPCID),' so that it is
> consistent with the code above?

Sure. Will check on it.

> 
> > +               if (!npt_enabled)
> > +                       set_intercept(svm, INTERCEPT_INVPCID);
> > +               else
> > +                       clr_intercept(svm, INTERCEPT_INVPCID);
> > +       }
> > +
> >         if (kvm_vcpu_apicv_active(&svm->vcpu))
> >                 avic_init_vmcb(svm);
> >
> > @@ -2715,6 +2732,43 @@ static int mwait_interception(struct vcpu_svm
> *svm)
> >         return nop_interception(svm);
> >  }
> >
> > +static int invpcid_interception(struct vcpu_svm *svm) {
> > +       struct kvm_vcpu *vcpu = &svm->vcpu;
> > +       struct x86_exception e;
> > +       unsigned long type;
> > +       gva_t gva;
> > +       struct {
> > +               u64 pcid;
> > +               u64 gla;
> > +       } operand;
> > +
> > +       if (!guest_cpuid_has(vcpu, X86_FEATURE_INVPCID)) {
> > +               kvm_queue_exception(vcpu, UD_VECTOR);
> > +               return 1;
> > +       }
> > +
> > +       /*
> > +        * For an INVPCID intercept:
> > +        * EXITINFO1 provides the linear address of the memory operand.
> > +        * EXITINFO2 provides the contents of the register operand.
> > +        */
> > +       type = svm->vmcb->control.exit_info_2;
> > +       gva = svm->vmcb->control.exit_info_1;
> > +
> > +       if (type > 3) {
> > +               kvm_inject_gp(vcpu, 0);
> > +               return 1;
> > +       }
> > +
> > +       if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
> > +               kvm_inject_emulated_page_fault(vcpu, &e);
> > +               return 1;
> > +       }
> 
> The emulated page fault is not always correct. See commit
> 7a35e515a7055 ("KVM: VMX: Properly handle kvm_read/write_guest_virt*()
> result"). I don't think the problems are only on the VMX side.

Ok. Sure. Will take a look.

> 
> > +
> > +       return kvm_handle_invpcid(vcpu, type, operand.pcid,
> > +operand.gla); }
> > +
> >  static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
> >         [SVM_EXIT_READ_CR0]                     = cr_interception,
> >         [SVM_EXIT_READ_CR3]                     = cr_interception,
> > @@ -2777,6 +2831,7 @@ static int (*const svm_exit_handlers[])(struct
> vcpu_svm *svm) = {
> >         [SVM_EXIT_MWAIT]                        = mwait_interception,
> >         [SVM_EXIT_XSETBV]                       = xsetbv_interception,
> >         [SVM_EXIT_RDPRU]                        = rdpru_interception,
> > +       [SVM_EXIT_INVPCID]                      = invpcid_interception,
> >         [SVM_EXIT_NPF]                          = npf_interception,
> >         [SVM_EXIT_RSM]                          = rsm_interception,
> >         [SVM_EXIT_AVIC_INCOMPLETE_IPI]          =
> avic_incomplete_ipi_interception,
> > @@ -3562,6 +3617,15 @@ static void svm_cpuid_update(struct kvm_vcpu
> *vcpu)
> >         svm->nrips_enabled = kvm_cpu_cap_has(X86_FEATURE_NRIPS) &&
> >                              guest_cpuid_has(&svm->vcpu,
> > X86_FEATURE_NRIPS);
> >
> > +       /* Check again if INVPCID interception if required */
> > +       if (boot_cpu_has(X86_FEATURE_INVPCID) &&
> 
> Again, shouldn't this be 'kvm_cpu_cap_has(X86_FEATURE_INVPCID)'?
> (Better, perhaps, would be to extract this common block of code into a separate
> function to be called from both places.)

Sure. Will work on it.
Thanks
> 
> > +           guest_cpuid_has(vcpu, X86_FEATURE_INVPCID)) {
> > +               if (!npt_enabled)
> > +                       set_intercept(svm, INTERCEPT_INVPCID);
> > +               else
> > +                       clr_intercept(svm, INTERCEPT_INVPCID);
> > +       }
> > +
> >         if (!kvm_vcpu_apicv_active(vcpu))
> >                 return;
> >
> >

Re: [PATCH v3 00/11] SVM cleanup and INVPCID support for the AMD guests

[Jim Mattson]

On Tue, Jul 28, 2020 at 4:37 PM Babu Moger <babu.moger@amd.com> wrote:
>
> The following series adds the support for PCID/INVPCID on AMD guests.
> While doing it re-structured the vmcb_control_area data structure to
> combine all the intercept vectors into one 32 bit array. Makes it easy
> for future additions.
>
> INVPCID interceptions are added only when the guest is running with
> shadow page table enabled. In this case the hypervisor needs to handle
> the tlbflush based on the type of invpcid instruction.
>
> For the guests with nested page table (NPT) support, the INVPCID feature
> works as running it natively. KVM does not need to do any special handling.
>
> AMD documentation for INVPCID feature is available at "AMD64 Architecture
> Programmer’s Manual Volume 2: System Programming, Pub. 24593 Rev. 3.34(or later)"
>
> The documentation can be obtained at the links below:
> Link: https://www.amd.com/system/files/TechDocs/24593.pdf
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537

Very nice cleanup. Thanks for doing this!