On Tue, Dec 01, 2020 at 11:48:23AM +0000, Peter Maydell wrote: > On Mon, 16 Nov 2020 at 19:07, Ashish Kalra <Ashish.Kalra@xxxxxxx> wrote: > > > > From: Ashish Kalra <ashish.kalra@xxxxxxx> > > > > 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. > Actually, the earlier patch-set used to do this at a lower level, i.e., at the address_space level, but then Paolo's feedback on that was that we don't want to add debug specific hooks into generic code such as address_space_* interfaces, hence, these hooks are introduced at a higher level so that we can do this "debug" abstraction at cpu_memory_rw_debug() and adding new interfaces for physical memory read/write debugging such as cpu_physical_memory_rw_debug(). This seems logical too as cpu_memory_rw_debug() is invoked via the debugger, i.e., either gdbstub or qemu monitor, so this interface seems to be the right place to add these hooks. Thanks, Ashish