* [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address
@ 2020-03-15 13:48 Philippe Mathieu-Daudé
2020-03-15 13:50 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-15 13:48 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Yoshinori Sato, qemu-block, Alistair Francis,
Stephanos Ioannidis, Philippe Mathieu-Daudé,
Paolo Bonzini, Philippe Mathieu-Daudé,
Richard Henderson
From: Philippe Mathieu-Daudé <philmd@redhat.com>
The RX code flash is not a Masked ROM but a EEPROM (electrically
erasable programmable flash memory).
When implementing the flash hardware, the rom_ptr() returns NULL
and the reset vector is not set.
Instead, use the address_space ld/st API to fetch the reset vector
address from the code flash.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Based-on: <20200315132810.7022-1-f4bug@amsat.org>
Same issue might occurs in Cortex-M arm_cpu_reset()
---
target/rx/cpu.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 9c224a273c..d3bd09e753 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -26,6 +26,8 @@
#include "hw/loader.h"
#include "fpu/softfloat.h"
+#define CPU_RESET_VECTOR 0xfffffffc
+
static void rx_cpu_set_pc(CPUState *cs, vaddr value)
{
RXCPU *cpu = RXCPU(cs);
@@ -51,17 +53,13 @@ static void rx_cpu_reset(CPUState *s)
RXCPU *cpu = RXCPU(s);
RXCPUClass *rcc = RXCPU_GET_CLASS(cpu);
CPURXState *env = &cpu->env;
- uint32_t *resetvec;
rcc->parent_reset(s);
memset(env, 0, offsetof(CPURXState, end_reset_fields));
- resetvec = rom_ptr(0xfffffffc, 4);
- if (resetvec) {
- /* In the case of kernel, it is ignored because it is not set. */
- env->pc = ldl_p(resetvec);
- }
+ env->pc = address_space_ldl(cpu_get_address_space(s, 0),
+ CPU_RESET_VECTOR, MEMTXATTRS_UNSPECIFIED, NULL);
rx_cpu_unpack_psw(env, 0, 1);
env->regs[0] = env->isp = env->usp = 0;
env->fpsw = 0;
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address
2020-03-15 13:48 [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address Philippe Mathieu-Daudé
@ 2020-03-15 13:50 ` Philippe Mathieu-Daudé
2020-03-15 18:50 ` Richard Henderson
2020-03-16 9:47 ` Peter Maydell
2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-15 13:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Yoshinori Sato, qemu-block, Alistair Francis,
Stephanos Ioannidis, Paolo Bonzini, Richard Henderson
On 3/15/20 2:48 PM, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
Hmm author email should be <f4bug@amsat.org>...
>
> The RX code flash is not a Masked ROM but a EEPROM (electrically
> erasable programmable flash memory).
> When implementing the flash hardware, the rom_ptr() returns NULL
> and the reset vector is not set.
> Instead, use the address_space ld/st API to fetch the reset vector
> address from the code flash.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Based-on: <20200315132810.7022-1-f4bug@amsat.org>
>
> Same issue might occurs in Cortex-M arm_cpu_reset()
> ---
> target/rx/cpu.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 9c224a273c..d3bd09e753 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -26,6 +26,8 @@
> #include "hw/loader.h"
> #include "fpu/softfloat.h"
>
> +#define CPU_RESET_VECTOR 0xfffffffc
> +
> static void rx_cpu_set_pc(CPUState *cs, vaddr value)
> {
> RXCPU *cpu = RXCPU(cs);
> @@ -51,17 +53,13 @@ static void rx_cpu_reset(CPUState *s)
> RXCPU *cpu = RXCPU(s);
> RXCPUClass *rcc = RXCPU_GET_CLASS(cpu);
> CPURXState *env = &cpu->env;
> - uint32_t *resetvec;
>
> rcc->parent_reset(s);
>
> memset(env, 0, offsetof(CPURXState, end_reset_fields));
>
> - resetvec = rom_ptr(0xfffffffc, 4);
> - if (resetvec) {
> - /* In the case of kernel, it is ignored because it is not set. */
> - env->pc = ldl_p(resetvec);
> - }
> + env->pc = address_space_ldl(cpu_get_address_space(s, 0),
> + CPU_RESET_VECTOR, MEMTXATTRS_UNSPECIFIED, NULL);
> rx_cpu_unpack_psw(env, 0, 1);
> env->regs[0] = env->isp = env->usp = 0;
> env->fpsw = 0;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address
2020-03-15 13:48 [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address Philippe Mathieu-Daudé
2020-03-15 13:50 ` Philippe Mathieu-Daudé
@ 2020-03-15 18:50 ` Richard Henderson
2020-03-16 9:47 ` Peter Maydell
2 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2020-03-15 18:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Yoshinori Sato, qemu-block, Alistair Francis,
Stephanos Ioannidis, Paolo Bonzini, Philippe Mathieu-Daudé,
Richard Henderson
On 3/15/20 6:48 AM, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> The RX code flash is not a Masked ROM but a EEPROM (electrically
> erasable programmable flash memory).
> When implementing the flash hardware, the rom_ptr() returns NULL
> and the reset vector is not set.
> Instead, use the address_space ld/st API to fetch the reset vector
> address from the code flash.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Based-on: <20200315132810.7022-1-f4bug@amsat.org>
>
> Same issue might occurs in Cortex-M arm_cpu_reset()
> ---
> target/rx/cpu.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address
2020-03-15 13:48 [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address Philippe Mathieu-Daudé
2020-03-15 13:50 ` Philippe Mathieu-Daudé
2020-03-15 18:50 ` Richard Henderson
@ 2020-03-16 9:47 ` Peter Maydell
2 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2020-03-16 9:47 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Qemu-block, Yoshinori Sato, Alistair Francis,
Stephanos Ioannidis, QEMU Developers, Paolo Bonzini,
Philippe Mathieu-Daudé,
Richard Henderson
On Sun, 15 Mar 2020 at 13:49, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> The RX code flash is not a Masked ROM but a EEPROM (electrically
> erasable programmable flash memory).
> When implementing the flash hardware, the rom_ptr() returns NULL
> and the reset vector is not set.
> Instead, use the address_space ld/st API to fetch the reset vector
> address from the code flash.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Based-on: <20200315132810.7022-1-f4bug@amsat.org>
>
> Same issue might occurs in Cortex-M arm_cpu_reset()
rom_ptr() does not mean "I'm trying to get this from ROM",
it means "I'm trying to get this from a user-supplied ELF
file or similar which hasn't been loaded into guest memory
yet". (This is a workaround for a reset ordering issue where
CPU reset happens before rom_reset() runs.)
Removing the usage of rom_ptr() altogether here doesn't
look right -- have you tested the case where the initial
reset vector contents are provided via -kernel or
-device loader,... ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-16 10:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-15 13:48 [PATCH] target/rx/cpu: Use address_space_ldl() to read reset vector address Philippe Mathieu-Daudé
2020-03-15 13:50 ` Philippe Mathieu-Daudé
2020-03-15 18:50 ` Richard Henderson
2020-03-16 9:47 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.