Re: [PATCH v3 3/3] iommu/s390: implement iommu passthrough via identity domain

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 1/30/25 2:43 AM, Niklas Schnelle wrote:
> On Fri, 2025-01-24 at 15:17 -0500, Matthew Rosato wrote:
>> Enabled via the kernel command-line 'iommu.passthrough=1' option.
>>
>> Introduce the concept of identity domains to s390-iommu, which relies on
>> the bus_dma_region to offset identity mappings to the start of the DMA
>> aperture advertized by CLP.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>> ---
>>  arch/s390/pci/pci.c        |  6 ++-
>>  drivers/iommu/s390-iommu.c | 95 +++++++++++++++++++++++++++++---------
>>  2 files changed, 76 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
>> index 88f72745fa59..758b23331754 100644
>> --- a/arch/s390/pci/pci.c
>> +++ b/arch/s390/pci/pci.c
>> @@ -124,14 +124,16 @@ int zpci_register_ioat(struct zpci_dev *zdev, u8 dmaas,
>>  	struct zpci_fib fib = {0};
>>  	u8 cc;
>>  
>> -	WARN_ON_ONCE(iota & 0x3fff);
>>  	fib.pba = base;
>>  	/* Work around off by one in ISM virt device */
>>  	if (zdev->pft == PCI_FUNC_TYPE_ISM && limit > base)
>>  		fib.pal = limit + (1 << 12);
>>  	else
>>  		fib.pal = limit;
>> -	fib.iota = iota | ZPCI_IOTA_RTTO_FLAG;
>> +	if (iota == 0)
>> +		fib.iota = iota;
> 
> Taking another look, I think there is a small problem with the logic of
> passing iota == 0 to indicate direct mapping. In
> zpci_hot_reset_device() we call zpci_register_ioat() with iota set to
> virt_to_phys(zdev->dma_table) and while zdev->dma_table is NULL for the
> identity domain we can't rely on virt_to_phys(NULL) == NULL.

Thanks for pointing this out.  We discussed this a fair bit off-list, but to summarize here: I'll include an additional patch in the next version that will drive IOAT registration code through s390-iommu, since really this owns the dma_table and knows when it does (not) make sense to register it.  Places in s390 code that need to perform IOAT re-registration (such as zpci_hot_reset_device) will call into this routine and let s390-iommu determine the correct action based upon the type of domain in use.  

I'll set that patch before this one, so then this patch remains unchanged except to replace the zpci_register_ioat call added by this patch in s390_attach_dev_identity with the new routine.

> 
>> +	else
>> +		fib.iota = iota | ZPCI_IOTA_RTTO_FLAG;
>>  	fib.gd = zdev->gisa;
>>  	cc = zpci_mod_fc(req, &fib, status);
>>  	if (cc)