On 2/5/16, 12:15 PM, "Mark Rutland" <mark.rutl...@arm.com> wrote:
>On Fri, Feb 05, 2016 at 08:02:09PM +0000, Mark Rutland wrote:
>> On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalama...@caviumnetworks.com
>> wrote:
>> > From: Tirumalesh Chalamarla <tchalama...@caviumnetworks.com>
>> >
>> > An issue exists whereby the Linux kernel requires that ASIDs are a
>> > unique namespace per SMMU.
>>
>> The SMMU architecture requires this, and Linux relies on hardware
>> following the architecture.
>>
>> Please describe the problem correctly. i.e. that the hardware
>> erroneously shares TLBs between SMMU instances.
>>
>> > That ASID-local choice conflicts with the
>> > CN88xx SMMU, where only shared ASID namespaces are supported;
>> > specifically within a given node SMMU0 and SMMU1 share, as does SMMU2
>> > and SMMU3. CN88xx SMMU also requires global VMIDs.
>> >
>> > This patch tries to address these issuee by supplying asid and vmid
>> > transformations from devicetree.
>> >
>> > Signed-off-by: Akula Geethasowjanya
>> > <geethasowjanya.ak...@caviumnetworks.com>
>> > Signed-off-by: Tirumalesh Chalamarla <tchalama...@caviumnetworks.com>
>> > ---
>> > .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> > drivers/iommu/arm-smmu.c | 59
>> > ++++++++++++++++++----
>> > 2 files changed, 65 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > index 7180745..eef06d0 100644
>> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > @@ -55,6 +55,22 @@ conditions.
>> > aliases of secure registers have to be used during
>> > SMMU configuration.
>> >
>> > +- thunderx,smmu-asid-transform : Enable proper handling for buggy
>> > + implementations that require special transformations
>> > + for smmu asid. if this property exists asid-transform
>> > + property must be present.
>> > +
>> > +- thunderx,smmu-vmid-transform : Enable proper handling for buggy
>> > + implementations that require special transformations
>> > + for smmu vmid. if this property exists vmid-transform
>> > + property must be present.
>> > +
>> > +- asid-transform : Transform mask that needs to be applied to asid.
>> > + This property has to be specified as '/bits/ 8' value.
>> > +
>> > +- vmid-transform : Transform mask that needs to be applied to vmid.
>> > + This property has to be specified as '/bits/ 8' value.
>>
>> Don't bother with /bits/ 8. It saves no space in the DTB and only adds
>> to confusion. Use a full cell, and of_property_read_u32, and validate
>> that the value is in range.
>>
>> Having two properties for each seems redundant. The presence of a mask
>> should be sufficient to make it clear that the use of the mask is
>> required.
>>
>> These properties aren't sufficiently described. How is the mask
>> "applied", and what is it applied to? What constraints does it impose
>> on the OS's selection of ASIDs/VMIDs?
>>
>> From the looks of the patch, it's ANDed into the value Linux has
>> selected, so it sounds like an OS must avoid using any bits set in any
>> mask on any SMMU instance. That sounds very difficult to ensure in
>> general, and very fragile.
>
>A much better approach would be to have something like:
>
>valid-asid-range : two u32 cells describing the minimum and maximum
> ASIDs that may be used on this SMMU instance. The OS
> must allocate ASIDs in the interval [minimum,
> maximum].
>
>That way the OS can prevent itself from allocating conflicting ASIDs,
>which is not possible with a single base value as you have with your
>asid-transform property.
The problem is, Will don’t want this to be part of general change, he want to
treat it as Errata.
>
>Mark.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
