From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40236) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g0N4A-0000J3-Fj for qemu-devel@nongnu.org; Thu, 13 Sep 2018 04:33:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g0N46-0006hz-GO for qemu-devel@nongnu.org; Thu, 13 Sep 2018 04:33:34 -0400 Date: Thu, 13 Sep 2018 16:33:17 +0800 From: Peter Xu Message-ID: <20180913083317.GG10763@xz-x1> References: <20180913075517.11140-1-peterx@redhat.com> <2ad9db94-3d74-2879-e0a5-a0c4369d3a7f@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2ad9db94-3d74-2879-e0a5-a0c4369d3a7f@redhat.com> Subject: Re: [Qemu-devel] [PATCH] intel_iommu: handle invalid ce for shadow sync List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Maxime Coquelin Cc: qemu-devel@nongnu.org, Pei Zhang , Alex Williamson , Jason Wang , "Michael S . Tsirkin" , QEMU Stable On Thu, Sep 13, 2018 at 10:16:20AM +0200, Maxime Coquelin wrote: > Hi Peter, > > On 09/13/2018 09:55 AM, Peter Xu wrote: > > There are two callers for vtd_sync_shadow_page_table_range(), one > > provided a valid context entry and one not. Move that fetching > > operation into the caller vtd_sync_shadow_page_table() where we need to > > fetch the context entry. > > > > Meanwhile, we should handle VTD_FR_CONTEXT_ENTRY_P properly when > > synchronizing shadow page tables. Having invalid context entry there is > > perfectly valid when we move a device out of an existing domain. When > > that happens, instead of posting an error we invalidate the whole region. > > > > Without this patch, QEMU will crash if we do these steps: > > > > (1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe) > > (2) bind the NICs with vfio-pci in the guest > > (3) start testpmd with the NICs applied > > (4) stop testpmd > > (5) rebind the NIC back to ixgbe kernel driver > > > > The patch should fix it. > > > > Reported-by: Pei Zhang > > Tested-by: Pei Zhang > > CC: Pei Zhang > > CC: Alex Williamson > > CC: Jason Wang > > CC: Maxime Coquelin > > CC: Michael S. Tsirkin > > CC: QEMU Stable > > Fixes:https://bugzilla.redhat.com/show_bug.cgi?id=1627272 > > It seems like a regression as it wasn't reported earlier, isn't it? > If it is, do you know what is the faulty commit? I think it should be 63b88968f1 ("intel-iommu: rework the page walk logic", 2018-05-23). The old code would possibly unmap all (using the old replay logic) before we introduce the shadow page sync helpers and I must have overlooked on that point. > > > Signed-off-by: Peter Xu > > --- > > hw/i386/intel_iommu.c | 54 ++++++++++++++++++++++++++----------------- > > 1 file changed, 33 insertions(+), 21 deletions(-) > > Other than that, the patch looks good to me. > FWIW: > Acked-by: Maxime Coquelin Thanks, -- Peter Xu