Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
> I'll let my current version simmer for a bit and wait until it's been > tested by a few people and will send it in a week or so then! New version has my T-b :) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
On Mon, Jul 19, 2021, at 20:15, Robin Murphy wrote:
> On 2021-07-15 17:41, Sven Peter via iommu wrote:
> [...]
> >>> + u64 sw_bypass_cpu_start;
> >>> + u64 sw_bypass_dma_start;
> >>> + u64 sw_bypass_len;
> >>> +
> >>> + struct list_head streams;
> >>
> >> I'm staring to think this could just be a bitmap, in a u16 even.
> >
> > The problem is that these streams may come from two different
> > DART instances. That is required for e.g. the dwc3 controller which
> > has a weird quirk where DMA transactions go through two separate
> > DARTs with no clear pattern (e.g. some xhci control structures use the
> > first dart while other structures use the second one).
>
> Ah right, I do remember discussing that situation, but I think I
> misinterpreted dart_domain->dart representing "the DART instance" here
> to mean we weren't trying to accommodate that just yet.
>
> > Both of them need to point to the same pagetable.
> > In the device tree the node will have an entry like this:
> >
> > dwc3_0: usb@38228{
> > ...
> > iommus = <_0_dart_0 0>, <_0_dart_1 1>;
> > };
> >
> > There's no need for a linked list though once I do this properly with
> > groups. I can just use an array allocated when the first device is
> > attached, which just contains apple_dart* and streamid values.
> >
> >
> >>
> >>> +
> >>> + spinlock_t lock;
> >>> +
> >>> + struct iommu_domain domain;
> >>> +};
> >>> +
> >>> +/*
> >>> + * This structure is attached to devices with dev_iommu_priv_set() on
> >>> of_xlate
> >>> + * and contains a list of streams bound to this device as defined in the
> >>> + * device tree. Multiple DART instances can be attached to a single
> >>> device
> >>> + * and each stream is identified by its stream id.
> >>> + * It's usually reference by a pointer called *cfg.
> >>> + *
> >>> + * A dynamic array instead of a linked list is used here since in almost
> >>> + * all cases a device will just be attached to a single stream and
> >>> streams
> >>> + * are never removed after they have been added.
> >>> + *
> >>> + * @num_streams: number of streams attached
> >>> + * @streams: array of structs to identify attached streams and the
> >>> device link
> >>> + * to the iommu
> >>> + */
> >>> +struct apple_dart_master_cfg {
> >>> + int num_streams;
> >>> + struct {
> >>> + struct apple_dart *dart;
> >>> + u32 sid;
> >>
> >> Can't you use the fwspec for this?
> >
> >
> > I'd be happy to use the fwspec code if that's somehow possible.
> > I'm not sure how though since I need to store both the reference to the DART
> > _and_ to the stream id. As far as I can tell the fwspec code would only
> > allow
> > to store the stream ids.
> > (see also the previous comment regarding the dwc3 node which requires stream
> > ids from two separate DART instances)
>
> Hmm, yes, as above I was overlooking that, although there are still
> various ideas that come to mind; the question becomes whether they're
> actually worthwhile or just too-clever-for-their-own-good hacks. The
> exact format of fwspec->ids is not fixed (other than the ACPI IORT code
> having a common understanding with the Arm SMMU drivers) so in principle
> you could munge some sort of DART instance index or indeed anything, but
> if it remains cleaner to manage your own data internally then by all
> means keep doing that.
Yeah, I can think of some hacks as well (like storing a global id->apple_dart*
map
or stuffing the 64bit pointer into two ints) and I've tried a few of them in
the past
days but didn't like either of them.
I do like the idea to just put two (struct apple_dart *dart, u16 sidmap)
in there though which will be plenty for all current configurations.
>
> >>> + struct device_link *link;
> >>
> >> Is it necessary to use stateless links, or could you use
> >> DL_FLAG_AUTOREMOVE_SUPPLIER and not have to keep track of them manually?
> >
> > I'll just use DL_FLAG_AUTOREMOVE_SUPPLIER. No idea why I went for stateless
> > links.
> >
> >>
> > [...]
> >>> + /* restore stream identity map */
> >>> + writel(0x03020100, dart->regs + DART_STREAM_REMAP);
> >>> + writel(0x07060504, dart->regs + DART_STREAM_REMAP + 4);
> >>> + writel(0x0b0a0908, dart->regs + DART_STREAM_REMAP + 8);
> >>> + writel(0x0f0e0d0c, dart->regs + DART_STREAM_REMAP + 12);
> >>
> >> Any hint of what the magic numbers mean?
> >
> > Yes, it's just 0,1,2,3...,0xe,0xf but I can't do 8bit writes to the bus
> > and 32 bit writes then require these slightly awkward "swapped" numbers.
> > I'll add a comment since it's not obvious at first glance.
>
> Sure, I guessed that much from "identity map" - it was more a question
> of why that means 0x03020100... rather than, say, 0x0c0d0e0f... or
> 0x76543210..., and perhaps the reason for "restoring" it in the first place.
So what this feature does is to allow the DART to take an incoming DMA stream
tagged with id i and pretend that it's actually been tagged with
readb(dart->regs + 0x80 + i)
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
On 2021-07-15 17:41, Sven Peter via iommu wrote:
[...]
+ u64 sw_bypass_cpu_start;
+ u64 sw_bypass_dma_start;
+ u64 sw_bypass_len;
+
+ struct list_head streams;
I'm staring to think this could just be a bitmap, in a u16 even.
The problem is that these streams may come from two different
DART instances. That is required for e.g. the dwc3 controller which
has a weird quirk where DMA transactions go through two separate
DARTs with no clear pattern (e.g. some xhci control structures use the
first dart while other structures use the second one).
Ah right, I do remember discussing that situation, but I think I
misinterpreted dart_domain->dart representing "the DART instance" here
to mean we weren't trying to accommodate that just yet.
Both of them need to point to the same pagetable.
In the device tree the node will have an entry like this:
dwc3_0: usb@38228{
...
iommus = <_0_dart_0 0>, <_0_dart_1 1>;
};
There's no need for a linked list though once I do this properly with
groups. I can just use an array allocated when the first device is
attached, which just contains apple_dart* and streamid values.
+
+ spinlock_t lock;
+
+ struct iommu_domain domain;
+};
+
+/*
+ * This structure is attached to devices with dev_iommu_priv_set() on of_xlate
+ * and contains a list of streams bound to this device as defined in the
+ * device tree. Multiple DART instances can be attached to a single device
+ * and each stream is identified by its stream id.
+ * It's usually reference by a pointer called *cfg.
+ *
+ * A dynamic array instead of a linked list is used here since in almost
+ * all cases a device will just be attached to a single stream and streams
+ * are never removed after they have been added.
+ *
+ * @num_streams: number of streams attached
+ * @streams: array of structs to identify attached streams and the device link
+ * to the iommu
+ */
+struct apple_dart_master_cfg {
+ int num_streams;
+ struct {
+ struct apple_dart *dart;
+ u32 sid;
Can't you use the fwspec for this?
I'd be happy to use the fwspec code if that's somehow possible.
I'm not sure how though since I need to store both the reference to the DART
_and_ to the stream id. As far as I can tell the fwspec code would only allow
to store the stream ids.
(see also the previous comment regarding the dwc3 node which requires stream
ids from two separate DART instances)
Hmm, yes, as above I was overlooking that, although there are still
various ideas that come to mind; the question becomes whether they're
actually worthwhile or just too-clever-for-their-own-good hacks. The
exact format of fwspec->ids is not fixed (other than the ACPI IORT code
having a common understanding with the Arm SMMU drivers) so in principle
you could munge some sort of DART instance index or indeed anything, but
if it remains cleaner to manage your own data internally then by all
means keep doing that.
+ struct device_link *link;
Is it necessary to use stateless links, or could you use
DL_FLAG_AUTOREMOVE_SUPPLIER and not have to keep track of them manually?
I'll just use DL_FLAG_AUTOREMOVE_SUPPLIER. No idea why I went for stateless
links.
[...]
+ /* restore stream identity map */
+ writel(0x03020100, dart->regs + DART_STREAM_REMAP);
+ writel(0x07060504, dart->regs + DART_STREAM_REMAP + 4);
+ writel(0x0b0a0908, dart->regs + DART_STREAM_REMAP + 8);
+ writel(0x0f0e0d0c, dart->regs + DART_STREAM_REMAP + 12);
Any hint of what the magic numbers mean?
Yes, it's just 0,1,2,3...,0xe,0xf but I can't do 8bit writes to the bus
and 32 bit writes then require these slightly awkward "swapped" numbers.
I'll add a comment since it's not obvious at first glance.
Sure, I guessed that much from "identity map" - it was more a question
of why that means 0x03020100... rather than, say, 0x0c0d0e0f... or
0x76543210..., and perhaps the reason for "restoring" it in the first place.
[...]
+ /*
+* we can't mix and match DARTs that support bypass mode with those who
don't
+* because the iova space in fake bypass mode generally has an offset
+*/
Erm, something doesn't sound right there... IOMMU_DOMAIN_IDENTITY should
be exactly what it says, regardless of how it's implemented. If you
can't provide a true identity mapping then you're probably better off
not pretending to support them in the first place.
Some background: the PCIe DART only supports a 32bit VA space but RAM
on these machines starts at 0x8__. I have something like
dma-ranges = <0x4200 0 0 0x8 0 0 0x>;
in the pcie nodes to add that offset to dma addresses.
What I want to do here then is to setup an identity mapping with respect
to the DMA layer understanding of addresses encoded in bus_dma_region.
Now this will always just be a constant offset of 0x8__ for
all M1s but I didn't want to
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
Hi,
Awesome, thanks a lot for the detailed review!
On Wed, Jul 14, 2021, at 01:23, Robin Murphy wrote:
> ^^ Nit: the subsystem style for the subject format should be
> "iommu/dart: Add..." - similarly on patch #1, which I just realised I
> missed (sorry!)
Sure!
>
> On 2021-06-27 15:34, Sven Peter wrote:
> > Apple's new SoCs use iommus for almost all peripherals. These Device
> > Address Resolution Tables must be setup before these peripherals can
> > act as DMA masters.
> >
> > Signed-off-by: Sven Peter
> > ---
> > MAINTAINERS |1 +
> > drivers/iommu/Kconfig| 15 +
> > drivers/iommu/Makefile |1 +
> > drivers/iommu/apple-dart-iommu.c | 1058 ++
>
> I'd be inclined to drop "-iommu" from the filename, unless there's some
> other "apple-dart" functionality that might lead to a module name clash
> in future?
Sure, DART should only be an iommu.
>
> > 4 files changed, 1075 insertions(+)
> > create mode 100644 drivers/iommu/apple-dart-iommu.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 29e5541c8f21..c1ffaa56b5f9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1245,6 +1245,7 @@ M:Sven Peter
> > L:iommu@lists.linux-foundation.org
> > S:Maintained
> > F:Documentation/devicetree/bindings/iommu/apple,dart.yaml
> > +F: drivers/iommu/apple-dart-iommu.c
> >
> > APPLE SMC DRIVER
> > M:Henrik Rydberg
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 1f111b399bca..87882c628b46 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -249,6 +249,21 @@ config SPAPR_TCE_IOMMU
> > Enables bits of IOMMU API required by VFIO. The iommu_ops
> > is not implemented as it is not necessary for VFIO.
> >
> > +config IOMMU_APPLE_DART
> > + tristate "Apple DART IOMMU Support"
> > + depends on ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)
> > + select IOMMU_API
> > + select IOMMU_IO_PGTABLE
>
> This is redundant - the individual formats already select it.
Removed for the next version.
>
[...]
> > +#include
>
> Redundant duplicate
Whoops, removed for the next version as well.
>
> > +#define DART_MAX_STREAMS 16
[...]
> > +
> > +/*
> > + * This structure is used to identify a single stream attached to a domain.
> > + * It's used as a list inside that domain to be able to attach multiple
> > + * streams to a single domain. Since multiple devices can use a single
> > stream
> > + * it additionally keeps track of how many devices are represented by this
> > + * stream. Once that number reaches zero it is detached from the IOMMU
> > domain
> > + * and all translations from this stream are disabled.
>
> That sounds a lot like something you should be doing properly with groups.
The hint to look at arm-smmu for a similar flow was very helpful, thanks!
Now that I understand how these groups works I completely agree that this
needs to be reworked and done properly.
>
> > + * @dart: DART instance to which this stream belongs
> > + * @sid: stream id within the DART instance
> > + * @num_devices: count of devices attached to this stream
> > + * @stream_head: list head for the next stream
> > + */
> > +struct apple_dart_stream {
> > + struct apple_dart *dart;
> > + u32 sid;
>
> What are the actual SID values like? If they're large and sparse then
> maybe a list makes sense, but if they're small and dense then an array
> hanging off the apple_dart structure itself might be more efficient.
> Given DART_MAX_STREAMS, I'm thinking the latter, and considerably so.
>
> The impression I'm getting so far is that this seems conceptually a bit
> like arm-smmu with stream indexing.
There are two (very similar) types of DARTs.
The one supported with this series has up to 16 stream ids which will be
integers <16. There's another variant used for Thunderbolt for which I will
add support in a follow-up that supports up to 64 stream ids then.
So at worst this is an array with 64 entries if this structure won't
disappear completely.
And yes, this is conceptually a bit like arm-smmu's stream indexing I think.
>
> > + u32 num_devices;
> > +
> > + struct list_head stream_head;
> > +};
> > +
> > +/*
> > + * This structure is attached to each iommu domain handled by a DART.
> > + * A single domain is used to represent a single virtual address space.
> > + * It is always allocated together with a page table.
> > + *
> > + * Streams are the smallest units the DART hardware can differentiate.
> > + * These are pointed to the page table of a domain whenever a device is
> > + * attached to it. A single stream can only be assigned to a single domain.
> > + *
> > + * Devices are assigned to at least a single and sometimes multiple
> > individual
> > + * streams (using the iommus property in the device tree). Multiple devices
> > + * can theoretically be represented by the same stream, though this is
> >
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
^^ Nit: the subsystem style for the subject format should be "iommu/dart: Add..." - similarly on patch #1, which I just realised I missed (sorry!) On 2021-06-27 15:34, Sven Peter wrote: Apple's new SoCs use iommus for almost all peripherals. These Device Address Resolution Tables must be setup before these peripherals can act as DMA masters. Signed-off-by: Sven Peter --- MAINTAINERS |1 + drivers/iommu/Kconfig| 15 + drivers/iommu/Makefile |1 + drivers/iommu/apple-dart-iommu.c | 1058 ++ I'd be inclined to drop "-iommu" from the filename, unless there's some other "apple-dart" functionality that might lead to a module name clash in future? 4 files changed, 1075 insertions(+) create mode 100644 drivers/iommu/apple-dart-iommu.c diff --git a/MAINTAINERS b/MAINTAINERS index 29e5541c8f21..c1ffaa56b5f9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1245,6 +1245,7 @@ M:Sven Peter L:iommu@lists.linux-foundation.org S:Maintained F:Documentation/devicetree/bindings/iommu/apple,dart.yaml +F: drivers/iommu/apple-dart-iommu.c APPLE SMC DRIVER M:Henrik Rydberg diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 1f111b399bca..87882c628b46 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -249,6 +249,21 @@ config SPAPR_TCE_IOMMU Enables bits of IOMMU API required by VFIO. The iommu_ops is not implemented as it is not necessary for VFIO. +config IOMMU_APPLE_DART + tristate "Apple DART IOMMU Support" + depends on ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64) + select IOMMU_API + select IOMMU_IO_PGTABLE This is redundant - the individual formats already select it. + select IOMMU_IO_PGTABLE_LPAE + default ARCH_APPLE + help + Support for Apple DART (Device Address Resolution Table) IOMMUs + found in Apple ARM SoCs like the M1. + This IOMMU is required for most peripherals using DMA to access + the main memory. + + Say Y here if you are using an Apple SoC with a DART IOMMU. + # ARM IOMMU support config ARM_SMMU tristate "ARM Ltd. System MMU (SMMU) Support" diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile index c0fb0ba88143..8c813f0ebc54 100644 --- a/drivers/iommu/Makefile +++ b/drivers/iommu/Makefile @@ -29,3 +29,4 @@ obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o obj-$(CONFIG_IOMMU_SVA_LIB) += iommu-sva-lib.o io-pgfault.o obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o +obj-$(CONFIG_IOMMU_APPLE_DART) += apple-dart-iommu.o diff --git a/drivers/iommu/apple-dart-iommu.c b/drivers/iommu/apple-dart-iommu.c new file mode 100644 index ..637ba6e7cef9 --- /dev/null +++ b/drivers/iommu/apple-dart-iommu.c @@ -0,0 +1,1058 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Apple DART (Device Address Resolution Table) IOMMU driver + * + * Copyright (C) 2021 The Asahi Linux Contributors + * + * Based on arm/arm-smmu/arm-ssmu.c and arm/arm-smmu-v3/arm-smmu-v3.c + * Copyright (C) 2013 ARM Limited + * Copyright (C) 2015 ARM Limited + * and on exynos-iommu.c + * Copyright (c) 2011,2016 Samsung Electronics Co., Ltd. + */ + +#include +#include +#include Oh, right, bus_dma_region. Fair enough :) +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include Redundant duplicate +#define DART_MAX_STREAMS 16 +#define DART_MAX_TTBR 4 + +#define DART_STREAM_ALL 0x + +#define DART_PARAMS1 0x00 +#define DART_PARAMS_PAGE_SHIFT GENMASK(27, 24) + +#define DART_PARAMS2 0x04 +#define DART_PARAMS_BYPASS_SUPPORT BIT(0) + +#define DART_STREAM_COMMAND 0x20 +#define DART_STREAM_COMMAND_BUSY BIT(2) +#define DART_STREAM_COMMAND_INVALIDATE BIT(20) + +#define DART_STREAM_SELECT 0x34 + +#define DART_ERROR 0x40 +#define DART_ERROR_STREAM GENMASK(27, 24) +#define DART_ERROR_CODE GENMASK(23, 0) +#define DART_ERROR_FLAG BIT(31) +#define DART_ERROR_READ_FAULT BIT(4) +#define DART_ERROR_WRITE_FAULT BIT(3) +#define DART_ERROR_NO_PTE BIT(2) +#define DART_ERROR_NO_PMD BIT(1) +#define DART_ERROR_NO_TTBR BIT(0) + +#define DART_CONFIG 0x60 +#define DART_CONFIG_LOCK BIT(15) + +#define DART_STREAM_COMMAND_BUSY_TIMEOUT 100 + +#define DART_STREAM_REMAP 0x80 + +#define DART_ERROR_ADDR_HI 0x54 +#define DART_ERROR_ADDR_LO 0x50 + +#define DART_TCR(sid) (0x100 + 4 * (sid)) +#define DART_TCR_TRANSLATE_ENABLE BIT(7) +#define DART_TCR_BYPASS0_ENABLE BIT(8) +#define DART_TCR_BYPASS1_ENABLE BIT(12) + +#define DART_TTBR(sid, idx) (0x200 + 16 * (sid) + 4 * (idx)) +#define DART_TTBR_VALID BIT(31) +#define DART_TTBR_SHIFT 12 + +/* + * Private structure associated with each DART device. + * + * @dev: device struct + * @regs: mapped MMIO region + * @irq: interrupt number, can be shared with
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
> > Should we be checking alignment here? Something like > > > > BUG_ON(paddr & ((1 << DART_TTBR_SHIFT) - 1)); > > > > Sure, right now paddr will always be aligned but adding that > BUG_ON doesn't hurt :) Probably should have suggested WARN_ON instead of BUG_ON but yes. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
Hi,
On Wed, Jun 30, 2021, at 15:49, Alyssa Rosenzweig wrote:
> Looks really good! Just a few minor comments. With them addressed,
>
> Reviewed-by: Alyssa Rosenzweig
Thanks!
>
> > + Say Y here if you are using an Apple SoC with a DART IOMMU.
>
> Nit: Do we need to spell out "with a DART IOMMU"? Don't all the apple
> socs need DART?
Good point, I'll remove it.
>
> > +/*
> > + * This structure is used to identify a single stream attached to a domain.
> > + * It's used as a list inside that domain to be able to attach multiple
> > + * streams to a single domain. Since multiple devices can use a single
> > stream
> > + * it additionally keeps track of how many devices are represented by this
> > + * stream. Once that number reaches zero it is detached from the IOMMU
> > domain
> > + * and all translations from this stream are disabled.
> > + *
> > + * @dart: DART instance to which this stream belongs
> > + * @sid: stream id within the DART instance
> > + * @num_devices: count of devices attached to this stream
> > + * @stream_head: list head for the next stream
> > + */
> > +struct apple_dart_stream {
> > + struct apple_dart *dart;
> > + u32 sid;
> > +
> > + u32 num_devices;
> > +
> > + struct list_head stream_head;
> > +};
>
> It wasn't obvious to me why we can get away without reference counting.
> Looking ahead it looks like we assert locks in each case. Maybe add
> that to the comment?
Sure, I'll add that to the comment.
>
> ```
> > +static void apple_dart_hw_set_ttbr(struct apple_dart *dart, u16 sid, u16
> > idx,
> > + phys_addr_t paddr)
> > +{
> > + writel(DART_TTBR_VALID | (paddr >> DART_TTBR_SHIFT),
> > + dart->regs + DART_TTBR(sid, idx));
> > +}
> ```
>
> Should we be checking alignment here? Something like
>
> BUG_ON(paddr & ((1 << DART_TTBR_SHIFT) - 1));
>
Sure, right now paddr will always be aligned but adding that
BUG_ON doesn't hurt :)
Best,
Sven
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
Looks really good! Just a few minor comments. With them addressed,
Reviewed-by: Alyssa Rosenzweig
> + Say Y here if you are using an Apple SoC with a DART IOMMU.
Nit: Do we need to spell out "with a DART IOMMU"? Don't all the apple
socs need DART?
> +/*
> + * This structure is used to identify a single stream attached to a domain.
> + * It's used as a list inside that domain to be able to attach multiple
> + * streams to a single domain. Since multiple devices can use a single stream
> + * it additionally keeps track of how many devices are represented by this
> + * stream. Once that number reaches zero it is detached from the IOMMU domain
> + * and all translations from this stream are disabled.
> + *
> + * @dart: DART instance to which this stream belongs
> + * @sid: stream id within the DART instance
> + * @num_devices: count of devices attached to this stream
> + * @stream_head: list head for the next stream
> + */
> +struct apple_dart_stream {
> + struct apple_dart *dart;
> + u32 sid;
> +
> + u32 num_devices;
> +
> + struct list_head stream_head;
> +};
It wasn't obvious to me why we can get away without reference counting.
Looking ahead it looks like we assert locks in each case. Maybe add
that to the comment?
```
> +static void apple_dart_hw_set_ttbr(struct apple_dart *dart, u16 sid, u16 idx,
> +phys_addr_t paddr)
> +{
> + writel(DART_TTBR_VALID | (paddr >> DART_TTBR_SHIFT),
> +dart->regs + DART_TTBR(sid, idx));
> +}
```
Should we be checking alignment here? Something like
BUG_ON(paddr & ((1 << DART_TTBR_SHIFT) - 1));
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
