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

[Alexey Kardashevskiy]

On 9/6/26 06:58, Dan Williams (nvidia) wrote:

> Aneesh Kumar K.V wrote:
> [..]
> > > I think we can wait to move it to its own IOMMU operation unless/until
> > > there is a need to set RUN outside of an explicit guest request, right?
> >
> > Something like the below? (the diff against this series)
> >
> > I have not yet integrated this into the full CCA patchset for testing,
> > but I wanted to make sure we are aligned on the UAPI.
> [..]
> > -static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
> > +static bool iommufd_vdevice_tsm_req_arch_valid(u32 tvm_arch)
> >   {
> > -     if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
> > +     switch (tvm_arch) {
> > +     case IOMMU_VDEVICE_TSM_TVM_ARCH_CCA:
> > +     case IOMMU_VDEVICE_TSM_TVM_ARCH_SEV:
> > +     case IOMMU_VDEVICE_TSM_TVM_ARCH_TDX:
>
> Makes sense for any command that needs tunneling. However, see below, what is
> that set, and do we need a IOMMU_VDEVICE_TSM_COMMON when architecture
> differentiation is not required?


I still do not follow why making these arches checks in runtime, should be caught at the build time (ARM vs x86 vs RiscV) or at the TSM modprobe (AMD vs Intel).

The scope becomes just IOMMU_VDEVICE_TSM_REQ_SCOPE_TUNNEL imho.


> > +             return true;
> > +     default:
> >                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:
> > +static bool iommufd_vdevice_tsm_req_op_valid(u32 op, u32 tvm_arch)
> > +{
> > +     switch (op) {
> > +     case TSM_REQ_READ_OBJECT:
> > +     case TSM_REQ_REGEN_OBJECT:
> > +     case TSM_REQ_OBJECT_INFO:
>
> The design goal of the netlink device-evidence interface is to be able
> to respond to all shapes of requests for evidence. So netlink caches
> objects that the hypercall handler can fill responses from.
>
> It eliminates a class of commands that need tunneling.

+1.

> > +     case TSM_REQ_VALIDATE_MMIO:
> > +     case TSM_REQ_SET_TDI_STATE:
>
> Are these potentially candidates for a IOMMU_VDEVICE_TSM_COMMON? The
> handler knows how to do the arch-specific response from the common
> iommufd result, or is there TSM-specific payload beyond @tsm_code for
> these.

These are not common to put in IOMMUFD - it is either TSM (for TDI states) or KVM (for MMIO validate) on AMD and other arches won't share much either, right?

> Make it the case that guest_req only needs non-common arch for
> operations that are implementation unique, or where the response payload
> exceeds what can be conveyed via @tsm_code.
>
> >                return true;
> > +     case TSM_REQ_SEV_ENABLE_DMA:
> > +     case TSM_REQ_SEV_DISABLE_DMA:
> > +             return tvm_arch == IOMMU_VDEVICE_TSM_TVM_ARCH_SEV;
> Right, this appears to be the only case where the command is
> implementation unique. The handler can only ask iommufd to take
> arch-specific action.

There are 2 arch-specific actions - one is calling TSM to execute actual guest request, the other one is notifying the host IOMMU driver about the device going secure. Like this:

https://github.com/AMDESE/linux-kvm/blob/tsm/drivers/iommu/iommufd/viommu.c#L603

I can tuck domain->ops->tsm_enable into my TSM but rather would not. Thanks,


--
Alexey