* [PATCH v6 1/6] platform/x86/intel/vsec.h: Move to include/linux
2024-07-10 19:22 [PATCH v6 0/6] Support PMT features in Xe Michael J. Ruhl
@ 2024-07-10 19:22 ` Michael J. Ruhl
2024-07-11 11:09 ` Ilpo Järvinen
2024-07-10 19:22 ` [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks Michael J. Ruhl
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Michael J. Ruhl @ 2024-07-10 19:22 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost
Cc: michael.j.ruhl
From: "David E. Box" <david.e.box@linux.intel.com>
Some drivers outside of PDX86 need access to the vsec header. Move it to
include/linux to make it easier to include.
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
MAINTAINERS | 1 +
drivers/platform/x86/intel/pmc/core_ssram.c | 2 +-
drivers/platform/x86/intel/pmt/class.c | 2 +-
drivers/platform/x86/intel/pmt/class.h | 2 +-
drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
drivers/platform/x86/intel/pmt/telemetry.c | 2 +-
drivers/platform/x86/intel/sdsi.c | 3 +-
drivers/platform/x86/intel/tpmi.c | 3 +-
drivers/platform/x86/intel/vsec.c | 7 ++--
.../vsec.h => include/linux/intel_vsec.h | 32 +++++++++++++++++--
10 files changed, 40 insertions(+), 16 deletions(-)
rename drivers/platform/x86/intel/vsec.h => include/linux/intel_vsec.h (71%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 782fb49059e7..832ab1a0011e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11426,6 +11426,7 @@ INTEL VENDOR SPECIFIC EXTENDED CAPABILITIES DRIVER
M: David E. Box <david.e.box@linux.intel.com>
S: Supported
F: drivers/platform/x86/intel/vsec.*
+F: include/linux/intel_vsec.h
INTEL VIRTUAL BUTTON DRIVER
M: AceLan Kao <acelan.kao@canonical.com>
diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
index 1bde86c54eb9..baddaaec25ee 100644
--- a/drivers/platform/x86/intel/pmc/core_ssram.c
+++ b/drivers/platform/x86/intel/pmc/core_ssram.c
@@ -9,11 +9,11 @@
*/
#include <linux/cleanup.h>
+#include <linux/intel_vsec.h>
#include <linux/pci.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include "core.h"
-#include "../vsec.h"
#include "../pmt/telemetry.h"
#define SSRAM_HDR_SIZE 0x100
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 4b53940a64e2..d7939b28e937 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -9,12 +9,12 @@
*/
#include <linux/kernel.h>
+#include <linux/intel_vsec.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/module.h>
#include <linux/mm.h>
#include <linux/pci.h>
-#include "../vsec.h"
#include "class.h"
#define PMT_XA_START 1
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index d23c63b73ab7..d6f9ccaf28c8 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -2,13 +2,13 @@
#ifndef _INTEL_PMT_CLASS_H
#define _INTEL_PMT_CLASS_H
+#include <linux/intel_vsec.h>
#include <linux/xarray.h>
#include <linux/types.h>
#include <linux/bits.h>
#include <linux/err.h>
#include <linux/io.h>
-#include "../vsec.h"
#include "telemetry.h"
/* PMT access types */
diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index 4014c02cafdb..9079d5dffc03 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -9,6 +9,7 @@
*/
#include <linux/auxiliary_bus.h>
+#include <linux/intel_vsec.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -16,7 +17,6 @@
#include <linux/uaccess.h>
#include <linux/overflow.h>
-#include "../vsec.h"
#include "class.h"
/* Crashlog discovery header types */
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index 09258564dfc4..3478f891ea0b 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -9,6 +9,7 @@
*/
#include <linux/auxiliary_bus.h>
+#include <linux/intel_vsec.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -16,7 +17,6 @@
#include <linux/uaccess.h>
#include <linux/overflow.h>
-#include "../vsec.h"
#include "class.h"
#define TELEM_SIZE_OFFSET 0x0
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 277e4f4b20ac..9d137621f0e6 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -12,6 +12,7 @@
#include <linux/bits.h>
#include <linux/bitfield.h>
#include <linux/device.h>
+#include <linux/intel_vsec.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -22,8 +23,6 @@
#include <linux/types.h>
#include <linux/uaccess.h>
-#include "vsec.h"
-
#define ACCESS_TYPE_BARID 2
#define ACCESS_TYPE_LOCAL 3
diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index 6c0cbccd80bb..b9fa1cbfdcf7 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -51,6 +51,7 @@
#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/intel_tpmi.h>
+#include <linux/intel_vsec.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/module.h>
@@ -59,8 +60,6 @@
#include <linux/sizes.h>
#include <linux/string_helpers.h>
-#include "vsec.h"
-
/**
* struct intel_tpmi_pfs_entry - TPMI PM Feature Structure (PFS) entry
* @tpmi_id: TPMI feature identifier (what the feature is and its data format).
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 0fdfaf3a4f5c..2b46807f868b 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -17,14 +17,13 @@
#include <linux/bits.h>
#include <linux/cleanup.h>
#include <linux/delay.h>
-#include <linux/kernel.h>
#include <linux/idr.h>
+#include <linux/intel_vsec.h>
+#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/types.h>
-#include "vsec.h"
-
#define PMT_XA_START 0
#define PMT_XA_MAX INT_MAX
#define PMT_XA_LIMIT XA_LIMIT(PMT_XA_START, PMT_XA_MAX)
@@ -341,7 +340,7 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
void intel_vsec_register(struct pci_dev *pdev,
struct intel_vsec_platform_info *info)
{
- if (!pdev || !info)
+ if (!pdev || !info || !info->headers)
return;
intel_vsec_walk_header(pdev, info);
diff --git a/drivers/platform/x86/intel/vsec.h b/include/linux/intel_vsec.h
similarity index 71%
rename from drivers/platform/x86/intel/vsec.h
rename to include/linux/intel_vsec.h
index e23e76129691..1a287541a2f9 100644
--- a/drivers/platform/x86/intel/vsec.h
+++ b/include/linux/intel_vsec.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _VSEC_H
-#define _VSEC_H
+#ifndef _INTEL_VSEC_H
+#define _INTEL_VSEC_H
#include <linux/auxiliary_bus.h>
#include <linux/bits.h>
@@ -67,7 +67,14 @@ enum intel_vsec_quirks {
VSEC_QUIRK_EARLY_HW = BIT(4),
};
-/* Platform specific data */
+/**
+ * struct intel_vsec_platform_info - Platform specific data
+ * @parent: parent device in the auxbus chain
+ * @headers: list of headers to define the PMT client devices to create
+ * @caps: bitmask of PMT capabilities for the given headers
+ * @quirks: bitmask of VSEC device quirks
+ * @base_addr: allow a base address to be specified (rather than derived)
+ */
struct intel_vsec_platform_info {
struct device *parent;
struct intel_vsec_header **headers;
@@ -76,6 +83,18 @@ struct intel_vsec_platform_info {
u64 base_addr;
};
+/**
+ * struct intel_sec_device - Auxbus specific device information
+ * @auxdev: auxbus device struct for auxbus access
+ * @pcidev: pci device associated with the device
+ * @resource: any resources shared by the parent
+ * @ida: id reference
+ * @num_resources: number of resources
+ * @id: xarrray id
+ * @priv_data: any private data needed
+ * @quirks: specified quirks
+ * @base_addr: base address of entries (if specified)
+ */
struct intel_vsec_device {
struct auxiliary_device auxdev;
struct pci_dev *pcidev;
@@ -103,6 +122,13 @@ static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device
return container_of(auxdev, struct intel_vsec_device, auxdev);
}
+#if IS_ENABLED(CONFIG_INTEL_VSEC)
void intel_vsec_register(struct pci_dev *pdev,
struct intel_vsec_platform_info *info);
+#else
+static inline void intel_vsec_register(struct pci_dev *pdev,
+ struct intel_vsec_platform_info *info)
+{
+}
+#endif
#endif
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v6 1/6] platform/x86/intel/vsec.h: Move to include/linux
2024-07-10 19:22 ` [PATCH v6 1/6] platform/x86/intel/vsec.h: Move to include/linux Michael J. Ruhl
@ 2024-07-11 11:09 ` Ilpo Järvinen
0 siblings, 0 replies; 15+ messages in thread
From: Ilpo Järvinen @ 2024-07-11 11:09 UTC (permalink / raw)
To: Michael J. Ruhl; +Cc: intel-xe, platform-driver-x86, david.e.box, matthew.brost
[-- Attachment #1: Type: text/plain, Size: 8627 bytes --]
On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
> From: "David E. Box" <david.e.box@linux.intel.com>
>
> Some drivers outside of PDX86 need access to the vsec header. Move it to
> include/linux to make it easier to include.
>
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> MAINTAINERS | 1 +
> drivers/platform/x86/intel/pmc/core_ssram.c | 2 +-
> drivers/platform/x86/intel/pmt/class.c | 2 +-
> drivers/platform/x86/intel/pmt/class.h | 2 +-
> drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
> drivers/platform/x86/intel/pmt/telemetry.c | 2 +-
> drivers/platform/x86/intel/sdsi.c | 3 +-
> drivers/platform/x86/intel/tpmi.c | 3 +-
> drivers/platform/x86/intel/vsec.c | 7 ++--
> .../vsec.h => include/linux/intel_vsec.h | 32 +++++++++++++++++--
> 10 files changed, 40 insertions(+), 16 deletions(-)
> rename drivers/platform/x86/intel/vsec.h => include/linux/intel_vsec.h (71%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 782fb49059e7..832ab1a0011e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11426,6 +11426,7 @@ INTEL VENDOR SPECIFIC EXTENDED CAPABILITIES DRIVER
> M: David E. Box <david.e.box@linux.intel.com>
> S: Supported
> F: drivers/platform/x86/intel/vsec.*
This can probably change to .c as the other file is now moved under
include/.
> +F: include/linux/intel_vsec.h
>
> INTEL VIRTUAL BUTTON DRIVER
> M: AceLan Kao <acelan.kao@canonical.com>
> diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
> index 1bde86c54eb9..baddaaec25ee 100644
> --- a/drivers/platform/x86/intel/pmc/core_ssram.c
> +++ b/drivers/platform/x86/intel/pmc/core_ssram.c
> @@ -9,11 +9,11 @@
> */
>
> #include <linux/cleanup.h>
> +#include <linux/intel_vsec.h>
> #include <linux/pci.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
>
> #include "core.h"
> -#include "../vsec.h"
> #include "../pmt/telemetry.h"
>
> #define SSRAM_HDR_SIZE 0x100
> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index 4b53940a64e2..d7939b28e937 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -9,12 +9,12 @@
> */
>
> #include <linux/kernel.h>
> +#include <linux/intel_vsec.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
> #include <linux/module.h>
> #include <linux/mm.h>
> #include <linux/pci.h>
>
> -#include "../vsec.h"
> #include "class.h"
>
> #define PMT_XA_START 1
> diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> index d23c63b73ab7..d6f9ccaf28c8 100644
> --- a/drivers/platform/x86/intel/pmt/class.h
> +++ b/drivers/platform/x86/intel/pmt/class.h
> @@ -2,13 +2,13 @@
> #ifndef _INTEL_PMT_CLASS_H
> #define _INTEL_PMT_CLASS_H
>
> +#include <linux/intel_vsec.h>
> #include <linux/xarray.h>
> #include <linux/types.h>
> #include <linux/bits.h>
> #include <linux/err.h>
> #include <linux/io.h>
>
> -#include "../vsec.h"
> #include "telemetry.h"
>
> /* PMT access types */
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index 4014c02cafdb..9079d5dffc03 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -9,6 +9,7 @@
> */
>
> #include <linux/auxiliary_bus.h>
> +#include <linux/intel_vsec.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> @@ -16,7 +17,6 @@
> #include <linux/uaccess.h>
> #include <linux/overflow.h>
>
> -#include "../vsec.h"
> #include "class.h"
>
> /* Crashlog discovery header types */
> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
> index 09258564dfc4..3478f891ea0b 100644
> --- a/drivers/platform/x86/intel/pmt/telemetry.c
> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> @@ -9,6 +9,7 @@
> */
>
> #include <linux/auxiliary_bus.h>
> +#include <linux/intel_vsec.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> @@ -16,7 +17,6 @@
> #include <linux/uaccess.h>
> #include <linux/overflow.h>
>
> -#include "../vsec.h"
> #include "class.h"
>
> #define TELEM_SIZE_OFFSET 0x0
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 277e4f4b20ac..9d137621f0e6 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -12,6 +12,7 @@
> #include <linux/bits.h>
> #include <linux/bitfield.h>
> #include <linux/device.h>
> +#include <linux/intel_vsec.h>
> #include <linux/iopoll.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> @@ -22,8 +23,6 @@
> #include <linux/types.h>
> #include <linux/uaccess.h>
>
> -#include "vsec.h"
> -
> #define ACCESS_TYPE_BARID 2
> #define ACCESS_TYPE_LOCAL 3
>
> diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
> index 6c0cbccd80bb..b9fa1cbfdcf7 100644
> --- a/drivers/platform/x86/intel/tpmi.c
> +++ b/drivers/platform/x86/intel/tpmi.c
> @@ -51,6 +51,7 @@
> #include <linux/debugfs.h>
> #include <linux/delay.h>
> #include <linux/intel_tpmi.h>
> +#include <linux/intel_vsec.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> #include <linux/module.h>
> @@ -59,8 +60,6 @@
> #include <linux/sizes.h>
> #include <linux/string_helpers.h>
>
> -#include "vsec.h"
> -
> /**
> * struct intel_tpmi_pfs_entry - TPMI PM Feature Structure (PFS) entry
> * @tpmi_id: TPMI feature identifier (what the feature is and its data format).
> diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
> index 0fdfaf3a4f5c..2b46807f868b 100644
> --- a/drivers/platform/x86/intel/vsec.c
> +++ b/drivers/platform/x86/intel/vsec.c
> @@ -17,14 +17,13 @@
> #include <linux/bits.h>
> #include <linux/cleanup.h>
> #include <linux/delay.h>
> -#include <linux/kernel.h>
> #include <linux/idr.h>
> +#include <linux/intel_vsec.h>
> +#include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> #include <linux/types.h>
>
> -#include "vsec.h"
> -
> #define PMT_XA_START 0
> #define PMT_XA_MAX INT_MAX
> #define PMT_XA_LIMIT XA_LIMIT(PMT_XA_START, PMT_XA_MAX)
> @@ -341,7 +340,7 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
> void intel_vsec_register(struct pci_dev *pdev,
> struct intel_vsec_platform_info *info)
> {
> - if (!pdev || !info)
> + if (!pdev || !info || !info->headers)
> return;
>
> intel_vsec_walk_header(pdev, info);
> diff --git a/drivers/platform/x86/intel/vsec.h b/include/linux/intel_vsec.h
> similarity index 71%
> rename from drivers/platform/x86/intel/vsec.h
> rename to include/linux/intel_vsec.h
> index e23e76129691..1a287541a2f9 100644
> --- a/drivers/platform/x86/intel/vsec.h
> +++ b/include/linux/intel_vsec.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _VSEC_H
> -#define _VSEC_H
> +#ifndef _INTEL_VSEC_H
> +#define _INTEL_VSEC_H
>
> #include <linux/auxiliary_bus.h>
> #include <linux/bits.h>
> @@ -67,7 +67,14 @@ enum intel_vsec_quirks {
> VSEC_QUIRK_EARLY_HW = BIT(4),
> };
>
> -/* Platform specific data */
> +/**
> + * struct intel_vsec_platform_info - Platform specific data
> + * @parent: parent device in the auxbus chain
> + * @headers: list of headers to define the PMT client devices to create
> + * @caps: bitmask of PMT capabilities for the given headers
> + * @quirks: bitmask of VSEC device quirks
> + * @base_addr: allow a base address to be specified (rather than derived)
> + */
> struct intel_vsec_platform_info {
> struct device *parent;
> struct intel_vsec_header **headers;
> @@ -76,6 +83,18 @@ struct intel_vsec_platform_info {
> u64 base_addr;
> };
>
> +/**
> + * struct intel_sec_device - Auxbus specific device information
> + * @auxdev: auxbus device struct for auxbus access
> + * @pcidev: pci device associated with the device
> + * @resource: any resources shared by the parent
> + * @ida: id reference
> + * @num_resources: number of resources
> + * @id: xarrray id
xarray
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks
2024-07-10 19:22 [PATCH v6 0/6] Support PMT features in Xe Michael J. Ruhl
2024-07-10 19:22 ` [PATCH v6 1/6] platform/x86/intel/vsec.h: Move to include/linux Michael J. Ruhl
@ 2024-07-10 19:22 ` Michael J. Ruhl
2024-07-11 11:16 ` Ilpo Järvinen
2024-07-10 19:22 ` [PATCH v6 3/6] platform/x86/intel/pmt: Use PMT callbacks Michael J. Ruhl
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Michael J. Ruhl @ 2024-07-10 19:22 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost
Cc: michael.j.ruhl
From: "David E. Box" <david.e.box@linux.intel.com>
Some PMT providers require device specific actions before their telemetry
can be read. Provide assignable PMT read callbacks to allow providers to
perform those actions.
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/vsec.c | 1 +
include/linux/intel_vsec.h | 14 ++++++++++++++
2 files changed, 15 insertions(+)
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 2b46807f868b..7b5cc9993974 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
intel_vsec_dev->num_resources = header->num_entries;
intel_vsec_dev->quirks = info->quirks;
intel_vsec_dev->base_addr = info->base_addr;
+ intel_vsec_dev->priv_data = info->priv_data;
if (header->id == VSEC_ID_SDSI)
intel_vsec_dev->ida = &intel_vsec_sdsi_ida;
diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
index 1a287541a2f9..4569a55e8645 100644
--- a/include/linux/intel_vsec.h
+++ b/include/linux/intel_vsec.h
@@ -67,6 +67,19 @@ enum intel_vsec_quirks {
VSEC_QUIRK_EARLY_HW = BIT(4),
};
+/**
+ * struct pmt_callbacks - Callback infrastructure for PMT devices
+ * ->read_telem() when specified, called by client driver to access PMT data (instead
+ * of direct copy).
+ * @args: pci device info pointer
+ * @guid: ID of data to acccss
+ * @data: buffer for the data to be copied
+ * @count: size of buffer
+ */
+struct pmt_callbacks {
+ int (*read_telem)(void *args, u32 guid, u64 *data, u32 count);
+};
+
/**
* struct intel_vsec_platform_info - Platform specific data
* @parent: parent device in the auxbus chain
@@ -78,6 +91,7 @@ enum intel_vsec_quirks {
struct intel_vsec_platform_info {
struct device *parent;
struct intel_vsec_header **headers;
+ void *priv_data;
unsigned long caps;
unsigned long quirks;
u64 base_addr;
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks
2024-07-10 19:22 ` [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks Michael J. Ruhl
@ 2024-07-11 11:16 ` Ilpo Järvinen
2024-07-11 15:57 ` Ruhl, Michael J
0 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2024-07-11 11:16 UTC (permalink / raw)
To: Michael J. Ruhl; +Cc: intel-xe, platform-driver-x86, david.e.box, matthew.brost
On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
> From: "David E. Box" <david.e.box@linux.intel.com>
>
> Some PMT providers require device specific actions before their telemetry
> can be read. Provide assignable PMT read callbacks to allow providers to
> perform those actions.
>
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> drivers/platform/x86/intel/vsec.c | 1 +
> include/linux/intel_vsec.h | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
> index 2b46807f868b..7b5cc9993974 100644
> --- a/drivers/platform/x86/intel/vsec.c
> +++ b/drivers/platform/x86/intel/vsec.c
> @@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
> intel_vsec_dev->num_resources = header->num_entries;
> intel_vsec_dev->quirks = info->quirks;
> intel_vsec_dev->base_addr = info->base_addr;
> + intel_vsec_dev->priv_data = info->priv_data;
>
> if (header->id == VSEC_ID_SDSI)
> intel_vsec_dev->ida = &intel_vsec_sdsi_ida;
> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
> index 1a287541a2f9..4569a55e8645 100644
> --- a/include/linux/intel_vsec.h
> +++ b/include/linux/intel_vsec.h
> @@ -67,6 +67,19 @@ enum intel_vsec_quirks {
> VSEC_QUIRK_EARLY_HW = BIT(4),
> };
>
> +/**
> + * struct pmt_callbacks - Callback infrastructure for PMT devices
> + * ->read_telem() when specified, called by client driver to access PMT data (instead
> + * of direct copy).
> + * @args: pci device info pointer
PCI
> + * @guid: ID of data to acccss
> + * @data: buffer for the data to be copied
> + * @count: size of buffer
> + */
> +struct pmt_callbacks {
> + int (*read_telem)(void *args, u32 guid, u64 *data, u32 count);
Can you explain why args is void * and not struct pci_dev *? I was already
unsure earlier why void * is needed but now you even explicitly documented
it to be "pci device info pointer".
> +};
> +
> /**
> * struct intel_vsec_platform_info - Platform specific data
> * @parent: parent device in the auxbus chain
> @@ -78,6 +91,7 @@ enum intel_vsec_quirks {
> struct intel_vsec_platform_info {
> struct device *parent;
> struct intel_vsec_header **headers;
> + void *priv_data;
> unsigned long caps;
> unsigned long quirks;
> u64 base_addr;
>
--
i.
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks
2024-07-11 11:16 ` Ilpo Järvinen
@ 2024-07-11 15:57 ` Ruhl, Michael J
0 siblings, 0 replies; 15+ messages in thread
From: Ruhl, Michael J @ 2024-07-11 15:57 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, david.e.box@linux.intel.com,
Brost, Matthew
>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>Sent: Thursday, July 11, 2024 7:17 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org;
>david.e.box@linux.intel.com; Brost, Matthew <matthew.brost@intel.com>
>Subject: Re: [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks
>
>On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
>
>> From: "David E. Box" <david.e.box@linux.intel.com>
>>
>> Some PMT providers require device specific actions before their telemetry
>> can be read. Provide assignable PMT read callbacks to allow providers to
>> perform those actions.
>>
>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>> ---
>> drivers/platform/x86/intel/vsec.c | 1 +
>> include/linux/intel_vsec.h | 14 ++++++++++++++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/vsec.c
>b/drivers/platform/x86/intel/vsec.c
>> index 2b46807f868b..7b5cc9993974 100644
>> --- a/drivers/platform/x86/intel/vsec.c
>> +++ b/drivers/platform/x86/intel/vsec.c
>> @@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
>struct intel_vsec_header *he
>> intel_vsec_dev->num_resources = header->num_entries;
>> intel_vsec_dev->quirks = info->quirks;
>> intel_vsec_dev->base_addr = info->base_addr;
>> + intel_vsec_dev->priv_data = info->priv_data;
>>
>> if (header->id == VSEC_ID_SDSI)
>> intel_vsec_dev->ida = &intel_vsec_sdsi_ida;
>> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
>> index 1a287541a2f9..4569a55e8645 100644
>> --- a/include/linux/intel_vsec.h
>> +++ b/include/linux/intel_vsec.h
>> @@ -67,6 +67,19 @@ enum intel_vsec_quirks {
>> VSEC_QUIRK_EARLY_HW = BIT(4),
>> };
>>
>> +/**
>> + * struct pmt_callbacks - Callback infrastructure for PMT devices
>> + * ->read_telem() when specified, called by client driver to access PMT data
>(instead
>> + * of direct copy).
>> + * @args: pci device info pointer
>
>PCI
>
>> + * @guid: ID of data to acccss
>> + * @data: buffer for the data to be copied
>> + * @count: size of buffer
>> + */
>> +struct pmt_callbacks {
>> + int (*read_telem)(void *args, u32 guid, u64 *data, u32 count);
>
>Can you explain why args is void * and not struct pci_dev *? I was already
>unsure earlier why void * is needed but now you even explicitly documented
>it to be "pci device info pointer".
This was an artifact of a previous attempt. I was going to use an opaque pointer
to pass the device info, but David pointed out that the PCI device was available
and sufficient.
I have updated to use struct pce_dev.
Thanks!
M
>> +};
>> +
>> /**
>> * struct intel_vsec_platform_info - Platform specific data
>> * @parent: parent device in the auxbus chain
>> @@ -78,6 +91,7 @@ enum intel_vsec_quirks {
>> struct intel_vsec_platform_info {
>> struct device *parent;
>> struct intel_vsec_header **headers;
>> + void *priv_data;
>> unsigned long caps;
>> unsigned long quirks;
>> u64 base_addr;
>>
>
>--
> i.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 3/6] platform/x86/intel/pmt: Use PMT callbacks
2024-07-10 19:22 [PATCH v6 0/6] Support PMT features in Xe Michael J. Ruhl
2024-07-10 19:22 ` [PATCH v6 1/6] platform/x86/intel/vsec.h: Move to include/linux Michael J. Ruhl
2024-07-10 19:22 ` [PATCH v6 2/6] platform/x86/intel/vsec: Add PMT read callbacks Michael J. Ruhl
@ 2024-07-10 19:22 ` Michael J. Ruhl
2024-07-10 19:22 ` [PATCH v6 4/6] drm/xe/vsec: Support BMG devices Michael J. Ruhl
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Michael J. Ruhl @ 2024-07-10 19:22 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost
Cc: michael.j.ruhl
From: "David E. Box" <david.e.box@linux.intel.com>
PMT providers may require device specific actions before their telemetry
may be read. If the read_telem() is assigned, call it instead of
memcpy_fromio() and return. Since this needs to be done in multiple
locations, add pmt_telem_read_mmio() as a wrapper function to perform this
and any other needed checks.
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/pmt/class.c | 26 +++++++++++++++++-----
drivers/platform/x86/intel/pmt/class.h | 8 +++++--
drivers/platform/x86/intel/pmt/telemetry.c | 10 +++++----
3 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index d7939b28e937..c04bb7f97a4d 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -58,6 +58,22 @@ pmt_memcpy64_fromio(void *to, const u64 __iomem *from, size_t count)
return count;
}
+int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb, u32 guid, void *buf,
+ void __iomem *addr, u32 count)
+{
+ if (cb && cb->read_telem)
+ return cb->read_telem(pdev, guid, buf, count);
+
+ if (guid == GUID_SPR_PUNIT)
+ /* PUNIT on SPR only supports aligned 64-bit read */
+ return pmt_memcpy64_fromio(buf, addr, count);
+
+ memcpy_fromio(buf, addr, count);
+
+ return count;
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_read_mmio, INTEL_PMT);
+
/*
* sysfs
*/
@@ -79,11 +95,8 @@ intel_pmt_read(struct file *filp, struct kobject *kobj,
if (count > entry->size - off)
count = entry->size - off;
- if (entry->guid == GUID_SPR_PUNIT)
- /* PUNIT on SPR only supports aligned 64-bit read */
- count = pmt_memcpy64_fromio(buf, entry->base + off, count);
- else
- memcpy_fromio(buf, entry->base + off, count);
+ count = pmt_telem_read_mmio(entry->ep->pcidev, entry->cb, entry->header.guid, buf,
+ entry->base + off, count);
return count;
}
@@ -239,6 +252,7 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
entry->guid = header->guid;
entry->size = header->size;
+ entry->cb = ivdev->priv_data;
return 0;
}
@@ -300,7 +314,7 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
goto fail_ioremap;
if (ns->pmt_add_endpoint) {
- ret = ns->pmt_add_endpoint(entry, ivdev->pcidev);
+ ret = ns->pmt_add_endpoint(ivdev, entry);
if (ret)
goto fail_add_endpoint;
}
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index d6f9ccaf28c8..a267ac964423 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -24,6 +24,7 @@ struct pci_dev;
struct telem_endpoint {
struct pci_dev *pcidev;
struct telem_header header;
+ struct pmt_callbacks *cb;
void __iomem *base;
bool present;
struct kref kref;
@@ -43,6 +44,7 @@ struct intel_pmt_entry {
struct kobject *kobj;
void __iomem *disc_table;
void __iomem *base;
+ struct pmt_callbacks *cb;
unsigned long base_addr;
size_t size;
u32 guid;
@@ -55,10 +57,12 @@ 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);
+ int (*pmt_add_endpoint)(struct intel_vsec_device *ivdev,
+ struct intel_pmt_entry *entry);
};
+int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb, u32 guid, void *buf,
+ void __iomem *addr, u32 count);
bool intel_pmt_is_early_client_hw(struct device *dev);
int intel_pmt_dev_create(struct intel_pmt_entry *entry,
struct intel_pmt_namespace *ns,
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index 3478f891ea0b..c9feac859e57 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -93,8 +93,8 @@ 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)
+static int pmt_telem_add_endpoint(struct intel_vsec_device *ivdev,
+ struct intel_pmt_entry *entry)
{
struct telem_endpoint *ep;
@@ -104,13 +104,14 @@ static int pmt_telem_add_endpoint(struct intel_pmt_entry *entry,
return -ENOMEM;
ep = entry->ep;
- ep->pcidev = pdev;
+ ep->pcidev = ivdev->pcidev;
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;
+ ep->cb = ivdev->priv_data;
kref_init(&ep->kref);
@@ -218,7 +219,8 @@ int pmt_telem_read(struct telem_endpoint *ep, u32 id, u64 *data, u32 count)
if (offset + NUM_BYTES_QWORD(count) > size)
return -EINVAL;
- memcpy_fromio(data, ep->base + offset, NUM_BYTES_QWORD(count));
+ pmt_telem_read_mmio(ep->pcidev, ep->cb, ep->header.guid, data, ep->base + offset,
+ NUM_BYTES_QWORD(count));
return ep->present ? 0 : -EPIPE;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v6 4/6] drm/xe/vsec: Support BMG devices
2024-07-10 19:22 [PATCH v6 0/6] Support PMT features in Xe Michael J. Ruhl
` (2 preceding siblings ...)
2024-07-10 19:22 ` [PATCH v6 3/6] platform/x86/intel/pmt: Use PMT callbacks Michael J. Ruhl
@ 2024-07-10 19:22 ` Michael J. Ruhl
2024-07-10 19:22 ` [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
2024-07-10 19:22 ` [PATCH v6 6/6] drm/xe/vsec: Add support for DG2 Michael J. Ruhl
5 siblings, 0 replies; 15+ messages in thread
From: Michael J. Ruhl @ 2024-07-10 19:22 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost
Cc: michael.j.ruhl
Utilize the PMT callback API to add support for the BMG
devices.
Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device.c | 5 +
drivers/gpu/drm/xe/xe_device_types.h | 6 +
drivers/gpu/drm/xe/xe_vsec.c | 222 +++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_vsec.h | 13 ++
5 files changed, 247 insertions(+)
create mode 100644 drivers/gpu/drm/xe/xe_vsec.c
create mode 100644 drivers/gpu/drm/xe/xe_vsec.h
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 1ff9602a52f6..a3c044b46fed 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -112,6 +112,7 @@ xe-y += xe_bb.o \
xe_vm.o \
xe_vram.o \
xe_vram_freq.o \
+ xe_vsec.o \
xe_wait_user_fence.o \
xe_wa.o \
xe_wopcm.o
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 64aea962afd5..c003eafdd873 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -53,6 +53,7 @@
#include "xe_ttm_sys_mgr.h"
#include "xe_vm.h"
#include "xe_vram.h"
+#include "xe_vsec.h"
#include "xe_wait_user_fence.h"
#include "xe_wa.h"
@@ -320,6 +321,8 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
goto err;
}
+ drmm_mutex_init(&xe->drm, &xe->pmt.lock);
+
err = xe_display_create(xe);
if (WARN_ON(err))
goto err;
@@ -695,6 +698,8 @@ int xe_device_probe(struct xe_device *xe)
for_each_gt(gt, xe, id)
xe_gt_sanitize_freq(gt);
+ xe_vsec_init(xe);
+
return devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
err_fini_display:
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index f0cf9020e463..f443af2cf0b2 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -455,6 +455,12 @@ struct xe_device {
struct mutex lock;
} d3cold;
+ /** @pmt: Support the PMT driver callback interface */
+ struct {
+ /** @pmt.lock: protect access for telemetry data */
+ struct mutex lock;
+ } pmt;
+
/**
* @pm_callback_task: Track the active task that is running in either
* the runtime_suspend or runtime_resume callbacks.
diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
new file mode 100644
index 000000000000..98999d467db1
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_vsec.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2022 - 2024 Intel Corporation
+ */
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/intel_vsec.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+
+#include "xe_device.h"
+#include "xe_device_types.h"
+#include "xe_drv.h"
+#include "xe_mmio.h"
+#include "xe_platform_types.h"
+#include "xe_pm.h"
+#include "xe_vsec.h"
+
+#define SOC_BASE 0x280000
+
+#define BMG_PMT_BASE 0xDB000
+#define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
+
+#define BMG_TELEMETRY_BASE 0xE0000
+#define BMG_TELEMETRY_OFFSET (SOC_BASE + BMG_TELEMETRY_BASE)
+
+#define BMG_DEVICE_ID 0xE2F8
+
+#define GFX_BAR 0
+
+#define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
+#define SG_REMAP_BITS GENMASK(31, 24)
+
+static struct intel_vsec_header bmg_telemetry = {
+ .length = 0x10,
+ .id = VSEC_ID_TELEMETRY,
+ .num_entries = 2,
+ .entry_size = 4,
+ .tbir = GFX_BAR,
+ .offset = BMG_DISCOVERY_OFFSET,
+};
+
+static struct intel_vsec_header *bmg_capabilities[] = {
+ &bmg_telemetry,
+ NULL
+};
+
+enum xe_vsec {
+ XE_VSEC_UNKNOWN = 0,
+ XE_VSEC_BMG,
+};
+
+static struct intel_vsec_platform_info xe_vsec_info[] = {
+ [XE_VSEC_BMG] = {
+ .caps = VSEC_CAP_TELEMETRY,
+ .headers = bmg_capabilities,
+ },
+ { }
+};
+
+/*
+ * The GUID will have the following bits to decode:
+ *
+ * X(4bits) - {Telemetry space iteration number (0,1,..)}
+ * X(4bits) - Segment (SEGMENT_INDEPENDENT-0, Client-1, Server-2)
+ * X(4bits) - SOC_SKU (SKU_INDEPENDENT-0, X3-1, X2-2, G31-3),
+ * XXXX(16bits)– Device ID – changes for each down bin SKU’s (0xE2F8 for BMG)
+ * X(2bits) - Capability Type (Crashlog-0, Telemetry Aggregator-1, Watcher-2)
+ * X(2bits) - Record-ID (0-PUNIT, 1-OOBMSM_0, 2-OOBMSM_1)
+ */
+#define GUID_TELEM_ITERATION GENMASK(3, 0)
+#define GUID_SEGMENT GENMASK(7, 4)
+#define GUID_SOC_SKU GENMASK(11, 8)
+#define GUID_DEVICE_ID GENMASK(27, 12)
+#define GUID_CAP_TYPE GENMASK(29, 28)
+#define GUID_RECORD_ID GENMASK(31, 30)
+
+#define PUNIT_TELEMETRY_OFFSET 0x0200
+#define PUNIT_WATCHER_OFFSET 0x14A0
+#define OOBMSM_0_WATCHER_OFFSET 0x18D8
+#define OOBMSM_1_TELEMETRY_OFFSET 0x1000
+
+enum record_id {
+ PUNIT,
+ OOBMSM_0,
+ OOBMSM_1
+};
+
+enum capability {
+ CRASHLOG,
+ TELEMETRY,
+ WATCHER
+};
+
+static int guid_decode(u32 guid, int *index, u32 *offset)
+{
+ u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
+ u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
+ u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
+
+ if (device_id != BMG_DEVICE_ID)
+ return -ENODEV;
+
+ if (record_id > OOBMSM_1 || cap_type > WATCHER)
+ return -EINVAL;
+
+ *offset = 0;
+
+ if (cap_type == CRASHLOG) {
+ *index = record_id == PUNIT ? 2 : 4;
+ return 0;
+ }
+
+ switch (record_id) {
+ case PUNIT:
+ *index = 0;
+ if (cap_type == TELEMETRY)
+ *offset = PUNIT_TELEMETRY_OFFSET;
+ else
+ *offset = PUNIT_WATCHER_OFFSET;
+ break;
+
+ case OOBMSM_0:
+ *index = 1;
+ if (cap_type == WATCHER)
+ *offset = OOBMSM_0_WATCHER_OFFSET;
+ break;
+
+ case OOBMSM_1:
+ *index = 1;
+ if (cap_type == TELEMETRY)
+ *offset = OOBMSM_1_TELEMETRY_OFFSET;
+ break;
+ }
+
+ return 0;
+}
+
+static int xe_pmt_telem_read(void *args, u32 guid, u64 *data, u32 count)
+{
+ struct xe_device *xe = pdev_to_xe_device(args);
+ void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
+ u32 mem_region;
+ u32 offset;
+ int ret;
+
+ ret = guid_decode(guid, &mem_region, &offset);
+ if (ret)
+ return ret;
+
+ telem_addr += offset;
+
+ mutex_lock(&xe->pmt.lock);
+
+ /* indicate that we are not at an appropriate power level */
+ ret = -ENODATA;
+ if (xe_pm_runtime_get_if_active(xe) > 0) {
+ /* set SoC re-mapper index register based on GUID memory region */
+ xe_mmio_rmw32(xe->tiles[0].primary_gt, SG_REMAP_INDEX1, SG_REMAP_BITS,
+ FIELD_PREP(SG_REMAP_BITS, mem_region));
+
+ memcpy_fromio(data, telem_addr, count);
+ ret = count;
+ xe_pm_runtime_put(xe);
+ }
+ mutex_unlock(&xe->pmt.lock);
+
+ return ret;
+}
+
+struct pmt_callbacks xe_pmt_cb = {
+ .read_telem = xe_pmt_telem_read,
+};
+
+static const int vsec_platforms[] = {
+ [XE_BATTLEMAGE] = XE_VSEC_BMG,
+};
+
+static enum xe_vsec get_platform_info(struct xe_device *xe)
+{
+ if (xe->info.platform > XE_BATTLEMAGE)
+ return XE_VSEC_UNKNOWN;
+
+ return vsec_platforms[xe->info.platform];
+}
+
+/**
+ * xe_vsec_init - Initialize resources and add intel_vsec auxiliary
+ * interface
+ * @xe: valid xe instance
+ */
+void xe_vsec_init(struct xe_device *xe)
+{
+ struct intel_vsec_platform_info *info;
+ struct device *dev = xe->drm.dev;
+ struct pci_dev *pdev = to_pci_dev(dev);
+ enum xe_vsec platform;
+
+ platform = get_platform_info(xe);
+ if (platform == XE_VSEC_UNKNOWN)
+ return;
+
+ info = &xe_vsec_info[platform];
+ if (!info->headers)
+ return;
+
+ switch (platform) {
+ case XE_VSEC_BMG:
+ info->priv_data = &xe_pmt_cb;
+ break;
+ default:
+ break;
+ }
+
+ /*
+ * Register a VSEC. Cleanup is handled using device managed
+ * resources.
+ */
+ intel_vsec_register(pdev, info);
+}
+MODULE_IMPORT_NS(INTEL_VSEC);
diff --git a/drivers/gpu/drm/xe/xe_vsec.h b/drivers/gpu/drm/xe/xe_vsec.h
new file mode 100644
index 000000000000..3fd29a21cad6
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_vsec.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright © 2022 - 2024 Intel Corporation
+ */
+
+#ifndef _XE_VSEC_H_
+#define _XE_VSEC_H_
+
+struct xe_device;
+
+void xe_vsec_init(struct xe_device *xe);
+
+#endif
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-10 19:22 [PATCH v6 0/6] Support PMT features in Xe Michael J. Ruhl
` (3 preceding siblings ...)
2024-07-10 19:22 ` [PATCH v6 4/6] drm/xe/vsec: Support BMG devices Michael J. Ruhl
@ 2024-07-10 19:22 ` Michael J. Ruhl
2024-07-11 11:34 ` Ilpo Järvinen
2024-07-10 19:22 ` [PATCH v6 6/6] drm/xe/vsec: Add support for DG2 Michael J. Ruhl
5 siblings, 1 reply; 15+ messages in thread
From: Michael J. Ruhl @ 2024-07-10 19:22 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost
Cc: michael.j.ruhl
DVSEC offsets are based on the endpoint BAR. If an endpoint is
not avialable allow the offset information to be adjusted by the
parent driver.
Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
drivers/platform/x86/intel/pmt/class.h | 1 +
drivers/platform/x86/intel/pmt/telemetry.c | 9 +++++++++
drivers/platform/x86/intel/vsec.c | 1 +
include/linux/intel_vsec.h | 2 ++
4 files changed, 13 insertions(+)
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index a267ac964423..984cd40ee814 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -46,6 +46,7 @@ struct intel_pmt_entry {
void __iomem *base;
struct pmt_callbacks *cb;
unsigned long base_addr;
+ s32 base_adjust;
size_t size;
u32 guid;
int devid;
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index c9feac859e57..5c44e500e8f6 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -78,6 +78,13 @@ static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
header->access_type = TELEM_ACCESS(readl(disc_table));
header->guid = readl(disc_table + TELEM_GUID_OFFSET);
header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
+ if (entry->base_adjust) {
+ u32 new_base = header->base_offset + entry->base_adjust;
+
+ dev_dbg(dev, "Adjusting baseoffset from 0x%x to 0x%x\n",
+ header->base_offset, new_base);
+ header->base_offset = new_base;
+ }
/* Size is measured in DWORDS, but accessor returns bytes */
header->size = TELEM_SIZE(readl(disc_table));
@@ -302,6 +309,8 @@ 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];
+ entry->base_adjust = intel_vsec_dev->base_adjust;
+
mutex_lock(&ep_lock);
ret = intel_pmt_dev_create(entry, &pmt_telem_ns, intel_vsec_dev, i);
mutex_unlock(&ep_lock);
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 7b5cc9993974..be079d62a7bc 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
intel_vsec_dev->num_resources = header->num_entries;
intel_vsec_dev->quirks = info->quirks;
intel_vsec_dev->base_addr = info->base_addr;
+ intel_vsec_dev->base_adjust = info->base_adjust;
intel_vsec_dev->priv_data = info->priv_data;
if (header->id == VSEC_ID_SDSI)
diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
index 4569a55e8645..1fd0fcc5615d 100644
--- a/include/linux/intel_vsec.h
+++ b/include/linux/intel_vsec.h
@@ -95,6 +95,7 @@ struct intel_vsec_platform_info {
unsigned long caps;
unsigned long quirks;
u64 base_addr;
+ s32 base_adjust;
};
/**
@@ -120,6 +121,7 @@ struct intel_vsec_device {
size_t priv_data_size;
unsigned long quirks;
u64 base_addr;
+ s32 base_adjust;
};
int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-10 19:22 ` [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
@ 2024-07-11 11:34 ` Ilpo Järvinen
2024-07-11 20:56 ` Ruhl, Michael J
0 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2024-07-11 11:34 UTC (permalink / raw)
To: Michael J. Ruhl; +Cc: intel-xe, platform-driver-x86, david.e.box, matthew.brost
On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
> DVSEC offsets are based on the endpoint BAR. If an endpoint is
> not avialable allow the offset information to be adjusted by the
available
> parent driver.
>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
> drivers/platform/x86/intel/pmt/class.h | 1 +
> drivers/platform/x86/intel/pmt/telemetry.c | 9 +++++++++
> drivers/platform/x86/intel/vsec.c | 1 +
> include/linux/intel_vsec.h | 2 ++
> 4 files changed, 13 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> index a267ac964423..984cd40ee814 100644
> --- a/drivers/platform/x86/intel/pmt/class.h
> +++ b/drivers/platform/x86/intel/pmt/class.h
> @@ -46,6 +46,7 @@ struct intel_pmt_entry {
> void __iomem *base;
> struct pmt_callbacks *cb;
> unsigned long base_addr;
> + s32 base_adjust;
> size_t size;
> u32 guid;
> int devid;
> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
> index c9feac859e57..5c44e500e8f6 100644
> --- a/drivers/platform/x86/intel/pmt/telemetry.c
> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> @@ -78,6 +78,13 @@ static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
> header->access_type = TELEM_ACCESS(readl(disc_table));
> header->guid = readl(disc_table + TELEM_GUID_OFFSET);
> header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
> + if (entry->base_adjust) {
> + u32 new_base = header->base_offset + entry->base_adjust;
The code setting ->base_adjust is responsible for avoiding stupid settings
that would lead to underflows and overflows?
> +
> + dev_dbg(dev, "Adjusting baseoffset from 0x%x to 0x%x\n",
base offset
--
i.
> + header->base_offset, new_base);
> + header->base_offset = new_base;
> + }
>
> /* Size is measured in DWORDS, but accessor returns bytes */
> header->size = TELEM_SIZE(readl(disc_table));
> @@ -302,6 +309,8 @@ 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];
>
> + entry->base_adjust = intel_vsec_dev->base_adjust;
> +
> mutex_lock(&ep_lock);
> ret = intel_pmt_dev_create(entry, &pmt_telem_ns, intel_vsec_dev, i);
> mutex_unlock(&ep_lock);
> diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
> index 7b5cc9993974..be079d62a7bc 100644
> --- a/drivers/platform/x86/intel/vsec.c
> +++ b/drivers/platform/x86/intel/vsec.c
> @@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev, struct intel_vsec_header *he
> intel_vsec_dev->num_resources = header->num_entries;
> intel_vsec_dev->quirks = info->quirks;
> intel_vsec_dev->base_addr = info->base_addr;
> + intel_vsec_dev->base_adjust = info->base_adjust;
> intel_vsec_dev->priv_data = info->priv_data;
>
> if (header->id == VSEC_ID_SDSI)
> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
> index 4569a55e8645..1fd0fcc5615d 100644
> --- a/include/linux/intel_vsec.h
> +++ b/include/linux/intel_vsec.h
> @@ -95,6 +95,7 @@ struct intel_vsec_platform_info {
> unsigned long caps;
> unsigned long quirks;
> u64 base_addr;
> + s32 base_adjust;
> };
>
> /**
> @@ -120,6 +121,7 @@ struct intel_vsec_device {
> size_t priv_data_size;
> unsigned long quirks;
> u64 base_addr;
> + s32 base_adjust;
> };
>
> int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
>
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-11 11:34 ` Ilpo Järvinen
@ 2024-07-11 20:56 ` Ruhl, Michael J
2024-07-12 8:31 ` Ilpo Järvinen
0 siblings, 1 reply; 15+ messages in thread
From: Ruhl, Michael J @ 2024-07-11 20:56 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, david.e.box@linux.intel.com,
Brost, Matthew
>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>Sent: Thursday, July 11, 2024 7:34 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org;
>david.e.box@linux.intel.com; Brost, Matthew <matthew.brost@intel.com>
>Subject: Re: [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT
>base adjust
>
>On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
>
>> DVSEC offsets are based on the endpoint BAR. If an endpoint is
>> not avialable allow the offset information to be adjusted by the
>
>available
>
>> parent driver.
>>
>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> ---
>> drivers/platform/x86/intel/pmt/class.h | 1 +
>> drivers/platform/x86/intel/pmt/telemetry.c | 9 +++++++++
>> drivers/platform/x86/intel/vsec.c | 1 +
>> include/linux/intel_vsec.h | 2 ++
>> 4 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/pmt/class.h
>b/drivers/platform/x86/intel/pmt/class.h
>> index a267ac964423..984cd40ee814 100644
>> --- a/drivers/platform/x86/intel/pmt/class.h
>> +++ b/drivers/platform/x86/intel/pmt/class.h
>> @@ -46,6 +46,7 @@ struct intel_pmt_entry {
>> void __iomem *base;
>> struct pmt_callbacks *cb;
>> unsigned long base_addr;
>> + s32 base_adjust;
>> size_t size;
>> u32 guid;
>> int devid;
>> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c
>b/drivers/platform/x86/intel/pmt/telemetry.c
>> index c9feac859e57..5c44e500e8f6 100644
>> --- a/drivers/platform/x86/intel/pmt/telemetry.c
>> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
>> @@ -78,6 +78,13 @@ static int pmt_telem_header_decode(struct
>intel_pmt_entry *entry,
>> header->access_type = TELEM_ACCESS(readl(disc_table));
>> header->guid = readl(disc_table + TELEM_GUID_OFFSET);
>> header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
>> + if (entry->base_adjust) {
>> + u32 new_base = header->base_offset + entry->base_adjust;
>
>The code setting ->base_adjust is responsible for avoiding stupid settings
>that would lead to underflows and overflows?
I would think so. Since this driver is not aware of why the adjust is set,
it is not clear how it would validate it.
Is this a request for documentation of the usage, or did you have a concern
that we need to verify the value?
Thanks
Mike
>> +
>> + dev_dbg(dev, "Adjusting baseoffset from 0x%x to 0x%x\n",
>
>base offset
>
>--
> i.
>
>> + header->base_offset, new_base);
>> + header->base_offset = new_base;
>> + }
>>
>> /* Size is measured in DWORDS, but accessor returns bytes */
>> header->size = TELEM_SIZE(readl(disc_table));
>> @@ -302,6 +309,8 @@ 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];
>>
>> + entry->base_adjust = intel_vsec_dev->base_adjust;
>> +
>> mutex_lock(&ep_lock);
>> ret = intel_pmt_dev_create(entry, &pmt_telem_ns,
>intel_vsec_dev, i);
>> mutex_unlock(&ep_lock);
>> diff --git a/drivers/platform/x86/intel/vsec.c
>b/drivers/platform/x86/intel/vsec.c
>> index 7b5cc9993974..be079d62a7bc 100644
>> --- a/drivers/platform/x86/intel/vsec.c
>> +++ b/drivers/platform/x86/intel/vsec.c
>> @@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev *pdev,
>struct intel_vsec_header *he
>> intel_vsec_dev->num_resources = header->num_entries;
>> intel_vsec_dev->quirks = info->quirks;
>> intel_vsec_dev->base_addr = info->base_addr;
>> + intel_vsec_dev->base_adjust = info->base_adjust;
>> intel_vsec_dev->priv_data = info->priv_data;
>>
>> if (header->id == VSEC_ID_SDSI)
>> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
>> index 4569a55e8645..1fd0fcc5615d 100644
>> --- a/include/linux/intel_vsec.h
>> +++ b/include/linux/intel_vsec.h
>> @@ -95,6 +95,7 @@ struct intel_vsec_platform_info {
>> unsigned long caps;
>> unsigned long quirks;
>> u64 base_addr;
>> + s32 base_adjust;
>> };
>>
>> /**
>> @@ -120,6 +121,7 @@ struct intel_vsec_device {
>> size_t priv_data_size;
>> unsigned long quirks;
>> u64 base_addr;
>> + s32 base_adjust;
>> };
>>
>> int intel_vsec_add_aux(struct pci_dev *pdev, struct device *parent,
>>
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-11 20:56 ` Ruhl, Michael J
@ 2024-07-12 8:31 ` Ilpo Järvinen
0 siblings, 0 replies; 15+ messages in thread
From: Ilpo Järvinen @ 2024-07-12 8:31 UTC (permalink / raw)
To: Ruhl, Michael J
Cc: intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, david.e.box@linux.intel.com,
Brost, Matthew
[-- Attachment #1: Type: text/plain, Size: 2914 bytes --]
On Thu, 11 Jul 2024, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >Sent: Thursday, July 11, 2024 7:34 AM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >Cc: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org;
> >david.e.box@linux.intel.com; Brost, Matthew <matthew.brost@intel.com>
> >Subject: Re: [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT
> >base adjust
> >
> >On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
> >
> >> DVSEC offsets are based on the endpoint BAR. If an endpoint is
> >> not avialable allow the offset information to be adjusted by the
> >
> >available
> >
> >> parent driver.
> >>
> >> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> >> ---
> >> drivers/platform/x86/intel/pmt/class.h | 1 +
> >> drivers/platform/x86/intel/pmt/telemetry.c | 9 +++++++++
> >> drivers/platform/x86/intel/vsec.c | 1 +
> >> include/linux/intel_vsec.h | 2 ++
> >> 4 files changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/intel/pmt/class.h
> >b/drivers/platform/x86/intel/pmt/class.h
> >> index a267ac964423..984cd40ee814 100644
> >> --- a/drivers/platform/x86/intel/pmt/class.h
> >> +++ b/drivers/platform/x86/intel/pmt/class.h
> >> @@ -46,6 +46,7 @@ struct intel_pmt_entry {
> >> void __iomem *base;
> >> struct pmt_callbacks *cb;
> >> unsigned long base_addr;
> >> + s32 base_adjust;
> >> size_t size;
> >> u32 guid;
> >> int devid;
> >> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c
> >b/drivers/platform/x86/intel/pmt/telemetry.c
> >> index c9feac859e57..5c44e500e8f6 100644
> >> --- a/drivers/platform/x86/intel/pmt/telemetry.c
> >> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> >> @@ -78,6 +78,13 @@ static int pmt_telem_header_decode(struct
> >intel_pmt_entry *entry,
> >> header->access_type = TELEM_ACCESS(readl(disc_table));
> >> header->guid = readl(disc_table + TELEM_GUID_OFFSET);
> >> header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
> >> + if (entry->base_adjust) {
> >> + u32 new_base = header->base_offset + entry->base_adjust;
> >
> >The code setting ->base_adjust is responsible for avoiding stupid settings
> >that would lead to underflows and overflows?
>
> I would think so. Since this driver is not aware of why the adjust is set,
> it is not clear how it would validate it.
>
> Is this a request for documentation of the usage, or did you have a concern
> that we need to verify the value?
It's just that usually in-kernel interfaces too feature sanity checks but
perhaps you're right and the telemetry side just doesn't know enough to
even make a sanity check on the value. Let's just leave this as is for
now, it can be revisited later if there starts to be many usecases for
this.
--
i.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 6/6] drm/xe/vsec: Add support for DG2
2024-07-10 19:22 [PATCH v6 0/6] Support PMT features in Xe Michael J. Ruhl
` (4 preceding siblings ...)
2024-07-10 19:22 ` [PATCH v6 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
@ 2024-07-10 19:22 ` Michael J. Ruhl
2024-07-11 11:44 ` Ilpo Järvinen
5 siblings, 1 reply; 15+ messages in thread
From: Michael J. Ruhl @ 2024-07-10 19:22 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost
Cc: michael.j.ruhl
DG2 needs to adjust the discovery offset WRT the GT BAR not the
P2SB bar so add the base_adjust value to allow for the difference
to be used.
Update xe_vsec.c to include DG2 header information.
Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
drivers/gpu/drm/xe/xe_vsec.c | 81 ++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
index 98999d467db1..531ddd32a1a6 100644
--- a/drivers/gpu/drm/xe/xe_vsec.c
+++ b/drivers/gpu/drm/xe/xe_vsec.c
@@ -19,6 +19,16 @@
#define SOC_BASE 0x280000
+/* from drivers/platform/x86/intel/pmt/telemetry.c */
+#define TELEM_BASE_OFFSET 0x8
+
+#define DG2_PMT_BASE 0xE8000
+#define DG2_DISCOVERY_START 0x6000
+#define DG2_TELEM_START 0x4000
+
+#define DG2_DISCOVERY_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_DISCOVERY_START)
+#define DG2_TELEM_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_TELEM_START)
+
#define BMG_PMT_BASE 0xDB000
#define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
@@ -32,6 +42,20 @@
#define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
#define SG_REMAP_BITS GENMASK(31, 24)
+static struct intel_vsec_header dg2_telemetry = {
+ .length = 0x10,
+ .id = VSEC_ID_TELEMETRY,
+ .num_entries = 1,
+ .entry_size = 3,
+ .tbir = GFX_BAR,
+ .offset = DG2_DISCOVERY_OFFSET,
+};
+
+static struct intel_vsec_header *dg2_capabilities[] = {
+ &dg2_telemetry,
+ NULL
+};
+
static struct intel_vsec_header bmg_telemetry = {
.length = 0x10,
.id = VSEC_ID_TELEMETRY,
@@ -48,10 +72,16 @@ static struct intel_vsec_header *bmg_capabilities[] = {
enum xe_vsec {
XE_VSEC_UNKNOWN = 0,
+ XE_VSEC_DG2,
XE_VSEC_BMG,
};
static struct intel_vsec_platform_info xe_vsec_info[] = {
+ [XE_VSEC_DG2] = {
+ .caps = VSEC_CAP_TELEMETRY,
+ .headers = dg2_capabilities,
+ .quirks = VSEC_QUIRK_EARLY_HW,
+ },
[XE_VSEC_BMG] = {
.caps = VSEC_CAP_TELEMETRY,
.headers = bmg_capabilities,
@@ -174,6 +204,7 @@ struct pmt_callbacks xe_pmt_cb = {
};
static const int vsec_platforms[] = {
+ [XE_DG2] = XE_VSEC_DG2,
[XE_BATTLEMAGE] = XE_VSEC_BMG,
};
@@ -185,6 +216,49 @@ static enum xe_vsec get_platform_info(struct xe_device *xe)
return vsec_platforms[xe->info.platform];
}
+/*
+ * Access the DG2 PMT MMIO discovery table
+ *
+ * The intel_vsec driver does not typically access the discovery table.
+ * Instead, it creates a memory resource for the table and passes it
+ * to the PMT telemetry driver. Each discovery table contains 3 items,
+ * - GUID
+ * - Telemetry size
+ * - Telemetry offset (offset from P2SB BAR, not GT)
+ *
+ * For DG2 we know what the telemetry offset is, but we still need to
+ * use the discovery table to pass the GUID and the size. So figure
+ * out the difference between the P2SB offset and the GT offset and
+ * save this so that the telemetry driver can use it to adjust the
+ * value.
+ */
+static int dg2_adjust_offset(struct pci_dev *pdev, struct device *dev,
+ struct intel_vsec_platform_info *info)
+{
+ void __iomem *base;
+ u32 telem_offset;
+ u64 addr;
+
+ /* compile check to verify that quirk has P2SB quirk added */
+
+ addr = pci_resource_start(pdev, GFX_BAR) + info->headers[0]->offset;
+ base = ioremap_wc(addr, 16);
+ if (!base)
+ return -ENOMEM;
+
+ telem_offset = readl(base + TELEM_BASE_OFFSET);
+
+ /* Use the base_addr + P2SB quirk to pass this info */
+ if (telem_offset < DG2_TELEM_OFFSET)
+ info->base_adjust = -(DG2_TELEM_OFFSET - telem_offset);
+ else
+ info->base_adjust = -(telem_offset - DG2_TELEM_OFFSET);
+
+ iounmap(base);
+
+ return 0;
+}
+
/**
* xe_vsec_init - Initialize resources and add intel_vsec auxiliary
* interface
@@ -196,6 +270,7 @@ void xe_vsec_init(struct xe_device *xe)
struct device *dev = xe->drm.dev;
struct pci_dev *pdev = to_pci_dev(dev);
enum xe_vsec platform;
+ u32 ret;
platform = get_platform_info(xe);
if (platform == XE_VSEC_UNKNOWN)
@@ -206,6 +281,12 @@ void xe_vsec_init(struct xe_device *xe)
return;
switch (platform) {
+ case XE_VSEC_DG2:
+ ret = dg2_adjust_offset(pdev, dev, info);
+ if (ret)
+ return;
+ break;
+
case XE_VSEC_BMG:
info->priv_data = &xe_pmt_cb;
break;
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v6 6/6] drm/xe/vsec: Add support for DG2
2024-07-10 19:22 ` [PATCH v6 6/6] drm/xe/vsec: Add support for DG2 Michael J. Ruhl
@ 2024-07-11 11:44 ` Ilpo Järvinen
2024-07-11 17:44 ` Ruhl, Michael J
0 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2024-07-11 11:44 UTC (permalink / raw)
To: Michael J. Ruhl; +Cc: intel-xe, platform-driver-x86, david.e.box, matthew.brost
On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
> DG2 needs to adjust the discovery offset WRT the GT BAR not the
WRT = w.r.t. ?
> P2SB bar so add the base_adjust value to allow for the difference
> to be used.
Could you please try to write out the problem in clearer terms. Currently
the reasonale given in patch 5 doesn't even match the one given here or
it is written in so unclear terms I cannot make the connection.
> Update xe_vsec.c to include DG2 header information.
>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
> drivers/gpu/drm/xe/xe_vsec.c | 81 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 81 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 98999d467db1..531ddd32a1a6 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -19,6 +19,16 @@
>
> #define SOC_BASE 0x280000
>
> +/* from drivers/platform/x86/intel/pmt/telemetry.c */
> +#define TELEM_BASE_OFFSET 0x8
> +
> +#define DG2_PMT_BASE 0xE8000
> +#define DG2_DISCOVERY_START 0x6000
> +#define DG2_TELEM_START 0x4000
> +
> +#define DG2_DISCOVERY_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_DISCOVERY_START)
> +#define DG2_TELEM_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_TELEM_START)
> +
> #define BMG_PMT_BASE 0xDB000
> #define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
>
> @@ -32,6 +42,20 @@
> #define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
> #define SG_REMAP_BITS GENMASK(31, 24)
>
> +static struct intel_vsec_header dg2_telemetry = {
> + .length = 0x10,
> + .id = VSEC_ID_TELEMETRY,
> + .num_entries = 1,
> + .entry_size = 3,
> + .tbir = GFX_BAR,
> + .offset = DG2_DISCOVERY_OFFSET,
> +};
> +
> +static struct intel_vsec_header *dg2_capabilities[] = {
> + &dg2_telemetry,
> + NULL
> +};
> +
> static struct intel_vsec_header bmg_telemetry = {
> .length = 0x10,
> .id = VSEC_ID_TELEMETRY,
> @@ -48,10 +72,16 @@ static struct intel_vsec_header *bmg_capabilities[] = {
>
> enum xe_vsec {
> XE_VSEC_UNKNOWN = 0,
> + XE_VSEC_DG2,
> XE_VSEC_BMG,
> };
>
> static struct intel_vsec_platform_info xe_vsec_info[] = {
> + [XE_VSEC_DG2] = {
> + .caps = VSEC_CAP_TELEMETRY,
> + .headers = dg2_capabilities,
> + .quirks = VSEC_QUIRK_EARLY_HW,
> + },
> [XE_VSEC_BMG] = {
> .caps = VSEC_CAP_TELEMETRY,
> .headers = bmg_capabilities,
> @@ -174,6 +204,7 @@ struct pmt_callbacks xe_pmt_cb = {
> };
>
> static const int vsec_platforms[] = {
> + [XE_DG2] = XE_VSEC_DG2,
> [XE_BATTLEMAGE] = XE_VSEC_BMG,
> };
>
> @@ -185,6 +216,49 @@ static enum xe_vsec get_platform_info(struct xe_device *xe)
> return vsec_platforms[xe->info.platform];
> }
>
> +/*
> + * Access the DG2 PMT MMIO discovery table
> + *
> + * The intel_vsec driver does not typically access the discovery table.
> + * Instead, it creates a memory resource for the table and passes it
> + * to the PMT telemetry driver. Each discovery table contains 3 items,
> + * - GUID
> + * - Telemetry size
> + * - Telemetry offset (offset from P2SB BAR, not GT)
> + *
> + * For DG2 we know what the telemetry offset is, but we still need to
> + * use the discovery table to pass the GUID and the size. So figure
> + * out the difference between the P2SB offset and the GT offset and
> + * save this so that the telemetry driver can use it to adjust the
> + * value.
> + */
> +static int dg2_adjust_offset(struct pci_dev *pdev, struct device *dev,
> + struct intel_vsec_platform_info *info)
> +{
> + void __iomem *base;
> + u32 telem_offset;
> + u64 addr;
> +
> + /* compile check to verify that quirk has P2SB quirk added */
I don't know what "quirk" the first instance refers to (I think I
probably know what the P2SB quirk is).
What is the purpose/meaning of this comment? Is it some leftover for
code that no longer exists as it talks about "compile check"??
--
i.
> +
> + addr = pci_resource_start(pdev, GFX_BAR) + info->headers[0]->offset;
> + base = ioremap_wc(addr, 16);
> + if (!base)
> + return -ENOMEM;
> +
> + telem_offset = readl(base + TELEM_BASE_OFFSET);
> +
> + /* Use the base_addr + P2SB quirk to pass this info */
> + if (telem_offset < DG2_TELEM_OFFSET)
> + info->base_adjust = -(DG2_TELEM_OFFSET - telem_offset);
> + else
> + info->base_adjust = -(telem_offset - DG2_TELEM_OFFSET);
> +
> + iounmap(base);
> +
> + return 0;
> +}
> +
> /**
> * xe_vsec_init - Initialize resources and add intel_vsec auxiliary
> * interface
> @@ -196,6 +270,7 @@ void xe_vsec_init(struct xe_device *xe)
> struct device *dev = xe->drm.dev;
> struct pci_dev *pdev = to_pci_dev(dev);
> enum xe_vsec platform;
> + u32 ret;
>
> platform = get_platform_info(xe);
> if (platform == XE_VSEC_UNKNOWN)
> @@ -206,6 +281,12 @@ void xe_vsec_init(struct xe_device *xe)
> return;
>
> switch (platform) {
> + case XE_VSEC_DG2:
> + ret = dg2_adjust_offset(pdev, dev, info);
> + if (ret)
> + return;
> + break;
> +
> case XE_VSEC_BMG:
> info->priv_data = &xe_pmt_cb;
> break;
>
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH v6 6/6] drm/xe/vsec: Add support for DG2
2024-07-11 11:44 ` Ilpo Järvinen
@ 2024-07-11 17:44 ` Ruhl, Michael J
0 siblings, 0 replies; 15+ messages in thread
From: Ruhl, Michael J @ 2024-07-11 17:44 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, david.e.box@linux.intel.com,
Brost, Matthew
>-----Original Message-----
>From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>Sent: Thursday, July 11, 2024 7:45 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org;
>david.e.box@linux.intel.com; Brost, Matthew <matthew.brost@intel.com>
>Subject: Re: [PATCH v6 6/6] drm/xe/vsec: Add support for DG2
>
>On Wed, 10 Jul 2024, Michael J. Ruhl wrote:
>
>> DG2 needs to adjust the discovery offset WRT the GT BAR not the
>
>WRT = w.r.t. ?
>
>> P2SB bar so add the base_adjust value to allow for the difference
>> to be used.
>
>Could you please try to write out the problem in clearer terms. Currently
>the reasonale given in patch 5 doesn't even match the one given here or
>it is written in so unclear terms I cannot make the connection.
Yes. I will right this up better.
>> Update xe_vsec.c to include DG2 header information.
>>
>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_vsec.c | 81
>++++++++++++++++++++++++++++++++++++
>> 1 file changed, 81 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
>> index 98999d467db1..531ddd32a1a6 100644
>> --- a/drivers/gpu/drm/xe/xe_vsec.c
>> +++ b/drivers/gpu/drm/xe/xe_vsec.c
>> @@ -19,6 +19,16 @@
>>
>> #define SOC_BASE 0x280000
>>
>> +/* from drivers/platform/x86/intel/pmt/telemetry.c */
>> +#define TELEM_BASE_OFFSET 0x8
>> +
>> +#define DG2_PMT_BASE 0xE8000
>> +#define DG2_DISCOVERY_START 0x6000
>> +#define DG2_TELEM_START 0x4000
>> +
>> +#define DG2_DISCOVERY_OFFSET (SOC_BASE + DG2_PMT_BASE +
>DG2_DISCOVERY_START)
>> +#define DG2_TELEM_OFFSET (SOC_BASE + DG2_PMT_BASE +
>DG2_TELEM_START)
>> +
>> #define BMG_PMT_BASE 0xDB000
>> #define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
>>
>> @@ -32,6 +42,20 @@
>> #define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
>> #define SG_REMAP_BITS GENMASK(31, 24)
>>
>> +static struct intel_vsec_header dg2_telemetry = {
>> + .length = 0x10,
>> + .id = VSEC_ID_TELEMETRY,
>> + .num_entries = 1,
>> + .entry_size = 3,
>> + .tbir = GFX_BAR,
>> + .offset = DG2_DISCOVERY_OFFSET,
>> +};
>> +
>> +static struct intel_vsec_header *dg2_capabilities[] = {
>> + &dg2_telemetry,
>> + NULL
>> +};
>> +
>> static struct intel_vsec_header bmg_telemetry = {
>> .length = 0x10,
>> .id = VSEC_ID_TELEMETRY,
>> @@ -48,10 +72,16 @@ static struct intel_vsec_header *bmg_capabilities[] =
>{
>>
>> enum xe_vsec {
>> XE_VSEC_UNKNOWN = 0,
>> + XE_VSEC_DG2,
>> XE_VSEC_BMG,
>> };
>>
>> static struct intel_vsec_platform_info xe_vsec_info[] = {
>> + [XE_VSEC_DG2] = {
>> + .caps = VSEC_CAP_TELEMETRY,
>> + .headers = dg2_capabilities,
>> + .quirks = VSEC_QUIRK_EARLY_HW,
>> + },
>> [XE_VSEC_BMG] = {
>> .caps = VSEC_CAP_TELEMETRY,
>> .headers = bmg_capabilities,
>> @@ -174,6 +204,7 @@ struct pmt_callbacks xe_pmt_cb = {
>> };
>>
>> static const int vsec_platforms[] = {
>> + [XE_DG2] = XE_VSEC_DG2,
>> [XE_BATTLEMAGE] = XE_VSEC_BMG,
>> };
>>
>> @@ -185,6 +216,49 @@ static enum xe_vsec get_platform_info(struct
>xe_device *xe)
>> return vsec_platforms[xe->info.platform];
>> }
>>
>> +/*
>> + * Access the DG2 PMT MMIO discovery table
>> + *
>> + * The intel_vsec driver does not typically access the discovery table.
>> + * Instead, it creates a memory resource for the table and passes it
>> + * to the PMT telemetry driver. Each discovery table contains 3 items,
>> + * - GUID
>> + * - Telemetry size
>> + * - Telemetry offset (offset from P2SB BAR, not GT)
>> + *
>> + * For DG2 we know what the telemetry offset is, but we still need to
>> + * use the discovery table to pass the GUID and the size. So figure
>> + * out the difference between the P2SB offset and the GT offset and
>> + * save this so that the telemetry driver can use it to adjust the
>> + * value.
>> + */
>> +static int dg2_adjust_offset(struct pci_dev *pdev, struct device *dev,
>> + struct intel_vsec_platform_info *info)
>> +{
>> + void __iomem *base;
>> + u32 telem_offset;
>> + u64 addr;
>> +
>> + /* compile check to verify that quirk has P2SB quirk added */
>
>I don't know what "quirk" the first instance refers to (I think I
>probably know what the P2SB quirk is).
>
>What is the purpose/meaning of this comment? Is it some leftover for
>code that no longer exists as it talks about "compile check"??
Artifact from my first attempt.
I will remove this and update the comment below (another quirk reference).
Thanks!
M
>--
> i.
>
>> +
>> + addr = pci_resource_start(pdev, GFX_BAR) + info->headers[0]->offset;
>> + base = ioremap_wc(addr, 16);
>> + if (!base)
>> + return -ENOMEM;
>> +
>> + telem_offset = readl(base + TELEM_BASE_OFFSET);
>> +
>> + /* Use the base_addr + P2SB quirk to pass this info */
>> + if (telem_offset < DG2_TELEM_OFFSET)
>> + info->base_adjust = -(DG2_TELEM_OFFSET - telem_offset);
>> + else
>> + info->base_adjust = -(telem_offset - DG2_TELEM_OFFSET);
>> +
>> + iounmap(base);
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * xe_vsec_init - Initialize resources and add intel_vsec auxiliary
>> * interface
>> @@ -196,6 +270,7 @@ void xe_vsec_init(struct xe_device *xe)
>> struct device *dev = xe->drm.dev;
>> struct pci_dev *pdev = to_pci_dev(dev);
>> enum xe_vsec platform;
>> + u32 ret;
>>
>> platform = get_platform_info(xe);
>> if (platform == XE_VSEC_UNKNOWN)
>> @@ -206,6 +281,12 @@ void xe_vsec_init(struct xe_device *xe)
>> return;
>>
>> switch (platform) {
>> + case XE_VSEC_DG2:
>> + ret = dg2_adjust_offset(pdev, dev, info);
>> + if (ret)
>> + return;
>> + break;
>> +
>> case XE_VSEC_BMG:
>> info->priv_data = &xe_pmt_cb;
>> break;
>>
^ permalink raw reply [flat|nested] 15+ messages in thread