[PATCH 1/1] iommu/vt-d: Update the virtual command related registers
The VT-d spec Revision 3.3 updated the virtual command registers, virtual
command opcode B register, virtual command response register and virtual
command capability register (Section 10.4.43, 10.4.44, 10.4.45, 10.4.46).
This updates the virtual command interface implementation in the Intel
IOMMU driver accordingly.
Fixes: 24f27d32ab6b7 ("iommu/vt-d: Enlightened PASID allocation")
Cc: Ashok Raj
Cc: Sanjay Kumar
Cc: Kevin Tian
Signed-off-by: Lu Baolu
---
include/linux/intel-iommu.h | 6 +++---
drivers/iommu/intel/pasid.h | 10 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index d0fa0b31994d..05a65eb155f7 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -124,9 +124,9 @@
#define DMAR_MTRR_PHYSMASK8_REG 0x208
#define DMAR_MTRR_PHYSBASE9_REG 0x210
#define DMAR_MTRR_PHYSMASK9_REG 0x218
-#define DMAR_VCCAP_REG 0xe00 /* Virtual command capability register */
-#define DMAR_VCMD_REG 0xe10 /* Virtual command register */
-#define DMAR_VCRSP_REG 0xe20 /* Virtual command response register */
+#define DMAR_VCCAP_REG 0xe30 /* Virtual command capability register */
+#define DMAR_VCMD_REG 0xe00 /* Virtual command register */
+#define DMAR_VCRSP_REG 0xe10 /* Virtual command response register */
#define DMAR_IQER_REG_IQEI(reg)FIELD_GET(GENMASK_ULL(3, 0),
reg)
#define DMAR_IQER_REG_ITESID(reg) FIELD_GET(GENMASK_ULL(47, 32), reg)
diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index 5ff61c3d401f..8c2efb85fb3b 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -28,12 +28,12 @@
#define VCMD_CMD_ALLOC 0x1
#define VCMD_CMD_FREE 0x2
#define VCMD_VRSP_IP 0x1
-#define VCMD_VRSP_SC(e)(((e) >> 1) & 0x3)
+#define VCMD_VRSP_SC(e)(((e) & 0xff) >> 1)
#define VCMD_VRSP_SC_SUCCESS 0
-#define VCMD_VRSP_SC_NO_PASID_AVAIL2
-#define VCMD_VRSP_SC_INVALID_PASID 2
-#define VCMD_VRSP_RESULT_PASID(e) (((e) >> 8) & 0xf)
-#define VCMD_CMD_OPERAND(e)((e) << 8)
+#define VCMD_VRSP_SC_NO_PASID_AVAIL16
+#define VCMD_VRSP_SC_INVALID_PASID 16
+#define VCMD_VRSP_RESULT_PASID(e) (((e) >> 16) & 0xf)
+#define VCMD_CMD_OPERAND(e)((e) << 16)
/*
* Domain ID reserved for pasid entries programmed for first-level
* only and pass-through transfer modes.
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] iommu/vt-d: Disable superpage for Geminilake igfx
On 7/12/21 11:47 PM, Ville Syrjälä wrote:
On Mon, Jul 12, 2021 at 07:23:07AM +0800, Lu Baolu wrote:
On 7/10/21 12:47 AM, Ville Syrjala wrote:
From: Ville Syrjälä
While running "gem_exec_big --r single" from igt-gpu-tools on
Geminilake as soon as a 2M mapping is made I tend to get a DMAR
write fault. Strangely the faulting address is always a 4K page
and usually very far away from the 2M page that got mapped.
But if no 2M mappings get used I can't reproduce the fault.
I also tried to dump the PTE for the faulting address but it actually
looks correct to me (ie. definitely seems to have the write bit set):
DMAR: DRHD: handling fault status reg 2
DMAR: [DMA Write] Request device [00:02.0] PASID fault addr
7fa8a78000 [fault reason 05] PTE Write access is not set
DMAR: fault 7fa8a78000 (level=1) PTE = 149efc003
So not really sure what's going on and this might just be full on duct
tape, but it seems to work here. The machine has now survived a whole day
running that test whereas with superpage enabled it fails in less than
a minute usually.
TODO: might be nice to disable superpage only for the igfx iommu
instead of both iommus
If all these quirks are about igfx dedicated iommu's, I would suggest to
disable superpage only for the igfx ones.
Sure. Unfortunately there's no convenient mechanism to do that in
the iommu driver that I can immediately see. So not something I
can just whip up easily. Since you're actually familiar with the
driver maybe you can come up with a decent solution for that?
How about something like below? [no compile, no test...]
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 1131b8efb050..2d51ef288a9e 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -338,6 +338,7 @@ static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
static int iommu_identity_mapping;
static int iommu_skip_te_disable;
+static int iommu_skip_igfx_superpage;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
@@ -652,6 +653,27 @@ static bool domain_update_iommu_snooping(struct
intel_iommu *skip)
return ret;
}
+static bool domain_use_super_page(struct dmar_domain *domain)
+{
+ struct dmar_drhd_unit *drhd;
+ struct intel_iommu *iommu;
+ bool ret = true;
+
+ if (!intel_iommu_superpage)
+ return false;
+
+ rcu_read_lock();
+ for_each_active_iommu(iommu, drhd) {
+ if (drhd->gfx_dedicated && iommu_skip_igfx_superpage) {
+ ret = false;
+ break
+ }
+ }
+ rcu_read_unlock();
+
+ return ret;
+}
+
static int domain_update_iommu_superpage(struct dmar_domain *domain,
struct intel_iommu *skip)
{
@@ -659,7 +681,7 @@ static int domain_update_iommu_superpage(struct
dmar_domain *domain,
struct intel_iommu *iommu;
int mask = 0x3;
- if (!intel_iommu_superpage)
+ if (!domain_use_super_page(domain))
return 0;
/* set iommu_superpage to the smallest common denominator */
@@ -5656,6 +5678,14 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,
0x1632, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
+static void quirk_skip_igfx_superpage(struct pci_dev *dev)
+{
+ pci_info(dev, "Disabling IOMMU superpage for graphics on this
chipset\n");
+ iommu_skip_igfx_superpage = 1;
+}
+
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x3184,
quirk_skip_igfx_superpage);
+
static void quirk_iommu_rwbf(struct pci_dev *dev)
{
if (risky_device(dev))
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v2] /dev/iommu uAPI proposal
> From: Alex Williamson
> Sent: Tuesday, July 13, 2021 2:42 AM
>
> On Mon, 12 Jul 2021 01:22:11 +
> "Tian, Kevin" wrote:
> > > From: Alex Williamson
> > > Sent: Saturday, July 10, 2021 5:51 AM
> > > On Fri, 9 Jul 2021 07:48:44 +
> > > "Tian, Kevin" wrote:
>
> > > > For mdev the struct device should be the pointer to the parent device.
> > >
> > > I don't get how iommu_register_device() differentiates an mdev from a
> > > pdev in this case.
> >
> > via device cookie.
>
>
> Let me re-add this section for more context:
>
> > 3. Sample structures and helper functions
> >
> >
> > Three helper functions are provided to support VFIO_BIND_IOMMU_FD:
> >
> > struct iommu_ctx *iommu_ctx_fdget(int fd);
> > struct iommu_dev *iommu_register_device(struct iommu_ctx *ctx,
> > struct device *device, u64 cookie);
> > int iommu_unregister_device(struct iommu_dev *dev);
> >
> > An iommu_ctx is created for each fd:
> >
> > struct iommu_ctx {
> > // a list of allocated IOASID data's
> > struct xarray ioasid_xa;
> >
> > // a list of registered devices
> > struct xarray dev_xa;
> > };
> >
> > Later some group-tracking fields will be also introduced to support
> > multi-devices group.
> >
> > Each registered device is represented by iommu_dev:
> >
> > struct iommu_dev {
> > struct iommu_ctx*ctx;
> > // always be the physical device
> > struct device *device;
> > u64 cookie;
> > struct kref kref;
> > };
> >
> > A successful binding establishes a security context for the bound
> > device and returns struct iommu_dev pointer to the caller. After this
> > point, the user is allowed to query device capabilities via IOMMU_
> > DEVICE_GET_INFO.
> >
> > For mdev the struct device should be the pointer to the parent device.
>
>
> So we'll have a VFIO_DEVICE_BIND_IOMMU_FD ioctl where the user
> provides
> the iommu_fd and a cookie. vfio will use iommu_ctx_fdget() to get an
> iommu_ctx* for that iommu_fd, then we'll call iommu_register_device()
> using that iommu_ctx* we got from the iommu_fd, the cookie provided by
> the user, and for an mdev, the parent of the device the user owns
> (the device_fd on which this ioctl is called)...
>
> How does an arbitrary user provided cookie let you differentiate that
> the request is actually for an mdev versus the parent device itself?
>
Maybe I misunderstood your question. Are you specifically worried
about establishing the security context for a mdev vs. for its parent?
At least in concept we should not change the security context of
the parent if this binding call is just for the mdev. And for mdev it will be
in a security context as long as the associated PASID entry is disabled
at the binding time. If this is the case, possibly we also need VFIO to
provide defPASID marking the mdev when calling iommu_register_device()
then IOMMU fd also provides defPASID when calling IOMMU API to
establish the security context.
Thanks,
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v2] /dev/iommu uAPI proposal
> From: Alex Williamson
> Sent: Tuesday, July 13, 2021 2:42 AM
>
> On Mon, 12 Jul 2021 01:22:11 +
> "Tian, Kevin" wrote:
> > > From: Alex Williamson
> > > Sent: Saturday, July 10, 2021 5:51 AM
> > > On Fri, 9 Jul 2021 07:48:44 +
> > > "Tian, Kevin" wrote:
>
> > > > For mdev the struct device should be the pointer to the parent device.
> > >
> > > I don't get how iommu_register_device() differentiates an mdev from a
> > > pdev in this case.
> >
> > via device cookie.
>
>
> Let me re-add this section for more context:
>
> > 3. Sample structures and helper functions
> >
> >
> > Three helper functions are provided to support VFIO_BIND_IOMMU_FD:
> >
> > struct iommu_ctx *iommu_ctx_fdget(int fd);
> > struct iommu_dev *iommu_register_device(struct iommu_ctx *ctx,
> > struct device *device, u64 cookie);
> > int iommu_unregister_device(struct iommu_dev *dev);
> >
> > An iommu_ctx is created for each fd:
> >
> > struct iommu_ctx {
> > // a list of allocated IOASID data's
> > struct xarray ioasid_xa;
> >
> > // a list of registered devices
> > struct xarray dev_xa;
> > };
> >
> > Later some group-tracking fields will be also introduced to support
> > multi-devices group.
> >
> > Each registered device is represented by iommu_dev:
> >
> > struct iommu_dev {
> > struct iommu_ctx*ctx;
> > // always be the physical device
> > struct device *device;
> > u64 cookie;
> > struct kref kref;
> > };
> >
> > A successful binding establishes a security context for the bound
> > device and returns struct iommu_dev pointer to the caller. After this
> > point, the user is allowed to query device capabilities via IOMMU_
> > DEVICE_GET_INFO.
> >
> > For mdev the struct device should be the pointer to the parent device.
>
>
> So we'll have a VFIO_DEVICE_BIND_IOMMU_FD ioctl where the user
> provides
> the iommu_fd and a cookie. vfio will use iommu_ctx_fdget() to get an
> iommu_ctx* for that iommu_fd, then we'll call iommu_register_device()
> using that iommu_ctx* we got from the iommu_fd, the cookie provided by
> the user, and for an mdev, the parent of the device the user owns
> (the device_fd on which this ioctl is called)...
>
> How does an arbitrary user provided cookie let you differentiate that
> the request is actually for an mdev versus the parent device itself?
>
> For instance, how can the IOMMU layer distinguish GVT-g (mdev) vs GVT-d
> (direct assignment) when both use the same struct device* and cookie is
> just a user provided value? Still confused. Thanks,
>
GVT-g is a special case here since it's purely software-emulated mdev
and reuse the default domain of the parent device. In this case IOASID
is treated as metadata for GVT-g device driver to conduct DMA isolation
in software. We won't install a new page table in the IOMMU just for
GVT-g mdev (this does reminds a missing flag in the attaching call to
indicate this requirement).
What you really care about is about SIOV mdev (with PASID-granular
DMA isolation in the IOMMU) and its parent. In this case mdev and
parent assignment are exclusive. When the parent is already assigned
to an user, it's not managed by the kernel anymore thus no mdev
per se. If mdev is created then it implies that the parent must be
managed by the kernel. In either case the user-provided cookie is
contained only within IOMMU fd. When calling IOMMU-API, it's
always about the routing information (RID, or RID+PASID) provided
in the attaching call.
Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Mon, 12 Jul 2021 01:22:11 +
"Tian, Kevin" wrote:
> > From: Alex Williamson
> > Sent: Saturday, July 10, 2021 5:51 AM
> > On Fri, 9 Jul 2021 07:48:44 +
> > "Tian, Kevin" wrote:
> > > For mdev the struct device should be the pointer to the parent device.
> >
> > I don't get how iommu_register_device() differentiates an mdev from a
> > pdev in this case.
>
> via device cookie.
Let me re-add this section for more context:
> 3. Sample structures and helper functions
>
>
> Three helper functions are provided to support VFIO_BIND_IOMMU_FD:
>
> struct iommu_ctx *iommu_ctx_fdget(int fd);
> struct iommu_dev *iommu_register_device(struct iommu_ctx *ctx,
> struct device *device, u64 cookie);
> int iommu_unregister_device(struct iommu_dev *dev);
>
> An iommu_ctx is created for each fd:
>
> struct iommu_ctx {
> // a list of allocated IOASID data's
> struct xarray ioasid_xa;
>
> // a list of registered devices
> struct xarray dev_xa;
> };
>
> Later some group-tracking fields will be also introduced to support
> multi-devices group.
>
> Each registered device is represented by iommu_dev:
>
> struct iommu_dev {
> struct iommu_ctx*ctx;
> // always be the physical device
> struct device *device;
> u64 cookie;
> struct kref kref;
> };
>
> A successful binding establishes a security context for the bound
> device and returns struct iommu_dev pointer to the caller. After this
> point, the user is allowed to query device capabilities via IOMMU_
> DEVICE_GET_INFO.
>
> For mdev the struct device should be the pointer to the parent device.
So we'll have a VFIO_DEVICE_BIND_IOMMU_FD ioctl where the user provides
the iommu_fd and a cookie. vfio will use iommu_ctx_fdget() to get an
iommu_ctx* for that iommu_fd, then we'll call iommu_register_device()
using that iommu_ctx* we got from the iommu_fd, the cookie provided by
the user, and for an mdev, the parent of the device the user owns
(the device_fd on which this ioctl is called)...
How does an arbitrary user provided cookie let you differentiate that
the request is actually for an mdev versus the parent device itself?
For instance, how can the IOMMU layer distinguish GVT-g (mdev) vs GVT-d
(direct assignment) when both use the same struct device* and cookie is
just a user provided value? Still confused. Thanks,
Alex
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] iommu/vt-d: Disable superpage for Geminilake igfx
On Mon, Jul 12, 2021 at 07:23:07AM +0800, Lu Baolu wrote:
> On 7/10/21 12:47 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > While running "gem_exec_big --r single" from igt-gpu-tools on
> > Geminilake as soon as a 2M mapping is made I tend to get a DMAR
> > write fault. Strangely the faulting address is always a 4K page
> > and usually very far away from the 2M page that got mapped.
> > But if no 2M mappings get used I can't reproduce the fault.
> >
> > I also tried to dump the PTE for the faulting address but it actually
> > looks correct to me (ie. definitely seems to have the write bit set):
> > DMAR: DRHD: handling fault status reg 2
> > DMAR: [DMA Write] Request device [00:02.0] PASID fault addr
> > 7fa8a78000 [fault reason 05] PTE Write access is not set
> > DMAR: fault 7fa8a78000 (level=1) PTE = 149efc003
> >
> > So not really sure what's going on and this might just be full on duct
> > tape, but it seems to work here. The machine has now survived a whole day
> > running that test whereas with superpage enabled it fails in less than
> > a minute usually.
> >
> > TODO: might be nice to disable superpage only for the igfx iommu
> >instead of both iommus
>
> If all these quirks are about igfx dedicated iommu's, I would suggest to
> disable superpage only for the igfx ones.
Sure. Unfortunately there's no convenient mechanism to do that in
the iommu driver that I can immediately see. So not something I
can just whip up easily. Since you're actually familiar with the
driver maybe you can come up with a decent solution for that?
>
> Best regards,
> baolu
>
> > TODO: would be nice to use the macros from include/drm/i915_pciids.h,
> >but can't do that with DECLARE_PCI_FIXUP_HEADER()
> >
> > Cc: David Woodhouse
> > Cc: Lu Baolu
> > Cc: iommu@lists.linux-foundation.org
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/iommu/intel/iommu.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 19c7888cbb86..4fff2c9c86af 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -5617,6 +5617,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,
> > 0x1632, quirk_iommu_igfx);
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
> >
> > +static void quirk_iommu_nosp(struct pci_dev *dev)
> > +{
> > + pci_info(dev, "Disabling IOMMU superpage for graphics on this
> > chipset\n");
> > + intel_iommu_superpage = 0;
> > +}
> > +
> > +/* Geminilake igfx appears to have issues with superpage */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x3184, quirk_iommu_nosp);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x3185, quirk_iommu_nosp);
> > +
> > static void quirk_iommu_rwbf(struct pci_dev *dev)
> > {
> > if (risky_device(dev))
> >
--
Ville Syrjälä
Intel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dt-bindings: arm-smmu: Fix json-schema syntax
On Tue, Jun 22, 2021 at 11:56 PM Krzysztof Kozlowski
wrote:
>
> On Mon, 21 Jun 2021 16:00:36 +0200, Thierry Reding wrote:
> > Commit 4287861dca9d ("dt-bindings: arm-smmu: Add Tegra186 compatible
> > string") introduced a jsonschema syntax error as a result of a rebase
> > gone wrong. Fix it.
>
> Applied, thanks!
>
> [1/1] dt-bindings: arm-smmu: Fix json-schema syntax
> commit: bf3ec9deaa33889630722c47f7bb86ba58872ea7
Applied where? Now Linus's master is broken.
Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Tue, Jul 06, 2021 at 12:59:57PM -0400, Konrad Rzeszutek Wilk wrote: > On Tue, Jul 06, 2021 at 05:57:21PM +0100, Will Deacon wrote: > > On Tue, Jul 06, 2021 at 10:46:07AM -0400, Konrad Rzeszutek Wilk wrote: > > > On Tue, Jul 06, 2021 at 04:05:13PM +0200, Christoph Hellwig wrote: > > > > On Tue, Jul 06, 2021 at 03:01:04PM +0100, Robin Murphy wrote: > > > > > FWIW I was pondering the question of whether to do something along > > > > > those > > > > > lines or just scrap the default assignment entirely, so since I > > > > > hadn't got > > > > > round to saying that I've gone ahead and hacked up the alternative > > > > > (similarly untested) for comparison :) > > > > > > > > > > TBH I'm still not sure which one I prefer... > > > > > > > > Claire did implement something like your suggestion originally, but > > > > I don't really like it as it doesn't scale for adding multiple global > > > > pools, e.g. for the 64-bit addressable one for the various encrypted > > > > secure guest schemes. > > > > > > Couple of things: > > > - I am not pushing to Linus the Claire's patchset until we have a > > >resolution on this. I hope you all agree that is a sensible way > > >forward as much as I hate doing that. > > > > Sure, it's a pity but we could clearly use a bit more time to get these > > just right and we've run out of time for 5.14. > > > > I think the main question I have is how would you like to see patches for > > 5.15? i.e. as patches on top of devel/for-linus-5.14 or something else? > > Yes that would be perfect. If there are any dependencies on the rc1, I > can rebase it on top of that. Yes, please, rebasing would be very helpful. The broader rework of 'io_tlb_default_mem' is going to conflict quite badly otherwise. Cheers, Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
> > Should we be checking alignment here? Something like > > > > BUG_ON(paddr & ((1 << DART_TTBR_SHIFT) - 1)); > > > > Sure, right now paddr will always be aligned but adding that > BUG_ON doesn't hurt :) Probably should have suggested WARN_ON instead of BUG_ON but yes. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v15 6/6] iommu: Remove mode argument from iommu_set_dma_strict()
We only ever now set strict mode enabled in iommu_set_dma_strict(), so
just remove the argument.
Signed-off-by: John Garry
Reviewed-by: Robin Murphy
Reviewed-by: Lu Baolu
---
drivers/iommu/amd/init.c| 2 +-
drivers/iommu/intel/iommu.c | 6 +++---
drivers/iommu/iommu.c | 5 ++---
include/linux/iommu.h | 2 +-
4 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 1e641cb6dddc..6e12a615117b 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3099,7 +3099,7 @@ static int __init parse_amd_iommu_options(char *str)
for (; *str; ++str) {
if (strncmp(str, "fullflush", 9) == 0) {
pr_warn("amd_iommu=fullflush deprecated; use
iommu.strict=1 instead\n");
- iommu_set_dma_strict(true);
+ iommu_set_dma_strict();
}
if (strncmp(str, "force_enable", 12) == 0)
amd_iommu_force_enable = true;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index d06e8f71c259..089c2607 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -454,7 +454,7 @@ static int __init intel_iommu_setup(char *str)
iommu_dma_forcedac = true;
} else if (!strncmp(str, "strict", 6)) {
pr_warn("intel_iommu=strict deprecated; use
iommu.strict=1 instead\n");
- iommu_set_dma_strict(true);
+ iommu_set_dma_strict();
} else if (!strncmp(str, "sp_off", 6)) {
pr_info("Disable supported super page\n");
intel_iommu_superpage = 0;
@@ -4382,7 +4382,7 @@ int __init intel_iommu_init(void)
*/
if (cap_caching_mode(iommu->cap)) {
pr_info_once("IOMMU batching disallowed due to
virtualization\n");
- iommu_set_dma_strict(true);
+ iommu_set_dma_strict();
}
iommu_device_sysfs_add(&iommu->iommu, NULL,
intel_iommu_groups,
@@ -5700,7 +5700,7 @@ static void quirk_calpella_no_shadow_gtt(struct pci_dev
*dev)
} else if (dmar_map_gfx) {
/* we have to ensure the gfx device is idle before we flush */
pci_info(dev, "Disabling batched IOTLB flush on Ironlake\n");
- iommu_set_dma_strict(true);
+ iommu_set_dma_strict();
}
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0040,
quirk_calpella_no_shadow_gtt);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 60b1ec42e73b..ff221d3ddcbc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -349,10 +349,9 @@ static int __init iommu_dma_setup(char *str)
}
early_param("iommu.strict", iommu_dma_setup);
-void iommu_set_dma_strict(bool strict)
+void iommu_set_dma_strict(void)
{
- if (strict || !(iommu_cmd_line & IOMMU_CMD_LINE_STRICT))
- iommu_dma_strict = strict;
+ iommu_dma_strict = true;
}
bool iommu_get_dma_strict(struct iommu_domain *domain)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..754f67d6dd90 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -476,7 +476,7 @@ int iommu_enable_nesting(struct iommu_domain *domain);
int iommu_set_pgtable_quirks(struct iommu_domain *domain,
unsigned long quirks);
-void iommu_set_dma_strict(bool val);
+void iommu_set_dma_strict(void);
bool iommu_get_dma_strict(struct iommu_domain *domain);
extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v15 5/6] iommu/amd: Add support for IOMMU default DMA mode build options
From: Zhen Lei
Make IOMMU_DEFAULT_LAZY default for when AMD_IOMMU config is set, which
matches current behaviour.
For "fullflush" param, just call iommu_set_dma_strict(true) directly.
Since we get a strict vs lazy mode print already in iommu_subsys_init(),
and maintain a deprecation print when "fullflush" param is passed, drop the
prints in amd_iommu_init_dma_ops().
Finally drop global flag amd_iommu_unmap_flush, as it has no longer has any
purpose.
[jpg: Rebase for relocated file and drop amd_iommu_unmap_flush]
Signed-off-by: Zhen Lei
Signed-off-by: John Garry
---
drivers/iommu/Kconfig | 2 +-
drivers/iommu/amd/amd_iommu_types.h | 6 --
drivers/iommu/amd/init.c| 3 +--
drivers/iommu/amd/iommu.c | 6 --
4 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 265d7a6c9d3a..c84da8205be7 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -94,7 +94,7 @@ choice
prompt "IOMMU default DMA IOTLB invalidation mode"
depends on IOMMU_DMA
- default IOMMU_DEFAULT_LAZY if INTEL_IOMMU
+ default IOMMU_DEFAULT_LAZY if (AMD_IOMMU || INTEL_IOMMU)
default IOMMU_DEFAULT_STRICT
help
This option allows an IOMMU DMA IOTLB invalidation mode to be
diff --git a/drivers/iommu/amd/amd_iommu_types.h
b/drivers/iommu/amd/amd_iommu_types.h
index 94c1a7a9876d..8dbe61e2b3c1 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -779,12 +779,6 @@ extern u16 amd_iommu_last_bdf;
/* allocation bitmap for domain ids */
extern unsigned long *amd_iommu_pd_alloc_bitmap;
-/*
- * If true, the addresses will be flushed on unmap time, not when
- * they are reused
- */
-extern bool amd_iommu_unmap_flush;
-
/* Smallest max PASID supported by any IOMMU in the system */
extern u32 amd_iommu_max_pasid;
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 3a2fb805f11e..1e641cb6dddc 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -161,7 +161,6 @@ u16 amd_iommu_last_bdf; /* largest PCI
device id we have
to handle */
LIST_HEAD(amd_iommu_unity_map);/* a list of required unity
mappings
we find in ACPI */
-bool amd_iommu_unmap_flush;/* if true, flush on every unmap */
LIST_HEAD(amd_iommu_list); /* list of all AMD IOMMUs in the
system */
@@ -3100,7 +3099,7 @@ static int __init parse_amd_iommu_options(char *str)
for (; *str; ++str) {
if (strncmp(str, "fullflush", 9) == 0) {
pr_warn("amd_iommu=fullflush deprecated; use
iommu.strict=1 instead\n");
- amd_iommu_unmap_flush = true;
+ iommu_set_dma_strict(true);
}
if (strncmp(str, "force_enable", 12) == 0)
amd_iommu_force_enable = true;
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 811a49a95d04..52fe2326042a 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1775,12 +1775,6 @@ void amd_iommu_domain_update(struct protection_domain
*domain)
static void __init amd_iommu_init_dma_ops(void)
{
swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
-
- if (amd_iommu_unmap_flush)
- pr_info("IO/TLB flush on unmap enabled\n");
- else
- pr_info("Lazy IO/TLB flushing enabled\n");
- iommu_set_dma_strict(amd_iommu_unmap_flush);
}
int __init amd_iommu_init_api(void)
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v15 4/6] iommu/vt-d: Add support for IOMMU default DMA mode build options
From: Zhen Lei
Make IOMMU_DEFAULT_LAZY default for when INTEL_IOMMU config is set,
as is current behaviour.
Also delete global flag intel_iommu_strict:
- In intel_iommu_setup(), call iommu_set_dma_strict(true) directly. Also
remove the print, as iommu_subsys_init() prints the mode and we have
already marked this param as deprecated.
- For cap_caching_mode() check in intel_iommu_setup(), call
iommu_set_dma_strict(true) directly; also reword the accompanying print
with a level downgrade and also add the missing '\n'.
- For Ironlake GPU, again call iommu_set_dma_strict(true) directly and
keep the accompanying print.
[jpg: Remove intel_iommu_strict]
Signed-off-by: Zhen Lei
Signed-off-by: John Garry
Reviewed-by: Lu Baolu
---
drivers/iommu/Kconfig | 1 +
drivers/iommu/intel/iommu.c | 15 ++-
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 9cd5d7afc766..265d7a6c9d3a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -94,6 +94,7 @@ choice
prompt "IOMMU default DMA IOTLB invalidation mode"
depends on IOMMU_DMA
+ default IOMMU_DEFAULT_LAZY if INTEL_IOMMU
default IOMMU_DEFAULT_STRICT
help
This option allows an IOMMU DMA IOTLB invalidation mode to be
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 67c52b60f8de..d06e8f71c259 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -361,7 +361,6 @@ int intel_iommu_enabled = 0;
EXPORT_SYMBOL_GPL(intel_iommu_enabled);
static int dmar_map_gfx = 1;
-static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
static int iommu_identity_mapping;
static int iommu_skip_te_disable;
@@ -455,8 +454,7 @@ static int __init intel_iommu_setup(char *str)
iommu_dma_forcedac = true;
} else if (!strncmp(str, "strict", 6)) {
pr_warn("intel_iommu=strict deprecated; use
iommu.strict=1 instead\n");
- pr_info("Disable batched IOTLB flush\n");
- intel_iommu_strict = 1;
+ iommu_set_dma_strict(true);
} else if (!strncmp(str, "sp_off", 6)) {
pr_info("Disable supported super page\n");
intel_iommu_superpage = 0;
@@ -4382,9 +4380,9 @@ int __init intel_iommu_init(void)
* is likely to be much lower than the overhead of synchronizing
* the virtual and physical IOMMU page-tables.
*/
- if (!intel_iommu_strict && cap_caching_mode(iommu->cap)) {
- pr_warn("IOMMU batching is disabled due to
virtualization");
- intel_iommu_strict = 1;
+ if (cap_caching_mode(iommu->cap)) {
+ pr_info_once("IOMMU batching disallowed due to
virtualization\n");
+ iommu_set_dma_strict(true);
}
iommu_device_sysfs_add(&iommu->iommu, NULL,
intel_iommu_groups,
@@ -4393,7 +4391,6 @@ int __init intel_iommu_init(void)
}
up_read(&dmar_global_lock);
- iommu_set_dma_strict(intel_iommu_strict);
bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
if (si_domain && !hw_pass_through)
register_memory_notifier(&intel_iommu_memory_nb);
@@ -5703,8 +5700,8 @@ static void quirk_calpella_no_shadow_gtt(struct pci_dev
*dev)
} else if (dmar_map_gfx) {
/* we have to ensure the gfx device is idle before we flush */
pci_info(dev, "Disabling batched IOTLB flush on Ironlake\n");
- intel_iommu_strict = 1;
- }
+ iommu_set_dma_strict(true);
+ }
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0040,
quirk_calpella_no_shadow_gtt);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0044,
quirk_calpella_no_shadow_gtt);
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v15 3/6] iommu: Enhance IOMMU default DMA mode build options
From: Zhen Lei
First, add build options IOMMU_DEFAULT_{LAZY|STRICT}, so that we have the
opportunity to set {lazy|strict} mode as default at build time. Then put
the two config options in an choice, as they are mutually exclusive.
[jpg: Make choice between strict and lazy only (and not passthrough)]
Signed-off-by: Zhen Lei
Signed-off-by: John Garry
Reviewed-by: Robin Murphy
Reviewed-by: Lu Baolu
---
.../admin-guide/kernel-parameters.txt | 3 +-
drivers/iommu/Kconfig | 40 +++
drivers/iommu/iommu.c | 2 +-
3 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index a04d2748c99a..90b525cf0ec2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2042,9 +2042,10 @@
throughput at the cost of reduced device isolation.
Will fall back to strict mode if not supported by
the relevant IOMMU driver.
- 1 - Strict mode (default).
+ 1 - Strict mode.
DMA unmap operations invalidate IOMMU hardware TLBs
synchronously.
+ unset - Use value of CONFIG_IOMMU_DEFAULT_{LAZY,STRICT}.
Note: on x86, the default behaviour depends on the
equivalent driver-specific parameters, but a strict
mode explicitly specified by either method takes
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 07b7c25cbed8..9cd5d7afc766 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -90,6 +90,46 @@ config IOMMU_DEFAULT_PASSTHROUGH
If unsure, say N here.
+choice
+ prompt "IOMMU default DMA IOTLB invalidation mode"
+ depends on IOMMU_DMA
+
+ default IOMMU_DEFAULT_STRICT
+ help
+ This option allows an IOMMU DMA IOTLB invalidation mode to be
+ chosen at build time, to override the default mode of each ARCH,
+ removing the need to pass in kernel parameters through command line.
+ It is still possible to provide common boot params to override this
+ config.
+
+ If unsure, keep the default.
+
+config IOMMU_DEFAULT_STRICT
+ bool "strict"
+ help
+ For every IOMMU DMA unmap operation, the flush operation of IOTLB and
+ the free operation of IOVA are guaranteed to be done in the unmap
+ function.
+
+config IOMMU_DEFAULT_LAZY
+ bool "lazy"
+ help
+ Support lazy mode, where for every IOMMU DMA unmap operation, the
+ flush operation of IOTLB and the free operation of IOVA are deferred.
+ They are only guaranteed to be done before the related IOVA will be
+ reused.
+
+ The isolation provided in this mode is not as secure as STRICT mode,
+ such that a vulnerable time window may be created between the DMA
+ unmap and the mappings cached in the IOMMU IOTLB or device TLB
+ finally being invalidated, where the device could still access the
+ memory which has already been unmapped by the device driver.
+ However this mode may provide better performance in high throughput
+ scenarios, and is still considerably more secure than passthrough
+ mode or no IOMMU.
+
+endchoice
+
config OF_IOMMU
def_bool y
depends on OF && IOMMU_API
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cf58949cc2f3..60b1ec42e73b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -29,7 +29,7 @@ static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
static unsigned int iommu_def_domain_type __read_mostly;
-static bool iommu_dma_strict __read_mostly = true;
+static bool iommu_dma_strict __read_mostly =
IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
static u32 iommu_cmd_line __read_mostly;
struct iommu_group {
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v15 2/6] iommu: Print strict or lazy mode at init time
As well as the default domain type, it's useful to know whether strict
or lazy for DMA domains, so add this info in a separate print.
The (stict/lazy) mode may be also set via iommu.strict earlyparm, but
this will be processed prior to iommu_subsys_init(), so that print will be
accurate for drivers which don't set the mode via custom means.
For the drivers which set the mode via custom means - AMD and Intel drivers
- they maintain prints to inform a change in policy or that custom cmdline
methods to change policy are deprecated.
Signed-off-by: John Garry
Reviewed-by: Robin Murphy
Reviewed-by: Lu Baolu
---
drivers/iommu/iommu.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..cf58949cc2f3 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -138,6 +138,11 @@ static int __init iommu_subsys_init(void)
(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
"(set via kernel command line)" : "");
+ pr_info("DMA domain TLB invalidation policy: %s mode %s\n",
+ iommu_dma_strict ? "strict" : "lazy",
+ (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
+ "(set via kernel command line)" : "");
+
return 0;
}
subsys_initcall(iommu_subsys_init);
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v15 1/6] iommu: Deprecate Intel and AMD cmdline methods to enable strict mode
Now that the x86 drivers support iommu.strict, deprecate the custom
methods.
Signed-off-by: John Garry
Acked-by: Robin Murphy
Reviewed-by: Lu Baolu
---
Documentation/admin-guide/kernel-parameters.txt | 9 ++---
drivers/iommu/amd/init.c| 4 +++-
drivers/iommu/intel/iommu.c | 1 +
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index bdb22006f713..a04d2748c99a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -290,10 +290,7 @@
amd_iommu= [HW,X86-64]
Pass parameters to the AMD IOMMU driver in the system.
Possible values are:
- fullflush - enable flushing of IO/TLB entries when
- they are unmapped. Otherwise they are
- flushed before they will be reused, which
- is a lot of faster
+ fullflush - Deprecated, equivalent to iommu.strict=1
off - do not initialize any AMD IOMMU found in
the system
force_isolation - Force device isolation for all
@@ -1944,9 +1941,7 @@
this case, gfx device will use physical address for
DMA.
strict [Default Off]
- With this option on every unmap_single operation will
- result in a hardware IOTLB flush operation as opposed
- to batching them for performance.
+ Deprecated, equivalent to iommu.strict=1.
sp_off [Default Off]
By default, super page will be supported if Intel IOMMU
has the capability. With this option, super page will
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 46280e6e1535..3a2fb805f11e 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3098,8 +3098,10 @@ static int __init parse_amd_iommu_intr(char *str)
static int __init parse_amd_iommu_options(char *str)
{
for (; *str; ++str) {
- if (strncmp(str, "fullflush", 9) == 0)
+ if (strncmp(str, "fullflush", 9) == 0) {
+ pr_warn("amd_iommu=fullflush deprecated; use
iommu.strict=1 instead\n");
amd_iommu_unmap_flush = true;
+ }
if (strncmp(str, "force_enable", 12) == 0)
amd_iommu_force_enable = true;
if (strncmp(str, "off", 3) == 0)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index a6a07d985709..67c52b60f8de 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -454,6 +454,7 @@ static int __init intel_iommu_setup(char *str)
pr_warn("intel_iommu=forcedac deprecated; use
iommu.forcedac instead\n");
iommu_dma_forcedac = true;
} else if (!strncmp(str, "strict", 6)) {
+ pr_warn("intel_iommu=strict deprecated; use
iommu.strict=1 instead\n");
pr_info("Disable batched IOTLB flush\n");
intel_iommu_strict = 1;
} else if (!strncmp(str, "sp_off", 6)) {
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v15 0/6] iommu: Enhance IOMMU default DMA mode build options
This is a reboot of Zhen Lei's series from a couple of years ago, which never made it across the line. I still think that it has some value, so taking up the mantle. Motivation: Allow lazy mode be default mode for DMA domains for all ARCHs, and not only those who hardcode it (to be lazy). For ARM64, currently we must use a kernel command line parameter to use lazy mode, which is less than ideal. I have now included the print for strict/lazy mode, which I originally sent in: https://lore.kernel.org/linux-iommu/72eb3de9-1d1c-ae46-c5a9-95f26525d...@huawei.com/ There was some concern there about drivers and their custom prints conflicting with the print in that patch, but I think that it should be ok. Based on v5.14-rc1 Differences to v14: - Add RB tags (Thanks!) - Rebase Differences to v13: - Improve strict mode deprecation messages and cut out some kernel-parameters.txt legacy description - Add tag in 1/6 - use pr_info_once() for vt-d message about VM and caching Differences to v12: - Add Robin's RB tags (thanks!) - Add a patch to mark x86 strict cmdline params as deprecated - Improve wording in Kconfig change and tweak iommu_dma_strict declaration John Garry (3): iommu: Deprecate Intel and AMD cmdline methods to enable strict mode iommu: Print strict or lazy mode at init time iommu: Remove mode argument from iommu_set_dma_strict() Zhen Lei (3): iommu: Enhance IOMMU default DMA mode build options iommu/vt-d: Add support for IOMMU default DMA mode build options iommu/amd: Add support for IOMMU default DMA mode build options .../admin-guide/kernel-parameters.txt | 12 ++ drivers/iommu/Kconfig | 41 +++ drivers/iommu/amd/amd_iommu_types.h | 6 --- drivers/iommu/amd/init.c | 7 ++-- drivers/iommu/amd/iommu.c | 6 --- drivers/iommu/intel/iommu.c | 16 drivers/iommu/iommu.c | 12 -- include/linux/iommu.h | 2 +- 8 files changed, 65 insertions(+), 37 deletions(-) -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
Hi,
On Wed, Jun 30, 2021, at 15:49, Alyssa Rosenzweig wrote:
> Looks really good! Just a few minor comments. With them addressed,
>
> Reviewed-by: Alyssa Rosenzweig
Thanks!
>
> > + Say Y here if you are using an Apple SoC with a DART IOMMU.
>
> Nit: Do we need to spell out "with a DART IOMMU"? Don't all the apple
> socs need DART?
Good point, I'll remove it.
>
> > +/*
> > + * This structure is used to identify a single stream attached to a domain.
> > + * It's used as a list inside that domain to be able to attach multiple
> > + * streams to a single domain. Since multiple devices can use a single
> > stream
> > + * it additionally keeps track of how many devices are represented by this
> > + * stream. Once that number reaches zero it is detached from the IOMMU
> > domain
> > + * and all translations from this stream are disabled.
> > + *
> > + * @dart: DART instance to which this stream belongs
> > + * @sid: stream id within the DART instance
> > + * @num_devices: count of devices attached to this stream
> > + * @stream_head: list head for the next stream
> > + */
> > +struct apple_dart_stream {
> > + struct apple_dart *dart;
> > + u32 sid;
> > +
> > + u32 num_devices;
> > +
> > + struct list_head stream_head;
> > +};
>
> It wasn't obvious to me why we can get away without reference counting.
> Looking ahead it looks like we assert locks in each case. Maybe add
> that to the comment?
Sure, I'll add that to the comment.
>
> ```
> > +static void apple_dart_hw_set_ttbr(struct apple_dart *dart, u16 sid, u16
> > idx,
> > + phys_addr_t paddr)
> > +{
> > + writel(DART_TTBR_VALID | (paddr >> DART_TTBR_SHIFT),
> > + dart->regs + DART_TTBR(sid, idx));
> > +}
> ```
>
> Should we be checking alignment here? Something like
>
> BUG_ON(paddr & ((1 << DART_TTBR_SHIFT) - 1));
>
Sure, right now paddr will always be aligned but adding that
BUG_ON doesn't hurt :)
Best,
Sven
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3] iommu/rockchip: Fix physical address decoding
Restore bits 39 to 32 at correct position.
It reverses the operation done in rk_dma_addr_dte_v2().
Fixes: c55356c534aa ("iommu: rockchip: Add support for iommu v2")
Reported-by: Dan Carpenter
Signed-off-by: Benjamin Gaignard
---
version 3:
- change commit header to match with IOMMU tree convention
drivers/iommu/rockchip-iommu.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 94b9d8e5b9a40..9febfb7f3025b 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -544,12 +544,14 @@ static inline u32 rk_dma_addr_dte(dma_addr_t dt_dma)
}
#define DT_HI_MASK GENMASK_ULL(39, 32)
+#define DTE_BASE_HI_MASK GENMASK(11, 4)
#define DT_SHIFT 28
static inline phys_addr_t rk_dte_addr_phys_v2(u32 addr)
{
- return (phys_addr_t)(addr & RK_DTE_PT_ADDRESS_MASK) |
- ((addr & DT_HI_MASK) << DT_SHIFT);
+ u64 addr64 = addr;
+ return (phys_addr_t)(addr64 & RK_DTE_PT_ADDRESS_MASK) |
+ ((addr64 & DTE_BASE_HI_MASK) << DT_SHIFT);
}
static inline u32 rk_dma_addr_dte_v2(dma_addr_t dt_dma)
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH V3 08/11] swiotlb: Add bounce buffer remap address setting function
Hi Christoph and Robin: I introduced new interface set_memory_decrypted_map() to hide all the hypervisor code behind it in the latest version. In current generic code, only swiotlb bounce buffer needs to be decrypted and remapped in the same time and so keep set_memory_decrypted(). If there were more requests of set_memory_decrypted_map() for other platform, we may replace set_memory_decrypted() step by step. Please have a look. https://lkml.org/lkml/2021/7/7/570 Thanks. On 6/15/2021 11:24 PM, Tianyu Lan wrote: On 6/14/2021 11:32 PM, Christoph Hellwig wrote: On Mon, Jun 14, 2021 at 02:49:51PM +0100, Robin Murphy wrote: FWIW, I think a better generalisation for this would be allowing set_memory_decrypted() to return an address rather than implicitly operating in-place, and hide all the various hypervisor hooks behind that. Yes, something like that would be a good idea. As-is set_memory_decrypted is a pretty horribly API anyway due to passing the address as void, and taking a size parameter while it works in units of pages. So I'd very much welcome a major overhaul of this API. Hi Christoph and Robin: Thanks for your suggestion. I will try this idea in the next version. Besides make the address translation into set_memory_ decrypted() and return address, do you want to make other changes to the API in order to make it more reasonable(e.g size parameter)? Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5.13 677/800] iommu/amd: Fix extended features logging
From: Alexander Monakov
[ Upstream commit 4b21a503adf597773e4b37db05db0e9b16a81d53 ]
print_iommu_info prints the EFR register and then the decoded list of
features on a separate line:
pci :00:00.2: AMD-Vi: Extended features (0x206d73ef22254ade):
PPR X2APIC NX GT IA GA PC GA_vAPIC
The second line is emitted via 'pr_cont', which causes it to have a
different ('warn') loglevel compared to the previous line ('info').
Commit 9a295ff0ffc9 attempted to rectify this by removing the newline
from the pci_info format string, but this doesn't work, as pci_info
calls implicitly append a newline anyway.
Printing the decoded features on the same line would make it quite long.
Instead, change pci_info() to pr_info() to omit PCI bus location info,
which is also shown in the preceding message. This results in:
pci :00:00.2: AMD-Vi: Found IOMMU cap 0x40
AMD-Vi: Extended features (0x206d73ef22254ade): PPR X2APIC NX GT IA GA PC
GA_vAPIC
AMD-Vi: Interrupt remapping enabled
Fixes: 9a295ff0ffc9 ("iommu/amd: Print extended features in one line to fix
divergent log levels")
Link:
https://lore.kernel.org/lkml/alpine.lnx.2.20.13.2104112326460.11...@monopod.intra.ispras.ru
Signed-off-by: Alexander Monakov
Cc: Paul Menzel
Cc: Joerg Roedel
Cc: Suravee Suthikulpanit
Cc: iommu@lists.linux-foundation.org
Reviewed-by: Paul Menzel
Link: https://lore.kernel.org/r/20210504102220.1793-1-amona...@ispras.ru
Signed-off-by: Joerg Roedel
Signed-off-by: Sasha Levin
---
drivers/iommu/amd/init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index d006724f4dc2..1ded8a69c246 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1908,8 +1908,8 @@ static void print_iommu_info(void)
pci_info(pdev, "Found IOMMU cap 0x%x\n", iommu->cap_ptr);
if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
- pci_info(pdev, "Extended features (%#llx):",
-iommu->features);
+ pr_info("Extended features (%#llx):", iommu->features);
+
for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
if (iommu_feature(iommu, (1ULL << i)))
pr_cont(" %s", feat_str[i]);
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5.12 591/700] iommu/amd: Fix extended features logging
From: Alexander Monakov
[ Upstream commit 4b21a503adf597773e4b37db05db0e9b16a81d53 ]
print_iommu_info prints the EFR register and then the decoded list of
features on a separate line:
pci :00:00.2: AMD-Vi: Extended features (0x206d73ef22254ade):
PPR X2APIC NX GT IA GA PC GA_vAPIC
The second line is emitted via 'pr_cont', which causes it to have a
different ('warn') loglevel compared to the previous line ('info').
Commit 9a295ff0ffc9 attempted to rectify this by removing the newline
from the pci_info format string, but this doesn't work, as pci_info
calls implicitly append a newline anyway.
Printing the decoded features on the same line would make it quite long.
Instead, change pci_info() to pr_info() to omit PCI bus location info,
which is also shown in the preceding message. This results in:
pci :00:00.2: AMD-Vi: Found IOMMU cap 0x40
AMD-Vi: Extended features (0x206d73ef22254ade): PPR X2APIC NX GT IA GA PC
GA_vAPIC
AMD-Vi: Interrupt remapping enabled
Fixes: 9a295ff0ffc9 ("iommu/amd: Print extended features in one line to fix
divergent log levels")
Link:
https://lore.kernel.org/lkml/alpine.lnx.2.20.13.2104112326460.11...@monopod.intra.ispras.ru
Signed-off-by: Alexander Monakov
Cc: Paul Menzel
Cc: Joerg Roedel
Cc: Suravee Suthikulpanit
Cc: iommu@lists.linux-foundation.org
Reviewed-by: Paul Menzel
Link: https://lore.kernel.org/r/20210504102220.1793-1-amona...@ispras.ru
Signed-off-by: Joerg Roedel
Signed-off-by: Sasha Levin
---
drivers/iommu/amd/init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index df7b19ff0a9e..ecc7308130ba 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1911,8 +1911,8 @@ static void print_iommu_info(void)
pci_info(pdev, "Found IOMMU cap 0x%x\n", iommu->cap_ptr);
if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
- pci_info(pdev, "Extended features (%#llx):",
-iommu->features);
+ pr_info("Extended features (%#llx):", iommu->features);
+
for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
if (iommu_feature(iommu, (1ULL << i)))
pr_cont(" %s", feat_str[i]);
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] iommu/vt-d: Fix clearing real DMA device's scalable-mode context entries
The commit 2b0140c69637e ("iommu/vt-d: Use pci_real_dma_dev() for mapping")
fixes an issue of "sub-device is removed where the context entry is cleared
for all aliases". But this commit didn't consider the PASID entry and PASID
table in VT-d scalable mode. This fix increases the coverage of scalable
mode.
Suggested-by: Sanjay Kumar
Fixes: 8038bdb855331 ("iommu/vt-d: Only clear real DMA device's context
entries")
Fixes: 2b0140c69637e ("iommu/vt-d: Use pci_real_dma_dev() for mapping")
Cc: sta...@vger.kernel.org # v5.6+
Cc: Jon Derrick
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 57270290d62b..dd22fc7d5176 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4472,14 +4472,13 @@ static void __dmar_remove_one_dev_info(struct
device_domain_info *info)
iommu = info->iommu;
domain = info->domain;
- if (info->dev) {
+ if (info->dev && !dev_is_real_dma_subdevice(info->dev)) {
if (dev_is_pci(info->dev) && sm_supported(iommu))
intel_pasid_tear_down_entry(iommu, info->dev,
PASID_RID2PASID, false);
iommu_disable_dev_iotlb(info);
- if (!dev_is_real_dma_subdevice(info->dev))
- domain_context_clear(info);
+ domain_context_clear(info);
intel_pasid_free_table(info->dev);
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] iommu/vt-d: Global devTLB flush when present context entry changed
From: Sanjay Kumar
This fixes a bug in context cache clear operation. The code was not
following the correct invalidation flow. A global device TLB invalidation
should be added after the IOTLB invalidation. At the same time, it
uses the domain ID from the context entry. But in scalable mode, the
domain ID is in PASID table entry, not context entry.
Fixes: 7373a8cc38197 ("iommu/vt-d: Setup context and enable RID2PASID support")
Cc: sta...@vger.kernel.org # v5.0+
Signed-off-by: Sanjay Kumar
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 31 ++-
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index a6a07d985709..57270290d62b 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2429,10 +2429,11 @@ __domain_mapping(struct dmar_domain *domain, unsigned
long iov_pfn,
return 0;
}
-static void domain_context_clear_one(struct intel_iommu *iommu, u8 bus, u8
devfn)
+static void domain_context_clear_one(struct device_domain_info *info, u8 bus,
u8 devfn)
{
- unsigned long flags;
+ struct intel_iommu *iommu = info->iommu;
struct context_entry *context;
+ unsigned long flags;
u16 did_old;
if (!iommu)
@@ -2444,7 +2445,16 @@ static void domain_context_clear_one(struct intel_iommu
*iommu, u8 bus, u8 devfn
spin_unlock_irqrestore(&iommu->lock, flags);
return;
}
- did_old = context_domain_id(context);
+
+ if (sm_supported(iommu)) {
+ if (hw_pass_through && domain_type_is_si(info->domain))
+ did_old = FLPT_DEFAULT_DID;
+ else
+ did_old = info->domain->iommu_did[iommu->seq_id];
+ } else {
+ did_old = context_domain_id(context);
+ }
+
context_clear_entry(context);
__iommu_flush_cache(iommu, context, sizeof(*context));
spin_unlock_irqrestore(&iommu->lock, flags);
@@ -2462,6 +2472,8 @@ static void domain_context_clear_one(struct intel_iommu
*iommu, u8 bus, u8 devfn
0,
0,
DMA_TLB_DSI_FLUSH);
+
+ __iommu_flush_dev_iotlb(info, 0, MAX_AGAW_PFN_WIDTH);
}
static inline void unlink_domain_info(struct device_domain_info *info)
@@ -4425,9 +4437,9 @@ int __init intel_iommu_init(void)
static int domain_context_clear_one_cb(struct pci_dev *pdev, u16 alias, void
*opaque)
{
- struct intel_iommu *iommu = opaque;
+ struct device_domain_info *info = opaque;
- domain_context_clear_one(iommu, PCI_BUS_NUM(alias), alias & 0xff);
+ domain_context_clear_one(info, PCI_BUS_NUM(alias), alias & 0xff);
return 0;
}
@@ -4437,12 +4449,13 @@ static int domain_context_clear_one_cb(struct pci_dev
*pdev, u16 alias, void *op
* devices, unbinding the driver from any one of them will possibly leave
* the others unable to operate.
*/
-static void domain_context_clear(struct intel_iommu *iommu, struct device *dev)
+static void domain_context_clear(struct device_domain_info *info)
{
- if (!iommu || !dev || !dev_is_pci(dev))
+ if (!info->iommu || !info->dev || !dev_is_pci(info->dev))
return;
- pci_for_each_dma_alias(to_pci_dev(dev), &domain_context_clear_one_cb,
iommu);
+ pci_for_each_dma_alias(to_pci_dev(info->dev),
+ &domain_context_clear_one_cb, info);
}
static void __dmar_remove_one_dev_info(struct device_domain_info *info)
@@ -4466,7 +4479,7 @@ static void __dmar_remove_one_dev_info(struct
device_domain_info *info)
iommu_disable_dev_iotlb(info);
if (!dev_is_real_dma_subdevice(info->dev))
- domain_context_clear(iommu, info->dev);
+ domain_context_clear(info);
intel_pasid_free_table(info->dev);
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
