On Fri, 2025-02-28 at 16:44 -0500, Matthew Rosato wrote:
> The origin_type of the dma_table is used to determine how many table
> levels must be traversed for the translation.
>
> Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> ---
> arch/s390/include/asm/pci_dma.h | 2 ++
> drivers/iommu/s390-iommu.c | 52 ++++++++++++++++++++++++++++++++-
> 2 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/include/asm/pci_dma.h b/arch/s390/include/asm/pci_dma.h
> index 42d7cc4262ca..8d8962e4fd58 100644
> --- a/arch/s390/include/asm/pci_dma.h
> +++ b/arch/s390/include/asm/pci_dma.h
> @@ -55,6 +55,8 @@ enum zpci_ioat_dtype {
> #define ZPCI_PT_BITS 8
> #define ZPCI_ST_SHIFT (ZPCI_PT_BITS + PAGE_SHIFT)
> #define ZPCI_RT_SHIFT (ZPCI_ST_SHIFT + ZPCI_TABLE_BITS)
> +#define ZPCI_RS_SHIFT (ZPCI_RT_SHIFT + ZPCI_TABLE_BITS)
> +#define ZPCI_RF_SHIFT (ZPCI_RS_SHIFT + ZPCI_TABLE_BITS)
>
> #define ZPCI_RTE_FLAG_MASK 0x3fffUL
> #define ZPCI_RTE_ADDR_MASK (~ZPCI_RTE_FLAG_MASK)
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index f2cda0ce0fe9..0a6aad11c327 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -36,6 +36,16 @@ struct s390_domain {
>
> static struct iommu_domain blocking_domain;
>
> +static inline unsigned int calc_rfx(dma_addr_t ptr)
> +{
> + return ((unsigned long)ptr >> ZPCI_RF_SHIFT) & ZPCI_INDEX_MASK;
> +}
> +
> +static inline unsigned int calc_rsx(dma_addr_t ptr)
> +{
> + return ((unsigned long)ptr >> ZPCI_RS_SHIFT) & ZPCI_INDEX_MASK;
> +}
> +
> static inline unsigned int calc_rtx(dma_addr_t ptr)
> {
> return ((unsigned long)ptr >> ZPCI_RT_SHIFT) & ZPCI_INDEX_MASK;
> @@ -759,6 +769,43 @@ static int s390_iommu_map_pages(struct iommu_domain *domain,
> return rc;
> }
>
> +static unsigned long *get_rto_from_iova(struct s390_domain *domain,
> + dma_addr_t iova)
> +{
> + unsigned long *rfo, *rso, *rto;
> + unsigned long rfe, rse;
> + unsigned int rfx, rsx;
> +
> + switch (domain->origin_type) {
> + case ZPCI_TABLE_TYPE_RFX:
> + rfo = domain->dma_table;
> + goto itp_rf;
> + case ZPCI_TABLE_TYPE_RSX:
> + rso = domain->dma_table;
> + goto itp_rs;
> + case ZPCI_TABLE_TYPE_RTX:
> + return domain->dma_table;
> + default:
> + return NULL;
> + }
> +
> +itp_rf:
> + rfx = calc_rfx(iova);
> + rfe = READ_ONCE(rfo[rfx]);
> + if (!reg_entry_isvalid(rfe))
> + return NULL;
> + rso = get_rf_rso(rfe);
> +
> +itp_rs:
> + rsx = calc_rsx(iova);
> + rse = READ_ONCE(rso[rsx]);
> + if (!reg_entry_isvalid(rse))
> + return NULL;
> + rto = get_rs_rto(rse);
> +
> + return rto;
> +}
I played around with re-organizing the above as the goto out of the
switch feels a bit cumbersome. One variant I came up with is a separate
get_rso_from_iova() function like below:
static unsigned long *get_rso_from_iova(struct s390_domain *domain,
dma_addr_t iova)
{
unsigned long *rfo;
unsigned long rfe;
unsigned int rfx;
switch (domain->origin_type) {
case ZPCI_TABLE_TYPE_RFX:
rfo = domain->dma_table;
rfx = calc_rfx(iova);
rfe = READ_ONCE(rfo[rfx]);
if (!reg_entry_isvalid(rfe))
return NULL;
return get_rf_rso(rfe);
case ZPCI_TABLE_TYPE_RSX:
return domain->dma_table;
default:
return NULL;
}
}
static unsigned long *get_rto_from_iova(struct s390_domain *domain,
dma_addr_t iova)
{
unsigned long *rso;
unsigned long rse;
unsigned int rsx;
switch (domain->origin_type) {
case ZPCI_TABLE_TYPE_RFX:
case ZPCI_TABLE_TYPE_RSX:
rso = get_rso_from_iova(domain, iova);
rsx = calc_rsx(iova);
rse = READ_ONCE(rso[rsx]);
if (!reg_entry_isvalid(rse))
return NULL;
return get_rs_rto(rse);
case ZPCI_TABLE_TYPE_RTX:
return domain->dma_table;
default:
return NULL;
}
}
I think this is slightly cleaner but not by enough that I'd say we have
to do it this way and I leave the choice to you.
> +
> static phys_addr_t s390_iommu_iova_to_phys(struct iommu_domain *domain,
> dma_addr_t iova)
> {
> @@ -772,10 +819,13 @@ static phys_addr_t s390_iommu_iova_to_phys(struct iommu_domain *domain,
> iova > domain->geometry.aperture_end)
> return 0;
>
> + rto = get_rto_from_iova(s390_domain, iova);
> + if (!rto)
> + return 0;
> +
> rtx = calc_rtx(iova);
> sx = calc_sx(iova);
> px = calc_px(iova);
> - rto = s390_domain->dma_table;
>
> rte = READ_ONCE(rto[rtx]);
> if (reg_entry_isvalid(rte)) {
So with or without my suggestion.
Reviewed-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
