Apologies for the delay replying here.
On Thu, Feb 22, 2024 at 01:36:41PM +0100, Heinrich Schuchardt wrote:
On 21.02.24 18:59, Conor Dooley wrote:
I mentioned this last night to Heinrich on IRC, supports_extension() is broken for ISA strings longer than 32 characters. M-Mode U-Boot doesn't parse a devicetree, so this doesn't apply there, but for S-mode supports_extension() looks like:
static inline bool supports_extension(char ext) {
struct udevice *dev; char desc[32]; int i;
uclass_find_first_device(UCLASS_CPU, &dev); if (!dev) { debug("unable to find the RISC-V cpu device\n"); return false; } if (!cpu_get_desc(dev, desc, sizeof(desc))) { /* * skip the first 4 characters (rv32|rv64) and * check until underscore */ for (i = 4; i < sizeof(desc); i++) { if (desc[i] == '_' || desc[i] == '\0') break; if (desc[i] == ext) return true; } }
return false; }
cpu_get_desc is implemented by riscv_cpu_get_desc(): static int riscv_cpu_get_desc(const struct udevice *dev, char *buf, int size)
Thanks Conor for reporting the issue. We should change all cpu_get_desc implementations to:
int riscv_cpu_get_desc(const struct udevice *dev, char *buf, size_t *size) { size_t old_size = *size;
*size = snprintf(buf, *size, "%s", info) + 1; if (*size > old_size) return -ENOSPC; return 0; }
With this change
size = 0; cpu_get_desc(dev, desc, &size);
can be used to get the size of the information before allocating a buffer.
desc = malloc(size); cpu_get_desc(dev, desc, size);
That seems overcomplicated to me, if we are just looking to fix this in the short term. Could we just write to the provided buffer up to a max of size and report ENOSPC if it does not fit? That way the calling code in 90% of cases does not need to change and the supports_extension() code, which does not care about anything other than single-letter extensions that have to be in the first 32 characters of the string, can opt to ignore the ENOSPC?
Cheers, Conor.