From: Joerg Roedel <joro@8bytes.org>
To: Maxime Ripard <maxime@cerno.tech>
Cc: Mark Rutland <mark.rutland@arm.com>,
devicetree@vger.kernel.org, iommu@lists.linux-foundation.org,
Chen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/4] iommu: Add Allwinner H6 IOMMU driver
Date: Wed, 8 Apr 2020 16:06:49 +0200 [thread overview]
Message-ID: <20200408140649.GI3103@8bytes.org> (raw)
In-Reply-To: <20200401114710.doioefzmjhte7jwu@gilmour.lan>
Hi Maxime,
On Wed, Apr 01, 2020 at 01:47:10PM +0200, Maxime Ripard wrote:
> As far as I understand it, the page table can be accessed concurrently
> since the framework doesn't seem to provide any serialization /
> locking, shouldn't we have some locks to prevent concurrent access?
The dma-iommu code makes sure that there are no concurrent accesses to
the same address-range of the page-table, but there can (and will) be
concurrent accesses to the same page-table, just for different parts of
the address space.
Making this lock-less usually involves updating non-leaf page-table
entries using atomic compare-exchange instructions.
> > > + *pte_addr = sun50i_mk_pte(paddr, prot);
> > > + sun50i_table_flush(sun50i_domain, pte_addr, 1);
> >
> > This maps only one page, right? But the function needs to map up to
> > 'size' as given in the parameter list.
>
> It does, but pgsize_bitmap is set to 4k only (since the hardware only
> supports that), so we would have multiple calls to map, each time with
> a single page judging from:
> https://elixir.bootlin.com/linux/latest/source/drivers/iommu/iommu.c#L1948
>
> Right?
Okay, you are right here. Just note that when this function is called
for every 4k page it should better be fast and avoid slow things like
TLB flushes.
> The vendor driver was doing something along those lines and I wanted
> to be conservative with the cache management if we didn't run into
> performances issues, but I'll convert to the iotlb callbacks then.
Yeah, that definitly helps performance.
Regards,
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Joerg Roedel <joro@8bytes.org>
To: Maxime Ripard <maxime@cerno.tech>
Cc: Mark Rutland <mark.rutland@arm.com>,
devicetree@vger.kernel.org, iommu@lists.linux-foundation.org,
Chen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/4] iommu: Add Allwinner H6 IOMMU driver
Date: Wed, 8 Apr 2020 16:06:49 +0200 [thread overview]
Message-ID: <20200408140649.GI3103@8bytes.org> (raw)
In-Reply-To: <20200401114710.doioefzmjhte7jwu@gilmour.lan>
Hi Maxime,
On Wed, Apr 01, 2020 at 01:47:10PM +0200, Maxime Ripard wrote:
> As far as I understand it, the page table can be accessed concurrently
> since the framework doesn't seem to provide any serialization /
> locking, shouldn't we have some locks to prevent concurrent access?
The dma-iommu code makes sure that there are no concurrent accesses to
the same address-range of the page-table, but there can (and will) be
concurrent accesses to the same page-table, just for different parts of
the address space.
Making this lock-less usually involves updating non-leaf page-table
entries using atomic compare-exchange instructions.
> > > + *pte_addr = sun50i_mk_pte(paddr, prot);
> > > + sun50i_table_flush(sun50i_domain, pte_addr, 1);
> >
> > This maps only one page, right? But the function needs to map up to
> > 'size' as given in the parameter list.
>
> It does, but pgsize_bitmap is set to 4k only (since the hardware only
> supports that), so we would have multiple calls to map, each time with
> a single page judging from:
> https://elixir.bootlin.com/linux/latest/source/drivers/iommu/iommu.c#L1948
>
> Right?
Okay, you are right here. Just note that when this function is called
for every 4k page it should better be fast and avoid slow things like
TLB flushes.
> The vendor driver was doing something along those lines and I wanted
> to be conservative with the cache management if we didn't run into
> performances issues, but I'll convert to the iotlb callbacks then.
Yeah, that definitly helps performance.
Regards,
Joerg
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Joerg Roedel <joro@8bytes.org>
To: Maxime Ripard <maxime@cerno.tech>
Cc: Chen-Yu Tsai <wens@csie.org>, Mark Rutland <mark.rutland@arm.com>,
Rob Herring <robh+dt@kernel.org>,
Frank Rowand <frowand.list@gmail.com>,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
iommu@lists.linux-foundation.org
Subject: Re: [PATCH v2 2/4] iommu: Add Allwinner H6 IOMMU driver
Date: Wed, 8 Apr 2020 16:06:49 +0200 [thread overview]
Message-ID: <20200408140649.GI3103@8bytes.org> (raw)
In-Reply-To: <20200401114710.doioefzmjhte7jwu@gilmour.lan>
Hi Maxime,
On Wed, Apr 01, 2020 at 01:47:10PM +0200, Maxime Ripard wrote:
> As far as I understand it, the page table can be accessed concurrently
> since the framework doesn't seem to provide any serialization /
> locking, shouldn't we have some locks to prevent concurrent access?
The dma-iommu code makes sure that there are no concurrent accesses to
the same address-range of the page-table, but there can (and will) be
concurrent accesses to the same page-table, just for different parts of
the address space.
Making this lock-less usually involves updating non-leaf page-table
entries using atomic compare-exchange instructions.
> > > + *pte_addr = sun50i_mk_pte(paddr, prot);
> > > + sun50i_table_flush(sun50i_domain, pte_addr, 1);
> >
> > This maps only one page, right? But the function needs to map up to
> > 'size' as given in the parameter list.
>
> It does, but pgsize_bitmap is set to 4k only (since the hardware only
> supports that), so we would have multiple calls to map, each time with
> a single page judging from:
> https://elixir.bootlin.com/linux/latest/source/drivers/iommu/iommu.c#L1948
>
> Right?
Okay, you are right here. Just note that when this function is called
for every 4k page it should better be fast and avoid slow things like
TLB flushes.
> The vendor driver was doing something along those lines and I wanted
> to be conservative with the cache management if we didn't run into
> performances issues, but I'll convert to the iotlb callbacks then.
Yeah, that definitly helps performance.
Regards,
Joerg
next prev parent reply other threads:[~2020-04-08 14:07 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-20 18:15 [PATCH v2 0/4] iommu: Add Allwinner H6 IOMMU driver Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` [PATCH v2 1/4] dt-bindings: iommu: Add Allwinner H6 IOMMU bindings Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` [PATCH v2 2/4] iommu: Add Allwinner H6 IOMMU driver Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-03-02 15:36 ` Joerg Roedel
2020-03-02 15:36 ` Joerg Roedel
2020-03-02 15:36 ` Joerg Roedel
2020-04-01 11:47 ` Maxime Ripard
2020-04-01 11:47 ` Maxime Ripard
2020-04-01 11:47 ` Maxime Ripard
2020-04-08 14:06 ` Joerg Roedel [this message]
2020-04-08 14:06 ` Joerg Roedel
2020-04-08 14:06 ` Joerg Roedel
2020-04-20 14:39 ` Maxime Ripard
2020-04-20 14:39 ` Maxime Ripard
2020-04-20 14:39 ` Maxime Ripard
2020-04-20 16:16 ` Robin Murphy
2020-04-20 16:16 ` Robin Murphy
2020-04-20 16:16 ` Robin Murphy
2020-02-20 18:15 ` [PATCH v2 3/4] arm64: dts: allwinner: h6: Add IOMMU Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` [PATCH v2 4/4] drm/sun4i: mixer: Call of_dma_configure if there's an IOMMU Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
2020-02-20 18:15 ` Maxime Ripard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200408140649.GI3103@8bytes.org \
--to=joro@8bytes.org \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=iommu@lists.linux-foundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=maxime@cerno.tech \
--cc=robh+dt@kernel.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.