hw/i386/kvm/apic.c | 7 ++ hw/i386/pc.c | 16 ++--- include/standard-headers/asm-x86/kvm_para.h | 1 + target/i386/cpu.c | 5 +- target/i386/kvm.c | 74 +++++++++++++++------ target/i386/kvm_i386.h | 2 + 6 files changed, 75 insertions(+), 30 deletions(-)
The IOAPIC has an 'Extended Destination ID' field in its RTE, which maps
to bits 11-4 of the MSI address. Since those address bits fall within a
given 4KiB page they were historically non-trivial to use on real hardware.
The Intel IOMMU uses the lowest bit to indicate a remappable format MSI,
and then the remaining 7 bits are part of the index.
Where the remappable format bit isn't set, we can actually use the other
seven to allow external (IOAPIC and MSI) interrupts to reach up to 32768
CPUs instead of just the 255 permitted on bare metal.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/i386/kvm/apic.c | 7 ++
hw/i386/pc.c | 16 ++---
include/standard-headers/asm-x86/kvm_para.h | 1 +
target/i386/cpu.c | 5 +-
target/i386/kvm.c | 74 +++++++++++++++------
target/i386/kvm_i386.h | 2 +
6 files changed, 75 insertions(+), 30 deletions(-)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 4eb2d77b87..aeb3366ae8 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -183,6 +183,13 @@ static void kvm_send_msi(MSIMessage *msg)
{
int ret;
+ /*
+ * The message has already passed through interrupt remapping if enabled,
+ * but the legacy extended destination ID in low bits still needs to be
+ * handled.
+ */
+ msg->address = kvm_swizzle_msi_ext_dest_id(msg->address);
+
ret = kvm_irqchip_send_msi(kvm_state, *msg);
if (ret < 0) {
fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n",
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e87be5d29a..a06c091227 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -99,6 +99,7 @@
GlobalProperty pc_compat_5_1[] = {
{ "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
+ { TYPE_X86_CPU, "kvm-msi-ext-dest-id", "off" },
};
const size_t pc_compat_5_1_len = G_N_ELEMENTS(pc_compat_5_1);
@@ -807,17 +808,12 @@ void pc_machine_done(Notifier *notifier, void *data)
fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
}
- if (x86ms->apic_id_limit > 255 && !xen_enabled()) {
- IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
- if (!iommu || !x86_iommu_ir_supported(X86_IOMMU_DEVICE(iommu)) ||
- iommu->intr_eim != ON_OFF_AUTO_ON) {
- error_report("current -smp configuration requires "
- "Extended Interrupt Mode enabled. "
- "You can add an IOMMU using: "
- "-device intel-iommu,intremap=on,eim=on");
- exit(EXIT_FAILURE);
- }
+ if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
+ !kvm_irqchip_in_kernel()) {
+ error_report("current -smp configuration requires kernel "
+ "irqchip support.");
+ exit(EXIT_FAILURE);
}
}
diff --git a/include/standard-headers/asm-x86/kvm_para.h b/include/standard-headers/asm-x86/kvm_para.h
index 07877d3295..215d01b4ec 100644
--- a/include/standard-headers/asm-x86/kvm_para.h
+++ b/include/standard-headers/asm-x86/kvm_para.h
@@ -32,6 +32,7 @@
#define KVM_FEATURE_POLL_CONTROL 12
#define KVM_FEATURE_PV_SCHED_YIELD 13
#define KVM_FEATURE_ASYNC_PF_INT 14
+#define KVM_FEATURE_MSI_EXT_DEST_ID 15
#define KVM_HINTS_REALTIME 0
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f37eb7b675..a93f50a6a7 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -799,7 +799,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
"kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
NULL, "kvm-pv-tlb-flush", NULL, "kvm-pv-ipi",
- "kvm-poll-control", "kvm-pv-sched-yield", "kvm-asyncpf-int", NULL,
+ "kvm-poll-control", "kvm-pv-sched-yield", "kvm-asyncpf-int", "kvm-msi-ext-dest-id",
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
"kvmclock-stable-bit", NULL, NULL, NULL,
@@ -4109,6 +4109,7 @@ static PropValue kvm_default_props[] = {
{ "kvm-pv-eoi", "on" },
{ "kvmclock-stable-bit", "on" },
{ "x2apic", "on" },
+ { "kvm-msi-ext-dest-id", "off" },
{ "acpi", "off" },
{ "monitor", "off" },
{ "svm", "off" },
@@ -5132,6 +5133,8 @@ static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model)
if (kvm_enabled()) {
if (!kvm_irqchip_in_kernel()) {
x86_cpu_change_kvm_default("x2apic", "off");
+ } else if (kvm_irqchip_is_split() && kvm_enable_x2apic()) {
+ x86_cpu_change_kvm_default("kvm-msi-ext-dest-id", "on");
}
x86_cpu_apply_props(cpu, kvm_default_props);
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f6dae4cfb6..90952cae7c 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -420,6 +420,9 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
if (!kvm_irqchip_in_kernel()) {
ret &= ~(1U << KVM_FEATURE_PV_UNHALT);
}
+ if (kvm_irqchip_is_split()) {
+ ret |= 1U << KVM_FEATURE_MSI_EXT_DEST_ID;
+ }
} else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
ret |= 1U << KVM_HINTS_REALTIME;
}
@@ -4583,38 +4586,71 @@ int kvm_arch_irqchip_create(KVMState *s)
}
}
+uint64_t kvm_swizzle_msi_ext_dest_id(uint64_t address)
+{
+ CPUX86State *env = &X86_CPU(first_cpu)->env;
+ uint64_t ext_id;
+
+ if (!first_cpu ||
+ !(env->features[FEAT_KVM] & (1 << KVM_FEATURE_MSI_EXT_DEST_ID))) {
+ return address;
+ }
+
+ /*
+ * If the remappable format bit is set, or the upper bits are
+ * already set in address_hi, or the low extended bits aren't
+ * there anyway, do nothing.
+ */
+ ext_id = address & (0xff << MSI_ADDR_DEST_IDX_SHIFT);
+ if (!ext_id || (ext_id & (1 << MSI_ADDR_DEST_IDX_SHIFT)) ||
+ (address >> 32))
+ return address;
+
+ address &= ~ext_id;
+ address |= ext_id << 35;
+ return address;
+}
+
int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
uint64_t address, uint32_t data, PCIDevice *dev)
{
X86IOMMUState *iommu = x86_iommu_get_default();
if (iommu) {
- int ret;
- MSIMessage src, dst;
X86IOMMUClass *class = X86_IOMMU_DEVICE_GET_CLASS(iommu);
- if (!class->int_remap) {
- return 0;
- }
+ if (class->int_remap) {
+ int ret;
+ MSIMessage src, dst;
- src.address = route->u.msi.address_hi;
- src.address <<= VTD_MSI_ADDR_HI_SHIFT;
- src.address |= route->u.msi.address_lo;
- src.data = route->u.msi.data;
+ src.address = route->u.msi.address_hi;
+ src.address <<= VTD_MSI_ADDR_HI_SHIFT;
+ src.address |= route->u.msi.address_lo;
+ src.data = route->u.msi.data;
- ret = class->int_remap(iommu, &src, &dst, dev ? \
- pci_requester_id(dev) : \
- X86_IOMMU_SID_INVALID);
- if (ret) {
- trace_kvm_x86_fixup_msi_error(route->gsi);
- return 1;
- }
+ ret = class->int_remap(iommu, &src, &dst, dev ? \
+ pci_requester_id(dev) : \
+ X86_IOMMU_SID_INVALID);
+ if (ret) {
+ trace_kvm_x86_fixup_msi_error(route->gsi);
+ return 1;
+ }
+
+ /*
+ * Handled untranslated compatibilty format interrupt with
+ * extended destination ID in the low bits 11-5. */
+ dst.address = kvm_swizzle_msi_ext_dest_id(dst.address);
- route->u.msi.address_hi = dst.address >> VTD_MSI_ADDR_HI_SHIFT;
- route->u.msi.address_lo = dst.address & VTD_MSI_ADDR_LO_MASK;
- route->u.msi.data = dst.data;
+ route->u.msi.address_hi = dst.address >> VTD_MSI_ADDR_HI_SHIFT;
+ route->u.msi.address_lo = dst.address & VTD_MSI_ADDR_LO_MASK;
+ route->u.msi.data = dst.data;
+ return 0;
+ }
}
+ address = kvm_swizzle_msi_ext_dest_id(address);
+ route->u.msi.address_hi = address >> VTD_MSI_ADDR_HI_SHIFT;
+ route->u.msi.address_lo = address & VTD_MSI_ADDR_LO_MASK;
return 0;
}
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index 0fce4e51d2..ede94760ae 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -49,4 +49,6 @@ bool kvm_has_waitpkg(void);
bool kvm_hv_vpindex_settable(void);
+uint64_t kvm_swizzle_msi_ext_dest_id(uint64_t address);
+
#endif
On 05/10/20 16:18, David Woodhouse wrote:
> + if (kvm_irqchip_is_split()) {
> + ret |= 1U << KVM_FEATURE_MSI_EXT_DEST_ID;
> + }
IIUC this is because in-kernel IOAPIC still doesn't work; and when it
does, KVM will advertise the feature itself so no other QEMU changes
will be needed.
I queued this, though of course it has to wait for the corresponding
kernel patches to be accepted (or separated into doc and non-KVM parts;
we'll see).
Paolo
On Thu, 2020-10-08 at 08:56 +0200, Paolo Bonzini wrote:
> On 05/10/20 16:18, David Woodhouse wrote:
> > + if (kvm_irqchip_is_split()) {
> > + ret |= 1U << KVM_FEATURE_MSI_EXT_DEST_ID;
> > + }
>
> IIUC this is because in-kernel IOAPIC still doesn't work; and when it
> does, KVM will advertise the feature itself so no other QEMU changes
> will be needed.
More the MSI handling than the IOAPIC. I haven't actually worked out
*what* handles cycles to addresses in the 0xFEExxxxx range for the in-
kernel irqchip and turns them into interrupts (after putting them
through interrupt remapping, if/when the kernel learns to do that).
Ideally the IOAPIC would just swizzle the bits in its RTE to create an
MSI message and pass it on to the same code to be (translated and)
delivered.
You'll note my qemu patch didn't touch IOAPIC code at all, because
qemu's IOAPIC really does just that.
> I queued this, though of course it has to wait for the corresponding
> kernel patches to be accepted (or separated into doc and non-KVM
> parts; we'll see).
Thanks.
On 08/10/20 09:29, David Woodhouse wrote:
> On Thu, 2020-10-08 at 08:56 +0200, Paolo Bonzini wrote:
>> On 05/10/20 16:18, David Woodhouse wrote:
>>> + if (kvm_irqchip_is_split()) {
>>> + ret |= 1U << KVM_FEATURE_MSI_EXT_DEST_ID;
>>> + }
>>
>> IIUC this is because in-kernel IOAPIC still doesn't work; and when it
>> does, KVM will advertise the feature itself so no other QEMU changes
>> will be needed.
>
> More the MSI handling than the IOAPIC. I haven't actually worked out
> *what* handles cycles to addresses in the 0xFEExxxxx range for the in-
> kernel irqchip and turns them into interrupts (after putting them
> through interrupt remapping, if/when the kernel learns to do that).
That's easy: it's QEMU. :) See kvm_apic_mem_write in hw/i386/kvm/apic.c
(note that this memory region is never used when the CPU accesses
0xFEExxxxx, only when QEMU does.
Conversion from the IOAPIC and MSI formats to struct kvm_lapic_irq is
completely separate in KVM, it is respectively in ioapic_service and
kvm_set_msi_irq. Both of them prepare a struct kvm_lapic_irq, but
they're two different paths.
> Ideally the IOAPIC would just swizzle the bits in its RTE to create an
> MSI message and pass it on to the same code to be (translated and)
> delivered.
>
> You'll note my qemu patch didn't touch IOAPIC code at all, because
> qemu's IOAPIC really does just that.
Indeed the nice thing about irqchip=split is that the handling of device
interrupts is entirely confined within QEMU, no matter if they're IOAPIC
or MSI. And because we had to implement interrupt remapping, the IOAPIC
is effectively using MSIs to deliver its interrupts.
There's still the hack to communicate IOAPIC routes to KVM and have it
set the EOI exit bitmap correctly, though. The code is in
kvm_scan_ioapic_routes and it uses kvm_set_msi_irq (with irqchip=split
everything is also an MSI within the kernel). I think you're not
handling that correctly for CPUs >255, so after all we _do_ need some
kernel support.
Paolo
>> I queued this, though of course it has to wait for the corresponding
>> kernel patches to be accepted (or separated into doc and non-KVM
>> parts; we'll see).
>
> Thanks.
>
On Thu, 2020-10-08 at 09:53 +0200, Paolo Bonzini wrote: > Indeed the nice thing about irqchip=split is that the handling of device > interrupts is entirely confined within QEMU, no matter if they're IOAPIC > or MSI. And because we had to implement interrupt remapping, the IOAPIC > is effectively using MSIs to deliver its interrupts. > > There's still the hack to communicate IOAPIC routes to KVM and have it > set the EOI exit bitmap correctly, though. The code is in > kvm_scan_ioapic_routes and it uses kvm_set_msi_irq (with irqchip=split > everything is also an MSI within the kernel). I think you're not > handling that correctly for CPUs >255, so after all we _do_ need some > kernel support. I think that works out OK. In QEMU's ioapic_update_kvm_routes() it calls ioapic_entry_parse() which generates the actual "bus" MSI with the extended dest ID in bits 11-5 of the address. That MSI message is passed to kvm_irqchip_update_msi_route() which passes it through translation — which does interrupt remapping and shifting the ext bits up into ->address_hi as the KVM X2APIC API expects. So when the kernel's kvm_scan_ioapic_routes() goes looking, kvm_set_msi_irq() fills 'irq' in with the correct dest_id, and kvm_apic_match_dest() does the right thing. No? As far as I can tell, we *do* have a QEMU bug — not related to the ext dest ID — because for MSIs of assigned devices we don't update the KVM IRQ routing table when the Interrupt Remapping IEC cache is flushed. > Paolo > > > > I queued this, though of course it has to wait for the corresponding > > > kernel patches to be accepted (or separated into doc and non-KVM > > > parts; we'll see). > > > > Thanks. So... it'll hit the tip.git tree and thus linux-next as soon as Linus releases 5.10-rc1, and it'll then get merged into 5.11-rc1 and be in the 5.11 release. At which of those three points in time would you be happy to merge it to QEMU? If it's either of the latter two, maybe it *is* worth doing a patch which *only* reserves the feature bit, and trying to slip it into 5.10?
On 19/10/20 14:21, David Woodhouse wrote: > On Thu, 2020-10-08 at 09:53 +0200, Paolo Bonzini wrote: >> I think you're not >> handling that correctly for CPUs >255, so after all we _do_ need some >> kernel support. > > I think that works out OK. > > In QEMU's ioapic_update_kvm_routes() it calls ioapic_entry_parse() > which generates the actual "bus" MSI with the extended dest ID in bits > 11-5 of the address. > > That MSI message is passed to kvm_irqchip_update_msi_route() which > passes it through translation — which does interrupt remapping and > shifting the ext bits up into ->address_hi as the KVM X2APIC API > expects. > > So when the kernel's kvm_scan_ioapic_routes() goes looking, > kvm_set_msi_irq() fills 'irq' in with the correct dest_id, and > kvm_apic_match_dest() does the right thing. > > No? Yeah, that seems fine. > As far as I can tell, we *do* have a QEMU bug — not related to the ext > dest ID — because for MSIs of assigned devices we don't update the KVM > IRQ routing table when the Interrupt Remapping IEC cache is flushed. > So... it'll hit the tip.git tree and thus linux-next as soon as Linus > releases 5.10-rc1, and it'll then get merged into 5.11-rc1 and be in > the 5.11 release. > > At which of those three points in time would you be happy to merge it > to QEMU? If it's either of the latter two, maybe it *is* worth doing a > patch which *only* reserves the feature bit, and trying to slip it into > 5.10? It would be 5.11-rc1 because of the KVM_FEATURE_MSI_EXT_DEST_ID definition which would not be in your patch but rather synchronized from the Linux tree by scripts/update-linux-headers.sh. If you send me the doc patch any time before 5.10-rc7, it will be in 5.10. Paolo
From: David Woodhouse <dwmw@amazon.co.uk>
No functional change; just reserve the feature bit for now so that VMMs
can start to implement it.
This will allow the host to indicate that MSI emulation supports 15-bit
destination IDs, allowing up to 32768 CPUs without interrupt remapping.
cf. https://patchwork.kernel.org/patch/11816693/ for qemu
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
Documentation/virt/kvm/cpuid.rst | 4 ++++
arch/x86/include/uapi/asm/kvm_para.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/Documentation/virt/kvm/cpuid.rst b/Documentation/virt/kvm/cpuid.rst
index 9150e9d1c39b..f70b655821d5 100644
--- a/Documentation/virt/kvm/cpuid.rst
+++ b/Documentation/virt/kvm/cpuid.rst
@@ -92,6 +92,10 @@ KVM_FEATURE_ASYNC_PF_INT 14 guest checks this feature bit
async pf acknowledgment msr
0x4b564d07.
+KVM_FEATURE_MSI_EXT_DEST_ID 15 guest checks this feature bit
+ before using extended destination
+ ID bits in MSI address bits 11-5.
+
KVM_FEATURE_CLOCSOURCE_STABLE_BIT 24 host will warn if no guest-side
per-cpu warps are expeced in
kvmclock
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 812e9b4c1114..950afebfba88 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -32,6 +32,7 @@
#define KVM_FEATURE_POLL_CONTROL 12
#define KVM_FEATURE_PV_SCHED_YIELD 13
#define KVM_FEATURE_ASYNC_PF_INT 14
+#define KVM_FEATURE_MSI_EXT_DEST_ID 15
#define KVM_HINTS_REALTIME 0
--
2.17.1
On Mon, 2020-10-05 at 15:18 +0100, David Woodhouse wrote: > The IOAPIC has an 'Extended Destination ID' field in its RTE, which maps > to bits 11-4 of the MSI address. Since those address bits fall within a > given 4KiB page they were historically non-trivial to use on real hardware. > > The Intel IOMMU uses the lowest bit to indicate a remappable format MSI, > and then the remaining 7 bits are part of the index. > > Where the remappable format bit isn't set, we can actually use the other > seven to allow external (IOAPIC and MSI) interrupts to reach up to 32768 > CPUs instead of just the 255 permitted on bare metal. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Corresponding kernel patch at https://patchwork.kernel.org/patch/11820535/
© 2016 - 2025 Red Hat, Inc.