| Message ID | 20220422162907.1276-5-shameerali.kolothum.thodi@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ACPI/IORT: Support for IORT RMR node | expand |
Hi Shameer,
I love your patch! Perhaps something to improve:
[auto build test WARNING on joro-iommu/next]
[also build test WARNING on rafael-pm/linux-next arm/for-next arm64/for-next/core soc/for-next linus/master v5.18-rc3 next-20220422]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Shameer-Kolothum/ACPI-IORT-Support-for-IORT-RMR-node/20220423-003822
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: arm64-randconfig-r012-20220422 (https://download.01.org/0day-ci/archive/20220423/202204231737.0jkKpxZk-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5bd87350a5ae429baf8f373cb226a57b62f87280)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/5b73fd681a27e2ad450bac28f8a81f4b35fe4d68
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Shameer-Kolothum/ACPI-IORT-Support-for-IORT-RMR-node/20220423-003822
git checkout 5b73fd681a27e2ad450bac28f8a81f4b35fe4d68
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/acpi/arm64/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/acpi/arm64/iort.c:801:29: warning: no previous prototype for function 'iort_rmr_alloc' [-Wmissing-prototypes]
struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc *rmr_desc,
^
drivers/acpi/arm64/iort.c:801:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc *rmr_desc,
^
static
drivers/acpi/arm64/iort.c:896:20: error: use of undeclared identifier 'ACPI_IORT_RMR_REMAP_PERMITTED'
if (rmr->flags & ACPI_IORT_RMR_REMAP_PERMITTED)
^
drivers/acpi/arm64/iort.c:901:20: error: use of undeclared identifier 'ACPI_IORT_RMR_ACCESS_PRIVILEGE'
if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
^
drivers/acpi/arm64/iort.c:905:7: error: call to undeclared function 'ACPI_IORT_RMR_ACCESS_ATTRIBUTES'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
^
drivers/acpi/arm64/iort.c:906:5: error: use of undeclared identifier 'ACPI_IORT_RMR_ATTR_DEVICE_GRE'
ACPI_IORT_RMR_ATTR_DEVICE_GRE)
^
drivers/acpi/arm64/iort.c:909:5: error: use of undeclared identifier 'ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB'
ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB)
^
1 warning and 5 errors generated.
vim +/iort_rmr_alloc +801 drivers/acpi/arm64/iort.c
800
> 801 struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc *rmr_desc,
802 int prot, enum iommu_resv_type type,
803 u32 *sids, u32 num_sids)
804 {
805 struct iommu_iort_rmr_data *rmr_data;
806 struct iommu_resv_region *region;
807 u32 *sids_copy;
808 u64 addr = rmr_desc->base_address, size = rmr_desc->length;
809
810 rmr_data = kmalloc(sizeof(*rmr_data), GFP_KERNEL);
811 if (!rmr_data)
812 return NULL;
813
814 /* Create a copy of SIDs array to associate with this rmr_data */
815 sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL);
816 if (!sids_copy) {
817 kfree(rmr_data);
818 return NULL;
819 }
820 rmr_data->sids = sids_copy;
821 rmr_data->num_sids = num_sids;
822
823 if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
824 /* PAGE align base addr and size */
825 addr &= PAGE_MASK;
826 size = PAGE_ALIGN(size + offset_in_page(rmr_desc->base_address));
827
828 pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to 64K, continue with [0x%llx - 0x%llx]\n",
829 rmr_desc->base_address,
830 rmr_desc->base_address + rmr_desc->length - 1,
831 addr, addr + size - 1);
832 }
833
834 region = &rmr_data->rr;
835 INIT_LIST_HEAD(®ion->list);
836 region->start = addr;
837 region->length = size;
838 region->prot = prot;
839 region->type = type;
840 region->free = iort_rmr_free;
841
842 return rmr_data;
843 }
844
> -----Original Message----- > From: kernel test robot [mailto:lkp@intel.com] > Sent: 23 April 2022 10:51 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > iommu@lists.linux-foundation.org > Cc: llvm@lists.linux.dev; kbuild-all@lists.01.org; Linuxarm > <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; joro@8bytes.org; > robin.murphy@arm.com; will@kernel.org; wanghuiqiang > <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com; > jon@solid-run.com; eric.auger@redhat.com; laurentiu.tudor@nxp.com; > hch@infradead.org > Subject: Re: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR > reserved regions > [...] > >> drivers/acpi/arm64/iort.c:801:29: warning: no previous prototype for > function 'iort_rmr_alloc' [-Wmissing-prototypes] > struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc > *rmr_desc, > ^ > drivers/acpi/arm64/iort.c:801:1: note: declare 'static' if the function is not > intended to be used outside of this translation unit > struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc > *rmr_desc, Oops..missed the 'static' here. The rest of the warnings are because of the dependency with ACPICA header patch here[1]. Hi Robin/Lorenzo, I am planning to send out an updated series soon with that 'static' added and the R-by tag received for patch #1 from Christoph. Appreciate if you can take a look and let me know if you have any further comments on this series. Thanks, Shameer [1] https://lore.kernel.org/all/44610361.fMDQidcC6G@kreacher/ > ^ > static > drivers/acpi/arm64/iort.c:896:20: error: use of undeclared identifier > 'ACPI_IORT_RMR_REMAP_PERMITTED' > if (rmr->flags & ACPI_IORT_RMR_REMAP_PERMITTED) > ^ > drivers/acpi/arm64/iort.c:901:20: error: use of undeclared identifier > 'ACPI_IORT_RMR_ACCESS_PRIVILEGE' > if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE) > ^ > drivers/acpi/arm64/iort.c:905:7: error: call to undeclared function > 'ACPI_IORT_RMR_ACCESS_ATTRIBUTES'; ISO C99 and later do not support > implicit function declarations [-Wimplicit-function-declaration] > if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <= > ^ > drivers/acpi/arm64/iort.c:906:5: error: use of undeclared identifier > 'ACPI_IORT_RMR_ATTR_DEVICE_GRE' > ACPI_IORT_RMR_ATTR_DEVICE_GRE) > ^ > drivers/acpi/arm64/iort.c:909:5: error: use of undeclared identifier > 'ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB' > > ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB) > ^ > 1 warning and 5 errors generated. > > > vim +/iort_rmr_alloc +801 drivers/acpi/arm64/iort.c > > 800 > > 801 struct iommu_iort_rmr_data *iort_rmr_alloc(struct > acpi_iort_rmr_desc *rmr_desc, > 802 int prot, enum iommu_resv_type type, > 803 u32 *sids, u32 num_sids) > 804 { > 805 struct iommu_iort_rmr_data *rmr_data; > 806 struct iommu_resv_region *region; > 807 u32 *sids_copy; > 808 u64 addr = rmr_desc->base_address, size = rmr_desc->length; > 809 > 810 rmr_data = kmalloc(sizeof(*rmr_data), GFP_KERNEL); > 811 if (!rmr_data) > 812 return NULL; > 813 > 814 /* Create a copy of SIDs array to associate with this rmr_data */ > 815 sids_copy = kmemdup(sids, num_sids * sizeof(*sids), > GFP_KERNEL); > 816 if (!sids_copy) { > 817 kfree(rmr_data); > 818 return NULL; > 819 } > 820 rmr_data->sids = sids_copy; > 821 rmr_data->num_sids = num_sids; > 822 > 823 if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) { > 824 /* PAGE align base addr and size */ > 825 addr &= PAGE_MASK; > 826 size = PAGE_ALIGN(size + > offset_in_page(rmr_desc->base_address)); > 827 > 828 pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not > aligned to 64K, continue with [0x%llx - 0x%llx]\n", > 829 rmr_desc->base_address, > 830 rmr_desc->base_address + rmr_desc->length - 1, > 831 addr, addr + size - 1); > 832 } > 833 > 834 region = &rmr_data->rr; > 835 INIT_LIST_HEAD(®ion->list); > 836 region->start = addr; > 837 region->length = size; > 838 region->prot = prot; > 839 region->type = type; > 840 region->free = iort_rmr_free; > 841 > 842 return rmr_data; > 843 } > 844 > > -- > 0-DAY CI Kernel Test Service > https://01.org/lkp
Hi Lorenzo, > -----Original Message----- > From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@arm.com] > Sent: 26 April 2022 16:30 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > iommu@lists.linux-foundation.org; Linuxarm <linuxarm@huawei.com>; > joro@8bytes.org; robin.murphy@arm.com; will@kernel.org; wanghuiqiang > <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com; > jon@solid-run.com; eric.auger@redhat.com; laurentiu.tudor@nxp.com; > hch@infradead.org > Subject: Re: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR > reserved regions > > On Fri, Apr 22, 2022 at 05:29:02PM +0100, Shameer Kolothum wrote: > > Parse through the IORT RMR nodes and populate the reserve region list > > corresponding to a given IOMMU and device(optional). Also, go through > > the ID mappings of the RMR node and retrieve all the SIDs associated > > with it. > > > > Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> > > --- > > drivers/acpi/arm64/iort.c | 290 > ++++++++++++++++++++++++++++++++++++++ > > include/linux/iommu.h | 8 ++ > > 2 files changed, 298 insertions(+) > > Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Thanks. I just realized that need a R-by from you for patch #2 as well. Also for patch #5, I dropped your R-by tag as there is a minor change w.r.t free up. If you are Ok, I can add that back. Thanks, Shameer
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index cd5d1d7823cb..5be6e8ecca38 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -788,6 +788,293 @@ void acpi_configure_pmsi_domain(struct device *dev) } #ifdef CONFIG_IOMMU_API +static void iort_rmr_free(struct device *dev, + struct iommu_resv_region *region) +{ + struct iommu_iort_rmr_data *rmr_data; + + rmr_data = container_of(region, struct iommu_iort_rmr_data, rr); + kfree(rmr_data->sids); + kfree(rmr_data); +} + +struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc *rmr_desc, + int prot, enum iommu_resv_type type, + u32 *sids, u32 num_sids) +{ + struct iommu_iort_rmr_data *rmr_data; + struct iommu_resv_region *region; + u32 *sids_copy; + u64 addr = rmr_desc->base_address, size = rmr_desc->length; + + rmr_data = kmalloc(sizeof(*rmr_data), GFP_KERNEL); + if (!rmr_data) + return NULL; + + /* Create a copy of SIDs array to associate with this rmr_data */ + sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL); + if (!sids_copy) { + kfree(rmr_data); + return NULL; + } + rmr_data->sids = sids_copy; + rmr_data->num_sids = num_sids; + + 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); + } + + region = &rmr_data->rr; + INIT_LIST_HEAD(®ion->list); + region->start = addr; + region->length = size; + region->prot = prot; + region->type = type; + region->free = iort_rmr_free; + + return rmr_data; +} + +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; + + if (!length) { + pr_err(FW_BUG "RMR descriptor[0x%llx] with zero length, continue anyway\n", + start); + continue; + } + + 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_get_rmrs(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_iort_rmr_data *rmr_data; + enum iommu_resv_type type; + int prot = IOMMU_READ | IOMMU_WRITE; + + 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_IWB_OWB) + prot |= IOMMU_CACHE; + + rmr_data = iort_rmr_alloc(rmr_desc, prot, type, + sids, num_sids); + if (!rmr_data) + return; + + list_add_tail(&rmr_data->rr.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; + + for (i = count; i < total_count; i++) + new_sids[i] = id_start++; + + return new_sids; +} + +static bool iort_rmr_has_dev(struct device *dev, u32 id_start, + u32 id_count) +{ + int i; + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + + /* + * Make sure the kernel has preserved the boot firmware PCIe + * configuration. This is required to ensure that the RMR PCIe + * StreamIDs are still valid (Refer: ARM DEN 0049E.d Section 3.1.1.5). + */ + if (dev_is_pci(dev)) { + struct pci_dev *pdev = to_pci_dev(dev); + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); + + if (!host->preserve_config) + return false; + } + + for (i = 0; i < fwspec->num_ids; i++) { + if (fwspec->ids[i] >= id_start && + fwspec->ids[i] <= id_start + id_count) + return true; + } + + return false; +} + +static void iort_node_get_rmr_info(struct acpi_iort_node *node, + struct acpi_iort_node *iommu, + struct device *dev, struct list_head *head) +{ + struct acpi_iort_node *smmu = NULL; + struct acpi_iort_rmr *rmr; + struct acpi_iort_id_mapping *map; + u32 *sids = NULL; + u32 num_sids = 0; + int i; + + if (!node->mapping_offset || !node->mapping_count) { + pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n", + node); + return; + } + + rmr = (struct acpi_iort_rmr *)node->node_data; + if (!rmr->rmr_offset || !rmr->rmr_count) + return; + + map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node, + node->mapping_offset); + + /* + * Go through the ID mappings and see if we have a match for SMMU + * and dev(if !NULL). If found, get the sids for the Node. + * Please note, id_count is equal to the number of IDs in the + * range minus one. + */ + for (i = 0; i < node->mapping_count; i++, map++) { + struct acpi_iort_node *parent; + + if (!map->id_count) + continue; + + parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, + map->output_reference); + if (parent != iommu) + continue; + + /* If dev is valid, check RMR node corresponds to the dev SID */ + if (dev && !iort_rmr_has_dev(dev, map->output_base, + map->id_count)) + continue; + + /* Retrieve SIDs associated with the Node. */ + sids = iort_rmr_alloc_sids(sids, num_sids, map->output_base, + map->id_count + 1); + if (!sids) + return; + + num_sids += map->id_count + 1; + } + + if (!sids) + return; + + iort_get_rmrs(node, smmu, sids, num_sids, head); + kfree(sids); +} + +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; + + /* Only supports ARM DEN 0049E.d onwards */ + if (iort_table->revision < 5) + return; + + iort = (struct acpi_table_iort *)iort_table; + + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, + iort->node_offset); + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort, + iort_table->length); + + for (i = 0; i < iort->node_count; i++) { + if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND, + "IORT node pointer overflows, bad table!\n")) + return; + + if (iort_node->type == ACPI_IORT_NODE_RMR) + iort_node_get_rmr_info(iort_node, iommu, dev, head); + + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node, + iort_node->length); + } +} + +/* + * Populate the RMR list associated with a given IOMMU and dev(if provided). + * If dev is NULL, the function populates all the RMRs associated with the + * given IOMMU. + */ +static void iort_iommu_rmr_get_resv_regions(struct fwnode_handle *iommu_fwnode, + struct device *dev, + struct list_head *head) +{ + struct acpi_iort_node *iommu; + + iommu = iort_get_iort_node(iommu_fwnode); + if (!iommu) + return; + + iort_find_rmrs(iommu, dev, head); +} + static struct acpi_iort_node *iort_get_msi_resv_iommu(struct device *dev) { struct acpi_iort_node *iommu; @@ -868,7 +1155,10 @@ static void iort_iommu_msi_get_resv_regions(struct device *dev, */ void iort_iommu_get_resv_regions(struct device *dev, struct list_head *head) { + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + iort_iommu_msi_get_resv_regions(dev, head); + iort_iommu_rmr_get_resv_regions(fwspec->iommu_fwnode, dev, head); } static inline bool iort_iommu_driver_enabled(u8 type) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 68bcfb3a06d7..0936d7980ce2 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -145,6 +145,14 @@ struct iommu_resv_region { void (*free)(struct device *dev, struct iommu_resv_region *region); }; +struct iommu_iort_rmr_data { + struct iommu_resv_region rr; + + /* Stream IDs associated with IORT RMR entry */ + const u32 *sids; + u32 num_sids; +}; + /** * enum iommu_dev_features - Per device IOMMU features * @IOMMU_DEV_FEAT_SVA: Shared Virtual Addresses
Parse through the IORT RMR nodes and populate the reserve region list corresponding to a given IOMMU and device(optional). Also, go through the ID mappings of the RMR node and retrieve all the SIDs associated with it. Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> --- drivers/acpi/arm64/iort.c | 290 ++++++++++++++++++++++++++++++++++++++ include/linux/iommu.h | 8 ++ 2 files changed, 298 insertions(+)