Re: [PATCH kernel 1/9] pci/tsm: Add TDISP report blob and helpers to parse it

[Arnd Bergmann]

On Wed, Feb 25, 2026, at 07:16, dan.j.williams@xxxxxxxxx wrote:
> Alexey Kardashevskiy wrote:
> [..]
>> +struct tdi_report_header {
>> +	__u16 interface_info; /* TSM_TDI_REPORT_xxx */
>> +	__u16 reserved2;
>> +	__u16 msi_x_message_control;
>> +	__u16 lnr_control;
>> +	__u32 tph_control;
>> +	__u32 mmio_range_count;
>> +} __packed;
>> +
>> +/*
>> + * Each MMIO Range of the TDI is reported with the MMIO reporting offset added.
>> + * Base and size in units of 4K pages
>> + */
>> +#define TSM_TDI_REPORT_MMIO_MSIX_TABLE		BIT(0)
>> +#define TSM_TDI_REPORT_MMIO_PBA			BIT(1)
>> +#define TSM_TDI_REPORT_MMIO_IS_NON_TEE		BIT(2)
>> +#define TSM_TDI_REPORT_MMIO_IS_UPDATABLE	BIT(3)
>> +#define TSM_TDI_REPORT_MMIO_RESERVED		GENMASK(15, 4)
>> +#define TSM_TDI_REPORT_MMIO_RANGE_ID		GENMASK(31, 16)
>> +
>> +struct tdi_report_mmio_range {
>> +	__u64 first_page;		/* First 4K page with offset added */
>> +	__u32 num;			/* Number of 4K pages in this range */
>> +	__u32 range_attributes;		/* TSM_TDI_REPORT_MMIO_xxx */
>
> Those should be __le64 and le32, right? But see below for another
> option...

If these come from DMA transfers from a device, yes.

>> +} __packed;

The structure appears to be allocated with kzalloc, so it is always
aligned to __alignof__(u64) or higher, and it's better to drop the
__packed annotation.

>
> /*
>  * PCIe ECN TEE Device Interface Security Protocol (TDISP)
>  *
>  * Device Interface Report data object layout as defined by PCIe r7.0 
> section
>  * 11.3.11
>  */
> #define PCI_TSM_DEVIF_REPORT_INFO 0
> #define PCI_TSM_DEVIF_REPORT_MSIX 4
> #define PCI_TSM_DEVIF_REPORT_LNR 6
> #define PCI_TSM_DEVIF_REPORT_TPH 8
> #define PCI_TSM_DEVIF_REPORT_MMIO_COUNT 12
> #define  PCI_TSM_DEVIF_REPORT_MMIO_PFN 0 /* An interface report 'pfn' 
> is 4K in size */
> #define  PCI_TSM_DEVIF_REPORT_MMIO_NR_PFNS 8
> #define  PCI_TSM_DEVIF_REPORT_MMIO_ATTR 12
> #define  PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_TABLE BIT(0)
> #define  PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_PBA BIT(1)
> #define  PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_NON_TEE BIT(2)
> #define  PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_UPDATABLE BIT(3)
> #define  PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID GENMASK(31, 16)
> #define  PCI_TSM_DEVIF_REPORT_MMIO_SIZE (16)
> #define PCI_TSM_DEVIF_REPORT_BASE_SIZE(nr_mmio) (16 + nr_mmio * 
> PCI_TSM_DEVIF_REPORT_MMIO_SIZE)
>
> Any strong feelings one way or the other? I have a mild preference for
> this offset+bitfields approach.

I assume by bitfield you mean the macros above, not the C structure
syntax with ':', right? The macros seem fine to me, while C bitfields
again would make the code nonportable due to architecture specific
bitfield positioning.

       Arnd