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

[PATCH 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>
---
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         | 43 +++++++++++++++++++++++++++++++++-------
 drivers/iommu/s390-iommu.c       |  8 +++-----
 3 files changed, 40 insertions(+), 13 deletions(-)
---
base-commit: 602042bf29f6efde39cfb5fdd9289bf4854bc0c5
change-id: 20261007-rpcit_trcfh_upstream-3e6fa73dff1b

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

[PATCH 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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
index 35ceb1bea1c6..2ba787e2304b 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -18,11 +18,14 @@
 #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: 8;
+	u32: 32;
 	union {
 		struct {
 			u64 req;
@@ -33,7 +36,10 @@ struct zpci_err_insn_data {
 			u64 len;
 		};
 	};
-} __packed;
+	u64: 64;
+};
+
+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 1/3] s390/pci: Adjust alignment in insn trace

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] s390/pci: info leak of kernel stack memory in trace buffer due to unnamed bit-fields
--

commit 9eddce7825c1b38a4a78855744726217e301528b
Author: Gerd Bayer <gbayer@linux.ibm.com>

s390/pci: Adjust alignment in insn trace

This patch adjusts the alignment of fields in the pci_error instruction
trace. It drops the packed attribute on struct zpci_err_insn_data and
adds unnamed bit-fields to explicitly pad the structure to 32 bytes for
easier human readability in hex dumps.

> diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> index 35ceb1bea1c61..2ba787e2304b5 100644
> --- a/arch/s390/pci/pci_insn.c
> +++ b/arch/s390/pci/pci_insn.c
> @@ -18,11 +18,14 @@
>  #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: 8;
> +	u32: 32;
>  	union {
>  		struct {
>  			u64 req;
> @@ -33,7 +36,10 @@ struct zpci_err_insn_data {
>  			u64 len;
>  		};
>  	};
> -} __packed;
> +	u64: 64;
> +};
> +

[Severity: Medium]
Does this change unintentionally leak kernel stack memory into the s390dbf
debug trace ring buffer?

By using unnamed bit-fields for padding, these fields are explicitly
excluded from zero-initialization when using a designated initializer.

When zpci_err_insn_req() in arch/s390/pci/pci_insn.c initializes this
structure on the stack without an explicit memset(), the unnamed padding
bytes retain their uninitialized stack memory:

static inline void zpci_err_insn_req(...)
{
        struct zpci_err_insn_data data = {
                .insn = insn, .cc = cc, .status = status,
                .req = req, .offset = offset};

        zpci_err_hex_level(lvl, &data, sizeof(data));
}

The subsequent call to zpci_err_hex_level() will copy the entire 32-byte
structure to the trace buffer, exposing 13 bytes of uninitialized stack
data.

Will this also defeat the goal of having clean traces for humans, since
the hex dumps will contain random stack data instead of zeroes?

> +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)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-rpcit_trcfh_upstream-v1-0-8a2718cfb90b@linux.ibm.com?part=1

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

[Gerd Bayer]

On Wed, 2026-10-07 at 11:14 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] s390/pci: info leak of kernel stack memory in trace buffer due to unnamed bit-fields
> --
> 
> commit 9eddce7825c1b38a4a78855744726217e301528b
> Author: Gerd Bayer <gbayer@linux.ibm.com>
> 
> s390/pci: Adjust alignment in insn trace
> 
> This patch adjusts the alignment of fields in the pci_error instruction
> trace. It drops the packed attribute on struct zpci_err_insn_data and
> adds unnamed bit-fields to explicitly pad the structure to 32 bytes for
> easier human readability in hex dumps.
> 
> > diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> > index 35ceb1bea1c61..2ba787e2304b5 100644
> > --- a/arch/s390/pci/pci_insn.c
> > +++ b/arch/s390/pci/pci_insn.c
> > @@ -18,11 +18,14 @@
> >  #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: 8;
> > +	u32: 32;
> >  	union {
> >  		struct {
> >  			u64 req;
> > @@ -33,7 +36,10 @@ struct zpci_err_insn_data {
> >  			u64 len;
> >  		};
> >  	};
> > -} __packed;
> > +	u64: 64;
> > +};
> > +
> 
> [Severity: Medium]
> Does this change unintentionally leak kernel stack memory into the s390dbf
> debug trace ring buffer?
> 
> By using unnamed bit-fields for padding, these fields are explicitly
> excluded from zero-initialization when using a designated initializer.
> 
> When zpci_err_insn_req() in arch/s390/pci/pci_insn.c initializes this
> structure on the stack without an explicit memset(), the unnamed padding
> bytes retain their uninitialized stack memory:
> 
> static inline void zpci_err_insn_req(...)
> {
>         struct zpci_err_insn_data data = {
>                 .insn = insn, .cc = cc, .status = status,
>                 .req = req, .offset = offset};
> 
>         zpci_err_hex_level(lvl, &data, sizeof(data));
> }
> 
> The subsequent call to zpci_err_hex_level() will copy the entire 32-byte
> structure to the trace buffer, exposing 13 bytes of uninitialized stack
> data.
> 
> Will this also defeat the goal of having clean traces for humans, since
> the hex dumps will contain random stack data instead of zeroes?


Alright, according to some research [1][2] unnamed bit-fields are not
guaranteed to be zero-initialized. I'll provide dummy-names for the
padding fields in the next iteration.

> 
> > +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)

[1] - https://stackoverflow.com/questions/56216243/how-do-bit-fields-interplay-with-bits-padding-in-c
[2] - https://news.ycombinator.com/item?id=43792948

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

[Niklas Schnelle]

On Wed, 2026-10-07 at 15:10 +0200, Gerd Bayer wrote:
> On Wed, 2026-10-07 at 11:14 +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] s390/pci: info leak of kernel stack memory in trace buffer due to unnamed bit-fields
> > --
> > 
> > commit 9eddce7825c1b38a4a78855744726217e301528b
> > Author: Gerd Bayer <gbayer@linux.ibm.com>
> > 
--- snip ---
> > 
> > [Severity: Medium]
> > Does this change unintentionally leak kernel stack memory into the s390dbf
> > debug trace ring buffer?
> > 
> > By using unnamed bit-fields for padding, these fields are explicitly
> > excluded from zero-initialization when using a designated initializer.
> > 
> > When zpci_err_insn_req() in arch/s390/pci/pci_insn.c initializes this
> > structure on the stack without an explicit memset(), the unnamed padding
> > bytes retain their uninitialized stack memory:
> > 
> > static inline void zpci_err_insn_req(...)
> > {
> >         struct zpci_err_insn_data data = {
> >                 .insn = insn, .cc = cc, .status = status,
> >                 .req = req, .offset = offset};
> > 
> >         zpci_err_hex_level(lvl, &data, sizeof(data));
> > }
> > 
> > The subsequent call to zpci_err_hex_level() will copy the entire 32-byte
> > structure to the trace buffer, exposing 13 bytes of uninitialized stack
> > data.
> > 
> > Will this also defeat the goal of having clean traces for humans, since
> > the hex dumps will contain random stack data instead of zeroes?
> 
> 
> Alright, according to some research [1][2] unnamed bit-fields are not
> guaranteed to be zero-initialized. I'll provide dummy-names for the
> padding fields in the next iteration.

Agree and to be honest I do like named reserved fields better for
readability too. Other than this issue this all looks good to me and
should definitely make the hex_error easier to read.

Thanks,
Niklas
> > 

[PATCH 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 2ba787e2304b..053afb3e7a99 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -117,13 +117,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 2/3] s390/pci: Use 32-bit fh outside of inline asm

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/pci: infinite loop in zpci_refresh_trans on CC=2
--

commit 1c8706bb963f0d551da7d8535f804ca9f9b4305a
Author: Gerd Bayer <gbayer@linux.ibm.com>

s390/pci: Use 32-bit fh outside of inline asm
    
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.

> diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> index 2ba787e2304b5..053afb3e7a990 100644
> --- a/arch/s390/pci/pci_insn.c
> +++ b/arch/s390/pci/pci_insn.c
> @@ -117,13 +117,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);

[Severity: High]
Does this retry loop risk causing an infinite loop or CPU stall when handling
large DMA spaces?

When the s390x RPCIT instruction sets CC=2, it indicates that it cannot complete
the operation in the allotted time. It then updates the address and range
registers to reflect the remaining uncompleted portion.

However, in __rpcit(), addr_range is passed using an input-only constraint ("d"
instead of "+d"), and the arguments are passed by value:

arch/s390/pci/pci_insn.c:__rpcit() {
	union register_pair addr_range = {.even = addr, .odd = range};
	...
		: [addr_range] "d" (addr_range.pair)
}

Because the hardware's partial progress updates to addr and range are discarded,
this loop continually re-invokes __rpcit() with the unmodified original
addr and range.

If scanning the initial portion consistently exhausts the hardware time slice
(such as during IOMMU TLB flushes on devices with large DMA spaces in
zpci_refresh_all()), will the operation fail to advance and loop infinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-rpcit_trcfh_upstream-v1-0-8a2718cfb90b@linux.ibm.com?part=2

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

[Gerd Bayer]

On Wed, 2026-10-07 at 11:19 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] s390/pci: infinite loop in zpci_refresh_trans on CC=2
> --
> 
> commit 1c8706bb963f0d551da7d8535f804ca9f9b4305a
> Author: Gerd Bayer <gbayer@linux.ibm.com>
> 
> s390/pci: Use 32-bit fh outside of inline asm
>     
> 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.
> 
> > diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> > index 2ba787e2304b5..053afb3e7a990 100644
> > --- a/arch/s390/pci/pci_insn.c
> > +++ b/arch/s390/pci/pci_insn.c
> > @@ -117,13 +117,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);
> 
> [Severity: High]
> Does this retry loop risk causing an infinite loop or CPU stall when handling
> large DMA spaces?
> 
> When the s390x RPCIT instruction sets CC=2, it indicates that it cannot complete
> the operation in the allotted time. It then updates the address and range
> registers to reflect the remaining uncompleted portion.

I've re-read the relevant ISA documentation a couple times now. I don't
find any mention of address and range being modified after RPCIT was
executed - regardless of condition code or status.

Maybe @Niklas or @Matt have an idea where this allegation of "CC=2
means partial completion and one should retry with an updated
iova/range" comes from?

> However, in __rpcit(), addr_range is passed using an input-only constraint ("d"
> instead of "+d"), and the arguments are passed by value:
> 
> arch/s390/pci/pci_insn.c:__rpcit() {
> 	union register_pair addr_range = {.even = addr, .odd = range};
> 	...
> 		: [addr_range] "d" (addr_range.pair)
> }
> 
> Because the hardware's partial progress updates to addr and range are discarded,
> this loop continually re-invokes __rpcit() with the unmodified original
> addr and range.
> 
> If scanning the initial portion consistently exhausts the hardware time slice
> (such as during IOMMU TLB flushes on devices with large DMA spaces in
> zpci_refresh_all()), will the operation fail to advance and loop infinitely?

I see that all the PCI related instructions in this file may retry
indefinitely (with delay) on CC=2. However, the architecture guarantees
that at some point the instruction will end with any of the other
condition codes.

Net: I think this review finding is not valid. Feel free to convince me
otherwise.

Thanks,
Gerd

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

[Matthew Rosato]

On 10/7/26 10:46 AM, Gerd Bayer wrote:
> On Wed, 2026-10-07 at 11:19 +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] s390/pci: infinite loop in zpci_refresh_trans on CC=2
>> --
>>
>> commit 1c8706bb963f0d551da7d8535f804ca9f9b4305a
>> Author: Gerd Bayer <gbayer@linux.ibm.com>
>>
>> s390/pci: Use 32-bit fh outside of inline asm
>>     
>> 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.
>>
>>> diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
>>> index 2ba787e2304b5..053afb3e7a990 100644
>>> --- a/arch/s390/pci/pci_insn.c
>>> +++ b/arch/s390/pci/pci_insn.c
>>> @@ -117,13 +117,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);
>>
>> [Severity: High]
>> Does this retry loop risk causing an infinite loop or CPU stall when handling
>> large DMA spaces?
>>

This should really have been flagged as a pre-existing issue vs a new
regression, it is not a behavior introduced (or changed) by your patch.

>> When the s390x RPCIT instruction sets CC=2, it indicates that it cannot complete
>> the operation in the allotted time. It then updates the address and range
>> registers to reflect the remaining uncompleted portion.
> 
> I've re-read the relevant ISA documentation a couple times now. I don't
> find any mention of address and range being modified after RPCIT was
> executed - regardless of condition code or status.
> 
> Maybe @Niklas or @Matt have an idea where this allegation of "CC=2
> means partial completion and one should retry with an updated
> iova/range" comes from?

FWIW, it's not coming from QEMU -- we don't even have a CC=2 return case
for RPCIT today, and we never update the iova/range on return.

Perhaps, lacking access to the ISA documentation, it made an assumption
that the range would be updated to shorten the next RPCIT as this would
allow us to converge towards a smaller and smaller range each attempt to
potentially not hit the busy condition eventually - sounds logical anyway?

But I also find nothing that indicates the range will/should be updated
on CC=2 (or otherwise) from RPCIT; so it does just sound like a bad
assumption.

> 
>> However, in __rpcit(), addr_range is passed using an input-only constraint ("d"
>> instead of "+d"), and the arguments are passed by value:
>>
>> arch/s390/pci/pci_insn.c:__rpcit() {
>> 	union register_pair addr_range = {.even = addr, .odd = range};
>> 	...
>> 		: [addr_range] "d" (addr_range.pair)
>> }
>>
>> Because the hardware's partial progress updates to addr and range are discarded,
>> this loop continually re-invokes __rpcit() with the unmodified original
>> addr and range.

That matches your observation:  we indeed aren't taking updates to the
range in __rpcit().  But as you say: there aren't supposed to be any.

>>
>> If scanning the initial portion consistently exhausts the hardware time slice
>> (such as during IOMMU TLB flushes on devices with large DMA spaces in
>> zpci_refresh_all()), will the operation fail to advance and loop infinitely?
> 

That concern sounds valid at face value; if we tried something and it
couldn't be completed in time, from a linux perspective we are simply
trying it again without changing anything and hoping everything works
for the best this time.

> I see that all the PCI related instructions in this file may retry
> indefinitely (with delay) on CC=2. However, the architecture guarantees
> that at some point the instruction will end with any of the other
> condition codes.

I think that is the rub.  Sashiko couldn't possibly know that such an
architecture guarantee exists.

I suspect the question is also a bit theoretical: what happens if the
range is so big that it's impossible for the RPCIT to process it before
the CC2 trigger point due simply to how big the range provided is?  Then
you're guaranteed to hit it every time unless you 'chip away' at it by
shrinking the range after each attempt.

I would assume/hope that the SDMA-EDMA range limitation we impose on the
aperture size already ensures that it is easily possible to handle a
RPCIT from SDMA thru EDMA without tripping CC=2.
So then the CC=2 case becomes purely situational vs a guaranteed result
even for the largest possible RPCIT request.  That already makes it far
more reasonable to simply try it again.

So: assuming that architecture guarantee stands (we know RPCIT can
possibly complete for the largest possible range SDMA-EDMA, and we have
some architected guarantee that the loop will be broken otherwise e.g.
architecture says it won't keep giving us CC2s forever) then it sounds
like nothing to fix, but maybe worth a comment in a future patch that
explains these guarantees, based on what you find in the ISA.

I suppose we can consider whether linux should have its own redundancy
here or not in addition to those guarantees.  That sounds well beyond
the scope of this series and goes back to what I mentioned at the start:
 this is pre-existing bevahior and should not hold up this patch either way.

Thanks,
Matt

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

[Gerd Bayer]

On Wed, 2026-10-07 at 12:21 -0400, Matthew Rosato wrote:
> On 10/7/26 10:46 AM, Gerd Bayer wrote:
> > On Wed, 2026-10-07 at 11:19 +0000, sashiko-bot@kernel.org wrote:
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > - [High] s390/pci: infinite loop in zpci_refresh_trans on CC=2
> > > --
> > > 
> > > commit 1c8706bb963f0d551da7d8535f804ca9f9b4305a
> > > Author: Gerd Bayer <gbayer@linux.ibm.com>
> > > 
> > > s390/pci: Use 32-bit fh outside of inline asm
> > >     
> > > 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.
> > > 
> > > > diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> > > > index 2ba787e2304b5..053afb3e7a990 100644
> > > > --- a/arch/s390/pci/pci_insn.c
> > > > +++ b/arch/s390/pci/pci_insn.c
> > > > @@ -117,13 +117,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);
> > > 
> > > [Severity: High]
> > > Does this retry loop risk causing an infinite loop or CPU stall when handling
> > > large DMA spaces?
> > > 
> 
> This should really have been flagged as a pre-existing issue vs a new
> regression, it is not a behavior introduced (or changed) by your patch.

Yes, I agree. This patch didn't touch/remove the capability to pass
back a modified addr or range.

> 
> > > When the s390x RPCIT instruction sets CC=2, it indicates that it cannot complete
> > > the operation in the allotted time. It then updates the address and range
> > > registers to reflect the remaining uncompleted portion.
> > 
> > I've re-read the relevant ISA documentation a couple times now. I don't
> > find any mention of address and range being modified after RPCIT was
> > executed - regardless of condition code or status.
> > 
> > Maybe @Niklas or @Matt have an idea where this allegation of "CC=2
> > means partial completion and one should retry with an updated
> > iova/range" comes from?
> 
> FWIW, it's not coming from QEMU -- we don't even have a CC=2 return case
> for RPCIT today, and we never update the iova/range on return.
> 
> Perhaps, lacking access to the ISA documentation, it made an assumption
> that the range would be updated to shorten the next RPCIT as this would
> allow us to converge towards a smaller and smaller range each attempt to
> potentially not hit the busy condition eventually - sounds logical anyway?
> 
> But I also find nothing that indicates the range will/should be updated
> on CC=2 (or otherwise) from RPCIT; so it does just sound like a bad
> assumption.

Thanks for the confirmation.

> > 
> > > However, in __rpcit(), addr_range is passed using an input-only constraint ("d"
> > > instead of "+d"), and the arguments are passed by value:
> > > 
> > > arch/s390/pci/pci_insn.c:__rpcit() {
> > > 	union register_pair addr_range = {.even = addr, .odd = range};
> > > 	...
> > > 		: [addr_range] "d" (addr_range.pair)
> > > }
> > > 
> > > Because the hardware's partial progress updates to addr and range are discarded,
> > > this loop continually re-invokes __rpcit() with the unmodified original
> > > addr and range.
> 
> That matches your observation:  we indeed aren't taking updates to the
> range in __rpcit().  But as you say: there aren't supposed to be any.
> 
> > > 
> > > If scanning the initial portion consistently exhausts the hardware time slice
> > > (such as during IOMMU TLB flushes on devices with large DMA spaces in
> > > zpci_refresh_all()), will the operation fail to advance and loop infinitely?
> > 
> 
> That concern sounds valid at face value; if we tried something and it
> couldn't be completed in time, from a linux perspective we are simply
> trying it again without changing anything and hoping everything works
> for the best this time.
> 
> > I see that all the PCI related instructions in this file may retry
> > indefinitely (with delay) on CC=2. However, the architecture guarantees
> > that at some point the instruction will end with any of the other
> > condition codes.
> 
> I think that is the rub.  Sashiko couldn't possibly know that such an
> architecture guarantee exists.
> 
> I suspect the question is also a bit theoretical: what happens if the
> range is so big that it's impossible for the RPCIT to process it before
> the CC2 trigger point due simply to how big the range provided is?  Then
> you're guaranteed to hit it every time unless you 'chip away' at it by
> shrinking the range after each attempt.
> 
> I would assume/hope that the SDMA-EDMA range limitation we impose on the
> aperture size already ensures that it is easily possible to handle a
> RPCIT from SDMA thru EDMA without tripping CC=2.
> So then the CC=2 case becomes purely situational vs a guaranteed result
> even for the largest possible RPCIT request.  That already makes it far
> more reasonable to simply try it again.
> 
> So: assuming that architecture guarantee stands (we know RPCIT can
> possibly complete for the largest possible range SDMA-EDMA, and we have
> some architected guarantee that the loop will be broken otherwise e.g.
> architecture says it won't keep giving us CC2s forever) then it sounds
> like nothing to fix, but maybe worth a comment in a future patch that
> explains these guarantees, based on what you find in the ISA.
> 
> I suppose we can consider whether linux should have its own redundancy
> here or not in addition to those guarantees.  That sounds well beyond
> the scope of this series and goes back to what I mentioned at the start:
> this is pre-existing bevahior and should not hold up this patch either way.

I went back to commit cd24834130ac ("s390/pci: base support") from 2012
when RPCIT was introduced: The potentially endless loop on CC=2 was
introduced at the very beginning.

I found explicit words in the ISA specifying that "no other action is
taken" when the instruction ends with CC=2. IIRC, CC=2 was introduced
to signal a program that there are internal inhibitors present that
prohibit to even start the requested "refresh" - or other PCI
instructions. I still have to search for a statement that guarantees at
the architecture level that CC=2 won't be presented forever. When I
find that, I'll think about how to frame that into a future patch.

> 
> Thanks,
> Matt

Thank you,
Gerd

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

[Niklas Schnelle]

On Thu, 2026-10-08 at 10:50 +0200, Gerd Bayer wrote:
> On Wed, 2026-10-07 at 12:21 -0400, Matthew Rosato wrote:
> > On 10/7/26 10:46 AM, Gerd Bayer wrote:
> > > On Wed, 2026-10-07 at 11:19 +0000, sashiko-bot@kernel.org wrote:
> > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > > - [High] s390/pci: infinite loop in zpci_refresh_trans on CC=2
> > > 
--- snip ---
> > > > If scanning the initial portion consistently exhausts the hardware time slice
> > > > (such as during IOMMU TLB flushes on devices with large DMA spaces in
> > > > zpci_refresh_all()), will the operation fail to advance and loop infinitely?
> > > 
> > 
> > That concern sounds valid at face value; if we tried something and it
> > couldn't be completed in time, from a linux perspective we are simply
> > trying it again without changing anything and hoping everything works
> > for the best this time.
> > 
> > > I see that all the PCI related instructions in this file may retry
> > > indefinitely (with delay) on CC=2. However, the architecture guarantees
> > > that at some point the instruction will end with any of the other
> > > condition codes.
> > 
> > I think that is the rub.  Sashiko couldn't possibly know that such an
> > architecture guarantee exists.
> > 
> > I suspect the question is also a bit theoretical: what happens if the
> > range is so big that it's impossible for the RPCIT to process it before
> > the CC2 trigger point due simply to how big the range provided is?  Then
> > you're guaranteed to hit it every time unless you 'chip away' at it by
> > shrinking the range after each attempt.
> > 
> > I would assume/hope that the SDMA-EDMA range limitation we impose on the
> > aperture size already ensures that it is easily possible to handle a
> > RPCIT from SDMA thru EDMA without tripping CC=2.
> > So then the CC=2 case becomes purely situational vs a guaranteed result
> > even for the largest possible RPCIT request.  That already makes it far
> > more reasonable to simply try it again.
> > 
> > So: assuming that architecture guarantee stands (we know RPCIT can
> > possibly complete for the largest possible range SDMA-EDMA, and we have
> > some architected guarantee that the loop will be broken otherwise e.g.
> > architecture says it won't keep giving us CC2s forever) then it sounds
> > like nothing to fix, but maybe worth a comment in a future patch that
> > explains these guarantees, based on what you find in the ISA.
> > 
> > I suppose we can consider whether linux should have its own redundancy
> > here or not in addition to those guarantees.  That sounds well beyond
> > the scope of this series and goes back to what I mentioned at the start:
> > this is pre-existing bevahior and should not hold up this patch either way.
> 
> I went back to commit cd24834130ac ("s390/pci: base support") from 2012
> when RPCIT was introduced: The potentially endless loop on CC=2 was
> introduced at the very beginning.
> 
> I found explicit words in the ISA specifying that "no other action is
> taken" when the instruction ends with CC=2. IIRC, CC=2 was introduced
> to signal a program that there are internal inhibitors present that
> prohibit to even start the requested "refresh" - or other PCI
> instructions. I still have to search for a statement that guarantees at
> the architecture level that CC=2 won't be presented forever. When I
> find that, I'll think about how to frame that into a future patch.
> > 

This looks like a very clear case of LLM hallucination to me too. It's
not that surprising either that the model would make something up when
it doesn't have access to a good reference and what it made up does
make some sense. Either way, you're right for the cc 2 "busy" returns
the platform must ensure that at some point it either succeeds or gives
us a different error cc. As you correctly state this assumption has
been there from the beginning and maybe we need to ask for a more
formal guarantee statement but in the end a "busy" also just doesn't
make sense otherwise no one can be busy forever ;)

Thanks,
Niklas

[PATCH 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 053afb3e7a99..ae9ed4aa2ec9 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -29,18 +29,40 @@ struct zpci_err_insn_data {
 	union {
 		struct {
 			u64 req;
-			u64 offset;
+			union {
+				/* PCILG/PCISTG/MPCIFC */
+				struct {
+					u64 offset;
+					u64: 64;
+				};
+				/* RPCIT */
+				struct {
+					u64 iova;
+					u64 range;
+				};
+			};
 		};
+		/* PCILGI/PCISTGI */
 		struct {
 			u64 addr;
 			u64 len;
+			u64: 64;
 		};
 	};
-	u64: 64;
 };
 
 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)
 {
@@ -128,16 +150,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 3/3] s390/pci: Add function handle to RPCIT insn trace

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] s390/pci: info leak of uninitialized struct padding in zpci_err_insn_rpcit
--

commit 9deafec98847a2a1de20826fbaa6431e1e3c973f
Author: Gerd Bayer <gbayer@linux.ibm.com>

s390/pci: Add function handle to RPCIT insn trace

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.

> diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> index 053afb3e7a990..ae9ed4aa2ec9c 100644
> --- a/arch/s390/pci/pci_insn.c
> +++ b/arch/s390/pci/pci_insn.c

[ ... ]

> @@ -29,18 +29,40 @@ struct zpci_err_insn_data {
>  	union {
>  		struct {
>  			u64 req;
> -			u64 offset;
> +			union {
> +				/* PCILG/PCISTG/MPCIFC */
> +				struct {
> +					u64 offset;
> +					u64: 64;
> +				};
> +				/* RPCIT */
> +				struct {
> +					u64 iova;
> +					u64 range;
> +				};
> +			};
>  		};
> +		/* PCILGI/PCISTGI */
>  		struct {
>  			u64 addr;
>  			u64 len;
> +			u64: 64;
>  		};
>  	};
> -	u64: 64;
>  };
>  
>  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));
> +}

[Severity: Medium]
Does this code leak uninitialized stack memory to the s390dbf debug trace
buffer?

According to the C standard, unnamed bit-fields do not participate in
designated initialization. The struct zpci_err_insn_data starts with:

struct zpci_err_insn_data {
	u8 insn;
	u8 cc;
	u8 status;
	u8: 8;
	u32: 32;
    ...

Because the unnamed bit-fields are skipped by the designated initializer here,
up to 5 bytes of uninitialized kernel stack memory are left in the padding
of "data".

Since sizeof(data) (which is 32 bytes) is then passed to zpci_err_hex_level(),
could these indeterminate stack bytes be copied into the trace buffer on every
invocation of zpci_err_insn_rpcit()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-rpcit_trcfh_upstream-v1-0-8a2718cfb90b@linux.ibm.com?part=3

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

[Gerd Bayer]

On Wed, 2026-10-07 at 11:17 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] s390/pci: info leak of uninitialized struct padding in zpci_err_insn_rpcit
> --
> 
> commit 9deafec98847a2a1de20826fbaa6431e1e3c973f
> Author: Gerd Bayer <gbayer@linux.ibm.com>
> 
> s390/pci: Add function handle to RPCIT insn trace
> 
> 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.
> 
> > diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> > index 053afb3e7a990..ae9ed4aa2ec9c 100644
> > --- a/arch/s390/pci/pci_insn.c
> > +++ b/arch/s390/pci/pci_insn.c
> 
> [ ... ]
> 
> > @@ -29,18 +29,40 @@ struct zpci_err_insn_data {
> >  	union {
> >  		struct {
> >  			u64 req;
> > -			u64 offset;
> > +			union {
> > +				/* PCILG/PCISTG/MPCIFC */
> > +				struct {
> > +					u64 offset;
> > +					u64: 64;
> > +				};
> > +				/* RPCIT */
> > +				struct {
> > +					u64 iova;
> > +					u64 range;
> > +				};
> > +			};
> >  		};
> > +		/* PCILGI/PCISTGI */
> >  		struct {
> >  			u64 addr;
> >  			u64 len;
> > +			u64: 64;
> >  		};
> >  	};
> > -	u64: 64;
> >  };
> >  
> >  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));
> > +}
> 
> [Severity: Medium]
> Does this code leak uninitialized stack memory to the s390dbf debug trace
> buffer?
> 
> According to the C standard, unnamed bit-fields do not participate in
> designated initialization. The struct zpci_err_insn_data starts with:
> 
> struct zpci_err_insn_data {
> 	u8 insn;
> 	u8 cc;
> 	u8 status;
> 	u8: 8;
> 	u32: 32;
>     ...
> 
> Because the unnamed bit-fields are skipped by the designated initializer here,
> up to 5 bytes of uninitialized kernel stack memory are left in the padding
> of "data".
> 
> Since sizeof(data) (which is 32 bytes) is then passed to zpci_err_hex_level(),
> could these indeterminate stack bytes be copied into the trace buffer on every
> invocation of zpci_err_insn_rpcit()?

This is basically the same concern as with patch 1/3 - I'll replace the
unnamed bit-fields with dummy members of appropriate size in a v2.

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

[Niklas Schnelle]

On Wed, 2026-10-07 at 15:12 +0200, Gerd Bayer wrote:
> On Wed, 2026-10-07 at 11:17 +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] s390/pci: info leak of uninitialized struct padding in zpci_err_insn_rpcit
> > --
> > 
> > commit 9deafec98847a2a1de20826fbaa6431e1e3c973f
> > Author: Gerd Bayer <gbayer@linux.ibm.com>
> > 
> > s390/pci: Add function handle to RPCIT insn trace
> > 
> > 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.
> > 
> > > diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> > > index 053afb3e7a990..ae9ed4aa2ec9c 100644
> > > --- a/arch/s390/pci/pci_insn.c
> > > +++ b/arch/s390/pci/pci_insn.c
--- snip ---
> > 
> > [Severity: Medium]
> > Does this code leak uninitialized stack memory to the s390dbf debug trace
> > buffer?
> > 
> > According to the C standard, unnamed bit-fields do not participate in
> > designated initialization. The struct zpci_err_insn_data starts with:
> > 
> > struct zpci_err_insn_data {
> > 	u8 insn;
> > 	u8 cc;
> > 	u8 status;
> > 	u8: 8;
> > 	u32: 32;
> >     ...
> > 
> > Because the unnamed bit-fields are skipped by the designated initializer here,
> > up to 5 bytes of uninitialized kernel stack memory are left in the padding
> > of "data".
> > 
> > Since sizeof(data) (which is 32 bytes) is then passed to zpci_err_hex_level(),
> > could these indeterminate stack bytes be copied into the trace buffer on every
> > invocation of zpci_err_insn_rpcit()?
> 
> This is basically the same concern as with patch 1/3 - I'll replace the
> unnamed bit-fields with dummy members of appropriate size in a v2.

Same as for the other patch this sounds good to me and the rest of the
change looks good too. Having the function handle could come in handy.

Thanks,
Niklas