Re: [Qemu-devel] [PATCH] RFCv3 kvm irqfd: support msimessage to irq translation in PHB

[Anthony Liguori]

On Sat, Jun 29, 2013 at 7:59 PM, Alexey Kardashevskiy <address@hidden> wrote:
> On 06/30/2013 12:28 AM, Anthony Liguori wrote:
>> Perhaps (2) can just be the default PCI bus implementation to simplify 
>> things.
>
>
> hw/pci/pci.c does not have any kvm code yet and I would like not to be the
> first person who tries adding this there :)
> But ok, I'll do it.

More often than not, the PHB is part of the northbridge and has direct
interactions with the CPUs.   Since KVM is really modeling the CPUs,
it makes sense that the PHB would need to have some knowledge of KVM.

It's a bit silly for a PCI device like virtio-pci to have direct
knowledge of KVM...

>> You don't need this function.  You can do this overloading as part of
>> the PCI bus initialization in spapr_pci.c
>
>
> pci_bus_set_route_irq_fn is there already and I tried to follow the
> existing pattern (yeah, missed assert though). Or this is different?

Yeah, the existing pattern sucks.  PCI is long overdue for a
significant refactoring.

Regards,

Anthony Liguori

>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>> +int pci_bus_map_msi(PCIBus *bus, MSIMessage msg)
>>> +{
>>> +    if (bus->map_msi) {
>>> +        return bus->map_msi(bus, msg);
>>> +    }
>>> +    return -1;
>>> +}
>>> +
>>>  /*
>>>   * PCI-to-PCI bridge specification
>>>   * 9.1: Interrupt routing. Table 9-1
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index 23dbc0e..bae4faf 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -486,6 +486,11 @@ static void spapr_msi_write(void *opaque, hwaddr addr,
>>>      qemu_irq_pulse(xics_get_qirq(spapr->icp, irq));
>>>  }
>>>
>>> +static int spapr_msi_get_irq(PCIBus *bus, MSIMessage msg)
>>> +{
>>> +    return msg.data;
>>> +}
>>> +
>>>  static const MemoryRegionOps spapr_msi_ops = {
>>>      /* There is no .read as the read result is undefined by PCI spec */
>>>      .read = NULL,
>>> @@ -657,6 +662,7 @@ static int spapr_phb_init(SysBusDevice *s)
>>>
>>>          sphb->lsi_table[i].irq = irq;
>>>      }
>>> +    pci_bus_set_map_msi_fn(bus, spapr_msi_get_irq);
>>>
>>>      return 0;
>>>  }
>>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>>> index b070b64..06a4e13 100644
>>> --- a/hw/virtio/virtio-pci.c
>>> +++ b/hw/virtio/virtio-pci.c
>>> @@ -481,7 +481,7 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy 
>>> *proxy,
>>>      int ret;
>>>
>>>      if (irqfd->users == 0) {
>>> -        ret = kvm_irqchip_add_msi_route(kvm_state, msg);
>>> +        ret = kvm_irqchip_add_msi_route(kvm_state, proxy->pci_dev.bus, 
>>> msg);
>>>          if (ret < 0) {
>>>              return ret;
>>>          }
>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>> index 6ef1f97..8c1edd6 100644
>>> --- a/include/hw/pci/pci.h
>>> +++ b/include/hw/pci/pci.h
>>> @@ -332,6 +332,7 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev);
>>>  typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
>>>  typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
>>>  typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
>>> +typedef int (*pci_map_msi_fn)(PCIBus *bus, MSIMessage msg);
>>>
>>>  typedef enum {
>>>      PCI_HOTPLUG_DISABLED,
>>> @@ -375,6 +376,9 @@ bool pci_intx_route_changed(PCIINTxRoute *old, 
>>> PCIINTxRoute *new);
>>>  void pci_bus_fire_intx_routing_notifier(PCIBus *bus);
>>>  void pci_device_set_intx_routing_notifier(PCIDevice *dev,
>>>                                            PCIINTxRoutingNotifier notifier);
>>> +void pci_bus_set_map_msi_fn(PCIBus *bus, pci_map_msi_fn map_msi_fn);
>>> +int pci_bus_map_msi(PCIBus *bus, MSIMessage msg);
>>> +
>>>  void pci_device_reset(PCIDevice *dev);
>>>  void pci_bus_reset(PCIBus *bus);
>>>
>>> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
>>> index 66762f6..81efd2b 100644
>>> --- a/include/hw/pci/pci_bus.h
>>> +++ b/include/hw/pci/pci_bus.h
>>> @@ -16,6 +16,7 @@ struct PCIBus {
>>>      pci_set_irq_fn set_irq;
>>>      pci_map_irq_fn map_irq;
>>>      pci_route_irq_fn route_intx_to_irq;
>>> +    pci_map_msi_fn map_msi;
>>>      pci_hotplug_fn hotplug;
>>>      DeviceState *hotplug_qdev;
>>>      void *irq_opaque;
>>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>>> index f404d16..1bf2abe 100644
>>> --- a/include/sysemu/kvm.h
>>> +++ b/include/sysemu/kvm.h
>>> @@ -305,8 +305,8 @@ static inline void cpu_synchronize_post_init(CPUState 
>>> *cpu)
>>>      }
>>>  }
>>>
>>> -int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg);
>>>  int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg);
>>> +int kvm_irqchip_add_msi_route(KVMState *s, PCIBus *pbus, MSIMessage msg);
>>>  void kvm_irqchip_release_virq(KVMState *s, int virq);
>>>
>>>  int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n, int 
>>> virq);
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 1f81cca..3b7710d 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -1180,11 +1180,16 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage 
>>> msg)
>>>      return kvm_set_irq(s, route->kroute.gsi, 1);
>>>  }
>>>
>>> -int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
>>> +int kvm_irqchip_add_msi_route(KVMState *s, PCIBus *pbus, MSIMessage msg)
>>>  {
>>>      struct kvm_irq_routing_entry kroute;
>>>      int virq;
>>>
>>> +    virq = pci_bus_map_msi(pbus, msg);
>>> +    if (virq >= 0) {
>>> +        return virq;
>>> +    }
>>> +
>>>      if (!kvm_gsi_routing_enabled()) {
>>>          return -ENOSYS;
>>>      }
>>> --
>>> 1.7.10.4
>>>
>>>
>
>
> --
> Alexey