Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification

[Peter Xu <peterx@redhat.com>]

On Wed, Aug 10, 2016 at 09:30:52AM +0300, David Kiarie wrote:
> On Wed, Aug 10, 2016 at 8:49 AM, Peter Xu <peterx@redhat.com> wrote:
> 
> > On Tue, Aug 09, 2016 at 05:32:17PM +0300, David Kiarie wrote:
> >
> > [...]
> >
> > > @@ -2252,14 +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;
> > > +    VTDAddressSpace *as = opaque;
> > > +    uint16_t sid = pci_bus_num(as->bus) << 8 | as->devfn;
> >
> > SID can be something not equals to BDF. E.g., when there are PCI
> > bridges. See pci_requester_id(). However...
> >
> > >
> > >      from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
> > >      from.data = (uint32_t) value;
> > >
> > > -    if (!attrs.unspecified) {
> > > -        /* We have explicit Source ID */
> > > -        sid = attrs.requester_id;
> > > +    if (attrs.requester_id != sid) {
> > > +        VTD_DPRINTF(GENERAL, "int remap request for sid 0x%04x"
> > > +                    " requester_id 0x%04x couldn't be verified",
> > > +                    sid, attrs.requester_id);
> > > +        return MEMTX_ERROR;
> >
> > ...I am not sure whether we need extra check here. In what case will
> > attrs.requester_id != sid ?
> >
> 
> Meaning I should remove this check ?

No, that's a question I asked. I thought this if() would never trigger.

-- peterx