On 21/08/2019 13:11, Will Deacon wrote:
On Tue, Aug 20, 2019 at 07:41:52PM +0100, Robin Murphy wrote:
On 20/08/2019 17:07, Will Deacon wrote:
On Tue, Aug 20, 2019 at 04:25:56PM +0100, Robin Murphy wrote:
On 20/08/2019 11:31, Will Deacon wrote:
On Mon, Aug 19, 2019 at 07:19:30PM +0100, Robin Murphy wrote:
Although it's conceptually nice for the io_pgtable_cfg to provide a
standard VMSA TCR value, the reality is that no VMSA-compliant IOMMU
looks exactly like an Arm CPU, and they all have various other TCR
controls which io-pgtable can't be expected to understand. Thus since
there is an expectation that drivers will have to add to the given TCR
value anyway, let's strip it down to just the essentials that are
directly relevant to io-pgatble's inner workings - namely the address
sizes, walk attributes, and where appropriate, format selection.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/arm-smmu-v3.c | 7 +------
drivers/iommu/arm-smmu.c | 1 +
drivers/iommu/arm-smmu.h | 2 ++
drivers/iommu/io-pgtable-arm-v7s.c | 6 ++----
drivers/iommu/io-pgtable-arm.c | 4 ----
drivers/iommu/qcom_iommu.c | 2 +-
6 files changed, 7 insertions(+), 15 deletions(-)
Hmm, so I'm a bit nervous about this one since I think we really should
be providing a TCR with EPD1 set if we're only giving you TTBR0. Relying
on the driver to do this worries me. See my comments on the next patch.
The whole idea is that we already know we can't provide a *complete* TCR
value (not least because anything above bit 31 is the wild west), thus
there's really no point in io-pgtable trying to provide anything other than
the parts it definitely controls. It makes sense to provide this partial TCR
value "as if" for TTBR0, since that's the most common case, but ultimately
io-pgatble doesn't know (or need to) which TTBR the caller intends to
actually use for this table. Even if the caller *is* allocating it for
TTBR0, io-pgtable doesn't know that they haven't got something live in TTBR1
already, so it still wouldn't be in a position to make the EPD1 call either
way.
Ok, but the driver can happily rewrite/ignore what it gets back. I suppose
an alternative would be scrapped the 'u64 tcr' and instead having a bunch
of named bitfields for the stuff we're actually providing, although I'd
still like EPDx to be in there.
I like the bitfield idea; it would certainly emphasise the "you have to do
something more with this" angle that I'm pushing towards here, but still
leave things framed in TCR terms without having to go to some more general
abstraction. It really doesn't play into your EPD argument though - such a
config would be providing TxSZ/TGx/IRGNx/ORGNx/SHx, but EPDy, for y = !x.
For a driver to understand that and do the right thing with it is even more
involved than for the driver to just set EPD1 by itself anyway.
Having considered the bitfield idea some more, I'm less attached to EPDx
because we simply wouldn't be making a statement about them, rather than a
(dangerous) zero value and expecting it to be ignored. So I think we're in
agreement on that.
Cool, I'll give bitfields a go for v2.
The only part I'm still stuck to is that I think io-pgtable should know
whether it's targetting TTBR0 or TTBR1 so that it can sanitise input
addresses correctly. Doing this in the driver code is possible, but I'd
rather not start from that position, particularly as it would require things
like sign-extension in the TLBI callbacks.
Good point, and thanks for the prod that the way I end up propagating
masked-off IOVAs through to the TLBI calls here is busted either way.
I'm OK with introducing an explicit TTBR1 quirk to begin with, as it
should make things a little easier to reason about, and we can always
revisit later if and when we do find a need for more flexibility.
If only LPAE had created these bits as enables rather than disables then
things would be logical and we could all be happy, but here we are...
I'm happy! :D:D:D
Yeah, but I refuse to believe it's ever because of pagetables ;)
Robin.
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
