Re: [RFC PATCH 1/3] iommu/riscv: Add guest IMSIC GPA mapping helpers

[fangyu . yu]

>Hi Fangyu,
>
>> From: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
>>
>> IOMMU implementations without the MSI_FLAT capability translate MSI
>> writes through the second-stage page table, so IRQ forwarding on them
>> will need guest IMSIC pages mapped into the second-stage domain.
>>
>> Add an xarray to the MSI table that tracks the HPA installed for each
>> mapped guest IMSIC GPA, and two helpers for use with the MSI table lock
>> held: riscv_iommu_msi_table_map_gpa() installs the initial 4 KiB
>> mapping, and riscv_iommu_msi_table_replace_gpa_leaf() atomically swaps
>> the leaf PTE's PFN when a vCPU's VS-file host page moves, rejecting
>> leaves that do not map the expected old HPA.
>>
>> Signed-off-by: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
>> ---
>>  drivers/iommu/riscv/iommu.c | 94 +++++++++++++++++++++++++++++++++++++
>>  drivers/iommu/riscv/iommu.h | 12 +++++
>>  2 files changed, 106 insertions(+)
>>
>> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
>> index e2e77469ea3c..48fc57d6409c 100644
>> --- a/drivers/iommu/riscv/iommu.c
>> +++ b/drivers/iommu/riscv/iommu.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/moduleparam.h>
>>  #include <linux/mutex.h>
>>  #include <linux/pci.h>
>> +#include <linux/pgtable.h>
>>  #include <linux/generic_pt/iommu.h>
>>
>>  #include "../dma-iommu.h"
>> @@ -1217,6 +1218,92 @@ void riscv_iommu_msi_table_inval_all(struct riscv_iommu_msi_table *msi_table)
>>  	riscv_iommu_iotlb_inval(domain, &gather);
>>  }
>>
>> +int riscv_iommu_msi_table_map_gpa(struct riscv_iommu_msi_table *msi_table,
>> +				  dma_addr_t gpa, phys_addr_t hpa)
>> +{
>> +	struct riscv_iommu_domain *domain =
>> +		container_of(msi_table, struct riscv_iommu_domain, msi_table);
>> +	const int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>> +
>> +	/* Guest IMSIC GPA mapping only exists in second-stage translations. */
>> +	if (!domain->gscid)
>> +		return -EOPNOTSUPP;
>> +
>> +	return iommu_map(&domain->domain, gpa, hpa, IMSIC_MMIO_PAGE_SZ, prot,
>> +			 GFP_ATOMIC);
>> +}
>> +
>> +int riscv_iommu_msi_table_replace_gpa_leaf(struct riscv_iommu_msi_table *msi_table,
>> +					   dma_addr_t gpa, phys_addr_t old_hpa,
>> +					   phys_addr_t new_hpa)
>
>
>This might fit better inside generic_pt rather than in the driver,
>since it walks the page tables on its own and duplicates things
>the library already owns, like the per-level index widths, the
>x4 root size, and the PTE layout.
>
>I recently tried something very similar, and doing this inside
>generic_pt turned out to be quite feasible (only a quick experiment,
>not cleaned up for posting).
>

Hi Shuai,

Thanks for the review. I agree the driver shouldn't carry format
knowledge that belongs to the library, but I think this particular
operation doesn't fit generic_pt.

This is not a generic page-table operation. It is a special
requirement of RISC-V IOMMU IRQ forwarding (irqbypass) on
implementations without an MSI page table: the guest IMSIC GPA
must be mapped into the second-stage domain, and when a vCPU's
VS-file host page migrates, its leaf must be replaced atomically.
Because a device may write an MSI at any moment, unmap + map
would fault. This operation is driven by the IRQ forwarding
state machine, and because I don't see another generic_pt format
sharing this flow, I think these helpers would be better placed
outside generic_pt. I plan to move them into iommu-ir.c so the
feature-scoped code stays together.

Thanks,
Fangyu

>Thanks,
>Shuai