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

[Jose E. Marchesi]

> 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.

Ok.  Sounds good, thanks.

> 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.

I think it is perfectly ok to have IOD specific configuration
parameters, sure.

Let's make it a Poke variable that the user can customize, plus a
setting in the registry so the .set dot-command works on it.  I suppose
it shall have the name mmap in it, like pk_mmap_alignment.  WDYT?