On Mon, 2024-02-19 at 11:08 +0100, Heiko Carstens wrote:
> On Fri, Feb 16, 2024 at 08:48:14PM -0400, Jason Gunthorpe wrote:
> > The signature for __iowrite64_copy() requires the number of 64 bit
> > quantities, not bytes. Multiple by 8 to get to a byte length before
> > invoking zpci_memcpy_toio()
> >
> > Fixes: 87bc359b9822 ("s390/pci: speed up __iowrite64_copy by using pci store block insn")
> > Acked-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > ---
> > arch/s390/pci/pci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > index 676ac74026a82b..52a44e353796c0 100644
> > --- a/arch/s390/pci/pci.c
> > +++ b/arch/s390/pci/pci.c
> > @@ -252,7 +252,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > /* combine single writes by using store-block insn */
> > void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
> > {
> > - zpci_memcpy_toio(to, from, count);
> > + zpci_memcpy_toio(to, from, count * 8);
> > }
>
> Odd, this bug is 11 years old, and there was never any bug report.
> Is this never called, or how is this possible?
>
> Niklas, would you happen to have an idea?
I did ask myself the same question when I first saw this change but
still don't have a satisfactory answer.
As far as I can tell there is only one call that could be relevant for
us in drivers/net/ethernet/mellanox/mlx4/en_tx.c:mlx4_bf_copy(). That
call looks like it would be done during transmit. Not sure under what
exact circumstances or if a short write would directly cause observable
issues though. Note also that contrary to what the mlx4 name suggests
even ConnectX-4 based devices already use the mlx5 driver and that
doesn't call this helper at the moment. Jason has plans to use it there
in the future which is what caused him to even stumble over this
though.
