Re: [Qemu-devel] [PATCH 38/43] windbg: implemented kd_api_read_physical_memory and kd_api_write_physical_memory

[Ladi Prosek]

On Tue, Sep 26, 2017 at 1:07 PM, Mihail Abakumov
<address@hidden> wrote:
> Signed-off-by: Mihail Abakumov <address@hidden>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> Signed-off-by: Dmitriy Koltunov <address@hidden>
> ---
>  include/exec/windbgstub-utils.h |    2 ++
>  windbgstub-utils.c              |   28 ++++++++++++++++++++++++++++
>  windbgstub.c                    |    8 ++++++++
>  3 files changed, 38 insertions(+)
>
> diff --git a/include/exec/windbgstub-utils.h b/include/exec/windbgstub-utils.h
> index 07e11c0494..d390bc1dfe 100755
> --- a/include/exec/windbgstub-utils.h
> +++ b/include/exec/windbgstub-utils.h
> @@ -113,6 +113,8 @@ void kd_api_read_control_space(CPUState *cpu, PacketData 
> *pd);
>  void kd_api_write_control_space(CPUState *cpu, PacketData *pd);
>  void kd_api_read_io_space(CPUState *cpu, PacketData *pd);
>  void kd_api_write_io_space(CPUState *cpu, PacketData *pd);
> +void kd_api_read_physical_memory(CPUState *cpu, PacketData *pd);
> +void kd_api_write_physical_memory(CPUState *cpu, PacketData *pd);
>  void kd_api_unsupported(CPUState *cpu, PacketData *pd);
>
>  SizedBuf kd_gen_exception_sc(CPUState *cpu);
> diff --git a/windbgstub-utils.c b/windbgstub-utils.c
> index 6bf9868e51..1fba54f9d9 100755
> --- a/windbgstub-utils.c
> +++ b/windbgstub-utils.c
> @@ -1170,6 +1170,34 @@ void kd_api_write_io_space(CPUState *cpu, PacketData 
> *pd)
>      pd->m64.ReturnStatus = STATUS_SUCCESS;
>  }
>
> +void kd_api_read_physical_memory(CPUState *cpu, PacketData *pd)
> +{
> +    DBGKD_READ_MEMORY64 *mem = &pd->m64.u.ReadMemory;
> +    uint32_t len;
> +    target_ulong addr;
> +
> +    len = MIN(ldl_p(&mem->TransferCount), PACKET_MAX_SIZE - M64_SIZE);
> +    addr = ldtul_p(&mem->TargetBaseAddress);
> +
> +    cpu_physical_memory_rw(addr, pd->extra, len, 0);
> +    pd->extra_size = len;
> +    mem->ActualBytesRead = ldl_p(&len);

stl_p(&mem->ActualBytesRead, len)

And the same pattern is in many other places. Stores to potentially
unaligned memory locations with the right endianness should be done
with st* not ld*.

> +}
> +
> +void kd_api_write_physical_memory(CPUState *cpu, PacketData *pd)
> +{
> +    DBGKD_WRITE_MEMORY64 *mem = &pd->m64.u.WriteMemory;
> +    uint32_t len;
> +    target_ulong addr;
> +
> +    len = MIN(ldl_p(&mem->TransferCount), pd->extra_size);
> +    addr = ldtul_p(&mem->TargetBaseAddress);
> +
> +    cpu_physical_memory_rw(addr, pd->extra, len, 1);
> +    pd->extra_size = 0;
> +    mem->ActualBytesWritten = ldl_p(&len);
> +}
> +
>  void kd_api_unsupported(CPUState *cpu, PacketData *pd)
>  {
>      WINDBG_ERROR("Catched unimplemented api %s",
> diff --git a/windbgstub.c b/windbgstub.c
> index 0363d9a5c8..118e9dd697 100755
> --- a/windbgstub.c
> +++ b/windbgstub.c
> @@ -182,6 +182,14 @@ static void 
> windbg_process_manipulate_packet(ParsingContext *ctx)
>          kd_api_write_control_space(cpu, &ctx->data);
>          break;
>
> +    case DbgKdReadPhysicalMemoryApi:
> +        kd_api_read_physical_memory(cpu, &ctx->data);
> +        break;
> +
> +    case DbgKdWritePhysicalMemoryApi:
> +        kd_api_write_physical_memory(cpu, &ctx->data);
> +        break;
> +
>      case DbgKdClearAllInternalBreakpointsApi:
>          return;
>
>