[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D5D0FF0@SHSMSX104.ccr.corp.intel.com>
Date: Fri, 25 Oct 2019 15:52:39 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Lu Baolu <baolu.lu@...ux.intel.com>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>
CC: "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
LKML <linux-kernel@...r.kernel.org>,
Joerg Roedel <joro@...tes.org>,
"David Woodhouse" <dwmw2@...radead.org>,
Alex Williamson <alex.williamson@...hat.com>,
Jean-Philippe Brucker <jean-philippe@...aro.com>,
"Liu, Yi L" <yi.l.liu@...el.com>,
"Raj, Ashok" <ashok.raj@...el.com>,
Christoph Hellwig <hch@...radead.org>,
"Jonathan Cameron" <jic23@...nel.org>,
Eric Auger <eric.auger@...hat.com>
Subject: RE: [PATCH v7 03/11] iommu/vt-d: Add custom allocator for IOASID
> From: Lu Baolu [mailto:baolu.lu@...ux.intel.com]
> Sent: Friday, October 25, 2019 10:39 PM
>
> Hi,
>
> On 10/25/19 2:40 PM, Tian, Kevin wrote:
> >>>> ioasid_register_allocator(&iommu->pasid_allocator);
> >>>> + if (ret) {
> >>>> + pr_warn("Custom PASID allocator
> >>>> registeration failed\n");
> >>>> + /*
> >>>> + * Disable scalable mode on this
> >>>> IOMMU if there
> >>>> + * is no custom allocator. Mixing
> >>>> SM capable vIOMMU
> >>>> + * and non-SM vIOMMU are not
> >>>> supported.
> >>>> + */
> >>>> + intel_iommu_sm = 0;
> >>> It's insufficient to disable scalable mode by only clearing
> >>> intel_iommu_sm. The DMA_RTADDR_SMT bit in root entry has already
> >> been
> >>> set. Probably, you need to
> >>>
> >>> for each iommu
> >>> clear DMA_RTADDR_SMT in root entry
> >>>
> >>> Alternatively, since vSVA is the only customer of this custom PASID
> >>> allocator, is it possible to only disable SVA here?
> >>>
> >> Yeah, I think disable SVA is better. We can still do gIOVA in SM. I
> >> guess we need to introduce a flag for sva_enabled.
> > I'm not sure whether tying above logic to SVA is the right approach.
> > If vcmd interface doesn't work, the whole SM mode doesn't make
> > sense which is based on PASID-granular protection (SVA is only one
> > usage atop). If the only remaining usage of SM is to map gIOVA using
> > reserved PASID#0, then why not disabling SM and just fallback to
> > legacy mode?
> >
> > Based on that I prefer to disabling the SM mode completely (better
> > through an interface), and move the logic out of CONFIG_INTEL_
> > IOMMU_SVM
> >
>
> Unfortunately, it is dangerous to disable SM after boot. SM uses
> different root/device contexts and pasid table formats. Disabling SM
> after boot requires changing above from SM format into legacy format.
You are correct.
>
> Since ioasid registration failure is a rare case. How about moving this
> part of code up to the early stage of intel_iommu_init() and returning
> error if hardware present vcmd capability but software fails to register
> a custom ioasid allocator?
>
It makes sense to me.
Thanks
Kevin
Powered by blists - more mailing lists