On 30/04/2025 20.32, Farhan Ali wrote:
On 4/30/2025 10:52 AM, Thomas Huth wrote:
On 30/04/2025 18.47, Farhan Ali wrote:
..snip...
+static inline uint32_t host_pci_ldl_le_p(const void *ioaddr)
+{
+ uint32_t ret = 0;
+#ifdef __s390x__
+ ret = le32_to_cpu(s390x_pci_mmio_read_32(ioaddr));
+#else
+ ret = (uint32_t)ldl_le_p(ioaddr);
This is the only spot where you used a cast. Is it necessary, or could
it be omitted?
Yes, the ldl_le_p returns an int. We do similar cast here https://
github.com/qemu/qemu/blob/73d29ea2417b58ca55fba1aa468ba38e3607b583/
include/ qemu/bswap.h#L416
... but that function there returns an 64-bit value, while you are
assigning the value to a 32-bit variable here, and you also only return a
32-bit value from the function here. So there is no way that this could
accidentally be sign-extended, could it?
Thomas
I checked the Linux kernel and there too we seem to be returning unsigned 32
bit values (readl) for 32 bit mmio reads. So I think it should be safe. Do
you think we should remove the cast?
It looks redundant to me, so I'd vote to remove it, yes!
Thomas