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

[Jose E. Marchesi]

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

>
>> Maybe we can relax alignment requirement using a flag.
>
> The truth is that this is stronly dependent on the hardware architecture. We
> run hardly into these kind of issues on x86 but we have to struggle with it on
> arm every now and then. When someone is only accessing files and no memory
> mapped io then this is also not an issue. So it depends on architecture 
> features
> and use case.
>
> In this v2 patch I tried to implement a one fits all solution by setting the
> alignment to 8 bytes. But this is of course not the optimum for all cases.
>
> An option could be to introduce a new .set variable to set the requested
> alignment. Then the user can decide if alignment matters and when yes which 
> one
> he or she wants to use. For example:
>
> On arm 32 bit systems:
> .set alignment 4
>
> on arm 64 bit:
> .set alignment 8
>
> when alignment is no issue:
> .set alignment 1
>
> Default value could be sizeof(void *).
>
> Comments?
>
> Andreas