Re: [PATCH v2] Add io space MMAP for memory mapping devices and files

[Andreas Klinger]

Hi Jose,

"Jose E. Marchesi" <jemarch@gnu.org> schrieb am Sa, 06. Jan 19:29:
> 
> > Hi Mohammad,
> >
> > Mohammad-Reza Nabipoor <mnabipoor@gnu.org> schrieb am Sa, 06. Jan 16:23:
> >> On Sat, Jan 06, 2024 at 03:40:15PM +0100, Mohammad-Reza Nabipoor wrote:
> >> > 
> >> > > +  if (align > 8)
> >> > > +    align = 8;
> >> > > +
> >> > > +  if (align > 1 && offset % align)
> >> > 
> >> > 
> >> > Why not returning IOD_EIOFF for "Invalid offset"? Or some other error?
> >> > If user is asking for an invalid range of memory, we have to return
> >> > an error, rather than trying to "fix" the problem.  It prevents mistakes.
> >> > 
> >> > Or we can handle the alignment like how some processors handle?
> >> >   For N byte read, you have to be aligned on N-byte boundaries.
> >
> > This was also an idea I already implemented. It turned out that when using a
> > pickle there are reads on unaligned adresses. This happened in my case with 
> > jpeg
> > markers. To give a simplified example:
> >
> > Pickle:
> > type MY_Tag = 
> >   struct
> >   {
> >     byte b;
> >     uint16 u;
> >   };
> >
> > poke:
> > (poke) var m = MY_Tag @ 0x0#B
> >
> > Will run into this error when trying to read 2 bytes at offset 1 which in 
> > turn
> > means someone is not able to use this struct in the pickle. Because of this
> > simply returning an error is not an option for my use case and I think 
> > there are
> > more use cases like this.
> 
> Can't the mmap IOD just do the right thing internally?  Reading byte by
> byte, for example, or whatever.  I would expect the IOD shall be able to
> handle any mappable Poke structure.

Reading byte by byte means that when reading 8 byte from a 64 bit machine the
same 8 byte address (or io memory) is read 8 times in the same way where every
time another one byte out of 8 bytes are returned. This seems to be a waste of
resources to me.

With the patch v2 while reading we are rounding down to 8 byte alignment and
return the requested bytes whatever alignment they have.

This is exactly such an approach which is working for all systems and the user
of poke don't need to take care about alignment issues but on the other side
when reading for example 8 aligned bytes or larger amounts of memory the full
performance of the architecture is used.

I think that the approach of patch v2 could be optimized by a .set alignment
variable for those guys who are aware of such hardware constraints on their
architecture and want to optimize this. But I would set the default to a value
which works for all, which is the size of the address bus.

Andreas