2022-01-14 23:06:45

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v2 13/30] s390/pci: return status from zpci_refresh_trans

Current callers of zpci_refresh_trans don't need to interrogate the status
returned from the underlying instructions. However, a subsequent patch
will add a KVM caller that needs this information. Add a new argument to
zpci_refresh_trans to pass the address of a status byte and update
existing call sites to provide it.

Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/pci_insn.h | 2 +-
arch/s390/pci/pci_dma.c | 6 ++++--
arch/s390/pci/pci_insn.c | 10 +++++-----
drivers/iommu/s390-iommu.c | 4 +++-
4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
index 5331082fa516..32759c407b8f 100644
--- a/arch/s390/include/asm/pci_insn.h
+++ b/arch/s390/include/asm/pci_insn.h
@@ -135,7 +135,7 @@ union zpci_sic_iib {
DECLARE_STATIC_KEY_FALSE(have_mio);

u8 zpci_mod_fc(u64 req, struct zpci_fib *fib, u8 *status);
-int zpci_refresh_trans(u64 fn, u64 addr, u64 range);
+int zpci_refresh_trans(u64 fn, u64 addr, u64 range, u8 *status);
int __zpci_load(u64 *data, u64 req, u64 offset);
int zpci_load(u64 *data, const volatile void __iomem *addr, unsigned long len);
int __zpci_store(u64 data, u64 req, u64 offset);
diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index a81de48d5ea7..b0a2380bcad8 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -23,8 +23,9 @@ static u32 s390_iommu_aperture_factor = 1;

static int zpci_refresh_global(struct zpci_dev *zdev)
{
+ u8 status;
return zpci_refresh_trans((u64) zdev->fh << 32, zdev->start_dma,
- zdev->iommu_pages * PAGE_SIZE);
+ zdev->iommu_pages * PAGE_SIZE, &status);
}

unsigned long *dma_alloc_cpu_table(void)
@@ -183,6 +184,7 @@ static int __dma_purge_tlb(struct zpci_dev *zdev, dma_addr_t dma_addr,
size_t size, int flags)
{
unsigned long irqflags;
+ u8 status;
int ret;

/*
@@ -201,7 +203,7 @@ static int __dma_purge_tlb(struct zpci_dev *zdev, dma_addr_t dma_addr,
}

ret = zpci_refresh_trans((u64) zdev->fh << 32, dma_addr,
- PAGE_ALIGN(size));
+ PAGE_ALIGN(size), &status);
if (ret == -ENOMEM && !s390_iommu_strict) {
/* enable the hypervisor to free some resources */
if (zpci_refresh_global(zdev))
diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
index 0509554301c7..ca6399d52767 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -77,20 +77,20 @@ static inline u8 __rpcit(u64 fn, u64 addr, u64 range, u8 *status)
return cc;
}

-int zpci_refresh_trans(u64 fn, u64 addr, u64 range)
+int zpci_refresh_trans(u64 fn, u64 addr, u64 range, u8 *status)
{
- u8 cc, status;
+ u8 cc;

do {
- cc = __rpcit(fn, addr, range, &status);
+ cc = __rpcit(fn, addr, range, status);
if (cc == 2)
udelay(ZPCI_INSN_BUSY_DELAY);
} while (cc == 2);

if (cc)
- zpci_err_insn(cc, status, addr, range);
+ zpci_err_insn(cc, *status, addr, range);

- if (cc == 1 && (status == 4 || status == 16))
+ if (cc == 1 && (*status == 4 || *status == 16))
return -ENOMEM;

return (cc) ? -EIO : 0;
diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index 50860ebdd087..845bb99c183e 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -214,6 +214,7 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
unsigned long irq_flags, nr_pages, i;
unsigned long *entry;
int rc = 0;
+ u8 status;

if (dma_addr < s390_domain->domain.geometry.aperture_start ||
dma_addr + size > s390_domain->domain.geometry.aperture_end)
@@ -238,7 +239,8 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
spin_lock(&s390_domain->list_lock);
list_for_each_entry(domain_device, &s390_domain->devices, list) {
rc = zpci_refresh_trans((u64) domain_device->zdev->fh << 32,
- start_dma_addr, nr_pages * PAGE_SIZE);
+ start_dma_addr, nr_pages * PAGE_SIZE,
+ &status);
if (rc)
break;
}
--
2.27.0


2022-01-21 19:56:17

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v2 13/30] s390/pci: return status from zpci_refresh_trans



On 1/14/22 21:31, Matthew Rosato wrote:
> Current callers of zpci_refresh_trans don't need to interrogate the status
> returned from the underlying instructions. However, a subsequent patch
> will add a KVM caller that needs this information. Add a new argument to
> zpci_refresh_trans to pass the address of a status byte and update
> existing call sites to provide it.
>
> Signed-off-by: Matthew Rosato <[email protected]>

Reviewed-by: Pierre Morel <[email protected]>


> ---
> arch/s390/include/asm/pci_insn.h | 2 +-
> arch/s390/pci/pci_dma.c | 6 ++++--
> arch/s390/pci/pci_insn.c | 10 +++++-----
> drivers/iommu/s390-iommu.c | 4 +++-
> 4 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
> index 5331082fa516..32759c407b8f 100644
> --- a/arch/s390/include/asm/pci_insn.h
> +++ b/arch/s390/include/asm/pci_insn.h
> @@ -135,7 +135,7 @@ union zpci_sic_iib {
> DECLARE_STATIC_KEY_FALSE(have_mio);
>
> u8 zpci_mod_fc(u64 req, struct zpci_fib *fib, u8 *status);
> -int zpci_refresh_trans(u64 fn, u64 addr, u64 range);
> +int zpci_refresh_trans(u64 fn, u64 addr, u64 range, u8 *status);
> int __zpci_load(u64 *data, u64 req, u64 offset);
> int zpci_load(u64 *data, const volatile void __iomem *addr, unsigned long len);
> int __zpci_store(u64 data, u64 req, u64 offset);
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index a81de48d5ea7..b0a2380bcad8 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -23,8 +23,9 @@ static u32 s390_iommu_aperture_factor = 1;
>
> static int zpci_refresh_global(struct zpci_dev *zdev)
> {
> + u8 status;
> return zpci_refresh_trans((u64) zdev->fh << 32, zdev->start_dma,
> - zdev->iommu_pages * PAGE_SIZE);
> + zdev->iommu_pages * PAGE_SIZE, &status);
> }
>
> unsigned long *dma_alloc_cpu_table(void)
> @@ -183,6 +184,7 @@ static int __dma_purge_tlb(struct zpci_dev *zdev, dma_addr_t dma_addr,
> size_t size, int flags)
> {
> unsigned long irqflags;
> + u8 status;
> int ret;
>
> /*
> @@ -201,7 +203,7 @@ static int __dma_purge_tlb(struct zpci_dev *zdev, dma_addr_t dma_addr,
> }
>
> ret = zpci_refresh_trans((u64) zdev->fh << 32, dma_addr,
> - PAGE_ALIGN(size));
> + PAGE_ALIGN(size), &status);
> if (ret == -ENOMEM && !s390_iommu_strict) {
> /* enable the hypervisor to free some resources */
> if (zpci_refresh_global(zdev))
> diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> index 0509554301c7..ca6399d52767 100644
> --- a/arch/s390/pci/pci_insn.c
> +++ b/arch/s390/pci/pci_insn.c
> @@ -77,20 +77,20 @@ static inline u8 __rpcit(u64 fn, u64 addr, u64 range, u8 *status)
> return cc;
> }
>
> -int zpci_refresh_trans(u64 fn, u64 addr, u64 range)
> +int zpci_refresh_trans(u64 fn, u64 addr, u64 range, u8 *status)
> {
> - u8 cc, status;
> + u8 cc;
>
> do {
> - cc = __rpcit(fn, addr, range, &status);
> + cc = __rpcit(fn, addr, range, status);
> if (cc == 2)
> udelay(ZPCI_INSN_BUSY_DELAY);
> } while (cc == 2);
>
> if (cc)
> - zpci_err_insn(cc, status, addr, range);
> + zpci_err_insn(cc, *status, addr, range);
>
> - if (cc == 1 && (status == 4 || status == 16))
> + if (cc == 1 && (*status == 4 || *status == 16))
> return -ENOMEM;
>
> return (cc) ? -EIO : 0;
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index 50860ebdd087..845bb99c183e 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -214,6 +214,7 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
> unsigned long irq_flags, nr_pages, i;
> unsigned long *entry;
> int rc = 0;
> + u8 status;
>
> if (dma_addr < s390_domain->domain.geometry.aperture_start ||
> dma_addr + size > s390_domain->domain.geometry.aperture_end)
> @@ -238,7 +239,8 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
> spin_lock(&s390_domain->list_lock);
> list_for_each_entry(domain_device, &s390_domain->devices, list) {
> rc = zpci_refresh_trans((u64) domain_device->zdev->fh << 32,
> - start_dma_addr, nr_pages * PAGE_SIZE);
> + start_dma_addr, nr_pages * PAGE_SIZE,
> + &status);
> if (rc)
> break;
> }
>

--
Pierre Morel
IBM Lab Boeblingen

2022-01-26 21:06:15

by Claudio Imbrenda

[permalink] [raw]
Subject: Re: [PATCH v2 13/30] s390/pci: return status from zpci_refresh_trans

On Fri, 14 Jan 2022 15:31:28 -0500
Matthew Rosato <[email protected]> wrote:

> Current callers of zpci_refresh_trans don't need to interrogate the status
> returned from the underlying instructions. However, a subsequent patch
> will add a KVM caller that needs this information. Add a new argument to
> zpci_refresh_trans to pass the address of a status byte and update
> existing call sites to provide it.
>
> Signed-off-by: Matthew Rosato <[email protected]>

Reviewed-by: Claudio Imbrenda <[email protected]>

> ---
> arch/s390/include/asm/pci_insn.h | 2 +-
> arch/s390/pci/pci_dma.c | 6 ++++--
> arch/s390/pci/pci_insn.c | 10 +++++-----
> drivers/iommu/s390-iommu.c | 4 +++-
> 4 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
> index 5331082fa516..32759c407b8f 100644
> --- a/arch/s390/include/asm/pci_insn.h
> +++ b/arch/s390/include/asm/pci_insn.h
> @@ -135,7 +135,7 @@ union zpci_sic_iib {
> DECLARE_STATIC_KEY_FALSE(have_mio);
>
> u8 zpci_mod_fc(u64 req, struct zpci_fib *fib, u8 *status);
> -int zpci_refresh_trans(u64 fn, u64 addr, u64 range);
> +int zpci_refresh_trans(u64 fn, u64 addr, u64 range, u8 *status);
> int __zpci_load(u64 *data, u64 req, u64 offset);
> int zpci_load(u64 *data, const volatile void __iomem *addr, unsigned long len);
> int __zpci_store(u64 data, u64 req, u64 offset);
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index a81de48d5ea7..b0a2380bcad8 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -23,8 +23,9 @@ static u32 s390_iommu_aperture_factor = 1;
>
> static int zpci_refresh_global(struct zpci_dev *zdev)
> {
> + u8 status;
> return zpci_refresh_trans((u64) zdev->fh << 32, zdev->start_dma,
> - zdev->iommu_pages * PAGE_SIZE);
> + zdev->iommu_pages * PAGE_SIZE, &status);
> }
>
> unsigned long *dma_alloc_cpu_table(void)
> @@ -183,6 +184,7 @@ static int __dma_purge_tlb(struct zpci_dev *zdev, dma_addr_t dma_addr,
> size_t size, int flags)
> {
> unsigned long irqflags;
> + u8 status;
> int ret;
>
> /*
> @@ -201,7 +203,7 @@ static int __dma_purge_tlb(struct zpci_dev *zdev, dma_addr_t dma_addr,
> }
>
> ret = zpci_refresh_trans((u64) zdev->fh << 32, dma_addr,
> - PAGE_ALIGN(size));
> + PAGE_ALIGN(size), &status);
> if (ret == -ENOMEM && !s390_iommu_strict) {
> /* enable the hypervisor to free some resources */
> if (zpci_refresh_global(zdev))
> diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> index 0509554301c7..ca6399d52767 100644
> --- a/arch/s390/pci/pci_insn.c
> +++ b/arch/s390/pci/pci_insn.c
> @@ -77,20 +77,20 @@ static inline u8 __rpcit(u64 fn, u64 addr, u64 range, u8 *status)
> return cc;
> }
>
> -int zpci_refresh_trans(u64 fn, u64 addr, u64 range)
> +int zpci_refresh_trans(u64 fn, u64 addr, u64 range, u8 *status)
> {
> - u8 cc, status;
> + u8 cc;
>
> do {
> - cc = __rpcit(fn, addr, range, &status);
> + cc = __rpcit(fn, addr, range, status);
> if (cc == 2)
> udelay(ZPCI_INSN_BUSY_DELAY);
> } while (cc == 2);
>
> if (cc)
> - zpci_err_insn(cc, status, addr, range);
> + zpci_err_insn(cc, *status, addr, range);
>
> - if (cc == 1 && (status == 4 || status == 16))
> + if (cc == 1 && (*status == 4 || *status == 16))
> return -ENOMEM;
>
> return (cc) ? -EIO : 0;
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index 50860ebdd087..845bb99c183e 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -214,6 +214,7 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
> unsigned long irq_flags, nr_pages, i;
> unsigned long *entry;
> int rc = 0;
> + u8 status;
>
> if (dma_addr < s390_domain->domain.geometry.aperture_start ||
> dma_addr + size > s390_domain->domain.geometry.aperture_end)
> @@ -238,7 +239,8 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
> spin_lock(&s390_domain->list_lock);
> list_for_each_entry(domain_device, &s390_domain->devices, list) {
> rc = zpci_refresh_trans((u64) domain_device->zdev->fh << 32,
> - start_dma_addr, nr_pages * PAGE_SIZE);
> + start_dma_addr, nr_pages * PAGE_SIZE,
> + &status);
> if (rc)
> break;
> }

2022-01-27 17:58:39

by Niklas Schnelle

[permalink] [raw]
Subject: Re: [PATCH v2 13/30] s390/pci: return status from zpci_refresh_trans

On Fri, 2022-01-14 at 15:31 -0500, Matthew Rosato wrote:
> Current callers of zpci_refresh_trans don't need to interrogate the status
> returned from the underlying instructions. However, a subsequent patch
> will add a KVM caller that needs this information. Add a new argument to
> zpci_refresh_trans to pass the address of a status byte and update
> existing call sites to provide it.
>
> Signed-off-by: Matthew Rosato <[email protected]>
> ---
> arch/s390/include/asm/pci_insn.h | 2 +-
> arch/s390/pci/pci_dma.c | 6 ++++--
> arch/s390/pci/pci_insn.c | 10 +++++-----
> drivers/iommu/s390-iommu.c | 4 +++-
> 4 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
> index 5331082fa516..32759c407b8f 100644
> --- a/arch/s390/include/asm/pci_insn.h
> +++ b/arch/s390/include/asm/pci_insn.h
> @@ -135,7 +135,7 @@ union zpci_sic_iib {
> DECLARE_STATIC_KEY_FALSE(have_mio);
>
> u8 zpci_mod_fc(u64 req, struct zpci_fib *fib, u8 *status);
> -int zpci_refresh_trans(u64 fn, u64 addr, u64 range);
> +int zpci_refresh_trans(u64 fn, u64 addr, u64 range, u8 *status);
>
---8<---
>
> return (cc) ? -EIO : 0;
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index 50860ebdd087..845bb99c183e 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -214,6 +214,7 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
> unsigned long irq_flags, nr_pages, i;
> unsigned long *entry;
> int rc = 0;
> + u8 status;
>
> if (dma_addr < s390_domain->domain.geometry.aperture_start ||
> dma_addr + size > s390_domain->domain.geometry.aperture_end)
> @@ -238,7 +239,8 @@ static int s390_iommu_update_trans(struct s390_domain *s390_domain,
> spin_lock(&s390_domain->list_lock);
> list_for_each_entry(domain_device, &s390_domain->devices, list) {
> rc = zpci_refresh_trans((u64) domain_device->zdev->fh << 32,
> - start_dma_addr, nr_pages * PAGE_SIZE);
> + start_dma_addr, nr_pages * PAGE_SIZE,
> + &status);
> if (rc)
> break;
> }

Looks good.

Reviewed-by: Niklas Schnelle <[email protected]>