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

Groups > linux.kernel > #1336801

RE: [patch 04/11] x86/perf/intel_uncore: Cleanup hardware on exit

Show all headers | View raw

> 
> > > @@ -201,6 +201,11 @@ static void nhmex_uncore_msr_init_box(st
> > >  	wrmsrl(NHMEX_U_MSR_PMON_GLOBAL_CTL,
> > > NHMEX_U_PMON_GLOBAL_EN_ALL);  }
> 
> > > +static void nhmex_uncore_msr_exit_box(struct intel_uncore_box
> *box)
> > > {
> > > +	wrmsrl(NHMEX_U_MSR_PMON_GLOBAL_CTL, 0); }
> 
> The reset value for this register is 0. So how is that wrong?
> 
> > > +static void snb_uncore_msr_exit_box(struct intel_uncore_box *box)
> {
> > > +	if (box->pmu->pmu_idx == 0)
> > > +		wrmsrl(SNB_UNC_PERF_GLOBAL_CTL, 0); }
> 
> Ditto.
> 
> > > +static void snb_uncore_imc_exit_box(struct intel_uncore_box *box) {
> > > +	iounmap(box->io_addr);
> 
> That's definitely required, because it would leak a mapping.
> 
> I know Intel folks do not care about error handling and a few reference
> leaks, but I care very much.
> 
> If there is a single instance of exit_box() in that patch which flips the wrong
> bits, then please point it out with the proper reference in the manual and
> not with such half baken statements as above.
> 

Sorry, I didn't make it clear.
For the older server platforms like nhmex and client platforms like snb, I agree
with you on nhmex_uncore_msr_exit_box and snb_uncore_imc_exit_box.

However, for newer server platforms (start from IVB server), we cannot write 0
to rsv bit of BOX control registers. The behavior is undefined.
The following codes may have issues.
It looks we also write 0 to rsv bit in box_init. We may need to fix it.

You can find all the server uncore documents here.
https://software.intel.com/en-us/blogs/2014/07/11/documentation-for-uncore-performance-monitoring-units

> +static void snbep_uncore_pci_exit_box(struct intel_uncore_box *box) {
> +	struct pci_dev *pdev = box->pci_dev;
> +	int box_ctl = uncore_pci_box_ctl(box);
> +
> +	pci_write_config_dword(pdev, box_ctl, 0); }
> +

> +static void snbep_uncore_msr_exit_box(struct intel_uncore_box *box) {
> +	unsigned msr = uncore_msr_box_ctl(box);
> +
> +	if (msr)
> +		wrmsrl(msr, 0);
> +}

> +static void ivbep_uncore_msr_exit_box(struct intel_uncore_box *box) {
> +	unsigned msr = uncore_msr_box_ctl(box);
> +
> +	if (msr)
> +		wrmsrl(msr, 0);
> +}

> +static void ivbep_uncore_pci_exit_box(struct intel_uncore_box *box) {
> +	struct pci_dev *pdev = box->pci_dev;
> +
> +	pci_write_config_dword(pdev, SNBEP_PCI_PMON_BOX_CTL, 0); }
> +

> +static void hswep_uncore_sbox_msr_exit_box(struct intel_uncore_box
> +*box) {
> +	unsigned msr = uncore_msr_box_ctl(box);
> +
> +	/* CHECKME: Does this need the bit dance like init() ? */
> +	if (msr)
> +		wrmsrl(msr, 0);
> +}
> +

Thanks,

Kan

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[patch 00/11] x86/perf/intel_uncore: Cleanup and enhancements Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 14:50 +0100
  [patch 05/11] x86/perf/intel_uncore: Make code readable Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 01/11] x86/perf/intel_uncore: Remove pointless mask check Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 02/11] x86/perf/intel_uncore: Simplify error rollback Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 09/11] x86/perf/intel_uncore: Make PCI and MSR uncore  independent Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 06/11] x86/topology: Provide helper to retrieve number of cpu  packages Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 03/11] x86/perf/intel_uncore: Fix error handling Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 11/11] x86/perf/intel_uncore: Make it modular Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 10/11] cpumask: Export cpumask_any_but Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
  [patch 04/11] x86/perf/intel_uncore: Cleanup hardware on exit Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 15:00 +0100
    RE: [patch 04/11] x86/perf/intel_uncore: Cleanup hardware on exit "Liang, Kan" <kan.liang@intel.com> - 2016-02-17 16:50 +0100
      RE: [patch 04/11] x86/perf/intel_uncore: Cleanup hardware on exit Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 19:20 +0100
        RE: [patch 04/11] x86/perf/intel_uncore: Cleanup hardware on exit "Liang, Kan" <kan.liang@intel.com> - 2016-02-17 23:00 +0100
          RE: [patch 04/11] x86/perf/intel_uncore: Cleanup hardware on exit Thomas Gleixner <tglx@linutronix.de> - 2016-02-17 23:10 +0100

csiph-web