* Re: [RFC PATCH v4 12/15] iommu/io-pgtable-arm-v7s: Implement arm_v7s_unmap_pages()
[not found] ` <20210408045241.27316-13-isaacm@codeaurora.org>
@ 2021-04-08 13:58 ` Will Deacon
[not found] ` <342d453c1e5d737122b43be006de6077@codeaurora.org>
0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2021-04-08 13:58 UTC (permalink / raw)
To: Isaac J. Manjarres; +Cc: iommu, linux-arm-kernel, robin.murphy, pratikp
On Wed, Apr 07, 2021 at 09:52:38PM -0700, Isaac J. Manjarres wrote:
> Implement the unmap_pages() callback for the ARM v7s io-pgtable
> format.
>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> ---
> drivers/iommu/io-pgtable-arm-v7s.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> index d4004bcf333a..5e203e03c352 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -710,15 +710,32 @@ static size_t __arm_v7s_unmap(struct arm_v7s_io_pgtable *data,
> return __arm_v7s_unmap(data, gather, iova, size, lvl + 1, ptep);
> }
>
> -static size_t arm_v7s_unmap(struct io_pgtable_ops *ops, unsigned long iova,
> - size_t size, struct iommu_iotlb_gather *gather)
> +static size_t arm_v7s_unmap_pages(struct io_pgtable_ops *ops, unsigned long iova,
> + size_t pgsize, size_t pgcount,
> + struct iommu_iotlb_gather *gather)
> {
> struct arm_v7s_io_pgtable *data = io_pgtable_ops_to_data(ops);
> + size_t unmapped = 0, ret;
>
> if (WARN_ON(iova >= (1ULL << data->iop.cfg.ias)))
> return 0;
>
> - return __arm_v7s_unmap(data, gather, iova, size, 1, data->pgd);
> + while (pgcount--) {
> + ret = __arm_v7s_unmap(data, gather, iova, pgsize, 1, data->pgd);
> + if (!ret)
> + break;
> +
> + unmapped += pgsize;
> + iova += pgsize;
> + }
> +
> + return unmapped;
> +}
Wait -- don't you need to hook this up somewhere (likewise for ->map_pages)?
How are you testing this?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v4 06/15] iommu: Split 'addr_merge' argument to iommu_pgsize() into separate parts
[not found] ` <20210408045241.27316-7-isaacm@codeaurora.org>
@ 2021-04-08 13:59 ` Will Deacon
0 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2021-04-08 13:59 UTC (permalink / raw)
To: Isaac J. Manjarres; +Cc: iommu, linux-arm-kernel, robin.murphy, pratikp
On Wed, Apr 07, 2021 at 09:52:32PM -0700, Isaac J. Manjarres wrote:
> From: Will Deacon <will@kernel.org>
>
> The 'addr_merge' parameter to iommu_pgsize() is a fabricated address
> intended to describe the alignment requirements to consider when
> choosing an appropriate page size. On the iommu_map() path, this address
> is the logical OR of the virtual and physical addresses.
>
> Subsequent improvements to iommu_pgsize() will need to check the
> alignment of the virtual and physical components of 'addr_merge'
> independently, so pass them in as separate parameters and reconstruct
> 'addr_merge' locally.
>
> No functional change.
>
> Signed-off-by: Will Deacon <will@kernel.org>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> ---
> drivers/iommu/iommu.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index bcd623862bf9..ab689611a03b 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2357,12 +2357,13 @@ phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
> }
> EXPORT_SYMBOL_GPL(iommu_iova_to_phys);
>
> -static size_t iommu_pgsize(struct iommu_domain *domain,
> - unsigned long addr_merge, size_t size)
> +static size_t iommu_pgsize(struct iommu_domain *domain, unsigned long iova,
> + phys_addr_t paddr, size_t size)
> {
> unsigned int pgsize_idx;
> unsigned long pgsizes;
> size_t pgsize;
> + phys_addr_t addr_merge = paddr | iova;
^^^ this needs to be 'unsigned long' as it was before (otherwise using
GENMASK _is_ a problem).
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v4 09/15] iommu/io-pgtable-arm: Prepare PTE methods for handling multiple entries
[not found] ` <20210408045241.27316-10-isaacm@codeaurora.org>
@ 2021-04-08 13:59 ` Will Deacon
2021-04-08 14:02 ` Christoph Hellwig
0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2021-04-08 13:59 UTC (permalink / raw)
To: Isaac J. Manjarres
Cc: iommu, linux-arm-kernel, baolu.lu, robin.murphy, pratikp
On Wed, Apr 07, 2021 at 09:52:35PM -0700, Isaac J. Manjarres wrote:
> The PTE methods currently operate on a single entry. In preparation
> for manipulating multiple PTEs in one map or unmap call, allow them
> to handle multiple PTEs.
>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> ---
> drivers/iommu/io-pgtable-arm.c | 78 +++++++++++++++++++---------------
> 1 file changed, 44 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 87def58e79b5..ea66b10c04c4 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -232,20 +232,23 @@ static void __arm_lpae_free_pages(void *pages, size_t size,
> free_pages((unsigned long)pages, get_order(size));
> }
>
> -static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep,
> +static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep, int num_entries,
> struct io_pgtable_cfg *cfg)
> {
> dma_sync_single_for_device(cfg->iommu_dev, __arm_lpae_dma_addr(ptep),
> - sizeof(*ptep), DMA_TO_DEVICE);
> + sizeof(*ptep) * num_entries, DMA_TO_DEVICE);
> }
Have you tested this with CONFIG_DMA_API_DEBUG=y? I _think_ it should be
ok as long as we don't attempt to sync across a page boundary, but it would
be good to give it a spin just to check.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v4 09/15] iommu/io-pgtable-arm: Prepare PTE methods for handling multiple entries
2021-04-08 13:59 ` [RFC PATCH v4 09/15] iommu/io-pgtable-arm: Prepare PTE methods for handling multiple entries Will Deacon
@ 2021-04-08 14:02 ` Christoph Hellwig
2021-04-08 14:20 ` Will Deacon
0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2021-04-08 14:02 UTC (permalink / raw)
To: Will Deacon
Cc: Isaac J. Manjarres, pratikp, iommu, robin.murphy, linux-arm-kernel
On Thu, Apr 08, 2021 at 02:59:26PM +0100, Will Deacon wrote:
> > -static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep,
> > +static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep, int num_entries,
> > struct io_pgtable_cfg *cfg)
> > {
> > dma_sync_single_for_device(cfg->iommu_dev, __arm_lpae_dma_addr(ptep),
> > - sizeof(*ptep), DMA_TO_DEVICE);
> > + sizeof(*ptep) * num_entries, DMA_TO_DEVICE);
> > }
>
> Have you tested this with CONFIG_DMA_API_DEBUG=y? I _think_ it should be
> ok as long as we don't attempt to sync across a page boundary, but it would
> be good to give it a spin just to check.
syncing over a page boundary is perfectly fine. It just needs to say in
the bounds of the original mapping.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v4 09/15] iommu/io-pgtable-arm: Prepare PTE methods for handling multiple entries
2021-04-08 14:02 ` Christoph Hellwig
@ 2021-04-08 14:20 ` Will Deacon
0 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2021-04-08 14:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Isaac J. Manjarres, pratikp, iommu, robin.murphy, linux-arm-kernel
On Thu, Apr 08, 2021 at 03:02:30PM +0100, Christoph Hellwig wrote:
> On Thu, Apr 08, 2021 at 02:59:26PM +0100, Will Deacon wrote:
> > > -static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep,
> > > +static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep, int num_entries,
> > > struct io_pgtable_cfg *cfg)
> > > {
> > > dma_sync_single_for_device(cfg->iommu_dev, __arm_lpae_dma_addr(ptep),
> > > - sizeof(*ptep), DMA_TO_DEVICE);
> > > + sizeof(*ptep) * num_entries, DMA_TO_DEVICE);
> > > }
> >
> > Have you tested this with CONFIG_DMA_API_DEBUG=y? I _think_ it should be
> > ok as long as we don't attempt to sync across a page boundary, but it would
> > be good to give it a spin just to check.
>
> syncing over a page boundary is perfectly fine. It just needs to say in
> the bounds of the original mapping.
Yes, you're right. I got the CPU page size mixed up with the IOMMU page
size, so I think we're good as the allocations here are made at IOMMU
page size granularity.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v4 10/15] iommu/io-pgtable-arm: Implement arm_lpae_unmap_pages()
[not found] ` <20210408045241.27316-11-isaacm@codeaurora.org>
@ 2021-04-08 14:32 ` Will Deacon
0 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2021-04-08 14:32 UTC (permalink / raw)
To: Isaac J. Manjarres
Cc: iommu, linux-arm-kernel, baolu.lu, robin.murphy, pratikp
On Wed, Apr 07, 2021 at 09:52:36PM -0700, Isaac J. Manjarres wrote:
> Implement the unmap_pages() callback for the ARM LPAE io-pgtable
> format.
>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> Suggested-by: Will Deacon <will@kernel.org>
> ---
> drivers/iommu/io-pgtable-arm.c | 70 ++++++++++++++++++++++------------
> 1 file changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index ea66b10c04c4..6700685f81d4 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -253,8 +253,8 @@ static void __arm_lpae_set_pte(arm_lpae_iopte *ptep, arm_lpae_iopte pte,
>
> static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
> struct iommu_iotlb_gather *gather,
> - unsigned long iova, size_t size, int lvl,
> - arm_lpae_iopte *ptep);
> + unsigned long iova, size_t size, size_t pgcount,
> + int lvl, arm_lpae_iopte *ptep);
>
> static void __arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
> phys_addr_t paddr, arm_lpae_iopte prot,
> @@ -298,7 +298,7 @@ static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
> size_t sz = ARM_LPAE_BLOCK_SIZE(lvl, data);
>
> tblp = ptep - ARM_LPAE_LVL_IDX(iova, lvl, data);
> - if (__arm_lpae_unmap(data, NULL, iova + i * sz, sz,
> + if (__arm_lpae_unmap(data, NULL, iova + i * sz, sz, 1,
> lvl, tblp) != sz) {
> WARN_ON(1);
> return -EINVAL;
> @@ -526,14 +526,14 @@ static size_t arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data,
> struct iommu_iotlb_gather *gather,
> unsigned long iova, size_t size,
> arm_lpae_iopte blk_pte, int lvl,
> - arm_lpae_iopte *ptep)
> + arm_lpae_iopte *ptep, size_t pgcount)
> {
> struct io_pgtable_cfg *cfg = &data->iop.cfg;
> arm_lpae_iopte pte, *tablep;
> phys_addr_t blk_paddr;
> size_t tablesz = ARM_LPAE_GRANULE(data);
> size_t split_sz = ARM_LPAE_BLOCK_SIZE(lvl, data);
> - int i, unmap_idx = -1;
> + int i, unmap_idx_start = -1, num_entries = 0, max_entries;
>
> if (WARN_ON(lvl == ARM_LPAE_MAX_LEVELS))
> return 0;
> @@ -542,15 +542,18 @@ static size_t arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data,
> if (!tablep)
> return 0; /* Bytes unmapped */
>
> - if (size == split_sz)
> - unmap_idx = ARM_LPAE_LVL_IDX(iova, lvl, data);
> + if (size == split_sz) {
> + unmap_idx_start = ARM_LPAE_LVL_IDX(iova, lvl, data);
> + max_entries = (tablesz >> ilog2(sizeof(pte))) - unmap_idx_start;
> + num_entries = min_t(int, pgcount, max_entries);
> + }
>
> blk_paddr = iopte_to_paddr(blk_pte, data);
> pte = iopte_prot(blk_pte);
>
> for (i = 0; i < tablesz / sizeof(pte); i++, blk_paddr += split_sz) {
Given that we already have a 'tablesz / sizeof(pte)' expression here, I'd be
inclined to have either a local variable or a macro helper to get at the
ptes_per_table value that you also need to compute max_entries.
> /* Unmap! */
> - if (i == unmap_idx)
> + if (i >= unmap_idx_start && i < (unmap_idx_start + num_entries))
> continue;
>
> __arm_lpae_init_pte(data, blk_paddr, pte, lvl, 1, &tablep[i]);
> @@ -568,38 +571,45 @@ static size_t arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data,
> return 0;
>
> tablep = iopte_deref(pte, data);
> - } else if (unmap_idx >= 0) {
> - io_pgtable_tlb_add_page(&data->iop, gather, iova, size);
> - return size;
> + } else if (unmap_idx_start >= 0) {
> + for (i = 0; i < num_entries; i++)
> + io_pgtable_tlb_add_page(&data->iop, gather, iova + i * size, size);
I suppose we could add a count paramater to the iotlb gather stuff in
future too, but for now this is fine as this series is already pretty big.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v4 12/15] iommu/io-pgtable-arm-v7s: Implement arm_v7s_unmap_pages()
[not found] ` <342d453c1e5d737122b43be006de6077@codeaurora.org>
@ 2021-04-08 14:32 ` Will Deacon
0 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2021-04-08 14:32 UTC (permalink / raw)
To: isaacm; +Cc: iommu, linux-arm-kernel, robin.murphy, pratikp
On Thu, Apr 08, 2021 at 07:19:29AM -0700, isaacm@codeaurora.org wrote:
> On 2021-04-08 06:58, Will Deacon wrote:
> > On Wed, Apr 07, 2021 at 09:52:38PM -0700, Isaac J. Manjarres wrote:
> > > Implement the unmap_pages() callback for the ARM v7s io-pgtable
> > > format.
> > >
> > > Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> > > ---
> > > drivers/iommu/io-pgtable-arm-v7s.c | 23 ++++++++++++++++++++---
> > > 1 file changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
> > > b/drivers/iommu/io-pgtable-arm-v7s.c
> > > index d4004bcf333a..5e203e03c352 100644
> > > --- a/drivers/iommu/io-pgtable-arm-v7s.c
> > > +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> > > @@ -710,15 +710,32 @@ static size_t __arm_v7s_unmap(struct
> > > arm_v7s_io_pgtable *data,
> > > return __arm_v7s_unmap(data, gather, iova, size, lvl + 1, ptep);
> > > }
> > >
> > > -static size_t arm_v7s_unmap(struct io_pgtable_ops *ops, unsigned
> > > long iova,
> > > - size_t size, struct iommu_iotlb_gather *gather)
> > > +static size_t arm_v7s_unmap_pages(struct io_pgtable_ops *ops,
> > > unsigned long iova,
> > > + size_t pgsize, size_t pgcount,
> > > + struct iommu_iotlb_gather *gather)
> > > {
> > > struct arm_v7s_io_pgtable *data = io_pgtable_ops_to_data(ops);
> > > + size_t unmapped = 0, ret;
> > >
> > > if (WARN_ON(iova >= (1ULL << data->iop.cfg.ias)))
> > > return 0;
> > >
> > > - return __arm_v7s_unmap(data, gather, iova, size, 1, data->pgd);
> > > + while (pgcount--) {
> > > + ret = __arm_v7s_unmap(data, gather, iova, pgsize, 1, data->pgd);
> > > + if (!ret)
> > > + break;
> > > +
> > > + unmapped += pgsize;
> > > + iova += pgsize;
> > > + }
> > > +
> > > + return unmapped;
> > > +}
> >
> > Wait -- don't you need to hook this up somewhere (likewise for
> > ->map_pages)?
> Done. Likewise for map_pages(). I'm not sure how the compiler didn't catch
> this; I'm compile testing this, as I don't have hardware that uses the short
> descriptor format.
Damn, neither do I :/ My seattle has all the memory high up iirc.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-08 14:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210408045241.27316-1-isaacm@codeaurora.org>
[not found] ` <20210408045241.27316-13-isaacm@codeaurora.org>
2021-04-08 13:58 ` [RFC PATCH v4 12/15] iommu/io-pgtable-arm-v7s: Implement arm_v7s_unmap_pages() Will Deacon
[not found] ` <342d453c1e5d737122b43be006de6077@codeaurora.org>
2021-04-08 14:32 ` Will Deacon
[not found] ` <20210408045241.27316-7-isaacm@codeaurora.org>
2021-04-08 13:59 ` [RFC PATCH v4 06/15] iommu: Split 'addr_merge' argument to iommu_pgsize() into separate parts Will Deacon
[not found] ` <20210408045241.27316-10-isaacm@codeaurora.org>
2021-04-08 13:59 ` [RFC PATCH v4 09/15] iommu/io-pgtable-arm: Prepare PTE methods for handling multiple entries Will Deacon
2021-04-08 14:02 ` Christoph Hellwig
2021-04-08 14:20 ` Will Deacon
[not found] ` <20210408045241.27316-11-isaacm@codeaurora.org>
2021-04-08 14:32 ` [RFC PATCH v4 10/15] iommu/io-pgtable-arm: Implement arm_lpae_unmap_pages() Will Deacon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).