IOMMU Archive on lore.kernel.org
 help / color / Atom feed
From: John Garry <john.garry@huawei.com>
To: Robin Murphy <robin.murphy@arm.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 09:47:45 +0100
Message-ID: <1d546b4b-a2ad-49da-b532-951232093a9f@huawei.com> (raw)
In-Reply-To: <66a3ce9f-d3cd-110f-7353-46e6eaf25b7c@arm.com>

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.

And, no, hi161x does not have any PMCGs.

>
> - 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?

Note: I do acknowledge that an overall issue is that we assume all PMCG 
IMP DEF events are same for a given SMMU model.

>
> 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.

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.

Cheers,
John

>
> Thanks,
> Robin.
>
>>
>> John Garry (6):
>>    ACPI/IORT: Set PMCG device parent
>>    iommu/arm-smmu-v3: Record IIDR in arm_smmu_device structure
>>    perf/smmuv3: Retrieve parent SMMUv3 IIDR
>>    perf/smmuv3: Support HiSilicon hip08 (hi1620) IMP DEF events
>>    perf/smmuv3: Match implementation options based on parent SMMU IIDR
>>    ACPI/IORT: Drop code to set the PMCG software-defined model
>>
>>   drivers/acpi/arm64/iort.c     | 69 ++++++++++++++--------------
>>   drivers/iommu/arm-smmu-v3.c   |  5 +++
>>   drivers/perf/arm_smmuv3_pmu.c | 84 ++++++++++++++++++++++++++++++-----
>>   include/linux/acpi_iort.h     |  8 ----
>>   4 files changed, 112 insertions(+), 54 deletions(-)
>>
>
> .
>


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-30 14:33 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 [this message]
2019-10-16 10:51     ` Robin Murphy
2019-10-16 12:07       ` John Garry

Reply instructions:

You may reply publically 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=1d546b4b-a2ad-49da-b532-951232093a9f@huawei.com \
    --to=john.garry@huawei.com \
    --cc=guohanjun@huawei.com \
    --cc=iommu@lists.linux-foundation.org \
    --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=robin.murphy@arm.com \
    --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

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org
	public-inbox-index linux-iommu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git