Messages in this thread | | | From | Michael Shavit <> | Date | Fri, 18 Aug 2023 02:29:22 +0800 | Subject | Re: [PATCH] iommu/arm-smmu-v3: Simplify stage selection logic |
| |
On Fri, Aug 18, 2023 at 2:19 AM Robin Murphy <robin.murphy@arm.com> wrote: > > On 2023-08-17 18:06, Michael Shavit wrote: > > On Fri, Aug 18, 2023 at 12:35 AM Robin Murphy <robin.murphy@arm.com> wrote: > >> > >> The reason it's like this is because of arm_smmu_enable_nesting(), which > >> *is* the additional thing that's going on with the stage selection logic. > >> > >> Thanks, > >> Robin. > > > > Right, but arm_smmu_enable_nesting isn't involved in this computation > > at this point in the flow. > > > > arm_smmu_enable_nesting returns early if smmu_domain->smmu isn't set, > > and smmu_domain->smmu is only set after arm_smmu_domain_finalise. > > So at this point, smmu_domain->stage is being initialized for the > > first time. If this code is responsible for handling some special > > nesting case, then it's probably not working as intended. > > I think you may have misread that code...
oof, yes, I did indeed misread.
> > Anyway, the point of the logic here is that it is not "selection", it > is, as the comment says, "restriction" - i.e. it is checking that the > already-selected stage is actually supported, and coercing it if not. > The default selection for a newly-allocated domain is always implicitly > ARM_SMMU_DOMAIN_S1 (which is explicitly defined as 0 to convey that > significance), but it may be set to ARM_SMMU_DOMAIN_NESTED before the > first attach finalises the pagetable format.
Thanks for the explanation, that does make sense :) .
> Obviously this could be clearer, especially for anyone not so familiar > with all the history, but at this point I honestly don't think it's > worth doing anything without completely ripping out > arm_smmu_enable_nesting() as well. Jason already had a patch a while > ago, and my bus rework is now also very close to the point of finally > fixing iommu_domain_alloc() to be able to return working domains, such > that all the "domain_finalise" bodges go away and that whole "modify the > domain between allocation and attach" paradigm is no longer valid at all. > > By this point I'm not too fussed about breaking the current meaning of > ARM_SMMU_DOMAIN_NESTED any more. But what I definitely don't want to do > is have a change like this which subtly but decisively breaks it while > still leaving all the now-dead code in place ;)
Ack, will drop this change.
| |