diff mbox series

[03/13] KVM: x86: Add emulation support for Extented LVT registers

Message ID 20230904095347.14994-4-manali.shukla@amd.com (mailing list archive)
State New, archived
Headers show
Series Implement support for IBS virtualization | expand

Commit Message

Manali Shukla Sept. 4, 2023, 9:53 a.m. UTC
From: Santosh Shukla <santosh.shukla@amd.com>

The local interrupts are extended to include more LVT registers in
order to allow additional interrupt sources, like Instruction Based
Sampling (IBS) and many more.

Currently there are four additional LVT registers defined and they are
located at APIC offsets 500h-530h.

AMD IBS driver is designed to use EXTLVT (Extended interrupt local
vector table) by default for driver initialization.

Extended LVT registers are required to be emulated to initialize the
guest IBS driver successfully.

Please refer to Section 16.4.5 in AMD Programmer's Manual Volume 2 at
https://bugzilla.kernel.org/attachment.cgi?id=304653 for more details
on EXTLVT.

Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
Co-developed-by: Manali Shukla <manali.shukla@amd.com>
Signed-off-by: Manali Shukla <manali.shukla@amd.com>
---
 arch/x86/include/asm/apicdef.h | 14 ++++++++
 arch/x86/kvm/lapic.c           | 66 ++++++++++++++++++++++++++++++++--
 arch/x86/kvm/lapic.h           |  1 +
 arch/x86/kvm/svm/avic.c        |  4 +++
 4 files changed, 83 insertions(+), 2 deletions(-)

Comments

Chao Gao Sept. 12, 2023, 2:36 a.m. UTC | #1
On Mon, Sep 04, 2023 at 09:53:37AM +0000, Manali Shukla wrote:
>From: Santosh Shukla <santosh.shukla@amd.com>
>
>The local interrupts are extended to include more LVT registers in
>order to allow additional interrupt sources, like Instruction Based
>Sampling (IBS) and many more.
>
>Currently there are four additional LVT registers defined and they are
>located at APIC offsets 500h-530h.
>
>AMD IBS driver is designed to use EXTLVT (Extended interrupt local
>vector table) by default for driver initialization.
>
>Extended LVT registers are required to be emulated to initialize the
>guest IBS driver successfully.
>
>Please refer to Section 16.4.5 in AMD Programmer's Manual Volume 2 at
>https://bugzilla.kernel.org/attachment.cgi?id=304653 for more details
>on EXTLVT.
>
>Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
>Co-developed-by: Manali Shukla <manali.shukla@amd.com>
>Signed-off-by: Manali Shukla <manali.shukla@amd.com>
>---
> arch/x86/include/asm/apicdef.h | 14 ++++++++
> arch/x86/kvm/lapic.c           | 66 ++++++++++++++++++++++++++++++++--
> arch/x86/kvm/lapic.h           |  1 +
> arch/x86/kvm/svm/avic.c        |  4 +++
> 4 files changed, 83 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
>index 4b125e5b3187..ac50919d10be 100644
>--- a/arch/x86/include/asm/apicdef.h
>+++ b/arch/x86/include/asm/apicdef.h
>@@ -139,6 +139,20 @@
> #define		APIC_EILVT_MSG_EXT	0x7
> #define		APIC_EILVT_MASKED	(1 << 16)
> 
>+/*
>+ * Initialize extended APIC registers to the default value when guest is started
>+ * and EXTAPIC feature is enabled on the guest.
>+ *
>+ * APIC_EFEAT is a read only Extended APIC feature register, whose default value
>+ * is 0x00040007.

bits 0/1/2 indicate other features. If KVM sets them to 1s, KVM needs to
enumerate the corresponding features.

>+ *
>+ * APIC_ECTRL is a read-write Extended APIC control register, whose default value
>+ * is 0x0.
>+ */
>+
>+#define		APIC_EFEAT_DEFAULT	0x00040007
>+#define		APIC_ECTRL_DEFAULT	0x0
>+
> #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
> #define APIC_BASE_MSR		0x800
> #define APIC_X2APIC_ID_MSR	0x802
>diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>index 7c1bd8594f1b..88985c481fe8 100644
>--- a/arch/x86/kvm/lapic.c
>+++ b/arch/x86/kvm/lapic.c
>@@ -1599,9 +1599,13 @@ static inline struct kvm_lapic *to_lapic(struct kvm_io_device *dev)
> }
> 
> #define APIC_REG_MASK(reg)	(1ull << ((reg) >> 4))
>+#define APIC_REG_EXT_MASK(reg)	(1ull << (((reg) >> 4) - 0x40))
> #define APIC_REGS_MASK(first, count) \
> 	(APIC_REG_MASK(first) * ((1ull << (count)) - 1))
> 
>+#define APIC_LAST_REG_OFFSET		0x3f0
>+#define APIC_EXT_LAST_REG_OFFSET	0x530
>+
> u64 kvm_lapic_readable_reg_mask(struct kvm_lapic *apic)
> {
> 	/* Leave bits '0' for reserved and write-only registers. */
>@@ -1643,6 +1647,8 @@ EXPORT_SYMBOL_GPL(kvm_lapic_readable_reg_mask);
> static int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
> 			      void *data)
> {
>+	u64 valid_reg_ext_mask = 0;
>+	unsigned int last_reg = APIC_LAST_REG_OFFSET;
> 	unsigned char alignment = offset & 0xf;
> 	u32 result;
> 
>@@ -1652,13 +1658,44 @@ static int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
> 	 */
> 	WARN_ON_ONCE(apic_x2apic_mode(apic) && offset == APIC_ICR);
> 
>+	/*
>+	 * The local interrupts are extended to include LVT registers to allow
>+	 * additional interrupt sources when the EXTAPIC feature bit is enabled.
>+	 * The Extended Interrupt LVT registers are located at APIC offsets 400-530h.
>+	 */
>+	if (guest_cpuid_has(apic->vcpu, X86_FEATURE_EXTAPIC)) {
>+		valid_reg_ext_mask =
>+			APIC_REG_EXT_MASK(APIC_EFEAT) |
>+			APIC_REG_EXT_MASK(APIC_ECTRL) |
>+			APIC_REG_EXT_MASK(APIC_EILVTn(0)) |
>+			APIC_REG_EXT_MASK(APIC_EILVTn(1)) |
>+			APIC_REG_EXT_MASK(APIC_EILVTn(2)) |
>+			APIC_REG_EXT_MASK(APIC_EILVTn(3));
>+		last_reg = APIC_EXT_LAST_REG_OFFSET;
>+	}
>+
> 	if (alignment + len > 4)
> 		return 1;
> 
>-	if (offset > 0x3f0 ||
>-	    !(kvm_lapic_readable_reg_mask(apic) & APIC_REG_MASK(offset)))
>+	if (offset > last_reg)
> 		return 1;
> 
>+	switch (offset) {
>+	/*
>+	 * Section 16.3.2 in the AMD Programmer's Manual Volume 2 states:
>+	 * "APIC registers are aligned to 16-byte offsets and must be accessed
>+	 * using naturally-aligned DWORD size read and writes."
>+	 */
>+	case KVM_APIC_REG_SIZE ... KVM_APIC_EXT_REG_SIZE - 16:
>+		if (!(valid_reg_ext_mask & APIC_REG_EXT_MASK(offset)))
>+			return 1;
>+		break;
>+	default:
>+		if (!(kvm_lapic_readable_reg_mask(apic) & APIC_REG_MASK(offset)))
>+			return 1;
>+
>+	}
>+
> 	result = __apic_read(apic, offset & ~0xf);
> 
> 	trace_kvm_apic_read(offset, result);
>@@ -2386,6 +2423,12 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
> 		else
> 			kvm_apic_send_ipi(apic, APIC_DEST_SELF | val, 0);
> 		break;
>+	case APIC_EILVTn(0):
>+	case APIC_EILVTn(1):
>+	case APIC_EILVTn(2):
>+	case APIC_EILVTn(3):
>+		kvm_lapic_set_reg(apic, reg, val);
>+		break;

APIC_ECTRL is writable, so, I think it should be handled here.

> 	default:
> 		ret = 1;
> 		break;
>@@ -2664,6 +2707,25 @@ void kvm_inhibit_apic_access_page(struct kvm_vcpu *vcpu)
> 	kvm_vcpu_srcu_read_lock(vcpu);
> }
> 
>+/*
>+ * Initialize extended APIC registers to the default value when guest is
>+ * started. The extended APIC registers should only be initialized when the
>+ * EXTAPIC feature is enabled on the guest.
>+ */
>+void kvm_apic_init_eilvt_regs(struct kvm_vcpu *vcpu)
>+{
>+	struct kvm_lapic *apic = vcpu->arch.apic;
>+	int i;
>+
>+	if (guest_cpuid_has(vcpu, X86_FEATURE_EXTAPIC)) {

Do you need to check hardware support here?

>+		kvm_lapic_set_reg(apic, APIC_EFEAT, APIC_EFEAT_DEFAULT);
>+		kvm_lapic_set_reg(apic, APIC_ECTRL, APIC_ECTRL_DEFAULT);
>+		for (i = 0; i < APIC_EILVT_NR_MAX; i++)
>+			kvm_lapic_set_reg(apic, APIC_EILVTn(i), APIC_EILVT_MASKED);
>+	}
>+}
>+EXPORT_SYMBOL_GPL(kvm_apic_init_eilvt_regs);

looks Extended APIC space is generic to x86. Can you just call this function
from kvm_vcpu_after_set_cpuid()?  then there is no need to expose this function.

>+
> void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)

The extended APIC space should be re-initialized on reset. Right?

> {
> 	struct kvm_lapic *apic = vcpu->arch.apic;
>diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
>index ad6c48938733..b0c7393cd6af 100644
>--- a/arch/x86/kvm/lapic.h
>+++ b/arch/x86/kvm/lapic.h
>@@ -93,6 +93,7 @@ int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu);
> int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu);
> int kvm_apic_accept_events(struct kvm_vcpu *vcpu);
> void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event);
>+void kvm_apic_init_eilvt_regs(struct kvm_vcpu *vcpu);
> u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu);
> void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8);
> void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu);
>diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
>index cfc8ab773025..081075674b1d 100644
>--- a/arch/x86/kvm/svm/avic.c
>+++ b/arch/x86/kvm/svm/avic.c
>@@ -679,6 +679,10 @@ static bool is_avic_unaccelerated_access_trap(u32 offset)
> 	case APIC_LVTERR:
> 	case APIC_TMICT:
> 	case APIC_TDCR:
>+	case APIC_EILVTn(0):
>+	case APIC_EILVTn(1):
>+	case APIC_EILVTn(2):
>+	case APIC_EILVTn(3):
> 		ret = true;
> 		break;
> 	default:
>-- 
>2.34.1
>
diff mbox series

Patch

diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index 4b125e5b3187..ac50919d10be 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -139,6 +139,20 @@ 
 #define		APIC_EILVT_MSG_EXT	0x7
 #define		APIC_EILVT_MASKED	(1 << 16)
 
+/*
+ * Initialize extended APIC registers to the default value when guest is started
+ * and EXTAPIC feature is enabled on the guest.
+ *
+ * APIC_EFEAT is a read only Extended APIC feature register, whose default value
+ * is 0x00040007.
+ *
+ * APIC_ECTRL is a read-write Extended APIC control register, whose default value
+ * is 0x0.
+ */
+
+#define		APIC_EFEAT_DEFAULT	0x00040007
+#define		APIC_ECTRL_DEFAULT	0x0
+
 #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
 #define APIC_BASE_MSR		0x800
 #define APIC_X2APIC_ID_MSR	0x802
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 7c1bd8594f1b..88985c481fe8 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1599,9 +1599,13 @@  static inline struct kvm_lapic *to_lapic(struct kvm_io_device *dev)
 }
 
 #define APIC_REG_MASK(reg)	(1ull << ((reg) >> 4))
+#define APIC_REG_EXT_MASK(reg)	(1ull << (((reg) >> 4) - 0x40))
 #define APIC_REGS_MASK(first, count) \
 	(APIC_REG_MASK(first) * ((1ull << (count)) - 1))
 
+#define APIC_LAST_REG_OFFSET		0x3f0
+#define APIC_EXT_LAST_REG_OFFSET	0x530
+
 u64 kvm_lapic_readable_reg_mask(struct kvm_lapic *apic)
 {
 	/* Leave bits '0' for reserved and write-only registers. */
@@ -1643,6 +1647,8 @@  EXPORT_SYMBOL_GPL(kvm_lapic_readable_reg_mask);
 static int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
 			      void *data)
 {
+	u64 valid_reg_ext_mask = 0;
+	unsigned int last_reg = APIC_LAST_REG_OFFSET;
 	unsigned char alignment = offset & 0xf;
 	u32 result;
 
@@ -1652,13 +1658,44 @@  static int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
 	 */
 	WARN_ON_ONCE(apic_x2apic_mode(apic) && offset == APIC_ICR);
 
+	/*
+	 * The local interrupts are extended to include LVT registers to allow
+	 * additional interrupt sources when the EXTAPIC feature bit is enabled.
+	 * The Extended Interrupt LVT registers are located at APIC offsets 400-530h.
+	 */
+	if (guest_cpuid_has(apic->vcpu, X86_FEATURE_EXTAPIC)) {
+		valid_reg_ext_mask =
+			APIC_REG_EXT_MASK(APIC_EFEAT) |
+			APIC_REG_EXT_MASK(APIC_ECTRL) |
+			APIC_REG_EXT_MASK(APIC_EILVTn(0)) |
+			APIC_REG_EXT_MASK(APIC_EILVTn(1)) |
+			APIC_REG_EXT_MASK(APIC_EILVTn(2)) |
+			APIC_REG_EXT_MASK(APIC_EILVTn(3));
+		last_reg = APIC_EXT_LAST_REG_OFFSET;
+	}
+
 	if (alignment + len > 4)
 		return 1;
 
-	if (offset > 0x3f0 ||
-	    !(kvm_lapic_readable_reg_mask(apic) & APIC_REG_MASK(offset)))
+	if (offset > last_reg)
 		return 1;
 
+	switch (offset) {
+	/*
+	 * Section 16.3.2 in the AMD Programmer's Manual Volume 2 states:
+	 * "APIC registers are aligned to 16-byte offsets and must be accessed
+	 * using naturally-aligned DWORD size read and writes."
+	 */
+	case KVM_APIC_REG_SIZE ... KVM_APIC_EXT_REG_SIZE - 16:
+		if (!(valid_reg_ext_mask & APIC_REG_EXT_MASK(offset)))
+			return 1;
+		break;
+	default:
+		if (!(kvm_lapic_readable_reg_mask(apic) & APIC_REG_MASK(offset)))
+			return 1;
+
+	}
+
 	result = __apic_read(apic, offset & ~0xf);
 
 	trace_kvm_apic_read(offset, result);
@@ -2386,6 +2423,12 @@  static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 		else
 			kvm_apic_send_ipi(apic, APIC_DEST_SELF | val, 0);
 		break;
+	case APIC_EILVTn(0):
+	case APIC_EILVTn(1):
+	case APIC_EILVTn(2):
+	case APIC_EILVTn(3):
+		kvm_lapic_set_reg(apic, reg, val);
+		break;
 	default:
 		ret = 1;
 		break;
@@ -2664,6 +2707,25 @@  void kvm_inhibit_apic_access_page(struct kvm_vcpu *vcpu)
 	kvm_vcpu_srcu_read_lock(vcpu);
 }
 
+/*
+ * Initialize extended APIC registers to the default value when guest is
+ * started. The extended APIC registers should only be initialized when the
+ * EXTAPIC feature is enabled on the guest.
+ */
+void kvm_apic_init_eilvt_regs(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+	int i;
+
+	if (guest_cpuid_has(vcpu, X86_FEATURE_EXTAPIC)) {
+		kvm_lapic_set_reg(apic, APIC_EFEAT, APIC_EFEAT_DEFAULT);
+		kvm_lapic_set_reg(apic, APIC_ECTRL, APIC_ECTRL_DEFAULT);
+		for (i = 0; i < APIC_EILVT_NR_MAX; i++)
+			kvm_lapic_set_reg(apic, APIC_EILVTn(i), APIC_EILVT_MASKED);
+	}
+}
+EXPORT_SYMBOL_GPL(kvm_apic_init_eilvt_regs);
+
 void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index ad6c48938733..b0c7393cd6af 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -93,6 +93,7 @@  int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu);
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu);
 int kvm_apic_accept_events(struct kvm_vcpu *vcpu);
 void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event);
+void kvm_apic_init_eilvt_regs(struct kvm_vcpu *vcpu);
 u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu);
 void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8);
 void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index cfc8ab773025..081075674b1d 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -679,6 +679,10 @@  static bool is_avic_unaccelerated_access_trap(u32 offset)
 	case APIC_LVTERR:
 	case APIC_TMICT:
 	case APIC_TDCR:
+	case APIC_EILVTn(0):
+	case APIC_EILVTn(1):
+	case APIC_EILVTn(2):
+	case APIC_EILVTn(3):
 		ret = true;
 		break;
 	default: