From: Robin Murphy <robin.murphy@arm.com>
To: John Garry <john.garry@huawei.com>,
lorenzo.pieralisi@arm.com, guohanjun@huawei.com,
sudeep.holla@arm.com, mark.rutland@arm.com, will@kernel.org
Cc: nleeder@codeaurora.org, rjw@rjwysocki.net,
linux-kernel@vger.kernel.org, linuxarm@huawei.com,
iommu@lists.linux-foundation.org,
linux-arm-kernel@lists.infradead.org, lenb@kernel.org
Subject: Re: [RFC PATCH 0/6] SMMUv3 PMCG IMP DEF event support
Date: Wed, 16 Oct 2019 11:51:22 +0100 [thread overview]
Message-ID: <fc2df5d8-561a-b25b-e8f1-79aeb913687f@arm.com> (raw)
In-Reply-To: <1d546b4b-a2ad-49da-b532-951232093a9f@huawei.com>
On 2019-10-16 9:47 am, John Garry wrote:
> On 15/10/2019 19:00, Robin Murphy wrote:
>> Hi John,
>>
>> On 30/09/2019 15:33, John Garry wrote:
>>> This patchset adds IMP DEF event support for the SMMUv3 PMCG.
>>>
>>> It is marked as an RFC as the method to identify the PMCG implementation
>>> may be a quite disliked. And, in general, the series is somewhat
>>> incomplete.
>>>
>>> So the background is that the PMCG supports IMP DEF events, yet we
>>> have no
>>> method to identify the PMCG to know the IMP DEF events.
>>>
>>> A method for identifying the PMCG implementation could be using
>>> PMDEVARCH, but we cannot rely on this being set properly, as whether
>>> this
>>> is implemented is not defined in SMMUv3 spec.
>>>
>>> Another method would be perf event aliasing, but this method of event
>>> matching is based on CPU id, which would not guarantee same
>>> uniqueness as PMCG implementation.
>>>
>>> Yet another method could be to continue using ACPI OEM ID in the IORT
>>> code, but this does not scale. And it is not suitable if we ever add DT
>>> support to the PMCG driver.
>>>
>>> The method used in this series is based on matching on the parent SMMUv3
>>> IIDR. We store this IIDR contents in the arm smmu structure as the first
>>> element, which means that we don't have to expose SMMU APIs - this is
>>> the part which may be disliked.
>>>
>>> The final two patches switch the pre-existing PMCG model identification
>>> from ACPI OEM ID to the same parent SMMUv3 IIDR matching.
>>>
>>> For now, we only consider SMMUv3' nodes being the associated node for
>>> PMCG.
>>
>
> Hi Robin,
>
>> Two significant concerns right off the bat:
>>
>> - It seems more common than not for silicon designers to fail to
>> implement IIDR correctly, so it's only a matter of time before
>> inevitably needing to bring back some firmware-level identifier
>> abstraction (if not already - does Hi161x have PMCGs?)
>
> Maybe there's a way that we can switch to this method, and leave the
> door open for an easy way to support firmware-level identifier again, if
> ever needed. I'm not too pushed - this was secondary to just allowing
> the PMCG driver know the associated SMMU model.
But that's the part I'm not buying - there's no clear advantage to
pushing that complexity down into the PMCG driver, vs. leaving the IORT
code responsible for translating an SMMU model into a PMCG model, yet
the aforementioned disadvantages jump out right away.
> And, no, hi161x does not have any PMCGs.
Hooray, I guess :)
>>
>> - This seems like a step in entirely the wrong direction for supporting
>> .
>
> So to support PMCGs that reference a Named Component or Root Complex, I
> thought that the IORT parsing code would have to do some secondary
> lookup to the associated SMMU, through the Named Component or Root
> Complex node.
>
> What was your idea here?
The associated SMMU has no relevance in that context - the reason for
the Node Reference to point to a non-SMMU node is for devices that
implement their own embedded TLB (e.g. AMBA DTI masters) and expose a
standard PMCG interface to monitor it. It isn't reasonable to expect any
old PCIe controller or on-chip-accelerator driver to expose a fake SMMU
IIDR just to keep some other driver happy.
> Note: I do acknowledge that an overall issue is that we assume all PMCG
> IMP DEF events are same for a given SMMU model.
That assumption does technically fail already - I know MMU-600 has
different IMP-DEF events for its TCU and TBUs, however as long as we can
get as far as "this is some part of an MMU-600" the driver should be
able to figure out the rest (annoyingly it looks like both PMCG types
expose the same PMCG_ID_REGS information, but they should be
distinguishable by PMCG_CEIDn).
>> Interpreting the Node Reference is definitely a welcome improvement over
>> matching table headers, but absent a truly compelling argument to the
>> contrary, I'd rather retain the "PMCG model" abstraction in between that
>> and the driver itself (especially since those can trivially be hung off
>> compatibles once it comes to DT support).
>
> For DT, I would assume that we just use compatible strings would allow
> us to identify the PMCG model.
Right, that was largely my point - DT probing can start with a PMCG
model, so it's a lot more logical for ACPI probing to do the same, with
the actual PMCG model determination hidden away in the ACPI code. That's
the basis of the current design.
I have been nagging the architects that PMCGs not having their own IIDR
is an unwelcome hole in the spec, so hopefully this might get a bit
easier some day.
> On a related matter, is there still a need to deal with scenarios of the
> PMCG being located within the SMMU register map? As you may remember, we
> did have this issue but relocated the PMCG to outside the SMMU register
> map in a later chip rev.
MMU-600 has its TCU PMCG page 0 in the middle of its SMMU page 0 space,
but given that it's an Arm IP, I expect that when the heat gets turned
up for making it work, it's most likely to be under me ;)
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2019-10-16 10:51 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-30 14:33 [RFC PATCH 0/6] SMMUv3 PMCG IMP DEF event support John Garry
2019-09-30 14:33 ` [RFC PATCH 1/6] ACPI/IORT: Set PMCG device parent John Garry
2019-10-15 2:35 ` Hanjun Guo
2019-10-15 9:07 ` John Garry
2019-09-30 14:33 ` [RFC PATCH 2/6] iommu/arm-smmu-v3: Record IIDR in arm_smmu_device structure John Garry
2019-09-30 14:33 ` [RFC PATCH 3/6] perf/smmuv3: Retrieve parent SMMUv3 IIDR John Garry
2019-09-30 14:33 ` [RFC PATCH 4/6] perf/smmuv3: Support HiSilicon hip08 (hi1620) IMP DEF events John Garry
2019-09-30 14:33 ` [RFC PATCH 5/6] perf/smmuv3: Match implementation options based on parent SMMU IIDR John Garry
2019-09-30 14:33 ` [RFC PATCH 6/6] ACPI/IORT: Drop code to set the PMCG software-defined model John Garry
2019-10-15 3:06 ` Hanjun Guo
2019-10-15 8:47 ` John Garry
2019-10-15 18:00 ` [RFC PATCH 0/6] SMMUv3 PMCG IMP DEF event support Robin Murphy
2019-10-16 8:47 ` John Garry
2019-10-16 10:51 ` Robin Murphy [this message]
2019-10-16 12:07 ` John Garry
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=fc2df5d8-561a-b25b-e8f1-79aeb913687f@arm.com \
--to=robin.murphy@arm.com \
--cc=guohanjun@huawei.com \
--cc=iommu@lists.linux-foundation.org \
--cc=john.garry@huawei.com \
--cc=lenb@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=mark.rutland@arm.com \
--cc=nleeder@codeaurora.org \
--cc=rjw@rjwysocki.net \
--cc=sudeep.holla@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).