clayborg added a comment. In https://reviews.llvm.org/D42145#991985, @owenpshaw wrote:
> This discussion has got me questioning if WriteMemory is even the best place > to put the flash commands. It seems like some of the concern here is around > the behavior of arbitrary memory writes to flash regions, which isn't really > the main objective of this patch (supporting object file loading into flash). I am ok with having an extra interface on Process if needed. The ObjectFile::Load would use this new API to load things. But most targets aren't going to have flash and it would be weird if a target with a ton of RAM and no flash had to worry about that. Then ObjectFile::Load would need to be able to get memory region info, detect where flash is and where it isn't, and submit either a WriteMemory to WriteFlash command. I am ok with that if this makes things cleaner. > I did a little testing with gdb, and it doesn't do arbitrary memory writes to > flash regions. Using the set command on a flash address, for example, just > gives an error that flash writes are not allowed. Perhaps that's a better > way to go than worrying about supporting one-off flash writes. That would be fine. > > > ---- > >> I still like my WriteMemory(ArrayRef<MemoryWriteDescriptor>) proposal the >> best. > > The one difference I see between a single function and begin/end is that > begin/end doesn't require all the bytes to be read into host memory up front, > and can instead read the source bytes in chunks. Perhaps that's not really a > reason for concern, though. RAM is cheap these days and if the file is large, then mmap it, No RAM required. I wouldn't worry about that for now. So the main question is: do we want WriteMemory to work anywhere and always try to do the right thing, or return an error an the user would be expected to know to check the memory region you are writing to and know to call "Process::WriteFlash(...)". I vote to keep things simple and have Process::WriteMemory() just do the right thing. I am open to differing opinions, so let me know what you guys think. https://reviews.llvm.org/D42145 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
