QEMU PCI test device specifies all registers as little endian. OFFSET
register is converted properly, but the COUNT register is not.

Apply the same conversion to the COUNT register also.

Signed-off-by: Marko Turk <mt@markoturk.info>
Fixes: 685376d18e9a ("samples: rust: add Rust PCI sample driver")
---
 samples/rust/rust_driver_pci.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 55a683c39ed9..3cbb3139fbcf 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -56,7 +56,7 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> {
         // Note that we need `try_write8`, since `offset` can't be checked at compile-time.
         bar.try_write8(data, offset)?;
 
-        Ok(bar.read32(Regs::COUNT))
+        Ok(u32::from_le(bar.read32(Regs::COUNT)))
     }
 }
 
-- 
2.51.0

On Sat, Nov 01, 2025 at 10:46:54PM +0100, Marko Turk wrote:
> QEMU PCI test device specifies all registers as little endian. OFFSET
> register is converted properly, but the COUNT register is not.
> 
> Apply the same conversion to the COUNT register also.
> 
> Signed-off-by: Marko Turk <mt@markoturk.info>
> Fixes: 685376d18e9a ("samples: rust: add Rust PCI sample driver")

Hi,

Can someone take a look?

Thanks.

Marko

On Wed, Nov 12, 2025 at 9:47 AM Marko Turk <mt@markoturk.info> wrote:
>
> On Sat, Nov 01, 2025 at 10:46:54PM +0100, Marko Turk wrote:
> > QEMU PCI test device specifies all registers as little endian. OFFSET
> > register is converted properly, but the COUNT register is not.
> >
> > Apply the same conversion to the COUNT register also.
> >
> > Signed-off-by: Marko Turk <mt@markoturk.info>
> > Fixes: 685376d18e9a ("samples: rust: add Rust PCI sample driver")
>
> Can someone take a look?

Your message was in my spam folder -- that may be affecting who saw it.

From https://www.qemu.org/docs/master/specs/pci-testdev.html:

    "All registers are little endian."

So this seems right. A couple tags:

Cc: stable@vger.kernel.org
Link: https://www.qemu.org/docs/master/specs/pci-testdev.html

Cc'ing Dirk, since he tested the sample originally.

Thanks!

Cheers,
Miguel

On 12/11/2025 10:37, Miguel Ojeda wrote:
> On Wed, Nov 12, 2025 at 9:47 AM Marko Turk <mt@markoturk.info> wrote:
>>
>> On Sat, Nov 01, 2025 at 10:46:54PM +0100, Marko Turk wrote:
>>> QEMU PCI test device specifies all registers as little endian. OFFSET
>>> register is converted properly, but the COUNT register is not.
>>>
>>> Apply the same conversion to the COUNT register also.
>>>
>>> Signed-off-by: Marko Turk <mt@markoturk.info>
>>> Fixes: 685376d18e9a ("samples: rust: add Rust PCI sample driver")
>>
>> Can someone take a look?
> 
> Your message was in my spam folder -- that may be affecting who saw it.
> 
>>From https://www.qemu.org/docs/master/specs/pci-testdev.html:
> 
>      "All registers are little endian."
> 
> So this seems right. A couple tags:
> 
> Cc: stable@vger.kernel.org
> Link: https://www.qemu.org/docs/master/specs/pci-testdev.html
> 
> Cc'ing Dirk, since he tested the sample originally.


Hmm, I can't find the initial patch in my Inbox. Even

https://lore.kernel.org/rust-for-linux/aRRJPZVkCv2i7kt2@vps.markoturk.info/

doesn't seem to have it?

Thanks

Dirk

On Wed, Nov 12, 2025 at 11:17 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Hmm, I can't find the initial patch in my Inbox. Even
>
> https://lore.kernel.org/rust-for-linux/aRRJPZVkCv2i7kt2@vps.markoturk.info/
>
> doesn't seem to have it?

It was only sent to the linux-pci list -- for situations like this,
you can click in the "[not found]" message at the bottom, and then on

    Message-ID: <20251101214629.10718-1-mt@markoturk.info>
    found in another inbox:

    ../../linux-pci/20251101214629.10718-1-mt@markoturk.info/

where that last line is a link to the other list that you can click to find it.

I hope that helps!

Cheers,
Miguel

On 12/11/2025 11:24, Miguel Ojeda wrote:
> On Wed, Nov 12, 2025 at 11:17 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>
>> Hmm, I can't find the initial patch in my Inbox. Even
>>
>> https://lore.kernel.org/rust-for-linux/aRRJPZVkCv2i7kt2@vps.markoturk.info/
>>
>> doesn't seem to have it?
> 
> It was only sent to the linux-pci list -- for situations like this,
> you can click in the "[not found]" message at the bottom, and then on
> 
>      Message-ID: <20251101214629.10718-1-mt@markoturk.info>
>      found in another inbox:
> 
>      ../../linux-pci/20251101214629.10718-1-mt@markoturk.info/
> 
> where that last line is a link to the other list that you can click to find it.
> 
> I hope that helps!


Yes, thanks, that helps a lot!

And yes, the change itself looks good. I gave this a try with x86 QEMU 
and as expected (as mentioned it is a no-op on x86) I get with and 
without this patch

rust_driver_pci 0000:00:04.0: pci-testdev data-match count: 1

With this:

Tested-by: Dirk Behme <dirk.behme@de.bosch.com>

Thanks,

Dirk

On Wed, Nov 12, 2025 at 11:16:40AM +0100, Dirk Behme wrote:
> On 12/11/2025 10:37, Miguel Ojeda wrote:
> > On Wed, Nov 12, 2025 at 9:47 AM Marko Turk <mt@markoturk.info> wrote:
> > > 
> > > On Sat, Nov 01, 2025 at 10:46:54PM +0100, Marko Turk wrote:
> > > > QEMU PCI test device specifies all registers as little endian. OFFSET
> > > > register is converted properly, but the COUNT register is not.
> > > > 
> > > > Apply the same conversion to the COUNT register also.
> > > > 
> > > > Signed-off-by: Marko Turk <mt@markoturk.info>
> > > > Fixes: 685376d18e9a ("samples: rust: add Rust PCI sample driver")
> > > 
> > > Can someone take a look?
> > 
> > Your message was in my spam folder -- that may be affecting who saw it.
> > 
> > > From https://www.qemu.org/docs/master/specs/pci-testdev.html:
> > 
> >      "All registers are little endian."
> > 
> > So this seems right. A couple tags:
> > 
> > Cc: stable@vger.kernel.org
> > Link: https://www.qemu.org/docs/master/specs/pci-testdev.html
> > 
> > Cc'ing Dirk, since he tested the sample originally.
> 
> 
> Hmm, I can't find the initial patch in my Inbox. Even
> 
> https://lore.kernel.org/rust-for-linux/aRRJPZVkCv2i7kt2@vps.markoturk.info/
> 
> doesn't seem to have it?

Initially I didn't send to rust-for-linux mailing list. It's here on lkml:
https://lore.kernel.org/lkml/20251101214629.10718-1-mt@markoturk.info/

Marko

On Wed, Nov 12, 2025 at 10:37 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Cc'ing Dirk, since he tested the sample originally.

i.e. a new Tested-by is appreciated if you have the time since you
tested with QEMU originally, even if this is a no-op in x86 (where I
think you tested).

I guess we could potentially consider something like returning a
wrapper to force us to explicitly pick either LE or BE to prevent
things like this.

Cheers,
Miguel

On Wed Nov 12, 2025 at 8:56 PM AEDT, Miguel Ojeda wrote:
> I guess we could potentially consider something like returning a
> wrapper to force us to explicitly pick either LE or BE to prevent
> things like this.

In general, the I/O backend (e.g. MMIO, I2C, etc.) is supposed to take care
of endianness.

In the case of MMIO we (currently) always assume that the device is
little-endian; this is also what the C functions used to implement the MMIO
backend (e.g. readl()) assume, i.e. they always convert from little-endian to
CPU endianness.

I don't think we should do anything else; the cases where we need to deal with
big-endian devices for MMIO should be extremely rare -- rare enough that there
aren't even corresponding *_be() implementations for all architectures.

Given that, I think the proper fix is to drop the existing u32::from_le() call
(that ended up in the sample driver by accident) and properly document the
little-endian assumption of the MMIO backend.

(Currently, this simply is the Io structure, but there is a patch series [1]
from Zhi splitting things up, so we can start supporting arbitrary I/O
backends.)

@Marko: Two separate patches for this would be very welcome. :)

[1] https://lore.kernel.org/all/20251110204119.18351-1-zhiw@nvidia.com/

On Thu Nov 13, 2025 at 7:22 PM AEDT, Danilo Krummrich wrote:
> On Wed Nov 12, 2025 at 8:56 PM AEDT, Miguel Ojeda wrote:
>> I guess we could potentially consider something like returning a
>> wrapper to force us to explicitly pick either LE or BE to prevent
>> things like this.
>
> In general, the I/O backend (e.g. MMIO, I2C, etc.) is supposed to take care
> of endianness.
>
> In the case of MMIO we (currently) always assume that the device is
> little-endian; this is also what the C functions used to implement the MMIO
> backend (e.g. readl()) assume, i.e. they always convert from little-endian to
> CPU endianness.

I.e. if we'd support a big-endian architecture in Rust

	u32::from_le(bar.read32())

would be a bug.

> I don't think we should do anything else; the cases where we need to deal with
> big-endian devices for MMIO should be extremely rare -- rare enough that there
> aren't even corresponding *_be() implementations for all architectures.
>
> Given that, I think the proper fix is to drop the existing u32::from_le() call
> (that ended up in the sample driver by accident) and properly document the
> little-endian assumption of the MMIO backend.
>
> (Currently, this simply is the Io structure, but there is a patch series [1]
> from Zhi splitting things up, so we can start supporting arbitrary I/O
> backends.)
>
> @Marko: Two separate patches for this would be very welcome. :)
>
> [1] https://lore.kernel.org/all/20251110204119.18351-1-zhiw@nvidia.com/

On Thu, Nov 13, 2025 at 9:22 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> I don't think we should do anything else; the cases where we need to deal with
> big-endian devices for MMIO should be extremely rare -- rare enough that there
> aren't even corresponding *_be() implementations for all architectures.
>
> Given that, I think the proper fix is to drop the existing u32::from_le() call
> (that ended up in the sample driver by accident) and properly document the
> little-endian assumption of the MMIO backend.

Sounds good then, thanks!

Cheers,
Miguel