Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

On 2/24/17 01:11, Peter Zijlstra wrote:

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.


Ok, I'll update this in V10 that I'll be sending out this week.

Thanks,
Suravee
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Peter Zijlstra]

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

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

Peter,

On 2/14/17 19:31, Peter Zijlstra wrote:

On Tue, Feb 07, 2017 at 08:57:52AM +0700, Suravee Suthikulpanit wrote:

But instead it looks like you get the counter form:

 #define _GET_CNTR(ev)   ((u8)(ev->hw.extra_reg.reg))

Which is absolutely insane.



So, the IOMMU counters are grouped into bank, and there could be
many banks. I use the extra_reg.reg to hold the bank and counter
indices. This will be used to program onto the counter configuration
register. This is handled in get_next_avail_iommu_bnk_cntr() and
clear_avail_iommu_bnk_cntr().


But this is crazy. That's not what extra_regs are for.


Ok, I understand that we should not be using the extra_regs
since it is intended for other purposes. Please see more detail below.


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.


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.

Thanks,
Suravee
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Peter Zijlstra]

On Tue, Feb 07, 2017 at 08:57:52AM +0700, Suravee Suthikulpanit wrote:
> >But instead it looks like you get the counter form:
> >
> >  #define _GET_CNTR(ev)   ((u8)(ev->hw.extra_reg.reg))
> >
> >Which is absolutely insane.
> >
> 
> So, the IOMMU counters are grouped into bank, and there could be
> many banks. I use the extra_reg.reg to hold the bank and counter
> indices. This will be used to program onto the counter configuration
> register. This is handled in get_next_avail_iommu_bnk_cntr() and
> clear_avail_iommu_bnk_cntr().

But this is crazy. That's not what extra_regs are for. Also, who cares
about the banks, why is this exposed?

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.


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

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

Boris,

On 1/25/17 16:55, Borislav Petkov wrote:

On Wed, Jan 25, 2017 at 10:46:53AM +0100, Peter Zijlstra wrote:

Which is absolutely insane.


Right,

IMO, the simplest thing to do for your purposes is to embed a struct
amd_iommu pointer into struct perf_amd_iommu at init time so that you
don't have to do all that crazy dance in the PMU functions and iterate
over the iommus in get_amd_iommu() each time.

Which would then simplify all your other functions. For example:

int amd_iommu_pc_get_reg(unsigned int idx, u8 bank, u8 cntr, u8 fxn, u64 *value)

should be

int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, u8 fxn, u64 
*value)

and you can save yourself a lot of glue code and get rid of that
get_amd_iommu() thing.



That's a good idea. Thanks. I'll do this in V9.

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

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

Peter,

On 1/25/17 16:46, Peter Zijlstra wrote:

On Mon, Jan 16, 2017 at 01:23:36AM -0600, Suravee Suthikulpanit wrote:


+   pi = container_of(event->pmu, struct perf_amd_iommu, pmu);
+   hwc->idx  = pi->idx;
+   hwc->config   = event->attr.config;
+   hwc->extra_reg.config = event->attr.config1;



 static void perf_iommu_enable_event(struct perf_event *ev)
 {
+   struct hw_perf_event *hwc = >hw;
u8 csource = _GET_CSOURCE(ev);
u16 devid = _GET_DEVID(ev);
u8 bank = _GET_BANK(ev);
@@ -253,30 +248,34 @@ static void perf_iommu_enable_event(struct perf_event *ev)
u64 reg = 0ULL;

reg = csource;
-   amd_iommu_pc_set_reg(0, bank, cntr,
+   amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
 IOMMU_PC_COUNTER_SRC_REG, );


Please explain about this IOMMU crud, this looks like fail.
hwc->idx should be the counter, not a random pmu index.


Ok, I thought this was not used by the code, so I used it for
holding PMU index. This might not be a good idea.
I will change this part per Boris comment in the reply of this
thread.


But instead it looks like you get the counter form:

  #define _GET_CNTR(ev)   ((u8)(ev->hw.extra_reg.reg))

Which is absolutely insane.



So, the IOMMU counters are grouped into bank, and there could be
many banks. I use the extra_reg.reg to hold the bank and counter
indices. This will be used to program onto the counter configuration
register. This is handled in get_next_avail_iommu_bnk_cntr() and
clear_avail_iommu_bnk_cntr().

Thanks,
Suravee
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

Boris,

On 1/23/17 02:55, Borislav Petkov wrote:

@@ -421,46 +427,46 @@ static __init void amd_iommu_pc_exit(void)
 };

 static __init int
-_init_perf_amd_iommu(struct perf_amd_iommu *perf_iommu, char *name)
+init_one_perf_amd_iommu(struct perf_amd_iommu *perf_iommu, unsigned int idx)
 {
int ret;

raw_spin_lock_init(_iommu->lock);

-   /* Init cpumask attributes to only core 0 */
-   cpumask_set_cpu(0, _cpumask);
-
-   perf_iommu->max_banks = amd_iommu_pc_get_max_banks(0);
-   perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
+   perf_iommu->idx  = idx;
+   perf_iommu->max_banks= amd_iommu_pc_get_max_banks(idx);
+   perf_iommu->max_counters = amd_iommu_pc_get_max_counters(idx);
if (!perf_iommu->max_banks || !perf_iommu->max_counters)
return -EINVAL;

+   snprintf(perf_iommu->name, PERF_AMD_IOMMU_NAME_SZ, "amd_iommu_%u", idx);
+
+   perf_iommu->pmu.event_init  = perf_iommu_event_init,
+   perf_iommu->pmu.add = perf_iommu_add,
+   perf_iommu->pmu.del = perf_iommu_del,
+   perf_iommu->pmu.start   = perf_iommu_start,
+   perf_iommu->pmu.stop= perf_iommu_stop,
+   perf_iommu->pmu.read= perf_iommu_read,

This compiles but it is yucky.

You should do that instead:

static struct pmu amd_iommu_pmu = {
.event_init  = perf_iommu_event_init,
.add = perf_iommu_add,
.del = perf_iommu_del,
.start   = perf_iommu_start,
.stop= perf_iommu_stop,
.read= perf_iommu_read,
.task_ctx_nr = perf_invalid_context,
.attr_groups = amd_iommu_attr_groups,
};

...

ret = perf_pmu_register(_iommu_pmu, perf_iommu->name, -1);

Because otherwise you're carrying a struct pmu in each struct
perf_amd_iommu which has identical contents.


Actually, only the callbacks above will be identical on each pmu, but
there are other parts of the structure which are different
(e.g. pmu->name, pmu->type, etc.) Also, we need one pmu instance per
IOMMU since each pmu reference will get assigned to perf_event, and
also used to reference back to struct perf_amd_iommu. Note that each
pmu can also have different events.


Now, you need to access the struct perf_amd_iommu pointer for each
IOMMU PMU in some of the functions like perf_iommu_event_init(), for
example. But for that you only need the index and to iterate the
perf_amd_iommu_list.


I think replacing the index w/ pointer to amd_iommu is a good idea.
I'm changing this in V9.

Thanks,
Suravee


I wasn't able to find a good way to do that from a quick stare but
PeterZ might have a better idea...


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

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Borislav Petkov]

On Wed, Jan 25, 2017 at 10:46:53AM +0100, Peter Zijlstra wrote:
> Which is absolutely insane.

Right,

IMO, the simplest thing to do for your purposes is to embed a struct
amd_iommu pointer into struct perf_amd_iommu at init time so that you
don't have to do all that crazy dance in the PMU functions and iterate
over the iommus in get_amd_iommu() each time.

Which would then simplify all your other functions. For example:

int amd_iommu_pc_get_reg(unsigned int idx, u8 bank, u8 cntr, u8 fxn, u64 *value)

should be

int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, u8 fxn, u64 
*value)

and you can save yourself a lot of glue code and get rid of that
get_amd_iommu() thing.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Borislav Petkov]

On Mon, Jan 16, 2017 at 01:23:36AM -0600, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit 
> 
> Add multi-IOMMU support for perf by exposing an AMD IOMMU PMU
> for each IOMMU found in the system via:
> 
>   /bus/event_source/devices/amd_iommu_x
> 
> where x is the IOMMU index. This allows users to specify
> different events to be programed onto performance counters

"programmed"

Please introduce a spellchecker into your patch creation workflow.

> of each IOMMU.
> 
> Cc: Peter Zijlstra 
> Cc: Borislav Petkov 
> Signed-off-by: Suravee Suthikulpanit 
> ---
>  arch/x86/events/amd/iommu.c | 114 
> ++--
>  1 file changed, 67 insertions(+), 47 deletions(-)
> 
> diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
> index 223c01d..38eafbf 100644
> --- a/arch/x86/events/amd/iommu.c
> +++ b/arch/x86/events/amd/iommu.c
> @@ -35,16 +35,21 @@
>  #define _GET_PASID_MASK(ev) ((ev->hw.extra_reg.config >> 16) & 0xULL)
>  #define _GET_DOMID_MASK(ev) ((ev->hw.extra_reg.config >> 32) & 0xULL)
>  
> -static struct perf_amd_iommu __perf_iommu;
> +#define PERF_AMD_IOMMU_NAME_SZ 16

AMD_IOMMU_PMU_NAME_SIZE

sounds more to the point to me.

>  struct perf_amd_iommu {
> + struct list_head list;
>   struct pmu pmu;
> + unsigned int idx;
> + char name[PERF_AMD_IOMMU_NAME_SZ];
>   u8 max_banks;
>   u8 max_counters;
>   u64 cntr_assign_mask;
>   raw_spinlock_t lock;
>  };

...

> @@ -253,30 +248,34 @@ static void perf_iommu_enable_event(struct perf_event 
> *ev)
>   u64 reg = 0ULL;
>  
>   reg = csource;
> - amd_iommu_pc_set_reg(0, bank, cntr,
> + amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
>IOMMU_PC_COUNTER_SRC_REG, );
>  
>   reg = devid | (_GET_DEVID_MASK(ev) << 32);
>   if (reg)
>   reg |= BIT(31);
> - amd_iommu_pc_set_reg(0, bank, cntr, IOMMU_PC_DEVID_MATCH_REG, );
> + amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
> +  IOMMU_PC_DEVID_MATCH_REG, );
>  
>   reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
>   if (reg)
>   reg |= BIT(31);
> - amd_iommu_pc_set_reg(0, bank, cntr, IOMMU_PC_PASID_MATCH_REG, );
> + amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
> +  IOMMU_PC_PASID_MATCH_REG, );
>  
>   reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
>   if (reg)
>   reg |= BIT(31);
> - amd_iommu_pc_set_reg(0, bank, cntr, IOMMU_PC_DOMID_MATCH_REG, );
> + amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
> +  IOMMU_PC_DOMID_MATCH_REG, );

You can let those stick out - the 80 cols rule is not a strict one:

reg = csource;
amd_iommu_pc_set_reg(hwc->idx, bank, cntr, IOMMU_PC_COUNTER_SRC_REG, 
);

reg = devid | (_GET_DEVID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
amd_iommu_pc_set_reg(hwc->idx, bank, cntr, IOMMU_PC_DEVID_MATCH_REG, 
);

reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
amd_iommu_pc_set_reg(hwc->idx, bank, cntr, IOMMU_PC_PASID_MATCH_REG, 
);

reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
amd_iommu_pc_set_reg(hwc->idx, bank, cntr, IOMMU_PC_DOMID_MATCH_REG, 
);
}

>  static void perf_iommu_disable_event(struct perf_event *event)
>  {
> + struct hw_perf_event *hwc = >hw;
>   u64 reg = 0ULL;
>  
> - amd_iommu_pc_set_reg(0, _GET_BANK(event), _GET_CNTR(event),
> + amd_iommu_pc_set_reg(hwc->idx, _GET_BANK(event), _GET_CNTR(event),
>IOMMU_PC_COUNTER_SRC_REG, );
>  }
>  
> @@ -295,7 +294,7 @@ static void perf_iommu_start(struct perf_event *event, 
> int flags)
>   return;
>  
>   val = local64_read(>prev_count) & GENMASK_ULL(48, 0);
> - if (amd_iommu_pc_set_reg(0, _GET_BANK(event), _GET_CNTR(event),
> + if (amd_iommu_pc_set_reg(hwc->idx, _GET_BANK(event), _GET_CNTR(event),
>  IOMMU_PC_COUNTER_REG, ))
>   return;
>  
> @@ -309,7 +308,7 @@ static void perf_iommu_read(struct perf_event *event)
>   s64 delta;
>   struct hw_perf_event *hwc = >hw;
>  
> - if (amd_iommu_pc_get_reg(0, _GET_BANK(event), _GET_CNTR(event),
> + if (amd_iommu_pc_get_reg(hwc->idx, _GET_BANK(event), _GET_CNTR(event),
>IOMMU_PC_COUNTER_REG, ))
>   return;
>  
> @@ -407,6 +406,13 @@ static __init int _init_events_attrs(void)
>  
>  static __init void amd_iommu_pc_exit(void)
>  {
> + struct perf_amd_iommu *pi, *next;
> +
> + list_for_each_entry_safe(pi, next, _amd_iommu_list, list) {
> + list_del(>list);
> + kfree(pi);
> + }
> +
>   if (amd_iommu_events_group.attrs) {
> 

[PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

Add multi-IOMMU support for perf by exposing an AMD IOMMU PMU
for each IOMMU found in the system via:

  /bus/event_source/devices/amd_iommu_x

where x is the IOMMU index. This allows users to specify
different events to be programed onto performance counters
of each IOMMU.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 114 ++--
 1 file changed, 67 insertions(+), 47 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 223c01d..38eafbf 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -35,16 +35,21 @@
 #define _GET_PASID_MASK(ev) ((ev->hw.extra_reg.config >> 16) & 0xULL)
 #define _GET_DOMID_MASK(ev) ((ev->hw.extra_reg.config >> 32) & 0xULL)
 
-static struct perf_amd_iommu __perf_iommu;
+#define PERF_AMD_IOMMU_NAME_SZ 16
 
 struct perf_amd_iommu {
+   struct list_head list;
struct pmu pmu;
+   unsigned int idx;
+   char name[PERF_AMD_IOMMU_NAME_SZ];
u8 max_banks;
u8 max_counters;
u64 cntr_assign_mask;
raw_spinlock_t lock;
 };
 
+static LIST_HEAD(perf_amd_iommu_list);
+
 /*-
  * sysfs format attributes
  *-*/
@@ -202,8 +207,7 @@ static int clear_avail_iommu_bnk_cntr(struct perf_amd_iommu 
*perf_iommu,
 static int perf_iommu_event_init(struct perf_event *event)
 {
struct hw_perf_event *hwc = >hw;
-   struct perf_amd_iommu *perf_iommu;
-   u64 config, config1;
+   struct perf_amd_iommu *pi;
 
/* test the event attr type check for PMU enumeration */
if (event->attr.type != event->pmu->type)
@@ -225,27 +229,18 @@ static int perf_iommu_event_init(struct perf_event *event)
if (event->cpu < 0)
return -EINVAL;
 
-   perf_iommu = &__perf_iommu;
-
-   if (event->pmu != _iommu->pmu)
-   return -ENOENT;
-
-   if (perf_iommu) {
-   config = event->attr.config;
-   config1 = event->attr.config1;
-   } else {
-   return -EINVAL;
-   }
-
/* update the hw_perf_event struct with the iommu config data */
-   hwc->config = config;
-   hwc->extra_reg.config = config1;
+   pi = container_of(event->pmu, struct perf_amd_iommu, pmu);
+   hwc->idx  = pi->idx;
+   hwc->config   = event->attr.config;
+   hwc->extra_reg.config = event->attr.config1;
 
return 0;
 }
 
 static void perf_iommu_enable_event(struct perf_event *ev)
 {
+   struct hw_perf_event *hwc = >hw;
u8 csource = _GET_CSOURCE(ev);
u16 devid = _GET_DEVID(ev);
u8 bank = _GET_BANK(ev);
@@ -253,30 +248,34 @@ static void perf_iommu_enable_event(struct perf_event *ev)
u64 reg = 0ULL;
 
reg = csource;
-   amd_iommu_pc_set_reg(0, bank, cntr,
+   amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
 IOMMU_PC_COUNTER_SRC_REG, );
 
reg = devid | (_GET_DEVID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_set_reg(0, bank, cntr, IOMMU_PC_DEVID_MATCH_REG, );
+   amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
+IOMMU_PC_DEVID_MATCH_REG, );
 
reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_set_reg(0, bank, cntr, IOMMU_PC_PASID_MATCH_REG, );
+   amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
+IOMMU_PC_PASID_MATCH_REG, );
 
reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_set_reg(0, bank, cntr, IOMMU_PC_DOMID_MATCH_REG, );
+   amd_iommu_pc_set_reg(hwc->idx, bank, cntr,
+IOMMU_PC_DOMID_MATCH_REG, );
 }
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
+   struct hw_perf_event *hwc = >hw;
u64 reg = 0ULL;
 
-   amd_iommu_pc_set_reg(0, _GET_BANK(event), _GET_CNTR(event),
+   amd_iommu_pc_set_reg(hwc->idx, _GET_BANK(event), _GET_CNTR(event),
 IOMMU_PC_COUNTER_SRC_REG, );
 }
 
@@ -295,7 +294,7 @@ static void perf_iommu_start(struct perf_event *event, int 
flags)
return;
 
val = local64_read(>prev_count) & GENMASK_ULL(48, 0);
-   if (amd_iommu_pc_set_reg(0, _GET_BANK(event), _GET_CNTR(event),
+   if (amd_iommu_pc_set_reg(hwc->idx, _GET_BANK(event), _GET_CNTR(event),
   IOMMU_PC_COUNTER_REG, ))
return;
 
@@ -309,7 +308,7 @@ static void perf_iommu_read(struct perf_event *event)
s64 delta;
struct hw_perf_event *hwc = >hw;
 
-   if