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

[Alexey Kardashevskiy <aik@amd.com>]

On 27/5/26 16:17, Dan Williams (nvidia) wrote:
> [You don't often get email from djbw@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> 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@amd.com>
>>>
>>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>> ---
>>>    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

These 3 are already in that netlink interface of the TSM (so common for all arches), right?

> TSM_REQ_VALIDATE_MMIO

SEV handles this in the KVM as this is where RMP and NPT are managed + opaque guest request to the TSM, I'd think it is the same for others.

> TSM_REQ_SET_TDI_STATE

This is a common one.

> TSM_REQ_TYPE_SEV: Commands only SEV needs
> TSM_REQ_SEV_ENABLE_DMA
> TSM_REQ_SEV_DISABLE_DMA

No change to host owned part of the IOMMU when TDX or CCA moves the device to secure? Or it is packed into those opaque requests to the TSM?

> ...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.

Dunno, besides the DMA thing, these CCA/SEV/TDX types will only appear in WARN_ON of the arch TSM drivers and will not really be seen. If a wrong TSM driver is loaded (say, TDX on AMD), then something just went terribly wrong. Thanks,


-- 
Alexey