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

[Gerd Bayer <gbayer@linux.ibm.com>]

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