From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33365) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXMOK-00044T-AH for qemu-devel@nongnu.org; Wed, 10 Aug 2016 01:49:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bXMOG-0002e8-5g for qemu-devel@nongnu.org; Wed, 10 Aug 2016 01:49:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54822) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXMOG-0002dn-0R for qemu-devel@nongnu.org; Wed, 10 Aug 2016 01:49:20 -0400 Date: Wed, 10 Aug 2016 13:49:15 +0800 From: Peter Xu Message-ID: <20160810054915.GI4201@pxdev.xzpeter.org> References: <1470753137-18354-1-git-send-email-davidkiarie4@gmail.com> <1470753137-18354-3-git-send-email-davidkiarie4@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1470753137-18354-3-git-send-email-davidkiarie4@gmail.com> Subject: Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Kiarie Cc: qemu-devel@nongnu.org, peter.maydell@linaro.org, ehabkost@redhat.com, mst@redhat.com, jan.kiszka@siemens.com, valentine.sinitsyn@gmail.com, pbonzini@redhat.com 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