Re: [PATCH 02/11] exec: Add new MemoryDebugOps.

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

On Mon, 16 Nov 2020 at 19:07, Ashish Kalra <Ashish.Kalra@amd.com> wrote:
>
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> Introduce new MemoryDebugOps which hook into guest virtual and physical
> memory debug interfaces such as cpu_memory_rw_debug, to allow vendor specific
> assist/hooks for debugging and delegating accessing the guest memory.
> This is required for example in case of AMD SEV platform where the guest
> memory is encrypted and a SEV specific debug assist/hook will be required
> to access the guest memory.
>
> The MemoryDebugOps are used by cpu_memory_rw_debug() and default to
> address_space_read and address_space_write_rom.

This seems like a weird place to insert these hooks. Not
all debug related accesses are going to go via
cpu_memory_rw_debug(). For instance when the gdb stub is in
"PhyMemMode" and all addresses from the debugger are treated as
physical rather than virtual, gdbstub.c will call
cpu_physical_memory_write()/_read().

I would have expected the "oh, this is a debug access, do
something special" to be at a lower level, so that any
address_space_* access to the guest memory with the debug
attribute gets the magic treatment, whether that was done
as a direct "read this physaddr" or via cpu_memory_rw_debug()
doing the virt-to-phys conversion first.

thanks
-- PMM