RE: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node

2022-07-01 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Shameerali Kolothum Thodi
> Sent: 28 June 2022 09:00
> To: 'Robin Murphy' ; j...@8bytes.org;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: j...@solid-run.com; Linuxarm ;
> h...@infradead.org; Guohanjun (Hanjun Guo) ;
> sami.muja...@arm.com; w...@kernel.org; wanghuiqiang
> ; lpieral...@kernel.org; Steven Price
> ; lorenzo.pieral...@gmail.com
> Subject: RE: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node
> > > Hi Will/Robin,
> > >
> > > Appreciate, if you could please take a look at the remaining SMMU
> > > related
> > > patches(7-9) and provide your approval?
> >
> > I said v12 looked fine, but for the avoidance of doubt, here it is
> > again, as formally as can be:
> >
> > Acked-by: Robin Murphy 
> 
> Thanks Robin.
> 
> Hi Joerg,
> 
> Now that we have all the required acks, could you please pick this series via
> IOMMU tree?

Hi Will,

Since Joerg hasn't replied yet, just wondering could you please take it through 
ARM
SMMU tree if that makes sense? Don't want to miss the 5.20 merge window for this
series.

Thanks,
Shameer


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node

2022-06-28 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 27 June 2022 13:26
> To: Shameerali Kolothum Thodi ;
> Steven Price ; linux-arm-ker...@lists.infradead.org;
> linux-a...@vger.kernel.org; iommu@lists.linux-foundation.org
> Cc: j...@solid-run.com; Linuxarm ;
> h...@infradead.org; Guohanjun (Hanjun Guo) ;
> sami.muja...@arm.com; w...@kernel.org; wanghuiqiang
> ; lpieral...@kernel.org
> Subject: Re: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node
> 
> On 2022-06-24 16:44, Shameerali Kolothum Thodi via iommu wrote:
> >
> >
> >> -Original Message-
> >> From: Steven Price [mailto:steven.pr...@arm.com]
> >> Sent: 17 June 2022 13:42
> >> 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)
> >> ; sami.muja...@arm.com;
> j...@solid-run.com;
> >> eric.au...@redhat.com; laurentiu.tu...@nxp.com; h...@infradead.org
> >> Subject: Re: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node
> >>
> >> On 15/06/2022 11:10, Shameer Kolothum wrote:
> >>> Hi
> >>>
> >>> v12 --> v13
> >>>-No changes. Rebased to 5.19-rc1.
> >>>-Picked up tags received from Laurentiu, Hanjun and Will. Thanks!.
> >>
> >> You've already got my Tested-by tags, but just to confirm I gave this a
> >> spin and it works fine.
> >
> > Thanks Steve.
> >
> > I think the series is now in a good shape to be merged.
> >
> > Hi Will/Robin,
> >
> > Appreciate, if you could please take a look at the remaining SMMU related
> > patches(7-9) and provide your approval?
> 
> I said v12 looked fine, but for the avoidance of doubt, here it is
> again, as formally as can be:
> 
> Acked-by: Robin Murphy 

Thanks Robin.

Hi Joerg,

Now that we have all the required acks, could you please pick this series via
IOMMU tree?

Thanks,
Shameer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node

2022-06-24 Thread Shameerali Kolothum Thodi via iommu


> -Original Message-
> From: Steven Price [mailto:steven.pr...@arm.com]
> Sent: 17 June 2022 13:42
> 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)
> ; sami.muja...@arm.com; j...@solid-run.com;
> eric.au...@redhat.com; laurentiu.tu...@nxp.com; h...@infradead.org
> Subject: Re: [PATCH v13 0/9] ACPI/IORT: Support for IORT RMR node
> 
> On 15/06/2022 11:10, Shameer Kolothum wrote:
> > Hi
> >
> > v12 --> v13
> >   -No changes. Rebased to 5.19-rc1.
> >   -Picked up tags received from Laurentiu, Hanjun and Will. Thanks!.
> 
> You've already got my Tested-by tags, but just to confirm I gave this a
> spin and it works fine.

Thanks Steve.

I think the series is now in a good shape to be merged.

Hi Will/Robin,

Appreciate, if you could please take a look at the remaining SMMU related 
patches(7-9) and provide your approval?

Thanks,
Shameer

> 
> Thanks,
> 
> Steve
> 
> >
> > Thanks,
> > Shameer
> >
> > From old:
> > We have faced issues with 3408iMR RAID controller cards which
> > fail to boot when SMMU is enabled. This is because these
> > controllers make use of host memory for various caching related
> > purposes and when SMMU is enabled the iMR firmware fails to
> > access these memory regions as there is no mapping for them.
> > IORT RMR provides a way for UEFI to describe and report these
> > memory regions so that the kernel can make a unity mapping for
> > these in SMMU.
> >
> > Change History:
> >
> > v11 --> v12
> >   -Minor fix in patch #4 to address the issue reported by the kernel test
> robot.
> >   -Added R-by tags by Christoph(patch #1) and Lorenzo(patch #4).
> >   -Added T-by from Steve to all relevant patches. Many thanks!.
> >
> > v10 --> v11
> >  -Addressed Christoph's comments. We now have a  callback to
> >   struct iommu_resv_region to free all related memory and also dropped
> >   the FW specific union and now has a container struct
> iommu_iort_rmr_data.
> >   See patches #1 & #4
> >  -Added R-by from Christoph.
> >  -Dropped R-by from Lorenzo for patches #4 & #5 due to the above
> changes.
> >  -Also dropped T-by from Steve and Laurentiu. Many thanks for your test
> >   efforts. I have done basic sanity testing on my platform but please
> >   do it again at your end.
> >
> > v9 --> v10
> >  - Dropped patch #1 ("Add temporary RMR node flag definitions") since
> >the ACPICA header updates patch is now in the mailing list
> >  - Based on the suggestion from Christoph, introduced a
> >resv_region_free_fw_data() callback in struct iommu_resv_region and
> >used that to free RMR specific memory allocations.
> >
> > v8 --> v9
> >  - Adressed comments from Robin on interfaces.
> >  - Addressed comments from Lorenzo.
> >
> > v7 --> v8
> >   - Patch #1 has temp definitions for RMR related changes till
> >     the ACPICA header changes are part of kernel.
> >   - No early parsing of RMR node info and is only parsed at the
> >     time of use.
> >   - Changes to the RMR get/put API format compared to the
> >     previous version.
> >   - Support for RMR descriptor shared by multiple stream IDs.
> >
> > v6 --> v7
> >  -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.
> >
> > v5 --> v6
> > - Addressed comments from Robin & Lorenzo.
> >   : Moved iort_parse_rmr() to acpi_iort_init() from
> >     iort_init_platform_devices().
> >   : Removed use of struct iort_rmr_entry during the initial
> >     parse. Using struct iommu_resv_region instead.
> >   : Report RMR address alignment and overlap errors, but continue.
> >   : Reworked arm_smmu_init_bypass_stes() (patch # 6).
> > - Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
> > - Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
> >   on Type of RMR region. Suggested by Jon N.
> >
> > v4 --> v5
> >  -Added a fw_data union to struct iommu_resv_region and removed
> >   struct iommu_rmr (Based on comments from Joerg/Robin).
> >  -Added iommu_put_rmrs() to release mem.
> >  -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
> >   yet because of the above changes.
> >
> > v3 -->v4
> > -Included the SMMUv2 SMR bypass install changes suggested by
> >  Steve(patch #7)
> > -As per Robin's comments, RMR reserve implementation is now
> >  more generic  (patch #8) and dropped v3 patches 8 and 10.
> > -Rebase to 5.13-rc1
> >
> > RFC v2 --> v3
> >  -Dropped RFC tag as the ACPICA header changes are now ready to be
> >   part of 5.13[0]. But this series still has a dependency on that patch.
> >  -Added IORT E.b related changes(node flags, _DSM function 5 checks for
> >   PCIe).
> >  -Changed RMR to stream id mapping from M:N to M:1 as per the spec
> and
> >   discussion here[1].
> >  -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
> >
> > Jon 

RE: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node

2022-05-25 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Shameerali Kolothum Thodi
> Sent: 17 May 2022 08:18
> To: 'Lorenzo Pieralisi' ; Robin Murphy
> ; raf...@kernel.org; j...@8bytes.org
> Cc: Guohanjun (Hanjun Guo) ;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org; Linuxarm ;
> w...@kernel.org; wanghuiqiang ;
> 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 v12 0/9] ACPI/IORT: Support for IORT RMR node
> 
> 
> > -Original Message-
> > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> > Sent: 13 May 2022 10:50
> > To: Robin Murphy ; Shameerali Kolothum Thodi
> > ; raf...@kernel.org;
> > j...@8bytes.org
> > Cc: Shameerali Kolothum Thodi ;
> > Guohanjun (Hanjun Guo) ;
> > linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> > iommu@lists.linux-foundation.org; Linuxarm ;
> > w...@kernel.org; wanghuiqiang ;
> > 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 v12 0/9] ACPI/IORT: Support for IORT RMR node
> >
> > [with Christoph's correct email address]
> >
> > On Tue, May 10, 2022 at 09:07:00AM +0100, Robin Murphy wrote:
> > > On 2022-05-10 08:23, Shameerali Kolothum Thodi wrote:
> > > > Hi Joerg/Robin,
> > > >
> > > > I think this series is now ready to be merged. Could you please let
> > > > me know if there is anything missing.
> > >
> > > Fine by me - these patches have had enough review and testing now that
> > > even if anything else did come up, I think it would be better done as
> > > follow-up work on the merged code.
> >
> > Given the ACPICA dependency I believe it is best for this series
> > to go via the ACPI tree, right ?
> >
> > I assume there are all the required ACKs for that to happen.
> 
> The SMMUv3/SMMU related changes (patches 6 - 9) still doesn't have
> explicit ACK from maintainers other than the go ahead above from Robin.
> 
> Just thought of highlighting it as not sure that will be an issue or not.
> 

All,

Just a gentle ping on this series again. Any chance this can make into 5.19?

Please consider.

Thanks,
Shameer

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node

2022-05-17 Thread Shameerali Kolothum Thodi via iommu


> -Original Message-
> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> Sent: 13 May 2022 10:50
> To: Robin Murphy ; Shameerali Kolothum Thodi
> ; raf...@kernel.org;
> j...@8bytes.org
> Cc: Shameerali Kolothum Thodi ;
> Guohanjun (Hanjun Guo) ;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org; Linuxarm ;
> w...@kernel.org; wanghuiqiang ;
> 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 v12 0/9] ACPI/IORT: Support for IORT RMR node
> 
> [with Christoph's correct email address]
> 
> On Tue, May 10, 2022 at 09:07:00AM +0100, Robin Murphy wrote:
> > On 2022-05-10 08:23, Shameerali Kolothum Thodi wrote:
> > > Hi Joerg/Robin,
> > >
> > > I think this series is now ready to be merged. Could you please let
> > > me know if there is anything missing.
> >
> > Fine by me - these patches have had enough review and testing now that
> > even if anything else did come up, I think it would be better done as
> > follow-up work on the merged code.
> 
> Given the ACPICA dependency I believe it is best for this series
> to go via the ACPI tree, right ?
> 
> I assume there are all the required ACKs for that to happen.

The SMMUv3/SMMU related changes (patches 6 - 9) still doesn't have
explicit ACK from maintainers other than the go ahead above from Robin.

Just thought of highlighting it as not sure that will be an issue or not.

Thanks,
Shameer
 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node

2022-05-10 Thread Shameerali Kolothum Thodi via iommu
Hi Joerg/Robin,

I think this series is now ready to be merged. Could you please let
me know if there is anything missing.

Thanks,
Shameer

> -Original Message-
> From: Guohanjun (Hanjun Guo)
> Sent: 05 May 2022 02:24
> 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
> ; 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 v12 0/9] ACPI/IORT: Support for IORT RMR node
> 
> On 2022/5/4 0:33, Shameer Kolothum wrote:
> > Hi
> >
> > v11 --> v12
> >-Minor fix in patch #4 to address the issue reported by the kernel test
> robot.
> >-Added R-by tags by Christoph(patch #1) and Lorenzo(patch #4).
> >-Added T-by from Steve to all relevant patches. Many thanks!.
> >
> > Please note, this series has a dependency on the ACPICA header patch
> > here[1].
> 
> Tested on a Kunpeng920 server machine with SMMUv3, the 3408iMR RAID
> controller card works as expected,
> 
> Tested-by: Hanjun Guo 
> 
> Thanks
> Hanjun
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH RFC 15/19] iommu/arm-smmu-v3: Add set_dirty_tracking_range() support

2022-05-05 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Joao Martins [mailto:joao.m.mart...@oracle.com]
> Sent: 29 April 2022 12:05
> To: Tian, Kevin 
> Cc: Joerg Roedel ; Suravee Suthikulpanit
> ; Will Deacon ; Robin
> Murphy ; Jean-Philippe Brucker
> ; zhukeqian ;
> Shameerali Kolothum Thodi ;
> David Woodhouse ; Lu Baolu
> ; Jason Gunthorpe ; Nicolin Chen
> ; Yishai Hadas ; Eric Auger
> ; Liu, Yi L ; Alex Williamson
> ; Cornelia Huck ;
> k...@vger.kernel.org; iommu@lists.linux-foundation.org
> Subject: Re: [PATCH RFC 15/19] iommu/arm-smmu-v3: Add
> set_dirty_tracking_range() support
> 
> On 4/29/22 09:28, Tian, Kevin wrote:
> >> From: Joao Martins 
> >> Sent: Friday, April 29, 2022 5:09 AM
> >>
> >> Similar to .read_and_clear_dirty() use the page table
> >> walker helper functions and set DBM|RDONLY bit, thus
> >> switching the IOPTE to writeable-clean.
> >
> > this should not be one-off if the operation needs to be
> > applied to IOPTE. Say a map request comes right after
> > set_dirty_tracking() is called. If it's agreed to remove
> > the range op then smmu driver should record the tracking
> > status internally and then apply the modifier to all the new
> > mappings automatically before dirty tracking is disabled.
> > Otherwise the same logic needs to be kept in iommufd to
> > call set_dirty_tracking_range() explicitly for every new
> > iopt_area created within the tracking window.
> 
> Gah, I totally missed that by mistake. New mappings aren't
> carrying over the "DBM is set". This needs a new io-pgtable
> quirk added post dirty-tracking toggling.
> 
> I can adjust, but I am at odds on including this in a future
> iteration given that I can't really test any of this stuff.
> Might drop the driver until I have hardware/emulation I can
> use (or maybe others can take over this). It was included
> for revising the iommu core ops and whether iommufd was
> affected by it.

[+Kunkun Jiang]. I think he is now looking into this and might have
a test setup to verify this.

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

2022-04-28 Thread Shameerali Kolothum Thodi via iommu
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

2022-04-26 Thread Shameerali Kolothum Thodi via iommu



> -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   rmr_desc->base_address + 
> rmr_desc->length - 1,
>831   addr, addr + size - 1);
>832}
>833
>834region = _data->rr;
>835INIT_LIST_HEAD(>list);
>836region->start = addr;
>837region->length = size;
>838  

RE: [PATCH v10 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions

2022-04-21 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: 21 April 2022 07:49
> To: Shameerali Kolothum Thodi 
> Cc: linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.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 v10 4/9] ACPI/IORT: Add support to retrieve IORT RMR
> reserved regions
> 
[...]

> >  void generic_iommu_put_resv_regions(struct device *dev, struct list_head
> *list)
> >  {
> > struct iommu_resv_region *entry, *next;
> >
> > -   list_for_each_entry_safe(entry, next, list, list)
> > +   list_for_each_entry_safe(entry, next, list, list) {
> > +   if (entry->resv_region_free_fw_data)
> > +   entry->resv_region_free_fw_data(dev, entry);
> > kfree(entry);
> 
> I'd move the kfree to the free callback if present.  This would also
> allow to hide the union from the common code entirely and use a
> container structure like:
> 
> struct iommu_iort_rmr_data {
>   struct iommu_resv_region rr;
> 
>   /* Stream IDs associated with IORT RMR entry */
>   const u32 *sids;
>   u32 num_sids;
> };

Ok. I will respin soon with the above changes.

Thanks,
Shameer 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v10 0/9] ACPI/IORT: Support for IORT RMR node

2022-04-21 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Steven Price [mailto:steven.pr...@arm.com]
> Sent: 21 April 2022 13:59
> 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)
> ; sami.muja...@arm.com; j...@solid-run.com;
> eric.au...@redhat.com; laurentiu.tu...@nxp.com; h...@infradead.org
> Subject: Re: [PATCH v10 0/9] ACPI/IORT: Support for IORT RMR node
> 
> On 20/04/2022 17:48, Shameer Kolothum wrote:
> > Hi
> >
> > v9 --> v10
> >  - Dropped patch #1 ("Add temporary RMR node flag definitions") since
> >the ACPICA header updates patch is now in the mailing list[1]
> >  - Based on the suggestion from Christoph, introduced a
> >resv_region_free_fw_data() callback in struct iommu_resv_region and
> >used that to free RMR specific memory allocations.
> >
> > Though there is a small change from v9 with respect to how we free up
> > the FW specific data, I have taken the liberty to pick up the R-by and
> > T-by tags from Lorenzo, Steve and Laurentiu. But please do take a look
> > again and let me know.
> 
> I've given this a go and it works fine on my Juno setup. So do keep my
> T-by tag.

Many thanks for that.

> Sami has been kind enough to give me an updated firmware which also
> fixes the RMR node in the IORT. Although as mentioned before the details
> of the RMR node are currently being ignored so this doesn't change the
> functionality but silences the warning.
> 
> My concern is that with the RMR region effectively ignored we may see
> more broken firmware, and while a length of zero produces a warning, an
> otherwise incorrect length will currently "silently work" but mean that
> any future tightening would cause problems. For example if the SMMU
> driver were to recreate the mappings to only cover the region specified
> in the RMR it may not be large enough if the RMR base/length are not
> correct.

Not sure how we can further validate the RMR if the firmware provides an
incorrect one. I see your point of future tightening causing problems
with broken firmware. But then it is indeed a "broken firmware"...

 It's up to the maintainers as to whether they see this as a
> problem or not.

Hi Robin,

Any thoughts on this?

Thanks,
Shameer

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v9 06/11] ACPI/IORT: Add support to retrieve IORT RMR reserved regions

2022-04-19 Thread Shameerali Kolothum Thodi via iommu


> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: 07 April 2022 15:00
> To: Robin Murphy 
> Cc: Christoph Hellwig ; Shameerali Kolothum Thodi
> ; j...@solid-run.com; Linuxarm
> ; steven.pr...@arm.com; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org; wanghuiqiang
> ; Guohanjun (Hanjun Guo)
> ; yangyicong ;
> sami.muja...@arm.com; w...@kernel.org;
> linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH v9 06/11] ACPI/IORT: Add support to retrieve IORT RMR
> reserved regions
> 
> On Thu, Apr 07, 2022 at 02:53:38PM +0100, Robin Murphy wrote:
> > > Why can't this just go into generic_iommu_put_resv_regions?  The idea
> > > that the iommu low-level drivers need to call into dma-iommu which is
> > > a consumer of the IOMMU API is odd.  Especially if that just calls out
> > > to ACPI code and generic IOMMU code only anyway.
> >
> > Because assuming ACPI means IORT is not generic. Part of the aim in adding
> > the union to iommu_resv_region is that stuff like AMD's unity_map_entry
> and
> > Intel's dmar_rmrr_unit can be folded into it as well, and their reserved
> > region handling correspondingly simplified too.
> >
> > The iommu_dma_{get,put}_resv_region() helpers are kind of intended to be
> > specific to the fwnode mechanism which deals with IORT and devicetree
> (once
> > the reserved region bindings are fully worked out).
> 
> But IORT is not driver₋specific code.  So we'll need a USE_IORT flag
> somewhere in core IOMMU code instead of trying to stuff this into
> driver operations.  and dma-iommu mostly certainly implies IORT even
> less than ACPI.

Sorry for the delayed response(was on holidays). So if we move the
iommu_dma_put_resv_region() call to generic_iommu_put_resv_regions() ,
will that address the concerns here?

I think it will resolve the issue in 05/11 as well pointed out by Christoph
where we end up not releasing reserved regions when
CONFIG_IOMMU_DMA is not set.

Something like below,

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f2c45b85b9fc..d08204a25ba8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2605,6 +2605,8 @@ void generic_iommu_put_resv_regions(struct
device *dev, struct list_head *list)
 {
struct iommu_resv_region *entry, *next;

+   iommu_dma_put_resv_regions(dev, list);
+
list_for_each_entry_safe(entry, next, list, list)
kfree(entry);
 }

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 5811233dc9fb..8cb1e419db49 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -393,8 +393,6 @@ void iommu_dma_put_resv_regions(struct device
*dev, struct list_head *list)
 {
if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
iort_iommu_put_resv_regions(dev, list);
-
-   generic_iommu_put_resv_regions(dev, list);
 }

Please let me know if this is not good enough.

Thanks,
Shameer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR memory regions

2022-03-23 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 22 March 2022 19:09
> To: Shameerali Kolothum Thodi ;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: j...@solid-run.com; Linuxarm ;
> steven.pr...@arm.com; Guohanjun (Hanjun Guo) ;
> yangyicong ; sami.muja...@arm.com;
> w...@kernel.org; wanghuiqiang 
> Subject: Re: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR
> memory regions
> 
> On 2022-02-21 15:43, Shameer Kolothum via iommu wrote:
> > Add helper functions (iort_iommu_get/put_rmrs()) that
> > retrieves/releases RMR memory descriptors associated
> > with a given IOMMU. This will be used by IOMMU drivers
> > to set up necessary mappings.
> >
> > Invoke it from the generic iommu helper functions.
> 
> iommu_dma_get_resv_regions() already exists - please extend that rather
> than adding a parallel implementation of the same thing but different.
> IORT should export a single get_resv_regions helper which combines the
> new RMRs with the existing MSI workaround, and a separate "do I need to
> bypass this StreamID" helper for the SMMU drivers to call directly at
> reset time, since the latter isn't really an iommu-dma responsibility.

Right. I actually had couple of back and forth on the interfaces and settled
on this mainly because it just requires a single interface from IORT for both
get_resv_regions() and SMMU driver reset bypass path.
ie, iort_iommu_get/put_rmrs()).

But agree the above comment is also valid. 

> I'm happy to do that just by shuffling wrappers around for now - we can
> come back and streamline the code properly afterwards - but the sheer
> amount of indirection currently at play here is so hard to follow that
> it's not even all that easy to see how it's crossing abstraction levels
> improperly.

Please find below the revised ones. Please take a look and let me know.

Thanks,
Shameer

iommu-dma:

void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) {

if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
iort_iommu_get_resv_regions(dev, list);
}

void iommu_dma_put_resv_regions(struct device *dev, struct list_head *list) {

if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
iort_iommu_put_resv_regions(dev, list);
generic_iommu_put_resv_regions(dev, list);
}

iort:

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_get_rmrs(fwspec->iommu_fwnode, dev, head);
}

void iort_iommu_put_resv_regions(struct device *dev, struct list_head *head) {
   ./*Free both RMRs and HW MSI ones */
}

/* The below ones will be directly called from SMMU drivers during reset */
void iort_get_rmr_sids(struct fwnode_handle *iommu_fwnode, struct list_head 
*head) {
iort_iommu_get_rmrs(iommu_fwnode, NULL, head); }
}

void iort_put_rmr_sids(struct fwnode_handle *iommu_fwnode, struct list_head 
*head) {
iort_iommu_put_resv_regions(NULL, head);
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v8 02/11] iommu: Introduce a union to struct iommu_resv_region

2022-03-23 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 22 March 2022 18:27
> 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; w...@kernel.org; wanghuiqiang
> ; Guohanjun (Hanjun Guo)
> ; steven.pr...@arm.com; sami.muja...@arm.com;
> j...@solid-run.com; eric.au...@redhat.com; yangyicong
> 
> Subject: Re: [PATCH v8 02/11] iommu: Introduce a union to struct
> iommu_resv_region
> 
> On 2022-02-21 15:43, Shameer Kolothum wrote:
> > A union is introduced to struct iommu_resv_region to hold
> > any firmware specific data. This is in preparation to add
> > support for IORT RMR reserve regions and the union now holds
> > the RMR specific information.
> >
> > Signed-off-by: Shameer Kolothum
> 
> > ---
> >   include/linux/iommu.h | 9 +
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index de0c57a567c8..b06952a75f95 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -126,6 +126,11 @@ enum iommu_resv_type {
> > IOMMU_RESV_SW_MSI,
> >   };
> >
> > +struct iommu_iort_rmr_data {
> > +   u32 *sids;  /* Stream Ids associated with IORT RMR entry */
> 
> Please make this const.
> 
> Further nit: capitalisation of "IDs" in the comment, otherwise I might
> worry about the possibility of Stream Egos too :P

True :). Will do that.

Thanks,
Shameer 

> 
> > +   u32 num_sids;
> > +};
> > +
> >   /**
> >* struct iommu_resv_region - descriptor for a reserved memory region
> >* @list: Linked list pointers
> > @@ -133,6 +138,7 @@ enum iommu_resv_type {
> >* @length: Length of the region in bytes
> >* @prot: IOMMU Protection flags (READ/WRITE/...)
> >* @type: Type of the reserved region
> > + * @fw_data: FW specific reserved region data
> 
> Nit: we've got plenty of room to spell out "Firmware-specific", and it
> never hurts to make documentation as easy to read as possible.
> 
> Thanks,
> Robin.
> 
> >*/
> >   struct iommu_resv_region {
> > struct list_headlist;
> > @@ -140,6 +146,9 @@ struct iommu_resv_region {
> > size_t  length;
> > int prot;
> > enum iommu_resv_typetype;
> > +   union {
> > +   struct iommu_iort_rmr_data rmr;
> > +   } fw_data;
> >   };
> >
> >   /**
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

2022-03-17 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Eric Auger [mailto:eric.au...@redhat.com]
> Sent: 15 March 2022 17:53
> 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 00/11] ACPI/IORT: Support for IORT RMR node
> 
> Hi Shameer,

[...]

> >
> > Jon Nettleton (1):
> >   iommu/arm-smmu: Get associated RMR info and install bypass SMR
> >
> > Shameer Kolothum (10):
> >   ACPI/IORT: Add temporary RMR node flag definitions
> >   iommu: Introduce a union to struct iommu_resv_region
> >   ACPI/IORT: Add helper functions to parse RMR nodes
> >   iommu/dma: Introduce generic helper to retrieve RMR info
> >   ACPI/IORT: Add a helper to retrieve RMR memory regions
> >   iommu/arm-smmu-v3: Introduce strtab init helper
> >   iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
> > bypass
> >   iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
> >   iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev
> >   iommu/arm-smmu: Reserve any RMR regions associated with a dev
> fyi, the last 2 patches have conflicts with
> [PATCH v4 9/9] iommu: Split struct iommu_ops
> which was applied on core branch.

Hi Eric,

Thanks for the heads up. I am going to respin this series soon.

All,

Please let me know if there are any further comments on this series.

Thanks,
Shameer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

2022-03-11 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Eric Auger [mailto:eric.au...@redhat.com]
> Sent: 11 March 2022 08:07
> 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 00/11] ACPI/IORT: Support for IORT RMR node
> 
> Hi Shameer,
> 
> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> > Hi,
> >
> > Since we now have an updated verion[0] of IORT spec(E.d) which
> > addresses the memory attributes issues discussed here [1],
> > this series now make use of it.
> >
> > The pull request for ACPICA E.d related changes are already
> > raised and can be found here,
> > https://github.com/acpica/acpica/pull/752
> >
> > v7 --> v8
> >   - Patch #1 has temp definitions for RMR related changes till
> > the ACPICA header changes are part of kernel.
> >   - No early parsing of RMR node info and is only parsed at the
> > time of use.
> >   - Changes to the RMR get/put API format compared to the
> > previous version.
> >   - Support for RMR descriptor shared by multiple stream IDs.
> 
> I tested it on guest side for host MSI SW RESV region flat mapping
> (using both the old single mapping layout and the now allowed multiple
> RID ID mapping format) and this worked for me. Feel free to add my
> 
> Tested-by: Eric Auger 

Thanks Eric for verifying this, especially the multiple RID mapping case.

Cheers,
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

2022-03-10 Thread Shameerali Kolothum Thodi via iommu
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 00/11] ACPI/IORT: Support for IORT RMR node

2022-03-03 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Steven Price [mailto:steven.pr...@arm.com]
> Sent: 03 March 2022 10:38
> 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)
> ; sami.muja...@arm.com; j...@solid-run.com;
> eric.au...@redhat.com; yangyicong 
> Subject: Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node
> 
> On 21/02/2022 15:43, Shameer Kolothum wrote:
> > Hi,
> >
> > Since we now have an updated verion[0] of IORT spec(E.d) which
> > addresses the memory attributes issues discussed here [1],
> > this series now make use of it.
> >
> > The pull request for ACPICA E.d related changes are already
> > raised and can be found here,
> > https://github.com/acpica/acpica/pull/752
> >
> > v7 --> v8
> >   - Patch #1 has temp definitions for RMR related changes till
> > the ACPICA header changes are part of kernel.
> >   - No early parsing of RMR node info and is only parsed at the
> > time of use.
> >   - Changes to the RMR get/put API format compared to the
> > previous version.
> >   - Support for RMR descriptor shared by multiple stream IDs.
> >
> > Please take a look and let me know your thoughts.
> 
> Hi Shameer,
> 
> I've now been able to test this on the Juno platform with a modified
> firmware supporting the newer spec (thanks Sami!). Everything works, so
> feel free to add my:
> 
> Tested-by: Steven Price 
> 
> (Note that I haven't tested the smmu-v3 support)

Thanks Steve, for giving it a spin and verifying.

Cheers,
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

2022-02-25 Thread Shameerali Kolothum Thodi via iommu


> -Original Message-
> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> Sent: 24 February 2022 10:14
> 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; yangyicong
> 
> Subject: Re: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR
> nodes
> 
> On Mon, Feb 21, 2022 at 03:43:36PM +, 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(+)
> 
> I have very minor comments - I would ask Robin to ack the updated
> flags management.
> 
> Functions should be introduced where they are used, this patch
> is not bisectable:

Right. I missed the below warning. Will do.

> drivers/acpi/arm64/iort.c:1028:13: warning: ‘iort_find_rmrs’ defined but not
> used [-Wunused-function]
>  1028 | static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device
> *dev,
> 
> > 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;
> 
> We could probably check for length != 0, the kernel is not there to
> validate firmware but this would not hurt either.

Ok.

> > +
> > +   /* 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 |= 

RE: [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

2022-01-25 Thread Shameerali Kolothum Thodi via iommu
Hi Robin/Lorenzo,

> -Original Message-
> From: iommu [mailto:iommu-boun...@lists.linux-foundation.org] On Behalf
> Of Shameer Kolothum
> Sent: 05 August 2021 09:07
> To: linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: robin.mur...@arm.com; j...@solid-run.com; Linuxarm
> ; steven.pr...@arm.com; Guohanjun (Hanjun Guo)
> ; yangyicong ;
> sami.muja...@arm.com; w...@kernel.org; wanghuiqiang
> 
> Subject: [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node
> 
> Hi,
> 
> The series adds support to IORT RMR nodes specified in IORT
> Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
> memory ranges that are used by endpoints and require a unity
> mapping in SMMU.
> 
> We have faced issues with 3408iMR RAID controller cards which
> fail to boot when SMMU is enabled. This is because these
> controllers make use of host memory for various caching related
> purposes and when SMMU is enabled the iMR firmware fails to
> access these memory regions as there is no mapping for them.
> IORT RMR provides a way for UEFI to describe and report these
> memory regions so that the kernel can make a unity mapping for
> these in SMMU.
> 
> Change History:
> 
> v6 --> v7
> 
> The only change from v6 is the fix pointed out by Steve to
> the SMMUv2 SMR bypass install in patch #8.
> 
> Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
> Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
> yet as the series still needs more review[1].
> 
> Feedback and tests on this series is very much appreciated.

Since we have an update to IORT spec(E.c) now[1] and includes additional
attributes/flags for the RMR node, I am planning to respin this series soon.

Going through the new spec, I have a few queries,

The memory range attributes can now be described as one of the following,

0x00: Device-nGnRnE memory
0x01: Device-nGnRE memory
0x02: Device-nGRE memory
0x03: Device-GRE memory
0x04: Normal Inner Non-cacheable Outer Non-cacheable
0x05: Normal Inner Write-back Outer Write-back Inner Shareable

I am not sure how this needs to be captured and used in the kernel. Is there
any intention of using these fine-grained attributes in the kernel now
or a generic mapping of the above to the struct iommu_rev_region prot field
is enough? i.e., something like,

{

prot = IOMMU_READ | IOMMU_WRITE;

if (rmr_attr == normal_mem) // 0x05
prot |= IOMMU_CACHE;

if (rmr_attr == device_mem) { //0x00 - 0x03
prot |= IOMMU_MMIO;
prot |= IOMMU_NOEXEC;
}

}

Similarly for the 'flags' field, the new 'Access Privilege' is intended to set 
the
IOMMU_PRIV ?
  
Please let me know.

Thanks,
Shameer

[1] https://developer.arm.com/documentation/den0049/ec/?lang=en

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [RFC v16 0/9] SMMUv3 Nested Stage Setup (IOMMU part)

2021-12-08 Thread Shameerali Kolothum Thodi via iommu


> -Original Message-
> From: Eric Auger [mailto:eric.au...@redhat.com]
> Sent: 07 December 2021 11:06
> To: Zhangfei Gao ; eric.auger@gmail.com;
> iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org;
> k...@vger.kernel.org; kvm...@lists.cs.columbia.edu; j...@8bytes.org;
> w...@kernel.org; robin.mur...@arm.com; jean-phili...@linaro.org;
> zhukeqian 
> Cc: alex.william...@redhat.com; jacob.jun@linux.intel.com;
> yi.l@intel.com; kevin.t...@intel.com; ashok@intel.com;
> m...@kernel.org; peter.mayd...@linaro.org; vivek.gau...@arm.com;
> Shameerali Kolothum Thodi ;
> wangxingang ; jiangkunkun
> ; yuzenghui ;
> nicoleots...@gmail.com; chenxiang (M) ;
> sum...@nvidia.com; nicol...@nvidia.com; vdu...@nvidia.com;
> zhangfei@gmail.com; lushenm...@huawei.com; vse...@nvidia.com
> Subject: Re: [RFC v16 0/9] SMMUv3 Nested Stage Setup (IOMMU part)
> 
> Hi Zhangfei,
> 
> On 12/7/21 11:35 AM, Zhangfei Gao wrote:
> >
> >
> > On 2021/12/7 下午6:27, Eric Auger wrote:
> >> Hi Zhangfei,
> >>
> >> On 12/3/21 1:27 PM, Zhangfei Gao wrote:
> >>> Hi, Eric
> >>>
> >>> On 2021/10/27 下午6:44, Eric Auger wrote:
>  This series brings the IOMMU part of HW nested paging support
>  in the SMMUv3.
> 
>  The SMMUv3 driver is adapted to support 2 nested stages.
> 
>  The IOMMU API is extended to convey the guest stage 1
>  configuration and the hook is implemented in the SMMUv3 driver.
> 
>  This allows the guest to own the stage 1 tables and context
>  descriptors (so-called PASID table) while the host owns the
>  stage 2 tables and main configuration structures (STE).
> 
>  This work mainly is provided for test purpose as the upper
>  layer integration is under rework and bound to be based on
>  /dev/iommu instead of VFIO tunneling. In this version we also get
>  rid of the MSI BINDING ioctl, assuming the guest enforces
>  flat mapping of host IOVAs used to bind physical MSI doorbells.
>  In the current QEMU integration this is achieved by exposing
>  RMRs to the guest, using Shameer's series [1]. This approach
>  is RFC as the IORT spec is not really meant to do that
>  (single mapping flag limitation).
> 
>  Best Regards
> 
>  Eric
> 
>  This series (Host) can be found at:
>  https://github.com/eauger/linux/tree/v5.15-rc7-nested-v16
>  This includes a rebased VFIO integration (although not meant
>  to be upstreamed)
> 
>  Guest kernel branch can be found at:
>  https://github.com/eauger/linux/tree/shameer_rmrr_v7
>  featuring [1]
> 
>  QEMU integration (still based on VFIO and exposing RMRs)
>  can be found at:
> 
> https://github.com/eauger/qemu/tree/v6.1.0-rmr-v2-nested_smmuv3_v10
>  (use iommu=nested-smmuv3 ARM virt option)
> 
>  Guest dependency:
>  [1] [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node
> >>> Thanks a lot for upgrading these patches.
> >>>
> >>> I have basically verified these patches on HiSilicon Kunpeng920.
> >>> And integrated them to these branches.
> >>> https://github.com/Linaro/linux-kernel-uadk/tree/uacce-devel-5.16
> >>> https://github.com/Linaro/qemu/tree/v6.1.0-rmr-v2-nested_smmuv3_v10
> >>>
> >>> Though they are provided for test purpose,
> >>>
> >>> Tested-by: Zhangfei Gao 
> >> Thank you very much. As you mentioned, until we do not have the
> >> /dev/iommu integration this is maintained for testing purpose. The SMMU
> >> changes shouldn't be much impacted though.
> >> The added value of this respin was to propose an MSI binding solution
> >> based on RMRRs which simplify things at kernel level.
> >
> > Current RMRR solution requires uefi enabled,
> > and QEMU_EFI.fd  has to be provided to start qemu.
> >
> > Any plan to support dtb as well, which will be simpler since no need
> > QEMU_EFI.fd anymore.
> Yes the solution is based on ACPI IORT nodes. No clue if some DT
> integration is under work. Shameer?

There was some attempt in the past to create identity mappings using DT.
This is the latest I can find on it,
https://lore.kernel.org/linux-iommu/yteldhx2reiivv%...@orome.fritz.box/T/

Thanks,
Shameer

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu