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

Dr. David Alan Gilbert
2020-12-01T11:38:43Z

* 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.
>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
> include/exec/memory.h | 11 +++++++++++
> softmmu/physmem.c | 24 ++++++++++++++++++++----
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index aff6ef7605..73deb4b456 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2394,6 +2394,17 @@ MemTxResult address_space_write_cached_slow(MemoryRegionCache *cache,
> hwaddr addr, const void *buf,
> hwaddr len);
>
> +typedef struct MemoryDebugOps {
> + MemTxResult (*read)(AddressSpace *as, hwaddr phys_addr,
> + MemTxAttrs attrs, void *buf,
> + hwaddr len);
> + MemTxResult (*write)(AddressSpace *as, hwaddr phys_addr,
> + MemTxAttrs attrs, const void *buf,
> + hwaddr len);
> +} MemoryDebugOps;
> +
> +void address_space_set_debug_ops(const MemoryDebugOps *ops);
> +
> static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
> {
> if (is_write) {
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index a9adedb9f8..057d6d4ce1 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -166,6 +166,18 @@ struct DirtyBitmapSnapshot {
> unsigned long dirty[];
> };
>
> +static const MemoryDebugOps default_debug_ops = {
> + .read = address_space_read,
> + .write = address_space_write_rom
> +};
> +
> +static const MemoryDebugOps *debug_ops = &default_debug_ops;
> +
> +void address_space_set_debug_ops(const MemoryDebugOps *ops)
> +{
> + debug_ops = ops;
> +}
> +
> static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes)
> {
> static unsigned alloc_hint = 16;
> @@ -3407,6 +3419,10 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
> page = addr & TARGET_PAGE_MASK;
> phys_addr = cpu_get_phys_page_attrs_debug(cpu, page, &attrs);
> asidx = cpu_asidx_from_attrs(cpu, attrs);
> +
> + /* set debug attrs to indicate memory access is from the debugger */
> + attrs.debug = 1;
> +
> /* if no physical page mapped, return an error */
> if (phys_addr == -1)
> return -1;
> @@ -3415,11 +3431,11 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
> l = len;
> phys_addr += (addr & ~TARGET_PAGE_MASK);
> if (is_write) {
> - res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr,
> - attrs, buf, l);
> + res = debug_ops->write(cpu->cpu_ases[asidx].as, phys_addr,
> + attrs, buf, l);
> } else {
> - res = address_space_read(cpu->cpu_ases[asidx].as, phys_addr,
> - attrs, buf, l);
> + res = debug_ops->read(cpu->cpu_ases[asidx].as, phys_addr,
> + attrs, buf, l);
> }
> if (res != MEMTX_OK) {
> return -1;
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK