Re: [PATCH v2 02/17] psp: base PSP device support

Jakub Kicinski wrote:

On Wed, 25 Jun 2025 19:55:01 -0400 Willem de Bruijn wrote:

quoted

quoted

+#define PSP_SPI_KEY_ID		GENMASK(30, 0)
+#define PSP_SPI_KEY_PHASE	BIT(31)
+
+#define PSPHDR_CRYPT_OFFSET	GENMASK(5, 0)
+
+#define PSPHDR_VERFL_SAMPLE	BIT(7)
+#define PSPHDR_VERFL_DROP	BIT(6)
+#define PSPHDR_VERFL_VERSION	GENMASK(5, 2)
+#define PSPHDR_VERFL_VIRT	BIT(1)
+#define PSPHDR_VERFL_ONE	BIT(0)  

Use bitfields in struct psphdr rather than manual bit twiddling?

Some call it manual bit twiddling, some call it the recommended kernel
coding style? ;)

:)

Preferable over the following?

	struct psphdr {
		u8      nexthdr;
		u8      hdrlen;
		u8      crypt_offset;

		u8      sample:1;
		u8      drop:1;
		u8	version:4;
		u8	vc_present:1;
		u8	reserved:1;

		__be32  spi;
		__be64  iv;
		__be64  vc[]; /* optional */
	};

I suppose that has an endianness issue requiring
variants with __LITTLE_ENDIAN_BITFIELD and
__BIG_ENDIAN_BITFIELD.

Either way, just a thought.

quoted

Or else just consider just calling it flags rather than verfl
(which stands for version and flags?).

(Yes.)

quoted

quoted

+
+/**
+ * struct psp_dev_config - PSP device configuration
+ * @versions: PSP versions enabled on the device
+ */
+struct psp_dev_config {
+	u32 versions;
+};
+
+/**
+ * struct psp_dev - PSP device struct
+ * @main_netdev: original netdevice of this PSP device  

This makes sense with a single physical device plus optional virtual
(vlan, bonding, ..) devices.

It may also be possible for a single physical device (with single
device key) to present multiple PFs and/or VFs. In that case, will
there be multiple struct psp_dev, or will one PF be the "main".

AFAIU we have no ability to represent multi-PCIe function devices 
in the kernel model today. So realistically I think psp_dev per
function and then propagate the rotation events.

IDPF does support multiple "vports" (num_alloc_vports), and with that
struct net_device, from a single BDF.