> -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@xxxxxxx] > Sent: 06 November 2020 13:06 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; > iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; devel@xxxxxxxxxx > Cc: Linuxarm <linuxarm@xxxxxxxxxx>; lorenzo.pieralisi@xxxxxxx; > joro@xxxxxxxxxx; wanghuiqiang <wanghuiqiang@xxxxxxxxxx>; Guohanjun > (Hanjun Guo) <guohanjun@xxxxxxxxxx>; Jonathan Cameron > <jonathan.cameron@xxxxxxxxxx> > Subject: Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation > helper > > On 2020-11-06 08:30, Shameerali Kolothum Thodi wrote: > > > > > >> -----Original Message----- > >> From: Robin Murphy [mailto:robin.murphy@xxxxxxx] > >> Sent: 05 November 2020 18:04 > >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; > >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; > >> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; devel@xxxxxxxxxx > >> Cc: Linuxarm <linuxarm@xxxxxxxxxx>; lorenzo.pieralisi@xxxxxxx; > >> joro@xxxxxxxxxx; wanghuiqiang <wanghuiqiang@xxxxxxxxxx>; Guohanjun > >> (Hanjun Guo) <guohanjun@xxxxxxxxxx>; Jonathan Cameron > >> <jonathan.cameron@xxxxxxxxxx> > >> Subject: Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions > reservation > >> helper > >> > >> On 2020-10-27 11:26, Shameer Kolothum wrote: > >>> Add a helper function that retrieves RMR memory descriptors > >>> associated with a given endpoint dev. These memory regions > >>> should have a unity mapping in the SMMU. So reserve them as > >>> IOMMU_RESV_DIRECT. > >> > >> As a general observation, we also need a way into this that isn't from > >> the perspective of endpoint devices. With SMMUv3 we need to know all the > >> active stream IDs relevant to a given SMMU instance at probe time, so > >> that we can set up some kind of valid stream table entries *before* > >> enabling the SMMU in the reset routine. > > > > So I guess, the idea is to provide an interface here to retrieve those active > > stream Ids? The problem is, at present(AFAICS), RMR doesn’t have any > > means to specify such devices. > > I'm thinking we need to check if any RMRs exist in the IORT, if so find > the devices that map to them, then resolve those devices' ID mappings. > In terms of the interface, maybe the better option is to do something > closer to what intel-iommu does and handle *all* the processing up-front > - let the IOMMU driver just call into the firmware code once to retrieve > a complete list of all the relevant abstracted RMR (or DT equivalent) > information, so it can then resolve both its own early initialisation > and the later per-device setup without calling back into firmware code > again. Ok. I will try to change the interface then. > > Otherwise we're just going to > >> kill ongoing traffic (e.g. EFI GOP) with C_BAD_STE long before we ever > >> start adding devices and worrying about reserved regions for them. > >> Similarly for the initial SMR/S2CR state on SMMUv2 with disable_bypass. > > > > Ok. I see the discussion here, > > > https://lore.kernel.org/linux-iommu/484b9e90-7395-6161-577c-4d3f3716997e > @arm.com/ > > > > From what I gather, the plan is to setup a default IDENTITY_DOMAIN for > > devices that have live stream going on during boot/SMMU probe time. > > Well, whether we use bypass or a temporary translation context for the > period before we even get to default domains... that's a discussion that > can wait until we have the ability to set up the STEs at all ;) Sure. Thanks, Shameer > Robin. > > >>> Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@xxxxxxxxxx> > >>> --- > >>> drivers/acpi/arm64/iort.c | 56 > >> +++++++++++++++++++++++++++++++++++++++ > >>> include/linux/acpi_iort.h | 4 +++ > >>> 2 files changed, 60 insertions(+) > >>> > >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > >>> index b32cd53cca08..c0700149e60b 100644 > >>> --- a/drivers/acpi/arm64/iort.c > >>> +++ b/drivers/acpi/arm64/iort.c > >>> @@ -842,6 +842,60 @@ static inline int iort_add_device_replay(struct > >> device *dev) > >>> return err; > >>> } > >>> > >>> +static bool iort_dev_has_rmr(struct device *dev, struct iort_rmr_entry > *e) > >>> +{ > >>> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > >>> + struct acpi_iort_node *iommu; > >>> + struct iort_rmr_id *rmr_ids = e->rmr_ids; > >>> + int i, j; > >>> + > >>> + iommu = iort_get_iort_node(fwspec->iommu_fwnode); > >>> + > >>> + for (i = 0; i < e->rmr_ids_num; i++, rmr_ids++) { > >>> + for (j = 0; j < fwspec->num_ids; j++) { > >>> + if (rmr_ids->sid == fwspec->ids[j] && > >>> + rmr_ids->smmu == iommu) > >>> + return true; > >>> + } > >>> + } > >>> + > >>> + return false; > >>> +} > >>> + > >>> +/** > >>> + * iort_dev_rmr_get_resv_regions - RMR Reserved region driver helper > >>> + * @dev: Device from iommu_get_resv_regions() > >>> + * @head: Reserved region list from iommu_get_resv_regions() > >>> + * > >>> + * Returns: 0 on success, <0 failure > >>> + */ > >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head > >> *head) > >>> +{ > >>> + struct iort_rmr_entry *e; > >>> + > >>> + list_for_each_entry(e, &iort_rmr_list, list) { > >>> + struct iommu_resv_region *region; > >>> + struct acpi_iort_rmr_desc *rmr; > >>> + int prot = IOMMU_READ | IOMMU_WRITE | > >>> + IOMMU_NOEXEC | IOMMU_MMIO; > >>> + > >>> + if (!iort_dev_has_rmr(dev, e)) > >>> + continue; > >>> + > >>> + rmr = e->rmr_desc; > >>> + region = iommu_alloc_resv_region(rmr->base_address, > >>> + rmr->length, prot, > >>> + IOMMU_RESV_DIRECT); > >>> + if (!region) { > >>> + dev_err(dev, "Out of memory allocating RMR regions\n"); > >>> + return -ENOMEM; > >>> + } > >>> + list_add_tail(®ion->list, head); > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> /** > >>> * iort_iommu_msi_get_resv_regions - Reserved region driver helper > >>> * @dev: Device from iommu_get_resv_regions() > >>> @@ -1112,6 +1166,8 @@ int iort_iommu_msi_get_resv_regions(struct > >> device *dev, struct list_head *head) > >>> const struct iommu_ops *iort_iommu_configure_id(struct device *dev, > >>> const u32 *input_id) > >>> { return NULL; } > >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head > >> *head) > >>> +{ return 0; } > >>> #endif > >>> > >>> static int nc_dma_get_range(struct device *dev, u64 *size) > >>> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h > >>> index 20a32120bb88..6dd89faf340c 100644 > >>> --- a/include/linux/acpi_iort.h > >>> +++ b/include/linux/acpi_iort.h > >>> @@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64 > >> *dma_addr, u64 *size); > >>> const struct iommu_ops *iort_iommu_configure_id(struct device *dev, > >>> const u32 *id_in); > >>> int iort_iommu_msi_get_resv_regions(struct device *dev, struct > list_head > >> *head); > >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head > >> *head); > >>> #else > >>> static inline void acpi_iort_init(void) { } > >>> static inline u32 iort_msi_map_id(struct device *dev, u32 id) > >>> @@ -55,6 +56,9 @@ static inline const struct iommu_ops > >> *iort_iommu_configure_id( > >>> static inline > >>> int iort_iommu_msi_get_resv_regions(struct device *dev, struct > list_head > >> *head) > >>> { return 0; } > >>> +static inline > >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head > >> *head) > >>> +{ return 0; } > >>> #endif > >>> > >>> #endif /* __ACPI_IORT_H__ */ > >>>
