From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55079) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8VHU-00021F-5l for qemu-devel@nongnu.org; Thu, 02 Jun 2016 12:15:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8VHP-0001ND-Ro for qemu-devel@nongnu.org; Thu, 02 Jun 2016 12:15:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57022) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8VHP-0001N2-K2 for qemu-devel@nongnu.org; Thu, 02 Jun 2016 12:15:31 -0400 Date: Thu, 2 Jun 2016 19:15:27 +0300 From: "Michael S. Tsirkin" Message-ID: <20160602190923-mutt-send-email-mst@redhat.com> References: <1463847590-22782-1-git-send-email-bd.aviv@gmail.com> <1463847590-22782-2-git-send-email-bd.aviv@gmail.com> <57408FDB.1010000@web.de> <20160602084439.GB3477@pxdev.xzpeter.org> <20160602070046.761be49c@ul30vt.home> <5750313C.4000709@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5750313C.4000709@web.de> Subject: Re: [Qemu-devel] [PATCH v3 1/3] IOMMU: add VTD_CAP_CM to vIOMMU capability exposed to guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Alex Williamson , Peter Xu , "Aviv B.D" , qemu-devel@nongnu.org On Thu, Jun 02, 2016 at 03:14:36PM +0200, Jan Kiszka wrote: > On 2016-06-02 15:00, Alex Williamson wrote: > > On Thu, 2 Jun 2016 16:44:39 +0800 > > Peter Xu wrote: > > > >> On Sat, May 21, 2016 at 06:42:03PM +0200, Jan Kiszka wrote: > >>> On 2016-05-21 18:19, Aviv B.D wrote: > >>>> From: "Aviv Ben-David" > >>>> > >>>> This flag tells the guest to invalidate tlb cache also after unmap operations. > >>>> > >>>> Signed-off-by: Aviv Ben-David > >>>> --- > >>>> hw/i386/intel_iommu.c | 3 ++- > >>>> hw/i386/intel_iommu_internal.h | 1 + > >>>> 2 files changed, 3 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > >>>> index 347718f..1af8da8 100644 > >>>> --- a/hw/i386/intel_iommu.c > >>>> +++ b/hw/i386/intel_iommu.c > >>>> @@ -1949,7 +1949,8 @@ static void vtd_init(IntelIOMMUState *s) > >>>> s->iq_last_desc_type = VTD_INV_DESC_NONE; > >>>> s->next_frcd_reg = 0; > >>>> s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND | VTD_CAP_MGAW | > >>>> - VTD_CAP_SAGAW | VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS; > >>>> + VTD_CAP_SAGAW | VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS | > >>>> + VTD_CAP_CM; > >>> > >>> Again, needs to be optional because not all guests will support it or > >>> behave differently when it's set (I've one that refuses to work). > >> > >> There should be more than one way to make it optional. Which is > >> better? What I can think of: > >> > >> (Assume we have Marcel's "-device intel_iommu" working already) > >> > >> 1. Let the CM bit optional, or say, we need to specify something like > >> "-device intel_iommu,cmbit=on" or we will disable CM bit. If we > >> have CM disabled but with VFIO device, let QEMU raise error. > >> > >> 2. We automatically detect whether we need CM bit. E.g., if we have > >> VFIO and vIOMMU both enabled, we automatically set the bit. Another > >> case is maybe we would in the future support nested vIOMMU? If so, > >> we can do the same thing for the nested feature. > > > > > > Why do we need to support VT-d for guests that do not support CM=1? I don't think we need to do this. Spec is rather clear on this point. Just fix the guests. Support for CM=0 might still be useful because CM=1 is not required if device is restartable (most software devices would be). I prefer cmbit=on by default, with cmbit=off, don't allow VFIO. > > The VT-d spec indicates that software should be written to handle both > > caching modes (6.1). Granted this is a *should* and not a *must*, > > but can't we consider guests that do not support CM=1 incompatible with > > emulated VT-d? If CM=0 needs to be supported then we need to shadow > > all of the remapping structures since vfio effectively becomes a cache > > of the that would otherwise depend on the invalidation of both present > > and non-present entries. What guests do not support CM=1? Thanks, > > - there is at least one guest that does not support CM=1 yet (Jailhouse) > - there might be more or there might be broken ones as hardware > generally doesn't have CM=1, thus this case is typically untested > - an AMD IOMMU (to my current understanding) will require shadowing > anyway has it has no comparable concept, I was rather sure this is it: 26 NpCache: not present table entries cached. RO. Reset Xb. 1=Indicates that the IOMMU caches page table entries that are marked as not present. When this bit is set, software must issue an invalidate after any change to a PDE or PTE. 0=Indicates that the IOMMU caches only page table entries that are marked as present. When NpCache is clear, software must issue an invalidate after any change to a PDE or PTE marked present before the change. Implementation note: For hardware implementations of the IOMMU, this bit must be 0b. this implementation seems to set this bit. > thus we will eventually be > able to use that strategy also for VT-d > > Jan > >