From: robin.murphy@arm•com (Robin Murphy)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 6/7] iommu/arm-smmu: Decouple context format from kernel config
Date: Mon, 25 Apr 2016 17:21:05 +0100 [thread overview]
Message-ID: <571E43F1.7070700@arm.com> (raw)
In-Reply-To: <20160425134108.GD30830@arm.com>
On 25/04/16 14:41, Will Deacon wrote:
> On Mon, Apr 25, 2016 at 02:14:57PM +0100, Robin Murphy wrote:
>> On 25/04/16 12:02, Will Deacon wrote:
>>> In what case would you not have a viable AArch64 granule, but the option
>>> of falling back to AArch32 makes things work?
>>
>> A 4KB page kernel with MMU-401, whose only supported AArch64 granule is
>> supposedly 64KB.
>
> That doesn't sound right at all!
>
>>>> Plus we'd have to remove the LPAE page sizes
>>> >from the bitmap beforehand in the case we don't support AARCH64_4K lest we
>>>> end up with a phantom format the hardware doesn't actually do, and it all
>>>> starts getting rather horrible...
>>>
>>> I'm not following. The io-pgtable code shouldn't give you back a phanton
>>> format.
>>
>> If you call alloc_io_pgtable_ops() with ARM_64_LPAE_S2 and an unmolested
>> MMU-401 pgsize_bitmap, you get back a 4K granule v8 pagetable, which per a
>> strict reading of the TRM isn't supported (although does appear to work in
>> practice...)
>
> I suspect that's a badly worded/incorrect TRM. In reality, I'd be happy
> to assume that a given granule is supported across all formats (in as
> much as it can be) if it's supported by one of them, but acknowledge
> that's not guaranteed by the architecture.
>
>> Consider also a stage 1 SMMU supporting all formats but only with 4K
>> granules: with a 64K page arm64 kernel, the presence of AARCH64_4K support
>> would lead us into arm_64_lpae_alloc_pgtable_s1(), which would be tricked
>> into giving us back a 64K granule v8 format by the short descriptor large
>> page size matching PAGE_SIZE, and things would definitely go downhill from
>> there.
>
> We could improve this by having a separate pgsize_bitmap per format, and
> passing the relevant one to the relevant page table constructor.
>
>> Given that, I think the only truly safe thing to do is to pass an explicit
>> granule in the io_pgtable_cfg and just get rid of the bitmap-based guessing
>> in arm_lpae_restrict_pgsizes() - otherwise, we'd still have to pre-restrict
>> the bitmap, making it pretty much redundant anyway. I'll have a hack at that
>
> Actually, I think I prefer to go the other way. Let's try moving more of
> this common logic into the io-pgtable code. For example, an IOMMU driver
> could say "I support these formats, and these options (page sizes, quirks,
> etc) for each format" and the io-pgtable code can choose the most
> appropriate thing.
Agreed - that sounds like the best approach in the long run, and I'd be
happy to put some thought into designing it once the to-do list shrinks
a bit. Right now, though, how about a quick fix like I just hacked up
below? (diff on top of this series because laziness)
Robin.
--->8---
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a917ad0e949f..0798926f28f1 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -784,28 +784,20 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
/*
* Choosing a suitable context format is even more fiddly. Until we
- * grow some way for the caller to express a preference, just aim for
- * the closest match to the CPU format out of what we might have.
+ * grow some way for the caller to express a preference, and/or move
+ * the decision into the io-pgtable code where it arguably belongs,
+ * just aim for the closest thing to the rest of the system, and hope
+ * that the hardware isn't mental enough that we can't assume AArch64
+ * support to be a superset of AArch32 support...
*/
if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_L)
cfg->fmt = ARM_SMMU_CTX_FMT_AARCH32_L;
- if (IS_ENABLED(CONFIG_64BIT) || cfg->fmt == ARM_SMMU_CTX_FMT_NONE) {
- switch (PAGE_SIZE) {
- case SZ_64K:
- if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH64_64K) {
- cfg->fmt = ARM_SMMU_CTX_FMT_AARCH64;
- break;
- } /* else fall through */
- case SZ_16K:
- if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH64_16K) {
- cfg->fmt = ARM_SMMU_CTX_FMT_AARCH64;
- break;
- } /* else fall through */
- case SZ_4K:
- if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH64_4K)
- cfg->fmt = ARM_SMMU_CTX_FMT_AARCH64;
- }
- }
+ if ((IS_ENABLED(CONFIG_64BIT) || cfg->fmt == ARM_SMMU_CTX_FMT_NONE) &&
+ (smmu->features & (ARM_SMMU_FEAT_FMT_AARCH64_64K |
+ ARM_SMMU_FEAT_FMT_AARCH64_16K |
+ ARM_SMMU_FEAT_FMT_AARCH64_4K)))
+ cfg->fmt = ARM_SMMU_CTX_FMT_AARCH64;
+
if (cfg->fmt == ARM_SMMU_CTX_FMT_NONE) {
ret = -EINVAL;
goto out_unlock;
next prev parent reply other threads:[~2016-04-25 16:21 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-13 17:12 [PATCH 0/7] arm-smmu: Implementation and context format differentiation Robin Murphy
2016-04-13 17:12 ` [PATCH 1/7] iommu/arm-smmu: Differentiate specific implementations Robin Murphy
2016-04-13 21:15 ` Chalamarla, Tirumalesh
2016-04-13 17:12 ` [PATCH 2/7] iommu/arm-smmu: Convert ThunderX workaround to new method Robin Murphy
2016-04-13 21:16 ` Chalamarla, Tirumalesh
2016-04-13 17:12 ` [PATCH 3/7] iommu/arm-smmu: Work around MMU-500 prefetch errata Robin Murphy
2016-04-21 16:15 ` Will Deacon
2016-04-21 16:16 ` Will Deacon
2016-04-13 17:13 ` [PATCH 4/7] io-64-nonatomic: Add relaxed accessor variants Robin Murphy
2016-04-21 16:18 ` Will Deacon
2016-04-22 17:08 ` Robin Murphy
2016-04-25 13:32 ` Will Deacon
2016-04-25 15:21 ` Arnd Bergmann
2016-04-25 15:28 ` Robin Murphy
2016-04-25 15:41 ` Arnd Bergmann
2016-04-25 16:11 ` Will Deacon
2016-04-25 16:11 ` Arnd Bergmann
2016-04-26 10:38 ` [PATCH v2] " Robin Murphy
2016-04-13 17:13 ` [PATCH 5/7] iommu/arm-smmu: Tidy up 64-bit/atomic I/O accesses Robin Murphy
2016-04-13 17:13 ` [PATCH 6/7] iommu/arm-smmu: Decouple context format from kernel config Robin Murphy
2016-04-21 16:30 ` Will Deacon
2016-04-22 17:38 ` Robin Murphy
2016-04-25 11:02 ` Will Deacon
2016-04-25 13:14 ` Robin Murphy
2016-04-25 13:41 ` Will Deacon
2016-04-25 16:21 ` Robin Murphy [this message]
2016-04-28 16:12 ` [PATCH v2] " Robin Murphy
2016-04-13 17:13 ` [PATCH 7/7] iommu/arm-smmu: Support SMMUv1 64KB supplement Robin Murphy
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=571E43F1.7070700@arm.com \
--to=robin.murphy@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox