qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: David Kiarie
Subject: Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
Date: Wed, 10 Aug 2016 09:30:52 +0300

On Wed, Aug 10, 2016 at 8:49 AM, Peter Xu <address@hidden> 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 ?


>
> Though I agree to remove the original if().
>
> >      }
> >
> >      ret = vtd_interrupt_remap_msi(opaque, &from, &to, sid);
> > @@ -2325,7 +2326,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
> *s, PCIBus *bus, int devfn)
> >          memory_region_init_iommu(&vtd_dev_as->iommu, OBJECT(s),
> >                                   &s->iommu_ops, "intel_iommu",
> UINT64_MAX);
> >          memory_region_init_io(&vtd_dev_as->iommu_ir, OBJECT(s),
> > -                              &vtd_mem_ir_ops, s, "intel_iommu_ir",
> > +                              &vtd_mem_ir_ops, vtd_dev_as,
> "intel_iommu_ir",
> >                                VTD_INTERRUPT_ADDR_SIZE);
> >          memory_region_add_subregion(&vtd_dev_as->iommu,
> VTD_INTERRUPT_ADDR_FIRST,
> >                                      &vtd_dev_as->iommu_ir);
> > @@ -2465,6 +2466,9 @@ static void vtd_realize(DeviceState *dev, Error
> **errp)
> >      vtd_init(s);
> >      sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
> >      pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
> > +    /* IOMMU expected IOAPIC SID */
> > +    x86_iommu->ioapic_bdf = Q35_PSEUDO_DEVFN_IOAPIC << 8 |
> > +        Q35_PSEUDO_DEVFN_IOAPIC;
>
> We can use PCI_BUILD_BDF() here.
>
> -- peterx
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]