Re: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl

[Aneesh Kumar K . V]

"Dan Williams (nvidia)" <djbw@xxxxxxxxxx> writes:

> Alexey Kardashevskiy wrote:
>> 
>> 
>> On 26/5/26 01:48, Aneesh Kumar K.V (Arm) wrote:
>> > Add IOMMU_VDEVICE_TSM_REQUEST for issuing TSM guest request/response
>> > transactions against an iommufd vdevice.
>> > 
>> > The ioctl takes a vdevice_id plus request/response user buffers and length
>> > fields, and forwards the request through tsm_guest_req() to the PCI TSM
>> > backend. This provides the host-side passthrough path used by CoCo guests
>> > for TSM device attestation and acceptance flows after the device has been
>> > bound to TSM.
>> > 
>> > Also add the supporting tsm_guest_req() helper and associated TSM core
>> > interface definitions.
>> > 
>> > Based on changes from: Alexey Kardashevskiy <aik@xxxxxxx>
>> > 
>> > Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@xxxxxxxxxx>
>> > ---
>> >   drivers/iommu/iommufd/iommufd_private.h |  6 ++
>> >   drivers/iommu/iommufd/main.c            |  3 +
>> >   drivers/iommu/iommufd/tsm.c             | 68 +++++++++++++++++++++
>> >   drivers/virt/coco/tsm-core.c            | 39 ++++++++++++
>> >   include/linux/pci-tsm.h                 |  9 +--
>> >   include/linux/tsm.h                     | 25 ++++++++
>> >   include/uapi/linux/iommufd.h            | 80 +++++++++++++++++++++++++
>> >   7 files changed, 226 insertions(+), 4 deletions(-)
> [..]
>> > diff --git a/drivers/iommu/iommufd/tsm.c b/drivers/iommu/iommufd/tsm.c
>> > index 09ee668dbed9..342fbdb6a6b9 100644
>> > --- a/drivers/iommu/iommufd/tsm.c
>> > +++ b/drivers/iommu/iommufd/tsm.c
>> > @@ -60,3 +60,71 @@ int iommufd_vdevice_tsm_op_ioctl(struct iommufd_ucmd *ucmd)
>> >   	iommufd_put_object(ucmd->ictx, &vdev->obj);
>> >   	return rc;
>> >   }
>> > +
>> > +static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
>> > +{
>> > +	if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
>> > +		return false;
>> > +
>> > +	switch (scope) {
>> > +	case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
>> > +	case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
>> > +	case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
>> > +	case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
>> 
>> This scope thing still needs clarification.
>> 
>> I have 3 types of requests to fit here, all go via VM -> KVM -> QEMU -> IOMMUFD -> TSM.
>> 
>> 1) bind/unbind TDI <- moves to CONFIG_LOCKED, this is "OP";
>> 2) start/stop TDI <- moves to RUN, this is "GR"? Right now I route it via "OP";
>> 3) enable/disable MMIO/DMA <- no TDI state change, this is "GR" but which scope is it here?
>
> The scope parameter was meant to enumerate a security model for classes
> of commands that are otherwise opaque to the kernel. However, none of
> the commands we are targeting are opaque (private specification with
> unknown effect). It now turns out there is no role for @scope for
> security.
>
> Now a command family that iommufd can validate seems useful. As it
> stands this implementation aliases command codes across TSMs. Do we
> proceed with creating an actual shared command uapi for the truly shared
> commands:
>
> TSM_REQ_TYPE_DEFAULT: Commands every arch needs
> TSM_REQ_READ_OBJECT
> TSM_REQ_REGEN_OBJECT
> TSM_REQ_OBJECT_INFO
> TSM_REQ_VALIDATE_MMIO
> TSM_REQ_SET_TDI_STATE
>
> TSM_REQ_TYPE_SEV: Commands only SEV needs
> TSM_REQ_SEV_ENABLE_DMA
> TSM_REQ_SEV_DISABLE_DMA
>
> ...or just observe that per CC arch commands are needed to setup the VM
> so per CC arch commands are needed to marshal device assignment support
> requests.
>
> In that case pci_tsm_req_scope becomes tsm_req_type and is just:
>
> TSM_REQ_TYPE_CCA
> TSM_REQ_TYPE_SEV
> TSM_REQ_TYPE_TDX
>
> I am leaning towards the latter at this point.

But we already have struct pci_tsm_ops::guest_req, which is specific to
the underlying CC architecture. From the above, pci_tsm_req_scope also
appears to carry the same information. Is that useful?

-aneesh