From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38103) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fBvhH-0008B9-Bn for qemu-devel@nongnu.org; Fri, 27 Apr 2018 01:13:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fBvhD-0003ZJ-D0 for qemu-devel@nongnu.org; Fri, 27 Apr 2018 01:13:27 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:54840 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fBvhD-0003Wz-8l for qemu-devel@nongnu.org; Fri, 27 Apr 2018 01:13:23 -0400 References: <20180425045129.17449-1-peterx@redhat.com> <20180425045129.17449-4-peterx@redhat.com> From: Jason Wang Message-ID: <2492f7d8-ed63-6f1c-773f-baa223020022@redhat.com> Date: Fri, 27 Apr 2018 13:13:02 +0800 MIME-Version: 1.0 In-Reply-To: <20180425045129.17449-4-peterx@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 03/10] intel-iommu: add iommu lock List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu , qemu-devel@nongnu.org Cc: "Michael S . Tsirkin" , Alex Williamson , Jintack Lim On 2018=E5=B9=B404=E6=9C=8825=E6=97=A5 12:51, Peter Xu wrote: > Add a per-iommu big lock to protect IOMMU status. Currently the only > thing to be protected is the IOTLB cache, since that can be accessed > even without BQL, e.g., in IO dataplane. > > Note that device page tables should not need any protection. The safet= y > of that should be provided by guest OS. E.g., when a page entry is > freed, the guest OS should be responsible to make sure that no device > will be using that page any more. > > Reported-by: Fam Zheng > Signed-off-by: Peter Xu > --- > include/hw/i386/intel_iommu.h | 8 ++++++++ > hw/i386/intel_iommu.c | 31 +++++++++++++++++++++++++++++-- > 2 files changed, 37 insertions(+), 2 deletions(-) > > diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iomm= u.h > index 220697253f..1a8ba8e415 100644 > --- a/include/hw/i386/intel_iommu.h > +++ b/include/hw/i386/intel_iommu.h > @@ -262,6 +262,14 @@ struct IntelIOMMUState { > uint8_t w1cmask[DMAR_REG_SIZE]; /* RW1C(Write 1 to Clear) bytes *= / > uint8_t womask[DMAR_REG_SIZE]; /* WO (write only - read returns = 0) */ > uint32_t version; > + /* > + * Protects IOMMU states in general. Normally we don't need to > + * take this lock when we are with BQL held. However we have code > + * paths that may run even without BQL. In those cases, we need > + * to take the lock when we have access to IOMMU state > + * informations, e.g., the IOTLB. > + */ > + QemuMutex iommu_lock; Some questions: 1) Do we need to protect context cache too? 2) Can we just reuse qemu BQL here? 3) I think the issue is common to all other kinds of IOMMU, so can we=20 simply synchronize before calling ->translate() in memory.c. This seems=20 a more common solution. Thanks