diff mbox

Info: mapping multiple BARs. Your kernel is fine.

Message ID 532AA3F6.9090402@intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Yan, Zheng March 20, 2014, 8:16 a.m. UTC 
On 03/20/2014 03:53 PM, Zhang, Rui wrote:
> The resource length is also hardcoded to 0x6000, right?
> This is probably a problem, because
> only if the resource length read from PCI config space is larger than 0x4000,
> drivers/pnp/quirks.c will detect the conflict and disable the PNP0C02
> resource 0xfed10000 - 0xfed13fff, and the PCI device can request this
> resource successfully.
> In order to check this, can you please attach the dmesg output after boot?

maybe the issue can be fixed by below untested patch

---
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Zhang Rui March 20, 2014, 1:43 p.m. UTC | #1 
On Thu, 2014-03-20 at 16:16 +0800, Yan, Zheng wrote:
> On 03/20/2014 03:53 PM, Zhang, Rui wrote:
> > The resource length is also hardcoded to 0x6000, right?
> > This is probably a problem, because
> > only if the resource length read from PCI config space is larger than 0x4000,
> > drivers/pnp/quirks.c will detect the conflict and disable the PNP0C02
> > resource 0xfed10000 - 0xfed13fff, and the PCI device can request this
> > resource successfully.
> > In order to check this, can you please attach the dmesg output after boot?
> 
> maybe the issue can be fixed by below untested patch
> 
> ---
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index fd5e883..2b3d834 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -1701,7 +1701,7 @@ static struct uncore_event_desc snb_uncore_imc_events[] = {
>  #define SNB_UNCORE_PCI_IMC_BAR_OFFSET		0x48
>  
>  /* page size multiple covering all config regs */
> -#define SNB_UNCORE_PCI_IMC_MAP_SIZE		0x6000
> +#define SNB_UNCORE_PCI_IMC_MAP_SIZE		0x8
>  
>  #define SNB_UNCORE_PCI_IMC_DATA_READS		0x1
>  #define SNB_UNCORE_PCI_IMC_DATA_READS_BASE	0x5050
> @@ -1736,7 +1736,8 @@ static void snb_uncore_imc_init_box(struct intel_uncore_box *box)
>  
>  	addr &= ~(PAGE_SIZE - 1);
>  
> -	box->io_addr = ioremap(addr, SNB_UNCORE_PCI_IMC_MAP_SIZE);
> +	box->io_addr = ioremap(addr + SNB_UNCORE_PCI_IMC_CTR_BASE,
> +			       SNB_UNCORE_PCI_IMC_MAP_SIZE);

you're remapping 0xfed15050 - 0xfed1b04f instead of 0xfed10000 -
0xfed15fff ?
I do not quite understand this, but apparently this is not a FIX.
If it works for this problem, it is because 0xfed15050 - 0xfed1b04f
happens to be not conflict with any resource reserved by PNP system
driver, on this machine.

thanks,
rui

>  	box->hrtimer_duration = UNCORE_SNB_IMC_HRTIMER_INTERVAL;
>  }
>  
> @@ -1832,7 +1833,7 @@ static int snb_uncore_imc_event_init(struct perf_event *event)
>  	}
>  
>  	/* must be done before validate_group */
> -	event->hw.event_base = base;
> +	event->hw.event_base = base - SNB_UNCORE_PCI_IMC_CTR_BASE;
>  	event->hw.config = cfg;
>  	event->hw.idx = idx;
>  


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephane Eranian March 20, 2014, 4:03 p.m. UTC | #2 
On Thu, Mar 20, 2014 at 9:16 AM, Yan, Zheng <zheng.z.yan@intel.com> wrote:
> On 03/20/2014 03:53 PM, Zhang, Rui wrote:
>> The resource length is also hardcoded to 0x6000, right?
>> This is probably a problem, because
>> only if the resource length read from PCI config space is larger than 0x4000,
>> drivers/pnp/quirks.c will detect the conflict and disable the PNP0C02
>> resource 0xfed10000 - 0xfed13fff, and the PCI device can request this
>> resource successfully.
>> In order to check this, can you please attach the dmesg output after boot?
>
> maybe the issue can be fixed by below untested patch
>
> ---
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index fd5e883..2b3d834 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -1701,7 +1701,7 @@ static struct uncore_event_desc snb_uncore_imc_events[] = {
>  #define SNB_UNCORE_PCI_IMC_BAR_OFFSET          0x48
>
>  /* page size multiple covering all config regs */
> -#define SNB_UNCORE_PCI_IMC_MAP_SIZE            0x6000
> +#define SNB_UNCORE_PCI_IMC_MAP_SIZE            0x8
>
I assume ioremap() works on page boundaries.
Eventually want to expose the other counters too, not just read and
writes ( 8 bytes total).

The size of 0x6000 comes from the counter offsets: BAR + 0x5040 to BAR + 0x5054.
May be a better way of doing this would be to remap just the one page
holding them
instead of the 6 covering the entire BAR + counters. That would need
changes in the
read_counter() but that is okay.

So that would something along the line of:

    addr = (addr + 0x5040) & (PAGE_SIZE - 1);
    ioremap(addr, 0x1000);


>  #define SNB_UNCORE_PCI_IMC_DATA_READS          0x1
>  #define SNB_UNCORE_PCI_IMC_DATA_READS_BASE     0x5050
> @@ -1736,7 +1736,8 @@ static void snb_uncore_imc_init_box(struct intel_uncore_box *box)
>
>         addr &= ~(PAGE_SIZE - 1);
>
> -       box->io_addr = ioremap(addr, SNB_UNCORE_PCI_IMC_MAP_SIZE);
> +       box->io_addr = ioremap(addr + SNB_UNCORE_PCI_IMC_CTR_BASE,
> +                              SNB_UNCORE_PCI_IMC_MAP_SIZE);
>         box->hrtimer_duration = UNCORE_SNB_IMC_HRTIMER_INTERVAL;
>  }
>
> @@ -1832,7 +1833,7 @@ static int snb_uncore_imc_event_init(struct perf_event *event)
>         }
>
>         /* must be done before validate_group */
> -       event->hw.event_base = base;
> +       event->hw.event_base = base - SNB_UNCORE_PCI_IMC_CTR_BASE;
>         event->hw.config = cfg;
>         event->hw.idx = idx;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index fd5e883..2b3d834 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -1701,7 +1701,7 @@  static struct uncore_event_desc snb_uncore_imc_events[] = {
 #define SNB_UNCORE_PCI_IMC_BAR_OFFSET		0x48
 
 /* page size multiple covering all config regs */
-#define SNB_UNCORE_PCI_IMC_MAP_SIZE		0x6000
+#define SNB_UNCORE_PCI_IMC_MAP_SIZE		0x8
 
 #define SNB_UNCORE_PCI_IMC_DATA_READS		0x1
 #define SNB_UNCORE_PCI_IMC_DATA_READS_BASE	0x5050
@@ -1736,7 +1736,8 @@  static void snb_uncore_imc_init_box(struct intel_uncore_box *box)
 
 	addr &= ~(PAGE_SIZE - 1);
 
-	box->io_addr = ioremap(addr, SNB_UNCORE_PCI_IMC_MAP_SIZE);
+	box->io_addr = ioremap(addr + SNB_UNCORE_PCI_IMC_CTR_BASE,
+			       SNB_UNCORE_PCI_IMC_MAP_SIZE);
 	box->hrtimer_duration = UNCORE_SNB_IMC_HRTIMER_INTERVAL;
 }
 
@@ -1832,7 +1833,7 @@  static int snb_uncore_imc_event_init(struct perf_event *event)
 	}
 
 	/* must be done before validate_group */
-	event->hw.event_base = base;
+	event->hw.event_base = base - SNB_UNCORE_PCI_IMC_CTR_BASE;
 	event->hw.config = cfg;
 	event->hw.idx = idx;