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

Arnd Bergmann
2026-02-25T10:11:16Z

On Wed, Feb 25, 2026, at 07:16, dan.j.williams@intel.com 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