RE: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR nodes
Hi Eric,
> -Original Message-
> From: Eric Auger [mailto:eric.au...@redhat.com]
> Sent: 10 March 2022 10:32
> To: Shameerali Kolothum Thodi ;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: Linuxarm ; lorenzo.pieral...@arm.com;
> j...@8bytes.org; robin.mur...@arm.com; w...@kernel.org; wanghuiqiang
> ; Guohanjun (Hanjun Guo)
> ; steven.pr...@arm.com; sami.muja...@arm.com;
> j...@solid-run.com; yangyicong
> Subject: Re: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR
> nodes
>
> Hi Shameer,
>
> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> > The helper functions here parse through the IORT RMR nodes and
> > populate a reserved region list corresponding to a given iommu
> > and device(optional). These also go through the ID mappings of
> > the RMR node and retrieves all the SIDs associated with a RMR
> > descriptor.
> >
> > Signed-off-by: Shameer Kolothum
>
> > ---
> > drivers/acpi/arm64/iort.c | 225
> ++
> > 1 file changed, 225 insertions(+)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 0730c4dbb700..05da9ebff50a 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -830,6 +830,231 @@ static struct acpi_iort_node
> *iort_get_msi_resv_iommu(struct device *dev)
> > return NULL;
> > }
> >
[...]
> > +static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device
> > *dev,
> > + struct list_head *head)
> > +{
> > + struct acpi_table_iort *iort;
> > + struct acpi_iort_node *iort_node, *iort_end;
> > + int i;
> > +
> > + if (iort_table->revision < 5)
> This means E.b and E.c revs are not supported. Is it what we want?
Yes. E.b lacks memory attributes info associated with RMR node. Though E.c
added those, it broke backward compatibility with ACPICA E.b support and
is now deprecated.
Thanks,
Shameer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR nodes
Hi Shameer,
On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> The helper functions here parse through the IORT RMR nodes and
> populate a reserved region list corresponding to a given iommu
> and device(optional). These also go through the ID mappings of
> the RMR node and retrieves all the SIDs associated with a RMR
> descriptor.
>
> Signed-off-by: Shameer Kolothum
> ---
> drivers/acpi/arm64/iort.c | 225 ++
> 1 file changed, 225 insertions(+)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 0730c4dbb700..05da9ebff50a 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -830,6 +830,231 @@ static struct acpi_iort_node
> *iort_get_msi_resv_iommu(struct device *dev)
> return NULL;
> }
>
> +static void iort_rmr_desc_check_overlap(struct acpi_iort_rmr_desc *desc, u32
> count)
> +{
> + int i, j;
> +
> + for (i = 0; i < count; i++) {
> + u64 end, start = desc[i].base_address, length = desc[i].length;
> +
> + end = start + length - 1;
> +
> + /* Check for address overlap */
> + for (j = i + 1; j < count; j++) {
> + u64 e_start = desc[j].base_address;
> + u64 e_end = e_start + desc[j].length - 1;
> +
> + if (start <= e_end && end >= e_start)
> + pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx]
> overlaps, continue anyway\n",
> +start, end);
> + }
> + }
> +}
> +
> +/*
> + * Please note, we will keep the already allocated RMR reserve
> + * regions in case of a memory allocation failure.
> + */
> +static void iort_rmr_get_resv_regions(struct acpi_iort_node *node,
> + struct acpi_iort_node *smmu,
> + u32 *sids, u32 num_sids,
> + struct list_head *head)
> +{
> + struct acpi_iort_rmr *rmr = (struct acpi_iort_rmr *)node->node_data;
> + struct acpi_iort_rmr_desc *rmr_desc;
> + int i;
> +
> + rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, node,
> + rmr->rmr_offset);
> +
> + iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
> +
> + for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
> + struct iommu_resv_region *region;
> + enum iommu_resv_type type;
> + u32 *sids_copy;
> + int prot = IOMMU_READ | IOMMU_WRITE;
> + u64 addr = rmr_desc->base_address, size = rmr_desc->length;
> +
> + if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
> + /* PAGE align base addr and size */
> + addr &= PAGE_MASK;
> + size = PAGE_ALIGN(size +
> offset_in_page(rmr_desc->base_address));
> +
> + pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not
> aligned to 64K, continue with [0x%llx - 0x%llx]\n",
> +rmr_desc->base_address,
> +rmr_desc->base_address + rmr_desc->length - 1,
> +addr, addr + size - 1);
> + }
> +
> + if (rmr->flags & ACPI_IORT_RMR_REMAP_PERMITTED)
> + type = IOMMU_RESV_DIRECT_RELAXABLE;
> + else
> + type = IOMMU_RESV_DIRECT;
> +
> + if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
> + prot |= IOMMU_PRIV;
> +
> + /* Attributes 0x00 - 0x03 represents device memory */
> + if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
> + ACPI_IORT_RMR_ATTR_DEVICE_GRE)
> + prot |= IOMMU_MMIO;
> + else if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) ==
> + ACPI_IORT_RMR_ATTR_NORMAL)
> + prot |= IOMMU_CACHE;
> +
> + /* Create a copy of sids array to associate with this resv
> region */
> + sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL);
> + if (!sids_copy)
> + return;
> +
> + region = iommu_alloc_resv_region(addr, size, prot, type);
> + if (!region) {
> + kfree(sids_copy);
> + return;
> + }
> +
> + region->fw_data.rmr.sids = sids_copy;
> + region->fw_data.rmr.num_sids = num_sids;
> + list_add_tail(®ion->list, head);
> + }
> +}
> +
> +static u32 *iort_rmr_alloc_sids(u32 *sids, u32 count, u32 id_start,
> + u32 new_count)
> +{
> + u32 *new_sids;
> + u32 total_count = count + new_count;
> + int i;
> +
> + new_sids = krealloc_array(sids, count + new_count,
> + sizeof(*new_sids), GFP_KERNEL);
> + if (!new_sids)
> + return NULL;
> +
> + /*Update ne
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: > - > https://lore.kernel.org/linux-iommu/2
