RE: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
Hi Lorenzo, > -Original Message- > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > Sent: 26 April 2022 16:30 > To: Shameerali Kolothum Thodi > Cc: linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org; > iommu@lists.linux-foundation.org; Linuxarm ; > 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; eric.au...@redhat.com; laurentiu.tu...@nxp.com; > h...@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 > > > --- > > drivers/acpi/arm64/iort.c | 290 > ++ > > include/linux/iommu.h | 8 ++ > > 2 files changed, 298 insertions(+) > > Reviewed-by: Lorenzo Pieralisi 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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
> ---
> drivers/acpi/arm64/iort.c | 290 ++
> include/linux/iommu.h | 8 ++
> 2 files changed, 298 insertions(+)
Reviewed-by: Lorenzo Pieralisi
> 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 = _data->rr;
> + INIT_LIST_HEAD(>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)
> +
RE: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
> -Original Message-
> From: kernel test robot [mailto:l...@intel.com]
> Sent: 23 April 2022 10:51
> To: Shameerali Kolothum Thodi ;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: l...@lists.linux.dev; kbuild-...@lists.01.org; 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; eric.au...@redhat.com; laurentiu.tu...@nxp.com;
> h...@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
> > 801struct 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{
>805struct iommu_iort_rmr_data *rmr_data;
>806struct iommu_resv_region *region;
>807u32 *sids_copy;
>808u64 addr = rmr_desc->base_address, size =
> rmr_desc->length;
>809
>810rmr_data = kmalloc(sizeof(*rmr_data), GFP_KERNEL);
>811if (!rmr_data)
>812return NULL;
>813
>814/* Create a copy of SIDs array to associate with this
> rmr_data */
>815sids_copy = kmemdup(sids, num_sids * sizeof(*sids),
> GFP_KERNEL);
>816if (!sids_copy) {
>817kfree(rmr_data);
>818return NULL;
>819}
>820rmr_data->sids = sids_copy;
>821rmr_data->num_sids = num_sids;
>822
>823if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size,
> SZ_64K)) {
>824/* PAGE align base addr and size */
>825addr &= PAGE_MASK;
>826size = PAGE_ALIGN(size +
> offset_in_page(rmr_desc->base_address));
>827
>828pr_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
Re: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
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-allyesconfig
(https://download.01.org/0day-ci/archive/20220423/202204232022.kmubee9l-...@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
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
#
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=gcc-11.2.0 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
All warnings (new ones prefixed by >>):
>> drivers/acpi/arm64/iort.c:801:29: warning: no previous prototype for
>> 'iort_rmr_alloc' [-Wmissing-prototypes]
801 | struct iommu_iort_rmr_data *iort_rmr_alloc(struct acpi_iort_rmr_desc
*rmr_desc,
| ^~
drivers/acpi/arm64/iort.c: In function 'iort_get_rmrs':
drivers/acpi/arm64/iort.c:896:34: error: 'ACPI_IORT_RMR_REMAP_PERMITTED'
undeclared (first use in this function)
896 | if (rmr->flags & ACPI_IORT_RMR_REMAP_PERMITTED)
| ^
drivers/acpi/arm64/iort.c:896:34: note: each undeclared identifier is
reported only once for each function it appears in
drivers/acpi/arm64/iort.c:901:34: error: 'ACPI_IORT_RMR_ACCESS_PRIVILEGE'
undeclared (first use in this function)
901 | if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
| ^~
drivers/acpi/arm64/iort.c:905:21: error: implicit declaration of function
'ACPI_IORT_RMR_ACCESS_ATTRIBUTES'; did you mean 'ACPI_IORT_MF_ATTRIBUTES'?
[-Werror=implicit-function-declaration]
905 | if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
| ^~~
| ACPI_IORT_MF_ATTRIBUTES
drivers/acpi/arm64/iort.c:906:33: error: 'ACPI_IORT_RMR_ATTR_DEVICE_GRE'
undeclared (first use in this function)
906 | ACPI_IORT_RMR_ATTR_DEVICE_GRE)
| ^
drivers/acpi/arm64/iort.c:909:33: error: 'ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB'
undeclared (first use in this function)
909 | ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB)
| ^
cc1: some warnings being treated as errors
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
Re: [PATCH v11 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
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-...@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
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,
