Re: [Qemu-devel] [PULL v5 29/57] intel_iommu: add SID validation for IR

[Peter Xu]

On Mon, Aug 01, 2016 at 06:39:05PM +0200, Jan Kiszka wrote:

[...]

> >  static MemTxResult vtd_mem_ir_read(void *opaque, hwaddr addr,
> > @@ -2209,11 +2250,17 @@ static MemTxResult vtd_mem_ir_write(void *opaque, 
> > hwaddr addr,
> >  {
> >      int ret = 0;
> >      MSIMessage from = {}, to = {};
> > +    uint16_t sid = X86_IOMMU_SID_INVALID;
> >  
> >      from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
> >      from.data = (uint32_t) value;
> >  
> > -    ret = vtd_interrupt_remap_msi(opaque, &from, &to);
> > +    if (!attrs.unspecified) {
> > +        /* We have explicit Source ID */
> > +        sid = attrs.requester_id;
> > +    }
> 
> ...here you fall back to X86_IOMMU_SID_INVALID if writer to this region
> has not provided some valid attrs. That is questionable, defeats
> validation of the IOAPIC e.g. (and you can see lots of
> X86_IOMMU_SID_INVALID in vtd_irte_get when booting a guest).
> 
> The credits also go to David who noticed that he still doesn't get a
> proper ID from the IOAPIC while implementing AMD IR. Looks like we need
> to enlighten the IOAPIC MSI writes...

Jan, David,

At the time when drafting the patch, I skipped SID verification for
IOAPIC interrupts since it differs from generic PCI devices (no
natural requester ID, so need some hacky lines to enable it).

I can try to cook another seperate patch to enable it (for 2.8
possibly?). Thanks for pointing out this issue.

-- peterx