[PATCH v2 0/3] s390/pci: Updates to PCI insn tracing

[PATCH v2 0/3] s390/pci: Updates to PCI insn tracing

[Gerd Bayer]

While debugging some issues, I found that insn traces for RPCIT are
lacking an indication for which PCI function translation tables needed
to be synchronized with hypervisors and/or firmware and that the 64bit
address fields are traced at odd addresses. While looking into how to
address this, I found another small oddity in that the code leading to
the RPCIT instruction was expanding the 32bit function handle to a 64bit
value for the inline assembly way too high in the call-chain.
    
So I started this small series to address these three issues:
1/3 - Insert padding into the insn trace at natural boundaries
2/3 - Pass function handle in 32bit variables as long as possible
3/3 - Introduce a RPCIT variant of insn trace - adding function handle
    
With this series applied (+ a test patch to trace RPCIT even in CC0
situations) an entry in /sys/kernel/debug/s390dbf/pci_error/hex_ascii
looks like the following (removed time-stamp and ASCII transcription,
replaced instruction address with "<IA>"):

 2 - 0001 <IA>  52 00 00 00 00 00 00 00 85 00 32 2e 00 00 00 00
                ^  ^  ^                 <--------->
                |  |  Status              function handle
                |  Condition Code
                "encoded" insn

 2 - 0001 <IA>  00 00 00 01 ff 04 90 00 00 00 00 00 00 00 10 00
                                        <--------------------->
                <--------------------->       IOVA range
                      start IOVA

I hope you'll find this useful,
Gerd

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
Changes in v2:
- 1/3 and 3/3: Replace unnamed bitfields with named dummy members for
  padding (address Sashiko findings)
- Link to v1: https://lore.kernel.org/r/20261007-rpcit_trcfh_upstream-v1-0-8a2718cfb90b@linux.ibm.com

---
Gerd Bayer (3):
      s390/pci: Adjust alignment in insn trace
      s390/pci: Use 32-bit fh outside of inline asm
      s390/pci: Add function handle to RPCIT insn trace

 arch/s390/include/asm/pci_insn.h |  2 +-
 arch/s390/pci/pci_insn.c         | 42 +++++++++++++++++++++++++++++++++-------
 drivers/iommu/s390-iommu.c       |  8 +++-----
 3 files changed, 39 insertions(+), 13 deletions(-)
---
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
change-id: 20261007-rpcit_trcfh_upstream-3e6fa73dff1b

Best regards,
-- 
Gerd Bayer <gbayer@linux.ibm.com>

[PATCH v2 1/3] s390/pci: Adjust alignment in insn trace

[Gerd Bayer]

Currently we have 64bit wide fields start at odd byte-offsets in the
insn traces for pci_error in the s390dbf debug area, which are hard to
decode in hex dumps. At the same time, trace entries are padded at the
end.

Drop the packed attribute on the struct definition to exploit the
compiler's natural alignment, introduce explicit padding before and
after the 64bit fields for humans, and add a static assertion to avoid
accidential overruns of 2 lines in the s390dbf trace.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
Reviewed-by: Benjamin Block <bblock@linux.ibm.com>
---
 arch/s390/pci/pci_insn.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
index 35ceb1bea1c6..1089b539aeb6 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -18,11 +18,13 @@
 #include <asm/asm.h>
 
 #define ZPCI_INSN_BUSY_DELAY	1	/* 1 microsecond */
+#define ZPCI_ERR_INSN_DATA_SIZE 32
 
 struct zpci_err_insn_data {
 	u8 insn;
 	u8 cc;
 	u8 status;
+	u8 pad0[5];
 	union {
 		struct {
 			u64 req;
@@ -33,7 +35,10 @@ struct zpci_err_insn_data {
 			u64 len;
 		};
 	};
-} __packed;
+	u64 pad1;
+};
+
+static_assert(sizeof(struct zpci_err_insn_data) <= ZPCI_ERR_INSN_DATA_SIZE);
 
 static inline void zpci_err_insn_req(int lvl, u8 insn, u8 cc, u8 status,
 				     u64 req, u64 offset)

-- 
2.53.0

Re: [PATCH v2 1/3] s390/pci: Adjust alignment in insn trace

[Matthew Rosato]

On 10/12/26 10:21 AM, Gerd Bayer wrote:
> Currently we have 64bit wide fields start at odd byte-offsets in the
> insn traces for pci_error in the s390dbf debug area, which are hard to
> decode in hex dumps. At the same time, trace entries are padded at the
> end.
> 
> Drop the packed attribute on the struct definition to exploit the
> compiler's natural alignment, introduce explicit padding before and
> after the 64bit fields for humans, and add a static assertion to avoid
> accidential overruns of 2 lines in the s390dbf trace.

nit: typo 'accidental'

> 
> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> Reviewed-by: Benjamin Block <bblock@linux.ibm.com>

AFAICT no impact to the amount of space used in s390dbf either since we
were already claiming 2 16B entries -- but the result is far more readable.

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

[PATCH v2 2/3] s390/pci: Use 32-bit fh outside of inline asm

[Gerd Bayer]

zpci_refresh_trans() declared the PCI function handle as a u64 type just
because the actual RPCIT machine instruction required that in a 64bit
register, while the function handle is architected as a 32bit type and
defined as such in the zpci_dev struct.

Push the extension from u32 to u64 down in the call chain to just before
calling the RPCIT wrapper function. Since the struct of the typical
req's matches the format expected by RPCIT, too, create one of those to
pass along.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 arch/s390/include/asm/pci_insn.h | 2 +-
 arch/s390/pci/pci_insn.c         | 5 +++--
 drivers/iommu/s390-iommu.c       | 8 +++-----
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
index 025c6dcbf893..e6613b037f93 100644
--- a/arch/s390/include/asm/pci_insn.h
+++ b/arch/s390/include/asm/pci_insn.h
@@ -147,7 +147,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(u32 fh, u64 addr, u64 range);
 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_insn.c b/arch/s390/pci/pci_insn.c
index 1089b539aeb6..3e55adc085c3 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -116,13 +116,14 @@ static inline u8 __rpcit(u64 fn, u64 addr, u64 range, u8 *status)
 	return CC_TRANSFORM(cc);
 }
 
-int zpci_refresh_trans(u64 fn, u64 addr, u64 range)
+int zpci_refresh_trans(u32 fh, u64 addr, u64 range)
 {
+	u64 req = ZPCI_CREATE_REQ(fh, 0, 0);
 	bool retried = false;
 	u8 cc, status;
 
 	do {
-		cc = __rpcit(fn, addr, range, &status);
+		cc = __rpcit(req, addr, range, &status);
 		if (cc == 2) {
 			udelay(ZPCI_INSN_BUSY_DELAY);
 			if (!retried) {
diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index 58ca7727b7f2..a280ad4f63b7 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -788,7 +788,7 @@ static struct iommu_device *s390_iommu_probe_device(struct device *dev)
 
 static int zpci_refresh_all(struct zpci_dev *zdev)
 {
-	return zpci_refresh_trans((u64)zdev->fh << 32, zdev->start_dma,
+	return zpci_refresh_trans(zdev->fh, zdev->start_dma,
 				  zdev->end_dma - zdev->start_dma + 1);
 }
 
@@ -819,8 +819,7 @@ static void s390_iommu_iotlb_sync(struct iommu_domain *domain,
 	rcu_read_lock();
 	list_for_each_entry_rcu(zdev, &s390_domain->devices, iommu_list) {
 		atomic64_inc(&s390_domain->ctrs.sync_rpcits);
-		zpci_refresh_trans((u64)zdev->fh << 32, gather->start,
-				   size);
+		zpci_refresh_trans(zdev->fh, gather->start, size);
 	}
 	rcu_read_unlock();
 }
@@ -837,8 +836,7 @@ static int s390_iommu_iotlb_sync_map(struct iommu_domain *domain,
 		if (!zdev->tlb_refresh)
 			continue;
 		atomic64_inc(&s390_domain->ctrs.sync_map_rpcits);
-		ret = zpci_refresh_trans((u64)zdev->fh << 32,
-					 iova, size);
+		ret = zpci_refresh_trans(zdev->fh, iova, size);
 		/*
 		 * let the hypervisor discover invalidated entries
 		 * allowing it to free IOVAs and unpin pages

-- 
2.53.0

Re: [PATCH v2 2/3] s390/pci: Use 32-bit fh outside of inline asm

[Matthew Rosato]

On 10/12/26 10:21 AM, Gerd Bayer wrote:
> zpci_refresh_trans() declared the PCI function handle as a u64 type just
> because the actual RPCIT machine instruction required that in a 64bit
> register, while the function handle is architected as a 32bit type and
> defined as such in the zpci_dev struct.
> 
> Push the extension from u32 to u64 down in the call chain to just before
> calling the RPCIT wrapper function. Since the struct of the typical
> req's matches the format expected by RPCIT, too, create one of those to
> pass along.
> 
> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

[PATCH v2 3/3] s390/pci: Add function handle to RPCIT insn trace

[Gerd Bayer]

In certain debug situations it would be helpful, if the insn trace for a
failing RPCIT would reveal the PCI function on which this was attempted.

Introduce a variant of zpci_err_insn_req() called zpci_err_insn_rpcit()
that accepts an IO virtual address and a range parameter together with
the req conveying the function handle. Switch zpci_refresh_trans() to
use that new error trace.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 arch/s390/pci/pci_insn.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
index 3e55adc085c3..0626667cecc6 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -28,18 +28,40 @@ struct zpci_err_insn_data {
 	union {
 		struct {
 			u64 req;
-			u64 offset;
+			union {
+				/* PCILG/PCISTG/MPCIFC */
+				struct {
+					u64 offset;
+					u64 pad1;
+				};
+				/* RPCIT */
+				struct {
+					u64 iova;
+					u64 range;
+				};
+			};
 		};
+		/* PCILGI/PCISTGI */
 		struct {
 			u64 addr;
 			u64 len;
+			u64 pad2;
 		};
 	};
-	u64 pad1;
 };
 
 static_assert(sizeof(struct zpci_err_insn_data) <= ZPCI_ERR_INSN_DATA_SIZE);
 
+static inline void zpci_err_insn_rpcit(int lvl, u8 insn, u8 cc, u8 status,
+				       u64 req, u64 iova, u64 range)
+{
+	struct zpci_err_insn_data data = {
+		.insn = insn, .cc = cc, .status = status,
+		.req = req, .iova = iova, .range = range};
+
+	zpci_err_hex_level(lvl, &data, sizeof(data));
+}
+
 static inline void zpci_err_insn_req(int lvl, u8 insn, u8 cc, u8 status,
 				     u64 req, u64 offset)
 {
@@ -127,16 +149,16 @@ int zpci_refresh_trans(u32 fh, u64 addr, u64 range)
 		if (cc == 2) {
 			udelay(ZPCI_INSN_BUSY_DELAY);
 			if (!retried) {
-				zpci_err_insn_addr(1, 'R', cc, status, addr, range);
+				zpci_err_insn_rpcit(1, 'R', cc, status, req, addr, range);
 				retried = true;
 			}
 		}
 	} while (cc == 2);
 
 	if (cc)
-		zpci_err_insn_addr(0, 'R', cc, status, addr, range);
+		zpci_err_insn_rpcit(0, 'R', cc, status, req, addr, range);
 	else if (retried)
-		zpci_err_insn_addr(1, 'R', cc, status, addr, range);
+		zpci_err_insn_rpcit(1, 'R', cc, status, req, addr, range);
 
 	if (cc == 1 && (status == 4 || status == 16))
 		return -ENOMEM;

-- 
2.53.0

Re: [PATCH v2 3/3] s390/pci: Add function handle to RPCIT insn trace

[Matthew Rosato]

On 10/12/26 10:21 AM, Gerd Bayer wrote:
> In certain debug situations it would be helpful, if the insn trace for a
> failing RPCIT would reveal the PCI function on which this was attempted.
> 
> Introduce a variant of zpci_err_insn_req() called zpci_err_insn_rpcit()
> that accepts an IO virtual address and a range parameter together with
> the req conveying the function handle. Switch zpci_refresh_trans() to
> use that new error trace.
> 
> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

Re: [PATCH v2 0/3] s390/pci: Updates to PCI insn tracing

[Heiko Carstens]

On Mon, Oct 12, 2026 at 04:21:48PM +0200, Gerd Bayer wrote:
> With this series applied (+ a test patch to trace RPCIT even in CC0
> situations) an entry in /sys/kernel/debug/s390dbf/pci_error/hex_ascii
> looks like the following (removed time-stamp and ASCII transcription,
> replaced instruction address with "<IA>"):

...

> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> ---
> Changes in v2:
> - 1/3 and 3/3: Replace unnamed bitfields with named dummy members for
>   padding (address Sashiko findings)
> - Link to v1: https://lore.kernel.org/r/20261007-rpcit_trcfh_upstream-v1-0-8a2718cfb90b@linux.ibm.com
> 
> ---
> Gerd Bayer (3):
>       s390/pci: Adjust alignment in insn trace
>       s390/pci: Use 32-bit fh outside of inline asm
>       s390/pci: Add function handle to RPCIT insn trace
> 
>  arch/s390/include/asm/pci_insn.h |  2 +-
>  arch/s390/pci/pci_insn.c         | 42 +++++++++++++++++++++++++++++++++-------
>  drivers/iommu/s390-iommu.c       |  8 +++-----
>  3 files changed, 39 insertions(+), 13 deletions(-)

Even though this touches drivers/iommu/s390-iommu.c I assume this
should go upstream via s390. Is this correct?

Re: [PATCH v2 0/3] s390/pci: Updates to PCI insn tracing

[Gerd Bayer]

On Mon, 2026-10-12 at 18:00 +0200, Heiko Carstens wrote:
> On Mon, Oct 12, 2026 at 04:21:48PM +0200, Gerd Bayer wrote:
> > With this series applied (+ a test patch to trace RPCIT even in CC0
> > situations) an entry in /sys/kernel/debug/s390dbf/pci_error/hex_ascii
> > looks like the following (removed time-stamp and ASCII transcription,
> > replaced instruction address with "<IA>"):
> 
> ...
> 
> > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> > ---
> > Changes in v2:
> > - 1/3 and 3/3: Replace unnamed bitfields with named dummy members for
> >   padding (address Sashiko findings)
> > - Link to v1: https://lore.kernel.org/r/20261007-rpcit_trcfh_upstream-v1-0-8a2718cfb90b@linux.ibm.com
> > 
> > ---
> > Gerd Bayer (3):
> >       s390/pci: Adjust alignment in insn trace
> >       s390/pci: Use 32-bit fh outside of inline asm
> >       s390/pci: Add function handle to RPCIT insn trace
> > 
> >  arch/s390/include/asm/pci_insn.h |  2 +-
> >  arch/s390/pci/pci_insn.c         | 42 +++++++++++++++++++++++++++++++++-------
> >  drivers/iommu/s390-iommu.c       |  8 +++-----
> >  3 files changed, 39 insertions(+), 13 deletions(-)
> 
> Even though this touches drivers/iommu/s390-iommu.c I assume this
> should go upstream via s390. Is this correct?

Hi Heiko,

that was my assumption, yes. I hope the iommu-maintainers agree, since
patch 2 touches s390 internals, exclusively.

Thanks,
Gerd