[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b4169471-fcd0-4415-8281-c5bd722e5f2b@arm.com>
Date: Fri, 18 Jul 2025 13:56:36 +0100
From: Robin Murphy <robin.murphy@....com>
To: Will Deacon <will@...nel.org>,
Benjamin Gaignard <benjamin.gaignard@...labora.com>
Cc: joro@...tes.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, heiko@...ech.de, nicolas.dufresne@...labora.com,
jgg@...pe.ca, iommu@...ts.linux.dev, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v6 3/5] iommu: Add verisilicon IOMMU driver
On 2025-07-18 12:45 pm, Will Deacon wrote:
> On Mon, Jul 14, 2025 at 04:56:53PM +0200, Benjamin Gaignard wrote:
>> Le 14/07/2025 à 14:08, Will Deacon a écrit :
>>> On Thu, Jul 10, 2025 at 10:24:44AM +0200, Benjamin Gaignard wrote:
>>>> +/* vsi iommu regs address */
>>>> +#define VSI_MMU_CONFIG1_BASE 0x1ac
>>>> +#define VSI_MMU_AHB_EXCEPTION_BASE 0x380
>>>> +#define VSI_MMU_AHB_CONTROL_BASE 0x388
>>>> +#define VSI_MMU_AHB_TLB_ARRAY_BASE_L_BASE 0x38C
>>>> +
>>>> +/* MMU register offsets */
>>>> +#define VSI_MMU_FLUSH_BASE 0x184
>>>> +#define VSI_MMU_BIT_FLUSH BIT(4)
>>>> +
>>>> +#define VSI_MMU_PAGE_FAULT_ADDR 0x380
>>>> +#define VSI_MMU_STATUS_BASE 0x384 /* IRQ status */
>>>> +
>>>> +#define VSI_MMU_BIT_ENABLE BIT(0)
>>>> +
>>>> +#define VSI_MMU_OUT_OF_BOUND BIT(28)
>>>> +/* Irq mask */
>>>> +#define VSI_MMU_IRQ_MASK 0x7
>>>> +
>>>> +#define VSI_DTE_PT_ADDRESS_MASK 0xffffffc0
>>>> +#define VSI_DTE_PT_VALID BIT(0)
>>>> +
>>>> +#define VSI_PAGE_DESC_LO_MASK 0xfffff000
>>>> +#define VSI_PAGE_DESC_HI_MASK GENMASK_ULL(39, 32)
>>>> +#define VSI_PAGE_DESC_HI_SHIFT (32 - 4)
>>> How does this page-table format relate to the one supported already by
>>> rockchip-iommu.c? From a quick glance, I suspect this is a derivative
>>> and so ideally we'd be able to have a common implementation of the
>>> page-table code which can be used by both of the drivers.
>>>
>>> Similarly:
>>
>> No they comes from different IP providers, this one is from Verisilicon.
>> I agree they looks very similar and my first attempt was to add it into
>> rockchip-iommu code but when doing it I realize that registers addresses
>> where all different so I had to code all the functions twice.
>
> Understood, and I'm not suggesting to merge the drivers or try to
> consolidate the register layouts. What I _am_ saying is that the
> in-memory page-table format should be factored out in a way that can
> be reused by the two drivers and also that some of the logic (as highlighted
> by vsi_iommu_domain_free()) is practically identical between the drivers
> and should also be shared.
All they really have in common is that they're 2-level formats with
32-bit PTEs and 10 bits per level (as is tegra-smmu too). The permission
encodings have some overlap but are not fully equivalent. Crucially, the
schemes for packing >32-bit PAs into PTEs are incompatibly different, so
if you're really keen to genericise things to *that* extent, that's what
Jason's already working on.
As for domain_free, you could argue that it also looks basically the
same as exynos_iommu_domain_free(), because at the end of the day,
there's only so many ways to free a 2-level pagetable, and it's at least
better than what, say, sun50i, omap or tegra are doing (or rather, not
doing...)
Cheers,
Robin.
Powered by blists - more mailing lists