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

[Peter Xu]

On Wed, Aug 10, 2016 at 09:35:25AM +0300, David Kiarie wrote:
> On Wed, Aug 10, 2016 at 8:41 AM, Peter Xu <address@hidden> wrote:
> 
> > On Tue, Aug 09, 2016 at 05:32:16PM +0300, David Kiarie wrote:
> > > When using IOMMU platform devices like IOAPIC are required to make
> > > interrupt remapping requests using explicit SID.We affiliate an MSI
> > > route with a requester ID and a PCI device if present which ensures
> > > that platform devices can call IOMMU interrupt remapping code with
> > > explicit SID while maintaining compatility with the original code
> > > which mainly dealt with PCI devices.
> > >
> > > Signed-off-by: David Kiarie <address@hidden>
> >
> > Hi,
> >
> > This idea is good to me overall, with some tiny comments below.
> >
> > [...]
> >
> > > -static void ioapic_service(IOAPICCommonState *s)
> > > +static void ioapic_write_ioapic_as(IOAPICCommonState *s, uint32_t
> > data, uint64_t addr)
> >
> > Rename to ioapic_as_write()?
> >
> > [...]
> >
> > > @@ -385,12 +393,23 @@ static void ioapic_machine_done_notify(Notifier
> > *notifier, void *data)
> > >
> > >      if (kvm_irqchip_is_split()) {
> > >          X86IOMMUState *iommu = x86_iommu_get_default();
> > > +        MSIMessage msg = {0, 0};
> > > +        int i;
> > > +
> > >          if (iommu) {
> > >              /* Register this IOAPIC with IOMMU IEC notifier, so that
> > >               * when there are IR invalidates, we can be notified to
> > >               * update kernel IR cache. */
> > > -            x86_iommu_iec_register_notifier(iommu,
> > ioapic_iec_notifier, s);
> > > +            s->devid = iommu->ioapic_bdf;
> > > +            /* update IOAPIC routes to the right SID */
> > > +            for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > +                kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL,
> > s->devid);
> > > +            }
> > > +            kvm_irqchip_commit_routes(kvm_state);
> >
> > Here, not sure whether it'll be better if we remove
> > kvm_irqchip_add_msi_route() in kvm_arch_init_irq_routing() directly
> > and call them here. So no extra update needed.
> >
> 
> Thought about that too but  I was worried another device might reserve
> routes before IOAPIC does.

Right. Had a quick look, only thing I am not sure is how ivshmem setup
its routes. It seems that it's done before this notifier. So maybe
your method is good to keep.

-- peterx