[PATCH kernel 0/4] pci/doe/ide: Capabilities, protocols

[PATCH kernel 0/4] pci/doe/ide: Capabilities, protocols

[Alexey Kardashevskiy]

Here are 4 small independent patches defining new PCIe caps and bits and
protocols, I am using them with my test device so they are real :)


Please comment. Thanks.



Alexey Kardashevskiy (4):
  pci/doe: Define protocol types and make those public
  pci/doe: Support discovery version
  pci: Define TEE-IO bit in PCIe device capabilities
  pci: Define Integrity and Data Encryption (IDE) extended capability

 include/linux/pci-doe.h       |  4 ++++
 include/uapi/linux/pci_regs.h |  5 ++++-
 drivers/pci/doe.c             | 13 ++++++++-----
 3 files changed, 16 insertions(+), 6 deletions(-)

-- 
2.41.0

[PATCH kernel 1/4] pci/doe: Define protocol types and make those public

[Alexey Kardashevskiy]

Already public pci_doe() takes a protocol type argument.
PCIe 6.0 defines three, define them in a header for use with pci_doe().

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 include/linux/pci-doe.h | 4 ++++
 drivers/pci/doe.c       | 2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/pci-doe.h b/include/linux/pci-doe.h
index 1f14aed4354b..5c33f7dc978b 100644
--- a/include/linux/pci-doe.h
+++ b/include/linux/pci-doe.h
@@ -13,6 +13,10 @@
 #ifndef LINUX_PCI_DOE_H
 #define LINUX_PCI_DOE_H
 
+#define PCI_DOE_PROTOCOL_DISCOVERY		0
+#define PCI_DOE_PROTOCOL_CMA_SPDM		1
+#define PCI_DOE_PROTOCOL_SECURED_CMA_SPDM	2
+
 struct pci_doe_mb;
 
 struct pci_doe_mb *pci_find_doe_mailbox(struct pci_dev *pdev, u16 vendor,
diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
index e3aab5edaf70..61f0531d2b1d 100644
--- a/drivers/pci/doe.c
+++ b/drivers/pci/doe.c
@@ -22,8 +22,6 @@
 
 #include "pci.h"
 
-#define PCI_DOE_PROTOCOL_DISCOVERY 0
-
 /* Timeout of 1 second from 6.30.2 Operation, PCI Spec r6.0 */
 #define PCI_DOE_TIMEOUT HZ
 #define PCI_DOE_POLL_INTERVAL	(PCI_DOE_TIMEOUT / 128)
-- 
2.41.0

[PATCH kernel 2/4] pci/doe: Support discovery version

[Alexey Kardashevskiy]

PCIe spec v6.1 defines a "DOE Discovery Version" field in the DOE Discovery
Request Data Object Contents (3rd DW) as:

15:8 DOE Discovery Version – must be 02h if the Capability Version in
the Data Object Exchange Extended Capability is 02h or greater.

Add support for the version on devices with the DOE v2 capability.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 include/uapi/linux/pci_regs.h |  1 +
 drivers/pci/doe.c             | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index a39193213ff2..b9c681f14181 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1144,6 +1144,7 @@
 #define PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH		0x0003ffff
 
 #define PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX		0x000000ff
+#define PCI_DOE_DATA_OBJECT_DISC_REQ_3_DISCOVER_VER	0x0000ff00
 #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID		0x0000ffff
 #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL		0x00ff0000
 #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX	0xff000000
diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
index 61f0531d2b1d..f57def002175 100644
--- a/drivers/pci/doe.c
+++ b/drivers/pci/doe.c
@@ -381,11 +381,13 @@ static void pci_doe_task_complete(struct pci_doe_task *task)
 	complete(task->private);
 }
 
-static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 *index, u16 *vid,
+static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 capver, u8 *index, u16 *vid,
 			     u8 *protocol)
 {
+	u32 disver = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_DISCOVER_VER,
+				(capver >= 2) ? 2 : 0);
 	u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX,
-				    *index);
+				    *index) | disver;
 	__le32 request_pl_le = cpu_to_le32(request_pl);
 	__le32 response_pl_le;
 	u32 response_pl;
@@ -419,13 +421,16 @@ static int pci_doe_cache_protocols(struct pci_doe_mb *doe_mb)
 {
 	u8 index = 0;
 	u8 xa_idx = 0;
+	u32 hdr = 0;
+
+	pci_read_config_dword(doe_mb->pdev, doe_mb->cap_offset, &hdr);
 
 	do {
 		int rc;
 		u16 vid;
 		u8 prot;
 
-		rc = pci_doe_discovery(doe_mb, &index, &vid, &prot);
+		rc = pci_doe_discovery(doe_mb, PCI_EXT_CAP_VER(hdr), &index, &vid, &prot);
 		if (rc)
 			return rc;
 
-- 
2.41.0

[PATCH kernel 3/4] pci: Define TEE-IO bit in PCIe device capabilities

[Alexey Kardashevskiy]

A new bit #30 from the PCI Express Device Capabilities Register is defined
in PCIe 6.1 as "TEE Device Interface Security Protocol (TDISP)".

Define the macro.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 include/uapi/linux/pci_regs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index b9c681f14181..e60b4df1f7d9 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -498,6 +498,7 @@
 #define  PCI_EXP_DEVCAP_PWR_VAL	0x03fc0000 /* Slot Power Limit Value */
 #define  PCI_EXP_DEVCAP_PWR_SCL	0x0c000000 /* Slot Power Limit Scale */
 #define  PCI_EXP_DEVCAP_FLR     0x10000000 /* Function Level Reset */
+#define  PCI_EXP_DEVCAP_TEE_IO  0x40000000 /* TEE-IO Supported (TDISP) */
 #define PCI_EXP_DEVCTL		0x08	/* Device Control */
 #define  PCI_EXP_DEVCTL_CERE	0x0001	/* Correctable Error Reporting En. */
 #define  PCI_EXP_DEVCTL_NFERE	0x0002	/* Non-Fatal Error Reporting Enable */
-- 
2.41.0

[PATCH kernel 4/4] pci: Define Integrity and Data Encryption (IDE) extended capability

[Alexey Kardashevskiy]

PCIe 6.0 introduces the "Integrity & Data Encryption (IDE)" feature which
adds a new capability with id=0x30.

Add the new id to the list of capabilities.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---

This only adds an id. The rest is here:
https://github.com/aik/pciutils/commit/ide

Not sure how much of that we want in the Linux.
---
 include/uapi/linux/pci_regs.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e60b4df1f7d9..b8d447b2c793 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -743,7 +743,8 @@
 #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
 #define PCI_EXT_CAP_ID_PL_32GT  0x2A    /* Physical Layer 32.0 GT/s */
 #define PCI_EXT_CAP_ID_DOE	0x2E	/* Data Object Exchange */
-#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DOE
+#define PCI_EXT_CAP_ID_IDE	0x30	/* Integrity and Data Encryption (IDE) */
+#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_IDE
 
 #define PCI_EXT_CAP_DSN_SIZEOF	12
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
-- 
2.41.0

Re: [PATCH kernel 1/4] pci/doe: Define protocol types and make those public

[Lukas Wunner]

On Thu, Feb 01, 2024 at 05:02:25PM +1100, Alexey Kardashevskiy wrote:
> Already public pci_doe() takes a protocol type argument.
> PCIe 6.0 defines three, define them in a header for use with pci_doe().
[...]
> --- a/include/linux/pci-doe.h
> +++ b/include/linux/pci-doe.h
> @@ -13,6 +13,10 @@
>  #ifndef LINUX_PCI_DOE_H
>  #define LINUX_PCI_DOE_H
>  
> +#define PCI_DOE_PROTOCOL_DISCOVERY		0
> +#define PCI_DOE_PROTOCOL_CMA_SPDM		1
> +#define PCI_DOE_PROTOCOL_SECURED_CMA_SPDM	2

These are deliberately defined in the .c files which actually need them,
i.e. DISCOVERY is defined in drivers/pci/doe.c and CMA_SPDM is defined in
drivers/pci/cma.c:

https://lore.kernel.org/all/7721bfa3b4f8a99a111f7808ad8890c3c13df56d.1695921657.git.lukas@wunner.de/

I don't see why they would have to be public if they're each only needed
in a single .c file.

Thanks,

Lukas

Re: [PATCH kernel 1/4] pci/doe: Define protocol types and make those public

[Alexey Kardashevskiy]

On 1/2/24 17:50, Lukas Wunner wrote:
> On Thu, Feb 01, 2024 at 05:02:25PM +1100, Alexey Kardashevskiy wrote:
>> Already public pci_doe() takes a protocol type argument.
>> PCIe 6.0 defines three, define them in a header for use with pci_doe().
> [...]
>> --- a/include/linux/pci-doe.h
>> +++ b/include/linux/pci-doe.h
>> @@ -13,6 +13,10 @@
>>   #ifndef LINUX_PCI_DOE_H
>>   #define LINUX_PCI_DOE_H
>>   
>> +#define PCI_DOE_PROTOCOL_DISCOVERY		0
>> +#define PCI_DOE_PROTOCOL_CMA_SPDM		1
>> +#define PCI_DOE_PROTOCOL_SECURED_CMA_SPDM	2
> 
> These are deliberately defined in the .c files which actually need them,
> i.e. DISCOVERY is defined in drivers/pci/doe.c and CMA_SPDM is defined in
> drivers/pci/cma.c:
> 
> https://lore.kernel.org/all/7721bfa3b4f8a99a111f7808ad8890c3c13df56d.1695921657.git.lukas@wunner.de/
> 
> I don't see why they would have to be public if they're each only needed
> in a single .c file.

SEV TIO needs both CMA and SECURED_CMA for DOE device<->firmware 
bouncing and it is going to use pci_doe() for this. I should have put 
this into the commit log, sorry about that.

Or the plan is to add pci_doe_secure_transport() to cma.c and force 
everyone use that?

The PCI SIG DOE protocol numbers (discovery, CMA, secure CMA) are all 
defined in one place in the PCIe spec and defining them in different 
places (doe.c, cma.c) is weird imho.


> 
> Thanks,
> 
> Lukas

-- 
Alexey

Re: [PATCH kernel 0/4] pci/doe/ide: Capabilities, protocols

[Bjorn Helgaas]

On Thu, Feb 01, 2024 at 05:02:24PM +1100, Alexey Kardashevskiy wrote:
> Here are 4 small independent patches defining new PCIe caps and bits and
> protocols, I am using them with my test device so they are real :)
> 
> 
> Please comment. Thanks.
> 
> 
> 
> Alexey Kardashevskiy (4):
>   pci/doe: Define protocol types and make those public
>   pci/doe: Support discovery version
>   pci: Define TEE-IO bit in PCIe device capabilities
>   pci: Define Integrity and Data Encryption (IDE) extended capability

For changes in drivers/pci and related files, follow the subject line
convention (learn from "git log --oneline drivers/pci/").

Re: [PATCH kernel 1/4] pci/doe: Define protocol types and make those public

[Bjorn Helgaas]

On Thu, Feb 01, 2024 at 05:02:25PM +1100, Alexey Kardashevskiy wrote:
> Already public pci_doe() takes a protocol type argument.
> PCIe 6.0 defines three, define them in a header for use with pci_doe().
> 
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
>  include/linux/pci-doe.h | 4 ++++
>  drivers/pci/doe.c       | 2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/pci-doe.h b/include/linux/pci-doe.h
> index 1f14aed4354b..5c33f7dc978b 100644
> --- a/include/linux/pci-doe.h
> +++ b/include/linux/pci-doe.h
> @@ -13,6 +13,10 @@
>  #ifndef LINUX_PCI_DOE_H
>  #define LINUX_PCI_DOE_H
>  
> +#define PCI_DOE_PROTOCOL_DISCOVERY		0
> +#define PCI_DOE_PROTOCOL_CMA_SPDM		1
> +#define PCI_DOE_PROTOCOL_SECURED_CMA_SPDM	2

So far these are only needed inside drivers/pci/.  I don't want to
expose them to the rest of the kernel via include/linux/pci-doe.h
until they're needed elsewhere.

>  struct pci_doe_mb;
>  
>  struct pci_doe_mb *pci_find_doe_mailbox(struct pci_dev *pdev, u16 vendor,
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index e3aab5edaf70..61f0531d2b1d 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -22,8 +22,6 @@
>  
>  #include "pci.h"
>  
> -#define PCI_DOE_PROTOCOL_DISCOVERY 0
> -
>  /* Timeout of 1 second from 6.30.2 Operation, PCI Spec r6.0 */
>  #define PCI_DOE_TIMEOUT HZ
>  #define PCI_DOE_POLL_INTERVAL	(PCI_DOE_TIMEOUT / 128)
> -- 
> 2.41.0
> 

Re: [PATCH kernel 2/4] pci/doe: Support discovery version

[Bjorn Helgaas]

On Thu, Feb 01, 2024 at 05:02:26PM +1100, Alexey Kardashevskiy wrote:
> PCIe spec v6.1 defines a "DOE Discovery Version" field in the DOE Discovery
> Request Data Object Contents (3rd DW) as:

Please include spec section number, e.g., "PCIe r6.1, sec xx.xx".

(Also note PCI-SIG uses "revision" as the major, "version" as the
minor, so this would be "PCIe r6.1", not "PCIe v6.1".)

> 15:8 DOE Discovery Version – must be 02h if the Capability Version in
> the Data Object Exchange Extended Capability is 02h or greater.
> 
> Add support for the version on devices with the DOE v2 capability.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
>  include/uapi/linux/pci_regs.h |  1 +
>  drivers/pci/doe.c             | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index a39193213ff2..b9c681f14181 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -1144,6 +1144,7 @@
>  #define PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH		0x0003ffff
>  
>  #define PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX		0x000000ff
> +#define PCI_DOE_DATA_OBJECT_DISC_REQ_3_DISCOVER_VER	0x0000ff00
>  #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID		0x0000ffff
>  #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL		0x00ff0000
>  #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX	0xff000000
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index 61f0531d2b1d..f57def002175 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -381,11 +381,13 @@ static void pci_doe_task_complete(struct pci_doe_task *task)
>  	complete(task->private);
>  }
>  
> -static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 *index, u16 *vid,
> +static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 capver, u8 *index, u16 *vid,
>  			     u8 *protocol)
>  {
> +	u32 disver = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_DISCOVER_VER,
> +				(capver >= 2) ? 2 : 0);
>  	u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX,
> -				    *index);
> +				    *index) | disver;
>  	__le32 request_pl_le = cpu_to_le32(request_pl);
>  	__le32 response_pl_le;
>  	u32 response_pl;
> @@ -419,13 +421,16 @@ static int pci_doe_cache_protocols(struct pci_doe_mb *doe_mb)
>  {
>  	u8 index = 0;
>  	u8 xa_idx = 0;
> +	u32 hdr = 0;
> +
> +	pci_read_config_dword(doe_mb->pdev, doe_mb->cap_offset, &hdr);
>  
>  	do {
>  		int rc;
>  		u16 vid;
>  		u8 prot;
>  
> -		rc = pci_doe_discovery(doe_mb, &index, &vid, &prot);
> +		rc = pci_doe_discovery(doe_mb, PCI_EXT_CAP_VER(hdr), &index, &vid, &prot);
>  		if (rc)
>  			return rc;
>  
> -- 
> 2.41.0
> 

Re: [PATCH kernel 4/4] pci: Define Integrity and Data Encryption (IDE) extended capability

[Bjorn Helgaas]

On Thu, Feb 01, 2024 at 05:02:28PM +1100, Alexey Kardashevskiy wrote:
> PCIe 6.0 introduces the "Integrity & Data Encryption (IDE)" feature which
> adds a new capability with id=0x30.
> 
> Add the new id to the list of capabilities.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
> 
> This only adds an id. The rest is here:
> https://github.com/aik/pciutils/commit/ide

We can add this #define when we have need for it in Linux, so let's
hold it until that need appears.

> Not sure how much of that we want in the Linux.
> ---
>  include/uapi/linux/pci_regs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e60b4df1f7d9..b8d447b2c793 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -743,7 +743,8 @@
>  #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
>  #define PCI_EXT_CAP_ID_PL_32GT  0x2A    /* Physical Layer 32.0 GT/s */
>  #define PCI_EXT_CAP_ID_DOE	0x2E	/* Data Object Exchange */
> -#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DOE
> +#define PCI_EXT_CAP_ID_IDE	0x30	/* Integrity and Data Encryption (IDE) */
> +#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_IDE
>  
>  #define PCI_EXT_CAP_DSN_SIZEOF	12
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
> -- 
> 2.41.0
> 

Re: [PATCH kernel 1/4] pci/doe: Define protocol types and make those public

[Lukas Wunner]

On Thu, Feb 01, 2024 at 09:16:37PM +1100, Alexey Kardashevskiy wrote:
> Or the plan is to add pci_doe_secure_transport() to cma.c and force everyone
> use that?

Right, the plan is to pass an additional callback to spdm_create()
which performs a secure transmission.  And cma.c would define that
to use the separate DOE type.

Thanks,

Lukas

Re: [PATCH kernel 1/4] pci/doe: Define protocol types and make those public

[Alexey Kardashevskiy]

On 9/2/24 19:52, Lukas Wunner wrote:
> On Thu, Feb 01, 2024 at 09:16:37PM +1100, Alexey Kardashevskiy wrote:
>> Or the plan is to add pci_doe_secure_transport() to cma.c and force everyone
>> use that?
> 
> Right, the plan is to pass an additional callback to spdm_create()
> which performs a secure transmission.  And cma.c would define that
> to use the separate DOE type.

But it is no use for the TDISP case which won't be calling spdm_create() 
and will have to define CMA_SPDM and SECURED_CMA_SPDM, again.


-- 
Alexey