Re: [PATCH v7 10/16] iommufd: Add the vdevice TSM request ioctl

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Oct 29, 2026 at 03:07:34PM +0530, Aneesh Kumar K.V wrote:
> Jason Gunthorpe <jgg@nvidia.com> writes:
> 
> >> [ ... 300 lines skipped ... ]
> >> +/**
> >> + * enum iommu_vdevice_tsm_guest_req_op - Guest TSM request operations
> >> + * @TSM_REQ_VALIDATE_MMIO: Validate and map a guest MMIO range for the trusted
> >> + *                         device.
> >> + * @TSM_REQ_SET_TDI_STATE: Transition the trusted device to the unlocked,
> >> + *                         locked, or running state.
> >> + * @TSM_REQ_SEV_ENABLE_DMA: Enable DMA for an SEV device.
> >> + * @TSM_REQ_SEV_DISABLE_DMA: Disable DMA for an SEV device.
> >> + * @TSM_REQ_READ_OBJECT: Read bytes from a device object, such as a certificate,
> >> + *                       measurement, or interface report.
> >> + * @TSM_REQ_REGEN_OBJECT: Regenerate a device object, such as a measurement or
> >> + *                        interface report.
> >> + * @TSM_REQ_OBJECT_INFO: Query the size of a device object.
> >> + * @TSM_REQ_MAX: One past the last request operation; not a command.
> >> + *
> >> + * The request payload and response format depend on the TVM architecture.
> >> + */
> >> +enum iommu_vdevice_tsm_guest_req_op {
> >> +	TSM_REQ_VALIDATE_MMIO = 1,
> >> +	TSM_REQ_SET_TDI_STATE,
> >> +	TSM_REQ_SEV_ENABLE_DMA,
> >> +	TSM_REQ_SEV_DISABLE_DMA,
> >> +	TSM_REQ_READ_OBJECT,
> >> +	TSM_REQ_REGEN_OBJECT,
> >> +	TSM_REQ_OBJECT_INFO,
> >> +	TSM_REQ_MAX,
> >> +};
> >
> > This is still no where close to being well defined enough.
> >
> > Everything here must tie back to defined spec references, listed in
> > the comments.
> >
> > I cannot guess what data is passed over any of these APIs, how things
> > like "REGEN_OBJECT" select which object to regen, or what the format
> > of those objects even is. The formats of req/resp are never specified.
> >
> 
> The request and response payload formats are specific to each CoCo
> backend. Since iommufd treats the payloads as opaque and does not
> validate their contents, what would documenting those formats here
> achieve?

Well if thats the case then no, this is entirely inappropriate.

iommufd requires every single struct to be documented and
described. We do not do undocumented opaque structs into drivers.

So figure out exactly what needs to flow here, give it driver specific
names for driver specific content and document everything.

This should be done patch by patch as the drivers implement their
stuff.

If the data passed is different then it gets a different enum
value. No sharing enum values for different structs because the
drivers are different.

You can say things like 'the content of the RHI message defined in
XYZ' as operations.

> The limited validation is intended to prevent this from becoming a
> general-purpose multiplexer ioctl. 

But we need that for other viommu use cases..

> It restricts the ioctl to a defined set of guest-initiated
> operations that the host forwards to the
> firmware. The path is essentially:
> 
> guest -> vmexit -> VMM -> iommufd ioctl -> TSM backend -> firmware 

Yeah, I'm not adverse to this, so long as it is fully documented and
understandable how each system call maps to both the guest and
firmware interfaces it is supporting.

> > The comment for this should probably be *enormous*. Every arch should
> > explain what they do on every one of these ops, and it should be
> > identical if it is structured like this.
> >
>
> That would require documenting a significant amount of backend-specific
> detail in iommufd. Could we instead require each TSM backend's
> guest_request callback to document its request and response formats?

It needs to be in this header, that is the standard.
 
> > My guesses with no information
> > 1) VALIDATE_MMIO why would the hypervisor ever need this? Guest
> >    validates MMIO through secure communication to the secure world.
> >
> 
> With VFIO PCI BARs backed by guest_memfd, the VMM can validate that the
> IPA and PA values provided by the guest belong to the VFIO device.

Why? Who cares? It isn't a trusted validation, and there is a RSI that
duplicates this?

> We previously also used this operation to ask the CoCo TSM backend
> to map the device memory (the BAR regions). I have removed this
> operation from the latest series because that responsibility is now
> split between the VMM and guest_memfd. TDX and SNP still need to be
> updated accordingly.

Right, it should happen automatically as part of lock.

> > 2) TDI_STATE I thought at least RMM did this with an RSI?
> 
> CCA uses RHI. The RHI reaches the VMM, which issues the ioctl to
> complete the request.

And then the seq numbers check that this went through OK?

> > 3) ENABLE/DISABLE DMA, see #1, must not be done by hypervisor
> >    See my reply to Alexey:
> >    https://lore.kernel.org/linux-iommu/20261022123058.GC3008132@nvidia.com/
> >
> > 4) READ/REGEN/INFO, what are the objects? What format are they in? How
> >    do you select them?
> 
> These objects are cached by the hypervisor. For CCA, they include the
> certificate, measurement, interface report, and VCA. They are read in
> response to guest requests. The requests reach the VMM through an
> RHI, and the VMM issues the ioctl to read the requested object. REGEN
> refreshes the objects/evidence at the guest's request.

> >    Does this overlap with attestation subsystem? Should attestation
> >    subsystem be used to access the SPDM related data?
> >
> 
> 
> I had hoped we could use it, but I could not find an easy way to
> integrate the two. We need a simple way to read these evidence objects
> into userspace. I am not yet sure whether evidence-get can be used for
> that, since it does more than simply read the objects; it also updates
> evidence.generation, among other things.

It should not do that, the generation has to follow the locked state,
not the evidence collection.. See my remark to Dave

I think the bigger issue was it had to use the VDEV flavour not the
PDEV flavor of RMM commands, and it isn't obvious how we switch from
PDEV to VDEV mode on the hypervisor..

> > I think this is probably a fine generic API, it should probably not be
> > "TSM request" so that the other viommu people can reuse the transport.
> >
> > I would probably try to stick to exactly like the other APIs do with a
> > single enum. Two enums for op/arch is saying already that the ops are
> > not well defined.
> >
> > So maybe strip this patch out to have no tsm stuff, define no ops and
> > just give a command transport into the viommu driver?
> >
> 
> Are you suggesting one ioctl per each guest request ops?

One 'u32 op'

Jason