From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>,
Kevin Tian <kevin.tian@intel.com>,
Qian Cai <quic_qiancai@quicinc.com>,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org
Subject: Re: [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
Date: Tue, 3 May 2022 12:23:36 -0300 [thread overview]
Message-ID: <20220503152336.GA3939@nvidia.com> (raw)
In-Reply-To: <8b3d31ef-caf7-da92-fa95-0df378d5b091@arm.com>
On Tue, May 03, 2022 at 02:04:37PM +0100, Robin Murphy wrote:
> > I'm guessing SMMU3 needs to call it's arm_smmu_detach_dev(master) from
> > the detach_dev op and null it's cached copy of the domain, but I don't
> > know this driver.. Robin?
>
> The original intent was that .detach_dev is deprecated in favour of default
> domains, and when the latter are in use, a device is always attached
> *somewhere* once probed (i.e. group->domain is never NULL). At face value,
> the neatest fix IMO would probably be for SMMUv3's .domain_free to handle
> smmu_domain->devices being non-empty and detach them at that point. However
> that wouldn't be viable for virtio-iommu or anyone else keeping an internal
> one-way association of devices to their current domains.
Oh wow that is not obvious
Actually, I think it is much worse than this because
iommu_group_claim_dma_owner() does a __iommu_detach_group() with the
expecation that this would actually result in DMA being blocked,
immediately. The idea that __iomuu_detatch_group() is a NOP is kind of
scary.
Leaving the group attached to the kernel DMA domain will allow
userspace to DMA to all kernel memory :\
So one approach could be to block use of iommu_group_claim_dma_owner()
if no detatch_dev op is present and then go through and put them back
or do something else. This could be short-term OK if we add an op to
SMMUv3, but long term everything would have to be fixed
Or we can allocate a dummy empty/blocked domain during
iommu_group_claim_dma_owner() and attach it whenever.
The really ugly trick is that detatch cannot fail, so attach to this
blocking domain must also not fail - IMHO this is a very complicated
API to expect for the driver to implement correctly... I see there is
already a WARN_ON that attaching to the default domain cannot
fail. Maybe this warrants an actual no-fail attach op so the driver
can be more aware of this..
And some of these internal APIs could stand some adjusting if we
really never want a true "detatch" it is always some kind of
replace/swap type operation, either to the default domain or to the
blocking domain.
> We *could* stay true to the original paradigm by introducing some real usage
> of IOMMU_DOMAIN_BLOCKED, such that we could keep one or more of those around
> to actively attach to instead of having groups in this unattached limbo
> state, but that's a bigger job involving adding support to drivers as well;
> too much for a quick fix now...
I suspect for the short term we can get by with an empty mapping
domain - using DOMAIN_BLOCKED is a bit of a refinement.
Thanks,
Jason
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Qian Cai <quic_qiancai@quicinc.com>,
Lu Baolu <baolu.lu@linux.intel.com>,
Joerg Roedel <joro@8bytes.org>, Kevin Tian <kevin.tian@intel.com>,
Liu Yi L <yi.l.liu@intel.com>,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
Jean-Philippe Brucker <jean-philippe@linaro.org>
Subject: Re: [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
Date: Tue, 3 May 2022 12:23:36 -0300 [thread overview]
Message-ID: <20220503152336.GA3939@nvidia.com> (raw)
In-Reply-To: <8b3d31ef-caf7-da92-fa95-0df378d5b091@arm.com>
On Tue, May 03, 2022 at 02:04:37PM +0100, Robin Murphy wrote:
> > I'm guessing SMMU3 needs to call it's arm_smmu_detach_dev(master) from
> > the detach_dev op and null it's cached copy of the domain, but I don't
> > know this driver.. Robin?
>
> The original intent was that .detach_dev is deprecated in favour of default
> domains, and when the latter are in use, a device is always attached
> *somewhere* once probed (i.e. group->domain is never NULL). At face value,
> the neatest fix IMO would probably be for SMMUv3's .domain_free to handle
> smmu_domain->devices being non-empty and detach them at that point. However
> that wouldn't be viable for virtio-iommu or anyone else keeping an internal
> one-way association of devices to their current domains.
Oh wow that is not obvious
Actually, I think it is much worse than this because
iommu_group_claim_dma_owner() does a __iommu_detach_group() with the
expecation that this would actually result in DMA being blocked,
immediately. The idea that __iomuu_detatch_group() is a NOP is kind of
scary.
Leaving the group attached to the kernel DMA domain will allow
userspace to DMA to all kernel memory :\
So one approach could be to block use of iommu_group_claim_dma_owner()
if no detatch_dev op is present and then go through and put them back
or do something else. This could be short-term OK if we add an op to
SMMUv3, but long term everything would have to be fixed
Or we can allocate a dummy empty/blocked domain during
iommu_group_claim_dma_owner() and attach it whenever.
The really ugly trick is that detatch cannot fail, so attach to this
blocking domain must also not fail - IMHO this is a very complicated
API to expect for the driver to implement correctly... I see there is
already a WARN_ON that attaching to the default domain cannot
fail. Maybe this warrants an actual no-fail attach op so the driver
can be more aware of this..
And some of these internal APIs could stand some adjusting if we
really never want a true "detatch" it is always some kind of
replace/swap type operation, either to the default domain or to the
blocking domain.
> We *could* stay true to the original paradigm by introducing some real usage
> of IOMMU_DOMAIN_BLOCKED, such that we could keep one or more of those around
> to actively attach to instead of having groups in this unattached limbo
> state, but that's a bigger job involving adding support to drivers as well;
> too much for a quick fix now...
I suspect for the short term we can get by with an empty mapping
domain - using DOMAIN_BLOCKED is a bit of a refinement.
Thanks,
Jason
next prev parent reply other threads:[~2022-05-03 15:23 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-18 0:49 [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier() Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 01/11] iommu: Add DMA ownership management interfaces Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-06-15 9:53 ` Steven Price
2022-06-15 9:53 ` Steven Price
2022-06-15 10:57 ` Robin Murphy
2022-06-15 10:57 ` Robin Murphy
2022-06-15 15:09 ` Steven Price
2022-06-15 15:09 ` Steven Price
2022-04-18 0:49 ` [RESEND PATCH v8 02/11] driver core: Add dma_cleanup callback in bus_type Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 03/11] amba: Stop sharing platform_dma_configure() Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 04/11] bus: platform, amba, fsl-mc, PCI: Add device DMA ownership management Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 04/11] bus: platform,amba,fsl-mc,PCI: " Lu Baolu
2023-06-26 13:02 ` [RESEND PATCH v8 04/11] bus: platform, amba, fsl-mc, PCI: " Zenghui Yu
2023-06-28 14:36 ` Jason Gunthorpe
2023-06-29 2:55 ` Zenghui Yu
2022-04-18 0:49 ` [RESEND PATCH v8 05/11] PCI: pci_stub: Set driver_managed_dma Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 06/11] PCI: portdrv: " Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 07/11] vfio: Set DMA ownership for VFIO devices Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 08/11] vfio: Remove use of vfio_group_viable() Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 09/11] vfio: Delete the unbound_list Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:49 ` [RESEND PATCH v8 10/11] vfio: Remove iommu group notifier Lu Baolu
2022-04-18 0:49 ` Lu Baolu
2022-04-18 0:50 ` [RESEND PATCH v8 11/11] iommu: Remove iommu group changes notifier Lu Baolu
2022-04-18 0:50 ` Lu Baolu
2022-04-28 9:32 ` [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier() Joerg Roedel
2022-04-28 9:32 ` Joerg Roedel
2022-04-28 11:54 ` Jason Gunthorpe via iommu
2022-04-28 11:54 ` Jason Gunthorpe
2022-04-28 13:34 ` Joerg Roedel
2022-04-28 13:34 ` Joerg Roedel
2022-05-02 16:12 ` Qian Cai
2022-05-02 16:12 ` Qian Cai
2022-05-02 16:42 ` Jason Gunthorpe via iommu
2022-05-02 16:42 ` Jason Gunthorpe
2022-05-03 13:04 ` Robin Murphy
2022-05-03 13:04 ` Robin Murphy
2022-05-03 15:23 ` Jason Gunthorpe via iommu [this message]
2022-05-03 15:23 ` Jason Gunthorpe
2022-05-03 17:22 ` Robin Murphy
2022-05-03 17:22 ` Robin Murphy
2022-05-04 8:42 ` Joerg Roedel
2022-05-04 8:42 ` Joerg Roedel
2022-05-04 11:51 ` Jason Gunthorpe via iommu
2022-05-04 11:51 ` Jason Gunthorpe
2022-05-04 11:57 ` Joerg Roedel
2022-05-04 11:57 ` Joerg Roedel
2022-05-09 18:33 ` Jason Gunthorpe via iommu
2022-05-09 18:33 ` Jason Gunthorpe
2022-05-13 8:13 ` Tian, Kevin
2022-05-13 8:13 ` Tian, Kevin
2022-05-04 16:29 ` Alex Williamson
2022-05-04 16:29 ` Alex Williamson
2022-05-13 15:49 ` Joerg Roedel
2022-05-13 15:49 ` Joerg Roedel
2022-05-13 16:25 ` Alex Williamson
2022-05-13 16:25 ` Alex Williamson
2022-05-13 19:06 ` Jason Gunthorpe via iommu
2022-05-13 19:06 ` Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220503152336.GA3939@nvidia.com \
--to=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=jgg@nvidia.com \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_qiancai@quicinc.com \
--cc=robin.murphy@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.