Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

2022-07-02 Thread Sam Protsenko
On Wed, 22 Jun 2022 at 12:57, Marek Szyprowski  wrote:
>
>
> On 22.06.2022 11:14, Robin Murphy wrote:
> > On 2022-06-21 20:57, Sam Protsenko wrote:
> >> Hi Marek,
> >>
> >> On Fri, 21 Jan 2022 at 14:31, Marek Szyprowski
> >>  wrote:
> >>
> >> [snip]
> >>
> >>>
> >>> Well, for starting point the existing exynos-iommu driver is really
> >>> enough. I've played a bit with newer Exyos SoCs some time ago. If I
> >>> remember right, if you limit the iommu functionality to the essential
> >>> things like mapping pages to IO-virtual space, the hardware differences
> >>> between SYSMMU v5 (already supported by the exynos-iommu driver) and v7
> >>> are just a matter of changing a one register during the initialization
> >>> and different bits the page fault reason decoding. You must of course
> >>> rely on the DMA-mapping framework and its implementation based on
> >>> mainline DMA-IOMMU helpers. All the code for custom iommu group(s)
> >>> handling or extended fault management are not needed for the initial
> >>> version.
> >>>
> >>
> >> Thanks for the advice! Just implemented some testing driver, which
> >> uses "Emulated Translation" registers available on SysMMU v7. That's
> >> one way to verify the IOMMU driver with no actual users of it. It
> >> works fine with vendor SysMMU driver I ported to mainline earlier, and
> >> now I'm trying to use it with exynos-sysmmu driver (existing
> >> upstream). If you're curious -- I can share the testing driver
> >> somewhere on GitHub.
> >>
> >> I believe the register you mentioned is PT_BASE one, so I used
> >> REG_V7_FLPT_BASE_VM = 0x800C instead of REG_V5_PT_BASE_PFN. But I
> >> didn't manage to get that far, unfortunately, as
> >> exynos_iommu_domain_alloc() function fails in my case, with BUG_ON()
> >> at this line:
> >>
> >>  /* For mapping page table entries we rely on dma == phys */
> >>  BUG_ON(handle != virt_to_phys(domain->pgtable));
> >>
> >> One possible explanation for this BUG is that "dma-ranges" property is
> >> not provided in DTS (which seems to be the case right now for all
> >> users of "samsung,exynos-sysmmu" driver). Because of that the SWIOTLB
> >> is used for dma_map_single() call (in exynos_iommu_domain_alloc()
> >> function), which in turn leads to that BUG. At least that's what
> >> happens in my case. The call chain looks like this:
> >>
> >>  exynos_iommu_domain_alloc()
> >>  v
> >>  dma_map_single()
> >>  v
> >>  dma_map_single_attrs()
> >>  v
> >>  dma_map_page_attrs()
> >>  v
> >>  dma_direct_map_page()  // dma_capable() == false
> >>  v
> >>  swiotlb_map()
> >>  v
> >>  swiotlb_tbl_map_single()
> >>
> >> And the last call of course always returns the address different than
> >> the address for allocated pgtable. E.g. in my case I see this:
> >>
> >>  handle = 0xfbfff000
> >>  virt_to_phys(domain->pgtable) = 0x000880d0c000
> >>
> >> Do you know what might be the reason for that? I just wonder how the
> >> SysMMU driver work for all existing Exynos platforms right now. I feel
> >> I might be missing something, like some DMA option should be enabled
> >> so that SWIOTLB is not used, or something like that. Please let me
> >> know if you have any idea on possible cause. The vendor's SysMMU
> >> driver is kinda different in that regard, as it doesn't use
> >> dma_map_single(), so I don't see such issue there.
> >
> > If this SysMMU version is capable of addressing more than 32 bits,
> > then exynos_iommu_probe_device() should set its DMA masks appropriately.
>
> Well, Sam's question was about the Exynos SYSMMU own platform device,
> not the device for which Exynos SYSMMU manages the IO virtual address
> space.
>
> Simply add something like
>
> dma_set_mask(dev, DMA_BIT_MASK(36));
>

Yep, that one worked, thanks! Just submitted a patch, with a bit of
additions: [1].

[1] https://lkml.org/lkml/2022/7/2/269

> to the beginning of the exynos_sysmmu_probe(). This will disable SWIOTLB
> and switch to DMA-direct for the Exynos SYSMMU platform device.
>
>
> > (as a side note since I looked, the use of PAGE_SIZE/PAGE_SHIFT in the
> > driver looks wrong, since I can't imagine that the hardware knows
> > whether Linux is using 4KB, 16KB or 64KB and adjusts itself
> > accordingly...)
>
> Right, this has to be cleaned up. This driver was never used on systems
> with kernel configuration for non-4KB pages.
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R Institute Poland
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

2022-06-22 Thread Marek Szyprowski

On 22.06.2022 11:14, Robin Murphy wrote:
> On 2022-06-21 20:57, Sam Protsenko wrote:
>> Hi Marek,
>>
>> On Fri, 21 Jan 2022 at 14:31, Marek Szyprowski 
>>  wrote:
>>
>> [snip]
>>
>>>
>>> Well, for starting point the existing exynos-iommu driver is really
>>> enough. I've played a bit with newer Exyos SoCs some time ago. If I
>>> remember right, if you limit the iommu functionality to the essential
>>> things like mapping pages to IO-virtual space, the hardware differences
>>> between SYSMMU v5 (already supported by the exynos-iommu driver) and v7
>>> are just a matter of changing a one register during the initialization
>>> and different bits the page fault reason decoding. You must of course
>>> rely on the DMA-mapping framework and its implementation based on
>>> mainline DMA-IOMMU helpers. All the code for custom iommu group(s)
>>> handling or extended fault management are not needed for the initial
>>> version.
>>>
>>
>> Thanks for the advice! Just implemented some testing driver, which
>> uses "Emulated Translation" registers available on SysMMU v7. That's
>> one way to verify the IOMMU driver with no actual users of it. It
>> works fine with vendor SysMMU driver I ported to mainline earlier, and
>> now I'm trying to use it with exynos-sysmmu driver (existing
>> upstream). If you're curious -- I can share the testing driver
>> somewhere on GitHub.
>>
>> I believe the register you mentioned is PT_BASE one, so I used
>> REG_V7_FLPT_BASE_VM = 0x800C instead of REG_V5_PT_BASE_PFN. But I
>> didn't manage to get that far, unfortunately, as
>> exynos_iommu_domain_alloc() function fails in my case, with BUG_ON()
>> at this line:
>>
>>  /* For mapping page table entries we rely on dma == phys */
>>  BUG_ON(handle != virt_to_phys(domain->pgtable));
>>
>> One possible explanation for this BUG is that "dma-ranges" property is
>> not provided in DTS (which seems to be the case right now for all
>> users of "samsung,exynos-sysmmu" driver). Because of that the SWIOTLB
>> is used for dma_map_single() call (in exynos_iommu_domain_alloc()
>> function), which in turn leads to that BUG. At least that's what
>> happens in my case. The call chain looks like this:
>>
>>  exynos_iommu_domain_alloc()
>>  v
>>  dma_map_single()
>>  v
>>  dma_map_single_attrs()
>>  v
>>  dma_map_page_attrs()
>>  v
>>  dma_direct_map_page()  // dma_capable() == false
>>  v
>>  swiotlb_map()
>>  v
>>  swiotlb_tbl_map_single()
>>
>> And the last call of course always returns the address different than
>> the address for allocated pgtable. E.g. in my case I see this:
>>
>>  handle = 0xfbfff000
>>  virt_to_phys(domain->pgtable) = 0x000880d0c000
>>
>> Do you know what might be the reason for that? I just wonder how the
>> SysMMU driver work for all existing Exynos platforms right now. I feel
>> I might be missing something, like some DMA option should be enabled
>> so that SWIOTLB is not used, or something like that. Please let me
>> know if you have any idea on possible cause. The vendor's SysMMU
>> driver is kinda different in that regard, as it doesn't use
>> dma_map_single(), so I don't see such issue there.
>
> If this SysMMU version is capable of addressing more than 32 bits, 
> then exynos_iommu_probe_device() should set its DMA masks appropriately.

Well, Sam's question was about the Exynos SYSMMU own platform device, 
not the device for which Exynos SYSMMU manages the IO virtual address 
space.

Simply add something like

dma_set_mask(dev, DMA_BIT_MASK(36));

to the beginning of the exynos_sysmmu_probe(). This will disable SWIOTLB 
and switch to DMA-direct for the Exynos SYSMMU platform device.


> (as a side note since I looked, the use of PAGE_SIZE/PAGE_SHIFT in the 
> driver looks wrong, since I can't imagine that the hardware knows 
> whether Linux is using 4KB, 16KB or 64KB and adjusts itself 
> accordingly...)

Right, this has to be cleaned up. This driver was never used on systems 
with kernel configuration for non-4KB pages.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

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

Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

2022-06-22 Thread Robin Murphy

On 2022-06-21 20:57, Sam Protsenko wrote:

Hi Marek,

On Fri, 21 Jan 2022 at 14:31, Marek Szyprowski  wrote:

[snip]



Well, for starting point the existing exynos-iommu driver is really
enough. I've played a bit with newer Exyos SoCs some time ago. If I
remember right, if you limit the iommu functionality to the essential
things like mapping pages to IO-virtual space, the hardware differences
between SYSMMU v5 (already supported by the exynos-iommu driver) and v7
are just a matter of changing a one register during the initialization
and different bits the page fault reason decoding. You must of course
rely on the DMA-mapping framework and its implementation based on
mainline DMA-IOMMU helpers. All the code for custom iommu group(s)
handling or extended fault management are not needed for the initial
version.



Thanks for the advice! Just implemented some testing driver, which
uses "Emulated Translation" registers available on SysMMU v7. That's
one way to verify the IOMMU driver with no actual users of it. It
works fine with vendor SysMMU driver I ported to mainline earlier, and
now I'm trying to use it with exynos-sysmmu driver (existing
upstream). If you're curious -- I can share the testing driver
somewhere on GitHub.

I believe the register you mentioned is PT_BASE one, so I used
REG_V7_FLPT_BASE_VM = 0x800C instead of REG_V5_PT_BASE_PFN. But I
didn't manage to get that far, unfortunately, as
exynos_iommu_domain_alloc() function fails in my case, with BUG_ON()
at this line:

 /* For mapping page table entries we rely on dma == phys */
 BUG_ON(handle != virt_to_phys(domain->pgtable));

One possible explanation for this BUG is that "dma-ranges" property is
not provided in DTS (which seems to be the case right now for all
users of "samsung,exynos-sysmmu" driver). Because of that the SWIOTLB
is used for dma_map_single() call (in exynos_iommu_domain_alloc()
function), which in turn leads to that BUG. At least that's what
happens in my case. The call chain looks like this:

 exynos_iommu_domain_alloc()
 v
 dma_map_single()
 v
 dma_map_single_attrs()
 v
 dma_map_page_attrs()
 v
 dma_direct_map_page()  // dma_capable() == false
 v
 swiotlb_map()
 v
 swiotlb_tbl_map_single()

And the last call of course always returns the address different than
the address for allocated pgtable. E.g. in my case I see this:

 handle = 0xfbfff000
 virt_to_phys(domain->pgtable) = 0x000880d0c000

Do you know what might be the reason for that? I just wonder how the
SysMMU driver work for all existing Exynos platforms right now. I feel
I might be missing something, like some DMA option should be enabled
so that SWIOTLB is not used, or something like that. Please let me
know if you have any idea on possible cause. The vendor's SysMMU
driver is kinda different in that regard, as it doesn't use
dma_map_single(), so I don't see such issue there.


If this SysMMU version is capable of addressing more than 32 bits, then 
exynos_iommu_probe_device() should set its DMA masks appropriately.


(as a side note since I looked, the use of PAGE_SIZE/PAGE_SHIFT in the 
driver looks wrong, since I can't imagine that the hardware knows 
whether Linux is using 4KB, 16KB or 64KB and adjusts itself accordingly...)


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


Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

2022-06-21 Thread Sam Protsenko
Hi Marek,

On Fri, 21 Jan 2022 at 14:31, Marek Szyprowski  wrote:

[snip]

>
> Well, for starting point the existing exynos-iommu driver is really
> enough. I've played a bit with newer Exyos SoCs some time ago. If I
> remember right, if you limit the iommu functionality to the essential
> things like mapping pages to IO-virtual space, the hardware differences
> between SYSMMU v5 (already supported by the exynos-iommu driver) and v7
> are just a matter of changing a one register during the initialization
> and different bits the page fault reason decoding. You must of course
> rely on the DMA-mapping framework and its implementation based on
> mainline DMA-IOMMU helpers. All the code for custom iommu group(s)
> handling or extended fault management are not needed for the initial
> version.
>

Thanks for the advice! Just implemented some testing driver, which
uses "Emulated Translation" registers available on SysMMU v7. That's
one way to verify the IOMMU driver with no actual users of it. It
works fine with vendor SysMMU driver I ported to mainline earlier, and
now I'm trying to use it with exynos-sysmmu driver (existing
upstream). If you're curious -- I can share the testing driver
somewhere on GitHub.

I believe the register you mentioned is PT_BASE one, so I used
REG_V7_FLPT_BASE_VM = 0x800C instead of REG_V5_PT_BASE_PFN. But I
didn't manage to get that far, unfortunately, as
exynos_iommu_domain_alloc() function fails in my case, with BUG_ON()
at this line:

/* For mapping page table entries we rely on dma == phys */
BUG_ON(handle != virt_to_phys(domain->pgtable));

One possible explanation for this BUG is that "dma-ranges" property is
not provided in DTS (which seems to be the case right now for all
users of "samsung,exynos-sysmmu" driver). Because of that the SWIOTLB
is used for dma_map_single() call (in exynos_iommu_domain_alloc()
function), which in turn leads to that BUG. At least that's what
happens in my case. The call chain looks like this:

exynos_iommu_domain_alloc()
v
dma_map_single()
v
dma_map_single_attrs()
v
dma_map_page_attrs()
v
dma_direct_map_page()  // dma_capable() == false
v
swiotlb_map()
v
swiotlb_tbl_map_single()

And the last call of course always returns the address different than
the address for allocated pgtable. E.g. in my case I see this:

handle = 0xfbfff000
virt_to_phys(domain->pgtable) = 0x000880d0c000

Do you know what might be the reason for that? I just wonder how the
SysMMU driver work for all existing Exynos platforms right now. I feel
I might be missing something, like some DMA option should be enabled
so that SWIOTLB is not used, or something like that. Please let me
know if you have any idea on possible cause. The vendor's SysMMU
driver is kinda different in that regard, as it doesn't use
dma_map_single(), so I don't see such issue there.

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


Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

2022-01-21 Thread Marek Szyprowski
Hi Sam,

On 21.01.2022 12:08, Sam Protsenko wrote:
> On Fri, 21 Jan 2022 at 10:40, Krzysztof Kozlowski
>  wrote:
>> On 20/01/2022 21:19, Sam Protsenko wrote:
>>> Introduce new driver for modern Exynos ARMv8 SoCs, e.g. Exynos850. Also
>>> it's used for Google's GS101 SoC.
>>>
>>> This is squashed commit, contains next patches of different authors. See
>>> `iommu-exynos850-dev' branch for details: [1].
>>>
>>> Original authors (Samsung):
>>>
>>>   - Cho KyongHo 
>>>   - Hyesoo Yu 
>>>   - Janghyuck Kim 
>>>   - Jinkyu Yang 
>>>
>>> Some improvements were made by Google engineers:
>>>
>>>   - Alex 
>>>   - Carlos Llamas 
>>>   - Daniel Mentz 
>>>   - Erick Reyes 
>>>   - J. Avila 
>>>   - Jonglin Lee 
>>>   - Mark Salyzyn 
>>>   - Thierry Strudel 
>>>   - Will McVicker 
>>>
>>> [1] 
>>> https://protect2.fireeye.com/v1/url?k=19bd3571-46260c3c-19bcbe3e-0cc47aa8f5ba-8a160a7fd38bb35a=1=eb3f71b3-8df2-46c6-b6d8-0a931ef99024=https%3A%2F%2Fgithub.com%2Fjoe-skb7%2Flinux%2Ftree%2Fiommu-exynos850-dev
>>>
>>> Signed-off-by: Sam Protsenko 
>>> ---
>>>   drivers/iommu/Kconfig   |   13 +
>>>   drivers/iommu/Makefile  |3 +
>>>   drivers/iommu/samsung-iommu-fault.c |  617 +++
>>>   drivers/iommu/samsung-iommu-group.c |   50 +
>>>   drivers/iommu/samsung-iommu.c   | 1521 +++
>>>   drivers/iommu/samsung-iommu.h   |  216 
>>>   6 files changed, 2420 insertions(+)
>>>   create mode 100644 drivers/iommu/samsung-iommu-fault.c
>>>   create mode 100644 drivers/iommu/samsung-iommu-group.c
>>>   create mode 100644 drivers/iommu/samsung-iommu.c
>>>   create mode 100644 drivers/iommu/samsung-iommu.h
>>>
>> Existing driver supports several different Exynos SysMMU IP block
>> versions. Several. Please explain why it cannot support one more version?
>>
>> Similarity of vendor driver with mainline is not an argument.
>>
>>> ...
>> You just copy-pasted vendor stuff, without actually going through it.
>>
>> It is very disappointing because instead of putting your own effort, you
>> expect community to do your job.
>>
>> What the hell is CONFIG_EXYNOS_CONTENT_PATH_PROTECTION?
>>
>> I'll stop reviewing. Please work on extending existing driver. If you
>> submitted something nice and clean, ready for upstream, instead of
>> vendor junk, you could get away with separate driver. But you did not.
>> It looks really bad.
>>
> Krzysztof, that's not what I asked in my patch 0/3. I probably wasn't
> really clear, sorry. Let me please try and describe that better, and
> maybe provide some context.
>
> I'm just starting to work on that driver, it's basically downstream
> version of it. Of course I'm going to rework it before sending the
> actual patch series (that's why this series has RFC tag). I'd never
> asked community to do my job for me and really review the downstream
> driver! I just want to know from the starters some *very* basic and
> high-level info, which could help me to rework the driver in correct
> way. Like naming of files, compatible strings, should it be part of
> existing driver or it's ok to have it as another platform_driver. In
> other words, that kind of "review" shouldn't take more than 2 minutes
> of your time. And it could spare us all unneeded extra review rounds
> in future. Right now I don't need the code review per se (and I'm
> really sorry you had to spend your time on that, knowing how busy
> maintainers can be during the MW). I thought about omitting the code
> at all, only asking the questions, but then I figured it's a good idea
> to attach some code for the reference. Maybe it wasn't a good idea
> after all.
>
> For the record, I'm well aware that we don't send downstream code
> without making it upstreamable first, and I know it must be tested
> well, etc. For example, you already saw me sending clk-exynos850
> driver, which I re-implemented from scratch, and it has ~0.0% of
> downstream code. So why would I change my policy about that all of the
> sudden... Anyway, hope you understand now that there weren't any ill
> intentions on my side, w.r.t. this RFC.


Well, for starting point the existing exynos-iommu driver is really 
enough. I've played a bit with newer Exyos SoCs some time ago. If I 
remember right, if you limit the iommu functionality to the essential 
things like mapping pages to IO-virtual space, the hardware differences 
between SYSMMU v5 (already supported by the exynos-iommu driver) and v7 
are just a matter of changing a one register during the initialization 
and different bits the page fault reason decoding. You must of course 
rely on the DMA-mapping framework and its implementation based on 
mainline DMA-IOMMU helpers. All the code for custom iommu group(s) 
handling or extended fault management are not needed for the initial 
version.

The IOMMU driver on its own doesn't really make much sense, so you need 
the other driver/device pair which will make use of it. You have 
mentioned DPU, so you are trying to bring the 

Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

2022-01-21 Thread Sam Protsenko
On Fri, 21 Jan 2022 at 10:40, Krzysztof Kozlowski
 wrote:
>
> On 20/01/2022 21:19, Sam Protsenko wrote:
> > Introduce new driver for modern Exynos ARMv8 SoCs, e.g. Exynos850. Also
> > it's used for Google's GS101 SoC.
> >
> > This is squashed commit, contains next patches of different authors. See
> > `iommu-exynos850-dev' branch for details: [1].
> >
> > Original authors (Samsung):
> >
> >  - Cho KyongHo 
> >  - Hyesoo Yu 
> >  - Janghyuck Kim 
> >  - Jinkyu Yang 
> >
> > Some improvements were made by Google engineers:
> >
> >  - Alex 
> >  - Carlos Llamas 
> >  - Daniel Mentz 
> >  - Erick Reyes 
> >  - J. Avila 
> >  - Jonglin Lee 
> >  - Mark Salyzyn 
> >  - Thierry Strudel 
> >  - Will McVicker 
> >
> > [1] https://github.com/joe-skb7/linux/tree/iommu-exynos850-dev
> >
> > Signed-off-by: Sam Protsenko 
> > ---
> >  drivers/iommu/Kconfig   |   13 +
> >  drivers/iommu/Makefile  |3 +
> >  drivers/iommu/samsung-iommu-fault.c |  617 +++
> >  drivers/iommu/samsung-iommu-group.c |   50 +
> >  drivers/iommu/samsung-iommu.c   | 1521 +++
> >  drivers/iommu/samsung-iommu.h   |  216 
> >  6 files changed, 2420 insertions(+)
> >  create mode 100644 drivers/iommu/samsung-iommu-fault.c
> >  create mode 100644 drivers/iommu/samsung-iommu-group.c
> >  create mode 100644 drivers/iommu/samsung-iommu.c
> >  create mode 100644 drivers/iommu/samsung-iommu.h
> >
>
> Existing driver supports several different Exynos SysMMU IP block
> versions. Several. Please explain why it cannot support one more version?
>
> Similarity of vendor driver with mainline is not an argument.
>
>
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 3eb68fa1b8cc..78e7039f18aa 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -452,6 +452,19 @@ config QCOM_IOMMU
> >   help
> > Support for IOMMU on certain Qualcomm SoCs.
> >
> > +config SAMSUNG_IOMMU
> > + tristate "Samsung IOMMU Support"
> > + select ARM_DMA_USE_IOMMU
> > + select IOMMU_DMA
> > + select SAMSUNG_IOMMU_GROUP
> > + help
> > +   Support for IOMMU on Samsung Exynos SoCs.
> > +
> > +config SAMSUNG_IOMMU_GROUP
> > + tristate "Samsung IOMMU Group Support"
> > + help
> > +   Support for IOMMU group on Samsung Exynos SoCs.
> > +
> >  config HYPERV_IOMMU
> >   bool "Hyper-V x2APIC IRQ Handling"
> >   depends on HYPERV && X86
> > diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> > index bc7f730edbb0..a8bdf449f1d4 100644
> > --- a/drivers/iommu/Makefile
> > +++ b/drivers/iommu/Makefile
> > @@ -27,6 +27,9 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
> >  obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
> >  obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
> >  obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> > +obj-$(CONFIG_SAMSUNG_IOMMU) += samsung_iommu.o
> > +samsung_iommu-objs += samsung-iommu.o samsung-iommu-fault.o
> > +obj-$(CONFIG_SAMSUNG_IOMMU_GROUP) += samsung-iommu-group.o
> >  obj-$(CONFIG_IOMMU_SVA_LIB) += iommu-sva-lib.o io-pgfault.o
> >  obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
> >  obj-$(CONFIG_APPLE_DART) += apple-dart.o
> > diff --git a/drivers/iommu/samsung-iommu-fault.c 
> > b/drivers/iommu/samsung-iommu-fault.c
> > new file mode 100644
> > index ..c6b4259976c4
> > --- /dev/null
> > +++ b/drivers/iommu/samsung-iommu-fault.c
> > @@ -0,0 +1,617 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2020 Samsung Electronics Co., Ltd.
> > + */
> > +
> > +#define pr_fmt(fmt) "sysmmu: " fmt
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "samsung-iommu.h"
> > +
> > +#define MMU_TLB_INFO(n)  (0x2000 + ((n) * 0x20))
> > +#define MMU_CAPA1_NUM_TLB_SET(reg)   (((reg) >> 16) & 0xFF)
> > +#define MMU_CAPA1_NUM_TLB_WAY(reg)   ((reg) & 0xFF)
> > +#define MMU_CAPA1_SET_TLB_READ_ENTRY(tid, set, way, line)\
> > + ((set) | ((way) << 8) | \
> > +  ((line) << 16) | ((tid) << 20))
> > +
> > +#define MMU_TLB_ENTRY_VALID(reg) ((reg) >> 28)
> > +#define MMU_SBB_ENTRY_VALID(reg) ((reg) >> 28)
> > +#define MMU_VADDR_FROM_TLB(reg, idx) reg) & 0xC) | ((idx) & 0x3)) 
> > << 12)
> > +#define MMU_VID_FROM_TLB(reg)(((reg) >> 20) & 0x7U)
> > +#define MMU_PADDR_FROM_TLB(reg)  ((phys_addr_t)((reg) & 
> > 0xFF) << 12)
> > +#define MMU_VADDR_FROM_SBB(reg)  (((reg) & 0xF) << 12)
> > +#define MMU_VID_FROM_SBB(reg)(((reg) >> 20) & 0x7U)
> > +#define MMU_PADDR_FROM_SBB(reg)  ((phys_addr_t)((reg) & 
> > 0x3FF) << 10)
> > +
> > +#define REG_MMU_INT_STATUS   0x060
> > +#define REG_MMU_INT_CLEAR0x064
> > +#define REG_MMU_FAULT_RW_MASKGENMASK(20, 20)
> > +#define IS_READ_FAULT(x) (((x) & REG_MMU_FAULT_RW_MASK) == 0)
> > +
> > 

Re: [RFC 2/3] iommu/samsung: Introduce Exynos sysmmu-v8 driver

2022-01-21 Thread Krzysztof Kozlowski
On 20/01/2022 21:19, Sam Protsenko wrote:
> Introduce new driver for modern Exynos ARMv8 SoCs, e.g. Exynos850. Also
> it's used for Google's GS101 SoC.
> 
> This is squashed commit, contains next patches of different authors. See
> `iommu-exynos850-dev' branch for details: [1].
> 
> Original authors (Samsung):
> 
>  - Cho KyongHo 
>  - Hyesoo Yu 
>  - Janghyuck Kim 
>  - Jinkyu Yang 
> 
> Some improvements were made by Google engineers:
> 
>  - Alex 
>  - Carlos Llamas 
>  - Daniel Mentz 
>  - Erick Reyes 
>  - J. Avila 
>  - Jonglin Lee 
>  - Mark Salyzyn 
>  - Thierry Strudel 
>  - Will McVicker 
> 
> [1] https://github.com/joe-skb7/linux/tree/iommu-exynos850-dev
> 
> Signed-off-by: Sam Protsenko 
> ---
>  drivers/iommu/Kconfig   |   13 +
>  drivers/iommu/Makefile  |3 +
>  drivers/iommu/samsung-iommu-fault.c |  617 +++
>  drivers/iommu/samsung-iommu-group.c |   50 +
>  drivers/iommu/samsung-iommu.c   | 1521 +++
>  drivers/iommu/samsung-iommu.h   |  216 
>  6 files changed, 2420 insertions(+)
>  create mode 100644 drivers/iommu/samsung-iommu-fault.c
>  create mode 100644 drivers/iommu/samsung-iommu-group.c
>  create mode 100644 drivers/iommu/samsung-iommu.c
>  create mode 100644 drivers/iommu/samsung-iommu.h
> 

Existing driver supports several different Exynos SysMMU IP block
versions. Several. Please explain why it cannot support one more version?

Similarity of vendor driver with mainline is not an argument.


> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 3eb68fa1b8cc..78e7039f18aa 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -452,6 +452,19 @@ config QCOM_IOMMU
>   help
> Support for IOMMU on certain Qualcomm SoCs.
>  
> +config SAMSUNG_IOMMU
> + tristate "Samsung IOMMU Support"
> + select ARM_DMA_USE_IOMMU
> + select IOMMU_DMA
> + select SAMSUNG_IOMMU_GROUP
> + help
> +   Support for IOMMU on Samsung Exynos SoCs.
> +
> +config SAMSUNG_IOMMU_GROUP
> + tristate "Samsung IOMMU Group Support"
> + help
> +   Support for IOMMU group on Samsung Exynos SoCs.
> +
>  config HYPERV_IOMMU
>   bool "Hyper-V x2APIC IRQ Handling"
>   depends on HYPERV && X86
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index bc7f730edbb0..a8bdf449f1d4 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -27,6 +27,9 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
>  obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
>  obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
>  obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> +obj-$(CONFIG_SAMSUNG_IOMMU) += samsung_iommu.o
> +samsung_iommu-objs += samsung-iommu.o samsung-iommu-fault.o
> +obj-$(CONFIG_SAMSUNG_IOMMU_GROUP) += samsung-iommu-group.o
>  obj-$(CONFIG_IOMMU_SVA_LIB) += iommu-sva-lib.o io-pgfault.o
>  obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
>  obj-$(CONFIG_APPLE_DART) += apple-dart.o
> diff --git a/drivers/iommu/samsung-iommu-fault.c 
> b/drivers/iommu/samsung-iommu-fault.c
> new file mode 100644
> index ..c6b4259976c4
> --- /dev/null
> +++ b/drivers/iommu/samsung-iommu-fault.c
> @@ -0,0 +1,617 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020 Samsung Electronics Co., Ltd.
> + */
> +
> +#define pr_fmt(fmt) "sysmmu: " fmt
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "samsung-iommu.h"
> +
> +#define MMU_TLB_INFO(n)  (0x2000 + ((n) * 0x20))
> +#define MMU_CAPA1_NUM_TLB_SET(reg)   (((reg) >> 16) & 0xFF)
> +#define MMU_CAPA1_NUM_TLB_WAY(reg)   ((reg) & 0xFF)
> +#define MMU_CAPA1_SET_TLB_READ_ENTRY(tid, set, way, line)\
> + ((set) | ((way) << 8) | \
> +  ((line) << 16) | ((tid) << 20))
> +
> +#define MMU_TLB_ENTRY_VALID(reg) ((reg) >> 28)
> +#define MMU_SBB_ENTRY_VALID(reg) ((reg) >> 28)
> +#define MMU_VADDR_FROM_TLB(reg, idx) reg) & 0xC) | ((idx) & 0x3)) << 
> 12)
> +#define MMU_VID_FROM_TLB(reg)(((reg) >> 20) & 0x7U)
> +#define MMU_PADDR_FROM_TLB(reg)  ((phys_addr_t)((reg) & 
> 0xFF) << 12)
> +#define MMU_VADDR_FROM_SBB(reg)  (((reg) & 0xF) << 12)
> +#define MMU_VID_FROM_SBB(reg)(((reg) >> 20) & 0x7U)
> +#define MMU_PADDR_FROM_SBB(reg)  ((phys_addr_t)((reg) & 
> 0x3FF) << 10)
> +
> +#define REG_MMU_INT_STATUS   0x060
> +#define REG_MMU_INT_CLEAR0x064
> +#define REG_MMU_FAULT_RW_MASKGENMASK(20, 20)
> +#define IS_READ_FAULT(x) (((x) & REG_MMU_FAULT_RW_MASK) == 0)
> +
> +#define SYSMMU_FAULT_PTW_ACCESS   0
> +#define SYSMMU_FAULT_PAGE_FAULT   1
> +#define SYSMMU_FAULT_ACCESS   2
> +#define SYSMMU_FAULT_RESERVED 3
> +#define SYSMMU_FAULT_UNKNOWN  4
> +
> +#define SYSMMU_SEC_FAULT_MASK(BIT(SYSMMU_FAULT_PTW_ACCESS) | 
> \
> +