[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification |
Date: |
Wed, 10 Aug 2016 13:49:15 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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 ?
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