From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56896) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUWwM-0000SL-Ca for qemu-devel@nongnu.org; Tue, 02 Aug 2016 06:28:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUWwF-0005h6-DQ for qemu-devel@nongnu.org; Tue, 02 Aug 2016 06:28:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58832) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUWwF-0005h2-4u for qemu-devel@nongnu.org; Tue, 02 Aug 2016 06:28:43 -0400 Date: Tue, 2 Aug 2016 18:28:38 +0800 From: Peter Xu Message-ID: <20160802102838.GK6207@pxdev.xzpeter.org> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4fcaa50f-6c09-f607-1c49-8494ec32f0c5@siemens.com> 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: Jan Kiszka Cc: David kiarie , qemu-devel@nongnu.org, "Michael S. Tsirkin" , Peter Maydell , Richard Henderson , Eduardo Habkost , Paolo Bonzini , Valentine Sinitsyn On Tue, Aug 02, 2016 at 10:46:13AM +0200, 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. Okay. > > > > > I can try to cook another seperate patch to enable it (for 2.8 > > possibly?). Thanks for pointing out this issue. > > David needs that IOAPIC ID as well in order to finish interrupt > remapping on AMD. Please synchronize with him who will implement what. Sure. David, so do you like to do it or I cook this patch? :) Thanks, -- peterx