Re: [PATCH V2 1/5] perf/x86/intel/uncore: Parse uncore discovery tables

From: Lucas De Marchi
Date: Tue Aug 02 2022 - 12:03:15 EST

On Tue, Aug 02, 2022 at 11:43:36AM -0400, Liang, Kan wrote:


On 2022-08-02 10:22 a.m., Lucas De Marchi wrote:
On Mon, Jul 25, 2022 at 10:51:44AM -0400, Liang, Kan wrote:


On 2022-07-23 2:56 p.m., Lucas De Marchi wrote:
On Fri, Jul 22, 2022 at 09:04:43AM -0400, Liang, Kan wrote:


On 2022-07-22 8:55 a.m., Lucas De Marchi wrote:
Hi Kan,

On Wed, Mar 17, 2021 at 10:59:33AM -0700, kan.liang@xxxxxxxxxxxxxxx
wrote:
From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>

A self-describing mechanism for the uncore PerfMon hardware has been
introduced with the latest Intel platforms. By reading through an
MMIO
page worth of information, perf can 'discover' all the standard
uncore
PerfMon registers in a machine.

The discovery mechanism relies on BIOS's support. With a proper BIOS,
a PCI device with the unique capability ID 0x23 can be found on each
die. Perf can retrieve the information of all available uncore
PerfMons
from the device via MMIO. The information is composed of one global
discovery table and several unit discovery tables.
- The global discovery table includes global uncore information of
the
 die, e.g., the address of the global control register, the offset of
 the global status register, the number of uncore units, the
offset of
 unit discovery tables, etc.
- The unit discovery table includes generic uncore unit information,
 e.g., the access type, the counter width, the address of counters,
 the address of the counter control, the unit ID, the unit type, etc.
 The unit is also called "box" in the code.
Perf can provide basic uncore support based on this information
with the following patches.

To locate the PCI device with the discovery tables, check the generic
PCI ID first. If it doesn't match, go through the entire PCI device
tree
and locate the device with the unique capability ID.

The uncore information is similar among dies. To save parsing time
and
space, only completely parse and store the discovery tables on the
first
die and the first box of each die. The parsed information is
stored in
an
RB tree structure, intel_uncore_discovery_type. The size of the
stored
discovery tables varies among platforms. It's around 4KB for a
Sapphire
Rapids server.

If a BIOS doesn't support the 'discovery' mechanism, the uncore
driver
will exit with -ENODEV. There is nothing changed.

Add a module parameter to disable the discovery feature. If a BIOS
gets
the discovery tables wrong, users can have an option to disable the
feature. For the current patchset, the uncore driver will exit with
-ENODEV. In the future, it may fall back to the hardcode uncore
driver
on a known platform.

Signed-off-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>

I observed one issue when upgrading a kernel from 5.10 to 5.15 and
after
bisecting it arrived to this commit. I also verified the same issue is
present in 5.19-rc7 and that the issue is gone when booting with
intel_uncore.uncore_no_discover.

Test system is a SPR host with a PVC gpu. Issue is that PVC is not
reaching pkg c6 state, even if we put it in rc6 state. It seems the
pcie
link is not idling, preventing it to go to pkg c6.

PMON discovery in bios is set to "auto".

We do see the following on dmesg while going through this code path:

    intel_uncore: Invalid Global Discovery State: 0xffffffffffffffff
0xffffffffffffffff 0xffffffffffffffff

On SPR, the uncore driver relies on the discovery table provided by the
BIOS/firmware. It looks like your BIOS/firmware is out of date. Could
you please update to the latest BIOS/firmware and have a try?

hum, the BIOS is up to date. It seems PVC itself has a 0x09a7 device
and it remains in D3, so the 0xffffffffffffffff we se below is
just the auto completion. No wonder the values don't match what we are
expecting here.

Is it expected the device to be in D0? Or should we do anything here to
move it to D0 before doing these reads?


It's OK to have a 0x09a7 device. But the device should not claim to
support the PMON Discovery if it doesn't comply the PMON discovery
mechanism.

See 1.10.1 Guidance on Finding PMON Discovery and Reading it in SPR
uncore document. https://cdrdv2.intel.com/v1/dl/getContent/642245
It demonstrates how the uncore driver find the device with the PMON
discovery mechanism.

ok, this is exactly the code in the kernel.


Simply speaking, the uncore driver looks for a DVSEC
structure with an unique capability ID 0x23. Then it checks whether the
PMON discovery entry (0x1) is supported. If both are detected, it means
that the device comply the PMON discovery mechanism. The uncore driver
will be enabled to parse the discovery table.

AFAIK, the PVC gpu doesn't support the PMON discovery mechanism. I guess
the firmwire of the PVC gpu mistakenly set the PMON discovery entry
(0x1). You may want to check the extended capabilities (DVSEC) in the
PCIe configuration space of the PVC gpu device.

However here it seems we have 2 issues being mixed:

1) PVC with that capability when it shouldn't

This is a firmware/HW issue. If PVC doesn't support the PMON discovery
mechanism, the PVC and its attached OOBMSM device should not enumerate
the discovery mechanism. However, the PVC enumerates the discovery
mechanism here, which doesn't comply the spec.

The uncore driver prints errors when the in-compliance is detected.
That's expected. There is noting more SW can do here.

The firmware issue must be fixed.

yes, that's what I said. It's exposing the capability when it shouldn't.
That's being worked on from the firmware side already.


2) Trying to read the MMIOs when device is possibly in D3 state:

The uncore driver skips the device which doesn't support the discovery
mechanism.
If 1) is fixed, the uncore driver will not touch the MMIO space of a PVC
device. The power issue should be gone.

I've already sent you a patch to ignore the PVC added OOBMSM device, you
can double check with the patch.

(2) is a more generic issue that I'm mentioning. Forget for a moment we
are talking about PVC - that will be fixed by (1). We are trying to read
the mmio from a device that can be in D3, either because it started in
D3 or because a driver, loaded before intel_uncore, moved it to that
state. That won't work even if the device supports the discovery
mechanism.

Lucas De Marchi


Thanks,
Kan


    /* Map whole discovery table */
    addr = pci_dword & ~(PAGE_SIZE - 1);
    io_addr = ioremap(addr, UNCORE_DISCOVERY_MAP_SIZE);

    /* Read Global Discovery table */
    memcpy_fromio(&global, io_addr, sizeof(struct
uncore_global_discovery));

Unless it's guaranteed that at this point the device must be in D0
state, this doesn't look right.  When we are binding a driver to a PCI
device, pci core will move it to D0 for us:

    static long local_pci_probe(void *_ddi)
    {
        ...
        /*
         * Unbound PCI devices are always put in D0, regardless of
         * runtime PM status.  During probe, the device is set to
         * active and the usage count is incremented.  If the driver
         * supports runtime PM, it should call pm_runtime_put_noidle(),
         * or any other runtime PM helper function decrementing the usage
         * count, in its probe routine and pm_runtime_get_noresume() in
         * its remove routine.
         */
         pm_runtime_get_sync(dev);
         ...

But here we are traversing the entire PCI device tree by ourselves.
Considering intel_uncore is a module that can be loaded at any time
(even after the driver supporting PVC, which already called
pm_runtime_put_noidle(), it looks like we are missing the pm integration
here.

On a quick hack, just forcing the device into D0 before doing the MMIO,
the PM issue is gone (but we still hit the problem of PVC having the cap
when it shouldn't)

thanks
Lucas De Marchi


Thanks,
Kan