Hi folks,
The iommu group is the minimal isolation boundary for DMA. Devices in
a group can access each other's MMIO registers via peer to peer DMA
and also need share the same I/O address space.
Once the I/O address space is assigned to user control it is no longer
available to the dma_map* API, which effectively makes the DMA API
non-working.
Second, userspace can use DMA initiated by a device that it controls
to access the MMIO spaces of other devices in the group. This allows
userspace to indirectly attack any kernel owned device and it's driver.
Therefore groups must either be entirely under kernel control or
userspace control, never a mixture. Unfortunately some systems have
problems with the granularity of groups and there are a couple of
important exceptions:
- pci_stub allows the admin to block driver binding on a device and
make it permanently shared with userspace. Since PCI stub does not
do DMA it is safe, however the admin must understand that using
pci_stub allows userspace to attack whatever device it was bound
it.
- PCI bridges are sometimes included in groups. Typically PCI bridges
do not use DMA, and generally do not have MMIO regions.
Generally any device that does not have any MMIO registers is a
possible candidate for an exception.
Currently vfio adopts a workaround to detect violations of the above
restrictions by monitoring the driver core BOUND event, and hardwiring
the above exceptions. Since there is no way for vfio to reject driver
binding at this point, BUG_ON() is triggered if a violation is
captured (kernel driver BOUND event on a group which already has some
devices assigned to userspace). Aside from the bad user experience
this opens a way for root userspace to crash the kernel, even in high
integrity configurations, by manipulating the module binding and
triggering the BUG_ON.
This series solves this problem by making the user/kernel ownership a
core concept at the IOMMU layer. The driver core enforces kernel
ownership while drivers are bound and violations now result in a error
codes during probe, not BUG_ON failures.
Patch partitions:
[PATCH 1-4]: Detect DMA ownership conflicts during driver binding;
[PATCH 5-7]: Add security context management for assigned devices;
[PATCH 8-11]: Various cleanups.
This is also part one of three initial series for IOMMUFD:
* Move IOMMU Group security into the iommu layer
- Generic IOMMUFD implementation
- VFIO ability to consume IOMMUFD
Change log:
v1: initial post
- https://lore.kernel.org/linux-iommu/[email protected]/
v2:
- https://lore.kernel.org/linux-iommu/[email protected]/
- Move kernel dma ownership auto-claiming from driver core to bus
callback. [Greg/Christoph/Robin/Jason]
https://lore.kernel.org/linux-iommu/[email protected]/T/#m153706912b770682cb12e3c28f57e171aa1f9d0c
- Code and interface refactoring for iommu_set/release_dma_owner()
interfaces. [Jason]
https://lore.kernel.org/linux-iommu/[email protected]/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e
- [NEW]Add new iommu_attach/detach_device_shared() interfaces for
multiple devices group. [Robin/Jason]
https://lore.kernel.org/linux-iommu/[email protected]/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e
- [NEW]Use iommu_attach/detach_device_shared() in drm/tegra drivers.
- Refactoring and description refinement.
v3:
- https://lore.kernel.org/linux-iommu/[email protected]/
- Rename bus_type::dma_unconfigure to bus_type::dma_cleanup. [Greg]
https://lore.kernel.org/linux-iommu/[email protected]/T/#m6711e041e47cb0cbe3964fad0a3466f5ae4b3b9b
- Avoid _platform_dma_configure for platform_bus_type::dma_configure.
[Greg]
https://lore.kernel.org/linux-iommu/[email protected]/T/#m43fc46286611aa56a5c0eeaad99d539e5519f3f6
- Patch "0012-iommu-Add-iommu_at-de-tach_device_shared-for-mult.patch"
and "0018-drm-tegra-Use-the-iommu-dma_owner-mechanism.patch" have
been tested by Dmitry Osipenko <[email protected]>.
v4:
- https://lore.kernel.org/linux-iommu/[email protected]/
- Remove unnecessary tegra->domain chech in the tegra patch. (Jason)
- Remove DMA_OWNER_NONE. (Joerg)
- Change refcount to unsigned int. (Christoph)
- Move mutex lock into group set_dma_owner functions. (Christoph)
- Add kernel doc for iommu_attach/detach_domain_shared(). (Christoph)
- Move dma auto-claim into driver core. (Jason/Christoph)
v5:
- https://lore.kernel.org/linux-iommu/[email protected]/
- Move kernel dma ownership auto-claiming from driver core to bus
callback. (Greg)
- Refactor the iommu interfaces to make them more specific.
(Jason/Robin)
- Simplify the dma ownership implementation by removing the owner
type. (Jason)
- Commit message refactoring for PCI drivers. (Bjorn)
- Move iommu_attach/detach_device() improvement patches into another
series as there are a lot of code refactoring and cleanup staffs
in various device drivers.
v6:
- https://lore.kernel.org/linux-iommu/[email protected]/
- Refine comments and commit mesages.
- Rename iommu_group_set_dma_owner() to iommu_group_claim_dma_owner().
- Rename iommu_device_use/unuse_kernel_dma() to
iommu_device_use/unuse_default_domain().
- Remove unnecessary EXPORT_SYMBOL_GPL.
- Change flag name from no_kernel_api_dma to driver_managed_dma.
- Merge 4 "Add driver dma ownership management" patches into single
one.
v7:
- We discussed about adding some fields in driver structure and
intercepting it in the bus notifier for driver unbinding. We agreed
that the driver structure should not be used out of the driver core.
- As iommu_group_claim/release_dma_owner() are only used by the VFIO,
there're no use cases for multiple calls for a single group.
- Add some commit messages in "vfio: Set DMA ownership for
VFIO" to describe the intentional enhancement of unsafe bridge
drivers.
- Comments refinement.
v8:
- Move iommu_use_default_domain() to the end of .dma_configure
callback to avoid firmware-data-ordering thing.
Link: https://lore.kernel.org/linux-iommu/[email protected]/
- Add Acked-by from PCI and VFIO maintainers.
This is based on next branch of linux-iommu tree:
https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git
and also available on github:
https://github.com/LuBaolu/intel-iommu/commits/iommu-dma-ownership-v8
Best regards,
baolu
Jason Gunthorpe (1):
vfio: Delete the unbound_list
Lu Baolu (10):
iommu: Add DMA ownership management interfaces
driver core: Add dma_cleanup callback in bus_type
amba: Stop sharing platform_dma_configure()
bus: platform,amba,fsl-mc,PCI: Add device DMA ownership management
PCI: pci_stub: Set driver_managed_dma
PCI: portdrv: Set driver_managed_dma
vfio: Set DMA ownership for VFIO devices
vfio: Remove use of vfio_group_viable()
vfio: Remove iommu group notifier
iommu: Remove iommu group changes notifier
include/linux/amba/bus.h | 8 +
include/linux/device/bus.h | 3 +
include/linux/fsl/mc.h | 8 +
include/linux/iommu.h | 54 +++---
include/linux/pci.h | 8 +
include/linux/platform_device.h | 10 +-
drivers/amba/bus.c | 37 +++-
drivers/base/dd.c | 5 +
drivers/base/platform.c | 21 ++-
drivers/bus/fsl-mc/fsl-mc-bus.c | 24 ++-
drivers/iommu/iommu.c | 228 ++++++++++++++++--------
drivers/pci/pci-driver.c | 18 ++
drivers/pci/pci-stub.c | 1 +
drivers/pci/pcie/portdrv_pci.c | 2 +
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 1 +
drivers/vfio/pci/vfio_pci.c | 1 +
drivers/vfio/platform/vfio_amba.c | 1 +
drivers/vfio/platform/vfio_platform.c | 1 +
drivers/vfio/vfio.c | 245 ++------------------------
19 files changed, 338 insertions(+), 338 deletions(-)
--
2.25.1
If a switch lacks ACS P2P Request Redirect, a device below the switch can
bypass the IOMMU and DMA directly to other devices below the switch, so
all the downstream devices must be in the same IOMMU group as the switch
itself.
The existing VFIO framework allows the portdrv driver to be bound to the
bridge while its downstream devices are assigned to user space. The
pci_dma_configure() marks the IOMMU group as containing only devices
with kernel drivers that manage DMA. Avoid this default behavior for the
portdrv driver in order for compatibility with the current VFIO usage.
We achieve this by setting ".driver_managed_dma = true" in pci_driver
structure. It is safe because the portdrv driver meets below criteria:
- This driver doesn't use DMA, as you can't find any related calls like
pci_set_master() or any kernel DMA API (dma_map_*() and etc.).
- It doesn't use MMIO as you can't find ioremap() or similar calls. It's
tolerant to userspace possibly also touching the same MMIO registers
via P2P DMA access.
Suggested-by: Jason Gunthorpe <[email protected]>
Suggested-by: Kevin Tian <[email protected]>
Signed-off-by: Lu Baolu <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/pcie/portdrv_pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 35eca6277a96..6b2adb678c21 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -202,6 +202,8 @@ static struct pci_driver pcie_portdriver = {
.err_handler = &pcie_portdrv_err_handler,
+ .driver_managed_dma = true,
+
.driver.pm = PCIE_PORTDRV_PM_OPS,
};
--
2.25.1
Multiple devices may be placed in the same IOMMU group because they
cannot be isolated from each other. These devices must either be
entirely under kernel control or userspace control, never a mixture.
This adds dma ownership management in iommu core and exposes several
interfaces for the device drivers and the device userspace assignment
framework (i.e. VFIO), so that any conflict between user and kernel
controlled dma could be detected at the beginning.
The device driver oriented interfaces are,
int iommu_device_use_default_domain(struct device *dev);
void iommu_device_unuse_default_domain(struct device *dev);
By calling iommu_device_use_default_domain(), the device driver tells
the iommu layer that the device dma is handled through the kernel DMA
APIs. The iommu layer will manage the IOVA and use the default domain
for DMA address translation.
The device user-space assignment framework oriented interfaces are,
int iommu_group_claim_dma_owner(struct iommu_group *group,
void *owner);
void iommu_group_release_dma_owner(struct iommu_group *group);
bool iommu_group_dma_owner_claimed(struct iommu_group *group);
The device userspace assignment must be disallowed if the DMA owner
claiming interface returns failure.
Signed-off-by: Jason Gunthorpe <[email protected]>
Signed-off-by: Kevin Tian <[email protected]>
Signed-off-by: Lu Baolu <[email protected]>
---
include/linux/iommu.h | 31 +++++++++
drivers/iommu/iommu.c | 153 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 181 insertions(+), 3 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9208eca4b0d1..77972ef978b5 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -675,6 +675,13 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
void iommu_sva_unbind_device(struct iommu_sva *handle);
u32 iommu_sva_get_pasid(struct iommu_sva *handle);
+int iommu_device_use_default_domain(struct device *dev);
+void iommu_device_unuse_default_domain(struct device *dev);
+
+int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner);
+void iommu_group_release_dma_owner(struct iommu_group *group);
+bool iommu_group_dma_owner_claimed(struct iommu_group *group);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -1031,6 +1038,30 @@ static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
}
+
+static inline int iommu_device_use_default_domain(struct device *dev)
+{
+ return 0;
+}
+
+static inline void iommu_device_unuse_default_domain(struct device *dev)
+{
+}
+
+static inline int
+iommu_group_claim_dma_owner(struct iommu_group *group, void *owner)
+{
+ return -ENODEV;
+}
+
+static inline void iommu_group_release_dma_owner(struct iommu_group *group)
+{
+}
+
+static inline bool iommu_group_dma_owner_claimed(struct iommu_group *group)
+{
+ return false;
+}
#endif /* CONFIG_IOMMU_API */
/**
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f2c45b85b9fc..eba8e8ccf19d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -48,6 +48,8 @@ struct iommu_group {
struct iommu_domain *default_domain;
struct iommu_domain *domain;
struct list_head entry;
+ unsigned int owner_cnt;
+ void *owner;
};
struct group_device {
@@ -294,7 +296,11 @@ int iommu_probe_device(struct device *dev)
mutex_lock(&group->mutex);
iommu_alloc_default_domain(group, dev);
- if (group->default_domain) {
+ /*
+ * If device joined an existing group which has been claimed, don't
+ * attach the default domain.
+ */
+ if (group->default_domain && !group->owner) {
ret = __iommu_attach_device(group->default_domain, dev);
if (ret) {
mutex_unlock(&group->mutex);
@@ -2109,7 +2115,7 @@ static int __iommu_attach_group(struct iommu_domain *domain,
{
int ret;
- if (group->default_domain && group->domain != group->default_domain)
+ if (group->domain && group->domain != group->default_domain)
return -EBUSY;
ret = __iommu_group_for_each_dev(group, domain,
@@ -2146,7 +2152,11 @@ static void __iommu_detach_group(struct iommu_domain *domain,
{
int ret;
- if (!group->default_domain) {
+ /*
+ * If the group has been claimed already, do not re-attach the default
+ * domain.
+ */
+ if (!group->default_domain || group->owner) {
__iommu_group_for_each_dev(group, domain,
iommu_group_do_detach_device);
group->domain = NULL;
@@ -3095,3 +3105,140 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
return ret;
}
+
+/**
+ * iommu_device_use_default_domain() - Device driver wants to handle device
+ * DMA through the kernel DMA API.
+ * @dev: The device.
+ *
+ * The device driver about to bind @dev wants to do DMA through the kernel
+ * DMA API. Return 0 if it is allowed, otherwise an error.
+ */
+int iommu_device_use_default_domain(struct device *dev)
+{
+ struct iommu_group *group = iommu_group_get(dev);
+ int ret = 0;
+
+ if (!group)
+ return 0;
+
+ mutex_lock(&group->mutex);
+ if (group->owner_cnt) {
+ if (group->domain != group->default_domain ||
+ group->owner) {
+ ret = -EBUSY;
+ goto unlock_out;
+ }
+ }
+
+ group->owner_cnt++;
+
+unlock_out:
+ mutex_unlock(&group->mutex);
+ iommu_group_put(group);
+
+ return ret;
+}
+
+/**
+ * iommu_device_unuse_default_domain() - Device driver stops handling device
+ * DMA through the kernel DMA API.
+ * @dev: The device.
+ *
+ * The device driver doesn't want to do DMA through kernel DMA API anymore.
+ * It must be called after iommu_device_use_default_domain().
+ */
+void iommu_device_unuse_default_domain(struct device *dev)
+{
+ struct iommu_group *group = iommu_group_get(dev);
+
+ if (!group)
+ return;
+
+ mutex_lock(&group->mutex);
+ if (!WARN_ON(!group->owner_cnt))
+ group->owner_cnt--;
+
+ mutex_unlock(&group->mutex);
+ iommu_group_put(group);
+}
+
+/**
+ * iommu_group_claim_dma_owner() - Set DMA ownership of a group
+ * @group: The group.
+ * @owner: Caller specified pointer. Used for exclusive ownership.
+ *
+ * This is to support backward compatibility for vfio which manages
+ * the dma ownership in iommu_group level. New invocations on this
+ * interface should be prohibited.
+ */
+int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner)
+{
+ int ret = 0;
+
+ mutex_lock(&group->mutex);
+ if (group->owner_cnt) {
+ ret = -EPERM;
+ goto unlock_out;
+ } else {
+ if (group->domain && group->domain != group->default_domain) {
+ ret = -EBUSY;
+ goto unlock_out;
+ }
+
+ group->owner = owner;
+ if (group->domain)
+ __iommu_detach_group(group->domain, group);
+ }
+
+ group->owner_cnt++;
+unlock_out:
+ mutex_unlock(&group->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_group_claim_dma_owner);
+
+/**
+ * iommu_group_release_dma_owner() - Release DMA ownership of a group
+ * @group: The group.
+ *
+ * Release the DMA ownership claimed by iommu_group_claim_dma_owner().
+ */
+void iommu_group_release_dma_owner(struct iommu_group *group)
+{
+ mutex_lock(&group->mutex);
+ if (WARN_ON(!group->owner_cnt || !group->owner))
+ goto unlock_out;
+
+ group->owner_cnt = 0;
+ /*
+ * The UNMANAGED domain should be detached before all USER
+ * owners have been released.
+ */
+ if (!WARN_ON(group->domain) && group->default_domain)
+ __iommu_attach_group(group->default_domain, group);
+ group->owner = NULL;
+unlock_out:
+ mutex_unlock(&group->mutex);
+}
+EXPORT_SYMBOL_GPL(iommu_group_release_dma_owner);
+
+/**
+ * iommu_group_dma_owner_claimed() - Query group dma ownership status
+ * @group: The group.
+ *
+ * This provides status query on a given group. It is racy and only for
+ * non-binding status reporting.
+ */
+bool iommu_group_dma_owner_claimed(struct iommu_group *group)
+{
+ unsigned int user;
+
+ mutex_lock(&group->mutex);
+ user = group->owner_cnt;
+ mutex_unlock(&group->mutex);
+
+ return user;
+}
+EXPORT_SYMBOL_GPL(iommu_group_dma_owner_claimed);
--
2.25.1
Hi Lu,
On 3/8/22 6:44 AM, Lu Baolu wrote:
> Hi folks,
>
> The iommu group is the minimal isolation boundary for DMA. Devices in
> a group can access each other's MMIO registers via peer to peer DMA
> and also need share the same I/O address space.
>
> Once the I/O address space is assigned to user control it is no longer
> available to the dma_map* API, which effectively makes the DMA API
> non-working.
>
> Second, userspace can use DMA initiated by a device that it controls
> to access the MMIO spaces of other devices in the group. This allows
> userspace to indirectly attack any kernel owned device and it's driver.
>
> Therefore groups must either be entirely under kernel control or
> userspace control, never a mixture. Unfortunately some systems have
> problems with the granularity of groups and there are a couple of
> important exceptions:
>
> - pci_stub allows the admin to block driver binding on a device and
> make it permanently shared with userspace. Since PCI stub does not
> do DMA it is safe, however the admin must understand that using
> pci_stub allows userspace to attack whatever device it was bound
> it.
>
> - PCI bridges are sometimes included in groups. Typically PCI bridges
> do not use DMA, and generally do not have MMIO regions.
>
> Generally any device that does not have any MMIO registers is a
> possible candidate for an exception.
>
> Currently vfio adopts a workaround to detect violations of the above
> restrictions by monitoring the driver core BOUND event, and hardwiring
> the above exceptions. Since there is no way for vfio to reject driver
> binding at this point, BUG_ON() is triggered if a violation is
> captured (kernel driver BOUND event on a group which already has some
> devices assigned to userspace). Aside from the bad user experience
> this opens a way for root userspace to crash the kernel, even in high
> integrity configurations, by manipulating the module binding and
> triggering the BUG_ON.
>
> This series solves this problem by making the user/kernel ownership a
> core concept at the IOMMU layer. The driver core enforces kernel
> ownership while drivers are bound and violations now result in a error
> codes during probe, not BUG_ON failures.
>
> Patch partitions:
> [PATCH 1-4]: Detect DMA ownership conflicts during driver binding;
> [PATCH 5-7]: Add security context management for assigned devices;
> [PATCH 8-11]: Various cleanups.
>
> This is also part one of three initial series for IOMMUFD:
> * Move IOMMU Group security into the iommu layer
> - Generic IOMMUFD implementation
> - VFIO ability to consume IOMMUFD
>
> Change log:
> v1: initial post
> - https://lore.kernel.org/linux-iommu/[email protected]/
>
> v2:
> - https://lore.kernel.org/linux-iommu/[email protected]/
>
> - Move kernel dma ownership auto-claiming from driver core to bus
> callback. [Greg/Christoph/Robin/Jason]
> https://lore.kernel.org/linux-iommu/[email protected]/T/#m153706912b770682cb12e3c28f57e171aa1f9d0c
>
> - Code and interface refactoring for iommu_set/release_dma_owner()
> interfaces. [Jason]
> https://lore.kernel.org/linux-iommu/[email protected]/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e
>
> - [NEW]Add new iommu_attach/detach_device_shared() interfaces for
> multiple devices group. [Robin/Jason]
> https://lore.kernel.org/linux-iommu/[email protected]/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e
>
> - [NEW]Use iommu_attach/detach_device_shared() in drm/tegra drivers.
>
> - Refactoring and description refinement.
>
> v3:
> - https://lore.kernel.org/linux-iommu/[email protected]/
>
> - Rename bus_type::dma_unconfigure to bus_type::dma_cleanup. [Greg]
> https://lore.kernel.org/linux-iommu/[email protected]/T/#m6711e041e47cb0cbe3964fad0a3466f5ae4b3b9b
>
> - Avoid _platform_dma_configure for platform_bus_type::dma_configure.
> [Greg]
> https://lore.kernel.org/linux-iommu/[email protected]/T/#m43fc46286611aa56a5c0eeaad99d539e5519f3f6
>
> - Patch "0012-iommu-Add-iommu_at-de-tach_device_shared-for-mult.patch"
> and "0018-drm-tegra-Use-the-iommu-dma_owner-mechanism.patch" have
> been tested by Dmitry Osipenko <[email protected]>.
>
> v4:
> - https://lore.kernel.org/linux-iommu/[email protected]/
> - Remove unnecessary tegra->domain chech in the tegra patch. (Jason)
> - Remove DMA_OWNER_NONE. (Joerg)
> - Change refcount to unsigned int. (Christoph)
> - Move mutex lock into group set_dma_owner functions. (Christoph)
> - Add kernel doc for iommu_attach/detach_domain_shared(). (Christoph)
> - Move dma auto-claim into driver core. (Jason/Christoph)
>
> v5:
> - https://lore.kernel.org/linux-iommu/[email protected]/
> - Move kernel dma ownership auto-claiming from driver core to bus
> callback. (Greg)
> - Refactor the iommu interfaces to make them more specific.
> (Jason/Robin)
> - Simplify the dma ownership implementation by removing the owner
> type. (Jason)
> - Commit message refactoring for PCI drivers. (Bjorn)
> - Move iommu_attach/detach_device() improvement patches into another
> series as there are a lot of code refactoring and cleanup staffs
> in various device drivers.
>
> v6:
> - https://lore.kernel.org/linux-iommu/[email protected]/
> - Refine comments and commit mesages.
> - Rename iommu_group_set_dma_owner() to iommu_group_claim_dma_owner().
> - Rename iommu_device_use/unuse_kernel_dma() to
> iommu_device_use/unuse_default_domain().
> - Remove unnecessary EXPORT_SYMBOL_GPL.
> - Change flag name from no_kernel_api_dma to driver_managed_dma.
> - Merge 4 "Add driver dma ownership management" patches into single
> one.
>
> v7:
> - We discussed about adding some fields in driver structure and
> intercepting it in the bus notifier for driver unbinding. We agreed
> that the driver structure should not be used out of the driver core.
> - As iommu_group_claim/release_dma_owner() are only used by the VFIO,
> there're no use cases for multiple calls for a single group.
> - Add some commit messages in "vfio: Set DMA ownership for
> VFIO" to describe the intentional enhancement of unsafe bridge
> drivers.
> - Comments refinement.
>
> v8:
> - Move iommu_use_default_domain() to the end of .dma_configure
> callback to avoid firmware-data-ordering thing.
> Link: https://lore.kernel.org/linux-iommu/[email protected]/
Feel free to add my T-b
Tested-by: Eric Auger <[email protected]>
Thanks
Eric
> - Add Acked-by from PCI and VFIO maintainers.
>
> This is based on next branch of linux-iommu tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git
> and also available on github:
> https://github.com/LuBaolu/intel-iommu/commits/iommu-dma-ownership-v8
>
> Best regards,
> baolu
>
> Jason Gunthorpe (1):
> vfio: Delete the unbound_list
>
> Lu Baolu (10):
> iommu: Add DMA ownership management interfaces
> driver core: Add dma_cleanup callback in bus_type
> amba: Stop sharing platform_dma_configure()
> bus: platform,amba,fsl-mc,PCI: Add device DMA ownership management
> PCI: pci_stub: Set driver_managed_dma
> PCI: portdrv: Set driver_managed_dma
> vfio: Set DMA ownership for VFIO devices
> vfio: Remove use of vfio_group_viable()
> vfio: Remove iommu group notifier
> iommu: Remove iommu group changes notifier
>
> include/linux/amba/bus.h | 8 +
> include/linux/device/bus.h | 3 +
> include/linux/fsl/mc.h | 8 +
> include/linux/iommu.h | 54 +++---
> include/linux/pci.h | 8 +
> include/linux/platform_device.h | 10 +-
> drivers/amba/bus.c | 37 +++-
> drivers/base/dd.c | 5 +
> drivers/base/platform.c | 21 ++-
> drivers/bus/fsl-mc/fsl-mc-bus.c | 24 ++-
> drivers/iommu/iommu.c | 228 ++++++++++++++++--------
> drivers/pci/pci-driver.c | 18 ++
> drivers/pci/pci-stub.c | 1 +
> drivers/pci/pcie/portdrv_pci.c | 2 +
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 1 +
> drivers/vfio/pci/vfio_pci.c | 1 +
> drivers/vfio/platform/vfio_amba.c | 1 +
> drivers/vfio/platform/vfio_platform.c | 1 +
> drivers/vfio/vfio.c | 245 ++------------------------
> 19 files changed, 338 insertions(+), 338 deletions(-)
>
On Tue, Mar 08, 2022 at 01:44:10PM +0800, Lu Baolu wrote:
> Hi folks,
>
> The iommu group is the minimal isolation boundary for DMA. Devices in
> a group can access each other's MMIO registers via peer to peer DMA
> and also need share the same I/O address space.
Joerg, are we good for the coming v5.18 merge window now? There are
several things backed up behind this series.
Thanks,
Jason
On Mon, Mar 14, 2022 at 09:21:25PM -0300, Jason Gunthorpe wrote:
> Joerg, are we good for the coming v5.18 merge window now? There are
> several things backed up behind this series.
I usually don't apply bigger changes like this after -rc7, so it didn't
make it. Please re-send after -rc3 is out and I will consider it.
Thanks,
Joerg
On Fri, 8 Apr 2022 10:59:22 -0500
Bjorn Helgaas <[email protected]> wrote:
> On Fri, Apr 08, 2022 at 05:37:16PM +0200, Joerg Roedel wrote:
> > On Fri, Apr 08, 2022 at 11:17:47AM -0300, Jason Gunthorpe wrote:
> > > You might consider using a linear tree instead of the topic branches,
> > > topics are tricky and I'm not sure it helps a small subsystem so much.
> > > Conflicts between topics are a PITA for everyone, and it makes
> > > handling conflicts with rc much harder than it needs to be.
> >
> > I like the concept of a branch per driver, because with that I can just
> > exclude that branch from my next-merge when there are issues with it.
> > Conflicts between branches happen too, but they are quite manageable
> > when the branches have the same base.
>
> FWIW, I use the same topic branch approach for PCI. I like the
> ability to squash in fixes or drop things without having to clutter
> the history with trivial commits and reverts. I haven't found
> conflicts to be a problem.
Same. I think I've generally modeled my branch handling after Bjorn
and Joerg, I don't always use topic branches, but will for larger
contributions and I don't generally find conflicts to be a problem.
I'm always open to adopting best practices though. Thanks,
Alex
On Fri, Apr 08, 2022 at 08:22:35PM +0800, Lu Baolu wrote:
> Hi Joerg,
>
> On 2022/4/8 15:57, Joerg Roedel wrote:
> > On Mon, Mar 14, 2022 at 09:21:25PM -0300, Jason Gunthorpe wrote:
> > > Joerg, are we good for the coming v5.18 merge window now? There are
> > > several things backed up behind this series.
> >
> > I usually don't apply bigger changes like this after -rc7, so it didn't
> > make it. Please re-send after -rc3 is out and I will consider it.
>
> Sure. I will do.
Why rc3? It has been 4 weeks now with no futher comments.
Jason
On Fri, Apr 08, 2022 at 04:00:31PM +0200, Joerg Roedel wrote:
> On Fri, Apr 08, 2022 at 09:23:52AM -0300, Jason Gunthorpe wrote:
> > Why rc3? It has been 4 weeks now with no futher comments.
>
> Because I start applying new code to branches based on -rc3. In the past
> I used different -rc's for the topic branches (usually the latest -rc
> available when I started applying to that branch), but that caused silly
> merge conflicts from time to time. So I am now basing every topic branch
> on the same -rc, which is usually -rc3. Rationale is that by -rc3 time
> the kernel should have reasonably stabilized after the merge window.
You might consider using a linear tree instead of the topic branches,
topics are tricky and I'm not sure it helps a small subsystem so much.
Conflicts between topics are a PITA for everyone, and it makes
handling conflicts with rc much harder than it needs to be.
At least I haven't felt a need for topics while running larger trees,
and would find it stressful to try and squeeze the entire patch flow
into only 3 weeks out of the 7 week cycle.
In any event, I'd like this on a branch so Alex can pull it too, I
guess it means Alex has to merge rc3 to VFIO as well?
Thanks for explaining
Jason
On Fri, Apr 08, 2022 at 05:37:16PM +0200, Joerg Roedel wrote:
> On Fri, Apr 08, 2022 at 11:17:47AM -0300, Jason Gunthorpe wrote:
> > You might consider using a linear tree instead of the topic branches,
> > topics are tricky and I'm not sure it helps a small subsystem so much.
> > Conflicts between topics are a PITA for everyone, and it makes
> > handling conflicts with rc much harder than it needs to be.
>
> I like the concept of a branch per driver, because with that I can just
> exclude that branch from my next-merge when there are issues with it.
> Conflicts between branches happen too, but they are quite manageable
> when the branches have the same base.
FWIW, I use the same topic branch approach for PCI. I like the
ability to squash in fixes or drop things without having to clutter
the history with trivial commits and reverts. I haven't found
conflicts to be a problem.
Bjorn
Hi Joerg,
On 2022/4/8 15:57, Joerg Roedel wrote:
> On Mon, Mar 14, 2022 at 09:21:25PM -0300, Jason Gunthorpe wrote:
>> Joerg, are we good for the coming v5.18 merge window now? There are
>> several things backed up behind this series.
>
> I usually don't apply bigger changes like this after -rc7, so it didn't
> make it. Please re-send after -rc3 is out and I will consider it.
Sure. I will do.
Best regards,
baolu
On Fri, Apr 08, 2022 at 09:23:52AM -0300, Jason Gunthorpe wrote:
> Why rc3? It has been 4 weeks now with no futher comments.
Because I start applying new code to branches based on -rc3. In the past
I used different -rc's for the topic branches (usually the latest -rc
available when I started applying to that branch), but that caused silly
merge conflicts from time to time. So I am now basing every topic branch
on the same -rc, which is usually -rc3. Rationale is that by -rc3 time
the kernel should have reasonably stabilized after the merge window.
Regards,
Joerg
On Fri, Apr 08, 2022 at 11:17:47AM -0300, Jason Gunthorpe wrote:
> You might consider using a linear tree instead of the topic branches,
> topics are tricky and I'm not sure it helps a small subsystem so much.
> Conflicts between topics are a PITA for everyone, and it makes
> handling conflicts with rc much harder than it needs to be.
I like the concept of a branch per driver, because with that I can just
exclude that branch from my next-merge when there are issues with it.
Conflicts between branches happen too, but they are quite manageable
when the branches have the same base.
Overall I am thinking of reorganizing the IOMMU tree, but it will likely
not end up to be a single-branch tree, although the number of patches
per cycle _could_ just be carried in a single branch.
> At least I haven't felt a need for topics while running larger trees,
> and would find it stressful to try and squeeze the entire patch flow
> into only 3 weeks out of the 7 week cycle.
Yeah, so it is 4 weeks in an 9 weeks cycle :) The merge window is 2
weeks and not a lot happens. The 2 weeks after are for stabilization and
I usually only pick up fixes. Then come the 4 weeks were new code gets
into the tree. In the last week everything gets testing in linux-next to
be ready for the merge window. I will pickup fixes in that week, of
course.
> In any event, I'd like this on a branch so Alex can pull it too, I
> guess it means Alex has to merge rc3 to VFIO as well?
Sure, I can put these patches in a separate branch for Alex to pull into
the VFIO tree.
Regards,
Joerg
On Fri, Apr 08, 2022 at 10:07:50AM -0600, Alex Williamson wrote:
> On Fri, 8 Apr 2022 10:59:22 -0500
> Bjorn Helgaas <[email protected]> wrote:
>
> > On Fri, Apr 08, 2022 at 05:37:16PM +0200, Joerg Roedel wrote:
> > > On Fri, Apr 08, 2022 at 11:17:47AM -0300, Jason Gunthorpe wrote:
> > > > You might consider using a linear tree instead of the topic branches,
> > > > topics are tricky and I'm not sure it helps a small subsystem so much.
> > > > Conflicts between topics are a PITA for everyone, and it makes
> > > > handling conflicts with rc much harder than it needs to be.
> > >
> > > I like the concept of a branch per driver, because with that I can just
> > > exclude that branch from my next-merge when there are issues with it.
> > > Conflicts between branches happen too, but they are quite manageable
> > > when the branches have the same base.
> >
> > FWIW, I use the same topic branch approach for PCI. I like the
> > ability to squash in fixes or drop things without having to clutter
> > the history with trivial commits and reverts. I haven't found
> > conflicts to be a problem.
>
> Same. I think I've generally modeled my branch handling after Bjorn
> and Joerg, I don't always use topic branches, but will for larger
> contributions and I don't generally find conflicts to be a problem.
> I'm always open to adopting best practices though. Thanks,
I don't know about best practices, but I see most maintainers fall
somewhere on a continuum between how Andrew Morton works and how David
Miller/Linus work.
Andrew's model is to try and send patches that are perfect and he
manipulates his queue continually. It is never quite clear what will
get merged until Linus actually merges it.
The David/Linus model is that git is immutable and they only move
forward. Never rebase, never manipulate an applied patch.
Andrew has significantly reigned in how much he manipulates his queue
- mostly due to pressure from Linus. Some of the remarks on this topic
over the last year are pretty informative. So I would say changing
patches once applied, or any rebasing, is now being seen as not best
practice. (Indeed if Linus catches it and a mistake was made you are
likely to get a sharp email)
Why I made the note, is that at least in my flow, I would not trade
two weeks of accepting patches for topics. I'll probably have 20-30
patches merged already before rc3 comes out. I never have problems
merging rc's because when a rc conflicts with the next I have only one
branch and can just merge the rc and resolve the conflict, or merge
the rc before applying a patch that would create a conflict in the
first place. Linus has given some guidance on when/how he prefers to
see those merges done.
Though I tend to advocate for a philosophy more like DaveM that the
maintainer role is to hurry up and accept good patches - to emphasize
flow as a way to build involvement and community.
That is not necessarily an entirely appropriate approach in some of
the more critical subsystems like mm/pci - if they are broken in a way
that impacts a large number of people at rc1 then it can cause a lot
of impact. For instance our QA team is always paniced if rc1 doesn't
work on our test environments.
Cheers,
Jason