| Message ID | 20211117144844.241072-1-jean-philippe@linaro.org |
|---|---|
| Headers | show |
| Series | perf/smmuv3: Support devicetree | expand |
On 17/11/2021 14:48, Jean-Philippe Brucker wrote: > From: Robin Murphy<robin.murphy@arm.com> > > The SMMU_PMCG_IIDR register was not present in older revisions of the > Arm SMMUv3 spec. On Arm Ltd. implementations, the IIDR value consists of > fields from several PIDR registers, allowing us to present a > standardized identifier to userspace. > So is there some userspace part to go with this now? Thanks, John
On 2021-12-07 09:14, John Garry wrote: > On 17/11/2021 14:48, Jean-Philippe Brucker wrote: >> From: Robin Murphy<robin.murphy@arm.com> >> >> The SMMU_PMCG_IIDR register was not present in older revisions of the >> Arm SMMUv3 spec. On Arm Ltd. implementations, the IIDR value consists of >> fields from several PIDR registers, allowing us to present a >> standardized identifier to userspace. >> > So is there some userspace part to go with this now? FWIW I've not looked into it - is it just a case of someone knocking out some JSON from the MMU-600/700 TRMs, or is there still mroe to do? I had the impression that *some* part of the process was stalled until implementations can start providing meaningful IIDRs, but I wasn't sure whether that was tooling or just data. I just work the low-level enablement angle :) Robin.
On 07/12/2021 12:04, Robin Murphy wrote: >>> >> So is there some userspace part to go with this now? > > FWIW I've not looked into it - is it just a case of someone knocking out > some JSON from the MMU-600/700 TRMs, or is there still mroe to do? That should just be it. > I had > the impression that *some* part of the process was stalled until > implementations can start providing meaningful IIDRs, but I wasn't sure > whether that was tooling or just data. I just work the low-level > enablement angle :) Tooling should be ok, but I would just like to see more of these JSONs so any tooling issues can be ironed out. Cheers, John
On 2021-12-07 12:28, John Garry via iommu wrote: > On 07/12/2021 12:04, Robin Murphy wrote: >>>> >>> So is there some userspace part to go with this now? >> >> FWIW I've not looked into it - is it just a case of someone knocking >> out some JSON from the MMU-600/700 TRMs, or is there still mroe to do? > > That should just be it. > >> I had the impression that *some* part of the process was stalled until >> implementations can start providing meaningful IIDRs, but I wasn't >> sure whether that was tooling or just data. I just work the low-level >> enablement angle :) > > Tooling should be ok, but I would just like to see more of these JSONs > so any tooling issues can be ironed out. Sounds good - Jean, Leo, is that something Linaro might like to pick up as part of the PMCG interest, or shall I make a note on my to-do list for the new year? Thanks, Robin. > > Cheers, > John > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu
On Tue, Dec 07, 2021 at 12:48:13PM +0000, Robin Murphy wrote: > On 2021-12-07 12:28, John Garry via iommu wrote: > > On 07/12/2021 12:04, Robin Murphy wrote: > > > > > > > > > So is there some userspace part to go with this now? > > > > > > FWIW I've not looked into it - is it just a case of someone knocking > > > out some JSON from the MMU-600/700 TRMs, or is there still mroe to > > > do? > > > > That should just be it. Hope I didn't arrive too late :) Yes, I think we just missed two things: the DT binding for SMMUv3 PMU which is just addressed by this patchset; and the PMU event aliasing for SMMUv3 PMU, before I inquired with John and John said he would upstream the related patches after kernel can export a IIDR value via sysfs node. Seems to me, after this patchset for DT binding and PMU event alias patches are landed to the mainline kernel, it would be perfect. > > > I had the impression that *some* part of the process was stalled > > > until implementations can start providing meaningful IIDRs, but I > > > wasn't sure whether that was tooling or just data. I just work the > > > low-level enablement angle :) > > > > Tooling should be ok, but I would just like to see more of these JSONs > > so any tooling issues can be ironed out. > > Sounds good - Jean, Leo, is that something Linaro might like to pick up as > part of the PMCG interest, or shall I make a note on my to-do list for the > new year? I took a look for current patch for using PIDR to synthesize IIDR, it looks good to me. But I tested it on Hisilicon D06 board and observed the composed IIDR values are still zeros. I added a printk sentence to dump iidr value at the end of the function smmu_pmu_get_iidr(): leoy@ubuntu:~$ dmesg | grep iidr [ 28.674087] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.8.auto: iidr=0x0 [ 28.705239] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.9.auto: iidr=0x0 [ 28.729924] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.10.auto: iidr=0x0 [ 28.754855] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.11.auto: iidr=0x0 [ 28.779811] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.12.auto: iidr=0x0 [ 28.804755] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.13.auto: iidr=0x0 [ 28.829825] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.14.auto: iidr=0x0 [ 28.854767] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.15.auto: iidr=0x0 Please confirm if this is expected or not? I think this might introduce difficulty for John for the PMU event alias patches, which is dependent on a non-zero IIDR. At last, very appreciate your (Jean-Philippe, Robin and John) help! Thanks, Leo
On 2021-12-07 13:20, Leo Yan wrote: > On Tue, Dec 07, 2021 at 12:48:13PM +0000, Robin Murphy wrote: >> On 2021-12-07 12:28, John Garry via iommu wrote: >>> On 07/12/2021 12:04, Robin Murphy wrote: >>>>>> >>>>> So is there some userspace part to go with this now? >>>> >>>> FWIW I've not looked into it - is it just a case of someone knocking >>>> out some JSON from the MMU-600/700 TRMs, or is there still mroe to >>>> do? >>> >>> That should just be it. > > Hope I didn't arrive too late :) > > Yes, I think we just missed two things: the DT binding for SMMUv3 PMU > which is just addressed by this patchset; and the PMU event aliasing > for SMMUv3 PMU, before I inquired with John and John said he would > upstream the related patches after kernel can export a IIDR value via > sysfs node. > > Seems to me, after this patchset for DT binding and PMU event alias > patches are landed to the mainline kernel, it would be perfect. > >>>> I had the impression that *some* part of the process was stalled >>>> until implementations can start providing meaningful IIDRs, but I >>>> wasn't sure whether that was tooling or just data. I just work the >>>> low-level enablement angle :) >>> >>> Tooling should be ok, but I would just like to see more of these JSONs >>> so any tooling issues can be ironed out. >> >> Sounds good - Jean, Leo, is that something Linaro might like to pick up as >> part of the PMCG interest, or shall I make a note on my to-do list for the >> new year? > > I took a look for current patch for using PIDR to synthesize IIDR, it > looks good to me. But I tested it on Hisilicon D06 board and observed > the composed IIDR values are still zeros. > > I added a printk sentence to dump iidr value at the end of the function > smmu_pmu_get_iidr(): > > leoy@ubuntu:~$ dmesg | grep iidr > [ 28.674087] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.8.auto: iidr=0x0 > [ 28.705239] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.9.auto: iidr=0x0 > [ 28.729924] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.10.auto: iidr=0x0 > [ 28.754855] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.11.auto: iidr=0x0 > [ 28.779811] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.12.auto: iidr=0x0 > [ 28.804755] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.13.auto: iidr=0x0 > [ 28.829825] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.14.auto: iidr=0x0 > [ 28.854767] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.15.auto: iidr=0x0 > > Please confirm if this is expected or not? I think this might > introduce difficulty for John for the PMU event alias patches, which > is dependent on a non-zero IIDR. Yes, from previous discussions I believe the HiSilicon implementations don't have much meaningful ID information at all (hence why we have to match ACPI table headers to identify the counter erratum). My trick only works for Arm Ltd. implementations since they happen to have the IMP-DEF CoreSight registers with the same information as would be in the future IIDR. To clarify, the proposal at this point is to write up JSON files for MMU-600/MMU-700, based on this patch, in order to pipe-clean the process for future SMMUv3.3 PMCG implementations with real IIDRs. Whether other implementers might retroactively define "equivalent" IIDR values for their existing implementations in a way we could potentially quirk in the driver is an orthogonal question. Cheers, Robin. > > At last, very appreciate your (Jean-Philippe, Robin and John) help! > > Thanks, > Leo >
On Tue, Dec 07, 2021 at 01:46:49PM +0000, Robin Murphy wrote: [...] > > [ 28.854767] arm-smmu-v3-pmcg arm-smmu-v3-pmcg.15.auto: iidr=0x0 > > > > Please confirm if this is expected or not? I think this might > > introduce difficulty for John for the PMU event alias patches, which > > is dependent on a non-zero IIDR. > > Yes, from previous discussions I believe the HiSilicon implementations don't > have much meaningful ID information at all (hence why we have to match ACPI > table headers to identify the counter erratum). My trick only works for Arm > Ltd. implementations since they happen to have the IMP-DEF CoreSight > registers with the same information as would be in the future IIDR. > > To clarify, the proposal at this point is to write up JSON files for > MMU-600/MMU-700, based on this patch, in order to pipe-clean the process for > future SMMUv3.3 PMCG implementations with real IIDRs. > > Whether other implementers might retroactively define "equivalent" IIDR > values for their existing implementations in a way we could potentially > quirk in the driver is an orthogonal question. Agreed, it makes sense that supports the standard IP modules in the mainline kernel at this stage. Thanks for explanation. Leo
On 07/12/2021 13:59, Leo Yan wrote: >> Whether other implementers might retroactively define "equivalent" IIDR >> values for their existing implementations in a way we could potentially >> quirk in the driver is an orthogonal question. > Agreed, it makes sense that supports the standard IP modules in > the mainline kernel at this stage. > > Thanks for explanation. Leo, if you really want this to work on D06, I could also hack some out-of-tree perf tool patches for you. I'm not sure if you're interested in that. Let me know. Thanks, John
On Tue, Dec 07, 2021 at 02:00:35PM +0000, John Garry wrote: > On 07/12/2021 13:59, Leo Yan wrote: > > > Whether other implementers might retroactively define "equivalent" IIDR > > > values for their existing implementations in a way we could potentially > > > quirk in the driver is an orthogonal question. > > Agreed, it makes sense that supports the standard IP modules in > > the mainline kernel at this stage. > > > > Thanks for explanation. > > Leo, if you really want this to work on D06, I could also hack some > out-of-tree perf tool patches for you. I'm not sure if you're interested in > that. Let me know. No, please don't spend time on this. I just use D06 platform to verify SMMUv3 relevant patches, but have no requirement for profiling SMMUv3 on it. Anyway, thanks a lot! Leo
On Wed, 17 Nov 2021 14:48:42 +0000, Jean-Philippe Brucker wrote: > Add devicetree binding for the SMMUv3 PMU, called Performance Monitoring > Counter Group (PMCG) in the spec. Each SMMUv3 implementation can have > multiple independent PMCGs, for example one for the Translation Control > Unit (TCU) and one per Translation Buffer Unit (TBU). > > Since v1 [1]: > * Fixed warnings in the binding doc > * Removed hip08 support > * Merged Robin's version. I took the liberty of splitting the driver > patch into 2 and 3. One fix in patch 3, and whitespace changes (the > driver uses spaces instead of tabs to align #define values, which I > was going to fix but actually seems more common across the tree.) > > [...] Applied to arm64 (for-next/perf-smmu), thanks! [1/3] dt-bindings: Add Arm SMMUv3 PMCG binding https://git.kernel.org/arm64/c/2704e7594383 [2/3] perf/smmuv3: Add devicetree support https://git.kernel.org/arm64/c/3f7be4356176 [3/3] perf/smmuv3: Synthesize IIDR from CoreSight ID registers https://git.kernel.org/arm64/c/df457ca973fe Cheers,