Loading object file to target flash memory using vFlashWrite

Looks like there was an addition about a year ago to load an object
file to bare metal target memory via "target modules load --load".
https://reviews.llvm.org/D28804

However, this isn't working in my case with openocd and a target that
uses flash memory. By comparison, gdb loads the object just fine, but
instead of using the M memory write command, it uses vFlashErase,
vFlashWrite, and vFlashDone.

These flash commands don't seem to be supported in lldb. Any reason
not to add them?

I've got a basic working implementation, which is probably easier to
break into two reviews:

1. Add support for qXfer:memory-map and parse the resulting xml to
determine which memory regions are flash and what their blocksizes are
(required for vFlashErase)
2. Update ProcessGDBRemote::DoWriteMemory to use the flash commands
when writing to a flash region, while continuing to use M when writing
to non-flash regions.

If this sounds like the right track, I'll open a Phabricator review
for #1 to start.

One question I have relates to the load address for object sections.
The current ObjectFile::LoadInMemory implementation will incorrectly
place my .data section at its virtual address in RAM when it really
should be placed at its physical address in flash memory. I have code
that can place it at the physical address, but it's unclear if that
change is correct for every case. Does anyone have insight/opinions?

Thanks,
Owen

Looks like there was an addition about a year ago to load an object
file to bare metal target memory via "target modules load --load".
https://reviews.llvm.org/D28804

However, this isn't working in my case with openocd and a target that
uses flash memory. By comparison, gdb loads the object just fine, but
instead of using the M memory write command, it uses vFlashErase,
vFlashWrite, and vFlashDone.

These flash commands don't seem to be supported in lldb. Any reason
not to add them?

I've got a basic working implementation, which is probably easier to
break into two reviews:

1. Add support for qXfer:memory-map and parse the resulting xml to
determine which memory regions are flash and what their blocksizes are
(required for vFlashErase)
2. Update ProcessGDBRemote::DoWriteMemory to use the flash commands
when writing to a flash region, while continuing to use M when writing
to non-flash regions.

If this sounds like the right track, I'll open a Phabricator review
for #1 to start.

Those two things sound about right. Though we typically don't like to add support for something if it isn't used. #1 would add support for "qXfer:memory-map", but nothing would use it. So it might be ok to submit #1 and #2 as one patch. I am open to suggestions from others if they believe differently. A patch should have tests for this and I am not sure how to test it, but just know that we will want some sort of tests.

One question I have relates to the load address for object sections.
The current ObjectFile::LoadInMemory implementation will incorrectly
place my .data section at its virtual address in RAM when it really
should be placed at its physical address in flash memory. I have code
that can place it at the physical address, but it's unclear if that
change is correct for every case. Does anyone have insight/opinions?

Right now we just load things at the virtual address found in the ELF file. Not sure if the physical address should be used for everyone as I am not sure of exactly who is using this right now. Most of the time when I see ELF files the virtual address is equal to the physical address, so it might just be ok. Feel free to let us know what you think should happen and submit a patch. If no one has any tests that break in response to this, then we can try it and see how things go.

Greg

Thanks!

ObjectFileELF::SetLoadAddress(...) is where I already have a change
that got things working, just wasn't sure if I'd be stepping on
anything else that relied on the values set there.

Looks like I dropped the dev list in my last message. Adding it back
so this is captured.

I'm not sure the existing lldb-server tests will be of much help here.
What the existing tests do is *test the server* by driving it with a
*specialized client* which verifies the server behavior. If I
understand correctly, here you would want to *test the client*
behavior when communicating with a *custom server*. Right now we don't
have any tests of this sort, although it would be very good to have
them -- we get patches to support all kinds of remote stubs, but we
have no way to verify that the next patch does not break any previous
ones. The low level details can be tested with a
gdbremotecommunicationclient unit tests (like you did with the
previous patch), but I don't think there is anything that would
capture interactions at a slightly higher level.

I think the best we can hope for is to mimic the packets that work with a client that supports the flash write packets.