Re: [PATCH 1/2] gdbstub: Set current_cpu for memory read write

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 24 Mar 2022 at 10:33, Alex Bennée <alex.bennee@linaro.org> wrote:
> I think we need to not use cpu_physical_memory_write (which is
> explicitly the system address space) but have a function that takes cpu
> so it can work out the correct address space to you
> address_space_read/write. If null we could probably reasonably use
> first_cpu as an approximation.

It's not sufficient to use address_space_read/write, because the
devices in question are written to figure out the accessing CPU
using current_cpu, not by having different MemoryRegions in the
different per-CPU AddressSpaces. (You can see this because the bug
is present in the common "gdb gives us a virtual address" case which
goes via cpu_memory_rw_debug() and does thus use address_space_read(),
not only in the oddball 'treat addresses from gdb as physaddrs' case.)

If we want to fix this without setting current_cpu, then we need to
rewrite these devices not to be accessing it, which we could do
in one of two ways:
 * have the devices arrange to put different MRs in the ASes
   for each CPU (this is possible today but a massive pain as
   it would likely involve restructuring a lot of board and
   SoC level code)
 * have the MemTxAttrs tell the device what the accessing CPU is
   (probably by extending the requester_id field); this is
   somewhat analogous to how it happens in some cases on real
   hardware, where the AXI bus signals include an ID field
   indicating what initiated the transaction. This feels neater,
   but it's still quite a bit of work for such an unimportant
   corner case.

Or we could work around things, by requiring that devices that
access current_cpu cope with it being NULL in some vaguely
plausible way. This means that even when you tell gdb or the
monitor "access this address using this CPU" you'll get CPU0's
view, of course. Some devices like hw/intc/openpic.c do this already.

Or we could continue to ignore the bug, like we've done for the
past six years, on the basis that you only hit it if you've
done something slightly silly in gdb or the monitor in
the first place :-)

-- PMM