Thread (52 messages) 52 messages, 5 authors, 2015-07-07

[PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

From: Eric Auger <hidden>
Date: 2015-07-06 15:02:16
Also in: kvm, kvmarm, lkml

Hi all,
On 07/06/2015 03:32 PM, Pavel Fedin wrote:
quoted hunk ↗ jump to hunk
 Hi!
quoted
quoted
Well, as we are about to implement this: yes. But the issue is that MSI
injection and GSI routing code is generic PCI code in userland (at least
in kvmtool, guess in QEMU, too), so I don't want to pull in any kind of
ARM specific code in there. The idea is to always provide the device ID
from the PCI code (for PCI devices it's just the B/D/F triplet), but
only send it to the kernel if needed. Querying a KVM capability is
perfectly fine for this IMO.
Yes, I agree.
 Actually, we already have this capability, it's KVM_CAP_IRQ_ROUTING. If we have this capability,
and want to use irqfds with GICv3, we need to set KVM_MSI_VALID_DEVID. And there is no other way to
use irqfds with GICv3.
 Just for example, this is what i have done in qemu:
--- cut ---
int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg, PCIDevice *dev)
{
    struct kvm_irq_routing_entry kroute = {};
    int virq;

    if (kvm_gsi_direct_mapping()) {
        return kvm_arch_msi_data_to_gsi(msg.data);
    }

    if (!kvm_gsi_routing_enabled()) {
        return -ENOSYS;
    }

    virq = kvm_irqchip_get_virq(s);
    if (virq < 0) {
        return virq;
    }

    kroute.gsi = virq;
    kroute.type = KVM_IRQ_ROUTING_MSI;
    kroute.u.msi.address_lo = (uint32_t)msg.address;
    kroute.u.msi.address_hi = msg.address >> 32;
    kroute.u.msi.data = le32_to_cpu(msg.data);
    kroute.flags = kvm_msi_flags;
    if (kroute.flags & KVM_MSI_VALID_DEVID) {
        kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
    }

    if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data)) {
        kvm_irqchip_release_virq(s, virq);
        return -EINVAL;
    }

    kvm_add_routing_entry(s, &kroute);
    kvm_irqchip_commit_routes(s);

    return virq;
}
--- cut ---

 ITS code in qemu just does:

---cut ---
    msi_supported = true;
    kvm_msi_flags = KVM_MSI_VALID_DEVID;
    kvm_msi_via_irqfd_allowed = kvm_has_gsi_routing();
    kvm_gsi_routing_allowed = kvm_msi_via_irqfd_allowed;
--- cut ---

 I set KVM_MSI_VALID_DEVID unconditionally here just because it will never be checked if
kvm_msi_via_irqfd_allowed is false, it's just qemu specifics. The more canonical form would perhaps
be:
--- cut ---
if (kvm_has_gsi_routing()) {
    kvm_msi_flags = KVM_MSI_VALID_DEVID;
Personally I prefer a capability rather than hardcoding a global
variable value in the qemu interrupt controller code. All the more so
typically there is KVM GSI routing cap that could/should? be queried
instead of hardcoding the value I think.

So not sure whether we eventually concluded;-)
- introduce a KVM_CAP_MSI_DEVID capability? All OK except Pavel not
convinced?
- userspaces puts the devid in struct kvm_irq_routing_msi pad field:
consensus (we do not intrduce a new kvm_irq_routing_ext_msi)
- userspace tells it conveyed a devid by setting
A) the kvm_irq_routing_entry's field?
B) the kvm_irq_routing_entry's type
no consensus. If there is a cap, does it really matter?

Best Regards

Eric
quoted hunk ↗ jump to hunk
    kvm_gsi_routing_allowed = true;
    kvm_msi_via_irqfd_allowed = true;
}
--- cut ---

 I can post my sets as RFCs to qemu mailing list, if you want to take a look at the whole change
set.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help