| Message ID | 1462568028-31037-5-git-send-email-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2016-05-06 22:53, Radim Kr?má? wrote:
> The capability alows us to express x2APIC destinations.
"allows"
Will the possibility to create >254 CPUs be indirectly coupled to this
capability, or should userland check for it explicitly then?
Will the kernel handle the case gracefully that AMD CPUs will report the
capability as well, although there is no x2APIC (at least so far) with
those CPU types, and we still inject MSI messages with that flag set
(just to double-check if the case was thought through)?
Jan
2016-05-07 16:03+0200, Jan Kiszka: > On 2016-05-06 22:53, Radim Kr?má? wrote: >> The capability alows us to express x2APIC destinations. > > "allows" Thanks. :) > Will the possibility to create >254 CPUs be indirectly coupled to this > capability, or should userland check for it explicitly then? Yes, for now, so I would prefer if userspace checked explicitly. Greater amount of VCPUs needs to be checked with KVM_CAP_MAX_VCPUS, but it's not guaranteed that all kernels with maximum above 255 will have KVM_CAP_MSI_X2APIC as there are other methods of sending an interrupt. > Will the kernel handle the case gracefully that AMD CPUs will report the > capability as well, although there is no x2APIC (at least so far) with > those CPU types, and we still inject MSI messages with that flag set > (just to double-check if the case was thought through)? Yes, it's perfectly backward compatible. We allowed x2APIC on emulated AMD cpus in the past and this just extends the address space to 32 bits. Maybe the capability should be called MSI_EXTENDED? (I though about using the same GSI type and just using those upper bits + notifiing QEMU with a capability, but userspace could have passed garbage in those bits, so it had non-zero risk.)
On 06/05/2016 22:53, Radim Kr?má? wrote: > + route->kroute.type = kvm_has_msi_x2apic() ? KVM_IRQ_ROUTING_MSI_X2APIC > + : KVM_IRQ_ROUTING_MSI; > route->kroute.flags = 0; Perhaps using flags here instead of a new route type gives simpler code in the kernel? It's a pity that the padding field of struct kvm_irq_routing_msi was not checked against zero. :( Paolo > route->kroute.u.msi.address_lo = (uint32_t)msg.address; > route->kroute.u.msi.address_hi = msg.address >> 32;
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index b7a20eb6ae69..e356438fdf5f 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -213,6 +213,7 @@ int kvm_has_pit_state2(void); int kvm_has_many_ioeventfds(void); int kvm_has_gsi_routing(void); int kvm_has_intx_set_mask(void); +bool kvm_has_msi_x2apic(void); int kvm_init_vcpu(CPUState *cpu); int kvm_cpu_exec(CPUState *cpu); diff --git a/kvm-all.c b/kvm-all.c index 8106efb19519..270152615fc2 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -81,6 +81,7 @@ struct KVMState #endif int many_ioeventfds; int intx_set_mask; + bool msi_x2apic; /* The man page (and posix) say ioctl numbers are signed int, but * they're not. Linux, glibc and *BSD all treat ioctl numbers as * unsigned, and treating them as signed here can break things */ @@ -1143,6 +1144,9 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) msi.address_hi = msg.address >> 32; msi.data = le32_to_cpu(msg.data); msi.flags = 0; + if (kvm_has_msi_x2apic()) { + msi.flags |= KVM_SIGNAL_MSI_X2APIC; + } memset(msi.pad, 0, sizeof(msi.pad)); return kvm_vm_ioctl(s, KVM_SIGNAL_MSI, &msi); @@ -1159,7 +1163,8 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) route = g_malloc0(sizeof(KVMMSIRoute)); route->kroute.gsi = virq; - route->kroute.type = KVM_IRQ_ROUTING_MSI; + route->kroute.type = kvm_has_msi_x2apic() ? KVM_IRQ_ROUTING_MSI_X2APIC + : KVM_IRQ_ROUTING_MSI; route->kroute.flags = 0; route->kroute.u.msi.address_lo = (uint32_t)msg.address; route->kroute.u.msi.address_hi = msg.address >> 32; @@ -1623,6 +1628,8 @@ static int kvm_init(MachineState *ms) s->intx_set_mask = kvm_check_extension(s, KVM_CAP_PCI_2_3); + s->msi_x2apic = kvm_check_extension(s, KVM_CAP_MSI_X2APIC); + s->irq_set_ioctl = KVM_IRQ_LINE; if (kvm_check_extension(s, KVM_CAP_IRQ_INJECT_STATUS)) { s->irq_set_ioctl = KVM_IRQ_LINE_STATUS; @@ -2104,6 +2111,11 @@ int kvm_has_intx_set_mask(void) return kvm_state->intx_set_mask; } +bool kvm_has_msi_x2apic(void) +{ + return kvm_state->msi_x2apic; +} + void kvm_setup_guest_memory(void *start, size_t size) { if (!kvm_has_sync_mmu()) { diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 80b325146a5e..4c9c53ad3e76 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -3351,6 +3351,10 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route, 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; + + if (kvm_has_msi_x2apic()) { + route->type = KVM_IRQ_ROUTING_MSI_X2APIC; + } } return 0;
The capability alows us to express x2APIC destinations. Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com> --- include/sysemu/kvm.h | 1 + kvm-all.c | 14 +++++++++++++- target-i386/kvm.c | 4 ++++ 3 files changed, 18 insertions(+), 1 deletion(-)