Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory access initiated by GDB is mishandled on some configurations #1184

Open
en-sc opened this issue Dec 10, 2024 · 1 comment
Open

Memory access initiated by GDB is mishandled on some configurations #1184

en-sc opened this issue Dec 10, 2024 · 1 comment
Labels
Good First Issue This label marks the first good issue for anyone willing to contribute to the project.

Comments

@en-sc
Copy link
Collaborator

en-sc commented Dec 10, 2024

Thanks to @Qidi-ic for finding out about this! See #1177 for the original issue.

Memory accesses initiated through GDB (unlike the once done via TCL) do not carry the desired access width
(https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html#Packets).

E.g. when handling m/M OpenOCD is free to choose which access width to use.
Such accesses are done through target->type->read/write_buffer.
Currently, RISC-V targets use the default implementation of these functions:

static int target_read_buffer_default(struct target *target, target_addr_t address, uint32_t count, uint8_t *buffer)
{
uint32_t size;
unsigned int data_bytes = target_data_bits(target) / 8;
/* Align up to maximum bytes. The loop condition makes sure the next pass
* will have something to do with the size we leave to it. */
for (size = 1;
size < data_bytes && count >= size * 2 + (address & size);
size *= 2) {
if (address & size) {
int retval = target_read_memory(target, address, size, 1, buffer);
if (retval != ERROR_OK)
return retval;
address += size;
count -= size;
buffer += size;
}
}
/* Read the data with as large access size as possible. */
for (; size > 0; size /= 2) {
uint32_t aligned = count - count % size;
if (aligned > 0) {
int retval = target_read_memory(target, address, size, aligned / size, buffer);
if (retval != ERROR_OK)
return retval;
address += aligned;
count -= aligned;
buffer += aligned;
}
}
return ERROR_OK;
}

It works by first accessing the memory using smaller sizes to align the start address by target_data_bits(), which is riscv013_data_bits() for a RISC-V target:

/* Try to find out the widest memory access size depending on the selected memory access methods. */
static unsigned int riscv013_data_bits(struct target *target)
{
RISCV013_INFO(info);
RISCV_INFO(r);
for (unsigned int i = 0; i < r->num_enabled_mem_access_methods; i++) {
riscv_mem_access_method_t method = r->mem_access_methods[i];
if (method == RISCV_MEM_ACCESS_PROGBUF) {
if (has_sufficient_progbuf(target, 3))
return riscv_xlen(target);
} else if (method == RISCV_MEM_ACCESS_SYSBUS) {
if (get_field(info->sbcs, DM_SBCS_SBACCESS128))
return 128;
if (get_field(info->sbcs, DM_SBCS_SBACCESS64))
return 64;
if (get_field(info->sbcs, DM_SBCS_SBACCESS32))
return 32;
if (get_field(info->sbcs, DM_SBCS_SBACCESS16))
return 16;
if (get_field(info->sbcs, DM_SBCS_SBACCESS8))
return 8;
} else if (method == RISCV_MEM_ACCESS_ABSTRACT) {
/* TODO: Once there is a spec for discovering abstract commands, we can
* take those into account as well. For now we assume abstract commands
* support XLEN-wide accesses. */
return riscv_xlen(target);
} else {
assert(false);
}
}
LOG_TARGET_ERROR(target, "Unable to determine supported data bits on this target. Assuming 32 bits.");
return 32;
}

Now imagine a target that supports only System Bus Access that is 64-bit wide. However, the target supports unaligned accesses. For such a target the algorithm will fail to write 32-bit aligned buffer, since it will first try to align it up by a 32-bit wide access.

This should be mitigated.

@en-sc en-sc added the Good First Issue This label marks the first good issue for anyone willing to contribute to the project. label Dec 10, 2024
@JanMatCodasip
Copy link
Collaborator

JanMatCodasip commented Dec 12, 2024

imagine a target that supports only System Bus Access that is 64-bit wide. However, the target supports unaligned accesses. For such a target the algorithm will fail to write 32-bit aligned buffer, since it will first try to align it up by a 32-bit wide access.

In my opinion, targets that have the debug memory access considerably restricted (for example as described above) simply won't offer good debugging experience to the user.

The debug software (OpenOCD) could in theory emulate the memory reads and writes of smaller sizes, however this is not always possible. Imagine memory mapped peripherals where emulating a write by doing read-modify-write may have unintended side effects.

My recommendation is that hardware designers, who expect their targets to be debugged, make sure that all memory access widths that loads & stores support are available to the debugger, too. Otherwise the debug experience may be poor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue This label marks the first good issue for anyone willing to contribute to the project.
Projects
None yet
Development

No branches or pull requests

2 participants