Re: [PATCH V4] Fix pointer cast for 32 bits arch

[Alan Cox <alan@linux.intel.com>]

On Thu, 2015-04-16 at 20:01 +0300, Dan Carpenter wrote:
> On Thu, Apr 16, 2015 at 06:14:55PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Apr 16, 2015 at 3:39 PM, Peter Senna Tschudin
> > <peter.senna@gmail.com> wrote:
> > > --- a/drivers/staging/goldfish/goldfish_audio.c
> > > +++ b/drivers/staging/goldfish/goldfish_audio.c
> > > @@ -63,7 +63,7 @@ struct goldfish_audio {
> > >  #define AUDIO_READ(data, addr)         (readl(data->reg_base + addr))
> > >  #define AUDIO_WRITE(data, addr, x)     (writel(x, data->reg_base + addr))
> > >  #define AUDIO_WRITE64(data, addr, addr2, x)    \
> > > -       (gf_write64((u64)(x), data->reg_base + addr, data->reg_base+addr2))
> > > +       (gf_write_ptr((void *)(x), data->reg_base + addr, data->reg_base+addr2))
> > 
> > This one should not be converted, as all callers pass a dma_addr_t, which may
> > be 64-bit on 32-bit systems, i.e. larger than void *.
> 
> Ugh...  You're right.
> 
> I've been avoiding asking this but I can't any longer.  What is
> gf_write64() actually doing?  We are writing dma addresses, user space
> pointers and kernel space pointers to this hardware?
> 
> This stuff doesn't seem to make any kind of sense and I can easily
> imagine a situation where it wrote a 64 bit pointer.  Then we partially
> write over it with a 32 bit userspace pointer.  Then it writes somewhere
> totally unintended.
> 
> This thing doesn't make any sort of sense to me.

Its a 64 on 64 or 32 on 32 virtual machine. Goldfish is used for Android
emulation for all the system level phone emulation tools. On the
emulation side it provides an interface for the emulated OS but makes no
effort to emulate it as if it was a real hardware. If you think of it as
a funky emulator interface all is good. If you think about it as
"hardware" you've got the wrong model and chunks of Goldfish make less
sense.

Alan