linux-kernel - Re: [PATCH 2/3] mfd: Intel Platform Monitoring Technology support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 5 May 2020 12:02:54 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     "David E. Box" <david.e.box@...ux.intel.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Andy Shevchenko <andy@...radead.org>,
        alexander.h.duyck@...el.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-pci@...r.kernel.org,
        Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Subject: Re: [PATCH 2/3] mfd: Intel Platform Monitoring Technology support

On Tue, May 5, 2020 at 5:32 AM David E. Box <david.e.box@...ux.intel.com> wrote:
>
> Intel Platform Monitoring Technology (PMT) is an architecture for
> enumerating and accessing hardware monitoring facilities. PMT supports
> multiple types of monitoring capabilities. Capabilities are discovered
> using PCIe DVSEC with the Intel VID. Each capability is discovered as a
> separate DVSEC instance in a device's config space. This driver uses MFD to
> manage the creation of platform devices for each type so that they may be
> controlled by their own drivers (to be introduced).  Support is included
> for the 3 current capability types, Telemetry, Watcher, and Crashlog. The
> features are available on new Intel platforms starting from Tiger Lake for
> which support is added. Tiger Lake however will not support Watcher and
> Crashlog even though the capabilities appear on the device. So add a quirk
> facility and use it to disable them.

...

>  include/linux/intel-dvsec.h |  44 +++++++++

I guess it's no go for a such header, since we may end up with tons of
a such. Perhaps simple pcie-dvsec.h ?

...

> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8783,6 +8783,11 @@ S:       Maintained
>  F:     arch/x86/include/asm/intel_telemetry.h
>  F:     drivers/platform/x86/intel_telemetry*
>
> +INTEL PMT DRIVER
> +M:     "David E. Box" <david.e.box@...ux.intel.com>
> +S:     Maintained
> +F:     drivers/mfd/intel_pmt.c

I believe you forgot to run parse-maintainers.pl --order
--input=MAINTAINERS --output=MAINTAINERS

...

> +       info = devm_kmemdup(&pdev->dev, (void *)id->driver_data, sizeof(*info),
> +                           GFP_KERNEL);

> +

Extra blank line.

> +       if (!info)
> +               return -ENOMEM;
> +
> +       while ((pos = pci_find_next_ext_capability(pdev, pos, PCI_EXT_CAP_ID_DVSEC))) {
> +               pci_read_config_word(pdev, pos + PCI_DVSEC_HEADER1, &vid);
> +               if (vid != PCI_VENDOR_ID_INTEL)
> +                       continue;

Perhaps a candidate for for_each_vendor_cap() macro in pcie-dvsec.h.
Or how is it done for the rest of capabilities?

> +       }

...

> +static const struct pci_device_id pmt_pci_ids[] = {
> +       /* TGL */

> +       { PCI_VDEVICE(INTEL, 0x9a0d), (kernel_ulong_t)&tgl_info },

PCI_DEVICE_DATA()?

> +       { }
> +};

-- 
With Best Regards,
Andy Shevchenko