linux-kernel - Re: [PATCH v5 4/5] iommu/s390: support map/unmap for additional table regions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1cb8f6c28a74abfbda24f551eb9981a832b5009a.camel@linux.ibm.com>
Date: Wed, 16 Apr 2025 11:19:52 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Matthew Rosato <mjrosato@...ux.ibm.com>, joro@...tes.org, will@...nel.org,
        robin.murphy@....com, gerald.schaefer@...ux.ibm.com
Cc: hca@...ux.ibm.com, gor@...ux.ibm.com, agordeev@...ux.ibm.com,
        svens@...ux.ibm.com, borntraeger@...ux.ibm.com, clg@...hat.com,
        iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
        linux-s390@...r.kernel.org
Subject: Re: [PATCH v5 4/5] iommu/s390: support map/unmap for additional
 table regions

On Fri, 2025-04-11 at 16:24 -0400, Matthew Rosato wrote:
> Map and unmap ops use the shared dma_walk_cpu_trans routine, update
> this using the origin_type of the dma_table to determine how many
> table levels must be walked.
> 
> Signed-off-by: Matthew Rosato <mjrosato@...ux.ibm.com>
> ---
>  drivers/iommu/s390-iommu.c | 127 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 119 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index 338a7381e918..46f45b136993 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -67,6 +67,20 @@ static inline void set_pt_pfaa(unsigned long *entry, phys_addr_t pfaa)
>  	*entry |= (pfaa & ZPCI_PTE_ADDR_MASK);
>  }
>  
> +static inline void set_rf_rso(unsigned long *entry, phys_addr_t rso)
> +{
> +	*entry &= ZPCI_RTE_FLAG_MASK;
> +	*entry |= (rso & ZPCI_RTE_ADDR_MASK);
> +	*entry |= ZPCI_TABLE_TYPE_RFX;
> +}
> +
> +static inline void set_rs_rto(unsigned long *entry, phys_addr_t rto)
> +{
> +	*entry &= ZPCI_RTE_FLAG_MASK;
> +	*entry |= (rto & ZPCI_RTE_ADDR_MASK);
> +	*entry |= ZPCI_TABLE_TYPE_RSX;
> +}
> +
>  static inline void set_rt_sto(unsigned long *entry, phys_addr_t sto)
>  {
>  	*entry &= ZPCI_RTE_FLAG_MASK;
> @@ -81,6 +95,22 @@ static inline void set_st_pto(unsigned long *entry, phys_addr_t pto)
>  	*entry |= ZPCI_TABLE_TYPE_SX;
>  }
>  
> +static inline void validate_rf_entry(unsigned long *entry)
> +{
> +	*entry &= ~ZPCI_TABLE_VALID_MASK;
> +	*entry &= ~ZPCI_TABLE_OFFSET_MASK;
> +	*entry |= ZPCI_TABLE_VALID;
> +	*entry |= ZPCI_TABLE_LEN_RFX;
> +}
> +
> +static inline void validate_rs_entry(unsigned long *entry)
> +{
> +	*entry &= ~ZPCI_TABLE_VALID_MASK;
> +	*entry &= ~ZPCI_TABLE_OFFSET_MASK;
> +	*entry |= ZPCI_TABLE_VALID;
> +	*entry |= ZPCI_TABLE_LEN_RSX;
> +}
> +
> 
--- snip ---
>  
> -static unsigned long *dma_walk_cpu_trans(unsigned long *rto, dma_addr_t dma_addr, gfp_t gfp)
> +static unsigned long *dma_walk_region_tables(struct s390_domain *domain,
> +					     dma_addr_t dma_addr, gfp_t gfp)
>  {
> -	unsigned long *sto, *pto;
> +	switch (domain->origin_type) {
> +	case ZPCI_TABLE_TYPE_RFX:
> +		return dma_walk_rf_table(domain->dma_table, dma_addr, gfp);
> +	case ZPCI_TABLE_TYPE_RSX:
> +		return dma_walk_rs_table(domain->dma_table, dma_addr, gfp);
> +	case ZPCI_TABLE_TYPE_RTX:
> +		return domain->dma_table;
> +	default:
> +		return NULL;
> +	}
> +}
> +
> +static unsigned long *dma_walk_cpu_trans(struct s390_domain *domain,
> +					 dma_addr_t dma_addr, gfp_t gfp)
> +{
> +	unsigned long *rto, *sto, *pto;
>  	unsigned int rtx, sx, px;
>  
> +	rto = dma_walk_region_tables(domain, dma_addr, gfp);
> +	if (!rto)
> +		return NULL;
> +
>  	rtx = calc_rtx(dma_addr);
>  	sto = dma_get_seg_table_origin(&rto[rtx], gfp);
>  	if (!sto)
> @@ -690,8 +804,7 @@ static int s390_iommu_validate_trans(struct s390_domain *s390_domain,
>  	int rc;
>  
>  	for (i = 0; i < nr_pages; i++) {
> -		entry = dma_walk_cpu_trans(s390_domain->dma_table, dma_addr,
> -					   gfp);
> +		entry = dma_walk_cpu_trans(s390_domain, dma_addr, gfp);
>  		if (unlikely(!entry)) {
>  			rc = -ENOMEM;
>  			goto undo_cpu_trans;
> @@ -706,8 +819,7 @@ static int s390_iommu_validate_trans(struct s390_domain *s390_domain,
>  undo_cpu_trans:
>  	while (i-- > 0) {
>  		dma_addr -= PAGE_SIZE;
> -		entry = dma_walk_cpu_trans(s390_domain->dma_table,
> -					   dma_addr, gfp);
> +		entry = dma_walk_cpu_trans(s390_domain, dma_addr, gfp);
>  		if (!entry)
>  			break;
>  		dma_update_cpu_trans(entry, 0, ZPCI_PTE_INVALID);
> @@ -724,8 +836,7 @@ static int s390_iommu_invalidate_trans(struct s390_domain *s390_domain,
>  	int rc = 0;
>  
>  	for (i = 0; i < nr_pages; i++) {
> -		entry = dma_walk_cpu_trans(s390_domain->dma_table, dma_addr,
> -					   GFP_ATOMIC);
> +		entry = dma_walk_cpu_trans(s390_domain, dma_addr, GFP_ATOMIC);
>  		if (unlikely(!entry)) {
>  			rc = -EINVAL;
>  			break;

Looks good to me and we even get nicer formatting :)

Reviewed-by: Niklas Schnelle <schnelle@...ux.ibm.com>