Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
On Fri, Apr 08, 2022 at 10:07:50AM -0600, Alex Williamson wrote: > On Fri, 8 Apr 2022 10:59:22 -0500 > Bjorn Helgaas 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
On Fri, 8 Apr 2022 10:59:22 -0500 Bjorn Helgaas 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()
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/2025020552.2378167-1-baolu...@linux.intel.com/ > > v2: > - > https://lore.kernel.org/linux-iommu/20211128025051.355578-1-baolu...@linux.intel.com/ > > - Move kernel dma ownership auto-claiming from driver core to bus > callback. [Greg/Christoph/Robin/Jason] > > https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/T/#m153706912b770682cb12e3c28f57e171aa1f9d0c > > - Code and interface refactoring for iommu_set/release_dma_owner() > interfaces. [Jason] > > https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e > > - [NEW]Add new iommu_attach/detach_device_shared() interfaces for > multiple devices group. [Robin/Jason] > > https://lore.kernel.org/linux-iommu/2025020552.2378167-1-baolu...@linux.intel.com/T/#mea70ed8e4e3665aedf32a5a0a7db095bf680325e > > - [NEW]Use iommu_attach/detach_device_shared() in drm/tegra drivers. > > - Refactoring and description refinement. > > v3: > - > https://lore.kernel.org/linux-iommu/20211206015903.88687-1-baolu...@linux.intel.com/ > > - Rename bus_type::dma_unconfigure to bus_type::dma_cleanup. [Greg] > > https://lore.kernel.org/linux-iommu/c3230ace-c878-39db-1663-2b752ff53...@linux.intel.com/T/#m6711e041e47cb0cbe3964fad0a3466f5ae4b3b9b > > - Avoid _platform_dma_configure for platform_bus_type::dma_configure. > [Greg] > > https://lore.kernel.org/linux-iommu/c3230ace-c878-39db-1663-2b752ff53...@linux.intel.com/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 . > > v4: > - > https://lore.kernel.org/linux-iommu/20211217063708.1740334-1-baolu...@linux.intel.com/ > - 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: > - >
