On Mon, Sep 04, 2023 at 09:53:37AM +0000, Manali Shukla wrote: >From: Santosh Shukla <santosh.shukla@xxxxxxx> > >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@xxxxxxx> >Co-developed-by: Manali Shukla <manali.shukla@xxxxxxx> >Signed-off-by: Manali Shukla <manali.shukla@xxxxxxx> >--- > 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 >