Memory mapped record object issue

[sidisyom]

Hi,

I am using a memory-mapped record object and have come across an issue around the memory addresses assigned to an object of that record.

Suppose the following record declaration and representation clause:

type ADC_DR_Register is
  record
     DATA     : Hal.UInt16;
     Reserved : Hal.Uint16;
  end record
 with 
   Volatile_Full_Access,
   Bit_Order => System.Low_Order_First,
   Object_Size => 32;
 
for ADC_DR_Register use
  record
     DATA at 0 range 0 .. 15;
     Reserved at 0 range 16 .. 31;
  end record;

which is then memory-mapped to some peripheral address.

An object of that record is then declared (as the composite part of a different record) and subsequently the addresses assigned both to the record object itself and the first item declared in the record are printed:

Addr1 : System.Address := ADC_Registers.ADC_Periph.ADC_DR'Address with Volatile;
Addr2 : System.Address := ADC_Registers.ADC_Periph.ADC_DR.DATA'Address with Volatile;

Printing these in gdb yields:

(gdb) p Addr1 
$1 = (system.address) 0x4001204c
(gdb) p Addr2
$2 = (system.address) 0x20004940

and contrary to what I intuitively believed would be the case, these addresses are different. This is flashed into an stm32f429 board (which is a cortex-M*-based board) so it looks like the object-based address Addr1 is correctly overlaid within the peripheral address space whereas the component-based one Addr2 seems to be in SRAM like any other declared non-memory-mapped object would be. Intuitively, it feels as if there’s some kind of handle or proxy in the middle which is indirectly delegating to one memory address or the other but I’m not too sure?

Not massively important, but for context, this is implementing a manual (i.e. non-ADL) ADC/DMA flow in the aforementioned board and it caused some interesting issues when running the code. Initially, I specified the DATA component as the value for the peripheral address in my DMA configuration as in:

--   Set peripheral address (the data register of the ADC module)
DMA_SxCR.DMA2_Controller.DMA_SPAR := Address_To_Integer (ADC_Registers.ADC_Periph.ADC_DR.DATA'Address);

Which, as demonstrated above, is the wrong address and had a somewhat interesting effect as what happens is that the DMA controller is hitting the SRAM address and not the peripheral one which means that the DATA register is never read which in turn means that the second channel conversion of the first ever scan causes an overrun (and the whole ADC/DMA flow stops and has to be reset).

After several days of intense debugging (that can be hard to spot!) and carefully going through the configuration on both sides again and again (I was definitely sure I’m missing something on that..) I thought of checking the addresses and finally spotted what was going on. Specifying Addr1 as the peripheral address results in the DMA controller reading from the correct address and the conversions/memory stores work just fine with no overrun errors.

I haven’t tried using a single scalar object for the representation of that register yet (which I assume should work?) but was keen to initially understand why this compiler behaviour occurs. Any pointers or references where this is detailed would be greatly appreciated.

(using a gnat_arm_elf = "=15.2.1" cross compiler)

Thanks!

[OneWingedShark]

@sidisyom

Printing these in gdb yields:

(gdb) p Addr1 
$1 = (system.address) 0x4001204c
(gdb) p Addr2
$2 = (system.address) 0x20004940

and contrary to what I intuitively believed would be the case, these addresses are different.

Try adding For ADC_DR_Register'Alignment use 0;, as IIRC, this is an alignment-issue, and (again, IIRC) the 0 is the special case of “no alignment restrictions”.

[sttaft]

This result looks very strange. Can you include the code you used to memory map the object to some peripheral address? The addresses are not just a little different. They are wildly different. That really makes no sense. This isn’t just an alignment issue…

[sidisyom]

Sure, good point, I may have messed something there!

Not sure if you are familiar with the registers structure for the ADC unit for this specific MCU (let me know and I can share a few more details on that) but there’s a number of registers that can be memory-mapped and in my case I am only mapping a subset of these by carefully setting the correct offsets in the representation clause.

So, the declaration of the enclosing record and the representation clauses look like the following:

As you can see there are some gaps in the mapping which the [-gnatw.h] flag is correctly reporting. Most importantly though the ADC_DR offset should be the right one based on what’s mentioned in the RM (which also aligns with the printed value of Addr1 mentioned above):

The ADC1_Base_Address is declared as:

which I believe is the correct address for the ADC1 unit.

So, in my current understanding (which could be flawed/limited), the addresses for the entire record object itself and the first component of that object should match?

Thank you.

[sidisyom]

Specifying that gives the following error:

So something in the word accesses that the compiler doesn’t like..

Thank you.

[sttaft]

@sidisyom

So, in my current understanding (which could be flawed/limited), the addresses for the entire record object itself and the first component of that object should match?

I agree. That is why the result you are getting looks so weird.

It would be ideal if you could provide a self-contained compilable and linkable piece of code including the definitions of Addr1 and Addr2. That would make it easier for someone else to try to duplicate the problem you are seeing.

[sidisyom]

Ok great, let me see if I can push that on some Github repo somehow..! Thank you.

[OneWingedShark]

@sidisyom

So, the declaration of the enclosing record and the representation clauses look like the following:

I find it’s a good idea to align your representation clause:

for ADC_Peripheral use record
   ADC_SR    at 16#00# range 0 .. 31;
   ADC_CR1   at 16#04# range 0 .. 31;
   ADC_CR2   at 16#08# range 0 .. 31;
   ADC_SMPR2 at 16#10# range 0 .. 31;
   ADC_SOR1  at 16#2C# range 0 .. 31;
   ADC_SQR2  at 16#30# range 0 .. 31;
   ADC_SQR3  at 16#34# range 0 .. 31;
   ADC_DR    at 16#4C# range 0 .. 31;
end record;

ADC_periph : aliased ADC_Peripheral 
   with Import, Address => System'To_Address (ADC1_Base_Address),

Another thing you can try (though 256-bits may be too big; IIRC, GNAT has a 128-bit size-limit) is defining everything by bit-position, so XXX at 0 range 32..63;

[sidisyom]

In case anyone wants to give that a try please clone this repo. If you place a copy of the ADL at the same level as the project file (or just give the absolute path if you already have that somewhere else) you should be able to compile and link in alire and get an elf file in the bin dir.

The main procedure is this one and the two addresses in question are here.

Apologies for this slightly chaotic repo but this is some really ad-hoc stuff I use as a sandbox area so please ignore anything else except the main file mentioned above (sorry, haven’t got too much time to properly separate these out now..)

The code should be fairly straightforward to follow but do let me know if something doesn’t make sense.

Thanks!

(Obviously, no crate-level material.. )

[SweetAda]

Your code is fine. Update your compiler.

That being said, some random annotations (that you can throw away if you don’t like them):

• ADC_SR_Register.Reserved field is 26 bits wide, not 25. It is even more dangerous that your compiler does not flag this
• Avoid, whenever possible, to define Volatile_Full_Access for a type, unless you pass a reference for these objects
• you may want to insert Reserved fields also in the final layout to highlight unused space

[sidisyom]

@SweetAda

Update your compiler

Hmm, I seem to be on the latest version according to alr toolchain --select

@SweetAda

ADC_SR_Register.Reserved field is 26 bits wide, not 25

Ah! good spot, thank you..based on my understanding it’s the representation clause that’s the final arbiter for this and things like Object_Size but could be wrong

@SweetAda

Avoid, whenever possible, to define Volatile_Full_Access for a type, unless you pass a reference for these objects

So maybe you’re already aware of this but this is a tricky one on this mcu. By design (feature/bug?) it only deals with 32-bit accesses on the APB buses which the peripherals are attached to. Any other access is silently converted to a 32-bit access by cloning the byte or half-word across the full 32 range:

This can cause some really bizarre behaviour when the compiler issues byte or half-word accesses cause the end effect is basically to override the contents of a register. I’ve had this issue in the past and it was somewhat hard to diagnose.
Was really keen to understand a bit more about using that when passing references of objects. Would you mind elaborating on that for a bit more, please?

@SweetAda

you may want to insert Reserved fields also in the final layout to highlight unused space

Makes total sense, I guess beneficial for documentation purposes too.

Thanks very much for looking into it, much appreciated.

[OneWingedShark]

@SweetAda

you may want to insert Reserved fields also in the final layout to highlight unused space

@sidisyom

Makes total sense, I guess beneficial for documentation purposes too.

There’s an argument for not having a Reserved field (or fields), and that is that if you don’t give it a name, you can’t manipulate it — and so (others => <>) can’t incorrectly stomp on the data — but if you must give the reserved areas a field, be sure that you have the proper model, which are null record types of the appropriate sizes.

[Micronian2]

I recall reading somewhere that since memory is frequently reused as objects are allocated/deallocated (from either heap or stack), a method to help avoid leaking sensitive data in the gaps/padding within data structures is to ensure every bit is covered by a field and assigned a value. The example given was to avoid sensitive kernel space data from leaking into user space when passing data from the former to the latter. The literature also mentioned thar simply having an allocator zero out the memory allocated to an object prior to populating the fields was insufficient since memory operations to update each field could still lead to the gaps getting populated with data.

[SweetAda]

Often when you write a register, you have to write all the fields – including reserved ones – with a defined value. Of course with proper size.

I was not able to find another way, but to name them “reserved” and give them a default value. You may raise an objection in that, in this case, my annotation is for whole registers in a peripheral layout, but mine was just a general approach. And explicitly touch a reserved field should ring a bell in the first instance.

[SweetAda]

@sidisyom

Hmm, I seem to be on the latest version according to alr toolchain --select

Your code does compile without warnings with stock FSF 14.2.

@sidisyom

Would you mind elaborating on that for a bit more, please?

Indeed I specified a type. Objects declared VFA are ok. Types should be VFA when access is involved, and they have to match across function call. But do not take it as a nazi rule, and it is more suited for whole registers that can fit in a CPU register.

@sidisyom

Thanks very much for looking into it, much appreciated.

Yay sir.