Re: [Qemu-devel] [v4 1/6] hw/msi: Allow platform devices to use explicit SID

[Peter Xu]

On Mon, Sep 12, 2016 at 02:26:14PM +0300, David Kiarie wrote:

[...]

> > > -int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
> > > +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev,
> > uint16_t requester_id)
> > >  {
> > >      struct kvm_irq_routing_entry kroute = {};
> > >      int virq;
> > > @@ -1275,7 +1275,8 @@ int kvm_irqchip_add_msi_route(KVMState *s, int
> > vector, PCIDevice *dev)
> > >      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);
> > > -    if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data, dev))
> > {
> > > +    if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data,
> > > +                                 requester_id)) {
> >
> > Can we just remove the PCIDevice parameter directly? I didn't go
> > deeper, but it seems to be explicitly introduced here:
> >
> >     commit dc9f06ca81e6e16d062ec382701142a3a2ab3f7d
> >     Author: Pavel Fedin <address@hidden>
> >     Date:   Thu Oct 15 16:44:52 2015 +0300
> >
> >         kvm: Pass PCI device pointer to MSI routing functions
> >
> > And we'd better make sure we really wanted to remove it.
> >
> > Also, I think we need to modify all target-*/kvm.c for this interface
> > change?
> >
> 
> The current code keeps track of PCI devices for purposes of easily
> constructing MSI messages
> 
> e.g in kvm_update_msi_routes_all which means to get rid of PCI device
> pointer means we need another way to keep track of PCI devices.

Actually I mean kvm_arch_fixup_msi_route(), not
kvm_{add|update}_msi_routes_all().

It looks like above commit (dc9f06ca81e) was preparing to pass in SID
in the future for ARM (though still haven't implemented on QEMU side).
So maybe removing PCIDevice is okay in this one since you are passing
in the SID directly. However I think possibly Paolo is the best one to
know the truth...

No matter what, you may still need to change the interface for other
platforms. Just try to grep kvm_arch_fixup_msi_route() in target-*/
directories. 

Thanks,

-- peterx