On 12 March 2018 at 11:24, Su Hang <suhan...@mails.ucas.ac.cn> wrote: > Dear Thomas, > I'm tring to work on this job: > BiteSizedTasks: > API conversion: > Replace calls to functions named cpu_physical_memory_* with > address_space_*.
This is a generic suggestion, but not all cpu_physical_memory_* functions currently have a direct counterpart in address_space_*. (Note also that it is not as simple as "change the function name at the call site -- there are more parameters to the address_space_* functions. Ideally it also requires examining each call site to see what it should do if the memory access fails, and what address space it should really be using. This is rather more complicated than it appears from the fact it's on a bite-sized-task list. Any individual conversion of a callsite is probably small enough to be bite-sized, but you shouldn't consider "change everywhere" to be a single small task.) As you've found, cpu_physical_memory_write_rom() is one of those functions that doesn't currently have an address_space_* equivalent. There's actually a 'TODO' note in our API docs in docs/devel/loads-stores.rst about this: # Note that unlike ``cpu_physical_memory_write()`` this function takes # an AddressSpace argument, but unlike ``address_space_write()`` this # function does not take a ``MemTxAttrs`` or return a ``MemTxResult``. # # **TODO**: we should probably clean up this inconsistency and # turn the function into ``address_space_write_rom`` with an API # matching ``address_space_write``. and I think that would be the best thing: (1) have address_space_write_rom() which takes the extra MemTxAttrs argument and returns the MemTxResult, and make cpu_physical_memory_write_rom() just be a trivial wrapper around it (2) then change the callers to use address_space_write_rom() (3) remove the cpu_physical_memory_write_rom() wrapper Step 2 is the harder part, because really it requires looking at the call sites to see what they should do if the memory access fails. thanks -- PMM
