From: Chao Gao <chao.gao@intel.com>
To: Manali Shukla <manali.shukla@amd.com>
Cc: <kvm@vger.kernel.org>, <seanjc@google.com>,
<linux-doc@vger.kernel.org>, <linux-perf-users@vger.kernel.org>,
<x86@kernel.org>, <pbonzini@redhat.com>, <peterz@infradead.org>,
<bp@alien8.de>, <santosh.shukla@amd.com>, <ravi.bangoria@amd.com>,
<thomas.lendacky@amd.com>, <nikunj@amd.com>
Subject: Re: [PATCH 03/13] KVM: x86: Add emulation support for Extented LVT registers
Date: Tue, 12 Sep 2023 10:36:31 +0800 [thread overview]
Message-ID: <ZP/Or2UqdA/EIzE7@chao-email> (raw)
In-Reply-To: <20230904095347.14994-4-manali.shukla@amd.com>
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
>
next prev parent reply other threads:[~2023-09-12 3:04 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-04 9:53 [PATCH 00/13] Implement support for IBS virtualization Manali Shukla
2023-09-04 9:53 ` [PATCH 01/13] KVM: Add KVM_GET_LAPIC_W_EXTAPIC and KVM_SET_LAPIC_W_EXTAPIC for extapic Manali Shukla
2023-09-12 1:47 ` Chao Gao
2023-09-04 9:53 ` [PATCH 02/13] x86/cpufeatures: Add CPUID feature bit for Extended LVT Manali Shukla
2023-09-04 9:53 ` [PATCH 03/13] KVM: x86: Add emulation support for Extented LVT registers Manali Shukla
2023-09-12 2:36 ` Chao Gao [this message]
2023-09-04 9:53 ` [PATCH 04/13] x86/cpufeatures: Add CPUID feature bit for virtualized IBS Manali Shukla
2023-09-04 9:53 ` [PATCH 05/13] KVM: x86/cpuid: Add a KVM-only leaf for IBS capabilities Manali Shukla
2023-09-04 9:53 ` [PATCH 06/13] KVM: x86: Extend CPUID range to include new leaf Manali Shukla
2023-09-12 2:46 ` Chao Gao
2023-09-04 9:53 ` [PATCH 07/13] KVM: SVM: Extend VMCB area for virtualized IBS registers Manali Shukla
2023-09-12 2:50 ` Chao Gao
2023-09-04 9:53 ` [PATCH 08/13] perf/x86/amd: Add framework to save/restore host IBS state Manali Shukla
2023-09-05 14:54 ` Tom Lendacky
2023-09-04 9:53 ` [PATCH 09/13] KVM: SVM: add support for IBS virtualization for non SEV-ES guests Manali Shukla
2023-09-05 15:30 ` Tom Lendacky
2023-09-06 1:51 ` Alexey Kardashevskiy
2023-09-12 3:09 ` Chao Gao
2023-09-04 9:53 ` [PATCH 10/13] x86/cpufeatures: Add CPUID feature bit for VIBS in SEV-ES guest Manali Shukla
2023-09-04 9:53 ` [PATCH 11/13] KVM: SVM: Add support for IBS virtualization for SEV-ES guests Manali Shukla
2023-09-05 15:43 ` Tom Lendacky
2023-09-04 9:53 ` [PATCH 12/13] KVM: SVM: Enable IBS virtualization on non SEV-ES and " Manali Shukla
2023-09-05 16:00 ` Tom Lendacky
2023-09-12 3:30 ` Chao Gao
2023-09-04 9:53 ` [PATCH 13/13] KVM: x86: nSVM: Implement support for nested IBS virtualization Manali Shukla
2023-09-05 15:47 ` [PATCH 00/13] Implement support for " Peter Zijlstra
2023-09-06 15:38 ` Manali Shukla
2023-09-06 19:56 ` Peter Zijlstra
2023-09-07 15:49 ` Manali Shukla
2023-09-08 13:31 ` Peter Zijlstra
2023-09-11 12:32 ` Manali Shukla
2023-09-28 11:18 ` Manali Shukla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZP/Or2UqdA/EIzE7@chao-email \
--to=chao.gao@intel.com \
--cc=bp@alien8.de \
--cc=kvm@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=manali.shukla@amd.com \
--cc=nikunj@amd.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
--cc=santosh.shukla@amd.com \
--cc=seanjc@google.com \
--cc=thomas.lendacky@amd.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).