Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1566428 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs Peter Zijlstra <peterz@infradead.org> - 2017-01-25 10:50 +0100
    Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs Borislav Petkov <bp@alien8.de> - 2017-01-25 11:00 +0100

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

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 = &ev->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);

Please explain about this IOMMU crud, this looks like fail.

hwc->idx should be the counter, not a random pmu index.

But instead it looks like you get the counter form:

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

Which is absolutely insane.

[toc] | [next] | [standalone]

#1566437

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web