On Fri, Feb 24, 2017 at 12:43:19AM +0700, Suravee Suthikulpanit wrote:
> >Also, who cares about the banks, why is this exposed?
>
> The bank and counter values are not exposed to the user-space.
> The amd_iommu PMU only expose, csource, devid, domid, pasid, devid_mask,
> domid_mask, and pasid_mask as event attributes.
Ah good, for a little while I was worried the BANK stuff came from
userspace; I misread extra_reg.config and extra_reg.reg, the former
being perf_event_attr::config1 and the latter holding the bank thing.
> >That is, I would very much expect a linear range of counters. You can
> >always decompose this counter number if you really need to somewhere
> >down near the hardware accessors.
> >
>
> Actually, the counters are treated as linear range of counters. For example,
> the IOMMU hardware has 2 banks with 4 counters/bank. So, we have total of 8
> counters. The driver then assigns an index to each events when an event is
> added.
> Here, the bank/counter are derived from the assigned index, and stored in
> the perf_event as bank and counter values.
>
> However, I have looked into reworking to not use the extra_regs, and I see
> that the union in struct hw_perf_event currently contains various PMU-specific
> structures (hardware, software, tracepoint, intel_cqm, itrace, amd_power,
> and breakpoint).
>
> For amd_iommu PMU, we need additional registers for holding amd_iommu-specific
> parameters. So, it seems that we can just introduce amd_iommu-specific struct
> instead of re-using the existing structure for hardware events.
>
> I'm planning to add the following structure in the same union:
>
> union {
> ......
> struct { /* amd_iommu */
> u8 iommu_csource;
> u8 iommu_bank;
> u8 iommu_cntr;
> u16 iommu_devid;
> u16 iommu_devid_msk;
> u16 iommu_domid;
> u16 iommu_domid_msk;
> u32 iommu_pasid;
> u32 iommu_pasid_msk;
> };
> };
>
> Please let me know what you think, of if I am still missing your points.
Yes, adding a struct to that union is fine and clarifies things. And
just because I'm weird like that, there's a u8 hole after iommu_cntr.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
