From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42823) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUVxd-00039w-Ji for qemu-devel@nongnu.org; Tue, 02 Aug 2016 05:26:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUVxW-0002az-LO for qemu-devel@nongnu.org; Tue, 02 Aug 2016 05:26:03 -0400 Received: from thoth.sbs.de ([192.35.17.2]:37807) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUVxM-0002Yo-WC for qemu-devel@nongnu.org; Tue, 02 Aug 2016 05:25:58 -0400 References: <1469123413-20809-1-git-send-email-mst@redhat.com> <1469123413-20809-30-git-send-email-mst@redhat.com> <98a5a157-ba83-bc61-df7e-546c21e23ad3@siemens.com> <20160802083648.GJ6207@pxdev.xzpeter.org> <4fcaa50f-6c09-f607-1c49-8494ec32f0c5@siemens.com> From: Jan Kiszka Message-ID: <3a975681-e6ec-102e-96f8-00c7fb402356@siemens.com> Date: Tue, 2 Aug 2016 10:59:17 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL v5 29/57] intel_iommu: add SID validation for IR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Kiarie Cc: Peter Xu , Valentine Sinitsyn , qemu-devel [always keep the list in CC] On 2016-08-02 10:54, David Kiarie wrote: > > > On Tue, Aug 2, 2016 at 11:46 AM, Jan Kiszka > wrote: > > On 2016-08-02 10:36, Peter Xu wrote: > > 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). > > It's not hacky at all if done properly. For Intel it is simply > (Q35_PSEUDO_BUS_PLATFORM << 8) | Q35_PSEUDO_DEVFN_IOAPIC, but it will be > 0x00a0 (as constant as well) for AMD. So we need some interface to tell > those parameters to the IOMMU. Keep in mind that we will need a similar > interface for other platform devices, e.g. the HPET. > > > In my case IOMMU knows about these parameters but IOAPIC makes interrupt > requests with a different ID. I thought we should make IOAPIC (and other > platform devices) make requests using specified IDs. > In fact, intel_iommu knows them as well: The write request comes in via a well-known address space, the one handed out to the IOMMU. Some for other platform devices. Associating the ID on write with the requester would basically mean using the pattern we had in the old iommu code prior to the introduction of MemTxAttrs. Not sure, though, what is cleaner here. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux