* Subject: [PATCH] loader: Add register setting support via cli @ 2024-12-06 3:29 Sam Price 2024-12-19 23:11 ` Sam Price 2025-01-06 4:41 ` Alistair Francis 0 siblings, 2 replies; 12+ messages in thread From: Sam Price @ 2024-12-06 3:29 UTC (permalink / raw) To: qemu-devel; +Cc: alistair I needed to set the registers prior to boot up to mimic what uboot would do prior to loading a binary. This adds a generic option of reg to the loader command, it uses the existing gcc commands for setting register values. I'm sorry I couldn't figure out how to work the git send-email properly. Configuring it with my gmail became too cumbersome, and my work email was also challenging to configure. I have the file staged here. https://gitlab.com/thesamprice/qemu/-/tree/loader?ref_type=heads I am unsure of how to add tests for this. I could continue working too polish this off with instructions from others if it is desired for the main line. Signed-off-by: Sam Price <thesamprice@gmail.com> --- --- hw/core/generic-loader.c | 28 ++++++++++++++++++++++++++++ include/hw/core/generic-loader.h | 6 +++++- roms/SLOF | 2 +- roms/edk2 | 2 +- roms/openbios | 2 +- roms/opensbi | 2 +- roms/seabios | 2 +- roms/seabios-hppa | 2 +- tests/lcitool/libvirt-ci | 2 +- 9 files changed, 40 insertions(+), 8 deletions(-) diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c index ea8628b892..ebda8ac43f 100644 --- a/hw/core/generic-loader.c +++ b/hw/core/generic-loader.c @@ -55,6 +55,19 @@ static void generic_loader_reset(void *opaque) } } + for (int i = 0; i < 31; i++) { + if (s->has_register_defaults[i]) { + CPUClass *cc = CPU_GET_CLASS(s->cpu); + uint8_t buf[sizeof(uint64_t)]; + memcpy(buf, &s->register_defaults[i], sizeof(uint64_t)); + if (cc && cc->gdb_write_register) { + cc->gdb_write_register(s->cpu, buf, i); + } + } + } + + + if (s->data_len) { assert(s->data_len <= sizeof(s->data)); dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, @@ -172,6 +185,20 @@ static void generic_loader_realize(DeviceState *dev, Error **errp) } else { s->data = cpu_to_le64(s->data); } + + /* Store the CPU register default if specified */ + if (s->reg) { + int reg_num; + if (sscanf(s->reg, "r%d", ®_num) == 1 && + reg_num >= 0 && reg_num < 31) { + s->register_defaults[reg_num] = s->data; + s->has_register_defaults[reg_num] = true; + } else { + error_setg(errp, "Unsupported register: %s", s->reg); + return; + } + } + } static void generic_loader_unrealize(DeviceState *dev) @@ -186,6 +213,7 @@ static Property generic_loader_props[] = { DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false), DEFINE_PROP_UINT32("cpu-num", GenericLoaderState, cpu_num, CPU_NONE), DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false), + DEFINE_PROP_STRING("reg", GenericLoaderState, reg), DEFINE_PROP_STRING("file", GenericLoaderState, file), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h index 19d87b39c8..d81e1632fd 100644 --- a/include/hw/core/generic-loader.h +++ b/include/hw/core/generic-loader.h @@ -35,10 +35,14 @@ struct GenericLoaderState { uint32_t cpu_num; char *file; - + char *reg; bool force_raw; bool data_be; bool set_pc; + + /* Add an array for storing default register values */ + bool has_register_defaults[31]; /* Track if a default value is provided */ + uint64_t register_defaults[31]; /* Default values for registers r0-r30 */ }; #define TYPE_GENERIC_LOADER "loader" diff --git a/roms/SLOF b/roms/SLOF index 3a259df244..6b6c16b4b4 160000 --- a/roms/SLOF +++ b/roms/SLOF @@ -1 +1 @@ -Subproject commit 3a259df2449fc4a4e43ab5f33f0b2c66484b4bc3 +Subproject commit 6b6c16b4b40763507cf1f518096f3c3883c5cf2d diff --git a/roms/edk2 b/roms/edk2 index 4dfdca63a9..f80f052277 160000 --- a/roms/edk2 +++ b/roms/edk2 @@ -1 +1 @@ -Subproject commit 4dfdca63a93497203f197ec98ba20e2327e4afe4 +Subproject commit f80f052277c88a67c55e107b550f504eeea947d3 diff --git a/roms/openbios b/roms/openbios index c3a19c1e54..af97fd7af5 160000 --- a/roms/openbios +++ b/roms/openbios @@ -1 +1 @@ -Subproject commit c3a19c1e54977a53027d6232050e1e3e39a98a1b +Subproject commit af97fd7af5e7c18f591a7b987291d3db4ffb28b5 diff --git a/roms/opensbi b/roms/opensbi index 43cace6c36..057eb10b6d 160000 --- a/roms/opensbi +++ b/roms/opensbi @@ -1 +1 @@ -Subproject commit 43cace6c3671e5172d0df0a8963e552bb04b7b20 +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 diff --git a/roms/seabios b/roms/seabios index a6ed6b701f..ea1b7a0733 160000 --- a/roms/seabios +++ b/roms/seabios @@ -1 +1 @@ -Subproject commit a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 +Subproject commit ea1b7a0733906b8425d948ae94fba63c32b1d425 diff --git a/roms/seabios-hppa b/roms/seabios-hppa index a528f01d7a..673d2595d4 160000 --- a/roms/seabios-hppa +++ b/roms/seabios-hppa @@ -1 +1 @@ -Subproject commit a528f01d7abd511d3cc71b7acaab6e036ee524bd +Subproject commit 673d2595d4f773cc266cbf8dbaf2f475a6adb949 diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci index 9ad3f70bde..9bff3b763b 160000 --- a/tests/lcitool/libvirt-ci +++ b/tests/lcitool/libvirt-ci @@ -1 +1 @@ -Subproject commit 9ad3f70bde9865d5ad18f36d256d472e72b5cbf3 +Subproject commit 9bff3b763b5531a1490e238bfbf77306dc3a6dbb -- 2.45.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2024-12-06 3:29 Subject: [PATCH] loader: Add register setting support via cli Sam Price @ 2024-12-19 23:11 ` Sam Price 2025-01-06 1:43 ` Sam Price 2025-01-06 4:41 ` Alistair Francis 1 sibling, 1 reply; 12+ messages in thread From: Sam Price @ 2024-12-19 23:11 UTC (permalink / raw) To: qemu-devel; +Cc: alistair [-- Attachment #1: Type: text/plain, Size: 6040 bytes --] Bump / ping Sincerely, Sam Price On Thu, Dec 5, 2024 at 10:29 PM Sam Price <thesamprice@gmail.com> wrote: > I needed to set the registers prior to boot up to mimic what uboot > would do prior to loading a binary. This adds a generic option of reg > to the loader command, it uses the existing gcc commands for setting > register values. > > I'm sorry I couldn't figure out how to work the git send-email > properly. Configuring it with my gmail became too cumbersome, and my > work email was also challenging to configure. > > I have the file staged here. > https://gitlab.com/thesamprice/qemu/-/tree/loader?ref_type=heads > I am unsure of how to add tests for this. > I could continue working too polish this off with instructions from > others if it is desired for the main line. > > Signed-off-by: Sam Price <thesamprice@gmail.com> > --- > > --- > hw/core/generic-loader.c | 28 ++++++++++++++++++++++++++++ > include/hw/core/generic-loader.h | 6 +++++- > roms/SLOF | 2 +- > roms/edk2 | 2 +- > roms/openbios | 2 +- > roms/opensbi | 2 +- > roms/seabios | 2 +- > roms/seabios-hppa | 2 +- > tests/lcitool/libvirt-ci | 2 +- > 9 files changed, 40 insertions(+), 8 deletions(-) > > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > index ea8628b892..ebda8ac43f 100644 > --- a/hw/core/generic-loader.c > +++ b/hw/core/generic-loader.c > @@ -55,6 +55,19 @@ static void generic_loader_reset(void *opaque) > } > } > > + for (int i = 0; i < 31; i++) { > + if (s->has_register_defaults[i]) { > + CPUClass *cc = CPU_GET_CLASS(s->cpu); > + uint8_t buf[sizeof(uint64_t)]; > + memcpy(buf, &s->register_defaults[i], sizeof(uint64_t)); > + if (cc && cc->gdb_write_register) { > + cc->gdb_write_register(s->cpu, buf, i); > + } > + } > + } > + > + > + > if (s->data_len) { > assert(s->data_len <= sizeof(s->data)); > dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, > @@ -172,6 +185,20 @@ static void generic_loader_realize(DeviceState > *dev, Error **errp) > } else { > s->data = cpu_to_le64(s->data); > } > + > + /* Store the CPU register default if specified */ > + if (s->reg) { > + int reg_num; > + if (sscanf(s->reg, "r%d", ®_num) == 1 && > + reg_num >= 0 && reg_num < 31) { > + s->register_defaults[reg_num] = s->data; > + s->has_register_defaults[reg_num] = true; > + } else { > + error_setg(errp, "Unsupported register: %s", s->reg); > + return; > + } > + } > + > } > > static void generic_loader_unrealize(DeviceState *dev) > @@ -186,6 +213,7 @@ static Property generic_loader_props[] = { > DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false), > DEFINE_PROP_UINT32("cpu-num", GenericLoaderState, cpu_num, CPU_NONE), > DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false), > + DEFINE_PROP_STRING("reg", GenericLoaderState, reg), > DEFINE_PROP_STRING("file", GenericLoaderState, file), > DEFINE_PROP_END_OF_LIST(), > }; > diff --git a/include/hw/core/generic-loader.h > b/include/hw/core/generic-loader.h > index 19d87b39c8..d81e1632fd 100644 > --- a/include/hw/core/generic-loader.h > +++ b/include/hw/core/generic-loader.h > @@ -35,10 +35,14 @@ struct GenericLoaderState { > uint32_t cpu_num; > > char *file; > - > + char *reg; > bool force_raw; > bool data_be; > bool set_pc; > + > + /* Add an array for storing default register values */ > + bool has_register_defaults[31]; /* Track if a default value is > provided */ > + uint64_t register_defaults[31]; /* Default values for registers > r0-r30 */ > }; > > #define TYPE_GENERIC_LOADER "loader" > diff --git a/roms/SLOF b/roms/SLOF > index 3a259df244..6b6c16b4b4 160000 > --- a/roms/SLOF > +++ b/roms/SLOF > @@ -1 +1 @@ > -Subproject commit 3a259df2449fc4a4e43ab5f33f0b2c66484b4bc3 > +Subproject commit 6b6c16b4b40763507cf1f518096f3c3883c5cf2d > diff --git a/roms/edk2 b/roms/edk2 > index 4dfdca63a9..f80f052277 160000 > --- a/roms/edk2 > +++ b/roms/edk2 > @@ -1 +1 @@ > -Subproject commit 4dfdca63a93497203f197ec98ba20e2327e4afe4 > +Subproject commit f80f052277c88a67c55e107b550f504eeea947d3 > diff --git a/roms/openbios b/roms/openbios > index c3a19c1e54..af97fd7af5 160000 > --- a/roms/openbios > +++ b/roms/openbios > @@ -1 +1 @@ > -Subproject commit c3a19c1e54977a53027d6232050e1e3e39a98a1b > +Subproject commit af97fd7af5e7c18f591a7b987291d3db4ffb28b5 > diff --git a/roms/opensbi b/roms/opensbi > index 43cace6c36..057eb10b6d 160000 > --- a/roms/opensbi > +++ b/roms/opensbi > @@ -1 +1 @@ > -Subproject commit 43cace6c3671e5172d0df0a8963e552bb04b7b20 > +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 > diff --git a/roms/seabios b/roms/seabios > index a6ed6b701f..ea1b7a0733 160000 > --- a/roms/seabios > +++ b/roms/seabios > @@ -1 +1 @@ > -Subproject commit a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 > +Subproject commit ea1b7a0733906b8425d948ae94fba63c32b1d425 > diff --git a/roms/seabios-hppa b/roms/seabios-hppa > index a528f01d7a..673d2595d4 160000 > --- a/roms/seabios-hppa > +++ b/roms/seabios-hppa > @@ -1 +1 @@ > -Subproject commit a528f01d7abd511d3cc71b7acaab6e036ee524bd > +Subproject commit 673d2595d4f773cc266cbf8dbaf2f475a6adb949 > diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci > index 9ad3f70bde..9bff3b763b 160000 > --- a/tests/lcitool/libvirt-ci > +++ b/tests/lcitool/libvirt-ci > @@ -1 +1 @@ > -Subproject commit 9ad3f70bde9865d5ad18f36d256d472e72b5cbf3 > +Subproject commit 9bff3b763b5531a1490e238bfbf77306dc3a6dbb > -- > 2.45.2 > [-- Attachment #2: Type: text/html, Size: 7459 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2024-12-19 23:11 ` Sam Price @ 2025-01-06 1:43 ` Sam Price 0 siblings, 0 replies; 12+ messages in thread From: Sam Price @ 2025-01-06 1:43 UTC (permalink / raw) To: qemu-devel; +Cc: alistair Bump / Ping On Thu, Dec 19, 2024 at 6:11 PM Sam Price <thesamprice@gmail.com> wrote: > > Bump / ping > > Sincerely, > > Sam Price > > > > On Thu, Dec 5, 2024 at 10:29 PM Sam Price <thesamprice@gmail.com> wrote: >> >> I needed to set the registers prior to boot up to mimic what uboot >> would do prior to loading a binary. This adds a generic option of reg >> to the loader command, it uses the existing gcc commands for setting >> register values. >> >> I'm sorry I couldn't figure out how to work the git send-email >> properly. Configuring it with my gmail became too cumbersome, and my >> work email was also challenging to configure. >> >> I have the file staged here. >> https://gitlab.com/thesamprice/qemu/-/tree/loader?ref_type=heads >> I am unsure of how to add tests for this. >> I could continue working too polish this off with instructions from >> others if it is desired for the main line. >> >> Signed-off-by: Sam Price <thesamprice@gmail.com> >> --- >> >> --- >> hw/core/generic-loader.c | 28 ++++++++++++++++++++++++++++ >> include/hw/core/generic-loader.h | 6 +++++- >> roms/SLOF | 2 +- >> roms/edk2 | 2 +- >> roms/openbios | 2 +- >> roms/opensbi | 2 +- >> roms/seabios | 2 +- >> roms/seabios-hppa | 2 +- >> tests/lcitool/libvirt-ci | 2 +- >> 9 files changed, 40 insertions(+), 8 deletions(-) >> >> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c >> index ea8628b892..ebda8ac43f 100644 >> --- a/hw/core/generic-loader.c >> +++ b/hw/core/generic-loader.c >> @@ -55,6 +55,19 @@ static void generic_loader_reset(void *opaque) >> } >> } >> >> + for (int i = 0; i < 31; i++) { >> + if (s->has_register_defaults[i]) { >> + CPUClass *cc = CPU_GET_CLASS(s->cpu); >> + uint8_t buf[sizeof(uint64_t)]; >> + memcpy(buf, &s->register_defaults[i], sizeof(uint64_t)); >> + if (cc && cc->gdb_write_register) { >> + cc->gdb_write_register(s->cpu, buf, i); >> + } >> + } >> + } >> + >> + >> + >> if (s->data_len) { >> assert(s->data_len <= sizeof(s->data)); >> dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, >> @@ -172,6 +185,20 @@ static void generic_loader_realize(DeviceState >> *dev, Error **errp) >> } else { >> s->data = cpu_to_le64(s->data); >> } >> + >> + /* Store the CPU register default if specified */ >> + if (s->reg) { >> + int reg_num; >> + if (sscanf(s->reg, "r%d", ®_num) == 1 && >> + reg_num >= 0 && reg_num < 31) { >> + s->register_defaults[reg_num] = s->data; >> + s->has_register_defaults[reg_num] = true; >> + } else { >> + error_setg(errp, "Unsupported register: %s", s->reg); >> + return; >> + } >> + } >> + >> } >> >> static void generic_loader_unrealize(DeviceState *dev) >> @@ -186,6 +213,7 @@ static Property generic_loader_props[] = { >> DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false), >> DEFINE_PROP_UINT32("cpu-num", GenericLoaderState, cpu_num, CPU_NONE), >> DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false), >> + DEFINE_PROP_STRING("reg", GenericLoaderState, reg), >> DEFINE_PROP_STRING("file", GenericLoaderState, file), >> DEFINE_PROP_END_OF_LIST(), >> }; >> diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h >> index 19d87b39c8..d81e1632fd 100644 >> --- a/include/hw/core/generic-loader.h >> +++ b/include/hw/core/generic-loader.h >> @@ -35,10 +35,14 @@ struct GenericLoaderState { >> uint32_t cpu_num; >> >> char *file; >> - >> + char *reg; >> bool force_raw; >> bool data_be; >> bool set_pc; >> + >> + /* Add an array for storing default register values */ >> + bool has_register_defaults[31]; /* Track if a default value is provided */ >> + uint64_t register_defaults[31]; /* Default values for registers r0-r30 */ >> }; >> >> #define TYPE_GENERIC_LOADER "loader" >> diff --git a/roms/SLOF b/roms/SLOF >> index 3a259df244..6b6c16b4b4 160000 >> --- a/roms/SLOF >> +++ b/roms/SLOF >> @@ -1 +1 @@ >> -Subproject commit 3a259df2449fc4a4e43ab5f33f0b2c66484b4bc3 >> +Subproject commit 6b6c16b4b40763507cf1f518096f3c3883c5cf2d >> diff --git a/roms/edk2 b/roms/edk2 >> index 4dfdca63a9..f80f052277 160000 >> --- a/roms/edk2 >> +++ b/roms/edk2 >> @@ -1 +1 @@ >> -Subproject commit 4dfdca63a93497203f197ec98ba20e2327e4afe4 >> +Subproject commit f80f052277c88a67c55e107b550f504eeea947d3 >> diff --git a/roms/openbios b/roms/openbios >> index c3a19c1e54..af97fd7af5 160000 >> --- a/roms/openbios >> +++ b/roms/openbios >> @@ -1 +1 @@ >> -Subproject commit c3a19c1e54977a53027d6232050e1e3e39a98a1b >> +Subproject commit af97fd7af5e7c18f591a7b987291d3db4ffb28b5 >> diff --git a/roms/opensbi b/roms/opensbi >> index 43cace6c36..057eb10b6d 160000 >> --- a/roms/opensbi >> +++ b/roms/opensbi >> @@ -1 +1 @@ >> -Subproject commit 43cace6c3671e5172d0df0a8963e552bb04b7b20 >> +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 >> diff --git a/roms/seabios b/roms/seabios >> index a6ed6b701f..ea1b7a0733 160000 >> --- a/roms/seabios >> +++ b/roms/seabios >> @@ -1 +1 @@ >> -Subproject commit a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 >> +Subproject commit ea1b7a0733906b8425d948ae94fba63c32b1d425 >> diff --git a/roms/seabios-hppa b/roms/seabios-hppa >> index a528f01d7a..673d2595d4 160000 >> --- a/roms/seabios-hppa >> +++ b/roms/seabios-hppa >> @@ -1 +1 @@ >> -Subproject commit a528f01d7abd511d3cc71b7acaab6e036ee524bd >> +Subproject commit 673d2595d4f773cc266cbf8dbaf2f475a6adb949 >> diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci >> index 9ad3f70bde..9bff3b763b 160000 >> --- a/tests/lcitool/libvirt-ci >> +++ b/tests/lcitool/libvirt-ci >> @@ -1 +1 @@ >> -Subproject commit 9ad3f70bde9865d5ad18f36d256d472e72b5cbf3 >> +Subproject commit 9bff3b763b5531a1490e238bfbf77306dc3a6dbb >> -- >> 2.45.2 -- Sincerely, Sam Price ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2024-12-06 3:29 Subject: [PATCH] loader: Add register setting support via cli Sam Price 2024-12-19 23:11 ` Sam Price @ 2025-01-06 4:41 ` Alistair Francis 2025-01-06 4:59 ` Sam Price 1 sibling, 1 reply; 12+ messages in thread From: Alistair Francis @ 2025-01-06 4:41 UTC (permalink / raw) To: Sam Price; +Cc: qemu-devel, alistair On Fri, Dec 6, 2024 at 1:30 PM Sam Price <thesamprice@gmail.com> wrote: > > I needed to set the registers prior to boot up to mimic what uboot > would do prior to loading a binary. This adds a generic option of reg > to the loader command, it uses the existing gcc commands for setting > register values. > > I'm sorry I couldn't figure out how to work the git send-email > properly. Configuring it with my gmail became too cumbersome, and my > work email was also challenging to configure. > > I have the file staged here. > https://gitlab.com/thesamprice/qemu/-/tree/loader?ref_type=heads > I am unsure of how to add tests for this. > I could continue working too polish this off with instructions from > others if it is desired for the main line. Thanks for the patch. This will need documentation added under `docs/system/generic-loader.rst` so that people know how to use it and what it does > > Signed-off-by: Sam Price <thesamprice@gmail.com> > --- > > --- > hw/core/generic-loader.c | 28 ++++++++++++++++++++++++++++ > include/hw/core/generic-loader.h | 6 +++++- > roms/SLOF | 2 +- > roms/edk2 | 2 +- > roms/openbios | 2 +- > roms/opensbi | 2 +- > roms/seabios | 2 +- > roms/seabios-hppa | 2 +- > tests/lcitool/libvirt-ci | 2 +- > 9 files changed, 40 insertions(+), 8 deletions(-) > > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > index ea8628b892..ebda8ac43f 100644 > --- a/hw/core/generic-loader.c > +++ b/hw/core/generic-loader.c > @@ -55,6 +55,19 @@ static void generic_loader_reset(void *opaque) > } > } > > + for (int i = 0; i < 31; i++) { Why 31? I'm guessing you picked this because that's how many registers your architecture supports, but that's not the same for all architectures > + if (s->has_register_defaults[i]) { > + CPUClass *cc = CPU_GET_CLASS(s->cpu); > + uint8_t buf[sizeof(uint64_t)]; > + memcpy(buf, &s->register_defaults[i], sizeof(uint64_t)); > + if (cc && cc->gdb_write_register) { > + cc->gdb_write_register(s->cpu, buf, i); > + } > + } > + } > + > + > + > if (s->data_len) { > assert(s->data_len <= sizeof(s->data)); > dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, > @@ -172,6 +185,20 @@ static void generic_loader_realize(DeviceState > *dev, Error **errp) > } else { > s->data = cpu_to_le64(s->data); > } > + > + /* Store the CPU register default if specified */ > + if (s->reg) { > + int reg_num; > + if (sscanf(s->reg, "r%d", ®_num) == 1 && > + reg_num >= 0 && reg_num < 31) { I don't think all architectures call their registers r* and they don't all have 31 registers > + s->register_defaults[reg_num] = s->data; > + s->has_register_defaults[reg_num] = true; > + } else { > + error_setg(errp, "Unsupported register: %s", s->reg); > + return; > + } > + } > + > } > > static void generic_loader_unrealize(DeviceState *dev) > @@ -186,6 +213,7 @@ static Property generic_loader_props[] = { > DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false), > DEFINE_PROP_UINT32("cpu-num", GenericLoaderState, cpu_num, CPU_NONE), > DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false), > + DEFINE_PROP_STRING("reg", GenericLoaderState, reg), > DEFINE_PROP_STRING("file", GenericLoaderState, file), > DEFINE_PROP_END_OF_LIST(), > }; > diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h > index 19d87b39c8..d81e1632fd 100644 > --- a/include/hw/core/generic-loader.h > +++ b/include/hw/core/generic-loader.h > @@ -35,10 +35,14 @@ struct GenericLoaderState { > uint32_t cpu_num; > > char *file; > - > + char *reg; > bool force_raw; > bool data_be; > bool set_pc; > + > + /* Add an array for storing default register values */ > + bool has_register_defaults[31]; /* Track if a default value is provided */ > + uint64_t register_defaults[31]; /* Default values for registers r0-r30 */ > }; > > #define TYPE_GENERIC_LOADER "loader" > diff --git a/roms/SLOF b/roms/SLOF > index 3a259df244..6b6c16b4b4 160000 > --- a/roms/SLOF > +++ b/roms/SLOF > @@ -1 +1 @@ > -Subproject commit 3a259df2449fc4a4e43ab5f33f0b2c66484b4bc3 > +Subproject commit 6b6c16b4b40763507cf1f518096f3c3883c5cf2d > diff --git a/roms/edk2 b/roms/edk2 > index 4dfdca63a9..f80f052277 160000 > --- a/roms/edk2 > +++ b/roms/edk2 > @@ -1 +1 @@ > -Subproject commit 4dfdca63a93497203f197ec98ba20e2327e4afe4 > +Subproject commit f80f052277c88a67c55e107b550f504eeea947d3 > diff --git a/roms/openbios b/roms/openbios > index c3a19c1e54..af97fd7af5 160000 > --- a/roms/openbios > +++ b/roms/openbios > @@ -1 +1 @@ > -Subproject commit c3a19c1e54977a53027d6232050e1e3e39a98a1b > +Subproject commit af97fd7af5e7c18f591a7b987291d3db4ffb28b5 > diff --git a/roms/opensbi b/roms/opensbi > index 43cace6c36..057eb10b6d 160000 > --- a/roms/opensbi > +++ b/roms/opensbi > @@ -1 +1 @@ > -Subproject commit 43cace6c3671e5172d0df0a8963e552bb04b7b20 > +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 > diff --git a/roms/seabios b/roms/seabios > index a6ed6b701f..ea1b7a0733 160000 > --- a/roms/seabios > +++ b/roms/seabios > @@ -1 +1 @@ > -Subproject commit a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 > +Subproject commit ea1b7a0733906b8425d948ae94fba63c32b1d425 > diff --git a/roms/seabios-hppa b/roms/seabios-hppa > index a528f01d7a..673d2595d4 160000 > --- a/roms/seabios-hppa > +++ b/roms/seabios-hppa > @@ -1 +1 @@ > -Subproject commit a528f01d7abd511d3cc71b7acaab6e036ee524bd > +Subproject commit 673d2595d4f773cc266cbf8dbaf2f475a6adb949 > diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci > index 9ad3f70bde..9bff3b763b 160000 > --- a/tests/lcitool/libvirt-ci > +++ b/tests/lcitool/libvirt-ci > @@ -1 +1 @@ > -Subproject commit 9ad3f70bde9865d5ad18f36d256d472e72b5cbf3 > +Subproject commit 9bff3b763b5531a1490e238bfbf77306dc3a6dbb Did you mean to change all of these submodules? Alistair > -- > 2.45.2 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-06 4:41 ` Alistair Francis @ 2025-01-06 4:59 ` Sam Price 2025-01-06 5:13 ` Alistair Francis 0 siblings, 1 reply; 12+ messages in thread From: Sam Price @ 2025-01-06 4:59 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel, alistair I didn't mean to change all of the submodules. I was somewhat porting from xilinx-qemu over to the main line, and messed up the commit on that. Ill get my gitlab branch fixed up on the next commit. I am horrible at this email part. I could malloc memory and push all of the register names/ values into a queue, and then call the backend gdb routines on the number of calls. I will rely on the gdb routine for doing validation of both the name / value arguments. I will make an implementation of this using the include/qemue/queue.h to store all passed in register arguments. https://gitlab.com/thesamprice/qemu/-/blob/master/include/qemu/queue.h If there is a better datastructure I should use for implementation let me know. Ill also update docs/system/generic-loader.rst Are unit tests needed? Any guidance on what you would want done for this would be appreciated. Thanks, Sam On Sun, Jan 5, 2025 at 11:41 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Fri, Dec 6, 2024 at 1:30 PM Sam Price <thesamprice@gmail.com> wrote: > > > > I needed to set the registers prior to boot up to mimic what uboot > > would do prior to loading a binary. This adds a generic option of reg > > to the loader command, it uses the existing gcc commands for setting > > register values. > > > > I'm sorry I couldn't figure out how to work the git send-email > > properly. Configuring it with my gmail became too cumbersome, and my > > work email was also challenging to configure. > > > > I have the file staged here. > > https://gitlab.com/thesamprice/qemu/-/tree/loader?ref_type=heads > > I am unsure of how to add tests for this. > > I could continue working too polish this off with instructions from > > others if it is desired for the main line. > > Thanks for the patch. This will need documentation added under > `docs/system/generic-loader.rst` so that people know how to use it and > what it does > > > > > Signed-off-by: Sam Price <thesamprice@gmail.com> > > --- > > > > --- > > hw/core/generic-loader.c | 28 ++++++++++++++++++++++++++++ > > include/hw/core/generic-loader.h | 6 +++++- > > roms/SLOF | 2 +- > > roms/edk2 | 2 +- > > roms/openbios | 2 +- > > roms/opensbi | 2 +- > > roms/seabios | 2 +- > > roms/seabios-hppa | 2 +- > > tests/lcitool/libvirt-ci | 2 +- > > 9 files changed, 40 insertions(+), 8 deletions(-) > > > > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > > index ea8628b892..ebda8ac43f 100644 > > --- a/hw/core/generic-loader.c > > +++ b/hw/core/generic-loader.c > > @@ -55,6 +55,19 @@ static void generic_loader_reset(void *opaque) > > } > > } > > > > + for (int i = 0; i < 31; i++) { > > Why 31? > > I'm guessing you picked this because that's how many registers your > architecture supports, but that's not the same for all architectures > > > + if (s->has_register_defaults[i]) { > > + CPUClass *cc = CPU_GET_CLASS(s->cpu); > > + uint8_t buf[sizeof(uint64_t)]; > > + memcpy(buf, &s->register_defaults[i], sizeof(uint64_t)); > > + if (cc && cc->gdb_write_register) { > > + cc->gdb_write_register(s->cpu, buf, i); > > + } > > + } > > + } > > + > > + > > + > > if (s->data_len) { > > assert(s->data_len <= sizeof(s->data)); > > dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, > > @@ -172,6 +185,20 @@ static void generic_loader_realize(DeviceState > > *dev, Error **errp) > > } else { > > s->data = cpu_to_le64(s->data); > > } > > + > > + /* Store the CPU register default if specified */ > > + if (s->reg) { > > + int reg_num; > > + if (sscanf(s->reg, "r%d", ®_num) == 1 && > > + reg_num >= 0 && reg_num < 31) { > > I don't think all architectures call their registers r* and they don't > all have 31 registers > > > + s->register_defaults[reg_num] = s->data; > > + s->has_register_defaults[reg_num] = true; > > + } else { > > + error_setg(errp, "Unsupported register: %s", s->reg); > > + return; > > + } > > + } > > + > > } > > > > static void generic_loader_unrealize(DeviceState *dev) > > @@ -186,6 +213,7 @@ static Property generic_loader_props[] = { > > DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false), > > DEFINE_PROP_UINT32("cpu-num", GenericLoaderState, cpu_num, CPU_NONE), > > DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false), > > + DEFINE_PROP_STRING("reg", GenericLoaderState, reg), > > DEFINE_PROP_STRING("file", GenericLoaderState, file), > > DEFINE_PROP_END_OF_LIST(), > > }; > > diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h > > index 19d87b39c8..d81e1632fd 100644 > > --- a/include/hw/core/generic-loader.h > > +++ b/include/hw/core/generic-loader.h > > @@ -35,10 +35,14 @@ struct GenericLoaderState { > > uint32_t cpu_num; > > > > char *file; > > - > > + char *reg; > > bool force_raw; > > bool data_be; > > bool set_pc; > > + > > + /* Add an array for storing default register values */ > > + bool has_register_defaults[31]; /* Track if a default value is provided */ > > + uint64_t register_defaults[31]; /* Default values for registers r0-r30 */ > > }; > > > > #define TYPE_GENERIC_LOADER "loader" > > diff --git a/roms/SLOF b/roms/SLOF > > index 3a259df244..6b6c16b4b4 160000 > > --- a/roms/SLOF > > +++ b/roms/SLOF > > @@ -1 +1 @@ > > -Subproject commit 3a259df2449fc4a4e43ab5f33f0b2c66484b4bc3 > > +Subproject commit 6b6c16b4b40763507cf1f518096f3c3883c5cf2d > > diff --git a/roms/edk2 b/roms/edk2 > > index 4dfdca63a9..f80f052277 160000 > > --- a/roms/edk2 > > +++ b/roms/edk2 > > @@ -1 +1 @@ > > -Subproject commit 4dfdca63a93497203f197ec98ba20e2327e4afe4 > > +Subproject commit f80f052277c88a67c55e107b550f504eeea947d3 > > diff --git a/roms/openbios b/roms/openbios > > index c3a19c1e54..af97fd7af5 160000 > > --- a/roms/openbios > > +++ b/roms/openbios > > @@ -1 +1 @@ > > -Subproject commit c3a19c1e54977a53027d6232050e1e3e39a98a1b > > +Subproject commit af97fd7af5e7c18f591a7b987291d3db4ffb28b5 > > diff --git a/roms/opensbi b/roms/opensbi > > index 43cace6c36..057eb10b6d 160000 > > --- a/roms/opensbi > > +++ b/roms/opensbi > > @@ -1 +1 @@ > > -Subproject commit 43cace6c3671e5172d0df0a8963e552bb04b7b20 > > +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 > > diff --git a/roms/seabios b/roms/seabios > > index a6ed6b701f..ea1b7a0733 160000 > > --- a/roms/seabios > > +++ b/roms/seabios > > @@ -1 +1 @@ > > -Subproject commit a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 > > +Subproject commit ea1b7a0733906b8425d948ae94fba63c32b1d425 > > diff --git a/roms/seabios-hppa b/roms/seabios-hppa > > index a528f01d7a..673d2595d4 160000 > > --- a/roms/seabios-hppa > > +++ b/roms/seabios-hppa > > @@ -1 +1 @@ > > -Subproject commit a528f01d7abd511d3cc71b7acaab6e036ee524bd > > +Subproject commit 673d2595d4f773cc266cbf8dbaf2f475a6adb949 > > diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci > > index 9ad3f70bde..9bff3b763b 160000 > > --- a/tests/lcitool/libvirt-ci > > +++ b/tests/lcitool/libvirt-ci > > @@ -1 +1 @@ > > -Subproject commit 9ad3f70bde9865d5ad18f36d256d472e72b5cbf3 > > +Subproject commit 9bff3b763b5531a1490e238bfbf77306dc3a6dbb > > Did you mean to change all of these submodules? > > > Alistair > > > -- > > 2.45.2 > > -- Sincerely, Sam Price ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-06 4:59 ` Sam Price @ 2025-01-06 5:13 ` Alistair Francis 2025-01-08 2:28 ` Sam Price 0 siblings, 1 reply; 12+ messages in thread From: Alistair Francis @ 2025-01-06 5:13 UTC (permalink / raw) To: Sam Price; +Cc: qemu-devel, alistair On Mon, Jan 6, 2025 at 2:59 PM Sam Price <thesamprice@gmail.com> wrote: > > I didn't mean to change all of the submodules. > I was somewhat porting from xilinx-qemu over to the main line, and > messed up the commit on that. > Ill get my gitlab branch fixed up on the next commit. > I am horrible at this email part. > > I could malloc memory and push all of the register names/ values into > a queue, and then call the backend gdb routines on the number of > calls. So GDB uses the XML files in `gdb-xml` for the register information. Considering that each architecture uses a different register naming structure I think you are going to have a hard time doing this manually. The best bet is probably just to use `cc->gdb_num_core_regs` to get the number of regs and then use integer offsets (so no strings, just 0, 1, 2...) for the registers. It's still not ideal, but might be good enough > I will rely on the gdb routine for doing validation of both the name / > value arguments. If there is a way to do this with the GDB stub that might also work well > > I will make an implementation of this using the include/qemue/queue.h > to store all passed in register arguments. > https://gitlab.com/thesamprice/qemu/-/blob/master/include/qemu/queue.h > If there is a better datastructure I should use for implementation let me know. I don't follow why you need a queue, but I might just be misunderstanding. > > Ill also update docs/system/generic-loader.rst Yes please > > Are unit tests needed? I think for now I wouldn't worry about it Alistair > Any guidance on what you would want done for this would be appreciated. > > Thanks, > Sam > > On Sun, Jan 5, 2025 at 11:41 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Fri, Dec 6, 2024 at 1:30 PM Sam Price <thesamprice@gmail.com> wrote: > > > > > > I needed to set the registers prior to boot up to mimic what uboot > > > would do prior to loading a binary. This adds a generic option of reg > > > to the loader command, it uses the existing gcc commands for setting > > > register values. > > > > > > I'm sorry I couldn't figure out how to work the git send-email > > > properly. Configuring it with my gmail became too cumbersome, and my > > > work email was also challenging to configure. > > > > > > I have the file staged here. > > > https://gitlab.com/thesamprice/qemu/-/tree/loader?ref_type=heads > > > I am unsure of how to add tests for this. > > > I could continue working too polish this off with instructions from > > > others if it is desired for the main line. > > > > Thanks for the patch. This will need documentation added under > > `docs/system/generic-loader.rst` so that people know how to use it and > > what it does > > > > > > > > Signed-off-by: Sam Price <thesamprice@gmail.com> > > > --- > > > > > > --- > > > hw/core/generic-loader.c | 28 ++++++++++++++++++++++++++++ > > > include/hw/core/generic-loader.h | 6 +++++- > > > roms/SLOF | 2 +- > > > roms/edk2 | 2 +- > > > roms/openbios | 2 +- > > > roms/opensbi | 2 +- > > > roms/seabios | 2 +- > > > roms/seabios-hppa | 2 +- > > > tests/lcitool/libvirt-ci | 2 +- > > > 9 files changed, 40 insertions(+), 8 deletions(-) > > > > > > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > > > index ea8628b892..ebda8ac43f 100644 > > > --- a/hw/core/generic-loader.c > > > +++ b/hw/core/generic-loader.c > > > @@ -55,6 +55,19 @@ static void generic_loader_reset(void *opaque) > > > } > > > } > > > > > > + for (int i = 0; i < 31; i++) { > > > > Why 31? > > > > I'm guessing you picked this because that's how many registers your > > architecture supports, but that's not the same for all architectures > > > > > + if (s->has_register_defaults[i]) { > > > + CPUClass *cc = CPU_GET_CLASS(s->cpu); > > > + uint8_t buf[sizeof(uint64_t)]; > > > + memcpy(buf, &s->register_defaults[i], sizeof(uint64_t)); > > > + if (cc && cc->gdb_write_register) { > > > + cc->gdb_write_register(s->cpu, buf, i); > > > + } > > > + } > > > + } > > > + > > > + > > > + > > > if (s->data_len) { > > > assert(s->data_len <= sizeof(s->data)); > > > dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, > > > @@ -172,6 +185,20 @@ static void generic_loader_realize(DeviceState > > > *dev, Error **errp) > > > } else { > > > s->data = cpu_to_le64(s->data); > > > } > > > + > > > + /* Store the CPU register default if specified */ > > > + if (s->reg) { > > > + int reg_num; > > > + if (sscanf(s->reg, "r%d", ®_num) == 1 && > > > + reg_num >= 0 && reg_num < 31) { > > > > I don't think all architectures call their registers r* and they don't > > all have 31 registers > > > > > + s->register_defaults[reg_num] = s->data; > > > + s->has_register_defaults[reg_num] = true; > > > + } else { > > > + error_setg(errp, "Unsupported register: %s", s->reg); > > > + return; > > > + } > > > + } > > > + > > > } > > > > > > static void generic_loader_unrealize(DeviceState *dev) > > > @@ -186,6 +213,7 @@ static Property generic_loader_props[] = { > > > DEFINE_PROP_BOOL("data-be", GenericLoaderState, data_be, false), > > > DEFINE_PROP_UINT32("cpu-num", GenericLoaderState, cpu_num, CPU_NONE), > > > DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false), > > > + DEFINE_PROP_STRING("reg", GenericLoaderState, reg), > > > DEFINE_PROP_STRING("file", GenericLoaderState, file), > > > DEFINE_PROP_END_OF_LIST(), > > > }; > > > diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h > > > index 19d87b39c8..d81e1632fd 100644 > > > --- a/include/hw/core/generic-loader.h > > > +++ b/include/hw/core/generic-loader.h > > > @@ -35,10 +35,14 @@ struct GenericLoaderState { > > > uint32_t cpu_num; > > > > > > char *file; > > > - > > > + char *reg; > > > bool force_raw; > > > bool data_be; > > > bool set_pc; > > > + > > > + /* Add an array for storing default register values */ > > > + bool has_register_defaults[31]; /* Track if a default value is provided */ > > > + uint64_t register_defaults[31]; /* Default values for registers r0-r30 */ > > > }; > > > > > > #define TYPE_GENERIC_LOADER "loader" > > > diff --git a/roms/SLOF b/roms/SLOF > > > index 3a259df244..6b6c16b4b4 160000 > > > --- a/roms/SLOF > > > +++ b/roms/SLOF > > > @@ -1 +1 @@ > > > -Subproject commit 3a259df2449fc4a4e43ab5f33f0b2c66484b4bc3 > > > +Subproject commit 6b6c16b4b40763507cf1f518096f3c3883c5cf2d > > > diff --git a/roms/edk2 b/roms/edk2 > > > index 4dfdca63a9..f80f052277 160000 > > > --- a/roms/edk2 > > > +++ b/roms/edk2 > > > @@ -1 +1 @@ > > > -Subproject commit 4dfdca63a93497203f197ec98ba20e2327e4afe4 > > > +Subproject commit f80f052277c88a67c55e107b550f504eeea947d3 > > > diff --git a/roms/openbios b/roms/openbios > > > index c3a19c1e54..af97fd7af5 160000 > > > --- a/roms/openbios > > > +++ b/roms/openbios > > > @@ -1 +1 @@ > > > -Subproject commit c3a19c1e54977a53027d6232050e1e3e39a98a1b > > > +Subproject commit af97fd7af5e7c18f591a7b987291d3db4ffb28b5 > > > diff --git a/roms/opensbi b/roms/opensbi > > > index 43cace6c36..057eb10b6d 160000 > > > --- a/roms/opensbi > > > +++ b/roms/opensbi > > > @@ -1 +1 @@ > > > -Subproject commit 43cace6c3671e5172d0df0a8963e552bb04b7b20 > > > +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 > > > diff --git a/roms/seabios b/roms/seabios > > > index a6ed6b701f..ea1b7a0733 160000 > > > --- a/roms/seabios > > > +++ b/roms/seabios > > > @@ -1 +1 @@ > > > -Subproject commit a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 > > > +Subproject commit ea1b7a0733906b8425d948ae94fba63c32b1d425 > > > diff --git a/roms/seabios-hppa b/roms/seabios-hppa > > > index a528f01d7a..673d2595d4 160000 > > > --- a/roms/seabios-hppa > > > +++ b/roms/seabios-hppa > > > @@ -1 +1 @@ > > > -Subproject commit a528f01d7abd511d3cc71b7acaab6e036ee524bd > > > +Subproject commit 673d2595d4f773cc266cbf8dbaf2f475a6adb949 > > > diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci > > > index 9ad3f70bde..9bff3b763b 160000 > > > --- a/tests/lcitool/libvirt-ci > > > +++ b/tests/lcitool/libvirt-ci > > > @@ -1 +1 @@ > > > -Subproject commit 9ad3f70bde9865d5ad18f36d256d472e72b5cbf3 > > > +Subproject commit 9bff3b763b5531a1490e238bfbf77306dc3a6dbb > > > > Did you mean to change all of these submodules? > > > > > > Alistair > > > > > -- > > > 2.45.2 > > > > > > > -- > Sincerely, > > Sam Price ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-06 5:13 ` Alistair Francis @ 2025-01-08 2:28 ` Sam Price 2025-01-10 0:33 ` Alistair Francis 2025-01-10 9:53 ` Philippe Mathieu-Daudé 0 siblings, 2 replies; 12+ messages in thread From: Sam Price @ 2025-01-08 2:28 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel, alistair I made the changes, and added documentation. https://gitlab.com/thesamprice/qemu/-/compare/master...loader?from_project_id=11167699 I left it as [PREFIX]<RegNumber> I can switch this to just RegNumber if desired. I am still struggling with the email format sorry. --- docs/system/generic-loader.rst | 98 ++++++++++++++++++++++++++++++++ hw/core/generic-loader.c | 46 +++++++++++---- include/hw/core/generic-loader.h | 7 +++ 3 files changed, 139 insertions(+), 12 deletions(-) diff --git a/docs/system/generic-loader.rst b/docs/system/generic-loader.rst index 4f9fb005f1..71d4aaa097 100644 --- a/docs/system/generic-loader.rst +++ b/docs/system/generic-loader.rst @@ -117,4 +117,102 @@ future the internal state 'set_pc' (which exists in the generic loader now) should be exposed to the user so that they can choose if the PC is set or not. +Loading Data into Registers +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The `loader` device allows the initialization of CPU registers from the command +line. This feature is particularly useful for setting up the processor state +before starting an executable. By configuring registers prior to execution, the +`loader` can mimic the state that a bootloader would leave the processor in +before transferring control to an ELF file or another executable. + +The syntax for loading data into registers is as follows:: + + -device loader,reg=<reg>,data=<data>,data-len=<data-len> + +**Parameters:** + +``<reg>`` + The target register to set. Format must pass the following regex + ``[a-zA-Z]+[0-9]+``. The numeric part corresponds to the processor's GDB \ + register index. For general-purpose registers, this is typically the + number in the register's name (e.g., ``r5`` translates to ``5``). + Special-purpose registers have specific IDs defined in their processor's + `gdbstub.c` file. Note that these IDs vary between processors. + +``<data>`` + The value to load into the specified register. The data must not exceed 8 + bytes in size. + +``<data-len>`` + The length of the data in bytes. This parameter is mandatory when using + the ``data`` argument. + +**Examples:** + +Set a general-purpose register +"""""""""""""""""""""""""""""" + +To set register ``r5`` to ``0xc0001000`` (4 bytes) on CPU 0:: + + -device loader,reg=r5,data=0xc0001000,data-len=4 + +Set a special register +"""""""""""""""""""""" + +To set the Program Counter (PC, register ``32``) to ``0x80000000`` on CPU 0:: + + -device loader,reg=pc32,data=0x80000000,data-len=4 + +You must look in your processor's `gdbstub.c` file to special register to index +mappings. + +**Special Registers:** + +Special registers are defined in the processor's ``gdbstub.c`` file with numeric IDs. +Examples from the MicroBlaze processor at one point looked like. include:: + + enum { + GDB_PC = 32 + 0, + GDB_MSR = 32 + 1, + GDB_EAR = 32 + 2, + GDB_ESR = 32 + 3, + GDB_FSR = 32 + 4, + GDB_BTR = 32 + 5, + GDB_PVR0 = 32 + 6, + GDB_PVR11 = 32 + 17, + GDB_EDR = 32 + 18, + GDB_SLR = 32 + 25, + GDB_SHR = 32 + 26, + }; + +For example, to set the Machine State Register (``GDB_MSR``) on a MicroBlaze +processor:: + + -device loader,reg=MSR33,data=0x00000001,data-len=4 + +**Register Loading Notes:** + +1. **Processor-Specific IDs**: + The numeric IDs for registers vary between processors. Always refer to the + `gdbstub.c` file for the target processor to identify the correct register + mappings. + +2. **Pre-Execution State**: + This capability is ideal for initializing a simulated environment to match + the state expected by an ELF file. For example, you can configure stack + pointers, machine state registers, and program counters to prepare the + processor to run a bootstrapped application. + +3. **Validation**: + Register numbers are validated by the `gdb_write_register` function. Ensure + the specified register is supported by the target architecture. + +4. **Endianess**: + The `data` value is written using the processor's native endian format. + +By using the `loader` device to initialize registers, you can simulate +realistic execution environments, enabling detailed testing and debugging +of embedded software, including bootloaders interactions and operating +system kernels. diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c index ea8628b892..9408ecd150 100644 --- a/hw/core/generic-loader.c +++ b/hw/core/generic-loader.c @@ -55,6 +55,14 @@ static void generic_loader_reset(void *opaque) } } + if(s->reg.name) { + CPUClass *cc = CPU_GET_CLASS(s->cpu); + int bytes_written = cc->gdb_write_register(s->cpu, (uint8_t*) &s->reg.value, s->reg.num); + if(bytes_written != s->reg.data_len) { + printf("Error setting register %d to value %lX expected to write %d, but wrote %d\n", s->reg.num, s->reg.value, s->reg.data_len, bytes_written); + } + } + if (s->data_len) { assert(s->data_len <= sizeof(s->data)); dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len, @@ -89,14 +97,12 @@ static void generic_loader_realize(DeviceState *dev, Error **errp) } else if (s->data_len > 8) { error_setg(errp, "data-len cannot be greater then 8 bytes"); return; - } - } else if (s->file || s->force_raw) { - /* User is loading an image */ - if (s->data || s->data_len || s->data_be) { - error_setg(errp, "data can not be specified when loading an " - "image"); + }else if (s->addr) { + error_setg(errp, "data can not be specified when setting a " + "program counter"); return; } + } else if (s->file || s->force_raw) { /* The user specified a file, only set the PC if they also specified * a CPU to use. */ @@ -105,17 +111,13 @@ static void generic_loader_realize(DeviceState *dev, Error **errp) } } else if (s->addr) { /* User is setting the PC */ - if (s->data || s->data_len || s->data_be) { - error_setg(errp, "data can not be specified when setting a " - "program counter"); - return; - } else if (s->cpu_num == CPU_NONE) { + if (s->cpu_num == CPU_NONE) { error_setg(errp, "cpu_num must be specified when setting a " "program counter"); return; } s->set_pc = true; - } else { + } else { /* Did the user specify anything? */ error_setg(errp, "please include valid arguments"); return; @@ -166,6 +168,25 @@ static void generic_loader_realize(DeviceState *dev, Error **errp) } } + if (s->reg.name) { + int reg_num; + CPUClass *cc = CPU_GET_CLASS(s->cpu); + char prefix[32]; /* Read up to 32 characters prior to the register number*/ + int scan_num = sscanf(s->reg.name, "%31[a-zA-Z]%d",prefix, ®_num); + if ( scan_num != 2){ + error_setg(errp, "Unsupported register: %s, Failed to deterimine register number", s->reg.name); + s->reg.name = 0x0; + }else if( reg_num < 0 || cc->gdb_num_core_regs < reg_num){ + error_setg(errp, "Unsupported register: %s, register number must be less than %d, got %d", s->reg.name, cc->gdb_num_core_regs, reg_num); + s->reg.name = 0x0; + }else{ + s->reg.value = s->data; + s->reg.num = reg_num; + s->reg.data_len = s->data_len; + s->data_len = 0; + } + } + /* Convert the data endianness */ if (s->data_be) { s->data = cpu_to_be64(s->data); @@ -187,6 +208,7 @@ static Property generic_loader_props[] = { DEFINE_PROP_UINT32("cpu-num", GenericLoaderState, cpu_num, CPU_NONE), DEFINE_PROP_BOOL("force-raw", GenericLoaderState, force_raw, false), DEFINE_PROP_STRING("file", GenericLoaderState, file), + DEFINE_PROP_STRING("reg", GenericLoaderState, reg.name), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h index 19d87b39c8..ba826806d3 100644 --- a/include/hw/core/generic-loader.h +++ b/include/hw/core/generic-loader.h @@ -39,6 +39,13 @@ struct GenericLoaderState { bool force_raw; bool data_be; bool set_pc; + + struct { + char * name; + int num; + int data_len; + uint64_t value; + } reg; }; #define TYPE_GENERIC_LOADER "loader" -- 2.45.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-08 2:28 ` Sam Price @ 2025-01-10 0:33 ` Alistair Francis 2025-01-10 5:33 ` Sam Price 2025-01-10 9:53 ` Philippe Mathieu-Daudé 1 sibling, 1 reply; 12+ messages in thread From: Alistair Francis @ 2025-01-10 0:33 UTC (permalink / raw) To: Sam Price; +Cc: qemu-devel, alistair On Wed, Jan 8, 2025 at 12:28 PM Sam Price <thesamprice@gmail.com> wrote: > > I made the changes, and added documentation. > https://gitlab.com/thesamprice/qemu/-/compare/master...loader?from_project_id=11167699 > > I left it as [PREFIX]<RegNumber> > > I can switch this to just RegNumber if desired. > > I am still struggling with the email format sorry. > --- > docs/system/generic-loader.rst | 98 ++++++++++++++++++++++++++++++++ > hw/core/generic-loader.c | 46 +++++++++++---- > include/hw/core/generic-loader.h | 7 +++ > 3 files changed, 139 insertions(+), 12 deletions(-) > > diff --git a/docs/system/generic-loader.rst b/docs/system/generic-loader.rst > index 4f9fb005f1..71d4aaa097 100644 > --- a/docs/system/generic-loader.rst > +++ b/docs/system/generic-loader.rst > @@ -117,4 +117,102 @@ future the internal state 'set_pc' (which exists > in the generic loader > now) should be exposed to the user so that they can choose if the PC > is set or not. > +Loading Data into Registers > +^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +The `loader` device allows the initialization of CPU registers from the command > +line. This feature is particularly useful for setting up the processor state > +before starting an executable. By configuring registers prior to execution, the > +`loader` can mimic the state that a bootloader would leave the processor in > +before transferring control to an ELF file or another executable. This isn't really true though. A bootloader generally will set more than the GP registers. A boot loader will configure devices and perhaps initalise memory. > + > +The syntax for loading data into registers is as follows:: > + > + -device loader,reg=<reg>,data=<data>,data-len=<data-len> > + > +**Parameters:** > + > +``<reg>`` > + The target register to set. Format must pass the following regex > + ``[a-zA-Z]+[0-9]+``. The numeric part corresponds to the processor's GDB \ > + register index. For general-purpose registers, this is typically the > + number in the register's name (e.g., ``r5`` translates to ``5``). > + Special-purpose registers have specific IDs defined in their processor's > + `gdbstub.c` file. Note that these IDs vary between processors. > + > +``<data>`` > + The value to load into the specified register. The data must not exceed 8 > + bytes in size. Why 8 bytes? > + > +``<data-len>`` > + The length of the data in bytes. This parameter is mandatory when using > + the ``data`` argument. Do we need data-len? Why not just use the register size > + > +**Examples:** > + > +Set a general-purpose register > +"""""""""""""""""""""""""""""" > + > +To set register ``r5`` to ``0xc0001000`` (4 bytes) on CPU 0:: > + > + -device loader,reg=r5,data=0xc0001000,data-len=4 > + > +Set a special register > +"""""""""""""""""""""" > + > +To set the Program Counter (PC, register ``32``) to ``0x80000000`` on CPU 0:: > + > + -device loader,reg=pc32,data=0x80000000,data-len=4 > + > +You must look in your processor's `gdbstub.c` file to special register to index > +mappings. That isn't really helpful for users, but I don't have a better idea > + > +**Special Registers:** > + > +Special registers are defined in the processor's ``gdbstub.c`` file > with numeric IDs. > +Examples from the MicroBlaze processor at one point looked like. include:: > + > + enum { > + GDB_PC = 32 + 0, > + GDB_MSR = 32 + 1, > + GDB_EAR = 32 + 2, > + GDB_ESR = 32 + 3, > + GDB_FSR = 32 + 4, > + GDB_BTR = 32 + 5, > + GDB_PVR0 = 32 + 6, > + GDB_PVR11 = 32 + 17, > + GDB_EDR = 32 + 18, > + GDB_SLR = 32 + 25, > + GDB_SHR = 32 + 26, > + }; > + > +For example, to set the Machine State Register (``GDB_MSR``) on a MicroBlaze > +processor:: > + > + -device loader,reg=MSR33,data=0x00000001,data-len=4 > + > +**Register Loading Notes:** > + > +1. **Processor-Specific IDs**: > + The numeric IDs for registers vary between processors. Always refer to the > + `gdbstub.c` file for the target processor to identify the correct register > + mappings. > + > +2. **Pre-Execution State**: > + This capability is ideal for initializing a simulated environment to match > + the state expected by an ELF file. For example, you can configure stack > + pointers, machine state registers, and program counters to prepare the > + processor to run a bootstrapped application. > + > +3. **Validation**: > + Register numbers are validated by the `gdb_write_register` function. Ensure > + the specified register is supported by the target architecture. > + > +4. **Endianess**: > + The `data` value is written using the processor's native endian format. > + > +By using the `loader` device to initialize registers, you can simulate > +realistic execution environments, enabling detailed testing and debugging > +of embedded software, including bootloaders interactions and operating > +system kernels. > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > index ea8628b892..9408ecd150 100644 > --- a/hw/core/generic-loader.c > +++ b/hw/core/generic-loader.c > @@ -55,6 +55,14 @@ static void generic_loader_reset(void *opaque) > } > } > + if(s->reg.name) { > + CPUClass *cc = CPU_GET_CLASS(s->cpu); > + int bytes_written = cc->gdb_write_register(s->cpu, (uint8_t*) > &s->reg.value, s->reg.num); > + if(bytes_written != s->reg.data_len) { > + printf("Error setting register %d to value %lX expected to write %d, > but wrote %d\n", s->reg.num, s->reg.value, s->reg.data_len, > bytes_written); The line wrapping is muddled up here. Can you please send it with git send-email. Do some sends against yourself to make sure it works. You mentioned gmail in an earlier thread I think, did you follow the instructions: https://git-scm.com/docs/git-send-email#_use_gmail_as_the_smtp_server Alistair ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-10 0:33 ` Alistair Francis @ 2025-01-10 5:33 ` Sam Price 2025-01-31 0:27 ` Alistair Francis 0 siblings, 1 reply; 12+ messages in thread From: Sam Price @ 2025-01-10 5:33 UTC (permalink / raw) To: Alistair Francis; +Cc: qemu-devel, alistair Yes that is true a boot loader will do more than just set registers. Ill rework the text a bit on the next update. In my case i need to set the r5 register that specifies the memory location to the device tree. I also use the device loader to load in a elf file to ram, and the device-loader to load in the device tree to the location specified by the r5 register I could add a gdb call that would return an array of string mappings to integers. If the machine doesn't implement the function/ leaves it as null pointer then you wouldn't get the cli support. Not sure where you would document all the machine register names / numbers at though. This might be too much though? I left the door somewhat open on this via the NAME_NUMBER format. There was some checking logic where if data is supplied then it forces a check for data-len. I could relax that check if you supply the reg.name field. I am unsure how to determine the machine register size. I assumed the max register size on any machine would be 8 bytes, this might be wrong. the gdb call seems to just pass in the full 8 bytes, but I didn't dig into it for all machines. Ill look at this a bit more and try to configure the git email. I also need to set up a docker container to build /test latest. I have been building / testing on an old ubuntu machine. (To test this I need to run it on qemu-xilinx). My workplace has us on ubuntu 20. So it might be a while before I have another version up. Thanks, Sam On Thu, Jan 9, 2025 at 7:34 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Wed, Jan 8, 2025 at 12:28 PM Sam Price <thesamprice@gmail.com> wrote: > > > > I made the changes, and added documentation. > > https://gitlab.com/thesamprice/qemu/-/compare/master...loader?from_project_id=11167699 > > > > I left it as [PREFIX]<RegNumber> > > > > I can switch this to just RegNumber if desired. > > > > I am still struggling with the email format sorry. > > --- > > docs/system/generic-loader.rst | 98 ++++++++++++++++++++++++++++++++ > > hw/core/generic-loader.c | 46 +++++++++++---- > > include/hw/core/generic-loader.h | 7 +++ > > 3 files changed, 139 insertions(+), 12 deletions(-) > > > > diff --git a/docs/system/generic-loader.rst b/docs/system/generic-loader.rst > > index 4f9fb005f1..71d4aaa097 100644 > > --- a/docs/system/generic-loader.rst > > +++ b/docs/system/generic-loader.rst > > @@ -117,4 +117,102 @@ future the internal state 'set_pc' (which exists > > in the generic loader > > now) should be exposed to the user so that they can choose if the PC > > is set or not. > > +Loading Data into Registers > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > + > > +The `loader` device allows the initialization of CPU registers from the command > > +line. This feature is particularly useful for setting up the processor state > > +before starting an executable. By configuring registers prior to execution, the > > +`loader` can mimic the state that a bootloader would leave the processor in > > +before transferring control to an ELF file or another executable. > > This isn't really true though. A bootloader generally will set more > than the GP registers. A boot loader will configure devices and > perhaps initalise memory. > > > + > > +The syntax for loading data into registers is as follows:: > > + > > + -device loader,reg=<reg>,data=<data>,data-len=<data-len> > > + > > +**Parameters:** > > + > > +``<reg>`` > > + The target register to set. Format must pass the following regex > > + ``[a-zA-Z]+[0-9]+``. The numeric part corresponds to the processor's GDB \ > > + register index. For general-purpose registers, this is typically the > > + number in the register's name (e.g., ``r5`` translates to ``5``). > > + Special-purpose registers have specific IDs defined in their processor's > > + `gdbstub.c` file. Note that these IDs vary between processors. > > + > > +``<data>`` > > + The value to load into the specified register. The data must not exceed 8 > > + bytes in size. > > Why 8 bytes? > > > + > > +``<data-len>`` > > + The length of the data in bytes. This parameter is mandatory when using > > + the ``data`` argument. > > Do we need data-len? Why not just use the register size > > > + > > +**Examples:** > > + > > +Set a general-purpose register > > +"""""""""""""""""""""""""""""" > > + > > +To set register ``r5`` to ``0xc0001000`` (4 bytes) on CPU 0:: > > + > > + -device loader,reg=r5,data=0xc0001000,data-len=4 > > + > > +Set a special register > > +"""""""""""""""""""""" > > + > > +To set the Program Counter (PC, register ``32``) to ``0x80000000`` on CPU 0:: > > + > > + -device loader,reg=pc32,data=0x80000000,data-len=4 > > + > > +You must look in your processor's `gdbstub.c` file to special register to index > > +mappings. > > That isn't really helpful for users, but I don't have a better idea > > > + > > +**Special Registers:** > > + > > +Special registers are defined in the processor's ``gdbstub.c`` file > > with numeric IDs. > > +Examples from the MicroBlaze processor at one point looked like. include:: > > + > > + enum { > > + GDB_PC = 32 + 0, > > + GDB_MSR = 32 + 1, > > + GDB_EAR = 32 + 2, > > + GDB_ESR = 32 + 3, > > + GDB_FSR = 32 + 4, > > + GDB_BTR = 32 + 5, > > + GDB_PVR0 = 32 + 6, > > + GDB_PVR11 = 32 + 17, > > + GDB_EDR = 32 + 18, > > + GDB_SLR = 32 + 25, > > + GDB_SHR = 32 + 26, > > + }; > > + > > +For example, to set the Machine State Register (``GDB_MSR``) on a MicroBlaze > > +processor:: > > + > > + -device loader,reg=MSR33,data=0x00000001,data-len=4 > > + > > +**Register Loading Notes:** > > + > > +1. **Processor-Specific IDs**: > > + The numeric IDs for registers vary between processors. Always refer to the > > + `gdbstub.c` file for the target processor to identify the correct register > > + mappings. > > + > > +2. **Pre-Execution State**: > > + This capability is ideal for initializing a simulated environment to match > > + the state expected by an ELF file. For example, you can configure stack > > + pointers, machine state registers, and program counters to prepare the > > + processor to run a bootstrapped application. > > + > > +3. **Validation**: > > + Register numbers are validated by the `gdb_write_register` function. Ensure > > + the specified register is supported by the target architecture. > > + > > +4. **Endianess**: > > + The `data` value is written using the processor's native endian format. > > + > > +By using the `loader` device to initialize registers, you can simulate > > +realistic execution environments, enabling detailed testing and debugging > > +of embedded software, including bootloaders interactions and operating > > +system kernels. > > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > > index ea8628b892..9408ecd150 100644 > > --- a/hw/core/generic-loader.c > > +++ b/hw/core/generic-loader.c > > @@ -55,6 +55,14 @@ static void generic_loader_reset(void *opaque) > > } > > } > > + if(s->reg.name) { > > + CPUClass *cc = CPU_GET_CLASS(s->cpu); > > + int bytes_written = cc->gdb_write_register(s->cpu, (uint8_t*) > > &s->reg.value, s->reg.num); > > + if(bytes_written != s->reg.data_len) { > > + printf("Error setting register %d to value %lX expected to write %d, > > but wrote %d\n", s->reg.num, s->reg.value, s->reg.data_len, > > bytes_written); > > The line wrapping is muddled up here. Can you please send it with git > send-email. Do some sends against yourself to make sure it works. > > You mentioned gmail in an earlier thread I think, did you follow the > instructions: https://git-scm.com/docs/git-send-email#_use_gmail_as_the_smtp_server > > Alistair -- Sincerely, Sam Price ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-10 5:33 ` Sam Price @ 2025-01-31 0:27 ` Alistair Francis 2025-02-04 21:42 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 12+ messages in thread From: Alistair Francis @ 2025-01-31 0:27 UTC (permalink / raw) To: Sam Price; +Cc: qemu-devel, alistair On Fri, Jan 10, 2025 at 3:33 PM Sam Price <thesamprice@gmail.com> wrote: > > Yes that is true a boot loader will do more than just set registers. > Ill rework the text a bit on the next update. > In my case i need to set the r5 register that specifies the memory > location to the device tree. Should that be done in the machine instead? It seems tricky to expect users to set this register > I also use the device loader to load in a elf file to ram, and the > device-loader to load in the device tree to the location specified by > the r5 register > > I could add a gdb call that would return an array of string mappings > to integers. > If the machine doesn't implement the function/ leaves it as null > pointer then you wouldn't get the cli support. > Not sure where you would document all the machine register names / > numbers at though. > This might be too much though? We probably don't need to document the register names Alistair > I left the door somewhat open on this via the NAME_NUMBER format. > > There was some checking logic where if data is supplied then it forces > a check for data-len. > I could relax that check if you supply the reg.name field. > > I am unsure how to determine the machine register size. > I assumed the max register size on any machine would be 8 bytes, this > might be wrong. > the gdb call seems to just pass in the full 8 bytes, but I didn't dig > into it for all machines. > > Ill look at this a bit more and try to configure the git email. > I also need to set up a docker container to build /test latest. > I have been building / testing on an old ubuntu machine. > (To test this I need to run it on qemu-xilinx). > My workplace has us on ubuntu 20. > > So it might be a while before I have another version up. > > Thanks, > Sam > > On Thu, Jan 9, 2025 at 7:34 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Wed, Jan 8, 2025 at 12:28 PM Sam Price <thesamprice@gmail.com> wrote: > > > > > > I made the changes, and added documentation. > > > https://gitlab.com/thesamprice/qemu/-/compare/master...loader?from_project_id=11167699 > > > > > > I left it as [PREFIX]<RegNumber> > > > > > > I can switch this to just RegNumber if desired. > > > > > > I am still struggling with the email format sorry. > > > --- > > > docs/system/generic-loader.rst | 98 ++++++++++++++++++++++++++++++++ > > > hw/core/generic-loader.c | 46 +++++++++++---- > > > include/hw/core/generic-loader.h | 7 +++ > > > 3 files changed, 139 insertions(+), 12 deletions(-) > > > > > > diff --git a/docs/system/generic-loader.rst b/docs/system/generic-loader.rst > > > index 4f9fb005f1..71d4aaa097 100644 > > > --- a/docs/system/generic-loader.rst > > > +++ b/docs/system/generic-loader.rst > > > @@ -117,4 +117,102 @@ future the internal state 'set_pc' (which exists > > > in the generic loader > > > now) should be exposed to the user so that they can choose if the PC > > > is set or not. > > > +Loading Data into Registers > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > + > > > +The `loader` device allows the initialization of CPU registers from the command > > > +line. This feature is particularly useful for setting up the processor state > > > +before starting an executable. By configuring registers prior to execution, the > > > +`loader` can mimic the state that a bootloader would leave the processor in > > > +before transferring control to an ELF file or another executable. > > > > This isn't really true though. A bootloader generally will set more > > than the GP registers. A boot loader will configure devices and > > perhaps initalise memory. > > > > > + > > > +The syntax for loading data into registers is as follows:: > > > + > > > + -device loader,reg=<reg>,data=<data>,data-len=<data-len> > > > + > > > +**Parameters:** > > > + > > > +``<reg>`` > > > + The target register to set. Format must pass the following regex > > > + ``[a-zA-Z]+[0-9]+``. The numeric part corresponds to the processor's GDB \ > > > + register index. For general-purpose registers, this is typically the > > > + number in the register's name (e.g., ``r5`` translates to ``5``). > > > + Special-purpose registers have specific IDs defined in their processor's > > > + `gdbstub.c` file. Note that these IDs vary between processors. > > > + > > > +``<data>`` > > > + The value to load into the specified register. The data must not exceed 8 > > > + bytes in size. > > > > Why 8 bytes? > > > > > + > > > +``<data-len>`` > > > + The length of the data in bytes. This parameter is mandatory when using > > > + the ``data`` argument. > > > > Do we need data-len? Why not just use the register size > > > > > + > > > +**Examples:** > > > + > > > +Set a general-purpose register > > > +"""""""""""""""""""""""""""""" > > > + > > > +To set register ``r5`` to ``0xc0001000`` (4 bytes) on CPU 0:: > > > + > > > + -device loader,reg=r5,data=0xc0001000,data-len=4 > > > + > > > +Set a special register > > > +"""""""""""""""""""""" > > > + > > > +To set the Program Counter (PC, register ``32``) to ``0x80000000`` on CPU 0:: > > > + > > > + -device loader,reg=pc32,data=0x80000000,data-len=4 > > > + > > > +You must look in your processor's `gdbstub.c` file to special register to index > > > +mappings. > > > > That isn't really helpful for users, but I don't have a better idea > > > > > + > > > +**Special Registers:** > > > + > > > +Special registers are defined in the processor's ``gdbstub.c`` file > > > with numeric IDs. > > > +Examples from the MicroBlaze processor at one point looked like. include:: > > > + > > > + enum { > > > + GDB_PC = 32 + 0, > > > + GDB_MSR = 32 + 1, > > > + GDB_EAR = 32 + 2, > > > + GDB_ESR = 32 + 3, > > > + GDB_FSR = 32 + 4, > > > + GDB_BTR = 32 + 5, > > > + GDB_PVR0 = 32 + 6, > > > + GDB_PVR11 = 32 + 17, > > > + GDB_EDR = 32 + 18, > > > + GDB_SLR = 32 + 25, > > > + GDB_SHR = 32 + 26, > > > + }; > > > + > > > +For example, to set the Machine State Register (``GDB_MSR``) on a MicroBlaze > > > +processor:: > > > + > > > + -device loader,reg=MSR33,data=0x00000001,data-len=4 > > > + > > > +**Register Loading Notes:** > > > + > > > +1. **Processor-Specific IDs**: > > > + The numeric IDs for registers vary between processors. Always refer to the > > > + `gdbstub.c` file for the target processor to identify the correct register > > > + mappings. > > > + > > > +2. **Pre-Execution State**: > > > + This capability is ideal for initializing a simulated environment to match > > > + the state expected by an ELF file. For example, you can configure stack > > > + pointers, machine state registers, and program counters to prepare the > > > + processor to run a bootstrapped application. > > > + > > > +3. **Validation**: > > > + Register numbers are validated by the `gdb_write_register` function. Ensure > > > + the specified register is supported by the target architecture. > > > + > > > +4. **Endianess**: > > > + The `data` value is written using the processor's native endian format. > > > + > > > +By using the `loader` device to initialize registers, you can simulate > > > +realistic execution environments, enabling detailed testing and debugging > > > +of embedded software, including bootloaders interactions and operating > > > +system kernels. > > > diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c > > > index ea8628b892..9408ecd150 100644 > > > --- a/hw/core/generic-loader.c > > > +++ b/hw/core/generic-loader.c > > > @@ -55,6 +55,14 @@ static void generic_loader_reset(void *opaque) > > > } > > > } > > > + if(s->reg.name) { > > > + CPUClass *cc = CPU_GET_CLASS(s->cpu); > > > + int bytes_written = cc->gdb_write_register(s->cpu, (uint8_t*) > > > &s->reg.value, s->reg.num); > > > + if(bytes_written != s->reg.data_len) { > > > + printf("Error setting register %d to value %lX expected to write %d, > > > but wrote %d\n", s->reg.num, s->reg.value, s->reg.data_len, > > > bytes_written); > > > > The line wrapping is muddled up here. Can you please send it with git > > send-email. Do some sends against yourself to make sure it works. > > > > You mentioned gmail in an earlier thread I think, did you follow the > > instructions: https://git-scm.com/docs/git-send-email#_use_gmail_as_the_smtp_server > > > > Alistair > > > > -- > Sincerely, > > Sam Price ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-31 0:27 ` Alistair Francis @ 2025-02-04 21:42 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2025-02-04 21:42 UTC (permalink / raw) To: Sam Price, Alex Bennée Cc: qemu-devel, Alistair Francis, alistair, Pierrick Bouvier Cc'ing Alex/Pierrick On 31/1/25 01:27, Alistair Francis wrote: > On Fri, Jan 10, 2025 at 3:33 PM Sam Price <thesamprice@gmail.com> wrote: >> >> Yes that is true a boot loader will do more than just set registers. >> Ill rework the text a bit on the next update. >> In my case i need to set the r5 register that specifies the memory >> location to the device tree. > > Should that be done in the machine instead? It seems tricky to expect > users to set this register > >> I also use the device loader to load in a elf file to ram, and the >> device-loader to load in the device tree to the location specified by >> the r5 register >> >> I could add a gdb call that would return an array of string mappings >> to integers. >> If the machine doesn't implement the function/ leaves it as null >> pointer then you wouldn't get the cli support. >> Not sure where you would document all the machine register names / >> numbers at though. >> This might be too much though? > > We probably don't need to document the register names > > Alistair > >> I left the door somewhat open on this via the NAME_NUMBER format. >> >> There was some checking logic where if data is supplied then it forces >> a check for data-len. >> I could relax that check if you supply the reg.name field. >> >> I am unsure how to determine the machine register size. >> I assumed the max register size on any machine would be 8 bytes, this >> might be wrong. >> the gdb call seems to just pass in the full 8 bytes, but I didn't dig >> into it for all machines. >> >> Ill look at this a bit more and try to configure the git email. >> I also need to set up a docker container to build /test latest. >> I have been building / testing on an old ubuntu machine. >> (To test this I need to run it on qemu-xilinx). >> My workplace has us on ubuntu 20. >> >> So it might be a while before I have another version up. >> >> Thanks, >> Sam >> >> On Thu, Jan 9, 2025 at 7:34 PM Alistair Francis <alistair23@gmail.com> wrote: >>> >>> On Wed, Jan 8, 2025 at 12:28 PM Sam Price <thesamprice@gmail.com> wrote: >>>> >>>> I made the changes, and added documentation. >>>> https://gitlab.com/thesamprice/qemu/-/compare/master...loader?from_project_id=11167699 >>>> >>>> I left it as [PREFIX]<RegNumber> >>>> >>>> I can switch this to just RegNumber if desired. >>>> >>>> I am still struggling with the email format sorry. >>>> --- >>>> docs/system/generic-loader.rst | 98 ++++++++++++++++++++++++++++++++ >>>> hw/core/generic-loader.c | 46 +++++++++++---- >>>> include/hw/core/generic-loader.h | 7 +++ >>>> 3 files changed, 139 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/docs/system/generic-loader.rst b/docs/system/generic-loader.rst >>>> index 4f9fb005f1..71d4aaa097 100644 >>>> --- a/docs/system/generic-loader.rst >>>> +++ b/docs/system/generic-loader.rst >>>> @@ -117,4 +117,102 @@ future the internal state 'set_pc' (which exists >>>> in the generic loader >>>> now) should be exposed to the user so that they can choose if the PC >>>> is set or not. >>>> +Loading Data into Registers >>>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^ >>>> + >>>> +The `loader` device allows the initialization of CPU registers from the command >>>> +line. This feature is particularly useful for setting up the processor state >>>> +before starting an executable. By configuring registers prior to execution, the >>>> +`loader` can mimic the state that a bootloader would leave the processor in >>>> +before transferring control to an ELF file or another executable. >>> >>> This isn't really true though. A bootloader generally will set more >>> than the GP registers. A boot loader will configure devices and >>> perhaps initalise memory. >>> >>>> + >>>> +The syntax for loading data into registers is as follows:: >>>> + >>>> + -device loader,reg=<reg>,data=<data>,data-len=<data-len> >>>> + >>>> +**Parameters:** >>>> + >>>> +``<reg>`` >>>> + The target register to set. Format must pass the following regex >>>> + ``[a-zA-Z]+[0-9]+``. The numeric part corresponds to the processor's GDB \ >>>> + register index. For general-purpose registers, this is typically the >>>> + number in the register's name (e.g., ``r5`` translates to ``5``). >>>> + Special-purpose registers have specific IDs defined in their processor's >>>> + `gdbstub.c` file. Note that these IDs vary between processors. >>>> + >>>> +``<data>`` >>>> + The value to load into the specified register. The data must not exceed 8 >>>> + bytes in size. >>> >>> Why 8 bytes? >>> >>>> + >>>> +``<data-len>`` >>>> + The length of the data in bytes. This parameter is mandatory when using >>>> + the ``data`` argument. >>> >>> Do we need data-len? Why not just use the register size >>> >>>> + >>>> +**Examples:** >>>> + >>>> +Set a general-purpose register >>>> +"""""""""""""""""""""""""""""" >>>> + >>>> +To set register ``r5`` to ``0xc0001000`` (4 bytes) on CPU 0:: >>>> + >>>> + -device loader,reg=r5,data=0xc0001000,data-len=4 >>>> + >>>> +Set a special register >>>> +"""""""""""""""""""""" >>>> + >>>> +To set the Program Counter (PC, register ``32``) to ``0x80000000`` on CPU 0:: >>>> + >>>> + -device loader,reg=pc32,data=0x80000000,data-len=4 >>>> + >>>> +You must look in your processor's `gdbstub.c` file to special register to index >>>> +mappings. >>> >>> That isn't really helpful for users, but I don't have a better idea >>> >>>> + >>>> +**Special Registers:** >>>> + >>>> +Special registers are defined in the processor's ``gdbstub.c`` file >>>> with numeric IDs. >>>> +Examples from the MicroBlaze processor at one point looked like. include:: >>>> + >>>> + enum { >>>> + GDB_PC = 32 + 0, >>>> + GDB_MSR = 32 + 1, >>>> + GDB_EAR = 32 + 2, >>>> + GDB_ESR = 32 + 3, >>>> + GDB_FSR = 32 + 4, >>>> + GDB_BTR = 32 + 5, >>>> + GDB_PVR0 = 32 + 6, >>>> + GDB_PVR11 = 32 + 17, >>>> + GDB_EDR = 32 + 18, >>>> + GDB_SLR = 32 + 25, >>>> + GDB_SHR = 32 + 26, >>>> + }; >>>> + >>>> +For example, to set the Machine State Register (``GDB_MSR``) on a MicroBlaze >>>> +processor:: >>>> + >>>> + -device loader,reg=MSR33,data=0x00000001,data-len=4 >>>> + >>>> +**Register Loading Notes:** >>>> + >>>> +1. **Processor-Specific IDs**: >>>> + The numeric IDs for registers vary between processors. Always refer to the >>>> + `gdbstub.c` file for the target processor to identify the correct register >>>> + mappings. >>>> + >>>> +2. **Pre-Execution State**: >>>> + This capability is ideal for initializing a simulated environment to match >>>> + the state expected by an ELF file. For example, you can configure stack >>>> + pointers, machine state registers, and program counters to prepare the >>>> + processor to run a bootstrapped application. >>>> + >>>> +3. **Validation**: >>>> + Register numbers are validated by the `gdb_write_register` function. Ensure >>>> + the specified register is supported by the target architecture. >>>> + >>>> +4. **Endianess**: >>>> + The `data` value is written using the processor's native endian format. >>>> + >>>> +By using the `loader` device to initialize registers, you can simulate >>>> +realistic execution environments, enabling detailed testing and debugging >>>> +of embedded software, including bootloaders interactions and operating >>>> +system kernels. >>>> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c >>>> index ea8628b892..9408ecd150 100644 >>>> --- a/hw/core/generic-loader.c >>>> +++ b/hw/core/generic-loader.c >>>> @@ -55,6 +55,14 @@ static void generic_loader_reset(void *opaque) >>>> } >>>> } >>>> + if(s->reg.name) { >>>> + CPUClass *cc = CPU_GET_CLASS(s->cpu); >>>> + int bytes_written = cc->gdb_write_register(s->cpu, (uint8_t*) >>>> &s->reg.value, s->reg.num); >>>> + if(bytes_written != s->reg.data_len) { >>>> + printf("Error setting register %d to value %lX expected to write %d, >>>> but wrote %d\n", s->reg.num, s->reg.value, s->reg.data_len, >>>> bytes_written); >>> >>> The line wrapping is muddled up here. Can you please send it with git >>> send-email. Do some sends against yourself to make sure it works. >>> >>> You mentioned gmail in an earlier thread I think, did you follow the >>> instructions: https://git-scm.com/docs/git-send-email#_use_gmail_as_the_smtp_server >>> >>> Alistair >> >> >> >> -- >> Sincerely, >> >> Sam Price > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Subject: [PATCH] loader: Add register setting support via cli 2025-01-08 2:28 ` Sam Price 2025-01-10 0:33 ` Alistair Francis @ 2025-01-10 9:53 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2025-01-10 9:53 UTC (permalink / raw) To: Sam Price, Alistair Francis; +Cc: qemu-devel, alistair Hi Sam, On 8/1/25 03:28, Sam Price wrote: > I made the changes, and added documentation. > https://gitlab.com/thesamprice/qemu/-/compare/master...loader?from_project_id=11167699 > > I left it as [PREFIX]<RegNumber> > > I can switch this to just RegNumber if desired. > > I am still struggling with the email format sorry. Possibly SourceHut can help you, see: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#if-you-cannot-send-patch-emails > --- > docs/system/generic-loader.rst | 98 ++++++++++++++++++++++++++++++++ > hw/core/generic-loader.c | 46 +++++++++++---- > include/hw/core/generic-loader.h | 7 +++ > 3 files changed, 139 insertions(+), 12 deletions(-) ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-02-04 21:42 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-06 3:29 Subject: [PATCH] loader: Add register setting support via cli Sam Price 2024-12-19 23:11 ` Sam Price 2025-01-06 1:43 ` Sam Price 2025-01-06 4:41 ` Alistair Francis 2025-01-06 4:59 ` Sam Price 2025-01-06 5:13 ` Alistair Francis 2025-01-08 2:28 ` Sam Price 2025-01-10 0:33 ` Alistair Francis 2025-01-10 5:33 ` Sam Price 2025-01-31 0:27 ` Alistair Francis 2025-02-04 21:42 ` Philippe Mathieu-Daudé 2025-01-10 9:53 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).