* [PATCH v2 0/3] INVPCID support for the AMD guests
@ 2020-06-16 22:03 Babu Moger
2020-06-16 22:03 ` [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86 Babu Moger
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Babu Moger @ 2020-06-16 22:03 UTC (permalink / raw)
To: wanpengli, joro, x86, sean.j.christopherson, mingo, bp, hpa,
pbonzini, vkuznets, tglx, jmattson
Cc: linux-kernel, kvm
The following series adds the support for PCID/INVPCID on AMD guests.
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.
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 type.
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
---
v2:
- 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.sed the
v1:
https://lore.kernel.org/lkml/159191202523.31436.11959784252237488867.stgit@bmoger-ubuntu/
Babu Moger (3):
KVM: X86: Move handling of INVPCID types to x86
KVM:SVM: Add extended intercept support
KVM:SVM: Enable INVPCID feature on AMD
arch/x86/include/asm/svm.h | 7 +++
arch/x86/include/uapi/asm/svm.h | 2 +
arch/x86/kvm/svm/nested.c | 6 ++-
arch/x86/kvm/svm/svm.c | 55 +++++++++++++++++++++++++++
arch/x86/kvm/svm/svm.h | 18 +++++++++
arch/x86/kvm/trace.h | 12 ++++--
arch/x86/kvm/vmx/vmx.c | 68 ----------------------------------
arch/x86/kvm/x86.c | 79 +++++++++++++++++++++++++++++++++++++++
arch/x86/kvm/x86.h | 3 +
9 files changed, 176 insertions(+), 74 deletions(-)
--
Signature
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86
2020-06-16 22:03 [PATCH v2 0/3] INVPCID support for the AMD guests Babu Moger
@ 2020-06-16 22:03 ` Babu Moger
2020-06-17 11:56 ` Vitaly Kuznetsov
2020-06-16 22:03 ` [PATCH v2 2/3] KVM:SVM: Add extended intercept support Babu Moger
2020-06-16 22:03 ` [PATCH v2 3/3] KVM:SVM: Enable INVPCID feature on AMD Babu Moger
2 siblings, 1 reply; 13+ messages in thread
From: Babu Moger @ 2020-06-16 22:03 UTC (permalink / raw)
To: wanpengli, joro, x86, sean.j.christopherson, mingo, bp, hpa,
pbonzini, vkuznets, tglx, jmattson
Cc: linux-kernel, kvm
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 | 68 +----------------------------------------
arch/x86/kvm/x86.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
arch/x86/kvm/x86.h | 3 +-
3 files changed, 82 insertions(+), 68 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 170cc76a581f..b4140cfd15fd 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5477,11 +5477,7 @@ 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;
@@ -5508,69 +5504,7 @@ static int handle_invpcid(struct kvm_vcpu *vcpu)
sizeof(operand), &gva))
return 1;
- if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
- kvm_inject_emulated_page_fault(vcpu, &e);
- return 1;
- }
-
- 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_types(vcpu, gva, type);
}
static int handle_pml_full(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9e41b5135340..9c858ca0e592 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>
@@ -10714,6 +10715,84 @@ u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
+int kvm_handle_invpcid_types(struct kvm_vcpu *vcpu, gva_t gva,
+ unsigned long type)
+{
+ unsigned long roots_to_free = 0;
+ struct x86_exception e;
+ bool pcid_enabled;
+ unsigned int i;
+ struct {
+ u64 pcid;
+ u64 gla;
+ } operand;
+
+ if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
+ kvm_inject_emulated_page_fault(vcpu, &e);
+ return 1;
+ }
+
+ 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 */
+ }
+}
+EXPORT_SYMBOL_GPL(kvm_handle_invpcid_types);
+
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..f706f6f7196d 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_types(struct kvm_vcpu *vcpu, gva_t gva,
+ unsigned long type);
#endif
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-16 22:03 [PATCH v2 0/3] INVPCID support for the AMD guests Babu Moger
2020-06-16 22:03 ` [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86 Babu Moger
@ 2020-06-16 22:03 ` Babu Moger
2020-06-16 23:17 ` Jim Mattson
2020-06-17 12:01 ` Vitaly Kuznetsov
2020-06-16 22:03 ` [PATCH v2 3/3] KVM:SVM: Enable INVPCID feature on AMD Babu Moger
2 siblings, 2 replies; 13+ messages in thread
From: Babu Moger @ 2020-06-16 22:03 UTC (permalink / raw)
To: wanpengli, joro, x86, sean.j.christopherson, mingo, bp, hpa,
pbonzini, vkuznets, tglx, jmattson
Cc: linux-kernel, kvm
The new intercept bits have been added in vmcb control
area to support the interception of INVPCID instruction.
The following bit is added to the VMCB layout control area
to control intercept of INVPCID:
Byte Offset Bit(s) Function
14h 2 intercept INVPCID
Add the interfaces to support these extended interception.
Also update the tracing for extended intercepts.
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/asm/svm.h | 3 ++-
arch/x86/kvm/svm/nested.c | 6 +++++-
arch/x86/kvm/svm/svm.c | 1 +
arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
arch/x86/kvm/trace.h | 12 ++++++++----
5 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 8a1f5382a4ea..62649fba8908 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -61,7 +61,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
u32 intercept_dr;
u32 intercept_exceptions;
u64 intercept;
- u8 reserved_1[40];
+ u32 intercept_extended;
+ u8 reserved_1[36];
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 8a6db11dcb43..7f6d0f2533e2 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -121,6 +121,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
c->intercept_dr = h->intercept_dr;
c->intercept_exceptions = h->intercept_exceptions;
c->intercept = h->intercept;
+ c->intercept_extended = h->intercept_extended;
if (g->int_ctl & V_INTR_MASKING_MASK) {
/* We only want the cr8 intercept bits of L1 */
@@ -142,6 +143,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
c->intercept_dr |= g->intercept_dr;
c->intercept_exceptions |= g->intercept_exceptions;
c->intercept |= g->intercept;
+ c->intercept_extended |= g->intercept_extended;
}
static void copy_vmcb_control_area(struct vmcb_control_area *dst,
@@ -151,6 +153,7 @@ static void copy_vmcb_control_area(struct vmcb_control_area *dst,
dst->intercept_dr = from->intercept_dr;
dst->intercept_exceptions = from->intercept_exceptions;
dst->intercept = from->intercept;
+ dst->intercept_extended = from->intercept_extended;
dst->iopm_base_pa = from->iopm_base_pa;
dst->msrpm_base_pa = from->msrpm_base_pa;
dst->tsc_offset = from->tsc_offset;
@@ -433,7 +436,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr & 0xffff,
nested_vmcb->control.intercept_cr >> 16,
nested_vmcb->control.intercept_exceptions,
- nested_vmcb->control.intercept);
+ nested_vmcb->control.intercept,
+ nested_vmcb->control.intercept_extended);
/* Clear internal status */
kvm_clear_exception_queue(&svm->vcpu);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 9e333b91ff78..285e5e1ff518 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2801,6 +2801,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
+ pr_err("%-20s%08x\n", "intercepts (extended):", control->intercept_extended);
pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
pr_err("%-20s%d\n", "pause filter threshold:",
control->pause_filter_thresh);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 6ac4c00a5d82..935d08fac03d 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -311,6 +311,24 @@ static inline void clr_intercept(struct vcpu_svm *svm, int bit)
recalc_intercepts(svm);
}
+static inline void set_extended_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_extended |= (1U << bit);
+
+ recalc_intercepts(svm);
+}
+
+static inline void clr_extended_intercept(struct vcpu_svm *svm, int bit)
+{
+ struct vmcb *vmcb = get_host_vmcb(svm);
+
+ vmcb->control.intercept_extended &= ~(1U << bit);
+
+ recalc_intercepts(svm);
+}
+
static inline bool is_intercept(struct vcpu_svm *svm, int bit)
{
return (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index b66432b015d2..5c841c42b33d 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -544,14 +544,16 @@ 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, __u64 intercept,
+ __u32 extended),
+ TP_ARGS(cr_read, cr_write, exceptions, intercept, extended),
TP_STRUCT__entry(
__field( __u16, cr_read )
__field( __u16, cr_write )
__field( __u32, exceptions )
__field( __u64, intercept )
+ __field( __u32, extended )
),
TP_fast_assign(
@@ -559,11 +561,13 @@ TRACE_EVENT(kvm_nested_intercepts,
__entry->cr_write = cr_write;
__entry->exceptions = exceptions;
__entry->intercept = intercept;
+ __entry->extended = extended;
),
- TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx",
+ TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx"
+ "intercept (extended): %08x",
__entry->cr_read, __entry->cr_write, __entry->exceptions,
- __entry->intercept)
+ __entry->intercept, __entry->extended)
);
/*
* Tracepoint for #VMEXIT while nested
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] KVM:SVM: Enable INVPCID feature on AMD
2020-06-16 22:03 [PATCH v2 0/3] INVPCID support for the AMD guests Babu Moger
2020-06-16 22:03 ` [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86 Babu Moger
2020-06-16 22:03 ` [PATCH v2 2/3] KVM:SVM: Add extended intercept support Babu Moger
@ 2020-06-16 22:03 ` Babu Moger
2 siblings, 0 replies; 13+ messages in thread
From: Babu Moger @ 2020-06-16 22:03 UTC (permalink / raw)
To: wanpengli, joro, x86, sean.j.christopherson, mingo, bp, hpa,
pbonzini, vkuznets, tglx, jmattson
Cc: linux-kernel, kvm
The following intercept is added for INVPCID instruction:
Code Name Cause
A2h VMEXIT_INVPCID INVPCID instruction
The following bit is added to the VMCB layout control area
to control intercept of INVPCID:
Byte Offset Bit(s) Function
14h 2 intercept INVPCID
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.
Enable the interceptions when the the guest is running with shadow
page table enabled and handle the tlbflush based on the type of
invpcid instruction type.
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/asm/svm.h | 4 +++
arch/x86/include/uapi/asm/svm.h | 2 +
arch/x86/kvm/svm/svm.c | 54 +++++++++++++++++++++++++++++++++++++++
3 files changed, 60 insertions(+)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 62649fba8908..6488094f67fa 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -55,6 +55,10 @@ enum {
INTERCEPT_RDPRU,
};
+/* Extended Intercept bits */
+enum {
+ INTERCEPT_INVPCID = 2,
+};
struct __attribute__ ((__packed__)) vmcb_control_area {
u32 intercept_cr;
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 285e5e1ff518..5d598a7a0289 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_extended_intercept(svm, INTERCEPT_INVPCID);
+ else
+ clr_extended_intercept(svm, INTERCEPT_INVPCID);
+ }
+
if (kvm_vcpu_apicv_active(&svm->vcpu))
avic_init_vmcb(svm);
@@ -2715,6 +2732,33 @@ 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;
+ unsigned long type;
+ gva_t gva;
+
+ 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;
+ }
+
+ return kvm_handle_invpcid_types(vcpu, gva, type);
+}
+
static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
[SVM_EXIT_READ_CR0] = cr_interception,
[SVM_EXIT_READ_CR3] = cr_interception,
@@ -2777,6 +2821,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 +3607,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_extended_intercept(svm, INTERCEPT_INVPCID);
+ else
+ clr_extended_intercept(svm, INTERCEPT_INVPCID);
+ }
+
if (!kvm_vcpu_apicv_active(vcpu))
return;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-16 22:03 ` [PATCH v2 2/3] KVM:SVM: Add extended intercept support Babu Moger
@ 2020-06-16 23:17 ` Jim Mattson
2020-06-17 14:30 ` Babu Moger
2020-06-17 12:01 ` Vitaly Kuznetsov
1 sibling, 1 reply; 13+ messages in thread
From: Jim Mattson @ 2020-06-16 23:17 UTC (permalink / raw)
To: Babu Moger
Cc: Wanpeng Li, Joerg Roedel, the arch/x86 maintainers,
Sean Christopherson, Ingo Molnar, Borislav Petkov,
H . Peter Anvin, Paolo Bonzini, Vitaly Kuznetsov,
Thomas Gleixner, LKML, kvm list
On Tue, Jun 16, 2020 at 3:03 PM Babu Moger <babu.moger@amd.com> wrote:
>
> The new intercept bits have been added in vmcb control
> area to support the interception of INVPCID instruction.
>
> The following bit is added to the VMCB layout control area
> to control intercept of INVPCID:
>
> Byte Offset Bit(s) Function
> 14h 2 intercept INVPCID
>
> Add the interfaces to support these extended interception.
> Also update the tracing for extended intercepts.
>
> 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
Not your change, but this documentation is terrible. There is no
INVLPCID instruction, nor is there a PCID instruction.
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> arch/x86/include/asm/svm.h | 3 ++-
> arch/x86/kvm/svm/nested.c | 6 +++++-
> arch/x86/kvm/svm/svm.c | 1 +
> arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
> arch/x86/kvm/trace.h | 12 ++++++++----
> 5 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 8a1f5382a4ea..62649fba8908 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -61,7 +61,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
> u32 intercept_dr;
> u32 intercept_exceptions;
> u64 intercept;
> - u8 reserved_1[40];
> + u32 intercept_extended;
> + u8 reserved_1[36];
It seems like a more straightforward implementation would simply
change 'u64 intercept' to 'u32 intercept[3].'
> 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 8a6db11dcb43..7f6d0f2533e2 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -121,6 +121,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> c->intercept_dr = h->intercept_dr;
> c->intercept_exceptions = h->intercept_exceptions;
> c->intercept = h->intercept;
> + c->intercept_extended = h->intercept_extended;
>
> if (g->int_ctl & V_INTR_MASKING_MASK) {
> /* We only want the cr8 intercept bits of L1 */
> @@ -142,6 +143,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> c->intercept_dr |= g->intercept_dr;
> c->intercept_exceptions |= g->intercept_exceptions;
> c->intercept |= g->intercept;
> + c->intercept_extended |= g->intercept_extended;
> }
>
> static void copy_vmcb_control_area(struct vmcb_control_area *dst,
> @@ -151,6 +153,7 @@ static void copy_vmcb_control_area(struct vmcb_control_area *dst,
> dst->intercept_dr = from->intercept_dr;
> dst->intercept_exceptions = from->intercept_exceptions;
> dst->intercept = from->intercept;
> + dst->intercept_extended = from->intercept_extended;
> dst->iopm_base_pa = from->iopm_base_pa;
> dst->msrpm_base_pa = from->msrpm_base_pa;
> dst->tsc_offset = from->tsc_offset;
> @@ -433,7 +436,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
> trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr & 0xffff,
> nested_vmcb->control.intercept_cr >> 16,
> nested_vmcb->control.intercept_exceptions,
> - nested_vmcb->control.intercept);
> + nested_vmcb->control.intercept,
> + nested_vmcb->control.intercept_extended);
>
> /* Clear internal status */
> kvm_clear_exception_queue(&svm->vcpu);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 9e333b91ff78..285e5e1ff518 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2801,6 +2801,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
> pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
> pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
> pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
> + pr_err("%-20s%08x\n", "intercepts (extended):", control->intercept_extended);
> pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
> pr_err("%-20s%d\n", "pause filter threshold:",
> control->pause_filter_thresh);
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 6ac4c00a5d82..935d08fac03d 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -311,6 +311,24 @@ static inline void clr_intercept(struct vcpu_svm *svm, int bit)
> recalc_intercepts(svm);
> }
>
> +static inline void set_extended_intercept(struct vcpu_svm *svm, int bit)
> +{
> + struct vmcb *vmcb = get_host_vmcb(svm);
> +
> + vmcb->control.intercept_extended |= (1U << bit);
> +
> + recalc_intercepts(svm);
> +}
> +
> +static inline void clr_extended_intercept(struct vcpu_svm *svm, int bit)
> +{
> + struct vmcb *vmcb = get_host_vmcb(svm);
> +
> + vmcb->control.intercept_extended &= ~(1U << bit);
> +
> + recalc_intercepts(svm);
> +}
You wouldn't need these new functions if you defined 'u32
intercept[3],' as I suggested above. Just change set_intercept and
clr_intercept to use __set_bit and __clear_bit.
> static inline bool is_intercept(struct vcpu_svm *svm, int bit)
> {
> return (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index b66432b015d2..5c841c42b33d 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -544,14 +544,16 @@ 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, __u64 intercept,
> + __u32 extended),
> + TP_ARGS(cr_read, cr_write, exceptions, intercept, extended),
>
> TP_STRUCT__entry(
> __field( __u16, cr_read )
> __field( __u16, cr_write )
> __field( __u32, exceptions )
> __field( __u64, intercept )
> + __field( __u32, extended )
> ),
>
> TP_fast_assign(
> @@ -559,11 +561,13 @@ TRACE_EVENT(kvm_nested_intercepts,
> __entry->cr_write = cr_write;
> __entry->exceptions = exceptions;
> __entry->intercept = intercept;
> + __entry->extended = extended;
> ),
>
> - TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx",
> + TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx"
> + "intercept (extended): %08x",
> __entry->cr_read, __entry->cr_write, __entry->exceptions,
> - __entry->intercept)
> + __entry->intercept, __entry->extended)
> );
> /*
> * Tracepoint for #VMEXIT while nested
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86
2020-06-16 22:03 ` [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86 Babu Moger
@ 2020-06-17 11:56 ` Vitaly Kuznetsov
2020-06-17 14:31 ` Babu Moger
0 siblings, 1 reply; 13+ messages in thread
From: Vitaly Kuznetsov @ 2020-06-17 11:56 UTC (permalink / raw)
To: Babu Moger
Cc: linux-kernel, kvm, wanpengli, joro, x86, sean.j.christopherson,
mingo, bp, hpa, pbonzini, tglx, jmattson
Babu Moger <babu.moger@amd.com> writes:
> 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 | 68 +----------------------------------------
> arch/x86/kvm/x86.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
> arch/x86/kvm/x86.h | 3 +-
> 3 files changed, 82 insertions(+), 68 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 170cc76a581f..b4140cfd15fd 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5477,11 +5477,7 @@ 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;
> @@ -5508,69 +5504,7 @@ static int handle_invpcid(struct kvm_vcpu *vcpu)
> sizeof(operand), &gva))
> return 1;
>
> - if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
> - kvm_inject_emulated_page_fault(vcpu, &e);
> - return 1;
> - }
> -
> - 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_types(vcpu, gva, type);
Nit: redundant space.
> }
>
> static int handle_pml_full(struct kvm_vcpu *vcpu)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 9e41b5135340..9c858ca0e592 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>
> @@ -10714,6 +10715,84 @@ u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu)
> }
> EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
>
> +int kvm_handle_invpcid_types(struct kvm_vcpu *vcpu, gva_t gva,
> + unsigned long type)
(sorry if this was discussed before) do we really need '_types' suffix?
> +{
> + unsigned long roots_to_free = 0;
> + struct x86_exception e;
> + bool pcid_enabled;
> + unsigned int i;
> + struct {
> + u64 pcid;
> + u64 gla;
> + } operand;
> +
> + if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
> + kvm_inject_emulated_page_fault(vcpu, &e);
> + return 1;
> + }
> +
> + 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 */
The check was left in VMX' handle_invpcid() so we either need to update
the comment to something like "the caller was supposed to check that
type <= 3" or move the check to kvm_handle_invpcid_types().
> + }
> +}
> +EXPORT_SYMBOL_GPL(kvm_handle_invpcid_types);
> +
> 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..f706f6f7196d 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_types(struct kvm_vcpu *vcpu, gva_t gva,
> + unsigned long type);
> #endif
>
--
Vitaly
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-16 22:03 ` [PATCH v2 2/3] KVM:SVM: Add extended intercept support Babu Moger
2020-06-16 23:17 ` Jim Mattson
@ 2020-06-17 12:01 ` Vitaly Kuznetsov
2020-06-17 14:31 ` Babu Moger
1 sibling, 1 reply; 13+ messages in thread
From: Vitaly Kuznetsov @ 2020-06-17 12:01 UTC (permalink / raw)
To: Babu Moger
Cc: linux-kernel, kvm, wanpengli, joro, x86, sean.j.christopherson,
mingo, bp, hpa, pbonzini, tglx, jmattson
Babu Moger <babu.moger@amd.com> writes:
> The new intercept bits have been added in vmcb control
> area to support the interception of INVPCID instruction.
>
> The following bit is added to the VMCB layout control area
> to control intercept of INVPCID:
>
> Byte Offset Bit(s) Function
> 14h 2 intercept INVPCID
>
> Add the interfaces to support these extended interception.
> Also update the tracing for extended intercepts.
>
> 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/asm/svm.h | 3 ++-
> arch/x86/kvm/svm/nested.c | 6 +++++-
> arch/x86/kvm/svm/svm.c | 1 +
> arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
> arch/x86/kvm/trace.h | 12 ++++++++----
> 5 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 8a1f5382a4ea..62649fba8908 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -61,7 +61,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
> u32 intercept_dr;
> u32 intercept_exceptions;
> u64 intercept;
> - u8 reserved_1[40];
> + u32 intercept_extended;
> + u8 reserved_1[36];
> 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 8a6db11dcb43..7f6d0f2533e2 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -121,6 +121,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> c->intercept_dr = h->intercept_dr;
> c->intercept_exceptions = h->intercept_exceptions;
> c->intercept = h->intercept;
> + c->intercept_extended = h->intercept_extended;
>
> if (g->int_ctl & V_INTR_MASKING_MASK) {
> /* We only want the cr8 intercept bits of L1 */
> @@ -142,6 +143,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> c->intercept_dr |= g->intercept_dr;
> c->intercept_exceptions |= g->intercept_exceptions;
> c->intercept |= g->intercept;
> + c->intercept_extended |= g->intercept_extended;
> }
>
> static void copy_vmcb_control_area(struct vmcb_control_area *dst,
> @@ -151,6 +153,7 @@ static void copy_vmcb_control_area(struct vmcb_control_area *dst,
> dst->intercept_dr = from->intercept_dr;
> dst->intercept_exceptions = from->intercept_exceptions;
> dst->intercept = from->intercept;
> + dst->intercept_extended = from->intercept_extended;
> dst->iopm_base_pa = from->iopm_base_pa;
> dst->msrpm_base_pa = from->msrpm_base_pa;
> dst->tsc_offset = from->tsc_offset;
> @@ -433,7 +436,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
> trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr & 0xffff,
> nested_vmcb->control.intercept_cr >> 16,
> nested_vmcb->control.intercept_exceptions,
> - nested_vmcb->control.intercept);
> + nested_vmcb->control.intercept,
> + nested_vmcb->control.intercept_extended);
>
> /* Clear internal status */
> kvm_clear_exception_queue(&svm->vcpu);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 9e333b91ff78..285e5e1ff518 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2801,6 +2801,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
> pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
> pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
> pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
> + pr_err("%-20s%08x\n", "intercepts (extended):", control->intercept_extended);
> pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
> pr_err("%-20s%d\n", "pause filter threshold:",
> control->pause_filter_thresh);
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 6ac4c00a5d82..935d08fac03d 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -311,6 +311,24 @@ static inline void clr_intercept(struct vcpu_svm *svm, int bit)
> recalc_intercepts(svm);
> }
>
> +static inline void set_extended_intercept(struct vcpu_svm *svm, int bit)
> +{
> + struct vmcb *vmcb = get_host_vmcb(svm);
> +
> + vmcb->control.intercept_extended |= (1U << bit);
> +
> + recalc_intercepts(svm);
> +}
> +
> +static inline void clr_extended_intercept(struct vcpu_svm *svm, int bit)
> +{
> + struct vmcb *vmcb = get_host_vmcb(svm);
> +
> + vmcb->control.intercept_extended &= ~(1U << bit);
> +
> + recalc_intercepts(svm);
> +}
> +
> static inline bool is_intercept(struct vcpu_svm *svm, int bit)
> {
> return (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index b66432b015d2..5c841c42b33d 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -544,14 +544,16 @@ 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, __u64 intercept,
> + __u32 extended),
> + TP_ARGS(cr_read, cr_write, exceptions, intercept, extended),
>
> TP_STRUCT__entry(
> __field( __u16, cr_read )
> __field( __u16, cr_write )
> __field( __u32, exceptions )
> __field( __u64, intercept )
> + __field( __u32, extended )
> ),
>
> TP_fast_assign(
> @@ -559,11 +561,13 @@ TRACE_EVENT(kvm_nested_intercepts,
> __entry->cr_write = cr_write;
> __entry->exceptions = exceptions;
> __entry->intercept = intercept;
> + __entry->extended = extended;
> ),
>
> - TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx",
> + TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx"
> + "intercept (extended): %08x",
> __entry->cr_read, __entry->cr_write, __entry->exceptions,
> - __entry->intercept)
> + __entry->intercept, __entry->extended)
Nit: I would've renamed 'extended' to something like 'intercept_ext' as
it is not clear what it is about otherwise. Also, if you decide to do
so, you may as well shorten 'intercept_extended' to 'intercept_ext'
everywhere else to be consistent. Or just use 'intercept_extended', with
no 80-character-per-line limitation we no longer need to be concise.
> );
> /*
> * Tracepoint for #VMEXIT while nested
>
--
Vitaly
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-16 23:17 ` Jim Mattson
@ 2020-06-17 14:30 ` Babu Moger
2020-06-17 18:11 ` Babu Moger
0 siblings, 1 reply; 13+ messages in thread
From: Babu Moger @ 2020-06-17 14:30 UTC (permalink / raw)
To: Jim Mattson
Cc: Wanpeng Li, Joerg Roedel, the arch/x86 maintainers,
Sean Christopherson, Ingo Molnar, Borislav Petkov,
H . Peter Anvin, Paolo Bonzini, Vitaly Kuznetsov,
Thomas Gleixner, LKML, kvm list
> -----Original Message-----
> From: Jim Mattson <jmattson@google.com>
> Sent: Tuesday, June 16, 2020 6:17 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel <joro@8bytes.org>;
> the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>;
> Thomas Gleixner <tglx@linutronix.de>; LKML <linux-kernel@vger.kernel.org>;
> kvm list <kvm@vger.kernel.org>
> Subject: Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
>
> On Tue, Jun 16, 2020 at 3:03 PM Babu Moger <babu.moger@amd.com> wrote:
> >
> > The new intercept bits have been added in vmcb control
> > area to support the interception of INVPCID instruction.
> >
> > The following bit is added to the VMCB layout control area
> > to control intercept of INVPCID:
> >
> > Byte Offset Bit(s) Function
> > 14h 2 intercept INVPCID
> >
> > Add the interfaces to support these extended interception.
> > Also update the tracing for extended intercepts.
> >
> > 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.a
> md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&data=02%7C01%7
> Cbabu.moger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C637279463210520563&s
> data=oRQq0hj0O43A4lnl8JEb%2BHt8oCFHWxcqvLaA1%2BacTJc%3D&reser
> ved=0
> > Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
> kernel.org%2Fshow_bug.cgi%3Fid%3D206537&data=02%7C01%7Cbabu.m
> oger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8961fe48
> 84e608e11a82d994e183d%7C0%7C0%7C637279463210520563&sdata=EtA
> rCUBB8etloN%2B%2Blx42RZqai12QFvtJefnxBn1ryMQ%3D&reserved=0
>
> Not your change, but this documentation is terrible. There is no
> INVLPCID instruction, nor is there a PCID instruction.
Sorry about that. I will bring this to their notice.
>
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > arch/x86/include/asm/svm.h | 3 ++-
> > arch/x86/kvm/svm/nested.c | 6 +++++-
> > arch/x86/kvm/svm/svm.c | 1 +
> > arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
> > arch/x86/kvm/trace.h | 12 ++++++++----
> > 5 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> > index 8a1f5382a4ea..62649fba8908 100644
> > --- a/arch/x86/include/asm/svm.h
> > +++ b/arch/x86/include/asm/svm.h
> > @@ -61,7 +61,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
> > u32 intercept_dr;
> > u32 intercept_exceptions;
> > u64 intercept;
> > - u8 reserved_1[40];
> > + u32 intercept_extended;
> > + u8 reserved_1[36];
>
> It seems like a more straightforward implementation would simply
> change 'u64 intercept' to 'u32 intercept[3].'
Sure. Will change it.
>
> > 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 8a6db11dcb43..7f6d0f2533e2 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -121,6 +121,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> > c->intercept_dr = h->intercept_dr;
> > c->intercept_exceptions = h->intercept_exceptions;
> > c->intercept = h->intercept;
> > + c->intercept_extended = h->intercept_extended;
> >
> > if (g->int_ctl & V_INTR_MASKING_MASK) {
> > /* We only want the cr8 intercept bits of L1 */
> > @@ -142,6 +143,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> > c->intercept_dr |= g->intercept_dr;
> > c->intercept_exceptions |= g->intercept_exceptions;
> > c->intercept |= g->intercept;
> > + c->intercept_extended |= g->intercept_extended;
> > }
> >
> > static void copy_vmcb_control_area(struct vmcb_control_area *dst,
> > @@ -151,6 +153,7 @@ static void copy_vmcb_control_area(struct
> vmcb_control_area *dst,
> > dst->intercept_dr = from->intercept_dr;
> > dst->intercept_exceptions = from->intercept_exceptions;
> > dst->intercept = from->intercept;
> > + dst->intercept_extended = from->intercept_extended;
> > dst->iopm_base_pa = from->iopm_base_pa;
> > dst->msrpm_base_pa = from->msrpm_base_pa;
> > dst->tsc_offset = from->tsc_offset;
> > @@ -433,7 +436,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
> > trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr &
> 0xffff,
> > nested_vmcb->control.intercept_cr >> 16,
> > nested_vmcb->control.intercept_exceptions,
> > - nested_vmcb->control.intercept);
> > + nested_vmcb->control.intercept,
> > + nested_vmcb->control.intercept_extended);
> >
> > /* Clear internal status */
> > kvm_clear_exception_queue(&svm->vcpu);
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 9e333b91ff78..285e5e1ff518 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -2801,6 +2801,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
> > pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
> > pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
> > pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
> > + pr_err("%-20s%08x\n", "intercepts (extended):", control-
> >intercept_extended);
> > pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
> > pr_err("%-20s%d\n", "pause filter threshold:",
> > control->pause_filter_thresh);
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index 6ac4c00a5d82..935d08fac03d 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -311,6 +311,24 @@ static inline void clr_intercept(struct vcpu_svm *svm,
> int bit)
> > recalc_intercepts(svm);
> > }
> >
> > +static inline void set_extended_intercept(struct vcpu_svm *svm, int bit)
> > +{
> > + struct vmcb *vmcb = get_host_vmcb(svm);
> > +
> > + vmcb->control.intercept_extended |= (1U << bit);
> > +
> > + recalc_intercepts(svm);
> > +}
> > +
> > +static inline void clr_extended_intercept(struct vcpu_svm *svm, int bit)
> > +{
> > + struct vmcb *vmcb = get_host_vmcb(svm);
> > +
> > + vmcb->control.intercept_extended &= ~(1U << bit);
> > +
> > + recalc_intercepts(svm);
> > +}
>
> You wouldn't need these new functions if you defined 'u32
> intercept[3],' as I suggested above. Just change set_intercept and
> clr_intercept to use __set_bit and __clear_bit.
Yes. Will change it. Thanks
>
> > static inline bool is_intercept(struct vcpu_svm *svm, int bit)
> > {
> > return (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
> > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> > index b66432b015d2..5c841c42b33d 100644
> > --- a/arch/x86/kvm/trace.h
> > +++ b/arch/x86/kvm/trace.h
> > @@ -544,14 +544,16 @@ 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, __u64
> intercept,
> > + __u32 extended),
> > + TP_ARGS(cr_read, cr_write, exceptions, intercept, extended),
> >
> > TP_STRUCT__entry(
> > __field( __u16, cr_read )
> > __field( __u16, cr_write )
> > __field( __u32, exceptions )
> > __field( __u64, intercept )
> > + __field( __u32, extended )
> > ),
> >
> > TP_fast_assign(
> > @@ -559,11 +561,13 @@ TRACE_EVENT(kvm_nested_intercepts,
> > __entry->cr_write = cr_write;
> > __entry->exceptions = exceptions;
> > __entry->intercept = intercept;
> > + __entry->extended = extended;
> > ),
> >
> > - TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx",
> > + TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx"
> > + "intercept (extended): %08x",
> > __entry->cr_read, __entry->cr_write, __entry->exceptions,
> > - __entry->intercept)
> > + __entry->intercept, __entry->extended)
> > );
> > /*
> > * Tracepoint for #VMEXIT while nested
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86
2020-06-17 11:56 ` Vitaly Kuznetsov
@ 2020-06-17 14:31 ` Babu Moger
0 siblings, 0 replies; 13+ messages in thread
From: Babu Moger @ 2020-06-17 14:31 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: linux-kernel, kvm, wanpengli, joro, x86, sean.j.christopherson,
mingo, bp, hpa, pbonzini, tglx, jmattson
> -----Original Message-----
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> Sent: Wednesday, June 17, 2020 6:56 AM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org;
> wanpengli@tencent.com; joro@8bytes.org; x86@kernel.org;
> sean.j.christopherson@intel.com; mingo@redhat.com; bp@alien8.de;
> hpa@zytor.com; pbonzini@redhat.com; tglx@linutronix.de;
> jmattson@google.com
> Subject: Re: [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86
>
> Babu Moger <babu.moger@amd.com> writes:
>
> > 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 | 68 +----------------------------------------
> > arch/x86/kvm/x86.c | 79
> ++++++++++++++++++++++++++++++++++++++++++++++++
> > arch/x86/kvm/x86.h | 3 +-
> > 3 files changed, 82 insertions(+), 68 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 170cc76a581f..b4140cfd15fd 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -5477,11 +5477,7 @@ 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;
> > @@ -5508,69 +5504,7 @@ static int handle_invpcid(struct kvm_vcpu *vcpu)
> > sizeof(operand), &gva))
> > return 1;
> >
> > - if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
> > - kvm_inject_emulated_page_fault(vcpu, &e);
> > - return 1;
> > - }
> > -
> > - 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_types(vcpu, gva, type);
>
> Nit: redundant space.
Sure. Will fix.
>
> > }
> >
> > static int handle_pml_full(struct kvm_vcpu *vcpu)
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 9e41b5135340..9c858ca0e592 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>
> > @@ -10714,6 +10715,84 @@ u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu
> *vcpu)
> > }
> > EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
> >
> > +int kvm_handle_invpcid_types(struct kvm_vcpu *vcpu, gva_t gva,
> > + unsigned long type)
>
> (sorry if this was discussed before) do we really need '_types' suffix?
Ok. I will remove _types.
>
> > +{
> > + unsigned long roots_to_free = 0;
> > + struct x86_exception e;
> > + bool pcid_enabled;
> > + unsigned int i;
> > + struct {
> > + u64 pcid;
> > + u64 gla;
> > + } operand;
> > +
> > + if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {
> > + kvm_inject_emulated_page_fault(vcpu, &e);
> > + return 1;
> > + }
> > +
> > + 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 */
>
> The check was left in VMX' handle_invpcid() so we either need to update
> the comment to something like "the caller was supposed to check that
> type <= 3" or move the check to kvm_handle_invpcid_types().
Ok. Will update the comment. Thanks
>
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(kvm_handle_invpcid_types);
> > +
> > 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..f706f6f7196d 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_types(struct kvm_vcpu *vcpu, gva_t gva,
> > + unsigned long type);
> > #endif
> >
>
> --
> Vitaly
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-17 12:01 ` Vitaly Kuznetsov
@ 2020-06-17 14:31 ` Babu Moger
0 siblings, 0 replies; 13+ messages in thread
From: Babu Moger @ 2020-06-17 14:31 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: linux-kernel, kvm, wanpengli, joro, x86, sean.j.christopherson,
mingo, bp, hpa, pbonzini, tglx, jmattson
> -----Original Message-----
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> Sent: Wednesday, June 17, 2020 7:02 AM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org;
> wanpengli@tencent.com; joro@8bytes.org; x86@kernel.org;
> sean.j.christopherson@intel.com; mingo@redhat.com; bp@alien8.de;
> hpa@zytor.com; pbonzini@redhat.com; tglx@linutronix.de;
> jmattson@google.com
> Subject: Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
>
> Babu Moger <babu.moger@amd.com> writes:
>
> > The new intercept bits have been added in vmcb control
> > area to support the interception of INVPCID instruction.
> >
> > The following bit is added to the VMCB layout control area
> > to control intercept of INVPCID:
> >
> > Byte Offset Bit(s) Function
> > 14h 2 intercept INVPCID
> >
> > Add the interfaces to support these extended interception.
> > Also update the tracing for extended intercepts.
> >
> > 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.a
> md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&data=02%7C01%7
> Cbabu.moger%40amd.com%7Cbacf7250d8b644d984e308d812b63d74%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C637279922105581873&s
> data=%2BGi374uikkiw2c35jk6w%2FYjMnh49R9%2FCw9twf%2BG6i%2FQ%3D&a
> mp;reserved=0
> > Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
> kernel.org%2Fshow_bug.cgi%3Fid%3D206537&data=02%7C01%7Cbabu.m
> oger%40amd.com%7Cbacf7250d8b644d984e308d812b63d74%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637279922105581873&sdata=dMz
> wBL9AfZAGYdqLFN9hHtC3BTTkuJLixFHNBl%2FnJbM%3D&reserved=0
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > arch/x86/include/asm/svm.h | 3 ++-
> > arch/x86/kvm/svm/nested.c | 6 +++++-
> > arch/x86/kvm/svm/svm.c | 1 +
> > arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
> > arch/x86/kvm/trace.h | 12 ++++++++----
> > 5 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> > index 8a1f5382a4ea..62649fba8908 100644
> > --- a/arch/x86/include/asm/svm.h
> > +++ b/arch/x86/include/asm/svm.h
> > @@ -61,7 +61,8 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
> > u32 intercept_dr;
> > u32 intercept_exceptions;
> > u64 intercept;
> > - u8 reserved_1[40];
> > + u32 intercept_extended;
> > + u8 reserved_1[36];
> > 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 8a6db11dcb43..7f6d0f2533e2 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -121,6 +121,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> > c->intercept_dr = h->intercept_dr;
> > c->intercept_exceptions = h->intercept_exceptions;
> > c->intercept = h->intercept;
> > + c->intercept_extended = h->intercept_extended;
> >
> > if (g->int_ctl & V_INTR_MASKING_MASK) {
> > /* We only want the cr8 intercept bits of L1 */
> > @@ -142,6 +143,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> > c->intercept_dr |= g->intercept_dr;
> > c->intercept_exceptions |= g->intercept_exceptions;
> > c->intercept |= g->intercept;
> > + c->intercept_extended |= g->intercept_extended;
> > }
> >
> > static void copy_vmcb_control_area(struct vmcb_control_area *dst,
> > @@ -151,6 +153,7 @@ static void copy_vmcb_control_area(struct
> vmcb_control_area *dst,
> > dst->intercept_dr = from->intercept_dr;
> > dst->intercept_exceptions = from->intercept_exceptions;
> > dst->intercept = from->intercept;
> > + dst->intercept_extended = from->intercept_extended;
> > dst->iopm_base_pa = from->iopm_base_pa;
> > dst->msrpm_base_pa = from->msrpm_base_pa;
> > dst->tsc_offset = from->tsc_offset;
> > @@ -433,7 +436,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
> > trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr &
> 0xffff,
> > nested_vmcb->control.intercept_cr >> 16,
> > nested_vmcb->control.intercept_exceptions,
> > - nested_vmcb->control.intercept);
> > + nested_vmcb->control.intercept,
> > + nested_vmcb->control.intercept_extended);
> >
> > /* Clear internal status */
> > kvm_clear_exception_queue(&svm->vcpu);
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 9e333b91ff78..285e5e1ff518 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -2801,6 +2801,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
> > pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
> > pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
> > pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
> > + pr_err("%-20s%08x\n", "intercepts (extended):", control-
> >intercept_extended);
> > pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
> > pr_err("%-20s%d\n", "pause filter threshold:",
> > control->pause_filter_thresh);
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index 6ac4c00a5d82..935d08fac03d 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -311,6 +311,24 @@ static inline void clr_intercept(struct vcpu_svm *svm,
> int bit)
> > recalc_intercepts(svm);
> > }
> >
> > +static inline void set_extended_intercept(struct vcpu_svm *svm, int bit)
> > +{
> > + struct vmcb *vmcb = get_host_vmcb(svm);
> > +
> > + vmcb->control.intercept_extended |= (1U << bit);
> > +
> > + recalc_intercepts(svm);
> > +}
> > +
> > +static inline void clr_extended_intercept(struct vcpu_svm *svm, int bit)
> > +{
> > + struct vmcb *vmcb = get_host_vmcb(svm);
> > +
> > + vmcb->control.intercept_extended &= ~(1U << bit);
> > +
> > + recalc_intercepts(svm);
> > +}
> > +
> > static inline bool is_intercept(struct vcpu_svm *svm, int bit)
> > {
> > return (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
> > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> > index b66432b015d2..5c841c42b33d 100644
> > --- a/arch/x86/kvm/trace.h
> > +++ b/arch/x86/kvm/trace.h
> > @@ -544,14 +544,16 @@ 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, __u64
> intercept,
> > + __u32 extended),
> > + TP_ARGS(cr_read, cr_write, exceptions, intercept, extended),
> >
> > TP_STRUCT__entry(
> > __field( __u16, cr_read )
> > __field( __u16, cr_write )
> > __field( __u32, exceptions )
> > __field( __u64, intercept )
> > + __field( __u32, extended )
> > ),
> >
> > TP_fast_assign(
> > @@ -559,11 +561,13 @@ TRACE_EVENT(kvm_nested_intercepts,
> > __entry->cr_write = cr_write;
> > __entry->exceptions = exceptions;
> > __entry->intercept = intercept;
> > + __entry->extended = extended;
> > ),
> >
> > - TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept:
> %016llx",
> > + TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept: %016llx"
> > + "intercept (extended): %08x",
> > __entry->cr_read, __entry->cr_write, __entry->exceptions,
> > - __entry->intercept)
> > + __entry->intercept, __entry->extended)
>
> Nit: I would've renamed 'extended' to something like 'intercept_ext' as
> it is not clear what it is about otherwise. Also, if you decide to do
> so, you may as well shorten 'intercept_extended' to 'intercept_ext'
> everywhere else to be consistent. Or just use 'intercept_extended', with
> no 80-character-per-line limitation we no longer need to be concise.
With new suggestion from Jim, we probably don’t need this change. Thanks
>
> > );
> > /*
> > * Tracepoint for #VMEXIT while nested
> >
>
> --
> Vitaly
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-17 14:30 ` Babu Moger
@ 2020-06-17 18:11 ` Babu Moger
2020-06-17 20:44 ` Jim Mattson
0 siblings, 1 reply; 13+ messages in thread
From: Babu Moger @ 2020-06-17 18:11 UTC (permalink / raw)
To: Babu Moger, Jim Mattson
Cc: Wanpeng Li, Joerg Roedel, the arch/x86 maintainers,
Sean Christopherson, Ingo Molnar, Borislav Petkov,
H . Peter Anvin, Paolo Bonzini, Vitaly Kuznetsov,
Thomas Gleixner, LKML, kvm list
Jim,
> -----Original Message-----
> From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf
> Of Babu Moger
> Sent: Wednesday, June 17, 2020 9:31 AM
> To: Jim Mattson <jmattson@google.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel <joro@8bytes.org>;
> the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>;
> Thomas Gleixner <tglx@linutronix.de>; LKML <linux-kernel@vger.kernel.org>;
> kvm list <kvm@vger.kernel.org>
> Subject: RE: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
>
>
>
> > -----Original Message-----
> > From: Jim Mattson <jmattson@google.com>
> > Sent: Tuesday, June 16, 2020 6:17 PM
> > To: Moger, Babu <Babu.Moger@amd.com>
> > Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel <joro@8bytes.org>;
> > the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> > <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> > Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> > Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>;
> > Thomas Gleixner <tglx@linutronix.de>; LKML <linux-kernel@vger.kernel.org>;
> > kvm list <kvm@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
> >
> > On Tue, Jun 16, 2020 at 3:03 PM Babu Moger <babu.moger@amd.com>
> wrote:
> > >
> > > The new intercept bits have been added in vmcb control
> > > area to support the interception of INVPCID instruction.
> > >
> > > The following bit is added to the VMCB layout control area
> > > to control intercept of INVPCID:
> > >
> > > Byte Offset Bit(s) Function
> > > 14h 2 intercept INVPCID
> > >
> > > Add the interfaces to support these extended interception.
> > > Also update the tracing for extended intercepts.
> > >
> > > 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.a
> >
> md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&data=02%7C01%7
> >
> Cbabu.moger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8
> >
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C637279463210520563&s
> >
> data=oRQq0hj0O43A4lnl8JEb%2BHt8oCFHWxcqvLaA1%2BacTJc%3D&reser
> > ved=0
> > > Link:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
> >
> kernel.org%2Fshow_bug.cgi%3Fid%3D206537&data=02%7C01%7Cbabu.m
> >
> oger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8961fe48
> >
> 84e608e11a82d994e183d%7C0%7C0%7C637279463210520563&sdata=EtA
> > rCUBB8etloN%2B%2Blx42RZqai12QFvtJefnxBn1ryMQ%3D&reserved=0
> >
> > Not your change, but this documentation is terrible. There is no
> > INVLPCID instruction, nor is there a PCID instruction.
>
> Sorry about that. I will bring this to their notice.
>
> >
> > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > ---
> > > arch/x86/include/asm/svm.h | 3 ++-
> > > arch/x86/kvm/svm/nested.c | 6 +++++-
> > > arch/x86/kvm/svm/svm.c | 1 +
> > > arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
> > > arch/x86/kvm/trace.h | 12 ++++++++----
> > > 5 files changed, 34 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> > > index 8a1f5382a4ea..62649fba8908 100644
> > > --- a/arch/x86/include/asm/svm.h
> > > +++ b/arch/x86/include/asm/svm.h
> > > @@ -61,7 +61,8 @@ struct __attribute__ ((__packed__))
> vmcb_control_area {
> > > u32 intercept_dr;
> > > u32 intercept_exceptions;
> > > u64 intercept;
> > > - u8 reserved_1[40];
> > > + u32 intercept_extended;
> > > + u8 reserved_1[36];
> >
> > It seems like a more straightforward implementation would simply
> > change 'u64 intercept' to 'u32 intercept[3].'
>
> Sure. Will change it.
This involves much more changes than I originally thought. All these
following code needs to be modified. Here is my cscope output for the C
symbol intercept.
0 nested.c recalc_intercepts 123 c->intercept = h->intercept;
1 nested.c recalc_intercepts 135 c->intercept &= ~(1ULL <<
INTERCEPT_VINTR);
2 nested.c recalc_intercepts 139 c->intercept &= ~(1ULL <<
INTERCEPT_VMMCALL);
3 nested.c recalc_intercepts 144 c->intercept |= g->intercept;
4 nested.c copy_vmcb_control_area 153 dst->intercept =
from->intercept;
5 nested.c nested_svm_vmrun_msrpm 186 if
(!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
6 nested.c nested_vmcb_check_controls 212 if ((control->intercept & (1ULL
<< INTERCEPT_VMRUN)) == 0)NIT));
7 nested.c nested_svm_vmrun 436
nested_vmcb->control.intercept);
8 nested.c nested_svm_exit_handled_msr 648 if
(!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
9 nested.c nested_svm_intercept_ioio 675 if
(!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_IOIO_PROT)))
a nested.c nested_svm_intercept 732 if
(svm->nested.ctl.intercept & exit_bits)
b nested.c nested_exit_on_init 840 return
(svm->nested.ctl.intercept & (1ULL << INTERCEPT_INIT));
c svm.c check_selective_cr0_intercepted 2205 u64 intercept;
d svm.c check_selective_cr0_intercepted 2207 intercept =
svm->nested.ctl.intercept;
e svm.c check_selective_cr0_intercepted 2210 (!(intercept & (1ULL <<
INTERCEPT_SELECTIVE_CR0))))
f svm.c dump_vmcb 2803 pr_err("%-20s%016llx\n",
"intercepts:", control->intercept);
m svm.c svm_check_intercept 3687 intercept =
svm->nested.ctl.intercept;
n svm.c svm_check_intercept 3689 if (!(intercept & (1ULL <<
INTERCEPT_SELECTIVE_CR0)))
6 svm.c svm_apic_init_signal_blocked 3948
(svm->vmcb->control.intercept & (1ULL << INTERCEPT_INIT));
7 svm.h set_intercept 300 vmcb->control.intercept |=
(1ULL << bit);
8 svm.h clr_intercept 309 vmcb->control.intercept &=
~(1ULL << bit);
9 svm.h is_intercept tercept_ioio 316 return
(svm->vmcb->control.intercept & (1ULL << bit)) != 0;
a svm.h nested_exit_on_smi 377 return
(svm->nested.ctl.intercept & (1ULL << INTERCEPT_SMI));
b svm.h nested_exit_on_intr 382 return
(svm->nested.ctl.intercept & (1ULL << INTERCEPT_INTR));
c svm.h nested_exit_on_nmi 387 return
(svm->nested.ctl.intercept & (1ULL << INTERCEPT_NMI));
I will have to test extensively if I go ahead with these changes. What do
you think?
>
> >
> > > 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 8a6db11dcb43..7f6d0f2533e2 100644
> > > --- a/arch/x86/kvm/svm/nested.c
> > > +++ b/arch/x86/kvm/svm/nested.c
> > > @@ -121,6 +121,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> > > c->intercept_dr = h->intercept_dr;
> > > c->intercept_exceptions = h->intercept_exceptions;
> > > c->intercept = h->intercept;
> > > + c->intercept_extended = h->intercept_extended;
> > >
> > > if (g->int_ctl & V_INTR_MASKING_MASK) {
> > > /* We only want the cr8 intercept bits of L1 */
> > > @@ -142,6 +143,7 @@ void recalc_intercepts(struct vcpu_svm *svm)
> > > c->intercept_dr |= g->intercept_dr;
> > > c->intercept_exceptions |= g->intercept_exceptions;
> > > c->intercept |= g->intercept;
> > > + c->intercept_extended |= g->intercept_extended;
> > > }
> > >
> > > static void copy_vmcb_control_area(struct vmcb_control_area *dst,
> > > @@ -151,6 +153,7 @@ static void copy_vmcb_control_area(struct
> > vmcb_control_area *dst,
> > > dst->intercept_dr = from->intercept_dr;
> > > dst->intercept_exceptions = from->intercept_exceptions;
> > > dst->intercept = from->intercept;
> > > + dst->intercept_extended = from->intercept_extended;
> > > dst->iopm_base_pa = from->iopm_base_pa;
> > > dst->msrpm_base_pa = from->msrpm_base_pa;
> > > dst->tsc_offset = from->tsc_offset;
> > > @@ -433,7 +436,8 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
> > > trace_kvm_nested_intercepts(nested_vmcb->control.intercept_cr &
> > 0xffff,
> > > nested_vmcb->control.intercept_cr >> 16,
> > > nested_vmcb->control.intercept_exceptions,
> > > - nested_vmcb->control.intercept);
> > > + nested_vmcb->control.intercept,
> > > + nested_vmcb->control.intercept_extended);
> > >
> > > /* Clear internal status */
> > > kvm_clear_exception_queue(&svm->vcpu);
> > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > > index 9e333b91ff78..285e5e1ff518 100644
> > > --- a/arch/x86/kvm/svm/svm.c
> > > +++ b/arch/x86/kvm/svm/svm.c
> > > @@ -2801,6 +2801,7 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
> > > pr_err("%-20s%04x\n", "dr_write:", control->intercept_dr >> 16);
> > > pr_err("%-20s%08x\n", "exceptions:", control->intercept_exceptions);
> > > pr_err("%-20s%016llx\n", "intercepts:", control->intercept);
> > > + pr_err("%-20s%08x\n", "intercepts (extended):", control-
> > >intercept_extended);
> > > pr_err("%-20s%d\n", "pause filter count:", control->pause_filter_count);
> > > pr_err("%-20s%d\n", "pause filter threshold:",
> > > control->pause_filter_thresh);
> > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > > index 6ac4c00a5d82..935d08fac03d 100644
> > > --- a/arch/x86/kvm/svm/svm.h
> > > +++ b/arch/x86/kvm/svm/svm.h
> > > @@ -311,6 +311,24 @@ static inline void clr_intercept(struct vcpu_svm
> *svm,
> > int bit)
> > > recalc_intercepts(svm);
> > > }
> > >
> > > +static inline void set_extended_intercept(struct vcpu_svm *svm, int bit)
> > > +{
> > > + struct vmcb *vmcb = get_host_vmcb(svm);
> > > +
> > > + vmcb->control.intercept_extended |= (1U << bit);
> > > +
> > > + recalc_intercepts(svm);
> > > +}
> > > +
> > > +static inline void clr_extended_intercept(struct vcpu_svm *svm, int bit)
> > > +{
> > > + struct vmcb *vmcb = get_host_vmcb(svm);
> > > +
> > > + vmcb->control.intercept_extended &= ~(1U << bit);
> > > +
> > > + recalc_intercepts(svm);
> > > +}
> >
> > You wouldn't need these new functions if you defined 'u32
> > intercept[3],' as I suggested above. Just change set_intercept and
> > clr_intercept to use __set_bit and __clear_bit.
>
> Yes. Will change it. Thanks
>
> >
> > > static inline bool is_intercept(struct vcpu_svm *svm, int bit)
> > > {
> > > return (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
> > > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> > > index b66432b015d2..5c841c42b33d 100644
> > > --- a/arch/x86/kvm/trace.h
> > > +++ b/arch/x86/kvm/trace.h
> > > @@ -544,14 +544,16 @@ 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, __u64
> > intercept,
> > > + __u32 extended),
> > > + TP_ARGS(cr_read, cr_write, exceptions, intercept, extended),
> > >
> > > TP_STRUCT__entry(
> > > __field( __u16, cr_read )
> > > __field( __u16, cr_write )
> > > __field( __u32, exceptions )
> > > __field( __u64, intercept )
> > > + __field( __u32, extended )
> > > ),
> > >
> > > TP_fast_assign(
> > > @@ -559,11 +561,13 @@ TRACE_EVENT(kvm_nested_intercepts,
> > > __entry->cr_write = cr_write;
> > > __entry->exceptions = exceptions;
> > > __entry->intercept = intercept;
> > > + __entry->extended = extended;
> > > ),
> > >
> > > - TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept:
> %016llx",
> > > + TP_printk("cr_read: %04x cr_write: %04x excp: %08x intercept:
> %016llx"
> > > + "intercept (extended): %08x",
> > > __entry->cr_read, __entry->cr_write, __entry->exceptions,
> > > - __entry->intercept)
> > > + __entry->intercept, __entry->extended)
> > > );
> > > /*
> > > * Tracepoint for #VMEXIT while nested
> > >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-17 18:11 ` Babu Moger
@ 2020-06-17 20:44 ` Jim Mattson
2020-06-17 21:42 ` Babu Moger
0 siblings, 1 reply; 13+ messages in thread
From: Jim Mattson @ 2020-06-17 20:44 UTC (permalink / raw)
To: Babu Moger
Cc: Wanpeng Li, Joerg Roedel, the arch/x86 maintainers,
Sean Christopherson, Ingo Molnar, Borislav Petkov,
H . Peter Anvin, Paolo Bonzini, Vitaly Kuznetsov,
Thomas Gleixner, LKML, kvm list
On Wed, Jun 17, 2020 at 11:11 AM Babu Moger <babu.moger@amd.com> wrote:
>
> Jim,
>
> > -----Original Message-----
> > From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf
> > Of Babu Moger
> > Sent: Wednesday, June 17, 2020 9:31 AM
> > To: Jim Mattson <jmattson@google.com>
> > Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel <joro@8bytes.org>;
> > the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> > <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> > Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> > Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>;
> > Thomas Gleixner <tglx@linutronix.de>; LKML <linux-kernel@vger.kernel.org>;
> > kvm list <kvm@vger.kernel.org>
> > Subject: RE: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
> >
> >
> >
> > > -----Original Message-----
> > > From: Jim Mattson <jmattson@google.com>
> > > Sent: Tuesday, June 16, 2020 6:17 PM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel <joro@8bytes.org>;
> > > the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> > > <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> > > Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> > > Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>;
> > > Thomas Gleixner <tglx@linutronix.de>; LKML <linux-kernel@vger.kernel.org>;
> > > kvm list <kvm@vger.kernel.org>
> > > Subject: Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
> > >
> > > On Tue, Jun 16, 2020 at 3:03 PM Babu Moger <babu.moger@amd.com>
> > wrote:
> > > >
> > > > The new intercept bits have been added in vmcb control
> > > > area to support the interception of INVPCID instruction.
> > > >
> > > > The following bit is added to the VMCB layout control area
> > > > to control intercept of INVPCID:
> > > >
> > > > Byte Offset Bit(s) Function
> > > > 14h 2 intercept INVPCID
> > > >
> > > > Add the interfaces to support these extended interception.
> > > > Also update the tracing for extended intercepts.
> > > >
> > > > 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.a
> > >
> > md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&data=02%7C01%7
> > >
> > Cbabu.moger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8
> > >
> > 961fe4884e608e11a82d994e183d%7C0%7C0%7C637279463210520563&s
> > >
> > data=oRQq0hj0O43A4lnl8JEb%2BHt8oCFHWxcqvLaA1%2BacTJc%3D&reser
> > > ved=0
> > > > Link:
> > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
> > >
> > kernel.org%2Fshow_bug.cgi%3Fid%3D206537&data=02%7C01%7Cbabu.m
> > >
> > oger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8961fe48
> > >
> > 84e608e11a82d994e183d%7C0%7C0%7C637279463210520563&sdata=EtA
> > > rCUBB8etloN%2B%2Blx42RZqai12QFvtJefnxBn1ryMQ%3D&reserved=0
> > >
> > > Not your change, but this documentation is terrible. There is no
> > > INVLPCID instruction, nor is there a PCID instruction.
> >
> > Sorry about that. I will bring this to their notice.
> >
> > >
> > > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > > ---
> > > > arch/x86/include/asm/svm.h | 3 ++-
> > > > arch/x86/kvm/svm/nested.c | 6 +++++-
> > > > arch/x86/kvm/svm/svm.c | 1 +
> > > > arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
> > > > arch/x86/kvm/trace.h | 12 ++++++++----
> > > > 5 files changed, 34 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> > > > index 8a1f5382a4ea..62649fba8908 100644
> > > > --- a/arch/x86/include/asm/svm.h
> > > > +++ b/arch/x86/include/asm/svm.h
> > > > @@ -61,7 +61,8 @@ struct __attribute__ ((__packed__))
> > vmcb_control_area {
> > > > u32 intercept_dr;
> > > > u32 intercept_exceptions;
> > > > u64 intercept;
> > > > - u8 reserved_1[40];
> > > > + u32 intercept_extended;
> > > > + u8 reserved_1[36];
> > >
> > > It seems like a more straightforward implementation would simply
> > > change 'u64 intercept' to 'u32 intercept[3].'
> >
> > Sure. Will change it.
>
> This involves much more changes than I originally thought. All these
> following code needs to be modified. Here is my cscope output for the C
> symbol intercept.
>
> 0 nested.c recalc_intercepts 123 c->intercept = h->intercept;
> 1 nested.c recalc_intercepts 135 c->intercept &= ~(1ULL <<
> INTERCEPT_VINTR);
> 2 nested.c recalc_intercepts 139 c->intercept &= ~(1ULL <<
> INTERCEPT_VMMCALL);
> 3 nested.c recalc_intercepts 144 c->intercept |= g->intercept;
> 4 nested.c copy_vmcb_control_area 153 dst->intercept =
> from->intercept;
> 5 nested.c nested_svm_vmrun_msrpm 186 if
> (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
> 6 nested.c nested_vmcb_check_controls 212 if ((control->intercept & (1ULL
> << INTERCEPT_VMRUN)) == 0)NIT));
> 7 nested.c nested_svm_vmrun 436
> nested_vmcb->control.intercept);
> 8 nested.c nested_svm_exit_handled_msr 648 if
> (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
> 9 nested.c nested_svm_intercept_ioio 675 if
> (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_IOIO_PROT)))
> a nested.c nested_svm_intercept 732 if
> (svm->nested.ctl.intercept & exit_bits)
> b nested.c nested_exit_on_init 840 return
> (svm->nested.ctl.intercept & (1ULL << INTERCEPT_INIT));
> c svm.c check_selective_cr0_intercepted 2205 u64 intercept;
> d svm.c check_selective_cr0_intercepted 2207 intercept =
> svm->nested.ctl.intercept;
> e svm.c check_selective_cr0_intercepted 2210 (!(intercept & (1ULL <<
> INTERCEPT_SELECTIVE_CR0))))
> f svm.c dump_vmcb 2803 pr_err("%-20s%016llx\n",
> "intercepts:", control->intercept);
> m svm.c svm_check_intercept 3687 intercept =
> svm->nested.ctl.intercept;
> n svm.c svm_check_intercept 3689 if (!(intercept & (1ULL <<
> INTERCEPT_SELECTIVE_CR0)))
> 6 svm.c svm_apic_init_signal_blocked 3948
> (svm->vmcb->control.intercept & (1ULL << INTERCEPT_INIT));
> 7 svm.h set_intercept 300 vmcb->control.intercept |=
> (1ULL << bit);
> 8 svm.h clr_intercept 309 vmcb->control.intercept &=
> ~(1ULL << bit);
> 9 svm.h is_intercept tercept_ioio 316 return
> (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
> a svm.h nested_exit_on_smi 377 return
> (svm->nested.ctl.intercept & (1ULL << INTERCEPT_SMI));
> b svm.h nested_exit_on_intr 382 return
> (svm->nested.ctl.intercept & (1ULL << INTERCEPT_INTR));
> c svm.h nested_exit_on_nmi 387 return
> (svm->nested.ctl.intercept & (1ULL << INTERCEPT_NMI));
>
> I will have to test extensively if I go ahead with these changes. What do
> you think?
I see a lot of open-coding of the nested version of is_intercept(),
which would be a good preparatory cleanup. It also looks like it
might be useful to introduce __set_intercept() and __clr_intercept()
which do the same thing as set_intercept() and clr_intercept(),
without calling recalc_intercepts(), for use *in* recalc_intercepts.
This code needs a little love. While your original proposal is more
expedient, taking the time to fix up the existing mess will be more
beneficial in the long run.
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
2020-06-17 20:44 ` Jim Mattson
@ 2020-06-17 21:42 ` Babu Moger
0 siblings, 0 replies; 13+ messages in thread
From: Babu Moger @ 2020-06-17 21:42 UTC (permalink / raw)
To: Jim Mattson
Cc: Wanpeng Li, Joerg Roedel, the arch/x86 maintainers,
Sean Christopherson, Ingo Molnar, Borislav Petkov,
H . Peter Anvin, Paolo Bonzini, Vitaly Kuznetsov,
Thomas Gleixner, LKML, kvm list
> -----Original Message-----
> From: Jim Mattson <jmattson@google.com>
> Sent: Wednesday, June 17, 2020 3:45 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel <joro@8bytes.org>;
> the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>;
> Thomas Gleixner <tglx@linutronix.de>; LKML <linux-kernel@vger.kernel.org>;
> kvm list <kvm@vger.kernel.org>
> Subject: Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
>
> On Wed, Jun 17, 2020 at 11:11 AM Babu Moger <babu.moger@amd.com>
> wrote:
> >
> > Jim,
> >
> > > -----Original Message-----
> > > From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On
> Behalf
> > > Of Babu Moger
> > > Sent: Wednesday, June 17, 2020 9:31 AM
> > > To: Jim Mattson <jmattson@google.com>
> > > Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel
> <joro@8bytes.org>;
> > > the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> > > <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> > > Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> > > Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>;
> > > Thomas Gleixner <tglx@linutronix.de>; LKML <linux-
> kernel@vger.kernel.org>;
> > > kvm list <kvm@vger.kernel.org>
> > > Subject: RE: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jim Mattson <jmattson@google.com>
> > > > Sent: Tuesday, June 16, 2020 6:17 PM
> > > > To: Moger, Babu <Babu.Moger@amd.com>
> > > > Cc: Wanpeng Li <wanpengli@tencent.com>; Joerg Roedel
> <joro@8bytes.org>;
> > > > the arch/x86 maintainers <x86@kernel.org>; Sean Christopherson
> > > > <sean.j.christopherson@intel.com>; Ingo Molnar <mingo@redhat.com>;
> > > > Borislav Petkov <bp@alien8.de>; H . Peter Anvin <hpa@zytor.com>; Paolo
> > > > Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>;
> > > > Thomas Gleixner <tglx@linutronix.de>; LKML <linux-
> kernel@vger.kernel.org>;
> > > > kvm list <kvm@vger.kernel.org>
> > > > Subject: Re: [PATCH v2 2/3] KVM:SVM: Add extended intercept support
> > > >
> > > > On Tue, Jun 16, 2020 at 3:03 PM Babu Moger <babu.moger@amd.com>
> > > wrote:
> > > > >
> > > > > The new intercept bits have been added in vmcb control
> > > > > area to support the interception of INVPCID instruction.
> > > > >
> > > > > The following bit is added to the VMCB layout control area
> > > > > to control intercept of INVPCID:
> > > > >
> > > > > Byte Offset Bit(s) Function
> > > > > 14h 2 intercept INVPCID
> > > > >
> > > > > Add the interfaces to support these extended interception.
> > > > > Also update the tracing for extended intercepts.
> > > > >
> > > > > 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.a
> > > >
> > >
> md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&data=02%7C01%7
> > > >
> > >
> Cbabu.moger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8
> > > >
> > >
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C637279463210520563&s
> > > >
> > >
> data=oRQq0hj0O43A4lnl8JEb%2BHt8oCFHWxcqvLaA1%2BacTJc%3D&reser
> > > > ved=0
> > > > > Link:
> > > >
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
> > > >
> > >
> kernel.org%2Fshow_bug.cgi%3Fid%3D206537&data=02%7C01%7Cbabu.m
> > > >
> > >
> oger%40amd.com%7C4cedcb3567194883601e08d8124b6be7%7C3dd8961fe48
> > > >
> > >
> 84e608e11a82d994e183d%7C0%7C0%7C637279463210520563&sdata=EtA
> > > >
> rCUBB8etloN%2B%2Blx42RZqai12QFvtJefnxBn1ryMQ%3D&reserved=0
> > > >
> > > > Not your change, but this documentation is terrible. There is no
> > > > INVLPCID instruction, nor is there a PCID instruction.
> > >
> > > Sorry about that. I will bring this to their notice.
> > >
> > > >
> > > > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > > > ---
> > > > > arch/x86/include/asm/svm.h | 3 ++-
> > > > > arch/x86/kvm/svm/nested.c | 6 +++++-
> > > > > arch/x86/kvm/svm/svm.c | 1 +
> > > > > arch/x86/kvm/svm/svm.h | 18 ++++++++++++++++++
> > > > > arch/x86/kvm/trace.h | 12 ++++++++----
> > > > > 5 files changed, 34 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> > > > > index 8a1f5382a4ea..62649fba8908 100644
> > > > > --- a/arch/x86/include/asm/svm.h
> > > > > +++ b/arch/x86/include/asm/svm.h
> > > > > @@ -61,7 +61,8 @@ struct __attribute__ ((__packed__))
> > > vmcb_control_area {
> > > > > u32 intercept_dr;
> > > > > u32 intercept_exceptions;
> > > > > u64 intercept;
> > > > > - u8 reserved_1[40];
> > > > > + u32 intercept_extended;
> > > > > + u8 reserved_1[36];
> > > >
> > > > It seems like a more straightforward implementation would simply
> > > > change 'u64 intercept' to 'u32 intercept[3].'
> > >
> > > Sure. Will change it.
> >
> > This involves much more changes than I originally thought. All these
> > following code needs to be modified. Here is my cscope output for the C
> > symbol intercept.
> >
> > 0 nested.c recalc_intercepts 123 c->intercept = h->intercept;
> > 1 nested.c recalc_intercepts 135 c->intercept &= ~(1ULL <<
> > INTERCEPT_VINTR);
> > 2 nested.c recalc_intercepts 139 c->intercept &= ~(1ULL <<
> > INTERCEPT_VMMCALL);
> > 3 nested.c recalc_intercepts 144 c->intercept |= g->intercept;
> > 4 nested.c copy_vmcb_control_area 153 dst->intercept =
> > from->intercept;
> > 5 nested.c nested_svm_vmrun_msrpm 186 if
> > (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
> > 6 nested.c nested_vmcb_check_controls 212 if ((control->intercept & (1ULL
> > << INTERCEPT_VMRUN)) == 0)NIT));
> > 7 nested.c nested_svm_vmrun 436
> > nested_vmcb->control.intercept);
> > 8 nested.c nested_svm_exit_handled_msr 648 if
> > (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_MSR_PROT)))
> > 9 nested.c nested_svm_intercept_ioio 675 if
> > (!(svm->nested.ctl.intercept & (1ULL << INTERCEPT_IOIO_PROT)))
> > a nested.c nested_svm_intercept 732 if
> > (svm->nested.ctl.intercept & exit_bits)
> > b nested.c nested_exit_on_init 840 return
> > (svm->nested.ctl.intercept & (1ULL << INTERCEPT_INIT));
> > c svm.c check_selective_cr0_intercepted 2205 u64 intercept;
> > d svm.c check_selective_cr0_intercepted 2207 intercept =
> > svm->nested.ctl.intercept;
> > e svm.c check_selective_cr0_intercepted 2210 (!(intercept & (1ULL <<
> > INTERCEPT_SELECTIVE_CR0))))
> > f svm.c dump_vmcb 2803 pr_err("%-20s%016llx\n",
> > "intercepts:", control->intercept);
> > m svm.c svm_check_intercept 3687 intercept =
> > svm->nested.ctl.intercept;
> > n svm.c svm_check_intercept 3689 if (!(intercept & (1ULL <<
> > INTERCEPT_SELECTIVE_CR0)))
> > 6 svm.c svm_apic_init_signal_blocked 3948
> > (svm->vmcb->control.intercept & (1ULL << INTERCEPT_INIT));
> > 7 svm.h set_intercept 300 vmcb->control.intercept |=
> > (1ULL << bit);
> > 8 svm.h clr_intercept 309 vmcb->control.intercept &=
> > ~(1ULL << bit);
> > 9 svm.h is_intercept tercept_ioio 316 return
> > (svm->vmcb->control.intercept & (1ULL << bit)) != 0;
> > a svm.h nested_exit_on_smi 377 return
> > (svm->nested.ctl.intercept & (1ULL << INTERCEPT_SMI));
> > b svm.h nested_exit_on_intr 382 return
> > (svm->nested.ctl.intercept & (1ULL << INTERCEPT_INTR));
> > c svm.h nested_exit_on_nmi 387 return
> > (svm->nested.ctl.intercept & (1ULL << INTERCEPT_NMI));
> >
> > I will have to test extensively if I go ahead with these changes. What do
> > you think?
>
> I see a lot of open-coding of the nested version of is_intercept(),
> which would be a good preparatory cleanup. It also looks like it
> might be useful to introduce __set_intercept() and __clr_intercept()
> which do the same thing as set_intercept() and clr_intercept(),
> without calling recalc_intercepts(), for use *in* recalc_intercepts.
> This code needs a little love. While your original proposal is more
> expedient, taking the time to fix up the existing mess will be more
> beneficial in the long run.
Ok. Sounds good. Will start working on it.Thanks
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-06-17 21:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 22:03 [PATCH v2 0/3] INVPCID support for the AMD guests Babu Moger
2020-06-16 22:03 ` [PATCH v2 1/3] KVM: X86: Move handling of INVPCID types to x86 Babu Moger
2020-06-17 11:56 ` Vitaly Kuznetsov
2020-06-17 14:31 ` Babu Moger
2020-06-16 22:03 ` [PATCH v2 2/3] KVM:SVM: Add extended intercept support Babu Moger
2020-06-16 23:17 ` Jim Mattson
2020-06-17 14:30 ` Babu Moger
2020-06-17 18:11 ` Babu Moger
2020-06-17 20:44 ` Jim Mattson
2020-06-17 21:42 ` Babu Moger
2020-06-17 12:01 ` Vitaly Kuznetsov
2020-06-17 14:31 ` Babu Moger
2020-06-16 22:03 ` [PATCH v2 3/3] KVM:SVM: Enable INVPCID feature on AMD Babu Moger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.