[Pdbg] [PATCH 13/14] gdbserver: use read-modify-write for put_mem that is not 8-byte aligned

[Joel Stanley]

On Mon, 14 Mar 2022 at 04:18, Nicholas Piggin <npiggin at gmail.com> wrote:
>
> Not all targets have memory access that can support arbitrary byte
> writes. Use RMW for put_mem commands that are not 8-byte aligned.

Do we know which targets can/can't support unaligned byte writes?

>
> This should really either be done in the target accessor, or at least
> an alignment capability should be exposed to the caller. But for now
> this will allow sbefifo mem ops to work to set breakpoints (4-byte
> writes).
>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
>  src/pdbgproxy.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
> index a37e997..e8aab70 100644
> --- a/src/pdbgproxy.c
> +++ b/src/pdbgproxy.c
> @@ -14,6 +14,7 @@
>  #include <assert.h>
>  #include <getopt.h>
>  #include <errno.h>
> +#include <byteswap.h>
>  #include <ccan/array_size/array_size.h>
>
>  #include <bitutils.h>
> @@ -281,15 +282,15 @@ out:
>  static void put_mem(uint64_t *stack, void *priv)
>  {
>         uint64_t addr, len;
> +       uint64_t start_addr, end_addr;
> +       uint64_t align = 8;
> +       uint32_t *insn;
>         uint8_t *data;
> +       uint8_t *buf;
>         uint8_t attn_opcode[] = {0x00, 0x00, 0x02, 0x00};
> +       uint8_t gdb_break_opcode[] = {0x7d, 0x82, 0x10, 0x08};
>         int err = 0;
>
> -       if (littleendian) {
> -               attn_opcode[1] = 0x02;
> -               attn_opcode[2] = 0x00;
> -       }
> -
>         addr = stack[0];
>         len = stack[1];
>         data = (uint8_t *) &stack[2];
> @@ -301,8 +302,7 @@ static void put_mem(uint64_t *stack, void *priv)
>                 goto out;
>         }
>
> -
> -       if (len == 4 && stack[2] == 0x0810827d) {
> +       if (len == 4 && !memcmp(data, gdb_break_opcode, 4)) {
>                 /* According to linux-ppc-low.c gdb only uses this
>                  * op-code for sw break points so we replace it with
>                  * the correct attn opcode which is what we need for
> @@ -318,17 +318,27 @@ static void put_mem(uint64_t *stack, void *priv)
>                         err = 2;
>                         goto out;
>                 }
> -       } else {
> -               stack[2] = __builtin_bswap64(stack[2]) >> 32;
>         }
>
> -       PR_INFO("put_mem 0x%016" PRIx64 " = 0x%016" PRIx64 "\n", addr, stack[2]);
> +       if (len == 4 && littleendian) {
> +               insn = (uint32_t *)data;
> +               *insn = bswap_32(*insn);
> +       }
>
> -       if (mem_write(adu_target, addr, data, len, 0, false)) {
> +       start_addr = addr & ~(align - 1);
> +       end_addr = (addr + len + (align - 1)) & ~(align - 1);

Would some ALIGN_UP / ALIGN_DOWN macros make sense here?

> +       if (addr != start_addr || (addr + len) != end_addr) {
> +               buf = malloc(end_addr - start_addr);
> +               mem_read(adu_target, start_addr, buf, end_addr - start_addr, 0, false);
> +               memcpy(buf + (addr - start_addr), data, len);
> +       } else {
> +               buf = data;
> +       }
> +
> +       if (mem_write(adu_target, start_addr, buf, end_addr - start_addr, 0, false)) {
>                 PR_ERROR("Unable to write memory\n");
>                 err = 3;
>         }

Needs to free buf if it was malloc'd.

> -
>  out:
>         if (err)
>                 send_response(fd, ERROR(EPERM));
> --
> 2.23.0
>
> --
> Pdbg mailing list
> Pdbg at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/pdbg