| Message ID | 20210915203021.1454bd968b28.I160cb938058758bba3cb1968419314dbeb9dc004@changeid |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/3] um: virt-pci: fix 32-bit compile | expand |
Hi Johannes, On Wed, Sep 15, 2021 at 8:30 PM Johannes Berg <johannes@sipsolutions.net> wrote: > From: Johannes Berg <johannes.berg@intel.com> > > There were a few 32-bit compile warnings that of course > turned into errors with -Werror, fix the 32-bit build. > > Fixes: 68f5d3f3b654 ("um: add PCI over virtio emulation driver") > Reported-by: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: Johannes Berg <johannes.berg@intel.com> Thanks for your patch! > --- a/arch/um/drivers/virt-pci.c > +++ b/arch/um/drivers/virt-pci.c > @@ -181,15 +181,15 @@ static unsigned long um_pci_cfgspace_read(void *priv, unsigned int offset, > /* buf->data is maximum size - we may only use parts of it */ > struct um_pci_message_buffer *buf; > u8 *data; > - unsigned long ret = ~0ULL; > + unsigned long ret = ULONG_MAX; Why not just drop the last "L"? ;-) > > if (!dev) > - return ~0ULL; > + return ULONG_MAX; > > buf = get_cpu_var(um_pci_msg_bufs); > data = buf->data; > > - memset(data, 0xff, sizeof(data)); > + memset(buf->data, 0xff, sizeof(buf->data)); The first change is not needed. The second change is a genuine bug fix, also on 64-bit, which should be submitted separately, and backported to stable. Gr{oetje,eeting}s, Geert
On Wed, 2021-09-15 at 20:42 +0200, Geert Uytterhoeven wrote: > > > - unsigned long ret = ~0ULL; > > + unsigned long ret = ULONG_MAX; > > Why not just drop the last "L"? ;-) > > > > > if (!dev) > > - return ~0ULL; > > + return ULONG_MAX; > > The first change is not needed. True, but it seemed inconsistent without that. > > - memset(data, 0xff, sizeof(data)); > > + memset(buf->data, 0xff, sizeof(buf->data)); > The second change is a genuine bug fix, also on 64-bit, which should be > submitted separately, and backported to stable. It's kind of a bug, I agree. However: On 64-bit platforms, the pointer size is 8 bytes, and thus all the data is memset to 0xff as needed. On 32-bit platforms, we cannot do 64-bit reads (there's no ioread64 on 32-bit), and thus we can only ever need four 0xff bytes (for a failing 32-bit read), matching the pointer size, so it ends up being correct as well. johannes
diff --git a/arch/um/drivers/virt-pci.c b/arch/um/drivers/virt-pci.c index c08066633023..0ab58016db22 100644 --- a/arch/um/drivers/virt-pci.c +++ b/arch/um/drivers/virt-pci.c @@ -181,15 +181,15 @@ static unsigned long um_pci_cfgspace_read(void *priv, unsigned int offset, /* buf->data is maximum size - we may only use parts of it */ struct um_pci_message_buffer *buf; u8 *data; - unsigned long ret = ~0ULL; + unsigned long ret = ULONG_MAX; if (!dev) - return ~0ULL; + return ULONG_MAX; buf = get_cpu_var(um_pci_msg_bufs); data = buf->data; - memset(data, 0xff, sizeof(data)); + memset(buf->data, 0xff, sizeof(buf->data)); switch (size) { case 1: @@ -304,7 +304,7 @@ static unsigned long um_pci_bar_read(void *priv, unsigned int offset, /* buf->data is maximum size - we may only use parts of it */ struct um_pci_message_buffer *buf; u8 *data; - unsigned long ret = ~0ULL; + unsigned long ret = ULONG_MAX; buf = get_cpu_var(um_pci_msg_bufs); data = buf->data;