* [PATCH V4 01/17] platform/x86/intel/vsec: Move structures to header
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-18 23:16 ` [PATCH V4 02/17] platform/x86/intel/vsec: remove platform_info from vsec device structure David E. Box
` (15 subsequent siblings)
16 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
In preparation for exporting an API to register Intel Vendor Specific
Extended Capabilities (VSEC) from other drivers, move needed structures to
the header file.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - No change
V3 - No change
V2 - New patch splitting previous PATCH 1
drivers/platform/x86/intel/vsec.c | 35 ------------------------------
drivers/platform/x86/intel/vsec.h | 36 +++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index c1f9e4471b28..e82a009be630 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -24,13 +24,6 @@
#include "vsec.h"
-/* Intel DVSEC offsets */
-#define INTEL_DVSEC_ENTRIES 0xA
-#define INTEL_DVSEC_SIZE 0xB
-#define INTEL_DVSEC_TABLE 0xC
-#define INTEL_DVSEC_TABLE_BAR(x) ((x) & GENMASK(2, 0))
-#define INTEL_DVSEC_TABLE_OFFSET(x) ((x) & GENMASK(31, 3))
-#define TABLE_OFFSET_SHIFT 3
#define PMT_XA_START 0
#define PMT_XA_MAX INT_MAX
#define PMT_XA_LIMIT XA_LIMIT(PMT_XA_START, PMT_XA_MAX)
@@ -39,34 +32,6 @@ static DEFINE_IDA(intel_vsec_ida);
static DEFINE_IDA(intel_vsec_sdsi_ida);
static DEFINE_XARRAY_ALLOC(auxdev_array);
-/**
- * struct intel_vsec_header - Common fields of Intel VSEC and DVSEC registers.
- * @rev: Revision ID of the VSEC/DVSEC register space
- * @length: Length of the VSEC/DVSEC register space
- * @id: ID of the feature
- * @num_entries: Number of instances of the feature
- * @entry_size: Size of the discovery table for each feature
- * @tbir: BAR containing the discovery tables
- * @offset: BAR offset of start of the first discovery table
- */
-struct intel_vsec_header {
- u8 rev;
- u16 length;
- u16 id;
- u8 num_entries;
- u8 entry_size;
- u8 tbir;
- u32 offset;
-};
-
-enum intel_vsec_id {
- VSEC_ID_TELEMETRY = 2,
- VSEC_ID_WATCHER = 3,
- VSEC_ID_CRASHLOG = 4,
- VSEC_ID_SDSI = 65,
- VSEC_ID_TPMI = 66,
-};
-
static const char *intel_vsec_name(enum intel_vsec_id id)
{
switch (id) {
diff --git a/drivers/platform/x86/intel/vsec.h b/drivers/platform/x86/intel/vsec.h
index 0fd042c171ba..8900cb95afd3 100644
--- a/drivers/platform/x86/intel/vsec.h
+++ b/drivers/platform/x86/intel/vsec.h
@@ -11,9 +11,45 @@
#define VSEC_CAP_SDSI BIT(3)
#define VSEC_CAP_TPMI BIT(4)
+/* Intel DVSEC offsets */
+#define INTEL_DVSEC_ENTRIES 0xA
+#define INTEL_DVSEC_SIZE 0xB
+#define INTEL_DVSEC_TABLE 0xC
+#define INTEL_DVSEC_TABLE_BAR(x) ((x) & GENMASK(2, 0))
+#define INTEL_DVSEC_TABLE_OFFSET(x) ((x) & GENMASK(31, 3))
+#define TABLE_OFFSET_SHIFT 3
+
struct pci_dev;
struct resource;
+enum intel_vsec_id {
+ VSEC_ID_TELEMETRY = 2,
+ VSEC_ID_WATCHER = 3,
+ VSEC_ID_CRASHLOG = 4,
+ VSEC_ID_SDSI = 65,
+ VSEC_ID_TPMI = 66,
+};
+
+/**
+ * struct intel_vsec_header - Common fields of Intel VSEC and DVSEC registers.
+ * @rev: Revision ID of the VSEC/DVSEC register space
+ * @length: Length of the VSEC/DVSEC register space
+ * @id: ID of the feature
+ * @num_entries: Number of instances of the feature
+ * @entry_size: Size of the discovery table for each feature
+ * @tbir: BAR containing the discovery tables
+ * @offset: BAR offset of start of the first discovery table
+ */
+struct intel_vsec_header {
+ u8 rev;
+ u16 length;
+ u16 id;
+ u8 num_entries;
+ u8 entry_size;
+ u8 tbir;
+ u32 offset;
+};
+
enum intel_vsec_quirks {
/* Watcher feature not supported */
VSEC_QUIRK_NO_WATCHER = BIT(0),
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V4 02/17] platform/x86/intel/vsec: remove platform_info from vsec device structure
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
2023-10-18 23:16 ` [PATCH V4 01/17] platform/x86/intel/vsec: Move structures to header David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-18 23:16 ` [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h David E. Box
` (14 subsequent siblings)
16 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
In preparation for exporting an API to register Intel Vendor Specific
Extended Capabilities (VSEC) from other drivers, remove the pointer to
platform_info from intel_vsec_device. This prevents a potential page fault
when auxiliary drivers probe and attempt to dereference this pointer to
access the needed quirks field. Instead, just add the quirks to
intel_vsec_device.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - No change
V3 - No change
V2 - New patch splitting previous PATCH 1
drivers/platform/x86/intel/pmt/class.c | 2 +-
drivers/platform/x86/intel/vsec.c | 2 +-
drivers/platform/x86/intel/vsec.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index f32a233470de..2ad91d2fd954 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -31,7 +31,7 @@ bool intel_pmt_is_early_client_hw(struct device *dev)
* differences from the server platforms (which use the Out Of Band
* Management Services Module OOBMSM).
*/
- return !!(ivdev->info->quirks & VSEC_QUIRK_EARLY_HW);
+ return !!(ivdev->quirks & VSEC_QUIRK_EARLY_HW);
}
EXPORT_SYMBOL_NS_GPL(intel_pmt_is_early_client_hw, INTEL_PMT);
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index e82a009be630..b14eba545770 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -193,7 +193,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
intel_vsec_dev->pcidev = pdev;
intel_vsec_dev->resource = res;
intel_vsec_dev->num_resources = header->num_entries;
- intel_vsec_dev->info = info;
+ intel_vsec_dev->quirks = info->quirks;
if (header->id == VSEC_ID_SDSI)
intel_vsec_dev->ida = &intel_vsec_sdsi_ida;
diff --git a/drivers/platform/x86/intel/vsec.h b/drivers/platform/x86/intel/vsec.h
index 8900cb95afd3..d3fefba3e623 100644
--- a/drivers/platform/x86/intel/vsec.h
+++ b/drivers/platform/x86/intel/vsec.h
@@ -79,10 +79,10 @@ struct intel_vsec_device {
struct pci_dev *pcidev;
struct resource *resource;
struct ida *ida;
- struct intel_vsec_platform_info *info;
int num_resources;
void *priv_data;
size_t priv_data_size;
+ unsigned long quirks;
};
int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
2023-10-18 23:16 ` [PATCH V4 01/17] platform/x86/intel/vsec: Move structures to header David E. Box
2023-10-18 23:16 ` [PATCH V4 02/17] platform/x86/intel/vsec: remove platform_info from vsec device structure David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-19 1:03 ` kernel test robot
2023-10-23 15:15 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 04/17] platform/x86/intel/vsec: Add intel_vsec_register David E. Box
` (13 subsequent siblings)
16 siblings, 2 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
Use cleanup.h helpers to handle cleanup of resources in
intel_vsec_add_dev() after failures.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - Do no_free_ptr() before and in call to intel_vsec_add_aux().
- Add resource cleanup in this patch.
V3 - New patch.
drivers/platform/x86/intel/vsec.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index b14eba545770..28191313d515 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -15,6 +15,7 @@
#include <linux/auxiliary_bus.h>
#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/idr.h>
@@ -147,10 +148,11 @@ EXPORT_SYMBOL_NS_GPL(intel_vsec_add_aux, INTEL_VSEC);
static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *header,
struct intel_vsec_platform_info *info)
{
- struct intel_vsec_device *intel_vsec_dev;
- struct resource *res, *tmp;
+ struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL;
+ struct resource __free(kfree) *res = NULL;
+ struct resource *tmp;
unsigned long quirks = info->quirks;
- int i;
+ int ret, i;
if (!intel_vsec_supported(header->id, info->caps))
return -EINVAL;
@@ -170,10 +172,8 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
return -ENOMEM;
res = kcalloc(header->num_entries, sizeof(*res), GFP_KERNEL);
- if (!res) {
- kfree(intel_vsec_dev);
+ if (!res)
return -ENOMEM;
- }
if (quirks & VSEC_QUIRK_TABLE_SHIFT)
header->offset >>= TABLE_OFFSET_SHIFT;
@@ -200,8 +200,15 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
else
intel_vsec_dev->ida = &intel_vsec_ida;
- return intel_vsec_add_aux(pdev, NULL, intel_vsec_dev,
- intel_vsec_name(header->id));
+ /*
+ * Pass the ownership of intel_vsec_dev and resource within it to
+ * intel_vsec_add_aux()
+ */
+ no_free_ptr(res);
+ ret = intel_vsec_add_aux(pdev, NULL, no_free_ptr(intel_vsec_dev),
+ intel_vsec_name(header->id));
+
+ return ret;
}
static bool intel_vsec_walk_header(struct pci_dev *pdev,
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h
2023-10-18 23:16 ` [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h David E. Box
@ 2023-10-19 1:03 ` kernel test robot
2023-10-23 15:15 ` Ilpo Järvinen
1 sibling, 0 replies; 33+ messages in thread
From: kernel test robot @ 2023-10-19 1:03 UTC (permalink / raw)
To: David E. Box, linux-kernel, platform-driver-x86, ilpo.jarvinen,
rajvi.jingar
Cc: oe-kbuild-all
Hi David,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 3f720b21ec5af466e50e99dc517af267b67d248c]
url: https://github.com/intel-lab-lkp/linux/commits/David-E-Box/platform-x86-intel-vsec-Move-structures-to-header/20231019-071914
base: 3f720b21ec5af466e50e99dc517af267b67d248c
patch link: https://lore.kernel.org/r/20231018231624.1044633-4-david.e.box%40linux.intel.com
patch subject: [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h
reproduce: (https://download.01.org/0day-ci/archive/20231019/202310190857.Vc2mlWmN-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190857.Vc2mlWmN-lkp@intel.com/
# many are suggestions rather than must-fix
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#32: FILE: drivers/platform/x86/intel/vsec.c:151:
+ struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL;
^
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#33: FILE: drivers/platform/x86/intel/vsec.c:152:
+ struct resource __free(kfree) *res = NULL;
^
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h
2023-10-18 23:16 ` [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h David E. Box
2023-10-19 1:03 ` kernel test robot
@ 2023-10-23 15:15 ` Ilpo Järvinen
1 sibling, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 15:15 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]
On Wed, 18 Oct 2023, David E. Box wrote:
> Use cleanup.h helpers to handle cleanup of resources in
> intel_vsec_add_dev() after failures.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - Do no_free_ptr() before and in call to intel_vsec_add_aux().
> - Add resource cleanup in this patch.
>
> V3 - New patch.
>
> drivers/platform/x86/intel/vsec.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
> index b14eba545770..28191313d515 100644
> --- a/drivers/platform/x86/intel/vsec.c
> +++ b/drivers/platform/x86/intel/vsec.c
> @@ -15,6 +15,7 @@
>
> #include <linux/auxiliary_bus.h>
> #include <linux/bits.h>
> +#include <linux/cleanup.h>
> #include <linux/delay.h>
> #include <linux/kernel.h>
> #include <linux/idr.h>
> @@ -147,10 +148,11 @@ EXPORT_SYMBOL_NS_GPL(intel_vsec_add_aux, INTEL_VSEC);
> static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *header,
> struct intel_vsec_platform_info *info)
> {
> - struct intel_vsec_device *intel_vsec_dev;
> - struct resource *res, *tmp;
> + struct intel_vsec_device __free(kfree) *intel_vsec_dev = NULL;
> + struct resource __free(kfree) *res = NULL;
> + struct resource *tmp;
> unsigned long quirks = info->quirks;
> - int i;
> + int ret, i;
>
> if (!intel_vsec_supported(header->id, info->caps))
> return -EINVAL;
> @@ -170,10 +172,8 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
> return -ENOMEM;
>
> res = kcalloc(header->num_entries, sizeof(*res), GFP_KERNEL);
> - if (!res) {
> - kfree(intel_vsec_dev);
> + if (!res)
> return -ENOMEM;
> - }
>
> if (quirks & VSEC_QUIRK_TABLE_SHIFT)
> header->offset >>= TABLE_OFFSET_SHIFT;
> @@ -200,8 +200,15 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
> else
> intel_vsec_dev->ida = &intel_vsec_ida;
>
> - return intel_vsec_add_aux(pdev, NULL, intel_vsec_dev,
> - intel_vsec_name(header->id));
> + /*
> + * Pass the ownership of intel_vsec_dev and resource within it to
> + * intel_vsec_add_aux()
> + */
> + no_free_ptr(res);
> + ret = intel_vsec_add_aux(pdev, NULL, no_free_ptr(intel_vsec_dev),
> + intel_vsec_name(header->id));
> +
> + return ret;
This looks good other than you don't need to add the ret variable at all.
After changing that, please add:
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 04/17] platform/x86/intel/vsec: Add intel_vsec_register
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (2 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 03/17] platform/x86/intel/vsec: Use cleanup.h David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 15:21 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 05/17] platform/x86/intel/vsec: Add base address field David E. Box
` (12 subsequent siblings)
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
From: Gayatri Kammela <gayatri.kammela@linux.intel.com>
Add and export intel_vsec_register() to allow the registration of Intel
extended capabilities from other drivers. Add check to look for memory
conflicts before registering a new capability. Add a parent field to
intel_vsec_platform_info to allow specifying the parent device for
device managed cleanup.
Signed-off-by: Gayatri Kammela <gayatri.kammela@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - Move res cleanup to previous patch
V3 - Replace kfree on request_mem_region fail with use of cleanup.h helper.
V2 - New patch splitting previous PATCH 1
drivers/platform/x86/intel/vsec.c | 19 +++++++++++++++++--
drivers/platform/x86/intel/vsec.h | 4 ++++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 28191313d515..638dfde6a9e2 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -188,6 +188,12 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
header->offset + i * (header->entry_size * sizeof(u32));
tmp->end = tmp->start + (header->entry_size * sizeof(u32)) - 1;
tmp->flags = IORESOURCE_MEM;
+
+ /* Check resource is not in use */
+ if (!request_mem_region(tmp->start, resource_size(tmp), ""))
+ return -EBUSY;
+
+ release_mem_region(tmp->start, resource_size(tmp));
}
intel_vsec_dev->pcidev = pdev;
@@ -205,9 +211,8 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
* intel_vsec_add_aux()
*/
no_free_ptr(res);
- ret = intel_vsec_add_aux(pdev, NULL, no_free_ptr(intel_vsec_dev),
+ ret = intel_vsec_add_aux(pdev, info->parent, no_free_ptr(intel_vsec_dev),
intel_vsec_name(header->id));
-
return ret;
}
@@ -325,6 +330,16 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
return have_devices;
}
+void intel_vsec_register(struct pci_dev *pdev,
+ struct intel_vsec_platform_info *info)
+{
+ if (!pdev || !info)
+ return;
+
+ intel_vsec_walk_header(pdev, info);
+}
+EXPORT_SYMBOL_NS_GPL(intel_vsec_register, INTEL_VSEC);
+
static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct intel_vsec_platform_info *info;
diff --git a/drivers/platform/x86/intel/vsec.h b/drivers/platform/x86/intel/vsec.h
index d3fefba3e623..a15fda2fcd28 100644
--- a/drivers/platform/x86/intel/vsec.h
+++ b/drivers/platform/x86/intel/vsec.h
@@ -69,6 +69,7 @@ enum intel_vsec_quirks {
/* Platform specific data */
struct intel_vsec_platform_info {
+ struct device *parent;
struct intel_vsec_header **headers;
unsigned long caps;
unsigned long quirks;
@@ -98,4 +99,7 @@ static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device
{
return container_of(auxdev, struct intel_vsec_device, auxdev);
}
+
+void intel_vsec_register(struct pci_dev *pdev,
+ struct intel_vsec_platform_info *info);
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 04/17] platform/x86/intel/vsec: Add intel_vsec_register
2023-10-18 23:16 ` [PATCH V4 04/17] platform/x86/intel/vsec: Add intel_vsec_register David E. Box
@ 2023-10-23 15:21 ` Ilpo Järvinen
2023-10-24 23:25 ` David E. Box
0 siblings, 1 reply; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 15:21 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
On Wed, 18 Oct 2023, David E. Box wrote:
Use () in the shortlog after the function name.
> From: Gayatri Kammela <gayatri.kammela@linux.intel.com>
>
> Add and export intel_vsec_register() to allow the registration of Intel
> extended capabilities from other drivers. Add check to look for memory
> conflicts before registering a new capability. Add a parent field to
> intel_vsec_platform_info to allow specifying the parent device for
> device managed cleanup.
While reviewing this patch, I couldn't understand why the parent is never
assigned with anything?
> Signed-off-by: Gayatri Kammela <gayatri.kammela@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - Move res cleanup to previous patch
>
> V3 - Replace kfree on request_mem_region fail with use of cleanup.h helper.
>
> V2 - New patch splitting previous PATCH 1
>
> drivers/platform/x86/intel/vsec.c | 19 +++++++++++++++++--
> drivers/platform/x86/intel/vsec.h | 4 ++++
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
> index 28191313d515..638dfde6a9e2 100644
> --- a/drivers/platform/x86/intel/vsec.c
> +++ b/drivers/platform/x86/intel/vsec.c
> @@ -188,6 +188,12 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
> header->offset + i * (header->entry_size * sizeof(u32));
> tmp->end = tmp->start + (header->entry_size * sizeof(u32)) - 1;
> tmp->flags = IORESOURCE_MEM;
> +
> + /* Check resource is not in use */
> + if (!request_mem_region(tmp->start, resource_size(tmp), ""))
> + return -EBUSY;
> +
> + release_mem_region(tmp->start, resource_size(tmp));
> }
>
> intel_vsec_dev->pcidev = pdev;
> @@ -205,9 +211,8 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
> * intel_vsec_add_aux()
> */
> no_free_ptr(res);
> - ret = intel_vsec_add_aux(pdev, NULL, no_free_ptr(intel_vsec_dev),
> + ret = intel_vsec_add_aux(pdev, info->parent, no_free_ptr(intel_vsec_dev),
> intel_vsec_name(header->id));
> -
Extra newline change.
--
i.
> return ret;
> }
>
> @@ -325,6 +330,16 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
> return have_devices;
> }
>
> +void intel_vsec_register(struct pci_dev *pdev,
> + struct intel_vsec_platform_info *info)
> +{
> + if (!pdev || !info)
> + return;
> +
> + intel_vsec_walk_header(pdev, info);
> +}
> +EXPORT_SYMBOL_NS_GPL(intel_vsec_register, INTEL_VSEC);
> +
> static int intel_vsec_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> struct intel_vsec_platform_info *info;
> diff --git a/drivers/platform/x86/intel/vsec.h b/drivers/platform/x86/intel/vsec.h
> index d3fefba3e623..a15fda2fcd28 100644
> --- a/drivers/platform/x86/intel/vsec.h
> +++ b/drivers/platform/x86/intel/vsec.h
> @@ -69,6 +69,7 @@ enum intel_vsec_quirks {
>
> /* Platform specific data */
> struct intel_vsec_platform_info {
> + struct device *parent;
> struct intel_vsec_header **headers;
> unsigned long caps;
> unsigned long quirks;
> @@ -98,4 +99,7 @@ static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device
> {
> return container_of(auxdev, struct intel_vsec_device, auxdev);
> }
> +
> +void intel_vsec_register(struct pci_dev *pdev,
> + struct intel_vsec_platform_info *info);
> #endif
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH V4 04/17] platform/x86/intel/vsec: Add intel_vsec_register
2023-10-23 15:21 ` Ilpo Järvinen
@ 2023-10-24 23:25 ` David E. Box
0 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-24 23:25 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: LKML, platform-driver-x86, rajvi.jingar
On Mon, 2023-10-23 at 18:21 +0300, Ilpo Järvinen wrote:
> On Wed, 18 Oct 2023, David E. Box wrote:
>
> Use () in the shortlog after the function name.
>
> > From: Gayatri Kammela <gayatri.kammela@linux.intel.com>
> >
> > Add and export intel_vsec_register() to allow the registration of Intel
> > extended capabilities from other drivers. Add check to look for memory
> > conflicts before registering a new capability. Add a parent field to
> > intel_vsec_platform_info to allow specifying the parent device for
> > device managed cleanup.
>
> While reviewing this patch, I couldn't understand why the parent is never
> assigned with anything?
That's because ...
>
> > Signed-off-by: Gayatri Kammela <gayatri.kammela@linux.intel.com>
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > ---
> > V4 - Move res cleanup to previous patch
> >
> > V3 - Replace kfree on request_mem_region fail with use of cleanup.h helper.
> >
> > V2 - New patch splitting previous PATCH 1
> >
> > drivers/platform/x86/intel/vsec.c | 19 +++++++++++++++++--
> > drivers/platform/x86/intel/vsec.h | 4 ++++
> > 2 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel/vsec.c
> > b/drivers/platform/x86/intel/vsec.c
> > index 28191313d515..638dfde6a9e2 100644
> > --- a/drivers/platform/x86/intel/vsec.c
> > +++ b/drivers/platform/x86/intel/vsec.c
> > @@ -188,6 +188,12 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
> > struct intel_vsec_header *he
> > header->offset + i * (header->entry_size *
> > sizeof(u32));
> > tmp->end = tmp->start + (header->entry_size * sizeof(u32)) -
> > 1;
> > tmp->flags = IORESOURCE_MEM;
> > +
> > + /* Check resource is not in use */
> > + if (!request_mem_region(tmp->start, resource_size(tmp), ""))
> > + return -EBUSY;
> > +
> > + release_mem_region(tmp->start, resource_size(tmp));
> > }
> >
> > intel_vsec_dev->pcidev = pdev;
> > @@ -205,9 +211,8 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
> > struct intel_vsec_header *he
> > * intel_vsec_add_aux()
> > */
> > no_free_ptr(res);
> > - ret = intel_vsec_add_aux(pdev, NULL, no_free_ptr(intel_vsec_dev),
> > + ret = intel_vsec_add_aux(pdev, info->parent,
> > no_free_ptr(intel_vsec_dev),
> > intel_vsec_name(header->id));
... for devices probed by this driver, intel_vsec_add_aux() will assign the
probe device as the parent if the argument is NULL. Originally this function
didn't have an argument for parent. It was added when this function was exported
for use by the tpmi driver which needed to specify its own parent. Users of
intel_vsec_register() need to do the same. I suppose for clarity this driver
could set it as well and we can remove the NULL check.
David
> > -
>
> Extra newline change.
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 05/17] platform/x86/intel/vsec: Add base address field
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (3 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 04/17] platform/x86/intel/vsec: Add intel_vsec_register David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-18 23:16 ` [PATCH V4 06/17] platform/x86/intel/pmt: Add header to struct intel_pmt_entry David E. Box
` (11 subsequent siblings)
16 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
Some devices may emulate PCI VSEC capabilities in MMIO. In such cases the
BAR is not readable from a config space. Provide a field for drivers to
indicate the base address to be used.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - No change
V3 - No change
V2 - No change
drivers/platform/x86/intel/pmt/class.c | 14 +++++++++++---
drivers/platform/x86/intel/vsec.c | 10 ++++++++--
drivers/platform/x86/intel/vsec.h | 2 ++
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 2ad91d2fd954..32608baaa56c 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -160,10 +160,11 @@ static struct class intel_pmt_class = {
static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
struct intel_pmt_header *header,
- struct device *dev,
+ struct intel_vsec_device *ivdev,
struct resource *disc_res)
{
- struct pci_dev *pci_dev = to_pci_dev(dev->parent);
+ struct pci_dev *pci_dev = ivdev->pcidev;
+ struct device *dev = &ivdev->auxdev.dev;
u8 bir;
/*
@@ -215,6 +216,13 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
break;
case ACCESS_BARID:
+ /* Use the provided base address if it exists */
+ if (ivdev->base_addr) {
+ entry->base_addr = ivdev->base_addr +
+ GET_ADDRESS(header->base_offset);
+ break;
+ }
+
/*
* If another BAR was specified then the base offset
* represents the offset within that BAR. SO retrieve the
@@ -319,7 +327,7 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
if (ret)
return ret;
- ret = intel_pmt_populate_entry(entry, &header, dev, disc_res);
+ ret = intel_pmt_populate_entry(entry, &header, intel_vsec_dev, disc_res);
if (ret)
return ret;
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 638dfde6a9e2..1e19bfe8e87e 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -152,6 +152,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
struct resource __free(kfree) *res = NULL;
struct resource *tmp;
unsigned long quirks = info->quirks;
+ u64 base_addr;
int ret, i;
if (!intel_vsec_supported(header->id, info->caps))
@@ -178,14 +179,18 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
if (quirks & VSEC_QUIRK_TABLE_SHIFT)
header->offset >>= TABLE_OFFSET_SHIFT;
+ if (info->base_addr)
+ base_addr = info->base_addr;
+ else
+ base_addr = pdev->resource[header->tbir].start;
+
/*
* The DVSEC/VSEC contains the starting offset and count for a block of
* discovery tables. Create a resource array of these tables to the
* auxiliary device driver.
*/
for (i = 0, tmp = res; i < header->num_entries; i++, tmp++) {
- tmp->start = pdev->resource[header->tbir].start +
- header->offset + i * (header->entry_size * sizeof(u32));
+ tmp->start = base_addr + header->offset + i * (header->entry_size * sizeof(u32));
tmp->end = tmp->start + (header->entry_size * sizeof(u32)) - 1;
tmp->flags = IORESOURCE_MEM;
@@ -200,6 +205,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
intel_vsec_dev->resource = res;
intel_vsec_dev->num_resources = header->num_entries;
intel_vsec_dev->quirks = info->quirks;
+ intel_vsec_dev->base_addr = info->base_addr;
if (header->id == VSEC_ID_SDSI)
intel_vsec_dev->ida = &intel_vsec_sdsi_ida;
diff --git a/drivers/platform/x86/intel/vsec.h b/drivers/platform/x86/intel/vsec.h
index a15fda2fcd28..78848e2329fb 100644
--- a/drivers/platform/x86/intel/vsec.h
+++ b/drivers/platform/x86/intel/vsec.h
@@ -73,6 +73,7 @@ struct intel_vsec_platform_info {
struct intel_vsec_header **headers;
unsigned long caps;
unsigned long quirks;
+ u64 base_addr;
};
struct intel_vsec_device {
@@ -84,6 +85,7 @@ struct intel_vsec_device {
void *priv_data;
size_t priv_data_size;
unsigned long quirks;
+ u64 base_addr;
};
int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V4 06/17] platform/x86/intel/pmt: Add header to struct intel_pmt_entry
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (4 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 05/17] platform/x86/intel/vsec: Add base address field David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 15:14 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 07/17] platform/x86/intel/pmt: telemetry: Export API to read telemetry David E. Box
` (10 subsequent siblings)
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
The PMT header is passed to several functions. Instead, store the header in
struct intel_pmt_entry which is also passed to these functions and shorten
the argument list. This simplifies the calls in preparation for later
changes.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - No change
V3 - No change
V2 - No change
drivers/platform/x86/intel/pmt/class.c | 8 +++-----
drivers/platform/x86/intel/pmt/class.h | 16 ++++++++--------
drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
drivers/platform/x86/intel/pmt/telemetry.c | 2 +-
4 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 32608baaa56c..142a24e3727d 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -159,12 +159,12 @@ static struct class intel_pmt_class = {
};
static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
- struct intel_pmt_header *header,
struct intel_vsec_device *ivdev,
struct resource *disc_res)
{
struct pci_dev *pci_dev = ivdev->pcidev;
struct device *dev = &ivdev->auxdev.dev;
+ struct intel_pmt_header *header = &entry->header;
u8 bir;
/*
@@ -313,7 +313,6 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
struct intel_vsec_device *intel_vsec_dev, int idx)
{
struct device *dev = &intel_vsec_dev->auxdev.dev;
- struct intel_pmt_header header;
struct resource *disc_res;
int ret;
@@ -323,16 +322,15 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
if (IS_ERR(entry->disc_table))
return PTR_ERR(entry->disc_table);
- ret = ns->pmt_header_decode(entry, &header, dev);
+ ret = ns->pmt_header_decode(entry, dev);
if (ret)
return ret;
- ret = intel_pmt_populate_entry(entry, &header, intel_vsec_dev, disc_res);
+ ret = intel_pmt_populate_entry(entry, intel_vsec_dev, disc_res);
if (ret)
return ret;
return intel_pmt_dev_register(entry, ns, dev);
-
}
EXPORT_SYMBOL_NS_GPL(intel_pmt_dev_create, INTEL_PMT);
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index db11d58867ce..e477a19f6700 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -18,7 +18,15 @@
#define GET_BIR(v) ((v) & GENMASK(2, 0))
#define GET_ADDRESS(v) ((v) & GENMASK(31, 3))
+struct intel_pmt_header {
+ u32 base_offset;
+ u32 size;
+ u32 guid;
+ u8 access_type;
+};
+
struct intel_pmt_entry {
+ struct intel_pmt_header header;
struct bin_attribute pmt_bin_attr;
struct kobject *kobj;
void __iomem *disc_table;
@@ -29,19 +37,11 @@ struct intel_pmt_entry {
int devid;
};
-struct intel_pmt_header {
- u32 base_offset;
- u32 size;
- u32 guid;
- u8 access_type;
-};
-
struct intel_pmt_namespace {
const char *name;
struct xarray *xa;
const struct attribute_group *attr_grp;
int (*pmt_header_decode)(struct intel_pmt_entry *entry,
- struct intel_pmt_header *header,
struct device *dev);
};
diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index bbb3d61d09f4..4014c02cafdb 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -223,10 +223,10 @@ static const struct attribute_group pmt_crashlog_group = {
};
static int pmt_crashlog_header_decode(struct intel_pmt_entry *entry,
- struct intel_pmt_header *header,
struct device *dev)
{
void __iomem *disc_table = entry->disc_table;
+ struct intel_pmt_header *header = &entry->header;
struct crashlog_entry *crashlog;
if (!pmt_crashlog_supported(entry))
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index 39cbc87cc28a..f86080e8bebd 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -58,10 +58,10 @@ static bool pmt_telem_region_overlaps(struct intel_pmt_entry *entry,
}
static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
- struct intel_pmt_header *header,
struct device *dev)
{
void __iomem *disc_table = entry->disc_table;
+ struct intel_pmt_header *header = &entry->header;
if (pmt_telem_region_overlaps(entry, dev))
return 1;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 06/17] platform/x86/intel/pmt: Add header to struct intel_pmt_entry
2023-10-18 23:16 ` [PATCH V4 06/17] platform/x86/intel/pmt: Add header to struct intel_pmt_entry David E. Box
@ 2023-10-23 15:14 ` Ilpo Järvinen
0 siblings, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 15:14 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
[-- Attachment #1: Type: text/plain, Size: 4829 bytes --]
On Wed, 18 Oct 2023, David E. Box wrote:
> The PMT header is passed to several functions. Instead, store the header in
> struct intel_pmt_entry which is also passed to these functions and shorten
> the argument list. This simplifies the calls in preparation for later
> changes.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> V4 - No change
>
> V3 - No change
>
> V2 - No change
>
> drivers/platform/x86/intel/pmt/class.c | 8 +++-----
> drivers/platform/x86/intel/pmt/class.h | 16 ++++++++--------
> drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
> drivers/platform/x86/intel/pmt/telemetry.c | 2 +-
> 4 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index 32608baaa56c..142a24e3727d 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -159,12 +159,12 @@ static struct class intel_pmt_class = {
> };
>
> static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> - struct intel_pmt_header *header,
> struct intel_vsec_device *ivdev,
> struct resource *disc_res)
> {
> struct pci_dev *pci_dev = ivdev->pcidev;
> struct device *dev = &ivdev->auxdev.dev;
> + struct intel_pmt_header *header = &entry->header;
> u8 bir;
>
> /*
> @@ -313,7 +313,6 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
> struct intel_vsec_device *intel_vsec_dev, int idx)
> {
> struct device *dev = &intel_vsec_dev->auxdev.dev;
> - struct intel_pmt_header header;
> struct resource *disc_res;
> int ret;
>
> @@ -323,16 +322,15 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
> if (IS_ERR(entry->disc_table))
> return PTR_ERR(entry->disc_table);
>
> - ret = ns->pmt_header_decode(entry, &header, dev);
> + ret = ns->pmt_header_decode(entry, dev);
> if (ret)
> return ret;
>
> - ret = intel_pmt_populate_entry(entry, &header, intel_vsec_dev, disc_res);
> + ret = intel_pmt_populate_entry(entry, intel_vsec_dev, disc_res);
> if (ret)
> return ret;
>
> return intel_pmt_dev_register(entry, ns, dev);
> -
Newline change that doesn't belong to this patch.
--
i.
> }
> EXPORT_SYMBOL_NS_GPL(intel_pmt_dev_create, INTEL_PMT);
>
> diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> index db11d58867ce..e477a19f6700 100644
> --- a/drivers/platform/x86/intel/pmt/class.h
> +++ b/drivers/platform/x86/intel/pmt/class.h
> @@ -18,7 +18,15 @@
> #define GET_BIR(v) ((v) & GENMASK(2, 0))
> #define GET_ADDRESS(v) ((v) & GENMASK(31, 3))
>
> +struct intel_pmt_header {
> + u32 base_offset;
> + u32 size;
> + u32 guid;
> + u8 access_type;
> +};
> +
> struct intel_pmt_entry {
> + struct intel_pmt_header header;
> struct bin_attribute pmt_bin_attr;
> struct kobject *kobj;
> void __iomem *disc_table;
> @@ -29,19 +37,11 @@ struct intel_pmt_entry {
> int devid;
> };
>
> -struct intel_pmt_header {
> - u32 base_offset;
> - u32 size;
> - u32 guid;
> - u8 access_type;
> -};
> -
> struct intel_pmt_namespace {
> const char *name;
> struct xarray *xa;
> const struct attribute_group *attr_grp;
> int (*pmt_header_decode)(struct intel_pmt_entry *entry,
> - struct intel_pmt_header *header,
> struct device *dev);
> };
>
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index bbb3d61d09f4..4014c02cafdb 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -223,10 +223,10 @@ static const struct attribute_group pmt_crashlog_group = {
> };
>
> static int pmt_crashlog_header_decode(struct intel_pmt_entry *entry,
> - struct intel_pmt_header *header,
> struct device *dev)
> {
> void __iomem *disc_table = entry->disc_table;
> + struct intel_pmt_header *header = &entry->header;
> struct crashlog_entry *crashlog;
>
> if (!pmt_crashlog_supported(entry))
> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
> index 39cbc87cc28a..f86080e8bebd 100644
> --- a/drivers/platform/x86/intel/pmt/telemetry.c
> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> @@ -58,10 +58,10 @@ static bool pmt_telem_region_overlaps(struct intel_pmt_entry *entry,
> }
>
> static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
> - struct intel_pmt_header *header,
> struct device *dev)
> {
> void __iomem *disc_table = entry->disc_table;
> + struct intel_pmt_header *header = &entry->header;
>
> if (pmt_telem_region_overlaps(entry, dev))
> return 1;
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 07/17] platform/x86/intel/pmt: telemetry: Export API to read telemetry
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (5 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 06/17] platform/x86/intel/pmt: Add header to struct intel_pmt_entry David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-18 23:16 ` [PATCH V4 08/17] platform/x86:intel/pmc: Call pmc_get_low_power_modes from platform init David E. Box
` (9 subsequent siblings)
16 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
Export symbols to allow access to Intel PMT Telemetry data on available
devices. Provides APIs to search, register, and read telemetry using a
kref managed pointer that serves as a handle to a telemetry endpoint.
To simplify searching for present devices, have the IDA start at 1
instead of 0 so that 0 can be used to indicate end of search.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - No change
V3 - Add comment for mutex lock
- Fix multiline comments
- Fix line lengths
- Fix space/tab inconsistences in header doc
V2 - Add explanation of PMT_XA_START change in changelog
- change return and argument type of pmt_telem_get_next_endpoint() to
unsigned long
- style fixes
drivers/platform/x86/intel/pmt/class.c | 21 ++-
drivers/platform/x86/intel/pmt/class.h | 14 ++
drivers/platform/x86/intel/pmt/telemetry.c | 191 ++++++++++++++++++++-
drivers/platform/x86/intel/pmt/telemetry.h | 126 ++++++++++++++
4 files changed, 344 insertions(+), 8 deletions(-)
create mode 100644 drivers/platform/x86/intel/pmt/telemetry.h
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 142a24e3727d..4b53940a64e2 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -17,7 +17,7 @@
#include "../vsec.h"
#include "class.h"
-#define PMT_XA_START 0
+#define PMT_XA_START 1
#define PMT_XA_MAX INT_MAX
#define PMT_XA_LIMIT XA_LIMIT(PMT_XA_START, PMT_XA_MAX)
#define GUID_SPR_PUNIT 0x9956f43f
@@ -247,6 +247,7 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
struct intel_pmt_namespace *ns,
struct device *parent)
{
+ struct intel_vsec_device *ivdev = dev_to_ivdev(parent);
struct resource res = {0};
struct device *dev;
int ret;
@@ -270,7 +271,7 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
if (ns->attr_grp) {
ret = sysfs_create_group(entry->kobj, ns->attr_grp);
if (ret)
- goto fail_sysfs;
+ goto fail_sysfs_create_group;
}
/* if size is 0 assume no data buffer, so no file needed */
@@ -295,13 +296,23 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
entry->pmt_bin_attr.size = entry->size;
ret = sysfs_create_bin_file(&dev->kobj, &entry->pmt_bin_attr);
- if (!ret)
- return 0;
+ if (ret)
+ goto fail_ioremap;
+ if (ns->pmt_add_endpoint) {
+ ret = ns->pmt_add_endpoint(entry, ivdev->pcidev);
+ if (ret)
+ goto fail_add_endpoint;
+ }
+
+ return 0;
+
+fail_add_endpoint:
+ sysfs_remove_bin_file(entry->kobj, &entry->pmt_bin_attr);
fail_ioremap:
if (ns->attr_grp)
sysfs_remove_group(entry->kobj, ns->attr_grp);
-fail_sysfs:
+fail_sysfs_create_group:
device_unregister(dev);
fail_dev_create:
xa_erase(ns->xa, entry->devid);
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index e477a19f6700..d23c63b73ab7 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -9,6 +9,7 @@
#include <linux/io.h>
#include "../vsec.h"
+#include "telemetry.h"
/* PMT access types */
#define ACCESS_BARID 2
@@ -18,6 +19,16 @@
#define GET_BIR(v) ((v) & GENMASK(2, 0))
#define GET_ADDRESS(v) ((v) & GENMASK(31, 3))
+struct pci_dev;
+
+struct telem_endpoint {
+ struct pci_dev *pcidev;
+ struct telem_header header;
+ void __iomem *base;
+ bool present;
+ struct kref kref;
+};
+
struct intel_pmt_header {
u32 base_offset;
u32 size;
@@ -26,6 +37,7 @@ struct intel_pmt_header {
};
struct intel_pmt_entry {
+ struct telem_endpoint *ep;
struct intel_pmt_header header;
struct bin_attribute pmt_bin_attr;
struct kobject *kobj;
@@ -43,6 +55,8 @@ struct intel_pmt_namespace {
const struct attribute_group *attr_grp;
int (*pmt_header_decode)(struct intel_pmt_entry *entry,
struct device *dev);
+ int (*pmt_add_endpoint)(struct intel_pmt_entry *entry,
+ struct pci_dev *pdev);
};
bool intel_pmt_is_early_client_hw(struct device *dev);
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index f86080e8bebd..09258564dfc4 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -30,6 +30,15 @@
/* Used by client hardware to identify a fixed telemetry entry*/
#define TELEM_CLIENT_FIXED_BLOCK_GUID 0x10000000
+#define NUM_BYTES_QWORD(v) ((v) << 3)
+#define SAMPLE_ID_OFFSET(v) ((v) << 3)
+
+#define NUM_BYTES_DWORD(v) ((v) << 2)
+#define SAMPLE_ID_OFFSET32(v) ((v) << 2)
+
+/* Protects access to the xarray of telemetry endpoint handles */
+static DEFINE_MUTEX(ep_lock);
+
enum telem_type {
TELEM_TYPE_PUNIT = 0,
TELEM_TYPE_CRASHLOG,
@@ -84,21 +93,195 @@ static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
return 0;
}
+static int pmt_telem_add_endpoint(struct intel_pmt_entry *entry,
+ struct pci_dev *pdev)
+{
+ struct telem_endpoint *ep;
+
+ /* Endpoint lifetimes are managed by kref, not devres */
+ entry->ep = kzalloc(sizeof(*(entry->ep)), GFP_KERNEL);
+ if (!entry->ep)
+ return -ENOMEM;
+
+ ep = entry->ep;
+ ep->pcidev = pdev;
+ ep->header.access_type = entry->header.access_type;
+ ep->header.guid = entry->header.guid;
+ ep->header.base_offset = entry->header.base_offset;
+ ep->header.size = entry->header.size;
+ ep->base = entry->base;
+ ep->present = true;
+
+ kref_init(&ep->kref);
+
+ return 0;
+}
+
static DEFINE_XARRAY_ALLOC(telem_array);
static struct intel_pmt_namespace pmt_telem_ns = {
.name = "telem",
.xa = &telem_array,
.pmt_header_decode = pmt_telem_header_decode,
+ .pmt_add_endpoint = pmt_telem_add_endpoint,
};
+/* Called when all users unregister and the device is removed */
+static void pmt_telem_ep_release(struct kref *kref)
+{
+ struct telem_endpoint *ep;
+
+ ep = container_of(kref, struct telem_endpoint, kref);
+ kfree(ep);
+}
+
+unsigned long pmt_telem_get_next_endpoint(unsigned long start)
+{
+ struct intel_pmt_entry *entry;
+ unsigned long found_idx;
+
+ mutex_lock(&ep_lock);
+ xa_for_each_start(&telem_array, found_idx, entry, start) {
+ /*
+ * Return first found index after start.
+ * 0 is not valid id.
+ */
+ if (found_idx > start)
+ break;
+ }
+ mutex_unlock(&ep_lock);
+
+ return found_idx == start ? 0 : found_idx;
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_get_next_endpoint, INTEL_PMT_TELEMETRY);
+
+struct telem_endpoint *pmt_telem_register_endpoint(int devid)
+{
+ struct intel_pmt_entry *entry;
+ unsigned long index = devid;
+
+ mutex_lock(&ep_lock);
+ entry = xa_find(&telem_array, &index, index, XA_PRESENT);
+ if (!entry) {
+ mutex_unlock(&ep_lock);
+ return ERR_PTR(-ENXIO);
+ }
+
+ kref_get(&entry->ep->kref);
+ mutex_unlock(&ep_lock);
+
+ return entry->ep;
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_register_endpoint, INTEL_PMT_TELEMETRY);
+
+void pmt_telem_unregister_endpoint(struct telem_endpoint *ep)
+{
+ kref_put(&ep->kref, pmt_telem_ep_release);
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_unregister_endpoint, INTEL_PMT_TELEMETRY);
+
+int pmt_telem_get_endpoint_info(int devid, struct telem_endpoint_info *info)
+{
+ struct intel_pmt_entry *entry;
+ unsigned long index = devid;
+ int err = 0;
+
+ if (!info)
+ return -EINVAL;
+
+ mutex_lock(&ep_lock);
+ entry = xa_find(&telem_array, &index, index, XA_PRESENT);
+ if (!entry) {
+ err = -ENXIO;
+ goto unlock;
+ }
+
+ info->pdev = entry->ep->pcidev;
+ info->header = entry->ep->header;
+
+unlock:
+ mutex_unlock(&ep_lock);
+ return err;
+
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_get_endpoint_info, INTEL_PMT_TELEMETRY);
+
+int pmt_telem_read(struct telem_endpoint *ep, u32 id, u64 *data, u32 count)
+{
+ u32 offset, size;
+
+ if (!ep->present)
+ return -ENODEV;
+
+ offset = SAMPLE_ID_OFFSET(id);
+ size = ep->header.size;
+
+ if (offset + NUM_BYTES_QWORD(count) > size)
+ return -EINVAL;
+
+ memcpy_fromio(data, ep->base + offset, NUM_BYTES_QWORD(count));
+
+ return ep->present ? 0 : -EPIPE;
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_read, INTEL_PMT_TELEMETRY);
+
+int pmt_telem_read32(struct telem_endpoint *ep, u32 id, u32 *data, u32 count)
+{
+ u32 offset, size;
+
+ if (!ep->present)
+ return -ENODEV;
+
+ offset = SAMPLE_ID_OFFSET32(id);
+ size = ep->header.size;
+
+ if (offset + NUM_BYTES_DWORD(count) > size)
+ return -EINVAL;
+
+ memcpy_fromio(data, ep->base + offset, NUM_BYTES_DWORD(count));
+
+ return ep->present ? 0 : -EPIPE;
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_read32, INTEL_PMT_TELEMETRY);
+
+struct telem_endpoint *
+pmt_telem_find_and_register_endpoint(struct pci_dev *pcidev, u32 guid, u16 pos)
+{
+ int devid = 0;
+ int inst = 0;
+ int err = 0;
+
+ while ((devid = pmt_telem_get_next_endpoint(devid))) {
+ struct telem_endpoint_info ep_info;
+
+ err = pmt_telem_get_endpoint_info(devid, &ep_info);
+ if (err)
+ return ERR_PTR(err);
+
+ if (ep_info.header.guid == guid && ep_info.pdev == pcidev) {
+ if (inst == pos)
+ return pmt_telem_register_endpoint(devid);
+ ++inst;
+ }
+ }
+
+ return ERR_PTR(-ENXIO);
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_find_and_register_endpoint, INTEL_PMT_TELEMETRY);
+
static void pmt_telem_remove(struct auxiliary_device *auxdev)
{
struct pmt_telem_priv *priv = auxiliary_get_drvdata(auxdev);
int i;
- for (i = 0; i < priv->num_entries; i++)
- intel_pmt_dev_destroy(&priv->entry[i], &pmt_telem_ns);
-}
+ mutex_lock(&ep_lock);
+ for (i = 0; i < priv->num_entries; i++) {
+ struct intel_pmt_entry *entry = &priv->entry[i];
+
+ kref_put(&entry->ep->kref, pmt_telem_ep_release);
+ intel_pmt_dev_destroy(entry, &pmt_telem_ns);
+ }
+ mutex_unlock(&ep_lock);
+};
static int pmt_telem_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
{
@@ -117,7 +300,9 @@ static int pmt_telem_probe(struct auxiliary_device *auxdev, const struct auxilia
for (i = 0; i < intel_vsec_dev->num_resources; i++) {
struct intel_pmt_entry *entry = &priv->entry[priv->num_entries];
+ mutex_lock(&ep_lock);
ret = intel_pmt_dev_create(entry, &pmt_telem_ns, intel_vsec_dev, i);
+ mutex_unlock(&ep_lock);
if (ret < 0)
goto abort_probe;
if (ret)
diff --git a/drivers/platform/x86/intel/pmt/telemetry.h b/drivers/platform/x86/intel/pmt/telemetry.h
new file mode 100644
index 000000000000..d45af5512b4e
--- /dev/null
+++ b/drivers/platform/x86/intel/pmt/telemetry.h
@@ -0,0 +1,126 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TELEMETRY_H
+#define _TELEMETRY_H
+
+/* Telemetry types */
+#define PMT_TELEM_TELEMETRY 0
+#define PMT_TELEM_CRASHLOG 1
+
+struct telem_endpoint;
+struct pci_dev;
+
+struct telem_header {
+ u8 access_type;
+ u16 size;
+ u32 guid;
+ u32 base_offset;
+};
+
+struct telem_endpoint_info {
+ struct pci_dev *pdev;
+ struct telem_header header;
+};
+
+/**
+ * pmt_telem_get_next_endpoint() - Get next device id for a telemetry endpoint
+ * @start: starting devid to look from
+ *
+ * This functions can be used in a while loop predicate to retrieve the devid
+ * of all available telemetry endpoints. Functions pmt_telem_get_next_endpoint()
+ * and pmt_telem_register_endpoint() can be used inside of the loop to examine
+ * endpoint info and register to receive a pointer to the endpoint. The pointer
+ * is then usable in the telemetry read calls to access the telemetry data.
+ *
+ * Return:
+ * * devid - devid of the next present endpoint from start
+ * * 0 - when no more endpoints are present after start
+ */
+unsigned long pmt_telem_get_next_endpoint(unsigned long start);
+
+/**
+ * pmt_telem_register_endpoint() - Register a telemetry endpoint
+ * @devid: device id/handle of the telemetry endpoint
+ *
+ * Increments the kref usage counter for the endpoint.
+ *
+ * Return:
+ * * endpoint - On success returns pointer to the telemetry endpoint
+ * * -ENXIO - telemetry endpoint not found
+ */
+struct telem_endpoint *pmt_telem_register_endpoint(int devid);
+
+/**
+ * pmt_telem_unregister_endpoint() - Unregister a telemetry endpoint
+ * @ep: ep structure to populate.
+ *
+ * Decrements the kref usage counter for the endpoint.
+ */
+void pmt_telem_unregister_endpoint(struct telem_endpoint *ep);
+
+/**
+ * pmt_telem_get_endpoint_info() - Get info for an endpoint from its devid
+ * @devid: device id/handle of the telemetry endpoint
+ * @info: Endpoint info structure to be populated
+ *
+ * Return:
+ * * 0 - Success
+ * * -ENXIO - telemetry endpoint not found for the devid
+ * * -EINVAL - @info is NULL
+ */
+int pmt_telem_get_endpoint_info(int devid, struct telem_endpoint_info *info);
+
+/**
+ * pmt_telem_find_and_register_endpoint() - Get a telemetry endpoint from
+ * pci_dev device, guid and pos
+ * @pdev: PCI device inside the Intel vsec
+ * @guid: GUID of the telemetry space
+ * @pos: Instance of the guid
+ *
+ * Return:
+ * * endpoint - On success returns pointer to the telemetry endpoint
+ * * -ENXIO - telemetry endpoint not found
+ */
+struct telem_endpoint *pmt_telem_find_and_register_endpoint(struct pci_dev *pcidev,
+ u32 guid, u16 pos);
+
+/**
+ * pmt_telem_read() - Read qwords from counter sram using sample id
+ * @ep: Telemetry endpoint to be read
+ * @id: The beginning sample id of the metric(s) to be read
+ * @data: Allocated qword buffer
+ * @count: Number of qwords requested
+ *
+ * Callers must ensure reads are aligned. When the call returns -ENODEV,
+ * the device has been removed and callers should unregister the telemetry
+ * endpoint.
+ *
+ * Return:
+ * * 0 - Success
+ * * -ENODEV - The device is not present.
+ * * -EINVAL - The offset is out bounds
+ * * -EPIPE - The device was removed during the read. Data written
+ * but should be considered invalid.
+ */
+int pmt_telem_read(struct telem_endpoint *ep, u32 id, u64 *data, u32 count);
+
+/**
+ * pmt_telem_read32() - Read qwords from counter sram using sample id
+ * @ep: Telemetry endpoint to be read
+ * @id: The beginning sample id of the metric(s) to be read
+ * @data: Allocated dword buffer
+ * @count: Number of dwords requested
+ *
+ * Callers must ensure reads are aligned. When the call returns -ENODEV,
+ * the device has been removed and callers should unregister the telemetry
+ * endpoint.
+ *
+ * Return:
+ * * 0 - Success
+ * * -ENODEV - The device is not present.
+ * * -EINVAL - The offset is out bounds
+ * * -EPIPE - The device was removed during the read. Data written
+ * but should be considered invalid.
+ */
+int pmt_telem_read32(struct telem_endpoint *ep, u32 id, u32 *data, u32 count);
+
+#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V4 08/17] platform/x86:intel/pmc: Call pmc_get_low_power_modes from platform init
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (6 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 07/17] platform/x86/intel/pmt: telemetry: Export API to read telemetry David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-18 23:16 ` [PATCH V4 09/17] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail David E. Box
` (8 subsequent siblings)
16 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
From: Xi Pardee <xi.pardee@intel.com>
In order to setup a table of low power mode requirements for Meteor Lake,
pmc_core_get_low_power_modes() will need to be run from platform init code
so that the enabled modes are known, allowing the use of the
pmc_for_each_mode helper. Make the function global and call it from the
platform init code.
Signed-off-by: Xi Pardee <xi.pardee@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - No change
V3 - No change
V2 - Added clearer explanation for the change in the changelog
drivers/platform/x86/intel/pmc/adl.c | 2 ++
drivers/platform/x86/intel/pmc/cnp.c | 2 ++
drivers/platform/x86/intel/pmc/core.c | 7 +++----
drivers/platform/x86/intel/pmc/core.h | 1 +
drivers/platform/x86/intel/pmc/icl.c | 10 +++++++++-
drivers/platform/x86/intel/pmc/mtl.c | 4 +++-
drivers/platform/x86/intel/pmc/spt.c | 10 +++++++++-
drivers/platform/x86/intel/pmc/tgl.c | 1 +
8 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c
index 5006008e01be..64c492391ede 100644
--- a/drivers/platform/x86/intel/pmc/adl.c
+++ b/drivers/platform/x86/intel/pmc/adl.c
@@ -319,6 +319,8 @@ int adl_core_init(struct pmc_dev *pmcdev)
if (ret)
return ret;
+ pmc_core_get_low_power_modes(pmcdev);
+
/* Due to a hardware limitation, the GBE LTR blocks PC10
* when a cable is attached. Tell the PMC to ignore it.
*/
diff --git a/drivers/platform/x86/intel/pmc/cnp.c b/drivers/platform/x86/intel/pmc/cnp.c
index 420aaa1d7c76..59298f184d0e 100644
--- a/drivers/platform/x86/intel/pmc/cnp.c
+++ b/drivers/platform/x86/intel/pmc/cnp.c
@@ -214,6 +214,8 @@ int cnp_core_init(struct pmc_dev *pmcdev)
if (ret)
return ret;
+ pmc_core_get_low_power_modes(pmcdev);
+
/* Due to a hardware limitation, the GBE LTR blocks PC10
* when a cable is attached. Tell the PMC to ignore it.
*/
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 84c175b9721a..3894119d61b0 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -966,9 +966,8 @@ static bool pmc_core_pri_verify(u32 lpm_pri, u8 *mode_order)
return true;
}
-static void pmc_core_get_low_power_modes(struct platform_device *pdev)
+void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev)
{
- struct pmc_dev *pmcdev = platform_get_drvdata(pdev);
struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
u8 pri_order[LPM_MAX_NUM_MODES] = LPM_DEFAULT_PRI;
u8 mode_order[LPM_MAX_NUM_MODES];
@@ -1000,7 +999,8 @@ static void pmc_core_get_low_power_modes(struct platform_device *pdev)
for (mode = 0; mode < LPM_MAX_NUM_MODES; mode++)
pri_order[mode_order[mode]] = mode;
else
- dev_warn(&pdev->dev, "Assuming a default substate order for this platform\n");
+ dev_warn(&pmcdev->pdev->dev,
+ "Assuming a default substate order for this platform\n");
/*
* Loop through all modes from lowest to highest priority,
@@ -1250,7 +1250,6 @@ static int pmc_core_probe(struct platform_device *pdev)
}
pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit(primary_pmc);
- pmc_core_get_low_power_modes(pdev);
pmc_core_do_dmi_quirks(primary_pmc);
pmc_core_dbgfs_register(pmcdev);
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index 0729f593c6a7..ccf24e0f5e50 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -490,6 +490,7 @@ extern int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value);
int pmc_core_resume_common(struct pmc_dev *pmcdev);
int get_primary_reg_base(struct pmc *pmc);
+extern void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev);
extern void pmc_core_ssram_init(struct pmc_dev *pmcdev);
diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c
index d08e3174230d..71b0fd6cb7d8 100644
--- a/drivers/platform/x86/intel/pmc/icl.c
+++ b/drivers/platform/x86/intel/pmc/icl.c
@@ -53,7 +53,15 @@ const struct pmc_reg_map icl_reg_map = {
int icl_core_init(struct pmc_dev *pmcdev)
{
struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
+ int ret;
pmc->map = &icl_reg_map;
- return get_primary_reg_base(pmc);
+
+ ret = get_primary_reg_base(pmc);
+ if (ret)
+ return ret;
+
+ pmc_core_get_low_power_modes(pmcdev);
+
+ return ret;
}
diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
index 2204bc666980..c3b5f4fe01d1 100644
--- a/drivers/platform/x86/intel/pmc/mtl.c
+++ b/drivers/platform/x86/intel/pmc/mtl.c
@@ -985,7 +985,7 @@ static int mtl_resume(struct pmc_dev *pmcdev)
int mtl_core_init(struct pmc_dev *pmcdev)
{
struct pmc *pmc = pmcdev->pmcs[PMC_IDX_SOC];
- int ret = 0;
+ int ret;
mtl_d3_fixup();
@@ -1002,6 +1002,8 @@ int mtl_core_init(struct pmc_dev *pmcdev)
return ret;
}
+ pmc_core_get_low_power_modes(pmcdev);
+
/* Due to a hardware limitation, the GBE LTR blocks PC10
* when a cable is attached. Tell the PMC to ignore it.
*/
diff --git a/drivers/platform/x86/intel/pmc/spt.c b/drivers/platform/x86/intel/pmc/spt.c
index 4b6f5cbda16c..ab993a69e33e 100644
--- a/drivers/platform/x86/intel/pmc/spt.c
+++ b/drivers/platform/x86/intel/pmc/spt.c
@@ -137,7 +137,15 @@ const struct pmc_reg_map spt_reg_map = {
int spt_core_init(struct pmc_dev *pmcdev)
{
struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
+ int ret;
pmc->map = &spt_reg_map;
- return get_primary_reg_base(pmc);
+
+ ret = get_primary_reg_base(pmc);
+ if (ret)
+ return ret;
+
+ pmc_core_get_low_power_modes(pmcdev);
+
+ return ret;
}
diff --git a/drivers/platform/x86/intel/pmc/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c
index 2449940102db..d5f1d2223c5a 100644
--- a/drivers/platform/x86/intel/pmc/tgl.c
+++ b/drivers/platform/x86/intel/pmc/tgl.c
@@ -263,6 +263,7 @@ int tgl_core_init(struct pmc_dev *pmcdev)
if (ret)
return ret;
+ pmc_core_get_low_power_modes(pmcdev);
pmc_core_get_tgl_lpm_reqs(pmcdev->pdev);
/* Due to a hardware limitation, the GBE LTR blocks PC10
* when a cable is attached. Tell the PMC to ignore it.
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V4 09/17] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (7 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 08/17] platform/x86:intel/pmc: Call pmc_get_low_power_modes from platform init David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 15:17 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support David E. Box
` (7 subsequent siblings)
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
Currently, if the PMC SSRAM initialization fails, no error is returned and
the only indication is that a PMC device has not been created. Instead,
allow an error to be returned and handled directly by the caller. Don't use
the return value yet. This is in preparation for a future rework of
pmc_core_sram_init().
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - Remove return value check in mtl.c. Proper use of the value would
require returning an error status from pmc_core_add_pmc(). But
the function that calls it will be removed in the next patch so wait
to use it then.
V3 - New patch split from V2 PATCH 9
- Add dev_warn on pmc_core_ssram_init fail
drivers/platform/x86/intel/pmc/core.h | 2 +-
drivers/platform/x86/intel/pmc/core_ssram.c | 21 +++++++++++++--------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index ccf24e0f5e50..edaa70067e41 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -492,7 +492,7 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev);
int get_primary_reg_base(struct pmc *pmc);
extern void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev);
-extern void pmc_core_ssram_init(struct pmc_dev *pmcdev);
+extern int pmc_core_ssram_init(struct pmc_dev *pmcdev);
int spt_core_init(struct pmc_dev *pmcdev);
int cnp_core_init(struct pmc_dev *pmcdev);
diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
index 13fa16f0d52e..815950713e25 100644
--- a/drivers/platform/x86/intel/pmc/core_ssram.c
+++ b/drivers/platform/x86/intel/pmc/core_ssram.c
@@ -35,20 +35,20 @@ static inline u64 get_base(void __iomem *addr, u32 offset)
return lo_hi_readq(addr + offset) & GENMASK_ULL(63, 3);
}
-static void
+static int
pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
const struct pmc_reg_map *reg_map, int pmc_index)
{
struct pmc *pmc = pmcdev->pmcs[pmc_index];
if (!pwrm_base)
- return;
+ return -ENODEV;
/* Memory for primary PMC has been allocated in core.c */
if (!pmc) {
pmc = devm_kzalloc(&pmcdev->pdev->dev, sizeof(*pmc), GFP_KERNEL);
if (!pmc)
- return;
+ return -ENOMEM;
}
pmc->map = reg_map;
@@ -57,10 +57,12 @@ pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
if (!pmc->regbase) {
devm_kfree(&pmcdev->pdev->dev, pmc);
- return;
+ return -ENOMEM;
}
pmcdev->pmcs[pmc_index] = pmc;
+
+ return 0;
}
static void
@@ -96,7 +98,7 @@ pmc_core_ssram_get_pmc(struct pmc_dev *pmcdev, void __iomem *ssram, u32 offset,
iounmap(ssram);
}
-void pmc_core_ssram_init(struct pmc_dev *pmcdev)
+int pmc_core_ssram_init(struct pmc_dev *pmcdev)
{
void __iomem *ssram;
struct pci_dev *pcidev;
@@ -105,7 +107,7 @@ void pmc_core_ssram_init(struct pmc_dev *pmcdev)
pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, 2));
if (!pcidev)
- goto out;
+ return -ENODEV;
ret = pcim_enable_device(pcidev);
if (ret)
@@ -123,11 +125,14 @@ void pmc_core_ssram_init(struct pmc_dev *pmcdev)
pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_PCH_OFFSET, PMC_IDX_PCH);
iounmap(ssram);
-out:
- return;
+
+ return 0;
disable_dev:
+ pmcdev->ssram_pcidev = NULL;
pci_disable_device(pcidev);
release_dev:
pci_dev_put(pcidev);
+
+ return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 09/17] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail
2023-10-18 23:16 ` [PATCH V4 09/17] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail David E. Box
@ 2023-10-23 15:17 ` Ilpo Järvinen
0 siblings, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 15:17 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
[-- Attachment #1: Type: text/plain, Size: 3726 bytes --]
On Wed, 18 Oct 2023, David E. Box wrote:
> Currently, if the PMC SSRAM initialization fails, no error is returned and
> the only indication is that a PMC device has not been created. Instead,
> allow an error to be returned and handled directly by the caller. Don't use
> the return value yet. This is in preparation for a future rework of
> pmc_core_sram_init().
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
> ---
> V4 - Remove return value check in mtl.c. Proper use of the value would
> require returning an error status from pmc_core_add_pmc(). But
> the function that calls it will be removed in the next patch so wait
> to use it then.
>
> V3 - New patch split from V2 PATCH 9
> - Add dev_warn on pmc_core_ssram_init fail
>
> drivers/platform/x86/intel/pmc/core.h | 2 +-
> drivers/platform/x86/intel/pmc/core_ssram.c | 21 +++++++++++++--------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index ccf24e0f5e50..edaa70067e41 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -492,7 +492,7 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev);
> int get_primary_reg_base(struct pmc *pmc);
> extern void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev);
>
> -extern void pmc_core_ssram_init(struct pmc_dev *pmcdev);
> +extern int pmc_core_ssram_init(struct pmc_dev *pmcdev);
>
> int spt_core_init(struct pmc_dev *pmcdev);
> int cnp_core_init(struct pmc_dev *pmcdev);
> diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
> index 13fa16f0d52e..815950713e25 100644
> --- a/drivers/platform/x86/intel/pmc/core_ssram.c
> +++ b/drivers/platform/x86/intel/pmc/core_ssram.c
> @@ -35,20 +35,20 @@ static inline u64 get_base(void __iomem *addr, u32 offset)
> return lo_hi_readq(addr + offset) & GENMASK_ULL(63, 3);
> }
>
> -static void
> +static int
> pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
> const struct pmc_reg_map *reg_map, int pmc_index)
> {
> struct pmc *pmc = pmcdev->pmcs[pmc_index];
>
> if (!pwrm_base)
> - return;
> + return -ENODEV;
>
> /* Memory for primary PMC has been allocated in core.c */
> if (!pmc) {
> pmc = devm_kzalloc(&pmcdev->pdev->dev, sizeof(*pmc), GFP_KERNEL);
> if (!pmc)
> - return;
> + return -ENOMEM;
> }
>
> pmc->map = reg_map;
> @@ -57,10 +57,12 @@ pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
>
> if (!pmc->regbase) {
> devm_kfree(&pmcdev->pdev->dev, pmc);
> - return;
> + return -ENOMEM;
> }
>
> pmcdev->pmcs[pmc_index] = pmc;
> +
> + return 0;
> }
>
> static void
> @@ -96,7 +98,7 @@ pmc_core_ssram_get_pmc(struct pmc_dev *pmcdev, void __iomem *ssram, u32 offset,
> iounmap(ssram);
> }
>
> -void pmc_core_ssram_init(struct pmc_dev *pmcdev)
> +int pmc_core_ssram_init(struct pmc_dev *pmcdev)
> {
> void __iomem *ssram;
> struct pci_dev *pcidev;
> @@ -105,7 +107,7 @@ void pmc_core_ssram_init(struct pmc_dev *pmcdev)
>
> pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, 2));
> if (!pcidev)
> - goto out;
> + return -ENODEV;
>
> ret = pcim_enable_device(pcidev);
> if (ret)
> @@ -123,11 +125,14 @@ void pmc_core_ssram_init(struct pmc_dev *pmcdev)
> pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_PCH_OFFSET, PMC_IDX_PCH);
>
> iounmap(ssram);
> -out:
> - return;
> +
> + return 0;
>
> disable_dev:
> + pmcdev->ssram_pcidev = NULL;
> pci_disable_device(pcidev);
> release_dev:
> pci_dev_put(pcidev);
> +
> + return ret;
> }
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (8 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 09/17] platform/x86/intel/pmc: Allow pmc_core_ssram_init to fail David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-19 2:12 ` kernel test robot
` (2 more replies)
2023-10-18 23:16 ` [PATCH V4 11/17] platform/x86/intel/pmc: Split pmc_core_ssram_get_pmc() David E. Box
` (6 subsequent siblings)
16 siblings, 3 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, dave.hansen, arnd, peterz,
aleksander.lobakin, ilpo.jarvinen, rajvi.jingar
Add auto-release cleanups for ioumap and ioport_unmap.
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - New patch
include/linux/io.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/io.h b/include/linux/io.h
index 7304f2a69960..1488832c4ad2 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -6,6 +6,7 @@
#ifndef _LINUX_IO_H
#define _LINUX_IO_H
+#include <linux/cleanup.h>
#include <linux/types.h>
#include <linux/init.h>
#include <linux/bug.h>
@@ -20,6 +21,9 @@ __visible void __iowrite32_copy(void __iomem *to, const void *from, size_t count
void __ioread32_copy(void *to, const void __iomem *from, size_t count);
void __iowrite64_copy(void __iomem *to, const void *from, size_t count);
+DEFINE_FREE(iounmap, void __iomem *, iounmap(_T));
+DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
+
#ifdef CONFIG_MMU
int ioremap_page_range(unsigned long addr, unsigned long end,
phys_addr_t phys_addr, pgprot_t prot);
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support
2023-10-18 23:16 ` [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support David E. Box
@ 2023-10-19 2:12 ` kernel test robot
2023-10-19 2:23 ` kernel test robot
2023-10-23 15:25 ` Ilpo Järvinen
2 siblings, 0 replies; 33+ messages in thread
From: kernel test robot @ 2023-10-19 2:12 UTC (permalink / raw)
To: David E. Box, linux-kernel, platform-driver-x86, dave.hansen,
arnd, peterz, aleksander.lobakin, ilpo.jarvinen, rajvi.jingar
Cc: llvm, oe-kbuild-all
Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3f720b21ec5af466e50e99dc517af267b67d248c]
url: https://github.com/intel-lab-lkp/linux/commits/David-E-Box/platform-x86-intel-vsec-Move-structures-to-header/20231019-071914
base: 3f720b21ec5af466e50e99dc517af267b67d248c
patch link: https://lore.kernel.org/r/20231018231624.1044633-11-david.e.box%40linux.intel.com
patch subject: [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20231019/202310191031.Nkp72kk7-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310191031.Nkp72kk7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310191031.Nkp72kk7-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from init/main.c:21:
In file included from include/linux/syscalls.h:90:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:9:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from init/main.c:21:
In file included from include/linux/syscalls.h:90:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:9:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from init/main.c:21:
In file included from include/linux/syscalls.h:90:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:9:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from init/main.c:21:
In file included from include/linux/syscalls.h:90:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:9:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
>> include/linux/io.h:25:43: error: call to undeclared function 'ioport_unmap'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^
12 warnings and 1 error generated.
--
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:10:
In file included from include/linux/trace_recursion.h:5:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
>> include/linux/io.h:25:43: error: call to undeclared function 'ioport_unmap'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:13:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:1075:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:11: warning: array index 3 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
97 | return (set->sig[3] | set->sig[2] |
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:13:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:1075:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:97:25: warning: array index 2 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
97 | return (set->sig[3] | set->sig[2] |
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:13:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:1075:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:98:4: warning: array index 1 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
98 | set->sig[1] | set->sig[0]) == 0;
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:13:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:1075:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:100:11: warning: array index 1 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
100 | return (set->sig[1] | set->sig[0]) == 0;
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:13:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:1075:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:113:11: warning: array index 3 is past the end of the array (that has type 'const unsigned long[1]') [-Warray-bounds]
113 | return (set1->sig[3] == set2->sig[3]) &&
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
In file included from include/linux/ftrace.h:13:
In file included from include/linux/kallsyms.h:13:
In file included from include/linux/mm.h:1075:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:113:27: warning: array index 3 is past the end of the array (that has type 'const unsigned long[1]') [-Warray-bounds]
113 | return (set1->sig[3] == set2->sig[3]) &&
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from init/init_task.c:2:
In file included from include/linux/init_task.h:9:
--
In file included from arch/um/kernel/mem.c:8:
In file included from include/linux/memblock.h:13:
In file included from arch/um/include/asm/dma.h:5:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from arch/um/kernel/mem.c:8:
In file included from include/linux/memblock.h:13:
In file included from arch/um/include/asm/dma.h:5:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from arch/um/kernel/mem.c:8:
In file included from include/linux/memblock.h:13:
In file included from arch/um/include/asm/dma.h:5:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from arch/um/kernel/mem.c:9:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
>> include/linux/io.h:25:43: error: call to undeclared function 'ioport_unmap'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^
arch/um/kernel/mem.c:202:8: warning: no previous prototype for function 'pgd_alloc' [-Wmissing-prototypes]
202 | pgd_t *pgd_alloc(struct mm_struct *mm)
| ^
arch/um/kernel/mem.c:202:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
202 | pgd_t *pgd_alloc(struct mm_struct *mm)
| ^
| static
arch/um/kernel/mem.c:215:7: warning: no previous prototype for function 'uml_kmalloc' [-Wmissing-prototypes]
215 | void *uml_kmalloc(int size, int flags)
| ^
arch/um/kernel/mem.c:215:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
215 | void *uml_kmalloc(int size, int flags)
| ^
| static
14 warnings and 1 error generated.
..
vim +/ioport_unmap +25 include/linux/io.h
23
24 DEFINE_FREE(iounmap, void __iomem *, iounmap(_T));
> 25 DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
26
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support
2023-10-18 23:16 ` [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support David E. Box
2023-10-19 2:12 ` kernel test robot
@ 2023-10-19 2:23 ` kernel test robot
2023-10-23 15:25 ` Ilpo Järvinen
2 siblings, 0 replies; 33+ messages in thread
From: kernel test robot @ 2023-10-19 2:23 UTC (permalink / raw)
To: David E. Box, linux-kernel, platform-driver-x86, dave.hansen,
arnd, peterz, aleksander.lobakin, ilpo.jarvinen, rajvi.jingar
Cc: oe-kbuild-all
Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3f720b21ec5af466e50e99dc517af267b67d248c]
url: https://github.com/intel-lab-lkp/linux/commits/David-E-Box/platform-x86-intel-vsec-Move-structures-to-header/20231019-071914
base: 3f720b21ec5af466e50e99dc517af267b67d248c
patch link: https://lore.kernel.org/r/20231018231624.1044633-11-david.e.box%40linux.intel.com
patch subject: [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support
config: um-defconfig (https://download.01.org/0day-ci/archive/20231019/202310191053.t22G0O9p-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310191053.t22G0O9p-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310191053.t22G0O9p-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/preempt.h:11,
from include/linux/spinlock.h:56,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from init/main.c:17:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/irqflags.h:16,
from include/linux/rcupdate.h:26,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from arch/x86/um/syscalls_64.c:8:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
arch/x86/um/syscalls_64.c: At top level:
arch/x86/um/syscalls_64.c:84:6: warning: no previous prototype for 'arch_switch_to' [-Wmissing-prototypes]
84 | void arch_switch_to(struct task_struct *to)
| ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/preempt.h:11,
from include/linux/percpu.h:6,
from include/linux/context_tracking_state.h:5,
from include/linux/hardirq.h:5,
from include/linux/interrupt.h:11,
from kernel/panic.c:14:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
kernel/panic.c: In function '__warn':
kernel/panic.c:666:17: warning: function '__warn' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
666 | vprintk(args->fmt, args->args);
| ^~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/irqflags.h:16,
from include/linux/rcupdate.h:26,
from include/linux/rcuwait.h:5,
from include/linux/irq_work.h:6,
from kernel/irq_work.c:12:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
kernel/irq_work.c: At top level:
kernel/irq_work.c:72:13: warning: no previous prototype for 'arch_irq_work_raise' [-Wmissing-prototypes]
72 | void __weak arch_irq_work_raise(void)
| ^~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/preempt.h:11,
from include/linux/spinlock.h:56,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from arch/um/kernel/mem.c:7:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
arch/um/kernel/mem.c: At top level:
arch/um/kernel/mem.c:202:8: warning: no previous prototype for 'pgd_alloc' [-Wmissing-prototypes]
202 | pgd_t *pgd_alloc(struct mm_struct *mm)
| ^~~~~~~~~
arch/um/kernel/mem.c:215:7: warning: no previous prototype for 'uml_kmalloc' [-Wmissing-prototypes]
215 | void *uml_kmalloc(int size, int flags)
| ^~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/preempt.h:11,
from include/linux/percpu.h:6,
from include/linux/context_tracking_state.h:5,
from include/linux/hardirq.h:5,
from arch/um/kernel/process.c:11:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
arch/um/kernel/process.c: At top level:
arch/um/kernel/process.c:51:5: warning: no previous prototype for 'pid_to_processor_id' [-Wmissing-prototypes]
51 | int pid_to_processor_id(int pid)
| ^~~~~~~~~~~~~~~~~~~
arch/um/kernel/process.c:87:7: warning: no previous prototype for '__switch_to' [-Wmissing-prototypes]
87 | void *__switch_to(struct task_struct *from, struct task_struct *to)
| ^~~~~~~~~~~
arch/um/kernel/process.c: In function 'new_thread_handler':
arch/um/kernel/process.c:122:28: warning: variable 'n' set but not used [-Wunused-but-set-variable]
122 | int (*fn)(void *), n;
| ^
arch/um/kernel/process.c: At top level:
arch/um/kernel/process.c:140:6: warning: no previous prototype for 'fork_handler' [-Wmissing-prototypes]
140 | void fork_handler(void)
| ^~~~~~~~~~~~
arch/um/kernel/process.c:217:6: warning: no previous prototype for 'arch_cpu_idle' [-Wmissing-prototypes]
217 | void arch_cpu_idle(void)
| ^~~~~~~~~~~~~
arch/um/kernel/process.c:253:5: warning: no previous prototype for 'copy_to_user_proc' [-Wmissing-prototypes]
253 | int copy_to_user_proc(void __user *to, void *from, int size)
| ^~~~~~~~~~~~~~~~~
arch/um/kernel/process.c:263:5: warning: no previous prototype for 'clear_user_proc' [-Wmissing-prototypes]
263 | int clear_user_proc(void __user *buf, int size)
| ^~~~~~~~~~~~~~~
arch/um/kernel/process.c:271:6: warning: no previous prototype for 'set_using_sysemu' [-Wmissing-prototypes]
271 | void set_using_sysemu(int value)
| ^~~~~~~~~~~~~~~~
arch/um/kernel/process.c:278:5: warning: no previous prototype for 'get_using_sysemu' [-Wmissing-prototypes]
278 | int get_using_sysemu(void)
| ^~~~~~~~~~~~~~~~
arch/um/kernel/process.c:316:12: warning: no previous prototype for 'make_proc_sysemu' [-Wmissing-prototypes]
316 | int __init make_proc_sysemu(void)
| ^~~~~~~~~~~~~~~~
arch/um/kernel/process.c:356:15: warning: no previous prototype for 'arch_align_stack' [-Wmissing-prototypes]
356 | unsigned long arch_align_stack(unsigned long sp)
| ^~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/irqflags.h:16,
from include/linux/rcupdate.h:26,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/node.h:18,
from include/linux/cpu.h:17,
from arch/um/kernel/um_arch.c:6:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
arch/um/kernel/um_arch.c: At top level:
arch/um/kernel/um_arch.c:408:19: warning: no previous prototype for 'read_initrd' [-Wmissing-prototypes]
408 | int __init __weak read_initrd(void)
| ^~~~~~~~~~~
arch/um/kernel/um_arch.c:461:7: warning: no previous prototype for 'text_poke' [-Wmissing-prototypes]
461 | void *text_poke(void *addr, const void *opcode, size_t len)
| ^~~~~~~~~
arch/um/kernel/um_arch.c:473:6: warning: no previous prototype for 'text_poke_sync' [-Wmissing-prototypes]
473 | void text_poke_sync(void)
| ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/mutex.h:22,
from include/linux/notifier.h:14,
from include/linux/clk.h:14,
from lib/vsprintf.c:22:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
lib/vsprintf.c: In function 'va_format':
lib/vsprintf.c:1682:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
1682 | buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
| ^~~
cc1: some warnings being treated as errors
--
In file included from include/linux/preempt.h:11,
from include/linux/spinlock.h:56,
from include/linux/debugobjects.h:6,
from include/linux/timer.h:8,
from include/linux/netdevice.h:24,
from arch/um/os-Linux/drivers/ethertap_kern.c:10:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
arch/um/os-Linux/drivers/ethertap_kern.c: At top level:
arch/um/os-Linux/drivers/ethertap_kern.c:66:5: warning: no previous prototype for 'ethertap_setup' [-Wmissing-prototypes]
66 | int ethertap_setup(char *str, char **mac_out, void *data)
| ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/preempt.h:11,
from include/linux/spinlock.h:56,
from include/linux/debugobjects.h:6,
from include/linux/timer.h:8,
from include/linux/netdevice.h:24,
from arch/um/os-Linux/drivers/tuntap_kern.c:6:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
arch/um/os-Linux/drivers/tuntap_kern.c: At top level:
arch/um/os-Linux/drivers/tuntap_kern.c:56:5: warning: no previous prototype for 'tuntap_setup' [-Wmissing-prototypes]
56 | int tuntap_setup(char *str, char **mac_out, void *data)
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/preempt.h:11,
from include/linux/spinlock.h:56,
from include/linux/kref.h:16,
from include/linux/mm_types.h:8,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from net/ipv4/route.c:63:
include/linux/io.h: In function '__free_ioport_unmap':
>> include/linux/io.h:25:43: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]
25 | DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
| ^~~~~~~~~~~~
include/linux/cleanup.h:38:78: note: in definition of macro 'DEFINE_FREE'
38 | static inline void __free_##_name(void *p) { _type _T = *(_type *)p; _free; }
| ^~~~~
net/ipv4/route.c: In function 'ip_rt_send_redirect':
net/ipv4/route.c:880:13: warning: variable 'log_martians' set but not used [-Wunused-but-set-variable]
880 | int log_martians;
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
..
vim +/ioport_unmap +25 include/linux/io.h
23
24 DEFINE_FREE(iounmap, void __iomem *, iounmap(_T));
> 25 DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
26
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support
2023-10-18 23:16 ` [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support David E. Box
2023-10-19 2:12 ` kernel test robot
2023-10-19 2:23 ` kernel test robot
@ 2023-10-23 15:25 ` Ilpo Järvinen
2 siblings, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 15:25 UTC (permalink / raw)
To: David E. Box
Cc: LKML, platform-driver-x86, dave.hansen, arnd, peterz,
aleksander.lobakin, rajvi.jingar
[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]
On Wed, 18 Oct 2023, David E. Box wrote:
> Add auto-release cleanups for ioumap and ioport_unmap.
ioumap -> iounmap
Add () into function names.
--
i.
>
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - New patch
>
> include/linux/io.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/io.h b/include/linux/io.h
> index 7304f2a69960..1488832c4ad2 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -6,6 +6,7 @@
> #ifndef _LINUX_IO_H
> #define _LINUX_IO_H
>
> +#include <linux/cleanup.h>
> #include <linux/types.h>
> #include <linux/init.h>
> #include <linux/bug.h>
> @@ -20,6 +21,9 @@ __visible void __iowrite32_copy(void __iomem *to, const void *from, size_t count
> void __ioread32_copy(void *to, const void __iomem *from, size_t count);
> void __iowrite64_copy(void __iomem *to, const void *from, size_t count);
>
> +DEFINE_FREE(iounmap, void __iomem *, iounmap(_T));
> +DEFINE_FREE(ioport_unmap, void __iomem *, ioport_unmap(_T));
> +
> #ifdef CONFIG_MMU
> int ioremap_page_range(unsigned long addr, unsigned long end,
> phys_addr_t phys_addr, pgprot_t prot);
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 11/17] platform/x86/intel/pmc: Split pmc_core_ssram_get_pmc()
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (9 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 10/17] linux/io.h: iounmap/ioport_unmap cleanup.h support David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 16:01 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 12/17] platform/x86/intel/pmc: Find and register PMC telemetry entries David E. Box
` (5 subsequent siblings)
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
On supported hardware, each PMC may have an associated SSRAM device for
accessing additional counters. However, only the SSRAM of the first
(primary) PMC is discoverable as a PCI device to the OS. The remaining
(secondary) devices are hidden but their BARs are still accessible and
their addresses are stored in the BAR of the exposed device. Clean up the
code handling the SSRAM discovery. Create two separate functions for
accessing the primary and secondary SSRAM devices.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - Add checking the return value from pmc_core_sram_init() to mtl.c
- Use iounmap cleanup from io.h
V3 - New patch split from previous PATCH 2
- Update changelog
- Use cleanup.h to cleanup ioremap
V2 - no change
drivers/platform/x86/intel/pmc/core_ssram.c | 91 +++++++++++++--------
drivers/platform/x86/intel/pmc/mtl.c | 12 ++-
2 files changed, 67 insertions(+), 36 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
index 815950713e25..ccb3748dbed9 100644
--- a/drivers/platform/x86/intel/pmc/core_ssram.c
+++ b/drivers/platform/x86/intel/pmc/core_ssram.c
@@ -8,6 +8,7 @@
*
*/
+#include <linux/cleanup.h>
#include <linux/pci.h>
#include <linux/io-64-nonatomic-lo-hi.h>
@@ -65,44 +66,74 @@ pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
return 0;
}
-static void
-pmc_core_ssram_get_pmc(struct pmc_dev *pmcdev, void __iomem *ssram, u32 offset,
- int pmc_idx)
+static int
+pmc_core_get_secondary_pmc(struct pmc_dev *pmcdev, int pmc_idx, u32 offset)
{
- u64 pwrm_base;
+ struct pci_dev *ssram_pcidev = pmcdev->ssram_pcidev;
+ void __iomem __free(iounmap) *main_ssram = NULL;
+ void __iomem __free(iounmap) *secondary_ssram = NULL;
+ const struct pmc_reg_map *map;
+ u64 ssram_base, pwrm_base;
u16 devid;
- if (pmc_idx != PMC_IDX_SOC) {
- u64 ssram_base = get_base(ssram, offset);
+ if (!pmcdev->regmap_list)
+ return -ENOENT;
- if (!ssram_base)
- return;
+ /*
+ * The secondary PMC BARS (which are behind hidden PCI devices) are read
+ * from fixed offsets in MMIO of the primary PMC BAR.
+ */
+ ssram_base = ssram_pcidev->resource[0].start;
+ main_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
+ if (!main_ssram)
+ return -ENOMEM;
- ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
- if (!ssram)
- return;
- }
+ ssram_base = get_base(main_ssram, offset);
+ secondary_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
+ if (!secondary_ssram)
+ return -ENOMEM;
+
+ pwrm_base = get_base(secondary_ssram, SSRAM_PWRM_OFFSET);
+ devid = readw(secondary_ssram + SSRAM_DEVID_OFFSET);
+
+ map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
+ if (!map)
+ return -ENODEV;
+
+ return pmc_core_pmc_add(pmcdev, pwrm_base, map, pmc_idx);
+}
+
+static int
+pmc_core_get_primary_pmc(struct pmc_dev *pmcdev)
+{
+ struct pci_dev *ssram_pcidev = pmcdev->ssram_pcidev;
+ void __iomem __free(iounmap) *ssram;
+ const struct pmc_reg_map *map;
+ u64 ssram_base, pwrm_base;
+ u16 devid;
+
+ if (!pmcdev->regmap_list)
+ return -ENOENT;
+
+ /* The primary PMC (SOC die) BAR is BAR 0 in config space. */
+ ssram_base = ssram_pcidev->resource[0].start;
+ ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
+ if (!ssram)
+ return -ENOMEM;
pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
devid = readw(ssram + SSRAM_DEVID_OFFSET);
- if (pmcdev->regmap_list) {
- const struct pmc_reg_map *map;
+ map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
+ if (!map)
+ return -ENODEV;
- map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
- if (map)
- pmc_core_pmc_add(pmcdev, pwrm_base, map, pmc_idx);
- }
-
- if (pmc_idx != PMC_IDX_SOC)
- iounmap(ssram);
+ return pmc_core_pmc_add(pmcdev, pwrm_base, map, PMC_IDX_MAIN);
}
int pmc_core_ssram_init(struct pmc_dev *pmcdev)
{
- void __iomem *ssram;
struct pci_dev *pcidev;
- u64 ssram_base;
int ret;
pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, 2));
@@ -113,18 +144,14 @@ int pmc_core_ssram_init(struct pmc_dev *pmcdev)
if (ret)
goto release_dev;
- ssram_base = pcidev->resource[0].start;
- ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
- if (!ssram)
- goto disable_dev;
-
pmcdev->ssram_pcidev = pcidev;
- pmc_core_ssram_get_pmc(pmcdev, ssram, 0, PMC_IDX_SOC);
- pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_IOE_OFFSET, PMC_IDX_IOE);
- pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_PCH_OFFSET, PMC_IDX_PCH);
+ ret = pmc_core_get_primary_pmc(pmcdev);
+ if (ret)
+ goto disable_dev;
- iounmap(ssram);
+ pmc_core_get_secondary_pmc(pmcdev, PMC_IDX_IOE, SSRAM_IOE_OFFSET);
+ pmc_core_get_secondary_pmc(pmcdev, PMC_IDX_PCH, SSRAM_PCH_OFFSET);
return 0;
diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
index c3b5f4fe01d1..d1d3d33fb4b8 100644
--- a/drivers/platform/x86/intel/pmc/mtl.c
+++ b/drivers/platform/x86/intel/pmc/mtl.c
@@ -990,12 +990,16 @@ int mtl_core_init(struct pmc_dev *pmcdev)
mtl_d3_fixup();
pmcdev->resume = mtl_resume;
-
pmcdev->regmap_list = mtl_pmc_info_list;
- pmc_core_ssram_init(pmcdev);
- /* If regbase not assigned, set map and discover using legacy method */
- if (!pmc->regbase) {
+ /*
+ * If ssram init fails use legacy method to at least get the
+ * primary PMC
+ */
+ ret = pmc_core_ssram_init(pmcdev);
+ if (ret) {
+ dev_warn(&pmcdev->pdev->dev,
+ "ssram init failed, %d, using legacy init\n", ret);
pmc->map = &mtl_socm_reg_map;
ret = get_primary_reg_base(pmc);
if (ret)
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 11/17] platform/x86/intel/pmc: Split pmc_core_ssram_get_pmc()
2023-10-18 23:16 ` [PATCH V4 11/17] platform/x86/intel/pmc: Split pmc_core_ssram_get_pmc() David E. Box
@ 2023-10-23 16:01 ` Ilpo Järvinen
0 siblings, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 16:01 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
On Wed, 18 Oct 2023, David E. Box wrote:
> On supported hardware, each PMC may have an associated SSRAM device for
> accessing additional counters. However, only the SSRAM of the first
> (primary) PMC is discoverable as a PCI device to the OS. The remaining
> (secondary) devices are hidden but their BARs are still accessible and
> their addresses are stored in the BAR of the exposed device. Clean up the
> code handling the SSRAM discovery. Create two separate functions for
> accessing the primary and secondary SSRAM devices.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - Add checking the return value from pmc_core_sram_init() to mtl.c
> - Use iounmap cleanup from io.h
>
> V3 - New patch split from previous PATCH 2
> - Update changelog
> - Use cleanup.h to cleanup ioremap
>
> V2 - no change
>
> drivers/platform/x86/intel/pmc/core_ssram.c | 91 +++++++++++++--------
> drivers/platform/x86/intel/pmc/mtl.c | 12 ++-
> 2 files changed, 67 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
> index 815950713e25..ccb3748dbed9 100644
> --- a/drivers/platform/x86/intel/pmc/core_ssram.c
> +++ b/drivers/platform/x86/intel/pmc/core_ssram.c
> @@ -8,6 +8,7 @@
> *
> */
>
> +#include <linux/cleanup.h>
> #include <linux/pci.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
>
> @@ -65,44 +66,74 @@ pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
> return 0;
> }
>
> -static void
> -pmc_core_ssram_get_pmc(struct pmc_dev *pmcdev, void __iomem *ssram, u32 offset,
> - int pmc_idx)
> +static int
> +pmc_core_get_secondary_pmc(struct pmc_dev *pmcdev, int pmc_idx, u32 offset)
> {
> - u64 pwrm_base;
> + struct pci_dev *ssram_pcidev = pmcdev->ssram_pcidev;
> + void __iomem __free(iounmap) *main_ssram = NULL;
> + void __iomem __free(iounmap) *secondary_ssram = NULL;
> + const struct pmc_reg_map *map;
> + u64 ssram_base, pwrm_base;
> u16 devid;
>
> - if (pmc_idx != PMC_IDX_SOC) {
> - u64 ssram_base = get_base(ssram, offset);
> + if (!pmcdev->regmap_list)
> + return -ENOENT;
>
> - if (!ssram_base)
> - return;
> + /*
> + * The secondary PMC BARS (which are behind hidden PCI devices) are read
> + * from fixed offsets in MMIO of the primary PMC BAR.
> + */
> + ssram_base = ssram_pcidev->resource[0].start;
> + main_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> + if (!main_ssram)
> + return -ENOMEM;
>
> - ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> - if (!ssram)
> - return;
> - }
> + ssram_base = get_base(main_ssram, offset);
> + secondary_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> + if (!secondary_ssram)
> + return -ENOMEM;
> +
> + pwrm_base = get_base(secondary_ssram, SSRAM_PWRM_OFFSET);
> + devid = readw(secondary_ssram + SSRAM_DEVID_OFFSET);
> +
> + map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
> + if (!map)
> + return -ENODEV;
> +
> + return pmc_core_pmc_add(pmcdev, pwrm_base, map, pmc_idx);
> +}
> +
> +static int
> +pmc_core_get_primary_pmc(struct pmc_dev *pmcdev)
> +{
> + struct pci_dev *ssram_pcidev = pmcdev->ssram_pcidev;
> + void __iomem __free(iounmap) *ssram;
> + const struct pmc_reg_map *map;
> + u64 ssram_base, pwrm_base;
> + u16 devid;
> +
> + if (!pmcdev->regmap_list)
> + return -ENOENT;
> +
> + /* The primary PMC (SOC die) BAR is BAR 0 in config space. */
> + ssram_base = ssram_pcidev->resource[0].start;
> + ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> + if (!ssram)
> + return -ENOMEM;
>
> pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
> devid = readw(ssram + SSRAM_DEVID_OFFSET);
>
> - if (pmcdev->regmap_list) {
> - const struct pmc_reg_map *map;
> + map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
> + if (!map)
> + return -ENODEV;
>
> - map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
> - if (map)
> - pmc_core_pmc_add(pmcdev, pwrm_base, map, pmc_idx);
> - }
> -
> - if (pmc_idx != PMC_IDX_SOC)
> - iounmap(ssram);
> + return pmc_core_pmc_add(pmcdev, pwrm_base, map, PMC_IDX_MAIN);
While I very much like the new way pmc_core_get_*_pmc() is structured with
early returns and use of cleanup.h, it feels somethat unnecessary to split
the main logic into two functions given how little there is different.
I'd just handle the differences with if (secondary) { ... } and create
pmc_ssram local variable so as much as possible can be kept in common.
It probably makes still sense to preserve
pmc_core_get_primary/secondary_pmc() functions from the caller point so
those two functions just become wrappers passing correct parameters to
pmc_core_get_pmc().
--
i.
> }
>
> int pmc_core_ssram_init(struct pmc_dev *pmcdev)
> {
> - void __iomem *ssram;
> struct pci_dev *pcidev;
> - u64 ssram_base;
> int ret;
>
> pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, 2));
> @@ -113,18 +144,14 @@ int pmc_core_ssram_init(struct pmc_dev *pmcdev)
> if (ret)
> goto release_dev;
>
> - ssram_base = pcidev->resource[0].start;
> - ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> - if (!ssram)
> - goto disable_dev;
> -
> pmcdev->ssram_pcidev = pcidev;
>
> - pmc_core_ssram_get_pmc(pmcdev, ssram, 0, PMC_IDX_SOC);
> - pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_IOE_OFFSET, PMC_IDX_IOE);
> - pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_PCH_OFFSET, PMC_IDX_PCH);
> + ret = pmc_core_get_primary_pmc(pmcdev);
> + if (ret)
> + goto disable_dev;
>
> - iounmap(ssram);
> + pmc_core_get_secondary_pmc(pmcdev, PMC_IDX_IOE, SSRAM_IOE_OFFSET);
> + pmc_core_get_secondary_pmc(pmcdev, PMC_IDX_PCH, SSRAM_PCH_OFFSET);
>
> return 0;
>
> diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
> index c3b5f4fe01d1..d1d3d33fb4b8 100644
> --- a/drivers/platform/x86/intel/pmc/mtl.c
> +++ b/drivers/platform/x86/intel/pmc/mtl.c
> @@ -990,12 +990,16 @@ int mtl_core_init(struct pmc_dev *pmcdev)
> mtl_d3_fixup();
>
> pmcdev->resume = mtl_resume;
> -
> pmcdev->regmap_list = mtl_pmc_info_list;
> - pmc_core_ssram_init(pmcdev);
>
> - /* If regbase not assigned, set map and discover using legacy method */
> - if (!pmc->regbase) {
> + /*
> + * If ssram init fails use legacy method to at least get the
> + * primary PMC
> + */
> + ret = pmc_core_ssram_init(pmcdev);
> + if (ret) {
> + dev_warn(&pmcdev->pdev->dev,
> + "ssram init failed, %d, using legacy init\n", ret);
> pmc->map = &mtl_socm_reg_map;
> ret = get_primary_reg_base(pmc);
> if (ret)
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 12/17] platform/x86/intel/pmc: Find and register PMC telemetry entries
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (10 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 11/17] platform/x86/intel/pmc: Split pmc_core_ssram_get_pmc() David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-18 23:16 ` [PATCH V4 13/17] platform/x86/intel/pmc: Display LPM requirements for multiple PMCs David E. Box
` (4 subsequent siblings)
16 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
The PMC SSRAM device contains counters that are structured in Intel
Platform Monitoring Technology (PMT) telemetry regions. Look for and
register these telemetry regions from the driver so that they may be read
using the Intel PMT ABI.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - no change
V3 - no change
V2 - no change
drivers/platform/x86/intel/pmc/Kconfig | 1 +
drivers/platform/x86/intel/pmc/core_ssram.c | 52 +++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/Kconfig b/drivers/platform/x86/intel/pmc/Kconfig
index b526597e4deb..d2f651fbec2c 100644
--- a/drivers/platform/x86/intel/pmc/Kconfig
+++ b/drivers/platform/x86/intel/pmc/Kconfig
@@ -7,6 +7,7 @@ config INTEL_PMC_CORE
tristate "Intel PMC Core driver"
depends on PCI
depends on ACPI
+ depends on INTEL_PMT_TELEMETRY
help
The Intel Platform Controller Hub for Intel Core SoCs provides access
to Power Management Controller registers via various interfaces. This
diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
index ccb3748dbed9..936aa0d5f452 100644
--- a/drivers/platform/x86/intel/pmc/core_ssram.c
+++ b/drivers/platform/x86/intel/pmc/core_ssram.c
@@ -13,6 +13,8 @@
#include <linux/io-64-nonatomic-lo-hi.h>
#include "core.h"
+#include "../vsec.h"
+#include "../pmt/telemetry.h"
#define SSRAM_HDR_SIZE 0x100
#define SSRAM_PWRM_OFFSET 0x14
@@ -22,6 +24,49 @@
#define SSRAM_IOE_OFFSET 0x68
#define SSRAM_DEVID_OFFSET 0x70
+static void
+pmc_add_pmt(struct pmc_dev *pmcdev, u64 ssram_base, void __iomem *ssram)
+{
+ struct pci_dev *pcidev = pmcdev->ssram_pcidev;
+ struct intel_vsec_platform_info info = {};
+ struct intel_vsec_header *headers[2] = {};
+ struct intel_vsec_header header;
+ void __iomem *dvsec;
+ u32 dvsec_offset;
+ u32 table, hdr;
+
+ ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
+ if (!ssram)
+ return;
+
+ dvsec_offset = readl(ssram + SSRAM_DVSEC_OFFSET);
+ iounmap(ssram);
+
+ dvsec = ioremap(ssram_base + dvsec_offset, SSRAM_DVSEC_SIZE);
+ if (!dvsec)
+ return;
+
+ hdr = readl(dvsec + PCI_DVSEC_HEADER1);
+ header.id = readw(dvsec + PCI_DVSEC_HEADER2);
+ header.rev = PCI_DVSEC_HEADER1_REV(hdr);
+ header.length = PCI_DVSEC_HEADER1_LEN(hdr);
+ header.num_entries = readb(dvsec + INTEL_DVSEC_ENTRIES);
+ header.entry_size = readb(dvsec + INTEL_DVSEC_SIZE);
+
+ table = readl(dvsec + INTEL_DVSEC_TABLE);
+ header.tbir = INTEL_DVSEC_TABLE_BAR(table);
+ header.offset = INTEL_DVSEC_TABLE_OFFSET(table);
+ iounmap(dvsec);
+
+ headers[0] = &header;
+ info.caps = VSEC_CAP_TELEMETRY;
+ info.headers = headers;
+ info.base_addr = ssram_base;
+ info.parent = &pmcdev->pdev->dev;
+
+ intel_vsec_register(pcidev, &info);
+}
+
static const struct pmc_reg_map *pmc_core_find_regmap(struct pmc_info *list, u16 devid)
{
for (; list->map; ++list)
@@ -96,6 +141,9 @@ pmc_core_get_secondary_pmc(struct pmc_dev *pmcdev, int pmc_idx, u32 offset)
pwrm_base = get_base(secondary_ssram, SSRAM_PWRM_OFFSET);
devid = readw(secondary_ssram + SSRAM_DEVID_OFFSET);
+ /* Find and register and PMC telemetry entries */
+ pmc_add_pmt(pmcdev, ssram_base, main_ssram);
+
map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
if (!map)
return -ENODEV;
@@ -124,6 +172,9 @@ pmc_core_get_primary_pmc(struct pmc_dev *pmcdev)
pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
devid = readw(ssram + SSRAM_DEVID_OFFSET);
+ /* Find and register and PMC telemetry entries */
+ pmc_add_pmt(pmcdev, ssram_base, ssram);
+
map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
if (!map)
return -ENODEV;
@@ -163,3 +214,4 @@ int pmc_core_ssram_init(struct pmc_dev *pmcdev)
return ret;
}
+MODULE_IMPORT_NS(INTEL_VSEC);
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V4 13/17] platform/x86/intel/pmc: Display LPM requirements for multiple PMCs
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (11 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 12/17] platform/x86/intel/pmc: Find and register PMC telemetry entries David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 16:15 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 14/17] platform/x86/intel/pmc: Retrieve LPM information using Intel PMT David E. Box
` (3 subsequent siblings)
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
From: Rajvi Jingar <rajvi.jingar@linux.intel.com>
Update the substate_requirements attribute to display the requirements for
all the PMCs on a package.
Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - No change
V3 - Add missing submitter signoff
V2 - no change
drivers/platform/x86/intel/pmc/core.c | 129 ++++++++++++++------------
1 file changed, 71 insertions(+), 58 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 3894119d61b0..fcb0dc702aea 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -728,7 +728,7 @@ static int pmc_core_substate_l_sts_regs_show(struct seq_file *s, void *unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_l_sts_regs);
-static void pmc_core_substate_req_header_show(struct seq_file *s)
+static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index)
{
struct pmc_dev *pmcdev = s->private;
int i, mode;
@@ -743,68 +743,81 @@ static void pmc_core_substate_req_header_show(struct seq_file *s)
static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
{
struct pmc_dev *pmcdev = s->private;
- struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
- const struct pmc_bit_map **maps = pmc->map->lpm_sts;
- const struct pmc_bit_map *map;
- const int num_maps = pmc->map->lpm_num_maps;
- u32 sts_offset = pmc->map->lpm_status_offset;
- u32 *lpm_req_regs = pmc->lpm_req_regs;
- int mp;
-
- /* Display the header */
- pmc_core_substate_req_header_show(s);
-
- /* Loop over maps */
- for (mp = 0; mp < num_maps; mp++) {
- u32 req_mask = 0;
- u32 lpm_status;
- int mode, idx, i, len = 32;
-
- /*
- * Capture the requirements and create a mask so that we only
- * show an element if it's required for at least one of the
- * enabled low power modes
- */
- pmc_for_each_mode(idx, mode, pmcdev)
- req_mask |= lpm_req_regs[mp + (mode * num_maps)];
-
- /* Get the last latched status for this map */
- lpm_status = pmc_core_reg_read(pmc, sts_offset + (mp * 4));
-
- /* Loop over elements in this map */
- map = maps[mp];
- for (i = 0; map[i].name && i < len; i++) {
- u32 bit_mask = map[i].bit_mask;
-
- if (!(bit_mask & req_mask))
- /*
- * Not required for any enabled states
- * so don't display
- */
- continue;
-
- /* Display the element name in the first column */
- seq_printf(s, "%30s |", map[i].name);
-
- /* Loop over the enabled states and display if required */
- pmc_for_each_mode(idx, mode, pmcdev) {
- if (lpm_req_regs[mp + (mode * num_maps)] & bit_mask)
- seq_printf(s, " %9s |",
- "Required");
+ u32 sts_offset;
+ u32 *lpm_req_regs;
+ int num_maps, mp, pmc_index;
+
+ for (pmc_index = 0; pmc_index < ARRAY_SIZE(pmcdev->pmcs); ++pmc_index) {
+ struct pmc *pmc = pmcdev->pmcs[pmc_index];
+ const struct pmc_bit_map **maps;
+
+ if (!pmc)
+ continue;
+
+ maps = pmc->map->lpm_sts;
+ num_maps = pmc->map->lpm_num_maps;
+ sts_offset = pmc->map->lpm_status_offset;
+ lpm_req_regs = pmc->lpm_req_regs;
+
+ if (!lpm_req_regs)
+ continue;
+
+ /* Display the header */
+ pmc_core_substate_req_header_show(s, pmc_index);
+
+ /* Loop over maps */
+ for (mp = 0; mp < num_maps; mp++) {
+ u32 req_mask = 0;
+ u32 lpm_status;
+ const struct pmc_bit_map *map;
+ int mode, idx, i, len = 32;
+
+ /*
+ * Capture the requirements and create a mask so that we only
+ * show an element if it's required for at least one of the
+ * enabled low power modes
+ */
+ pmc_for_each_mode(idx, mode, pmcdev)
+ req_mask |= lpm_req_regs[mp + (mode * num_maps)];
+
+ /* Get the last latched status for this map */
+ lpm_status = pmc_core_reg_read(pmc, sts_offset + (mp * 4));
+
+ /* Loop over elements in this map */
+ map = maps[mp];
+ for (i = 0; map[i].name && i < len; i++) {
+ u32 bit_mask = map[i].bit_mask;
+
+ if (!(bit_mask & req_mask)) {
+ /*
+ * Not required for any enabled states
+ * so don't display
+ */
+ continue;
+ }
+
+ /* Display the element name in the first column */
+ seq_printf(s, "pmc%d: %26s |", pmc_index, map[i].name);
+
+ /* Loop over the enabled states and display if required */
+ pmc_for_each_mode(idx, mode, pmcdev) {
+ if (lpm_req_regs[mp + (mode * num_maps)] & bit_mask)
+ seq_printf(s, " %9s |",
+ "Required");
+ else
+ seq_printf(s, " %9s |", " ");
+ }
+
+ /* In Status column, show the last captured state of this agent */
+ if (lpm_status & bit_mask)
+ seq_printf(s, " %9s |", "Yes");
else
seq_printf(s, " %9s |", " ");
+
+ seq_puts(s, "\n");
}
-
- /* In Status column, show the last captured state of this agent */
- if (lpm_status & bit_mask)
- seq_printf(s, " %9s |", "Yes");
- else
- seq_printf(s, " %9s |", " ");
-
- seq_puts(s, "\n");
}
}
-
return 0;
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs);
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 13/17] platform/x86/intel/pmc: Display LPM requirements for multiple PMCs
2023-10-18 23:16 ` [PATCH V4 13/17] platform/x86/intel/pmc: Display LPM requirements for multiple PMCs David E. Box
@ 2023-10-23 16:15 ` Ilpo Järvinen
0 siblings, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 16:15 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
On Wed, 18 Oct 2023, David E. Box wrote:
> From: Rajvi Jingar <rajvi.jingar@linux.intel.com>
>
> Update the substate_requirements attribute to display the requirements for
> all the PMCs on a package.
>
> Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - No change
>
> V3 - Add missing submitter signoff
>
> V2 - no change
>
> drivers/platform/x86/intel/pmc/core.c | 129 ++++++++++++++------------
> 1 file changed, 71 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 3894119d61b0..fcb0dc702aea 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -728,7 +728,7 @@ static int pmc_core_substate_l_sts_regs_show(struct seq_file *s, void *unused)
> }
> DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_l_sts_regs);
>
> -static void pmc_core_substate_req_header_show(struct seq_file *s)
> +static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index)
> {
> struct pmc_dev *pmcdev = s->private;
> int i, mode;
> @@ -743,68 +743,81 @@ static void pmc_core_substate_req_header_show(struct seq_file *s)
> static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> {
> struct pmc_dev *pmcdev = s->private;
> - struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
> - const struct pmc_bit_map **maps = pmc->map->lpm_sts;
> - const struct pmc_bit_map *map;
> - const int num_maps = pmc->map->lpm_num_maps;
> - u32 sts_offset = pmc->map->lpm_status_offset;
> - u32 *lpm_req_regs = pmc->lpm_req_regs;
> - int mp;
> -
> - /* Display the header */
> - pmc_core_substate_req_header_show(s);
> -
> - /* Loop over maps */
> - for (mp = 0; mp < num_maps; mp++) {
> - u32 req_mask = 0;
> - u32 lpm_status;
> - int mode, idx, i, len = 32;
> -
> - /*
> - * Capture the requirements and create a mask so that we only
> - * show an element if it's required for at least one of the
> - * enabled low power modes
> - */
> - pmc_for_each_mode(idx, mode, pmcdev)
> - req_mask |= lpm_req_regs[mp + (mode * num_maps)];
> -
> - /* Get the last latched status for this map */
> - lpm_status = pmc_core_reg_read(pmc, sts_offset + (mp * 4));
> -
> - /* Loop over elements in this map */
> - map = maps[mp];
> - for (i = 0; map[i].name && i < len; i++) {
> - u32 bit_mask = map[i].bit_mask;
> -
> - if (!(bit_mask & req_mask))
> - /*
> - * Not required for any enabled states
> - * so don't display
> - */
> - continue;
> -
> - /* Display the element name in the first column */
> - seq_printf(s, "%30s |", map[i].name);
> -
> - /* Loop over the enabled states and display if required */
> - pmc_for_each_mode(idx, mode, pmcdev) {
> - if (lpm_req_regs[mp + (mode * num_maps)] & bit_mask)
> - seq_printf(s, " %9s |",
> - "Required");
> + u32 sts_offset;
> + u32 *lpm_req_regs;
> + int num_maps, mp, pmc_index;
> +
> + for (pmc_index = 0; pmc_index < ARRAY_SIZE(pmcdev->pmcs); ++pmc_index) {
> + struct pmc *pmc = pmcdev->pmcs[pmc_index];
> + const struct pmc_bit_map **maps;
> +
> + if (!pmc)
> + continue;
> +
> + maps = pmc->map->lpm_sts;
> + num_maps = pmc->map->lpm_num_maps;
> + sts_offset = pmc->map->lpm_status_offset;
> + lpm_req_regs = pmc->lpm_req_regs;
> +
> + if (!lpm_req_regs)
> + continue;
> +
> + /* Display the header */
> + pmc_core_substate_req_header_show(s, pmc_index);
> +
> + /* Loop over maps */
> + for (mp = 0; mp < num_maps; mp++) {
> + u32 req_mask = 0;
> + u32 lpm_status;
> + const struct pmc_bit_map *map;
> + int mode, idx, i, len = 32;
> +
> + /*
> + * Capture the requirements and create a mask so that we only
> + * show an element if it's required for at least one of the
> + * enabled low power modes
> + */
> + pmc_for_each_mode(idx, mode, pmcdev)
> + req_mask |= lpm_req_regs[mp + (mode * num_maps)];
> +
> + /* Get the last latched status for this map */
> + lpm_status = pmc_core_reg_read(pmc, sts_offset + (mp * 4));
> +
> + /* Loop over elements in this map */
> + map = maps[mp];
> + for (i = 0; map[i].name && i < len; i++) {
> + u32 bit_mask = map[i].bit_mask;
> +
> + if (!(bit_mask & req_mask)) {
> + /*
> + * Not required for any enabled states
> + * so don't display
> + */
> + continue;
> + }
> +
> + /* Display the element name in the first column */
> + seq_printf(s, "pmc%d: %26s |", pmc_index, map[i].name);
> +
> + /* Loop over the enabled states and display if required */
> + pmc_for_each_mode(idx, mode, pmcdev) {
> + if (lpm_req_regs[mp + (mode * num_maps)] & bit_mask)
> + seq_printf(s, " %9s |",
> + "Required");
> + else
> + seq_printf(s, " %9s |", " ");
It would be better to not branch like this but alter param instead:
bool required = lpm_req_regs[...
seq_printf(s, " %9s |",
required ? "Required" : " ");
> + }
> +
> + /* In Status column, show the last captured state of this agent */
> + if (lpm_status & bit_mask)
> + seq_printf(s, " %9s |", "Yes");
> else
> seq_printf(s, " %9s |", " ");
Likewise here although I know this comes from the original.
--
i.
> +
> + seq_puts(s, "\n");
> }
> -
> - /* In Status column, show the last captured state of this agent */
> - if (lpm_status & bit_mask)
> - seq_printf(s, " %9s |", "Yes");
> - else
> - seq_printf(s, " %9s |", " ");
> -
> - seq_puts(s, "\n");
> }
> }
> -
> return 0;
> }
> DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs);
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 14/17] platform/x86/intel/pmc: Retrieve LPM information using Intel PMT
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (12 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 13/17] platform/x86/intel/pmc: Display LPM requirements for multiple PMCs David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 16:19 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 15/17] platform/x86/intel/pmc: Read low power mode requirements for MTL-M and MTL-P David E. Box
` (2 subsequent siblings)
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
From: Xi Pardee <xi.pardee@intel.com>
On supported platforms, the low power mode (LPM) requirements for entering
each idle substate are described in Platform Monitoring Technology (PMT)
telemetry entries. Provide a function for platform code to attempt to find
and read the requirements from the telemetry entries.
Signed-off-by: Xi Pardee <xi.pardee@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - no change
V3 - no change
V2 - remove extra parens
drivers/platform/x86/intel/pmc/core.h | 3 +
drivers/platform/x86/intel/pmc/core_ssram.c | 135 ++++++++++++++++++++
2 files changed, 138 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index edaa70067e41..85b6f6ae4995 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -320,6 +320,7 @@ struct pmc_reg_map {
const u32 lpm_status_offset;
const u32 lpm_live_status_offset;
const u32 etr3_offset;
+ const u8 *lpm_reg_index;
};
/**
@@ -329,6 +330,7 @@ struct pmc_reg_map {
* specific attributes
*/
struct pmc_info {
+ u32 guid;
u16 devid;
const struct pmc_reg_map *map;
};
@@ -486,6 +488,7 @@ extern const struct pmc_bit_map *mtl_ioem_lpm_maps[];
extern const struct pmc_reg_map mtl_ioem_reg_map;
extern void pmc_core_get_tgl_lpm_reqs(struct platform_device *pdev);
+extern int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev);
extern int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value);
int pmc_core_resume_common(struct pmc_dev *pmcdev);
diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
index 936aa0d5f452..f007964156bc 100644
--- a/drivers/platform/x86/intel/pmc/core_ssram.c
+++ b/drivers/platform/x86/intel/pmc/core_ssram.c
@@ -24,6 +24,140 @@
#define SSRAM_IOE_OFFSET 0x68
#define SSRAM_DEVID_OFFSET 0x70
+/* PCH query */
+#define LPM_HEADER_OFFSET 1
+#define LPM_REG_COUNT 28
+#define LPM_MODE_OFFSET 1
+
+static u32 pmc_core_find_guid(struct pmc_info *list, const struct pmc_reg_map *map)
+{
+ for (; list->map; ++list)
+ if (list->map == map)
+ return list->guid;
+
+ return 0;
+}
+
+static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc)
+{
+ struct telem_endpoint *ep;
+ const u8 *lpm_indices;
+ int num_maps, mode_offset = 0;
+ int ret, mode, i;
+ int lpm_size;
+ u32 guid;
+
+ lpm_indices = pmc->map->lpm_reg_index;
+ num_maps = pmc->map->lpm_num_maps;
+ lpm_size = LPM_MAX_NUM_MODES * num_maps;
+
+ guid = pmc_core_find_guid(pmcdev->regmap_list, pmc->map);
+ if (!guid)
+ return -ENXIO;
+
+ ep = pmt_telem_find_and_register_endpoint(pmcdev->ssram_pcidev, guid, 0);
+ if (IS_ERR(ep)) {
+ dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %ld",
+ PTR_ERR(ep));
+ return -EPROBE_DEFER;
+ }
+
+ pmc->lpm_req_regs = devm_kzalloc(&pmcdev->pdev->dev,
+ lpm_size * sizeof(u32),
+ GFP_KERNEL);
+ if (!pmc->lpm_req_regs) {
+ ret = -ENOMEM;
+ goto unregister_ep;
+ }
+
+ /*
+ * PMC Low Power Mode (LPM) table
+ *
+ * In telemetry space, the LPM table contains a 4 byte header followed
+ * by 8 consecutive mode blocks (one for each LPM mode). Each block
+ * has a 4 byte header followed by a set of registers that describe the
+ * IP state requirements for the given mode. The IP mapping is platform
+ * specific but the same for each block, making for easy analysis.
+ * Platforms only use a subset of the space to track the requirements
+ * for their IPs. Callers provide the requirement registers they use as
+ * a list of indices. Each requirement register is associated with an
+ * IP map that's maintained by the caller.
+ *
+ * Header
+ * +----+----------------------------+----------------------------+
+ * | 0 | REVISION | ENABLED MODES |
+ * +----+--------------+-------------+-------------+--------------+
+ *
+ * Low Power Mode 0 Block
+ * +----+--------------+-------------+-------------+--------------+
+ * | 1 | SUB ID | SIZE | MAJOR | MINOR |
+ * +----+--------------+-------------+-------------+--------------+
+ * | 2 | LPM0 Requirements 0 |
+ * +----+---------------------------------------------------------+
+ * | | ... |
+ * +----+---------------------------------------------------------+
+ * | 29 | LPM0 Requirements 27 |
+ * +----+---------------------------------------------------------+
+ *
+ * ...
+ *
+ * Low Power Mode 7 Block
+ * +----+--------------+-------------+-------------+--------------+
+ * | | SUB ID | SIZE | MAJOR | MINOR |
+ * +----+--------------+-------------+-------------+--------------+
+ * | 60 | LPM7 Requirements 0 |
+ * +----+---------------------------------------------------------+
+ * | | ... |
+ * +----+---------------------------------------------------------+
+ * | 87 | LPM7 Requirements 27 |
+ * +----+---------------------------------------------------------+
+ *
+ */
+ mode_offset = LPM_HEADER_OFFSET + LPM_MODE_OFFSET;
+ pmc_for_each_mode(i, mode, pmcdev) {
+ u32 *req_offset = pmc->lpm_req_regs + (mode * num_maps);
+ int m;
+
+ for (m = 0; m < num_maps; m++) {
+ u8 sample_id = lpm_indices[m] + mode_offset;
+
+ ret = pmt_telem_read32(ep, sample_id, req_offset, 1);
+ if (ret) {
+ dev_err(&pmcdev->pdev->dev,
+ "couldn't read Low Power Mode requirements: %d\n", ret);
+ devm_kfree(&pmcdev->pdev->dev, pmc->lpm_req_regs);
+ goto unregister_ep;
+ }
+ ++req_offset;
+ }
+ mode_offset += LPM_REG_COUNT + LPM_MODE_OFFSET;
+ }
+
+unregister_ep:
+ pmt_telem_unregister_endpoint(ep);
+
+ return ret;
+}
+
+int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev)
+{
+ int ret, i;
+
+ if (!pmcdev->ssram_pcidev)
+ return -ENODEV;
+
+ for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
+ if (!pmcdev->pmcs[i])
+ continue;
+
+ ret = pmc_core_get_lpm_req(pmcdev, pmcdev->pmcs[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static void
pmc_add_pmt(struct pmc_dev *pmcdev, u64 ssram_base, void __iomem *ssram)
{
@@ -215,3 +349,4 @@ int pmc_core_ssram_init(struct pmc_dev *pmcdev)
return ret;
}
MODULE_IMPORT_NS(INTEL_VSEC);
+MODULE_IMPORT_NS(INTEL_PMT_TELEMETRY);
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 14/17] platform/x86/intel/pmc: Retrieve LPM information using Intel PMT
2023-10-18 23:16 ` [PATCH V4 14/17] platform/x86/intel/pmc: Retrieve LPM information using Intel PMT David E. Box
@ 2023-10-23 16:19 ` Ilpo Järvinen
0 siblings, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 16:19 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
[-- Attachment #1: Type: text/plain, Size: 7006 bytes --]
On Wed, 18 Oct 2023, David E. Box wrote:
> From: Xi Pardee <xi.pardee@intel.com>
>
> On supported platforms, the low power mode (LPM) requirements for entering
> each idle substate are described in Platform Monitoring Technology (PMT)
> telemetry entries. Provide a function for platform code to attempt to find
> and read the requirements from the telemetry entries.
>
> Signed-off-by: Xi Pardee <xi.pardee@intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - no change
>
> V3 - no change
>
> V2 - remove extra parens
>
> drivers/platform/x86/intel/pmc/core.h | 3 +
> drivers/platform/x86/intel/pmc/core_ssram.c | 135 ++++++++++++++++++++
> 2 files changed, 138 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index edaa70067e41..85b6f6ae4995 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -320,6 +320,7 @@ struct pmc_reg_map {
> const u32 lpm_status_offset;
> const u32 lpm_live_status_offset;
> const u32 etr3_offset;
> + const u8 *lpm_reg_index;
> };
>
> /**
> @@ -329,6 +330,7 @@ struct pmc_reg_map {
> * specific attributes
> */
> struct pmc_info {
> + u32 guid;
> u16 devid;
> const struct pmc_reg_map *map;
> };
> @@ -486,6 +488,7 @@ extern const struct pmc_bit_map *mtl_ioem_lpm_maps[];
> extern const struct pmc_reg_map mtl_ioem_reg_map;
>
> extern void pmc_core_get_tgl_lpm_reqs(struct platform_device *pdev);
> +extern int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev);
> extern int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value);
>
> int pmc_core_resume_common(struct pmc_dev *pmcdev);
> diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
> index 936aa0d5f452..f007964156bc 100644
> --- a/drivers/platform/x86/intel/pmc/core_ssram.c
> +++ b/drivers/platform/x86/intel/pmc/core_ssram.c
> @@ -24,6 +24,140 @@
> #define SSRAM_IOE_OFFSET 0x68
> #define SSRAM_DEVID_OFFSET 0x70
>
> +/* PCH query */
> +#define LPM_HEADER_OFFSET 1
> +#define LPM_REG_COUNT 28
> +#define LPM_MODE_OFFSET 1
> +
> +static u32 pmc_core_find_guid(struct pmc_info *list, const struct pmc_reg_map *map)
> +{
> + for (; list->map; ++list)
> + if (list->map == map)
> + return list->guid;
> +
> + return 0;
> +}
> +
> +static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc)
> +{
> + struct telem_endpoint *ep;
> + const u8 *lpm_indices;
> + int num_maps, mode_offset = 0;
> + int ret, mode, i;
> + int lpm_size;
> + u32 guid;
> +
> + lpm_indices = pmc->map->lpm_reg_index;
> + num_maps = pmc->map->lpm_num_maps;
> + lpm_size = LPM_MAX_NUM_MODES * num_maps;
> +
> + guid = pmc_core_find_guid(pmcdev->regmap_list, pmc->map);
> + if (!guid)
> + return -ENXIO;
> +
> + ep = pmt_telem_find_and_register_endpoint(pmcdev->ssram_pcidev, guid, 0);
> + if (IS_ERR(ep)) {
> + dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %ld",
> + PTR_ERR(ep));
> + return -EPROBE_DEFER;
> + }
> +
> + pmc->lpm_req_regs = devm_kzalloc(&pmcdev->pdev->dev,
> + lpm_size * sizeof(u32),
> + GFP_KERNEL);
> + if (!pmc->lpm_req_regs) {
> + ret = -ENOMEM;
> + goto unregister_ep;
> + }
> +
> + /*
> + * PMC Low Power Mode (LPM) table
> + *
> + * In telemetry space, the LPM table contains a 4 byte header followed
> + * by 8 consecutive mode blocks (one for each LPM mode). Each block
> + * has a 4 byte header followed by a set of registers that describe the
> + * IP state requirements for the given mode. The IP mapping is platform
> + * specific but the same for each block, making for easy analysis.
> + * Platforms only use a subset of the space to track the requirements
> + * for their IPs. Callers provide the requirement registers they use as
> + * a list of indices. Each requirement register is associated with an
> + * IP map that's maintained by the caller.
> + *
> + * Header
> + * +----+----------------------------+----------------------------+
> + * | 0 | REVISION | ENABLED MODES |
> + * +----+--------------+-------------+-------------+--------------+
> + *
> + * Low Power Mode 0 Block
> + * +----+--------------+-------------+-------------+--------------+
> + * | 1 | SUB ID | SIZE | MAJOR | MINOR |
> + * +----+--------------+-------------+-------------+--------------+
> + * | 2 | LPM0 Requirements 0 |
> + * +----+---------------------------------------------------------+
> + * | | ... |
> + * +----+---------------------------------------------------------+
> + * | 29 | LPM0 Requirements 27 |
> + * +----+---------------------------------------------------------+
> + *
> + * ...
> + *
> + * Low Power Mode 7 Block
> + * +----+--------------+-------------+-------------+--------------+
> + * | | SUB ID | SIZE | MAJOR | MINOR |
> + * +----+--------------+-------------+-------------+--------------+
> + * | 60 | LPM7 Requirements 0 |
> + * +----+---------------------------------------------------------+
> + * | | ... |
> + * +----+---------------------------------------------------------+
> + * | 87 | LPM7 Requirements 27 |
> + * +----+---------------------------------------------------------+
> + *
> + */
> + mode_offset = LPM_HEADER_OFFSET + LPM_MODE_OFFSET;
> + pmc_for_each_mode(i, mode, pmcdev) {
> + u32 *req_offset = pmc->lpm_req_regs + (mode * num_maps);
> + int m;
> +
> + for (m = 0; m < num_maps; m++) {
> + u8 sample_id = lpm_indices[m] + mode_offset;
> +
> + ret = pmt_telem_read32(ep, sample_id, req_offset, 1);
> + if (ret) {
> + dev_err(&pmcdev->pdev->dev,
> + "couldn't read Low Power Mode requirements: %d\n", ret);
> + devm_kfree(&pmcdev->pdev->dev, pmc->lpm_req_regs);
> + goto unregister_ep;
> + }
> + ++req_offset;
> + }
> + mode_offset += LPM_REG_COUNT + LPM_MODE_OFFSET;
> + }
> +
> +unregister_ep:
> + pmt_telem_unregister_endpoint(ep);
> +
> + return ret;
> +}
> +
> +int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev)
> +{
> + int ret, i;
> +
> + if (!pmcdev->ssram_pcidev)
> + return -ENODEV;
> +
> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
> + if (!pmcdev->pmcs[i])
> + continue;
> +
> + ret = pmc_core_get_lpm_req(pmcdev, pmcdev->pmcs[i]);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static void
> pmc_add_pmt(struct pmc_dev *pmcdev, u64 ssram_base, void __iomem *ssram)
> {
> @@ -215,3 +349,4 @@ int pmc_core_ssram_init(struct pmc_dev *pmcdev)
> return ret;
> }
> MODULE_IMPORT_NS(INTEL_VSEC);
> +MODULE_IMPORT_NS(INTEL_PMT_TELEMETRY);
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 15/17] platform/x86/intel/pmc: Read low power mode requirements for MTL-M and MTL-P
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (13 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 14/17] platform/x86/intel/pmc: Retrieve LPM information using Intel PMT David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-18 23:16 ` [PATCH V4 16/17] platform/x86/intel/pmc: Add debug attribute for Die C6 counter David E. Box
2023-10-18 23:16 ` [PATCH V4 17/17] platform/x86/intel/pmc: Show Die C6 counter on Meteor Lake David E. Box
16 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
From: Xi Pardee <xi.pardee@intel.com>
Add support to read the low power mode requirements for Meteor Lake M and
Meteor Lake P.
Signed-off-by: Xi Pardee <xi.pardee@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V4 - no change
V3 - directly return value from pmc_core_ssram_get_lpm_reqs()
V2 - fixed unused return value
drivers/platform/x86/intel/pmc/mtl.c | 39 +++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
index d1d3d33fb4b8..7ceeae507f4c 100644
--- a/drivers/platform/x86/intel/pmc/mtl.c
+++ b/drivers/platform/x86/intel/pmc/mtl.c
@@ -11,6 +11,13 @@
#include <linux/pci.h>
#include "core.h"
+/* PMC SSRAM PMT Telemetry GUIDS */
+#define SOCP_LPM_REQ_GUID 0x2625030
+#define IOEM_LPM_REQ_GUID 0x4357464
+#define IOEP_LPM_REQ_GUID 0x5077612
+
+static const u8 MTL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
+
/*
* Die Mapping to Product.
* Product SOCDie IOEDie PCHDie
@@ -465,6 +472,7 @@ const struct pmc_reg_map mtl_socm_reg_map = {
.lpm_sts = mtl_socm_lpm_maps,
.lpm_status_offset = MTL_LPM_STATUS_OFFSET,
.lpm_live_status_offset = MTL_LPM_LIVE_STATUS_OFFSET,
+ .lpm_reg_index = MTL_LPM_REG_INDEX,
};
const struct pmc_bit_map mtl_ioep_pfear_map[] = {
@@ -782,6 +790,13 @@ const struct pmc_reg_map mtl_ioep_reg_map = {
.ltr_show_sts = mtl_ioep_ltr_show_map,
.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
.ltr_ignore_max = ADL_NUM_IP_IGN_ALLOWED,
+ .lpm_num_maps = ADL_LPM_NUM_MAPS,
+ .lpm_res_counter_step_x2 = TGL_PMC_LPM_RES_COUNTER_STEP_X2,
+ .lpm_residency_offset = MTL_LPM_RESIDENCY_OFFSET,
+ .lpm_priority_offset = MTL_LPM_PRI_OFFSET,
+ .lpm_en_offset = MTL_LPM_EN_OFFSET,
+ .lpm_sts_latch_en_offset = MTL_LPM_STATUS_LATCH_EN_OFFSET,
+ .lpm_reg_index = MTL_LPM_REG_INDEX,
};
const struct pmc_bit_map mtl_ioem_pfear_map[] = {
@@ -922,6 +937,13 @@ const struct pmc_reg_map mtl_ioem_reg_map = {
.ltr_show_sts = mtl_ioep_ltr_show_map,
.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
.ltr_ignore_max = ADL_NUM_IP_IGN_ALLOWED,
+ .lpm_sts_latch_en_offset = MTL_LPM_STATUS_LATCH_EN_OFFSET,
+ .lpm_num_maps = ADL_LPM_NUM_MAPS,
+ .lpm_priority_offset = MTL_LPM_PRI_OFFSET,
+ .lpm_en_offset = MTL_LPM_EN_OFFSET,
+ .lpm_res_counter_step_x2 = TGL_PMC_LPM_RES_COUNTER_STEP_X2,
+ .lpm_residency_offset = MTL_LPM_RESIDENCY_OFFSET,
+ .lpm_reg_index = MTL_LPM_REG_INDEX,
};
#define PMC_DEVID_SOCM 0x7e7f
@@ -929,16 +951,19 @@ const struct pmc_reg_map mtl_ioem_reg_map = {
#define PMC_DEVID_IOEM 0x7ebf
static struct pmc_info mtl_pmc_info_list[] = {
{
- .devid = PMC_DEVID_SOCM,
- .map = &mtl_socm_reg_map,
+ .guid = SOCP_LPM_REQ_GUID,
+ .devid = PMC_DEVID_SOCM,
+ .map = &mtl_socm_reg_map,
},
{
- .devid = PMC_DEVID_IOEP,
- .map = &mtl_ioep_reg_map,
+ .guid = IOEP_LPM_REQ_GUID,
+ .devid = PMC_DEVID_IOEP,
+ .map = &mtl_ioep_reg_map,
},
{
- .devid = PMC_DEVID_IOEM,
- .map = &mtl_ioem_reg_map
+ .guid = IOEM_LPM_REQ_GUID,
+ .devid = PMC_DEVID_IOEM,
+ .map = &mtl_ioem_reg_map
},
{}
};
@@ -1014,5 +1039,5 @@ int mtl_core_init(struct pmc_dev *pmcdev)
dev_dbg(&pmcdev->pdev->dev, "ignoring GBE LTR\n");
pmc_core_send_ltr_ignore(pmcdev, 3);
- return 0;
+ return pmc_core_ssram_get_lpm_reqs(pmcdev);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V4 16/17] platform/x86/intel/pmc: Add debug attribute for Die C6 counter
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (14 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 15/17] platform/x86/intel/pmc: Read low power mode requirements for MTL-M and MTL-P David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 16:31 ` Ilpo Järvinen
2023-10-18 23:16 ` [PATCH V4 17/17] platform/x86/intel/pmc: Show Die C6 counter on Meteor Lake David E. Box
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
Add a "die_c6_us_show" debugfs attribute. Reads the counter value using
Intel Platform Monitoring Technology (PMT) driver API. This counter is
useful for determining the idle residency of CPUs in the compute tile.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - no change
V3 - Split previous PATCH V2 13. Separates implementation (this patch) from
platform specific use (next patch)
V2 - Remove use of __func__
- Use HZ_PER_MHZ
- Fix missing newlines in printks
drivers/platform/x86/intel/pmc/core.c | 55 +++++++++++++++++++++++++++
drivers/platform/x86/intel/pmc/core.h | 4 ++
2 files changed, 59 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index fcb0dc702aea..02f3e909cf22 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -20,6 +20,7 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <linux/units.h>
#include <asm/cpu_device_id.h>
#include <asm/intel-family.h>
@@ -27,6 +28,7 @@
#include <asm/tsc.h>
#include "core.h"
+#include "../pmt/telemetry.h"
/* Maximum number of modes supported by platfoms that has low power mode capability */
const char *pmc_lpm_modes[] = {
@@ -822,6 +824,47 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs);
+static unsigned int pmc_core_get_crystal_freq(void)
+{
+ unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
+
+ if (boot_cpu_data.cpuid_level < 0x15)
+ return 0;
+
+ eax_denominator = ebx_numerator = ecx_hz = edx = 0;
+
+ /* CPUID 15H TSC/Crystal ratio, plus optionally Crystal Hz */
+ cpuid(0x15, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
+
+ if (ebx_numerator == 0 || eax_denominator == 0)
+ return 0;
+
+ return ecx_hz;
+}
+
+static int pmc_core_die_c6_us_show(struct seq_file *s, void *unused)
+{
+ struct pmc_dev *pmcdev = s->private;
+ u64 die_c6_res, count;
+ int ret;
+
+ if (!pmcdev->crystal_freq) {
+ dev_warn_once(&pmcdev->pdev->dev, "Bad crystal frequency\n");
+ return -EINVAL;
+ }
+
+ ret = pmt_telem_read(pmcdev->punit_ep, pmcdev->die_c6_offset,
+ &count, 1);
+ if (ret)
+ return ret;
+
+ die_c6_res = div64_u64(count * HZ_PER_MHZ, pmcdev->crystal_freq);
+ seq_printf(s, "%llu\n", die_c6_res);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_die_c6_us);
+
static int pmc_core_lpm_latch_mode_show(struct seq_file *s, void *unused)
{
struct pmc_dev *pmcdev = s->private;
@@ -1118,6 +1161,12 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
pmcdev->dbgfs_dir, pmcdev,
&pmc_core_substate_req_regs_fops);
}
+
+ if (pmcdev->has_die_c6) {
+ debugfs_create_file("die_c6_us_show", 0444,
+ pmcdev->dbgfs_dir, pmcdev,
+ &pmc_core_die_c6_us_fops);
+ }
}
static const struct x86_cpu_id intel_pmc_core_ids[] = {
@@ -1212,6 +1261,10 @@ static void pmc_core_clean_structure(struct platform_device *pdev)
pci_dev_put(pmcdev->ssram_pcidev);
pci_disable_device(pmcdev->ssram_pcidev);
}
+
+ if (pmcdev->punit_ep)
+ pmt_telem_unregister_endpoint(pmcdev->punit_ep);
+
platform_set_drvdata(pdev, NULL);
mutex_destroy(&pmcdev->lock);
}
@@ -1232,6 +1285,8 @@ static int pmc_core_probe(struct platform_device *pdev)
if (!pmcdev)
return -ENOMEM;
+ pmcdev->crystal_freq = pmc_core_get_crystal_freq();
+
platform_set_drvdata(pdev, pmcdev);
pmcdev->pdev = pdev;
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index 85b6f6ae4995..6d7673145f90 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -16,6 +16,8 @@
#include <linux/bits.h>
#include <linux/platform_device.h>
+struct telem_endpoint;
+
#define SLP_S0_RES_COUNTER_MASK GENMASK(31, 0)
#define PMC_BASE_ADDR_DEFAULT 0xFE000000
@@ -357,6 +359,7 @@ struct pmc {
* @devs: pointer to an array of pmc pointers
* @pdev: pointer to platform_device struct
* @ssram_pcidev: pointer to pci device struct for the PMC SSRAM
+ * @crystal_freq: crystal frequency from cpuid
* @dbgfs_dir: path to debugfs interface
* @pmc_xram_read_bit: flag to indicate whether PMC XRAM shadow registers
* used to read MPHY PG and PLL status are available
@@ -374,6 +377,7 @@ struct pmc_dev {
struct dentry *dbgfs_dir;
struct platform_device *pdev;
struct pci_dev *ssram_pcidev;
+ unsigned int crystal_freq;
int pmc_xram_read_bit;
struct mutex lock; /* generic mutex lock for PMC Core */
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 16/17] platform/x86/intel/pmc: Add debug attribute for Die C6 counter
2023-10-18 23:16 ` [PATCH V4 16/17] platform/x86/intel/pmc: Add debug attribute for Die C6 counter David E. Box
@ 2023-10-23 16:31 ` Ilpo Järvinen
2023-10-26 20:28 ` David E. Box
0 siblings, 1 reply; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 16:31 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
On Wed, 18 Oct 2023, David E. Box wrote:
> Add a "die_c6_us_show" debugfs attribute. Reads the counter value using
> Intel Platform Monitoring Technology (PMT) driver API. This counter is
> useful for determining the idle residency of CPUs in the compute tile.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - no change
>
> V3 - Split previous PATCH V2 13. Separates implementation (this patch) from
> platform specific use (next patch)
>
> V2 - Remove use of __func__
> - Use HZ_PER_MHZ
> - Fix missing newlines in printks
>
> drivers/platform/x86/intel/pmc/core.c | 55 +++++++++++++++++++++++++++
> drivers/platform/x86/intel/pmc/core.h | 4 ++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index fcb0dc702aea..02f3e909cf22 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -20,6 +20,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> +#include <linux/units.h>
>
> #include <asm/cpu_device_id.h>
> #include <asm/intel-family.h>
> @@ -27,6 +28,7 @@
> #include <asm/tsc.h>
>
> #include "core.h"
> +#include "../pmt/telemetry.h"
>
> /* Maximum number of modes supported by platfoms that has low power mode capability */
> const char *pmc_lpm_modes[] = {
> @@ -822,6 +824,47 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> }
> DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs);
>
> +static unsigned int pmc_core_get_crystal_freq(void)
> +{
> + unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
> +
> + if (boot_cpu_data.cpuid_level < 0x15)
> + return 0;
> +
> + eax_denominator = ebx_numerator = ecx_hz = edx = 0;
> +
> + /* CPUID 15H TSC/Crystal ratio, plus optionally Crystal Hz */
> + cpuid(0x15, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
> +
> + if (ebx_numerator == 0 || eax_denominator == 0)
> + return 0;
> +
> + return ecx_hz;
> +}
> +
> +static int pmc_core_die_c6_us_show(struct seq_file *s, void *unused)
> +{
> + struct pmc_dev *pmcdev = s->private;
> + u64 die_c6_res, count;
> + int ret;
> +
> + if (!pmcdev->crystal_freq) {
> + dev_warn_once(&pmcdev->pdev->dev, "Bad crystal frequency\n");
Isn't it more like crystal frequency is not provided rather than bad
frequency?
> + return -EINVAL;
-EINVAL is not good value to return here since there was nothing wrong
with the input. Maybe -ENXIO would be better.
> + }
> +
> + ret = pmt_telem_read(pmcdev->punit_ep, pmcdev->die_c6_offset,
> + &count, 1);
> + if (ret)
> + return ret;
> +
> + die_c6_res = div64_u64(count * HZ_PER_MHZ, pmcdev->crystal_freq);
> + seq_printf(s, "%llu\n", die_c6_res);
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(pmc_core_die_c6_us);
> +
> static int pmc_core_lpm_latch_mode_show(struct seq_file *s, void *unused)
> {
> struct pmc_dev *pmcdev = s->private;
> @@ -1118,6 +1161,12 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
> pmcdev->dbgfs_dir, pmcdev,
> &pmc_core_substate_req_regs_fops);
> }
> +
> + if (pmcdev->has_die_c6) {
> + debugfs_create_file("die_c6_us_show", 0444,
> + pmcdev->dbgfs_dir, pmcdev,
> + &pmc_core_die_c6_us_fops);
> + }
> }
>
> static const struct x86_cpu_id intel_pmc_core_ids[] = {
> @@ -1212,6 +1261,10 @@ static void pmc_core_clean_structure(struct platform_device *pdev)
> pci_dev_put(pmcdev->ssram_pcidev);
> pci_disable_device(pmcdev->ssram_pcidev);
> }
> +
> + if (pmcdev->punit_ep)
> + pmt_telem_unregister_endpoint(pmcdev->punit_ep);
> +
> platform_set_drvdata(pdev, NULL);
> mutex_destroy(&pmcdev->lock);
> }
> @@ -1232,6 +1285,8 @@ static int pmc_core_probe(struct platform_device *pdev)
> if (!pmcdev)
> return -ENOMEM;
>
> + pmcdev->crystal_freq = pmc_core_get_crystal_freq();
> +
> platform_set_drvdata(pdev, pmcdev);
> pmcdev->pdev = pdev;
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index 85b6f6ae4995..6d7673145f90 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -16,6 +16,8 @@
> #include <linux/bits.h>
> #include <linux/platform_device.h>
>
> +struct telem_endpoint;
> +
This seems unrelated to the patch.
--
i.
> #define SLP_S0_RES_COUNTER_MASK GENMASK(31, 0)
>
> #define PMC_BASE_ADDR_DEFAULT 0xFE000000
> @@ -357,6 +359,7 @@ struct pmc {
> * @devs: pointer to an array of pmc pointers
> * @pdev: pointer to platform_device struct
> * @ssram_pcidev: pointer to pci device struct for the PMC SSRAM
> + * @crystal_freq: crystal frequency from cpuid
> * @dbgfs_dir: path to debugfs interface
> * @pmc_xram_read_bit: flag to indicate whether PMC XRAM shadow registers
> * used to read MPHY PG and PLL status are available
> @@ -374,6 +377,7 @@ struct pmc_dev {
> struct dentry *dbgfs_dir;
> struct platform_device *pdev;
> struct pci_dev *ssram_pcidev;
> + unsigned int crystal_freq;
> int pmc_xram_read_bit;
> struct mutex lock; /* generic mutex lock for PMC Core */
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH V4 16/17] platform/x86/intel/pmc: Add debug attribute for Die C6 counter
2023-10-23 16:31 ` Ilpo Järvinen
@ 2023-10-26 20:28 ` David E. Box
0 siblings, 0 replies; 33+ messages in thread
From: David E. Box @ 2023-10-26 20:28 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: LKML, platform-driver-x86, rajvi.jingar
On Mon, 2023-10-23 at 19:31 +0300, Ilpo Järvinen wrote:
> On Wed, 18 Oct 2023, David E. Box wrote:
>
> > Add a "die_c6_us_show" debugfs attribute. Reads the counter value using
> > Intel Platform Monitoring Technology (PMT) driver API. This counter is
> > useful for determining the idle residency of CPUs in the compute tile.
> >
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > ---
> > V4 - no change
> >
> > V3 - Split previous PATCH V2 13. Separates implementation (this patch) from
> > platform specific use (next patch)
> >
> > V2 - Remove use of __func__
> > - Use HZ_PER_MHZ
> > - Fix missing newlines in printks
> >
> > drivers/platform/x86/intel/pmc/core.c | 55 +++++++++++++++++++++++++++
> > drivers/platform/x86/intel/pmc/core.h | 4 ++
> > 2 files changed, 59 insertions(+)
> >
> > diff --git a/drivers/platform/x86/intel/pmc/core.c
> > b/drivers/platform/x86/intel/pmc/core.c
> > index fcb0dc702aea..02f3e909cf22 100644
> > --- a/drivers/platform/x86/intel/pmc/core.c
> > +++ b/drivers/platform/x86/intel/pmc/core.c
> > @@ -20,6 +20,7 @@
> > #include <linux/pci.h>
> > #include <linux/slab.h>
> > #include <linux/suspend.h>
> > +#include <linux/units.h>
> >
> > #include <asm/cpu_device_id.h>
> > #include <asm/intel-family.h>
> > @@ -27,6 +28,7 @@
> > #include <asm/tsc.h>
> >
> > #include "core.h"
> > +#include "../pmt/telemetry.h"
> >
> > /* Maximum number of modes supported by platfoms that has low power mode
> > capability */
> > const char *pmc_lpm_modes[] = {
> > @@ -822,6 +824,47 @@ static int pmc_core_substate_req_regs_show(struct
> > seq_file *s, void *unused)
> > }
> > DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs);
> >
> > +static unsigned int pmc_core_get_crystal_freq(void)
> > +{
> > + unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
> > +
> > + if (boot_cpu_data.cpuid_level < 0x15)
> > + return 0;
> > +
> > + eax_denominator = ebx_numerator = ecx_hz = edx = 0;
> > +
> > + /* CPUID 15H TSC/Crystal ratio, plus optionally Crystal Hz */
> > + cpuid(0x15, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
> > +
> > + if (ebx_numerator == 0 || eax_denominator == 0)
> > + return 0;
> > +
> > + return ecx_hz;
> > +}
> > +
> > +static int pmc_core_die_c6_us_show(struct seq_file *s, void *unused)
> > +{
> > + struct pmc_dev *pmcdev = s->private;
> > + u64 die_c6_res, count;
> > + int ret;
> > +
> > + if (!pmcdev->crystal_freq) {
> > + dev_warn_once(&pmcdev->pdev->dev, "Bad crystal
> > frequency\n");
>
> Isn't it more like crystal frequency is not provided rather than bad
> frequency?
>
> > + return -EINVAL;
>
> -EINVAL is not good value to return here since there was nothing wrong
> with the input. Maybe -ENXIO would be better.
Will make these changes.
>
> > + }
> > +
> > + ret = pmt_telem_read(pmcdev->punit_ep, pmcdev->die_c6_offset,
> > + &count, 1);
> > + if (ret)
> > + return ret;
> > +
> > + die_c6_res = div64_u64(count * HZ_PER_MHZ, pmcdev->crystal_freq);
> > + seq_printf(s, "%llu\n", die_c6_res);
> > +
> > + return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(pmc_core_die_c6_us);
> > +
> > static int pmc_core_lpm_latch_mode_show(struct seq_file *s, void *unused)
> > {
> > struct pmc_dev *pmcdev = s->private;
> > @@ -1118,6 +1161,12 @@ static void pmc_core_dbgfs_register(struct pmc_dev
> > *pmcdev)
> > pmcdev->dbgfs_dir, pmcdev,
> > &pmc_core_substate_req_regs_fops);
> > }
> > +
> > + if (pmcdev->has_die_c6) {
> > + debugfs_create_file("die_c6_us_show", 0444,
> > + pmcdev->dbgfs_dir, pmcdev,
> > + &pmc_core_die_c6_us_fops);
> > + }
> > }
> >
> > static const struct x86_cpu_id intel_pmc_core_ids[] = {
> > @@ -1212,6 +1261,10 @@ static void pmc_core_clean_structure(struct
> > platform_device *pdev)
> > pci_dev_put(pmcdev->ssram_pcidev);
> > pci_disable_device(pmcdev->ssram_pcidev);
> > }
> > +
> > + if (pmcdev->punit_ep)
...
> > + pmt_telem_unregister_endpoint(pmcdev->punit_ep);
> > +
> > platform_set_drvdata(pdev, NULL);
> > mutex_destroy(&pmcdev->lock);
> > }
> > @@ -1232,6 +1285,8 @@ static int pmc_core_probe(struct platform_device
> > *pdev)
> > if (!pmcdev)
> > return -ENOMEM;
> >
> > + pmcdev->crystal_freq = pmc_core_get_crystal_freq();
> > +
> > platform_set_drvdata(pdev, pmcdev);
> > pmcdev->pdev = pdev;
> >
> > diff --git a/drivers/platform/x86/intel/pmc/core.h
> > b/drivers/platform/x86/intel/pmc/core.h
> > index 85b6f6ae4995..6d7673145f90 100644
> > --- a/drivers/platform/x86/intel/pmc/core.h
> > +++ b/drivers/platform/x86/intel/pmc/core.h
> > @@ -16,6 +16,8 @@
> > #include <linux/bits.h>
> > #include <linux/platform_device.h>
> >
> > +struct telem_endpoint;
> > +
>
> This seems unrelated to the patch.
This was missing when "struct telem_endpoint *punit_ep" was declared in this
header in an earlier patch that went upstream. But punit_ep does not get used
yet until this patch as shown above. Will clarify in the changelog.
David
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V4 17/17] platform/x86/intel/pmc: Show Die C6 counter on Meteor Lake
2023-10-18 23:16 [PATCH V4 00/17] intel_pmc: Add telemetry API to read counters David E. Box
` (15 preceding siblings ...)
2023-10-18 23:16 ` [PATCH V4 16/17] platform/x86/intel/pmc: Add debug attribute for Die C6 counter David E. Box
@ 2023-10-18 23:16 ` David E. Box
2023-10-23 16:33 ` Ilpo Järvinen
16 siblings, 1 reply; 33+ messages in thread
From: David E. Box @ 2023-10-18 23:16 UTC (permalink / raw)
To: linux-kernel, platform-driver-x86, ilpo.jarvinen, rajvi.jingar
Expose the Die C6 counter on Meteor Lake.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V4 - no change
V3 - Split PATCH V2 13. Separates implementation (previous patch)
from platform specific use (this patch)
drivers/platform/x86/intel/pmc/mtl.c | 32 ++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
index 7ceeae507f4c..38c2f946ec23 100644
--- a/drivers/platform/x86/intel/pmc/mtl.c
+++ b/drivers/platform/x86/intel/pmc/mtl.c
@@ -10,12 +10,17 @@
#include <linux/pci.h>
#include "core.h"
+#include "../pmt/telemetry.h"
/* PMC SSRAM PMT Telemetry GUIDS */
#define SOCP_LPM_REQ_GUID 0x2625030
#define IOEM_LPM_REQ_GUID 0x4357464
#define IOEP_LPM_REQ_GUID 0x5077612
+/* Die C6 from PUNIT telemetry */
+#define MTL_PMT_DMU_DIE_C6_OFFSET 15
+#define MTL_PMT_DMU_GUID 0x1A067102
+
static const u8 MTL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
/*
@@ -968,6 +973,32 @@ static struct pmc_info mtl_pmc_info_list[] = {
{}
};
+static void mtl_punit_pmt_init(struct pmc_dev *pmcdev)
+{
+ struct telem_endpoint *ep;
+ struct pci_dev *pcidev;
+
+ pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(10, 0));
+ if (!pcidev) {
+ dev_err(&pmcdev->pdev->dev, "PUNIT PMT device not found.\n");
+ return;
+ }
+
+ ep = pmt_telem_find_and_register_endpoint(pcidev, MTL_PMT_DMU_GUID, 0);
+ if (IS_ERR(ep)) {
+ dev_err(&pmcdev->pdev->dev,
+ "pmc_core: couldn't get DMU telem endpoint, %ld\n",
+ PTR_ERR(ep));
+ return;
+ }
+
+ pci_dev_put(pcidev);
+ pmcdev->punit_ep = ep;
+
+ pmcdev->has_die_c6 = true;
+ pmcdev->die_c6_offset = MTL_PMT_DMU_DIE_C6_OFFSET;
+}
+
#define MTL_GNA_PCI_DEV 0x7e4c
#define MTL_IPU_PCI_DEV 0x7d19
#define MTL_VPU_PCI_DEV 0x7d1d
@@ -1032,6 +1063,7 @@ int mtl_core_init(struct pmc_dev *pmcdev)
}
pmc_core_get_low_power_modes(pmcdev);
+ mtl_punit_pmt_init(pmcdev);
/* Due to a hardware limitation, the GBE LTR blocks PC10
* when a cable is attached. Tell the PMC to ignore it.
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V4 17/17] platform/x86/intel/pmc: Show Die C6 counter on Meteor Lake
2023-10-18 23:16 ` [PATCH V4 17/17] platform/x86/intel/pmc: Show Die C6 counter on Meteor Lake David E. Box
@ 2023-10-23 16:33 ` Ilpo Järvinen
0 siblings, 0 replies; 33+ messages in thread
From: Ilpo Järvinen @ 2023-10-23 16:33 UTC (permalink / raw)
To: David E. Box; +Cc: LKML, platform-driver-x86, rajvi.jingar
[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]
On Wed, 18 Oct 2023, David E. Box wrote:
> Expose the Die C6 counter on Meteor Lake.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V4 - no change
>
> V3 - Split PATCH V2 13. Separates implementation (previous patch)
> from platform specific use (this patch)
>
> drivers/platform/x86/intel/pmc/mtl.c | 32 ++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
> index 7ceeae507f4c..38c2f946ec23 100644
> --- a/drivers/platform/x86/intel/pmc/mtl.c
> +++ b/drivers/platform/x86/intel/pmc/mtl.c
> @@ -10,12 +10,17 @@
>
> #include <linux/pci.h>
> #include "core.h"
> +#include "../pmt/telemetry.h"
>
> /* PMC SSRAM PMT Telemetry GUIDS */
> #define SOCP_LPM_REQ_GUID 0x2625030
> #define IOEM_LPM_REQ_GUID 0x4357464
> #define IOEP_LPM_REQ_GUID 0x5077612
>
> +/* Die C6 from PUNIT telemetry */
> +#define MTL_PMT_DMU_DIE_C6_OFFSET 15
> +#define MTL_PMT_DMU_GUID 0x1A067102
> +
> static const u8 MTL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
>
> /*
> @@ -968,6 +973,32 @@ static struct pmc_info mtl_pmc_info_list[] = {
> {}
> };
>
> +static void mtl_punit_pmt_init(struct pmc_dev *pmcdev)
> +{
> + struct telem_endpoint *ep;
> + struct pci_dev *pcidev;
> +
> + pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(10, 0));
> + if (!pcidev) {
> + dev_err(&pmcdev->pdev->dev, "PUNIT PMT device not found.\n");
> + return;
> + }
> +
> + ep = pmt_telem_find_and_register_endpoint(pcidev, MTL_PMT_DMU_GUID, 0);
> + if (IS_ERR(ep)) {
> + dev_err(&pmcdev->pdev->dev,
> + "pmc_core: couldn't get DMU telem endpoint, %ld\n",
> + PTR_ERR(ep));
> + return;
> + }
> +
> + pci_dev_put(pcidev);
> + pmcdev->punit_ep = ep;
> +
> + pmcdev->has_die_c6 = true;
> + pmcdev->die_c6_offset = MTL_PMT_DMU_DIE_C6_OFFSET;
> +}
> +
> #define MTL_GNA_PCI_DEV 0x7e4c
> #define MTL_IPU_PCI_DEV 0x7d19
> #define MTL_VPU_PCI_DEV 0x7d1d
> @@ -1032,6 +1063,7 @@ int mtl_core_init(struct pmc_dev *pmcdev)
> }
>
> pmc_core_get_low_power_modes(pmcdev);
> + mtl_punit_pmt_init(pmcdev);
>
> /* Due to a hardware limitation, the GBE LTR blocks PC10
> * when a cable is attached. Tell the PMC to ignore it.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 33+ messages in thread