* [PATCH 01/17] hw/core/cpu: Use size_t for memory_rw_debug len argument
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-23 21:25 ` Philippe Mathieu-Daudé
2025-03-25 0:43 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 02/17] target/avr: Fix buffer read in avr_print_insn Richard Henderson
` (15 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Match the prototype of cpu_memory_rw_debug().
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/hw/core/cpu.h | 2 +-
target/sparc/cpu.h | 2 +-
target/sparc/mmu_helper.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5d11d26556..abd8764e83 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -154,7 +154,7 @@ struct CPUClass {
int (*mmu_index)(CPUState *cpu, bool ifetch);
int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
- uint8_t *buf, int len, bool is_write);
+ uint8_t *buf, size_t len, bool is_write);
void (*dump_state)(CPUState *cpu, FILE *, int flags);
void (*query_cpu_fast)(CPUState *cpu, CpuInfoFast *value);
int64_t (*get_arch_id)(CPUState *cpu);
diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 462bcb6c0e..68f8c21e7c 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -604,7 +604,7 @@ void dump_mmu(CPUSPARCState *env);
#if !defined(TARGET_SPARC64) && !defined(CONFIG_USER_ONLY)
int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
- uint8_t *buf, int len, bool is_write);
+ uint8_t *buf, size_t len, bool is_write);
#endif
/* translate.c */
diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
index 7548d01777..3821cd91ec 100644
--- a/target/sparc/mmu_helper.c
+++ b/target/sparc/mmu_helper.c
@@ -389,7 +389,7 @@ void dump_mmu(CPUSPARCState *env)
* that the sparc ABI is followed.
*/
int sparc_cpu_memory_rw_debug(CPUState *cs, vaddr address,
- uint8_t *buf, int len, bool is_write)
+ uint8_t *buf, size_t len, bool is_write)
{
CPUSPARCState *env = cpu_env(cs);
target_ulong addr = address;
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 01/17] hw/core/cpu: Use size_t for memory_rw_debug len argument
2025-03-23 17:37 ` [PATCH 01/17] hw/core/cpu: Use size_t for memory_rw_debug len argument Richard Henderson
@ 2025-03-23 21:25 ` Philippe Mathieu-Daudé
2025-03-25 0:43 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:25 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 18:37, Richard Henderson wrote:
> Match the prototype of cpu_memory_rw_debug().
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/hw/core/cpu.h | 2 +-
> target/sparc/cpu.h | 2 +-
> target/sparc/mmu_helper.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 01/17] hw/core/cpu: Use size_t for memory_rw_debug len argument
2025-03-23 17:37 ` [PATCH 01/17] hw/core/cpu: Use size_t for memory_rw_debug len argument Richard Henderson
2025-03-23 21:25 ` Philippe Mathieu-Daudé
@ 2025-03-25 0:43 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 0:43 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Match the prototype of cpu_memory_rw_debug().
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/hw/core/cpu.h | 2 +-
> target/sparc/cpu.h | 2 +-
> target/sparc/mmu_helper.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 5d11d26556..abd8764e83 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -154,7 +154,7 @@ struct CPUClass {
>
> int (*mmu_index)(CPUState *cpu, bool ifetch);
> int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
> - uint8_t *buf, int len, bool is_write);
> + uint8_t *buf, size_t len, bool is_write);
> void (*dump_state)(CPUState *cpu, FILE *, int flags);
> void (*query_cpu_fast)(CPUState *cpu, CpuInfoFast *value);
> int64_t (*get_arch_id)(CPUState *cpu);
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index 462bcb6c0e..68f8c21e7c 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -604,7 +604,7 @@ void dump_mmu(CPUSPARCState *env);
>
> #if !defined(TARGET_SPARC64) && !defined(CONFIG_USER_ONLY)
> int sparc_cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
> - uint8_t *buf, int len, bool is_write);
> + uint8_t *buf, size_t len, bool is_write);
> #endif
>
> /* translate.c */
> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
> index 7548d01777..3821cd91ec 100644
> --- a/target/sparc/mmu_helper.c
> +++ b/target/sparc/mmu_helper.c
> @@ -389,7 +389,7 @@ void dump_mmu(CPUSPARCState *env)
> * that the sparc ABI is followed.
> */
> int sparc_cpu_memory_rw_debug(CPUState *cs, vaddr address,
> - uint8_t *buf, int len, bool is_write)
> + uint8_t *buf, size_t len, bool is_write)
> {
> CPUSPARCState *env = cpu_env(cs);
> target_ulong addr = address;
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 02/17] target/avr: Fix buffer read in avr_print_insn
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
2025-03-23 17:37 ` [PATCH 01/17] hw/core/cpu: Use size_t for memory_rw_debug len argument Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 0:52 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 03/17] target/avr: Improve decode of LDS, STS Richard Henderson
` (14 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier, qemu-stable
Do not unconditionally attempt to read 4 bytes, as there
may only be 2 bytes remaining in the translator cache.
Cc: qemu-stable@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/disas.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/target/avr/disas.c b/target/avr/disas.c
index b7689e8d7c..d341030174 100644
--- a/target/avr/disas.c
+++ b/target/avr/disas.c
@@ -68,28 +68,35 @@ static bool decode_insn(DisasContext *ctx, uint16_t insn);
int avr_print_insn(bfd_vma addr, disassemble_info *info)
{
- DisasContext ctx;
+ DisasContext ctx = { info };
DisasContext *pctx = &ctx;
bfd_byte buffer[4];
uint16_t insn;
int status;
- ctx.info = info;
-
- status = info->read_memory_func(addr, buffer, 4, info);
+ status = info->read_memory_func(addr, buffer, 2, info);
if (status != 0) {
info->memory_error_func(status, addr, info);
return -1;
}
insn = bfd_getl16(buffer);
- ctx.next_word = bfd_getl16(buffer + 2);
- ctx.next_word_used = false;
+
+ status = info->read_memory_func(addr + 2, buffer + 2, 2, info);
+ if (status == 0) {
+ ctx.next_word = bfd_getl16(buffer + 2);
+ }
if (!decode_insn(&ctx, insn)) {
output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
}
- return ctx.next_word_used ? 4 : 2;
+ if (!ctx.next_word_used) {
+ return 2;
+ } else if (status == 0) {
+ return 4;
+ }
+ info->memory_error_func(status, addr + 2, info);
+ return -1;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 02/17] target/avr: Fix buffer read in avr_print_insn
2025-03-23 17:37 ` [PATCH 02/17] target/avr: Fix buffer read in avr_print_insn Richard Henderson
@ 2025-03-25 0:52 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 0:52 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd, qemu-stable
On 3/23/25 10:37, Richard Henderson wrote:
> Do not unconditionally attempt to read 4 bytes, as there
> may only be 2 bytes remaining in the translator cache.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/disas.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/target/avr/disas.c b/target/avr/disas.c
> index b7689e8d7c..d341030174 100644
> --- a/target/avr/disas.c
> +++ b/target/avr/disas.c
> @@ -68,28 +68,35 @@ static bool decode_insn(DisasContext *ctx, uint16_t insn);
>
> int avr_print_insn(bfd_vma addr, disassemble_info *info)
> {
> - DisasContext ctx;
> + DisasContext ctx = { info };
> DisasContext *pctx = &ctx;
> bfd_byte buffer[4];
> uint16_t insn;
> int status;
>
> - ctx.info = info;
> -
> - status = info->read_memory_func(addr, buffer, 4, info);
> + status = info->read_memory_func(addr, buffer, 2, info);
> if (status != 0) {
> info->memory_error_func(status, addr, info);
> return -1;
> }
> insn = bfd_getl16(buffer);
> - ctx.next_word = bfd_getl16(buffer + 2);
> - ctx.next_word_used = false;
> +
> + status = info->read_memory_func(addr + 2, buffer + 2, 2, info);
> + if (status == 0) {
> + ctx.next_word = bfd_getl16(buffer + 2);
> + }
>
> if (!decode_insn(&ctx, insn)) {
> output(".db", "0x%02x, 0x%02x", buffer[0], buffer[1]);
> }
>
> - return ctx.next_word_used ? 4 : 2;
> + if (!ctx.next_word_used) {
> + return 2;
> + } else if (status == 0) {
> + return 4;
> + }
> + info->memory_error_func(status, addr + 2, info);
> + return -1;
> }
>
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 03/17] target/avr: Improve decode of LDS, STS
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
2025-03-23 17:37 ` [PATCH 01/17] hw/core/cpu: Use size_t for memory_rw_debug len argument Richard Henderson
2025-03-23 17:37 ` [PATCH 02/17] target/avr: Fix buffer read in avr_print_insn Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 0:53 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 04/17] target/avr: Remove OFFSET_CPU_REGISTERS Richard Henderson
` (13 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier, qemu-stable
The comment about not being able to define a field with
zero bits is out of date since 94597b6146f3
("decodetree: Allow !function with no input bits").
This fixes the missing load of imm in the disassembler.
Cc: qemu-stable@nongnu.org
Fixes: 9d8caa67a24 ("target/avr: Add support for disassembling via option '-d in_asm'")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/translate.c | 2 --
target/avr/insn.decode | 7 ++-----
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 4ab71d8138..e7f8ced9b3 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1578,7 +1578,6 @@ static bool trans_LDS(DisasContext *ctx, arg_LDS *a)
TCGv Rd = cpu_r[a->rd];
TCGv addr = tcg_temp_new_i32();
TCGv H = cpu_rampD;
- a->imm = next_word(ctx);
tcg_gen_mov_tl(addr, H); /* addr = H:M:L */
tcg_gen_shli_tl(addr, addr, 16);
@@ -1783,7 +1782,6 @@ static bool trans_STS(DisasContext *ctx, arg_STS *a)
TCGv Rd = cpu_r[a->rd];
TCGv addr = tcg_temp_new_i32();
TCGv H = cpu_rampD;
- a->imm = next_word(ctx);
tcg_gen_mov_tl(addr, H); /* addr = H:M:L */
tcg_gen_shli_tl(addr, addr, 16);
diff --git a/target/avr/insn.decode b/target/avr/insn.decode
index 482c23ad0c..cc302249db 100644
--- a/target/avr/insn.decode
+++ b/target/avr/insn.decode
@@ -118,11 +118,8 @@ BRBC 1111 01 ....... ... @op_bit_imm
@io_rd_imm .... . .. ..... .... &rd_imm rd=%rd imm=%io_imm
@ldst_d .. . . .. . rd:5 . ... &rd_imm imm=%ldst_d_imm
-# The 16-bit immediate is completely in the next word.
-# Fields cannot be defined with no bits, so we cannot play
-# the same trick and append to a zero-bit value.
-# Defer reading the immediate until trans_{LDS,STS}.
-@ldst_s .... ... rd:5 .... imm=0
+%ldst_imm !function=next_word
+@ldst_s .... ... rd:5 .... imm=%ldst_imm
MOV 0010 11 . ..... .... @op_rd_rr
MOVW 0000 0001 .... .... &rd_rr rd=%rd_d rr=%rr_d
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 03/17] target/avr: Improve decode of LDS, STS
2025-03-23 17:37 ` [PATCH 03/17] target/avr: Improve decode of LDS, STS Richard Henderson
@ 2025-03-25 0:53 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 0:53 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd, qemu-stable
On 3/23/25 10:37, Richard Henderson wrote:
> The comment about not being able to define a field with
> zero bits is out of date since 94597b6146f3
> ("decodetree: Allow !function with no input bits").
>
> This fixes the missing load of imm in the disassembler.
>
> Cc: qemu-stable@nongnu.org
> Fixes: 9d8caa67a24 ("target/avr: Add support for disassembling via option '-d in_asm'")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/translate.c | 2 --
> target/avr/insn.decode | 7 ++-----
> 2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index 4ab71d8138..e7f8ced9b3 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -1578,7 +1578,6 @@ static bool trans_LDS(DisasContext *ctx, arg_LDS *a)
> TCGv Rd = cpu_r[a->rd];
> TCGv addr = tcg_temp_new_i32();
> TCGv H = cpu_rampD;
> - a->imm = next_word(ctx);
>
> tcg_gen_mov_tl(addr, H); /* addr = H:M:L */
> tcg_gen_shli_tl(addr, addr, 16);
> @@ -1783,7 +1782,6 @@ static bool trans_STS(DisasContext *ctx, arg_STS *a)
> TCGv Rd = cpu_r[a->rd];
> TCGv addr = tcg_temp_new_i32();
> TCGv H = cpu_rampD;
> - a->imm = next_word(ctx);
>
> tcg_gen_mov_tl(addr, H); /* addr = H:M:L */
> tcg_gen_shli_tl(addr, addr, 16);
> diff --git a/target/avr/insn.decode b/target/avr/insn.decode
> index 482c23ad0c..cc302249db 100644
> --- a/target/avr/insn.decode
> +++ b/target/avr/insn.decode
> @@ -118,11 +118,8 @@ BRBC 1111 01 ....... ... @op_bit_imm
> @io_rd_imm .... . .. ..... .... &rd_imm rd=%rd imm=%io_imm
> @ldst_d .. . . .. . rd:5 . ... &rd_imm imm=%ldst_d_imm
>
> -# The 16-bit immediate is completely in the next word.
> -# Fields cannot be defined with no bits, so we cannot play
> -# the same trick and append to a zero-bit value.
> -# Defer reading the immediate until trans_{LDS,STS}.
> -@ldst_s .... ... rd:5 .... imm=0
> +%ldst_imm !function=next_word
> +@ldst_s .... ... rd:5 .... imm=%ldst_imm
>
> MOV 0010 11 . ..... .... @op_rd_rr
> MOVW 0000 0001 .... .... &rd_rr rd=%rd_d rr=%rr_d
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 04/17] target/avr: Remove OFFSET_CPU_REGISTERS
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (2 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 03/17] target/avr: Improve decode of LDS, STS Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-23 21:27 ` Philippe Mathieu-Daudé
2025-03-25 0:55 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 05/17] target/avr: Move cpu register accesses into system memory Richard Henderson
` (12 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
This define isn't really used.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/cpu.h | 2 --
target/avr/helper.c | 3 +--
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index 06f5ae4d1b..84a8f5cc8c 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -60,8 +60,6 @@
#define OFFSET_CODE 0x00000000
/* CPU registers, IO registers, and SRAM */
#define OFFSET_DATA 0x00800000
-/* CPU registers specifically, these are mapped at the start of data */
-#define OFFSET_CPU_REGISTERS OFFSET_DATA
/*
* IO registers, including status register, stack pointer, and memory
* mapped peripherals, mapped just after CPU registers
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 3412312ad5..e5bf16c6b7 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -340,8 +340,7 @@ void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
env->fullacc = false;
/* Following logic assumes this: */
- assert(OFFSET_CPU_REGISTERS == OFFSET_DATA);
- assert(OFFSET_IO_REGISTERS == OFFSET_CPU_REGISTERS +
+ assert(OFFSET_IO_REGISTERS == OFFSET_DATA +
NUMBER_OF_CPU_REGISTERS);
if (addr < NUMBER_OF_CPU_REGISTERS) {
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 04/17] target/avr: Remove OFFSET_CPU_REGISTERS
2025-03-23 17:37 ` [PATCH 04/17] target/avr: Remove OFFSET_CPU_REGISTERS Richard Henderson
@ 2025-03-23 21:27 ` Philippe Mathieu-Daudé
2025-03-25 0:55 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:27 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 18:37, Richard Henderson wrote:
> This define isn't really used.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.h | 2 --
> target/avr/helper.c | 3 +--
> 2 files changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 04/17] target/avr: Remove OFFSET_CPU_REGISTERS
2025-03-23 17:37 ` [PATCH 04/17] target/avr: Remove OFFSET_CPU_REGISTERS Richard Henderson
2025-03-23 21:27 ` Philippe Mathieu-Daudé
@ 2025-03-25 0:55 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 0:55 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> This define isn't really used.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.h | 2 --
> target/avr/helper.c | 3 +--
> 2 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
> index 06f5ae4d1b..84a8f5cc8c 100644
> --- a/target/avr/cpu.h
> +++ b/target/avr/cpu.h
> @@ -60,8 +60,6 @@
> #define OFFSET_CODE 0x00000000
> /* CPU registers, IO registers, and SRAM */
> #define OFFSET_DATA 0x00800000
> -/* CPU registers specifically, these are mapped at the start of data */
> -#define OFFSET_CPU_REGISTERS OFFSET_DATA
> /*
> * IO registers, including status register, stack pointer, and memory
> * mapped peripherals, mapped just after CPU registers
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 3412312ad5..e5bf16c6b7 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -340,8 +340,7 @@ void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
> env->fullacc = false;
>
> /* Following logic assumes this: */
> - assert(OFFSET_CPU_REGISTERS == OFFSET_DATA);
> - assert(OFFSET_IO_REGISTERS == OFFSET_CPU_REGISTERS +
> + assert(OFFSET_IO_REGISTERS == OFFSET_DATA +
> NUMBER_OF_CPU_REGISTERS);
>
> if (addr < NUMBER_OF_CPU_REGISTERS) {
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 05/17] target/avr: Move cpu register accesses into system memory
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (3 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 04/17] target/avr: Remove OFFSET_CPU_REGISTERS Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:07 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 06/17] target/avr: Use cpu_stb_mmuidx_ra in helper_fullwr Richard Henderson
` (11 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Integrate the i/o 0x00-0x1f and 0x38-0x3f loopbacks into
the cpu registers with normal address space accesses.
We no longer need to trap accesses to the first page within
avr_cpu_tlb_fill but can wait until a write occurs.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/cpu.h | 7 ++
target/avr/helper.h | 3 -
target/avr/cpu.c | 16 +++
target/avr/helper.c | 239 +++++++++++++++++------------------------
target/avr/translate.c | 42 ++++----
5 files changed, 146 insertions(+), 161 deletions(-)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index 84a8f5cc8c..be27b0152b 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -23,6 +23,7 @@
#include "cpu-qom.h"
#include "exec/cpu-defs.h"
+#include "exec/memory.h"
#ifdef CONFIG_USER_ONLY
#error "AVR 8-bit does not support user mode"
@@ -142,6 +143,9 @@ struct ArchCPU {
CPUAVRState env;
+ MemoryRegion cpu_reg1;
+ MemoryRegion cpu_reg2;
+
/* Initial value of stack pointer */
uint32_t init_sp;
};
@@ -242,6 +246,9 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr);
+extern const MemoryRegionOps avr_cpu_reg1;
+extern const MemoryRegionOps avr_cpu_reg2;
+
#include "exec/cpu-all.h"
#endif /* QEMU_AVR_CPU_H */
diff --git a/target/avr/helper.h b/target/avr/helper.h
index 4d02e648fa..e8d13e925f 100644
--- a/target/avr/helper.h
+++ b/target/avr/helper.h
@@ -23,7 +23,4 @@ DEF_HELPER_1(debug, noreturn, env)
DEF_HELPER_1(break, noreturn, env)
DEF_HELPER_1(sleep, noreturn, env)
DEF_HELPER_1(unsupported, noreturn, env)
-DEF_HELPER_3(outb, void, env, i32, i32)
-DEF_HELPER_2(inb, tl, env, i32)
DEF_HELPER_3(fullwr, void, env, i32, i32)
-DEF_HELPER_2(fullrd, tl, env, i32)
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 834c7082aa..0b14b36c17 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -23,6 +23,7 @@
#include "qemu/qemu-print.h"
#include "exec/exec-all.h"
#include "exec/translation-block.h"
+#include "exec/address-spaces.h"
#include "cpu.h"
#include "disas/dis-asm.h"
#include "tcg/debug-assert.h"
@@ -110,6 +111,8 @@ static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
{
CPUState *cs = CPU(dev);
+ CPUAVRState *env = cpu_env(cs);
+ AVRCPU *cpu = env_archcpu(env);
AVRCPUClass *mcc = AVR_CPU_GET_CLASS(dev);
Error *local_err = NULL;
@@ -122,6 +125,19 @@ static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
cpu_reset(cs);
mcc->parent_realize(dev, errp);
+
+ /*
+ * Two blocks in the low data space loop back into cpu registers.
+ */
+ memory_region_init_io(&cpu->cpu_reg1, OBJECT(cpu), &avr_cpu_reg1, env,
+ "avr-cpu-reg1", 32);
+ memory_region_add_subregion(get_system_memory(),
+ OFFSET_DATA, &cpu->cpu_reg1);
+
+ memory_region_init_io(&cpu->cpu_reg2, OBJECT(cpu), &avr_cpu_reg2, env,
+ "avr-cpu-reg2", 8);
+ memory_region_add_subregion(get_system_memory(),
+ OFFSET_DATA + 0x58, &cpu->cpu_reg2);
}
static void avr_cpu_set_int(void *opaque, int irq, int level)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index e5bf16c6b7..df7e2109d4 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -108,7 +108,7 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr)
{
- int prot, page_size = TARGET_PAGE_SIZE;
+ int prot;
uint32_t paddr;
address &= TARGET_PAGE_MASK;
@@ -133,23 +133,9 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
/* Access to memory. */
paddr = OFFSET_DATA + address;
prot = PAGE_READ | PAGE_WRITE;
- if (address < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
- /*
- * Access to CPU registers, exit and rebuilt this TB to use
- * full access in case it touches specially handled registers
- * like SREG or SP. For probing, set page_size = 1, in order
- * to force tlb_fill to be called for the next access.
- */
- if (probe) {
- page_size = 1;
- } else {
- cpu_env(cs)->fullacc = 1;
- cpu_loop_exit_restore(cs, retaddr);
- }
- }
}
- tlb_set_page(cs, address, paddr, prot, mmu_idx, page_size);
+ tlb_set_page(cs, address, paddr, prot, mmu_idx, TARGET_PAGE_SIZE);
return true;
}
@@ -203,134 +189,78 @@ void helper_wdr(CPUAVRState *env)
}
/*
- * This function implements IN instruction
- *
- * It does the following
- * a. if an IO register belongs to CPU, its value is read and returned
- * b. otherwise io address is translated to mem address and physical memory
- * is read.
- * c. it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
- *
+ * The first 32 bytes of the data space are mapped to the cpu regs.
+ * We cannot write these from normal store operations because TCG
+ * does not expect global temps to be modified -- a global may be
+ * live in a host cpu register across the store. We can however
+ * read these, as TCG does make sure the global temps are saved
+ * in case the load operation traps.
*/
-target_ulong helper_inb(CPUAVRState *env, uint32_t port)
+
+static uint64_t avr_cpu_reg1_read(void *opaque, hwaddr addr, unsigned size)
{
- target_ulong data = 0;
+ CPUAVRState *env = opaque;
- switch (port) {
- case 0x38: /* RAMPD */
- data = 0xff & (env->rampD >> 16);
- break;
- case 0x39: /* RAMPX */
- data = 0xff & (env->rampX >> 16);
- break;
- case 0x3a: /* RAMPY */
- data = 0xff & (env->rampY >> 16);
- break;
- case 0x3b: /* RAMPZ */
- data = 0xff & (env->rampZ >> 16);
- break;
- case 0x3c: /* EIND */
- data = 0xff & (env->eind >> 16);
- break;
- case 0x3d: /* SPL */
- data = env->sp & 0x00ff;
- break;
- case 0x3e: /* SPH */
- data = env->sp >> 8;
- break;
- case 0x3f: /* SREG */
- data = cpu_get_sreg(env);
- break;
- default:
- /* not a special register, pass to normal memory access */
- data = address_space_ldub(&address_space_memory,
- OFFSET_IO_REGISTERS + port,
- MEMTXATTRS_UNSPECIFIED, NULL);
- }
-
- return data;
+ assert(addr < 32);
+ return env->r[addr];
}
/*
- * This function implements OUT instruction
- *
- * It does the following
- * a. if an IO register belongs to CPU, its value is written into the register
- * b. otherwise io address is translated to mem address and physical memory
- * is written.
- * c. it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
- *
+ * The range 0x58-0x5f of the data space are mapped to cpu regs.
+ * As above, we cannot write these from normal store operations.
*/
-void helper_outb(CPUAVRState *env, uint32_t port, uint32_t data)
-{
- data &= 0x000000ff;
- switch (port) {
- case 0x38: /* RAMPD */
- if (avr_feature(env, AVR_FEATURE_RAMPD)) {
- env->rampD = (data & 0xff) << 16;
- }
- break;
- case 0x39: /* RAMPX */
- if (avr_feature(env, AVR_FEATURE_RAMPX)) {
- env->rampX = (data & 0xff) << 16;
- }
- break;
- case 0x3a: /* RAMPY */
- if (avr_feature(env, AVR_FEATURE_RAMPY)) {
- env->rampY = (data & 0xff) << 16;
- }
- break;
- case 0x3b: /* RAMPZ */
- if (avr_feature(env, AVR_FEATURE_RAMPZ)) {
- env->rampZ = (data & 0xff) << 16;
- }
- break;
- case 0x3c: /* EIDN */
- env->eind = (data & 0xff) << 16;
- break;
- case 0x3d: /* SPL */
- env->sp = (env->sp & 0xff00) | (data);
- break;
- case 0x3e: /* SPH */
- if (avr_feature(env, AVR_FEATURE_2_BYTE_SP)) {
- env->sp = (env->sp & 0x00ff) | (data << 8);
- }
- break;
- case 0x3f: /* SREG */
- cpu_set_sreg(env, data);
- break;
- default:
- /* not a special register, pass to normal memory access */
- address_space_stb(&address_space_memory, OFFSET_IO_REGISTERS + port,
- data, MEMTXATTRS_UNSPECIFIED, NULL);
+static uint64_t avr_cpu_reg2_read(void *opaque, hwaddr addr, unsigned size)
+{
+ CPUAVRState *env = opaque;
+
+ switch (addr) {
+ case 0: /* RAMPD */
+ return 0xff & (env->rampD >> 16);
+ case 1: /* RAMPX */
+ return 0xff & (env->rampX >> 16);
+ case 2: /* RAMPY */
+ return 0xff & (env->rampY >> 16);
+ case 3: /* RAMPZ */
+ return 0xff & (env->rampZ >> 16);
+ case 4: /* EIND */
+ return 0xff & (env->eind >> 16);
+ case 5: /* SPL */
+ return env->sp & 0x00ff;
+ case 6: /* SPH */
+ return 0xff & (env->sp >> 8);
+ case 7: /* SREG */
+ return cpu_get_sreg(env);
}
+ g_assert_not_reached();
}
-/*
- * this function implements LD instruction when there is a possibility to read
- * from a CPU register
- */
-target_ulong helper_fullrd(CPUAVRState *env, uint32_t addr)
+static void avr_cpu_trap_write(void *opaque, hwaddr addr,
+ uint64_t data64, unsigned size)
{
- uint8_t data;
+ CPUAVRState *env = opaque;
+ CPUState *cs = env_cpu(env);
- env->fullacc = false;
-
- if (addr < NUMBER_OF_CPU_REGISTERS) {
- /* CPU registers */
- data = env->r[addr];
- } else if (addr < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
- /* IO registers */
- data = helper_inb(env, addr - NUMBER_OF_CPU_REGISTERS);
- } else {
- /* memory */
- data = address_space_ldub(&address_space_memory, OFFSET_DATA + addr,
- MEMTXATTRS_UNSPECIFIED, NULL);
- }
- return data;
+ env->fullacc = true;
+ cpu_loop_exit_restore(cs, cs->mem_io_pc);
}
+const MemoryRegionOps avr_cpu_reg1 = {
+ .read = avr_cpu_reg1_read,
+ .write = avr_cpu_trap_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .valid.min_access_size = 1,
+ .valid.max_access_size = 1,
+};
+
+const MemoryRegionOps avr_cpu_reg2 = {
+ .read = avr_cpu_reg2_read,
+ .write = avr_cpu_trap_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .valid.min_access_size = 1,
+ .valid.max_access_size = 1,
+};
+
/*
* this function implements ST instruction when there is a possibility to write
* into a CPU register
@@ -339,19 +269,50 @@ void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
{
env->fullacc = false;
- /* Following logic assumes this: */
- assert(OFFSET_IO_REGISTERS == OFFSET_DATA +
- NUMBER_OF_CPU_REGISTERS);
-
- if (addr < NUMBER_OF_CPU_REGISTERS) {
+ switch (addr) {
+ case 0 ... 31:
/* CPU registers */
env->r[addr] = data;
- } else if (addr < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
- /* IO registers */
- helper_outb(env, addr - NUMBER_OF_CPU_REGISTERS, data);
- } else {
- /* memory */
+ break;
+
+ case 0x58: /* RAMPD */
+ if (avr_feature(env, AVR_FEATURE_RAMPD)) {
+ env->rampD = data << 16;
+ }
+ break;
+ case 0x59: /* RAMPX */
+ if (avr_feature(env, AVR_FEATURE_RAMPX)) {
+ env->rampX = data << 16;
+ }
+ break;
+ case 0x5a: /* RAMPY */
+ if (avr_feature(env, AVR_FEATURE_RAMPY)) {
+ env->rampY = data << 16;
+ }
+ break;
+ case 0x5b: /* RAMPZ */
+ if (avr_feature(env, AVR_FEATURE_RAMPZ)) {
+ env->rampZ = data << 16;
+ }
+ break;
+ case 0x5c: /* EIDN */
+ env->eind = data << 16;
+ break;
+ case 0x5d: /* SPL */
+ env->sp = (env->sp & 0xff00) | data;
+ break;
+ case 0x5e: /* SPH */
+ if (avr_feature(env, AVR_FEATURE_2_BYTE_SP)) {
+ env->sp = (env->sp & 0x00ff) | (data << 8);
+ }
+ break;
+ case 0x5f: /* SREG */
+ cpu_set_sreg(env, data);
+ break;
+
+ default:
address_space_stb(&address_space_memory, OFFSET_DATA + addr, data,
MEMTXATTRS_UNSPECIFIED, NULL);
+ break;
}
}
diff --git a/target/avr/translate.c b/target/avr/translate.c
index e7f8ced9b3..0490936cd5 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -194,6 +194,9 @@ static bool avr_have_feature(DisasContext *ctx, int feature)
static bool decode_insn(DisasContext *ctx, uint16_t insn);
#include "decode-insn.c.inc"
+static void gen_inb(DisasContext *ctx, TCGv data, int port);
+static void gen_outb(DisasContext *ctx, TCGv data, int port);
+
/*
* Arithmetic Instructions
*/
@@ -1293,9 +1296,8 @@ static bool trans_SBRS(DisasContext *ctx, arg_SBRS *a)
static bool trans_SBIC(DisasContext *ctx, arg_SBIC *a)
{
TCGv data = tcg_temp_new_i32();
- TCGv port = tcg_constant_i32(a->reg);
- gen_helper_inb(data, tcg_env, port);
+ gen_inb(ctx, data, a->reg);
tcg_gen_andi_tl(data, data, 1 << a->bit);
ctx->skip_cond = TCG_COND_EQ;
ctx->skip_var0 = data;
@@ -1311,9 +1313,8 @@ static bool trans_SBIC(DisasContext *ctx, arg_SBIC *a)
static bool trans_SBIS(DisasContext *ctx, arg_SBIS *a)
{
TCGv data = tcg_temp_new_i32();
- TCGv port = tcg_constant_i32(a->reg);
- gen_helper_inb(data, tcg_env, port);
+ gen_inb(ctx, data, a->reg);
tcg_gen_andi_tl(data, data, 1 << a->bit);
ctx->skip_cond = TCG_COND_NE;
ctx->skip_var0 = data;
@@ -1502,11 +1503,18 @@ static void gen_data_store(DisasContext *ctx, TCGv data, TCGv addr)
static void gen_data_load(DisasContext *ctx, TCGv data, TCGv addr)
{
- if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
- gen_helper_fullrd(data, tcg_env, addr);
- } else {
- tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, MO_UB);
- }
+ tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, MO_UB);
+}
+
+static void gen_inb(DisasContext *ctx, TCGv data, int port)
+{
+ gen_data_load(ctx, data, tcg_constant_i32(port + NUMBER_OF_CPU_REGISTERS));
+}
+
+static void gen_outb(DisasContext *ctx, TCGv data, int port)
+{
+ gen_helper_fullwr(tcg_env, data,
+ tcg_constant_i32(port + NUMBER_OF_CPU_REGISTERS));
}
/*
@@ -2126,9 +2134,8 @@ static bool trans_SPMX(DisasContext *ctx, arg_SPMX *a)
static bool trans_IN(DisasContext *ctx, arg_IN *a)
{
TCGv Rd = cpu_r[a->rd];
- TCGv port = tcg_constant_i32(a->imm);
- gen_helper_inb(Rd, tcg_env, port);
+ gen_inb(ctx, Rd, a->imm);
return true;
}
@@ -2139,9 +2146,8 @@ static bool trans_IN(DisasContext *ctx, arg_IN *a)
static bool trans_OUT(DisasContext *ctx, arg_OUT *a)
{
TCGv Rd = cpu_r[a->rd];
- TCGv port = tcg_constant_i32(a->imm);
- gen_helper_outb(tcg_env, port, Rd);
+ gen_outb(ctx, Rd, a->imm);
return true;
}
@@ -2407,11 +2413,10 @@ static bool trans_SWAP(DisasContext *ctx, arg_SWAP *a)
static bool trans_SBI(DisasContext *ctx, arg_SBI *a)
{
TCGv data = tcg_temp_new_i32();
- TCGv port = tcg_constant_i32(a->reg);
- gen_helper_inb(data, tcg_env, port);
+ gen_inb(ctx, data, a->reg);
tcg_gen_ori_tl(data, data, 1 << a->bit);
- gen_helper_outb(tcg_env, port, data);
+ gen_outb(ctx, data, a->reg);
return true;
}
@@ -2422,11 +2427,10 @@ static bool trans_SBI(DisasContext *ctx, arg_SBI *a)
static bool trans_CBI(DisasContext *ctx, arg_CBI *a)
{
TCGv data = tcg_temp_new_i32();
- TCGv port = tcg_constant_i32(a->reg);
- gen_helper_inb(data, tcg_env, port);
+ gen_inb(ctx, data, a->reg);
tcg_gen_andi_tl(data, data, ~(1 << a->bit));
- gen_helper_outb(tcg_env, port, data);
+ gen_outb(ctx, data, a->reg);
return true;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 05/17] target/avr: Move cpu register accesses into system memory
2025-03-23 17:37 ` [PATCH 05/17] target/avr: Move cpu register accesses into system memory Richard Henderson
@ 2025-03-25 1:07 ` Pierrick Bouvier
2025-03-25 13:48 ` Richard Henderson
0 siblings, 1 reply; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:07 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Integrate the i/o 0x00-0x1f and 0x38-0x3f loopbacks into
> the cpu registers with normal address space accesses.
> We no longer need to trap accesses to the first page within
> avr_cpu_tlb_fill but can wait until a write occurs.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.h | 7 ++
> target/avr/helper.h | 3 -
> target/avr/cpu.c | 16 +++
> target/avr/helper.c | 239 +++++++++++++++++------------------------
> target/avr/translate.c | 42 ++++----
> 5 files changed, 146 insertions(+), 161 deletions(-)
>
> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
> index 84a8f5cc8c..be27b0152b 100644
> --- a/target/avr/cpu.h
> +++ b/target/avr/cpu.h
> @@ -23,6 +23,7 @@
>
> #include "cpu-qom.h"
> #include "exec/cpu-defs.h"
> +#include "exec/memory.h"
>
> #ifdef CONFIG_USER_ONLY
> #error "AVR 8-bit does not support user mode"
> @@ -142,6 +143,9 @@ struct ArchCPU {
>
> CPUAVRState env;
>
> + MemoryRegion cpu_reg1;
> + MemoryRegion cpu_reg2;
> +
> /* Initial value of stack pointer */
> uint32_t init_sp;
> };
> @@ -242,6 +246,9 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> MMUAccessType access_type, int mmu_idx,
> bool probe, uintptr_t retaddr);
>
> +extern const MemoryRegionOps avr_cpu_reg1;
> +extern const MemoryRegionOps avr_cpu_reg2;
> +
> #include "exec/cpu-all.h"
>
> #endif /* QEMU_AVR_CPU_H */
> diff --git a/target/avr/helper.h b/target/avr/helper.h
> index 4d02e648fa..e8d13e925f 100644
> --- a/target/avr/helper.h
> +++ b/target/avr/helper.h
> @@ -23,7 +23,4 @@ DEF_HELPER_1(debug, noreturn, env)
> DEF_HELPER_1(break, noreturn, env)
> DEF_HELPER_1(sleep, noreturn, env)
> DEF_HELPER_1(unsupported, noreturn, env)
> -DEF_HELPER_3(outb, void, env, i32, i32)
> -DEF_HELPER_2(inb, tl, env, i32)
> DEF_HELPER_3(fullwr, void, env, i32, i32)
> -DEF_HELPER_2(fullrd, tl, env, i32)
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 834c7082aa..0b14b36c17 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -23,6 +23,7 @@
> #include "qemu/qemu-print.h"
> #include "exec/exec-all.h"
> #include "exec/translation-block.h"
> +#include "exec/address-spaces.h"
> #include "cpu.h"
> #include "disas/dis-asm.h"
> #include "tcg/debug-assert.h"
> @@ -110,6 +111,8 @@ static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
> static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> CPUState *cs = CPU(dev);
> + CPUAVRState *env = cpu_env(cs);
> + AVRCPU *cpu = env_archcpu(env);
> AVRCPUClass *mcc = AVR_CPU_GET_CLASS(dev);
> Error *local_err = NULL;
>
> @@ -122,6 +125,19 @@ static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
> cpu_reset(cs);
>
> mcc->parent_realize(dev, errp);
> +
> + /*
> + * Two blocks in the low data space loop back into cpu registers.
> + */
> + memory_region_init_io(&cpu->cpu_reg1, OBJECT(cpu), &avr_cpu_reg1, env,
> + "avr-cpu-reg1", 32);
> + memory_region_add_subregion(get_system_memory(),
> + OFFSET_DATA, &cpu->cpu_reg1);
> +
> + memory_region_init_io(&cpu->cpu_reg2, OBJECT(cpu), &avr_cpu_reg2, env,
> + "avr-cpu-reg2", 8);
> + memory_region_add_subregion(get_system_memory(),
> + OFFSET_DATA + 0x58, &cpu->cpu_reg2);
> }
>
> static void avr_cpu_set_int(void *opaque, int irq, int level)
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index e5bf16c6b7..df7e2109d4 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -108,7 +108,7 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> MMUAccessType access_type, int mmu_idx,
> bool probe, uintptr_t retaddr)
> {
> - int prot, page_size = TARGET_PAGE_SIZE;
> + int prot;
> uint32_t paddr;
>
> address &= TARGET_PAGE_MASK;
> @@ -133,23 +133,9 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> /* Access to memory. */
> paddr = OFFSET_DATA + address;
> prot = PAGE_READ | PAGE_WRITE;
> - if (address < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
> - /*
> - * Access to CPU registers, exit and rebuilt this TB to use
> - * full access in case it touches specially handled registers
> - * like SREG or SP. For probing, set page_size = 1, in order
> - * to force tlb_fill to be called for the next access.
> - */
> - if (probe) {
> - page_size = 1;
> - } else {
> - cpu_env(cs)->fullacc = 1;
> - cpu_loop_exit_restore(cs, retaddr);
> - }
> - }
> }
>
> - tlb_set_page(cs, address, paddr, prot, mmu_idx, page_size);
> + tlb_set_page(cs, address, paddr, prot, mmu_idx, TARGET_PAGE_SIZE);
> return true;
> }
>
> @@ -203,134 +189,78 @@ void helper_wdr(CPUAVRState *env)
> }
>
> /*
> - * This function implements IN instruction
> - *
> - * It does the following
> - * a. if an IO register belongs to CPU, its value is read and returned
> - * b. otherwise io address is translated to mem address and physical memory
> - * is read.
> - * c. it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
> - *
> + * The first 32 bytes of the data space are mapped to the cpu regs.
> + * We cannot write these from normal store operations because TCG
> + * does not expect global temps to be modified -- a global may be
> + * live in a host cpu register across the store. We can however
> + * read these, as TCG does make sure the global temps are saved
> + * in case the load operation traps.
> */
> -target_ulong helper_inb(CPUAVRState *env, uint32_t port)
> +
> +static uint64_t avr_cpu_reg1_read(void *opaque, hwaddr addr, unsigned size)
> {
> - target_ulong data = 0;
> + CPUAVRState *env = opaque;
>
> - switch (port) {
> - case 0x38: /* RAMPD */
> - data = 0xff & (env->rampD >> 16);
> - break;
> - case 0x39: /* RAMPX */
> - data = 0xff & (env->rampX >> 16);
> - break;
> - case 0x3a: /* RAMPY */
> - data = 0xff & (env->rampY >> 16);
> - break;
> - case 0x3b: /* RAMPZ */
> - data = 0xff & (env->rampZ >> 16);
> - break;
> - case 0x3c: /* EIND */
> - data = 0xff & (env->eind >> 16);
> - break;
> - case 0x3d: /* SPL */
> - data = env->sp & 0x00ff;
> - break;
> - case 0x3e: /* SPH */
> - data = env->sp >> 8;
> - break;
> - case 0x3f: /* SREG */
> - data = cpu_get_sreg(env);
> - break;
> - default:
> - /* not a special register, pass to normal memory access */
> - data = address_space_ldub(&address_space_memory,
> - OFFSET_IO_REGISTERS + port,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - }
> -
> - return data;
> + assert(addr < 32);
> + return env->r[addr];
> }
>
> /*
> - * This function implements OUT instruction
> - *
> - * It does the following
> - * a. if an IO register belongs to CPU, its value is written into the register
> - * b. otherwise io address is translated to mem address and physical memory
> - * is written.
> - * c. it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
> - *
> + * The range 0x58-0x5f of the data space are mapped to cpu regs.
> + * As above, we cannot write these from normal store operations.
> */
> -void helper_outb(CPUAVRState *env, uint32_t port, uint32_t data)
> -{
> - data &= 0x000000ff;
>
> - switch (port) {
> - case 0x38: /* RAMPD */
> - if (avr_feature(env, AVR_FEATURE_RAMPD)) {
> - env->rampD = (data & 0xff) << 16;
> - }
> - break;
> - case 0x39: /* RAMPX */
> - if (avr_feature(env, AVR_FEATURE_RAMPX)) {
> - env->rampX = (data & 0xff) << 16;
> - }
> - break;
> - case 0x3a: /* RAMPY */
> - if (avr_feature(env, AVR_FEATURE_RAMPY)) {
> - env->rampY = (data & 0xff) << 16;
> - }
> - break;
> - case 0x3b: /* RAMPZ */
> - if (avr_feature(env, AVR_FEATURE_RAMPZ)) {
> - env->rampZ = (data & 0xff) << 16;
> - }
> - break;
> - case 0x3c: /* EIDN */
> - env->eind = (data & 0xff) << 16;
> - break;
> - case 0x3d: /* SPL */
> - env->sp = (env->sp & 0xff00) | (data);
> - break;
> - case 0x3e: /* SPH */
> - if (avr_feature(env, AVR_FEATURE_2_BYTE_SP)) {
> - env->sp = (env->sp & 0x00ff) | (data << 8);
> - }
> - break;
> - case 0x3f: /* SREG */
> - cpu_set_sreg(env, data);
> - break;
> - default:
> - /* not a special register, pass to normal memory access */
> - address_space_stb(&address_space_memory, OFFSET_IO_REGISTERS + port,
> - data, MEMTXATTRS_UNSPECIFIED, NULL);
> +static uint64_t avr_cpu_reg2_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + CPUAVRState *env = opaque;
> +
> + switch (addr) {
> + case 0: /* RAMPD */
> + return 0xff & (env->rampD >> 16);
> + case 1: /* RAMPX */
> + return 0xff & (env->rampX >> 16);
> + case 2: /* RAMPY */
> + return 0xff & (env->rampY >> 16);
> + case 3: /* RAMPZ */
> + return 0xff & (env->rampZ >> 16);
> + case 4: /* EIND */
> + return 0xff & (env->eind >> 16);
> + case 5: /* SPL */
> + return env->sp & 0x00ff;
> + case 6: /* SPH */
> + return 0xff & (env->sp >> 8);
> + case 7: /* SREG */
> + return cpu_get_sreg(env);
> }
> + g_assert_not_reached();
> }
>
> -/*
> - * this function implements LD instruction when there is a possibility to read
> - * from a CPU register
> - */
> -target_ulong helper_fullrd(CPUAVRState *env, uint32_t addr)
> +static void avr_cpu_trap_write(void *opaque, hwaddr addr,
> + uint64_t data64, unsigned size)
> {
> - uint8_t data;
> + CPUAVRState *env = opaque;
> + CPUState *cs = env_cpu(env);
>
> - env->fullacc = false;
> -
> - if (addr < NUMBER_OF_CPU_REGISTERS) {
> - /* CPU registers */
> - data = env->r[addr];
> - } else if (addr < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
> - /* IO registers */
> - data = helper_inb(env, addr - NUMBER_OF_CPU_REGISTERS);
> - } else {
> - /* memory */
> - data = address_space_ldub(&address_space_memory, OFFSET_DATA + addr,
> - MEMTXATTRS_UNSPECIFIED, NULL);
> - }
> - return data;
> + env->fullacc = true;
> + cpu_loop_exit_restore(cs, cs->mem_io_pc);
> }
>
> +const MemoryRegionOps avr_cpu_reg1 = {
> + .read = avr_cpu_reg1_read,
> + .write = avr_cpu_trap_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> + .valid.min_access_size = 1,
> + .valid.max_access_size = 1,
> +};
> +
> +const MemoryRegionOps avr_cpu_reg2 = {
> + .read = avr_cpu_reg2_read,
> + .write = avr_cpu_trap_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> + .valid.min_access_size = 1,
> + .valid.max_access_size = 1,
> +};
> +
> /*
> * this function implements ST instruction when there is a possibility to write
> * into a CPU register
> @@ -339,19 +269,50 @@ void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
> {
> env->fullacc = false;
>
> - /* Following logic assumes this: */
> - assert(OFFSET_IO_REGISTERS == OFFSET_DATA +
> - NUMBER_OF_CPU_REGISTERS);
> -
> - if (addr < NUMBER_OF_CPU_REGISTERS) {
> + switch (addr) {
> + case 0 ... 31:
> /* CPU registers */
> env->r[addr] = data;
> - } else if (addr < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
> - /* IO registers */
> - helper_outb(env, addr - NUMBER_OF_CPU_REGISTERS, data);
> - } else {
> - /* memory */
> + break;
> +
> + case 0x58: /* RAMPD */
> + if (avr_feature(env, AVR_FEATURE_RAMPD)) {
> + env->rampD = data << 16;
> + }
> + break;
> + case 0x59: /* RAMPX */
> + if (avr_feature(env, AVR_FEATURE_RAMPX)) {
> + env->rampX = data << 16;
> + }
> + break;
> + case 0x5a: /* RAMPY */
> + if (avr_feature(env, AVR_FEATURE_RAMPY)) {
> + env->rampY = data << 16;
> + }
> + break;
> + case 0x5b: /* RAMPZ */
> + if (avr_feature(env, AVR_FEATURE_RAMPZ)) {
> + env->rampZ = data << 16;
> + }
> + break;
> + case 0x5c: /* EIDN */
> + env->eind = data << 16;
> + break;
> + case 0x5d: /* SPL */
> + env->sp = (env->sp & 0xff00) | data;
> + break;
> + case 0x5e: /* SPH */
> + if (avr_feature(env, AVR_FEATURE_2_BYTE_SP)) {
> + env->sp = (env->sp & 0x00ff) | (data << 8);
> + }
> + break;
> + case 0x5f: /* SREG */
> + cpu_set_sreg(env, data);
> + break;
> +
> + default:
> address_space_stb(&address_space_memory, OFFSET_DATA + addr, data,
> MEMTXATTRS_UNSPECIFIED, NULL);
> + break;
> }
> }
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index e7f8ced9b3..0490936cd5 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -194,6 +194,9 @@ static bool avr_have_feature(DisasContext *ctx, int feature)
> static bool decode_insn(DisasContext *ctx, uint16_t insn);
> #include "decode-insn.c.inc"
>
> +static void gen_inb(DisasContext *ctx, TCGv data, int port);
> +static void gen_outb(DisasContext *ctx, TCGv data, int port);
> +
> /*
> * Arithmetic Instructions
> */
> @@ -1293,9 +1296,8 @@ static bool trans_SBRS(DisasContext *ctx, arg_SBRS *a)
> static bool trans_SBIC(DisasContext *ctx, arg_SBIC *a)
> {
> TCGv data = tcg_temp_new_i32();
> - TCGv port = tcg_constant_i32(a->reg);
>
> - gen_helper_inb(data, tcg_env, port);
> + gen_inb(ctx, data, a->reg);
> tcg_gen_andi_tl(data, data, 1 << a->bit);
> ctx->skip_cond = TCG_COND_EQ;
> ctx->skip_var0 = data;
> @@ -1311,9 +1313,8 @@ static bool trans_SBIC(DisasContext *ctx, arg_SBIC *a)
> static bool trans_SBIS(DisasContext *ctx, arg_SBIS *a)
> {
> TCGv data = tcg_temp_new_i32();
> - TCGv port = tcg_constant_i32(a->reg);
>
> - gen_helper_inb(data, tcg_env, port);
> + gen_inb(ctx, data, a->reg);
> tcg_gen_andi_tl(data, data, 1 << a->bit);
> ctx->skip_cond = TCG_COND_NE;
> ctx->skip_var0 = data;
> @@ -1502,11 +1503,18 @@ static void gen_data_store(DisasContext *ctx, TCGv data, TCGv addr)
>
> static void gen_data_load(DisasContext *ctx, TCGv data, TCGv addr)
> {
> - if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
> - gen_helper_fullrd(data, tcg_env, addr);
> - } else {
> - tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, MO_UB);
> - }
> + tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, MO_UB);
> +}
> +
> +static void gen_inb(DisasContext *ctx, TCGv data, int port)
> +{
> + gen_data_load(ctx, data, tcg_constant_i32(port + NUMBER_OF_CPU_REGISTERS));
> +}
> +
> +static void gen_outb(DisasContext *ctx, TCGv data, int port)
> +{
> + gen_helper_fullwr(tcg_env, data,
> + tcg_constant_i32(port + NUMBER_OF_CPU_REGISTERS));
> }
>
> /*
> @@ -2126,9 +2134,8 @@ static bool trans_SPMX(DisasContext *ctx, arg_SPMX *a)
> static bool trans_IN(DisasContext *ctx, arg_IN *a)
> {
> TCGv Rd = cpu_r[a->rd];
> - TCGv port = tcg_constant_i32(a->imm);
>
> - gen_helper_inb(Rd, tcg_env, port);
> + gen_inb(ctx, Rd, a->imm);
> return true;
> }
>
> @@ -2139,9 +2146,8 @@ static bool trans_IN(DisasContext *ctx, arg_IN *a)
> static bool trans_OUT(DisasContext *ctx, arg_OUT *a)
> {
> TCGv Rd = cpu_r[a->rd];
> - TCGv port = tcg_constant_i32(a->imm);
>
> - gen_helper_outb(tcg_env, port, Rd);
> + gen_outb(ctx, Rd, a->imm);
> return true;
> }
>
> @@ -2407,11 +2413,10 @@ static bool trans_SWAP(DisasContext *ctx, arg_SWAP *a)
> static bool trans_SBI(DisasContext *ctx, arg_SBI *a)
> {
> TCGv data = tcg_temp_new_i32();
> - TCGv port = tcg_constant_i32(a->reg);
>
> - gen_helper_inb(data, tcg_env, port);
> + gen_inb(ctx, data, a->reg);
> tcg_gen_ori_tl(data, data, 1 << a->bit);
> - gen_helper_outb(tcg_env, port, data);
> + gen_outb(ctx, data, a->reg);
> return true;
> }
>
> @@ -2422,11 +2427,10 @@ static bool trans_SBI(DisasContext *ctx, arg_SBI *a)
> static bool trans_CBI(DisasContext *ctx, arg_CBI *a)
> {
> TCGv data = tcg_temp_new_i32();
> - TCGv port = tcg_constant_i32(a->reg);
>
> - gen_helper_inb(data, tcg_env, port);
> + gen_inb(ctx, data, a->reg);
> tcg_gen_andi_tl(data, data, ~(1 << a->bit));
> - gen_helper_outb(tcg_env, port, data);
> + gen_outb(ctx, data, a->reg);
> return true;
> }
>
A simple nit, maybe we could define constant for register names. This
way, it can be used in the two switch for read/access.
By the way, are there other architectures where cpu registers are mapped
in memory like AVR?
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH 05/17] target/avr: Move cpu register accesses into system memory
2025-03-25 1:07 ` Pierrick Bouvier
@ 2025-03-25 13:48 ` Richard Henderson
2025-03-25 14:34 ` Pierrick Bouvier
0 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-25 13:48 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel; +Cc: mrolnik, philmd
On 3/24/25 18:07, Pierrick Bouvier wrote:
> A simple nit, maybe we could define constant for register names. This way, it can be used
> in the two switch for read/access.
Which constant?
- The absolute address (0x58-0x5f)
- The i/o port address (0x38-0x3f)
- or the region offset (0-7)
? That's where I got stuck and didn't add such.
> By the way, are there other architectures where cpu registers are mapped in memory like AVR?
Not that I know of.
r~
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 05/17] target/avr: Move cpu register accesses into system memory
2025-03-25 13:48 ` Richard Henderson
@ 2025-03-25 14:34 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 14:34 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/25/25 06:48, Richard Henderson wrote:
> On 3/24/25 18:07, Pierrick Bouvier wrote:
>> A simple nit, maybe we could define constant for register names. This way, it can be used
>> in the two switch for read/access.
>
> Which constant?
>
> - The absolute address (0x58-0x5f)
> - The i/o port address (0x38-0x3f)
> - or the region offset (0-7)
>
> ? That's where I got stuck and didn't add such.
>
Something like:
#define RAMPD 0
...
#define SREG 7
#define REG_IOPORT_BASE_ADDR 0x38
#define REG_BASE_ADDR 0x58
And in the switch, we can substract base addresses, so only cases 0..7
are left.
I don't have a strong opinion on names, just thought seeing those 2
address constants + register offset is useful and makes intention clearer.
>> By the way, are there other architectures where cpu registers are mapped in memory like AVR?
>
> Not that I know of.
>
>
> r~
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 06/17] target/avr: Use cpu_stb_mmuidx_ra in helper_fullwr
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (4 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 05/17] target/avr: Move cpu register accesses into system memory Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:08 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 07/17] target/avr: Use do_stb in avr_cpu_do_interrupt Richard Henderson
` (10 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Avoid direct use of address_space_memory.
Make use of the softmmu cache of the i/o page.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/helper.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index df7e2109d4..7cfd3d1093 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -23,10 +23,10 @@
#include "qemu/error-report.h"
#include "cpu.h"
#include "accel/tcg/cpu-ops.h"
+#include "accel/tcg/getpc.h"
#include "exec/cputlb.h"
#include "exec/page-protection.h"
#include "exec/cpu_ldst.h"
-#include "exec/address-spaces.h"
#include "exec/helper-proto.h"
bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
@@ -67,6 +67,11 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
return false;
}
+static void do_stb(CPUAVRState *env, uint32_t addr, uint8_t data, uintptr_t ra)
+{
+ cpu_stb_mmuidx_ra(env, addr, data, MMU_DATA_IDX, ra);
+}
+
void avr_cpu_do_interrupt(CPUState *cs)
{
CPUAVRState *env = cpu_env(cs);
@@ -311,8 +316,7 @@ void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
break;
default:
- address_space_stb(&address_space_memory, OFFSET_DATA + addr, data,
- MEMTXATTRS_UNSPECIFIED, NULL);
+ do_stb(env, addr, data, GETPC());
break;
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH 07/17] target/avr: Use do_stb in avr_cpu_do_interrupt
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (5 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 06/17] target/avr: Use cpu_stb_mmuidx_ra in helper_fullwr Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-23 21:31 ` Philippe Mathieu-Daudé
2025-03-25 1:09 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 08/17] target/avr: Add offset-io cpu property Richard Henderson
` (9 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/helper.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 7cfd3d1093..9608e59584 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -88,14 +88,14 @@ void avr_cpu_do_interrupt(CPUState *cs)
}
if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
- cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
- cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >> 8);
- cpu_stb_data(env, env->sp--, (ret & 0xff0000) >> 16);
+ do_stb(env, env->sp--, ret, 0);
+ do_stb(env, env->sp--, ret >> 8, 0);
+ do_stb(env, env->sp--, ret >> 16, 0);
} else if (avr_feature(env, AVR_FEATURE_2_BYTE_PC)) {
- cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
- cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >> 8);
+ do_stb(env, env->sp--, ret, 0);
+ do_stb(env, env->sp--, ret >> 8, 0);
} else {
- cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
+ do_stb(env, env->sp--, ret, 0);
}
env->pc_w = base + vector * size;
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 07/17] target/avr: Use do_stb in avr_cpu_do_interrupt
2025-03-23 17:37 ` [PATCH 07/17] target/avr: Use do_stb in avr_cpu_do_interrupt Richard Henderson
@ 2025-03-23 21:31 ` Philippe Mathieu-Daudé
2025-03-25 1:09 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:31 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 18:37, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/helper.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 07/17] target/avr: Use do_stb in avr_cpu_do_interrupt
2025-03-23 17:37 ` [PATCH 07/17] target/avr: Use do_stb in avr_cpu_do_interrupt Richard Henderson
2025-03-23 21:31 ` Philippe Mathieu-Daudé
@ 2025-03-25 1:09 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:09 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/helper.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 7cfd3d1093..9608e59584 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -88,14 +88,14 @@ void avr_cpu_do_interrupt(CPUState *cs)
> }
>
> if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
> - cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
> - cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >> 8);
> - cpu_stb_data(env, env->sp--, (ret & 0xff0000) >> 16);
> + do_stb(env, env->sp--, ret, 0);
> + do_stb(env, env->sp--, ret >> 8, 0);
> + do_stb(env, env->sp--, ret >> 16, 0);
> } else if (avr_feature(env, AVR_FEATURE_2_BYTE_PC)) {
> - cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
> - cpu_stb_data(env, env->sp--, (ret & 0x00ff00) >> 8);
> + do_stb(env, env->sp--, ret, 0);
> + do_stb(env, env->sp--, ret >> 8, 0);
> } else {
> - cpu_stb_data(env, env->sp--, (ret & 0x0000ff));
> + do_stb(env, env->sp--, ret, 0);
> }
>
> env->pc_w = base + vector * size;
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 08/17] target/avr: Add offset-io cpu property
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (6 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 07/17] target/avr: Use do_stb in avr_cpu_do_interrupt Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:10 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 09/17] target/avr: Introduce gen_data_{load,store}_raw Richard Henderson
` (8 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Communicate the offset of io within the first page between the
board, the cpu, and the translator. So far this is always 0.
This will be used to optimize memory layout.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/cpu.h | 2 ++
hw/avr/atmega.c | 2 ++
target/avr/cpu.c | 1 +
target/avr/translate.c | 2 ++
4 files changed, 7 insertions(+)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index be27b0152b..0f5e1a53bc 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -148,6 +148,8 @@ struct ArchCPU {
/* Initial value of stack pointer */
uint32_t init_sp;
+ /* Offset of the beginning of I/O within the first page. */
+ uint32_t offset_io;
};
/**
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index f6844bf118..273582b8af 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -236,6 +236,8 @@ static void atmega_realize(DeviceState *dev, Error **errp)
object_property_set_uint(OBJECT(&s->cpu), "init-sp",
mc->io_size + mc->sram_size - 1, &error_abort);
+ object_property_set_uint(OBJECT(&s->cpu), "offset-io",
+ 0, &error_abort);
qdev_realize(DEVICE(&s->cpu), NULL, &error_abort);
cpudev = DEVICE(&s->cpu);
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 0b14b36c17..080f6f30d3 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -169,6 +169,7 @@ static void avr_cpu_initfn(Object *obj)
static const Property avr_cpu_properties[] = {
DEFINE_PROP_UINT32("init-sp", AVRCPU, init_sp, 0),
+ DEFINE_PROP_UINT32("offset-io", AVRCPU, offset_io, 0),
};
static ObjectClass *avr_cpu_class_by_name(const char *cpu_model)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 0490936cd5..e9fef1aaad 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -92,6 +92,7 @@ struct DisasContext {
/* Routine used to access memory */
int memidx;
+ uint32_t offset_io;
/*
* some AVR instructions can make the following instruction to be skipped
@@ -2664,6 +2665,7 @@ static void avr_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->cs = cs;
ctx->env = cpu_env(cs);
ctx->npc = ctx->base.pc_first / 2;
+ ctx->offset_io = env_archcpu(ctx->env)->offset_io;
ctx->skip_cond = TCG_COND_NEVER;
if (tb_flags & TB_FLAGS_SKIP) {
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 08/17] target/avr: Add offset-io cpu property
2025-03-23 17:37 ` [PATCH 08/17] target/avr: Add offset-io cpu property Richard Henderson
@ 2025-03-25 1:10 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:10 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Communicate the offset of io within the first page between the
> board, the cpu, and the translator. So far this is always 0.
> This will be used to optimize memory layout.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.h | 2 ++
> hw/avr/atmega.c | 2 ++
> target/avr/cpu.c | 1 +
> target/avr/translate.c | 2 ++
> 4 files changed, 7 insertions(+)
>
> diff --git a/target/avr/cpu.h b/target/avr/cpu.h
> index be27b0152b..0f5e1a53bc 100644
> --- a/target/avr/cpu.h
> +++ b/target/avr/cpu.h
> @@ -148,6 +148,8 @@ struct ArchCPU {
>
> /* Initial value of stack pointer */
> uint32_t init_sp;
> + /* Offset of the beginning of I/O within the first page. */
> + uint32_t offset_io;
> };
>
> /**
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index f6844bf118..273582b8af 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -236,6 +236,8 @@ static void atmega_realize(DeviceState *dev, Error **errp)
>
> object_property_set_uint(OBJECT(&s->cpu), "init-sp",
> mc->io_size + mc->sram_size - 1, &error_abort);
> + object_property_set_uint(OBJECT(&s->cpu), "offset-io",
> + 0, &error_abort);
>
> qdev_realize(DEVICE(&s->cpu), NULL, &error_abort);
> cpudev = DEVICE(&s->cpu);
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 0b14b36c17..080f6f30d3 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -169,6 +169,7 @@ static void avr_cpu_initfn(Object *obj)
>
> static const Property avr_cpu_properties[] = {
> DEFINE_PROP_UINT32("init-sp", AVRCPU, init_sp, 0),
> + DEFINE_PROP_UINT32("offset-io", AVRCPU, offset_io, 0),
> };
>
> static ObjectClass *avr_cpu_class_by_name(const char *cpu_model)
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index 0490936cd5..e9fef1aaad 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -92,6 +92,7 @@ struct DisasContext {
>
> /* Routine used to access memory */
> int memidx;
> + uint32_t offset_io;
>
> /*
> * some AVR instructions can make the following instruction to be skipped
> @@ -2664,6 +2665,7 @@ static void avr_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->cs = cs;
> ctx->env = cpu_env(cs);
> ctx->npc = ctx->base.pc_first / 2;
> + ctx->offset_io = env_archcpu(ctx->env)->offset_io;
>
> ctx->skip_cond = TCG_COND_NEVER;
> if (tb_flags & TB_FLAGS_SKIP) {
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 09/17] target/avr: Introduce gen_data_{load,store}_raw
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (7 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 08/17] target/avr: Add offset-io cpu property Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:12 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 10/17] target/avr: Update cpu_sp after push and pop Richard Henderson
` (7 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Prepare for offset_io being non-zero; also allow folding
stack pointer offsets into the arithmetic.
So far, all offsets are 0.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/translate.c | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index e9fef1aaad..6bb4154dff 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -198,6 +198,28 @@ static bool decode_insn(DisasContext *ctx, uint16_t insn);
static void gen_inb(DisasContext *ctx, TCGv data, int port);
static void gen_outb(DisasContext *ctx, TCGv data, int port);
+static void gen_data_store_raw(DisasContext *ctx, TCGv data,
+ TCGv addr, int offset, MemOp mop)
+{
+ if (ctx->offset_io + offset) {
+ TCGv t = tcg_temp_new();
+ tcg_gen_addi_tl(t, addr, ctx->offset_io + offset);
+ addr = t;
+ }
+ tcg_gen_qemu_st_tl(data, addr, MMU_DATA_IDX, mop);
+}
+
+static void gen_data_load_raw(DisasContext *ctx, TCGv data,
+ TCGv addr, int offset, MemOp mop)
+{
+ if (ctx->offset_io + offset) {
+ TCGv t = tcg_temp_new();
+ tcg_gen_addi_tl(t, addr, ctx->offset_io + offset);
+ addr = t;
+ }
+ tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, mop);
+}
+
/*
* Arithmetic Instructions
*/
@@ -940,21 +962,21 @@ static void gen_push_ret(DisasContext *ctx, int ret)
if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) {
TCGv t0 = tcg_constant_i32(ret & 0x0000ff);
- tcg_gen_qemu_st_tl(t0, cpu_sp, MMU_DATA_IDX, MO_UB);
+ gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_UB);
tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
} else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
TCGv t0 = tcg_constant_i32(ret & 0x00ffff);
tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
- tcg_gen_qemu_st_tl(t0, cpu_sp, MMU_DATA_IDX, MO_BEUW);
+ gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_BEUW);
tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
} else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
TCGv lo = tcg_constant_i32(ret & 0x0000ff);
TCGv hi = tcg_constant_i32((ret & 0xffff00) >> 8);
- tcg_gen_qemu_st_tl(lo, cpu_sp, MMU_DATA_IDX, MO_UB);
+ gen_data_store_raw(ctx, lo, cpu_sp, 0, MO_UB);
tcg_gen_subi_tl(cpu_sp, cpu_sp, 2);
- tcg_gen_qemu_st_tl(hi, cpu_sp, MMU_DATA_IDX, MO_BEUW);
+ gen_data_store_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
}
}
@@ -963,20 +985,20 @@ static void gen_pop_ret(DisasContext *ctx, TCGv ret)
{
if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) {
tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
- tcg_gen_qemu_ld_tl(ret, cpu_sp, MMU_DATA_IDX, MO_UB);
+ gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_UB);
} else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
- tcg_gen_qemu_ld_tl(ret, cpu_sp, MMU_DATA_IDX, MO_BEUW);
+ gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_BEUW);
tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
} else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
TCGv lo = tcg_temp_new_i32();
TCGv hi = tcg_temp_new_i32();
tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
- tcg_gen_qemu_ld_tl(hi, cpu_sp, MMU_DATA_IDX, MO_BEUW);
+ gen_data_load_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
tcg_gen_addi_tl(cpu_sp, cpu_sp, 2);
- tcg_gen_qemu_ld_tl(lo, cpu_sp, MMU_DATA_IDX, MO_UB);
+ gen_data_load_raw(ctx, lo, cpu_sp, 0, MO_UB);
tcg_gen_deposit_tl(ret, lo, hi, 8, 16);
}
@@ -1498,13 +1520,13 @@ static void gen_data_store(DisasContext *ctx, TCGv data, TCGv addr)
if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
gen_helper_fullwr(tcg_env, data, addr);
} else {
- tcg_gen_qemu_st_tl(data, addr, MMU_DATA_IDX, MO_UB);
+ gen_data_store_raw(ctx, data, addr, 0, MO_UB);
}
}
static void gen_data_load(DisasContext *ctx, TCGv data, TCGv addr)
{
- tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, MO_UB);
+ gen_data_load_raw(ctx, data, addr, 0, MO_UB);
}
static void gen_inb(DisasContext *ctx, TCGv data, int port)
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 09/17] target/avr: Introduce gen_data_{load,store}_raw
2025-03-23 17:37 ` [PATCH 09/17] target/avr: Introduce gen_data_{load,store}_raw Richard Henderson
@ 2025-03-25 1:12 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:12 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Prepare for offset_io being non-zero; also allow folding
> stack pointer offsets into the arithmetic.
> So far, all offsets are 0.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/translate.c | 42 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index e9fef1aaad..6bb4154dff 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -198,6 +198,28 @@ static bool decode_insn(DisasContext *ctx, uint16_t insn);
> static void gen_inb(DisasContext *ctx, TCGv data, int port);
> static void gen_outb(DisasContext *ctx, TCGv data, int port);
>
> +static void gen_data_store_raw(DisasContext *ctx, TCGv data,
> + TCGv addr, int offset, MemOp mop)
> +{
> + if (ctx->offset_io + offset) {
> + TCGv t = tcg_temp_new();
> + tcg_gen_addi_tl(t, addr, ctx->offset_io + offset);
> + addr = t;
> + }
> + tcg_gen_qemu_st_tl(data, addr, MMU_DATA_IDX, mop);
> +}
> +
> +static void gen_data_load_raw(DisasContext *ctx, TCGv data,
> + TCGv addr, int offset, MemOp mop)
> +{
> + if (ctx->offset_io + offset) {
> + TCGv t = tcg_temp_new();
> + tcg_gen_addi_tl(t, addr, ctx->offset_io + offset);
> + addr = t;
> + }
> + tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, mop);
> +}
> +
> /*
> * Arithmetic Instructions
> */
> @@ -940,21 +962,21 @@ static void gen_push_ret(DisasContext *ctx, int ret)
> if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) {
> TCGv t0 = tcg_constant_i32(ret & 0x0000ff);
>
> - tcg_gen_qemu_st_tl(t0, cpu_sp, MMU_DATA_IDX, MO_UB);
> + gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_UB);
> tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
> } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
> TCGv t0 = tcg_constant_i32(ret & 0x00ffff);
>
> tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
> - tcg_gen_qemu_st_tl(t0, cpu_sp, MMU_DATA_IDX, MO_BEUW);
> + gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_BEUW);
> tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
> } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
> TCGv lo = tcg_constant_i32(ret & 0x0000ff);
> TCGv hi = tcg_constant_i32((ret & 0xffff00) >> 8);
>
> - tcg_gen_qemu_st_tl(lo, cpu_sp, MMU_DATA_IDX, MO_UB);
> + gen_data_store_raw(ctx, lo, cpu_sp, 0, MO_UB);
> tcg_gen_subi_tl(cpu_sp, cpu_sp, 2);
> - tcg_gen_qemu_st_tl(hi, cpu_sp, MMU_DATA_IDX, MO_BEUW);
> + gen_data_store_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
> tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
> }
> }
> @@ -963,20 +985,20 @@ static void gen_pop_ret(DisasContext *ctx, TCGv ret)
> {
> if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) {
> tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> - tcg_gen_qemu_ld_tl(ret, cpu_sp, MMU_DATA_IDX, MO_UB);
> + gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_UB);
> } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
> tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> - tcg_gen_qemu_ld_tl(ret, cpu_sp, MMU_DATA_IDX, MO_BEUW);
> + gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_BEUW);
> tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
> TCGv lo = tcg_temp_new_i32();
> TCGv hi = tcg_temp_new_i32();
>
> tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> - tcg_gen_qemu_ld_tl(hi, cpu_sp, MMU_DATA_IDX, MO_BEUW);
> + gen_data_load_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
>
> tcg_gen_addi_tl(cpu_sp, cpu_sp, 2);
> - tcg_gen_qemu_ld_tl(lo, cpu_sp, MMU_DATA_IDX, MO_UB);
> + gen_data_load_raw(ctx, lo, cpu_sp, 0, MO_UB);
>
> tcg_gen_deposit_tl(ret, lo, hi, 8, 16);
> }
> @@ -1498,13 +1520,13 @@ static void gen_data_store(DisasContext *ctx, TCGv data, TCGv addr)
> if (ctx->base.tb->flags & TB_FLAGS_FULL_ACCESS) {
> gen_helper_fullwr(tcg_env, data, addr);
> } else {
> - tcg_gen_qemu_st_tl(data, addr, MMU_DATA_IDX, MO_UB);
> + gen_data_store_raw(ctx, data, addr, 0, MO_UB);
> }
> }
>
> static void gen_data_load(DisasContext *ctx, TCGv data, TCGv addr)
> {
> - tcg_gen_qemu_ld_tl(data, addr, MMU_DATA_IDX, MO_UB);
> + gen_data_load_raw(ctx, data, addr, 0, MO_UB);
> }
>
> static void gen_inb(DisasContext *ctx, TCGv data, int port)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 10/17] target/avr: Update cpu_sp after push and pop
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (8 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 09/17] target/avr: Introduce gen_data_{load,store}_raw Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:36 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 11/17] target/avr: Implement CPUState.memory_rw_debug Richard Henderson
` (6 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Not that AVR has memory paging traps, but it's better
form to allow the memory operation to finish before
updating the cpu register.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/translate.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 6bb4154dff..3446007be1 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -967,40 +967,38 @@ static void gen_push_ret(DisasContext *ctx, int ret)
} else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
TCGv t0 = tcg_constant_i32(ret & 0x00ffff);
- tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
- gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_BEUW);
- tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
+ gen_data_store_raw(ctx, t0, cpu_sp, -1, MO_BEUW);
+ tcg_gen_subi_tl(cpu_sp, cpu_sp, 2);
} else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
TCGv lo = tcg_constant_i32(ret & 0x0000ff);
TCGv hi = tcg_constant_i32((ret & 0xffff00) >> 8);
gen_data_store_raw(ctx, lo, cpu_sp, 0, MO_UB);
- tcg_gen_subi_tl(cpu_sp, cpu_sp, 2);
- gen_data_store_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
- tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
+ gen_data_store_raw(ctx, hi, cpu_sp, -2, MO_BEUW);
+ tcg_gen_subi_tl(cpu_sp, cpu_sp, 3);
+ } else {
+ g_assert_not_reached();
}
}
static void gen_pop_ret(DisasContext *ctx, TCGv ret)
{
if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) {
+ gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_UB);
tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
- gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_UB);
} else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
- tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
- gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_BEUW);
- tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
+ gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_BEUW);
+ tcg_gen_addi_tl(cpu_sp, cpu_sp, 2);
} else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
- TCGv lo = tcg_temp_new_i32();
TCGv hi = tcg_temp_new_i32();
- tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
- gen_data_load_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
+ gen_data_load_raw(ctx, hi, cpu_sp, 1, MO_BEUW);
+ gen_data_load_raw(ctx, ret, cpu_sp, 3, MO_UB);
+ tcg_gen_addi_tl(cpu_sp, cpu_sp, 3);
- tcg_gen_addi_tl(cpu_sp, cpu_sp, 2);
- gen_data_load_raw(ctx, lo, cpu_sp, 0, MO_UB);
-
- tcg_gen_deposit_tl(ret, lo, hi, 8, 16);
+ tcg_gen_deposit_tl(ret, ret, hi, 8, 16);
+ } else {
+ g_assert_not_reached();
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 10/17] target/avr: Update cpu_sp after push and pop
2025-03-23 17:37 ` [PATCH 10/17] target/avr: Update cpu_sp after push and pop Richard Henderson
@ 2025-03-25 1:36 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:36 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Not that AVR has memory paging traps, but it's better
> form to allow the memory operation to finish before
> updating the cpu register.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/translate.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index 6bb4154dff..3446007be1 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -967,40 +967,38 @@ static void gen_push_ret(DisasContext *ctx, int ret)
> } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
> TCGv t0 = tcg_constant_i32(ret & 0x00ffff);
>
> - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
> - gen_data_store_raw(ctx, t0, cpu_sp, 0, MO_BEUW);
> - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
> + gen_data_store_raw(ctx, t0, cpu_sp, -1, MO_BEUW);
> + tcg_gen_subi_tl(cpu_sp, cpu_sp, 2);
> } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
> TCGv lo = tcg_constant_i32(ret & 0x0000ff);
> TCGv hi = tcg_constant_i32((ret & 0xffff00) >> 8);
>
> gen_data_store_raw(ctx, lo, cpu_sp, 0, MO_UB);
> - tcg_gen_subi_tl(cpu_sp, cpu_sp, 2);
> - gen_data_store_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
> - tcg_gen_subi_tl(cpu_sp, cpu_sp, 1);
> + gen_data_store_raw(ctx, hi, cpu_sp, -2, MO_BEUW);
> + tcg_gen_subi_tl(cpu_sp, cpu_sp, 3);
> + } else {
> + g_assert_not_reached();
> }
> }
>
> static void gen_pop_ret(DisasContext *ctx, TCGv ret)
> {
> if (avr_feature(ctx->env, AVR_FEATURE_1_BYTE_PC)) {
> + gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_UB);
> tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> - gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_UB);
> } else if (avr_feature(ctx->env, AVR_FEATURE_2_BYTE_PC)) {
> - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> - gen_data_load_raw(ctx, ret, cpu_sp, 0, MO_BEUW);
> - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> + gen_data_load_raw(ctx, ret, cpu_sp, 1, MO_BEUW);
> + tcg_gen_addi_tl(cpu_sp, cpu_sp, 2);
> } else if (avr_feature(ctx->env, AVR_FEATURE_3_BYTE_PC)) {
> - TCGv lo = tcg_temp_new_i32();
> TCGv hi = tcg_temp_new_i32();
>
> - tcg_gen_addi_tl(cpu_sp, cpu_sp, 1);
> - gen_data_load_raw(ctx, hi, cpu_sp, 0, MO_BEUW);
> + gen_data_load_raw(ctx, hi, cpu_sp, 1, MO_BEUW);
> + gen_data_load_raw(ctx, ret, cpu_sp, 3, MO_UB);
> + tcg_gen_addi_tl(cpu_sp, cpu_sp, 3);
>
> - tcg_gen_addi_tl(cpu_sp, cpu_sp, 2);
> - gen_data_load_raw(ctx, lo, cpu_sp, 0, MO_UB);
> -
> - tcg_gen_deposit_tl(ret, lo, hi, 8, 16);
> + tcg_gen_deposit_tl(ret, ret, hi, 8, 16);
> + } else {
> + g_assert_not_reached();
> }
> }
>
A bit hard to review with the side effect of gen_data_*_raw, but number
seems to match.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 11/17] target/avr: Implement CPUState.memory_rw_debug
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (9 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 10/17] target/avr: Update cpu_sp after push and pop Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-23 21:33 ` Philippe Mathieu-Daudé
2025-03-25 1:19 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 12/17] target/avr: Handle offset_io in helper.c Richard Henderson
` (5 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Prepare for offset_io being non-zero when accessing from gdb.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/cpu.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 080f6f30d3..e4011004b4 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -68,6 +68,35 @@ static void avr_restore_state_to_opc(CPUState *cs,
cpu_env(cs)->pc_w = data[0];
}
+static int avr_memory_rw_debug(CPUState *cpu, vaddr addr,
+ uint8_t *buf, size_t len, bool is_write)
+{
+ if (addr < OFFSET_DATA) {
+ size_t len_code;
+ int ret;
+
+ if (addr + len <= OFFSET_DATA) {
+ return cpu_memory_rw_debug(cpu, addr, buf, len, is_write);
+ }
+
+ len_code = addr + len - OFFSET_DATA;
+ ret = cpu_memory_rw_debug(cpu, addr, buf, len_code, is_write);
+ if (ret != 0) {
+ return ret;
+ }
+ addr = OFFSET_DATA;
+ len -= len_code;
+ }
+
+ /*
+ * Data is biased such that SRAM begins at TARGET_PAGE_SIZE,
+ * and I/O is immediately prior. This leave a hole between
+ * OFFSET_DATA and the relative start of the address space.
+ */
+ addr += env_archcpu(cpu_env(cpu))->offset_io;
+ return cpu_memory_rw_debug(cpu, addr, buf, len, is_write);
+}
+
static void avr_cpu_reset_hold(Object *obj, ResetType type)
{
CPUState *cs = CPU(obj);
@@ -262,6 +291,7 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
cc->gdb_write_register = avr_cpu_gdb_write_register;
cc->gdb_adjust_breakpoint = avr_cpu_gdb_adjust_breakpoint;
cc->gdb_core_xml_file = "avr-cpu.xml";
+ cc->memory_rw_debug = avr_memory_rw_debug;
cc->tcg_ops = &avr_tcg_ops;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 11/17] target/avr: Implement CPUState.memory_rw_debug
2025-03-23 17:37 ` [PATCH 11/17] target/avr: Implement CPUState.memory_rw_debug Richard Henderson
@ 2025-03-23 21:33 ` Philippe Mathieu-Daudé
2025-03-25 1:19 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:33 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 18:37, Richard Henderson wrote:
> Prepare for offset_io being non-zero when accessing from gdb.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 11/17] target/avr: Implement CPUState.memory_rw_debug
2025-03-23 17:37 ` [PATCH 11/17] target/avr: Implement CPUState.memory_rw_debug Richard Henderson
2025-03-23 21:33 ` Philippe Mathieu-Daudé
@ 2025-03-25 1:19 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:19 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Prepare for offset_io being non-zero when accessing from gdb.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 080f6f30d3..e4011004b4 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -68,6 +68,35 @@ static void avr_restore_state_to_opc(CPUState *cs,
> cpu_env(cs)->pc_w = data[0];
> }
>
> +static int avr_memory_rw_debug(CPUState *cpu, vaddr addr,
> + uint8_t *buf, size_t len, bool is_write)
> +{
> + if (addr < OFFSET_DATA) {
> + size_t len_code;
> + int ret;
> +
> + if (addr + len <= OFFSET_DATA) {
> + return cpu_memory_rw_debug(cpu, addr, buf, len, is_write);
> + }
> +
> + len_code = addr + len - OFFSET_DATA;
> + ret = cpu_memory_rw_debug(cpu, addr, buf, len_code, is_write);
> + if (ret != 0) {
> + return ret;
> + }
> + addr = OFFSET_DATA;
> + len -= len_code;
> + }
> +
> + /*
> + * Data is biased such that SRAM begins at TARGET_PAGE_SIZE,
> + * and I/O is immediately prior. This leave a hole between
> + * OFFSET_DATA and the relative start of the address space.
> + */
> + addr += env_archcpu(cpu_env(cpu))->offset_io;
> + return cpu_memory_rw_debug(cpu, addr, buf, len, is_write);
> +}
> +
> static void avr_cpu_reset_hold(Object *obj, ResetType type)
> {
> CPUState *cs = CPU(obj);
> @@ -262,6 +291,7 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
> cc->gdb_write_register = avr_cpu_gdb_write_register;
> cc->gdb_adjust_breakpoint = avr_cpu_gdb_adjust_breakpoint;
> cc->gdb_core_xml_file = "avr-cpu.xml";
> + cc->memory_rw_debug = avr_memory_rw_debug;
> cc->tcg_ops = &avr_tcg_ops;
> }
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 12/17] target/avr: Handle offset_io in helper.c
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (10 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 11/17] target/avr: Implement CPUState.memory_rw_debug Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-23 21:34 ` Philippe Mathieu-Daudé
2025-03-25 1:20 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn Richard Henderson
` (4 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Prepare for offset_io being non-zero in do_stb.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index 9608e59584..3323f32c22 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -69,7 +69,8 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
static void do_stb(CPUAVRState *env, uint32_t addr, uint8_t data, uintptr_t ra)
{
- cpu_stb_mmuidx_ra(env, addr, data, MMU_DATA_IDX, ra);
+ cpu_stb_mmuidx_ra(env, addr + env_archcpu(env)->offset_io,
+ data, MMU_DATA_IDX, ra);
}
void avr_cpu_do_interrupt(CPUState *cs)
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 12/17] target/avr: Handle offset_io in helper.c
2025-03-23 17:37 ` [PATCH 12/17] target/avr: Handle offset_io in helper.c Richard Henderson
@ 2025-03-23 21:34 ` Philippe Mathieu-Daudé
2025-03-25 1:20 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:34 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 18:37, Richard Henderson wrote:
> Prepare for offset_io being non-zero in do_stb.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/helper.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 12/17] target/avr: Handle offset_io in helper.c
2025-03-23 17:37 ` [PATCH 12/17] target/avr: Handle offset_io in helper.c Richard Henderson
2025-03-23 21:34 ` Philippe Mathieu-Daudé
@ 2025-03-25 1:20 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:20 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Prepare for offset_io being non-zero in do_stb.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/helper.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 9608e59584..3323f32c22 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -69,7 +69,8 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>
> static void do_stb(CPUAVRState *env, uint32_t addr, uint8_t data, uintptr_t ra)
> {
> - cpu_stb_mmuidx_ra(env, addr, data, MMU_DATA_IDX, ra);
> + cpu_stb_mmuidx_ra(env, addr + env_archcpu(env)->offset_io,
> + data, MMU_DATA_IDX, ra);
> }
>
> void avr_cpu_do_interrupt(CPUState *cs)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (11 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 12/17] target/avr: Handle offset_io in helper.c Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-23 21:35 ` Philippe Mathieu-Daudé
2025-03-25 1:20 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 14/17] hw/avr: Set offset_io and increase page size to 1k Richard Henderson
` (3 subsequent siblings)
16 siblings, 2 replies; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/cpu.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index e4011004b4..538fcbc215 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -161,12 +161,14 @@ static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
memory_region_init_io(&cpu->cpu_reg1, OBJECT(cpu), &avr_cpu_reg1, env,
"avr-cpu-reg1", 32);
memory_region_add_subregion(get_system_memory(),
- OFFSET_DATA, &cpu->cpu_reg1);
+ OFFSET_DATA + cpu->offset_io,
+ &cpu->cpu_reg1);
memory_region_init_io(&cpu->cpu_reg2, OBJECT(cpu), &avr_cpu_reg2, env,
"avr-cpu-reg2", 8);
memory_region_add_subregion(get_system_memory(),
- OFFSET_DATA + 0x58, &cpu->cpu_reg2);
+ OFFSET_DATA + cpu->offset_io + 0x58,
+ &cpu->cpu_reg2);
}
static void avr_cpu_set_int(void *opaque, int irq, int level)
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn
2025-03-23 17:37 ` [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn Richard Henderson
@ 2025-03-23 21:35 ` Philippe Mathieu-Daudé
2025-03-23 21:38 ` Philippe Mathieu-Daudé
2025-03-25 1:20 ` Pierrick Bouvier
1 sibling, 1 reply; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:35 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 18:37, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index e4011004b4..538fcbc215 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -161,12 +161,14 @@ static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
> memory_region_init_io(&cpu->cpu_reg1, OBJECT(cpu), &avr_cpu_reg1, env,
> "avr-cpu-reg1", 32);
> memory_region_add_subregion(get_system_memory(),
> - OFFSET_DATA, &cpu->cpu_reg1);
> + OFFSET_DATA + cpu->offset_io,
> + &cpu->cpu_reg1);
>
> memory_region_init_io(&cpu->cpu_reg2, OBJECT(cpu), &avr_cpu_reg2, env,
> "avr-cpu-reg2", 8);
> memory_region_add_subregion(get_system_memory(),
> - OFFSET_DATA + 0x58, &cpu->cpu_reg2);
> + OFFSET_DATA + cpu->offset_io + 0x58,
> + &cpu->cpu_reg2);
Always zero, but I agree it is clearer.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> }
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn
2025-03-23 21:35 ` Philippe Mathieu-Daudé
@ 2025-03-23 21:38 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:38 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 22:35, Philippe Mathieu-Daudé wrote:
> On 23/3/25 18:37, Richard Henderson wrote:
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> target/avr/cpu.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
>> index e4011004b4..538fcbc215 100644
>> --- a/target/avr/cpu.c
>> +++ b/target/avr/cpu.c
>> @@ -161,12 +161,14 @@ static void avr_cpu_realizefn(DeviceState *dev,
>> Error **errp)
>> memory_region_init_io(&cpu->cpu_reg1, OBJECT(cpu),
>> &avr_cpu_reg1, env,
>> "avr-cpu-reg1", 32);
>> memory_region_add_subregion(get_system_memory(),
>> - OFFSET_DATA, &cpu->cpu_reg1);
>> + OFFSET_DATA + cpu->offset_io,
>> + &cpu->cpu_reg1);
>> memory_region_init_io(&cpu->cpu_reg2, OBJECT(cpu),
>> &avr_cpu_reg2, env,
>> "avr-cpu-reg2", 8);
>> memory_region_add_subregion(get_system_memory(),
>> - OFFSET_DATA + 0x58, &cpu->cpu_reg2);
>> + OFFSET_DATA + cpu->offset_io + 0x58,
>> + &cpu->cpu_reg2);
>
> Always zero, but I agree it is clearer.
I figured out the use after reading the following patch ;)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>> }
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn
2025-03-23 17:37 ` [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn Richard Henderson
2025-03-23 21:35 ` Philippe Mathieu-Daudé
@ 2025-03-25 1:20 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:20 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index e4011004b4..538fcbc215 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -161,12 +161,14 @@ static void avr_cpu_realizefn(DeviceState *dev, Error **errp)
> memory_region_init_io(&cpu->cpu_reg1, OBJECT(cpu), &avr_cpu_reg1, env,
> "avr-cpu-reg1", 32);
> memory_region_add_subregion(get_system_memory(),
> - OFFSET_DATA, &cpu->cpu_reg1);
> + OFFSET_DATA + cpu->offset_io,
> + &cpu->cpu_reg1);
>
> memory_region_init_io(&cpu->cpu_reg2, OBJECT(cpu), &avr_cpu_reg2, env,
> "avr-cpu-reg2", 8);
> memory_region_add_subregion(get_system_memory(),
> - OFFSET_DATA + 0x58, &cpu->cpu_reg2);
> + OFFSET_DATA + cpu->offset_io + 0x58,
> + &cpu->cpu_reg2);
> }
>
> static void avr_cpu_set_int(void *opaque, int irq, int level)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 14/17] hw/avr: Set offset_io and increase page size to 1k
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (12 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 13/17] target/avr: Handle offset_io in avr_cpu_realizefn Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:21 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 15/17] hw/avr: Pass mcu_type to class_base_init via .class_data Richard Henderson
` (2 subsequent siblings)
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/cpu-param.h | 8 +------
hw/avr/atmega.c | 54 ++++++++++++++++++++++++++----------------
2 files changed, 35 insertions(+), 27 deletions(-)
diff --git a/target/avr/cpu-param.h b/target/avr/cpu-param.h
index 81f3f49ee1..f5248ce9e7 100644
--- a/target/avr/cpu-param.h
+++ b/target/avr/cpu-param.h
@@ -21,13 +21,7 @@
#ifndef AVR_CPU_PARAM_H
#define AVR_CPU_PARAM_H
-/*
- * TARGET_PAGE_BITS cannot be more than 8 bits because
- * 1. all IO registers occupy [0x0000 .. 0x00ff] address range, and they
- * should be implemented as a device and not memory
- * 2. SRAM starts at the address 0x0100
- */
-#define TARGET_PAGE_BITS 8
+#define TARGET_PAGE_BITS 10
#define TARGET_PHYS_ADDR_SPACE_BITS 24
#define TARGET_VIRT_ADDR_SPACE_BITS 24
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 273582b8af..d4fc9c4aee 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -14,6 +14,7 @@
#include "qapi/error.h"
#include "exec/memory.h"
#include "exec/address-spaces.h"
+#include "exec/target_page.h"
#include "system/system.h"
#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
@@ -222,6 +223,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
DeviceState *cpudev;
SysBusDevice *sbd;
char *devname;
+ uint32_t offset_io, offset_sram;
size_t i;
assert(mc->io_size <= 0x200);
@@ -231,13 +233,25 @@ static void atmega_realize(DeviceState *dev, Error **errp)
return;
}
+ /*
+ * Bias the virtual data section so that the start of RAM is at
+ * the start of the second page of the physical data section.
+ * This puts all of the I/O at the end of the first page of the
+ * physical data section.
+ */
+ offset_io = TARGET_PAGE_SIZE - mc->io_size;
+ offset_sram = TARGET_PAGE_SIZE;
+
/* CPU */
object_initialize_child(OBJECT(dev), "cpu", &s->cpu, mc->cpu_type);
object_property_set_uint(OBJECT(&s->cpu), "init-sp",
mc->io_size + mc->sram_size - 1, &error_abort);
object_property_set_uint(OBJECT(&s->cpu), "offset-io",
- 0, &error_abort);
+ offset_io, &error_abort);
+
+ offset_io += OFFSET_DATA;
+ offset_sram += OFFSET_DATA;
qdev_realize(DEVICE(&s->cpu), NULL, &error_abort);
cpudev = DEVICE(&s->cpu);
@@ -245,8 +259,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
/* SRAM */
memory_region_init_ram(&s->sram, OBJECT(dev), "sram", mc->sram_size,
&error_abort);
- memory_region_add_subregion(get_system_memory(),
- OFFSET_DATA + mc->io_size, &s->sram);
+ memory_region_add_subregion(get_system_memory(), offset_sram, &s->sram);
/* Flash */
memory_region_init_rom(&s->flash, OBJECT(dev),
@@ -258,13 +271,14 @@ static void atmega_realize(DeviceState *dev, Error **errp)
*
* 0x00 - 0x1f: Registers
* 0x20 - 0x5f: I/O memory
- * 0x60 - 0xff: Extended I/O
+ * 0x60 - 0x[1]ff: Extended I/O
*/
s->io = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
qdev_prop_set_string(s->io, "name", "I/O");
qdev_prop_set_uint64(s->io, "size", mc->io_size);
sysbus_realize_and_unref(SYS_BUS_DEVICE(s->io), &error_fatal);
- sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->io), 0, OFFSET_DATA, -1234);
+
+ sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->io), 0, offset_io, -1234);
/* Power Reduction */
for (i = 0; i < POWER_MAX; i++) {
@@ -277,7 +291,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
TYPE_AVR_MASK);
sysbus_realize(SYS_BUS_DEVICE(&s->pwr[i]), &error_abort);
sysbus_mmio_map(SYS_BUS_DEVICE(&s->pwr[i]), 0,
- OFFSET_DATA + mc->dev[idx].addr);
+ offset_io + mc->dev[idx].addr);
g_free(devname);
}
@@ -289,7 +303,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
}
devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
create_unimplemented_device(devname,
- OFFSET_DATA + mc->dev[idx].addr, 3);
+ offset_io + mc->dev[idx].addr, 3);
g_free(devname);
}
@@ -305,7 +319,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
qdev_prop_set_chr(DEVICE(&s->usart[i]), "chardev", serial_hd(i));
sbd = SYS_BUS_DEVICE(&s->usart[i]);
sysbus_realize(sbd, &error_abort);
- sysbus_mmio_map(sbd, 0, OFFSET_DATA + mc->dev[USART(i)].addr);
+ sysbus_mmio_map(sbd, 0, offset_io + mc->dev[USART(i)].addr);
connect_peripheral_irq(mc, sbd, 0, cpudev, USART_RXC_IRQ(i));
connect_peripheral_irq(mc, sbd, 1, cpudev, USART_DRE_IRQ(i));
connect_peripheral_irq(mc, sbd, 2, cpudev, USART_TXC_IRQ(i));
@@ -321,12 +335,12 @@ static void atmega_realize(DeviceState *dev, Error **errp)
}
if (!mc->dev[idx].is_timer16) {
create_unimplemented_device("avr-timer8",
- OFFSET_DATA + mc->dev[idx].addr, 5);
+ offset_io + mc->dev[idx].addr, 5);
create_unimplemented_device("avr-timer8-intmask",
- OFFSET_DATA
+ offset_io
+ mc->dev[idx].intmask_addr, 1);
create_unimplemented_device("avr-timer8-intflag",
- OFFSET_DATA
+ offset_io
+ mc->dev[idx].intflag_addr, 1);
continue;
}
@@ -337,9 +351,9 @@ static void atmega_realize(DeviceState *dev, Error **errp)
s->xtal_freq_hz, &error_abort);
sbd = SYS_BUS_DEVICE(&s->timer[i]);
sysbus_realize(sbd, &error_abort);
- sysbus_mmio_map(sbd, 0, OFFSET_DATA + mc->dev[idx].addr);
- sysbus_mmio_map(sbd, 1, OFFSET_DATA + mc->dev[idx].intmask_addr);
- sysbus_mmio_map(sbd, 2, OFFSET_DATA + mc->dev[idx].intflag_addr);
+ sysbus_mmio_map(sbd, 0, offset_io + mc->dev[idx].addr);
+ sysbus_mmio_map(sbd, 1, offset_io + mc->dev[idx].intmask_addr);
+ sysbus_mmio_map(sbd, 2, offset_io + mc->dev[idx].intflag_addr);
connect_peripheral_irq(mc, sbd, 0, cpudev, TIMER_CAPT_IRQ(i));
connect_peripheral_irq(mc, sbd, 1, cpudev, TIMER_COMPA_IRQ(i));
connect_peripheral_irq(mc, sbd, 2, cpudev, TIMER_COMPB_IRQ(i));
@@ -349,12 +363,12 @@ static void atmega_realize(DeviceState *dev, Error **errp)
g_free(devname);
}
- create_unimplemented_device("avr-twi", OFFSET_DATA + 0x0b8, 6);
- create_unimplemented_device("avr-adc", OFFSET_DATA + 0x078, 8);
- create_unimplemented_device("avr-ext-mem-ctrl", OFFSET_DATA + 0x074, 2);
- create_unimplemented_device("avr-watchdog", OFFSET_DATA + 0x060, 1);
- create_unimplemented_device("avr-spi", OFFSET_DATA + 0x04c, 3);
- create_unimplemented_device("avr-eeprom", OFFSET_DATA + 0x03f, 3);
+ create_unimplemented_device("avr-twi", offset_io + 0x0b8, 6);
+ create_unimplemented_device("avr-adc", offset_io + 0x078, 8);
+ create_unimplemented_device("avr-ext-mem-ctrl", offset_io + 0x074, 2);
+ create_unimplemented_device("avr-watchdog", offset_io + 0x060, 1);
+ create_unimplemented_device("avr-spi", offset_io + 0x04c, 3);
+ create_unimplemented_device("avr-eeprom", offset_io + 0x03f, 3);
}
static const Property atmega_props[] = {
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 14/17] hw/avr: Set offset_io and increase page size to 1k
2025-03-23 17:37 ` [PATCH 14/17] hw/avr: Set offset_io and increase page size to 1k Richard Henderson
@ 2025-03-25 1:21 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:21 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu-param.h | 8 +------
> hw/avr/atmega.c | 54 ++++++++++++++++++++++++++----------------
> 2 files changed, 35 insertions(+), 27 deletions(-)
>
> diff --git a/target/avr/cpu-param.h b/target/avr/cpu-param.h
> index 81f3f49ee1..f5248ce9e7 100644
> --- a/target/avr/cpu-param.h
> +++ b/target/avr/cpu-param.h
> @@ -21,13 +21,7 @@
> #ifndef AVR_CPU_PARAM_H
> #define AVR_CPU_PARAM_H
>
> -/*
> - * TARGET_PAGE_BITS cannot be more than 8 bits because
> - * 1. all IO registers occupy [0x0000 .. 0x00ff] address range, and they
> - * should be implemented as a device and not memory
> - * 2. SRAM starts at the address 0x0100
> - */
> -#define TARGET_PAGE_BITS 8
> +#define TARGET_PAGE_BITS 10
> #define TARGET_PHYS_ADDR_SPACE_BITS 24
> #define TARGET_VIRT_ADDR_SPACE_BITS 24
>
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index 273582b8af..d4fc9c4aee 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -14,6 +14,7 @@
> #include "qapi/error.h"
> #include "exec/memory.h"
> #include "exec/address-spaces.h"
> +#include "exec/target_page.h"
> #include "system/system.h"
> #include "hw/qdev-properties.h"
> #include "hw/sysbus.h"
> @@ -222,6 +223,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> DeviceState *cpudev;
> SysBusDevice *sbd;
> char *devname;
> + uint32_t offset_io, offset_sram;
> size_t i;
>
> assert(mc->io_size <= 0x200);
> @@ -231,13 +233,25 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> + /*
> + * Bias the virtual data section so that the start of RAM is at
> + * the start of the second page of the physical data section.
> + * This puts all of the I/O at the end of the first page of the
> + * physical data section.
> + */
> + offset_io = TARGET_PAGE_SIZE - mc->io_size;
> + offset_sram = TARGET_PAGE_SIZE;
> +
> /* CPU */
> object_initialize_child(OBJECT(dev), "cpu", &s->cpu, mc->cpu_type);
>
> object_property_set_uint(OBJECT(&s->cpu), "init-sp",
> mc->io_size + mc->sram_size - 1, &error_abort);
> object_property_set_uint(OBJECT(&s->cpu), "offset-io",
> - 0, &error_abort);
> + offset_io, &error_abort);
> +
> + offset_io += OFFSET_DATA;
> + offset_sram += OFFSET_DATA;
>
> qdev_realize(DEVICE(&s->cpu), NULL, &error_abort);
> cpudev = DEVICE(&s->cpu);
> @@ -245,8 +259,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> /* SRAM */
> memory_region_init_ram(&s->sram, OBJECT(dev), "sram", mc->sram_size,
> &error_abort);
> - memory_region_add_subregion(get_system_memory(),
> - OFFSET_DATA + mc->io_size, &s->sram);
> + memory_region_add_subregion(get_system_memory(), offset_sram, &s->sram);
>
> /* Flash */
> memory_region_init_rom(&s->flash, OBJECT(dev),
> @@ -258,13 +271,14 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> *
> * 0x00 - 0x1f: Registers
> * 0x20 - 0x5f: I/O memory
> - * 0x60 - 0xff: Extended I/O
> + * 0x60 - 0x[1]ff: Extended I/O
> */
> s->io = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
> qdev_prop_set_string(s->io, "name", "I/O");
> qdev_prop_set_uint64(s->io, "size", mc->io_size);
> sysbus_realize_and_unref(SYS_BUS_DEVICE(s->io), &error_fatal);
> - sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->io), 0, OFFSET_DATA, -1234);
> +
> + sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->io), 0, offset_io, -1234);
>
> /* Power Reduction */
> for (i = 0; i < POWER_MAX; i++) {
> @@ -277,7 +291,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> TYPE_AVR_MASK);
> sysbus_realize(SYS_BUS_DEVICE(&s->pwr[i]), &error_abort);
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->pwr[i]), 0,
> - OFFSET_DATA + mc->dev[idx].addr);
> + offset_io + mc->dev[idx].addr);
> g_free(devname);
> }
>
> @@ -289,7 +303,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> }
> devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
> create_unimplemented_device(devname,
> - OFFSET_DATA + mc->dev[idx].addr, 3);
> + offset_io + mc->dev[idx].addr, 3);
> g_free(devname);
> }
>
> @@ -305,7 +319,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> qdev_prop_set_chr(DEVICE(&s->usart[i]), "chardev", serial_hd(i));
> sbd = SYS_BUS_DEVICE(&s->usart[i]);
> sysbus_realize(sbd, &error_abort);
> - sysbus_mmio_map(sbd, 0, OFFSET_DATA + mc->dev[USART(i)].addr);
> + sysbus_mmio_map(sbd, 0, offset_io + mc->dev[USART(i)].addr);
> connect_peripheral_irq(mc, sbd, 0, cpudev, USART_RXC_IRQ(i));
> connect_peripheral_irq(mc, sbd, 1, cpudev, USART_DRE_IRQ(i));
> connect_peripheral_irq(mc, sbd, 2, cpudev, USART_TXC_IRQ(i));
> @@ -321,12 +335,12 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> }
> if (!mc->dev[idx].is_timer16) {
> create_unimplemented_device("avr-timer8",
> - OFFSET_DATA + mc->dev[idx].addr, 5);
> + offset_io + mc->dev[idx].addr, 5);
> create_unimplemented_device("avr-timer8-intmask",
> - OFFSET_DATA
> + offset_io
> + mc->dev[idx].intmask_addr, 1);
> create_unimplemented_device("avr-timer8-intflag",
> - OFFSET_DATA
> + offset_io
> + mc->dev[idx].intflag_addr, 1);
> continue;
> }
> @@ -337,9 +351,9 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> s->xtal_freq_hz, &error_abort);
> sbd = SYS_BUS_DEVICE(&s->timer[i]);
> sysbus_realize(sbd, &error_abort);
> - sysbus_mmio_map(sbd, 0, OFFSET_DATA + mc->dev[idx].addr);
> - sysbus_mmio_map(sbd, 1, OFFSET_DATA + mc->dev[idx].intmask_addr);
> - sysbus_mmio_map(sbd, 2, OFFSET_DATA + mc->dev[idx].intflag_addr);
> + sysbus_mmio_map(sbd, 0, offset_io + mc->dev[idx].addr);
> + sysbus_mmio_map(sbd, 1, offset_io + mc->dev[idx].intmask_addr);
> + sysbus_mmio_map(sbd, 2, offset_io + mc->dev[idx].intflag_addr);
> connect_peripheral_irq(mc, sbd, 0, cpudev, TIMER_CAPT_IRQ(i));
> connect_peripheral_irq(mc, sbd, 1, cpudev, TIMER_COMPA_IRQ(i));
> connect_peripheral_irq(mc, sbd, 2, cpudev, TIMER_COMPB_IRQ(i));
> @@ -349,12 +363,12 @@ static void atmega_realize(DeviceState *dev, Error **errp)
> g_free(devname);
> }
>
> - create_unimplemented_device("avr-twi", OFFSET_DATA + 0x0b8, 6);
> - create_unimplemented_device("avr-adc", OFFSET_DATA + 0x078, 8);
> - create_unimplemented_device("avr-ext-mem-ctrl", OFFSET_DATA + 0x074, 2);
> - create_unimplemented_device("avr-watchdog", OFFSET_DATA + 0x060, 1);
> - create_unimplemented_device("avr-spi", OFFSET_DATA + 0x04c, 3);
> - create_unimplemented_device("avr-eeprom", OFFSET_DATA + 0x03f, 3);
> + create_unimplemented_device("avr-twi", offset_io + 0x0b8, 6);
> + create_unimplemented_device("avr-adc", offset_io + 0x078, 8);
> + create_unimplemented_device("avr-ext-mem-ctrl", offset_io + 0x074, 2);
> + create_unimplemented_device("avr-watchdog", offset_io + 0x060, 1);
> + create_unimplemented_device("avr-spi", offset_io + 0x04c, 3);
> + create_unimplemented_device("avr-eeprom", offset_io + 0x03f, 3);
> }
>
> static const Property atmega_props[] = {
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 15/17] hw/avr: Pass mcu_type to class_base_init via .class_data
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (13 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 14/17] hw/avr: Set offset_io and increase page size to 1k Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-23 21:38 ` Philippe Mathieu-Daudé
2025-03-25 1:25 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 16/17] hw/avr: Move AtmegaMcuClass to atmega.h Richard Henderson
2025-03-23 17:37 ` [PATCH 17/17] target/avr: Enable TARGET_PAGE_BITS_VARY Richard Henderson
16 siblings, 2 replies; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
We want to be able to do more common work on MachineClass.
Pass the class name as a string in .class_data.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/avr/arduino.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
index 48ef478346..29cb776848 100644
--- a/hw/avr/arduino.c
+++ b/hw/avr/arduino.c
@@ -69,6 +69,13 @@ static void arduino_machine_class_init(ObjectClass *oc, void *data)
mc->no_parallel = 1;
}
+static void arduino_machine_class_base_init(ObjectClass *oc, void *data)
+{
+ ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
+
+ amc->mcu_type = data;
+}
+
static void arduino_duemilanove_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -80,7 +87,6 @@ static void arduino_duemilanove_class_init(ObjectClass *oc, void *data)
*/
mc->desc = "Arduino Duemilanove (ATmega168)",
mc->alias = "2009";
- amc->mcu_type = TYPE_ATMEGA168_MCU;
amc->xtal_hz = 16 * 1000 * 1000;
};
@@ -95,7 +101,6 @@ static void arduino_uno_class_init(ObjectClass *oc, void *data)
*/
mc->desc = "Arduino UNO (ATmega328P)";
mc->alias = "uno";
- amc->mcu_type = TYPE_ATMEGA328_MCU;
amc->xtal_hz = 16 * 1000 * 1000;
};
@@ -110,7 +115,6 @@ static void arduino_mega_class_init(ObjectClass *oc, void *data)
*/
mc->desc = "Arduino Mega (ATmega1280)";
mc->alias = "mega";
- amc->mcu_type = TYPE_ATMEGA1280_MCU;
amc->xtal_hz = 16 * 1000 * 1000;
};
@@ -125,7 +129,6 @@ static void arduino_mega2560_class_init(ObjectClass *oc, void *data)
*/
mc->desc = "Arduino Mega 2560 (ATmega2560)";
mc->alias = "mega2560";
- amc->mcu_type = TYPE_ATMEGA2560_MCU;
amc->xtal_hz = 16 * 1000 * 1000; /* CSTCE16M0V53-R0 */
};
@@ -134,24 +137,29 @@ static const TypeInfo arduino_machine_types[] = {
.name = MACHINE_TYPE_NAME("arduino-duemilanove"),
.parent = TYPE_ARDUINO_MACHINE,
.class_init = arduino_duemilanove_class_init,
+ .class_data = (void *)TYPE_ATMEGA168_MCU,
}, {
.name = MACHINE_TYPE_NAME("arduino-uno"),
.parent = TYPE_ARDUINO_MACHINE,
.class_init = arduino_uno_class_init,
+ .class_data = (void *)TYPE_ATMEGA328_MCU,
}, {
.name = MACHINE_TYPE_NAME("arduino-mega"),
.parent = TYPE_ARDUINO_MACHINE,
.class_init = arduino_mega_class_init,
+ .class_data = (void *)TYPE_ATMEGA1280_MCU,
}, {
.name = MACHINE_TYPE_NAME("arduino-mega-2560-v3"),
.parent = TYPE_ARDUINO_MACHINE,
.class_init = arduino_mega2560_class_init,
+ .class_data = (void *)TYPE_ATMEGA2560_MCU,
}, {
.name = TYPE_ARDUINO_MACHINE,
.parent = TYPE_MACHINE,
.instance_size = sizeof(ArduinoMachineState),
.class_size = sizeof(ArduinoMachineClass),
.class_init = arduino_machine_class_init,
+ .class_base_init = arduino_machine_class_base_init,
.abstract = true,
}
};
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 15/17] hw/avr: Pass mcu_type to class_base_init via .class_data
2025-03-23 17:37 ` [PATCH 15/17] hw/avr: Pass mcu_type to class_base_init via .class_data Richard Henderson
@ 2025-03-23 21:38 ` Philippe Mathieu-Daudé
2025-03-25 1:25 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-23 21:38 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, pierrick.bouvier
On 23/3/25 18:37, Richard Henderson wrote:
> We want to be able to do more common work on MachineClass.
> Pass the class name as a string in .class_data.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/avr/arduino.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 15/17] hw/avr: Pass mcu_type to class_base_init via .class_data
2025-03-23 17:37 ` [PATCH 15/17] hw/avr: Pass mcu_type to class_base_init via .class_data Richard Henderson
2025-03-23 21:38 ` Philippe Mathieu-Daudé
@ 2025-03-25 1:25 ` Pierrick Bouvier
1 sibling, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:25 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> We want to be able to do more common work on MachineClass.
> Pass the class name as a string in .class_data.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/avr/arduino.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 16/17] hw/avr: Move AtmegaMcuClass to atmega.h
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (14 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 15/17] hw/avr: Pass mcu_type to class_base_init via .class_data Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:22 ` Pierrick Bouvier
2025-03-23 17:37 ` [PATCH 17/17] target/avr: Enable TARGET_PAGE_BITS_VARY Richard Henderson
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/avr/atmega.h | 20 ++++++++++++++++++++
hw/avr/atmega.c | 22 +---------------------
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e..f031e6c10a 100644
--- a/hw/avr/atmega.h
+++ b/hw/avr/atmega.h
@@ -23,6 +23,10 @@
#define TYPE_ATMEGA1280_MCU "ATmega1280"
#define TYPE_ATMEGA2560_MCU "ATmega2560"
+typedef struct AtmegaMcuClass AtmegaMcuClass;
+DECLARE_CLASS_CHECKERS(AtmegaMcuClass, ATMEGA_MCU,
+ TYPE_ATMEGA_MCU)
+
typedef struct AtmegaMcuState AtmegaMcuState;
DECLARE_INSTANCE_CHECKER(AtmegaMcuState, ATMEGA_MCU,
TYPE_ATMEGA_MCU)
@@ -32,6 +36,22 @@ DECLARE_INSTANCE_CHECKER(AtmegaMcuState, ATMEGA_MCU,
#define TIMER_MAX 6
#define GPIO_MAX 12
+struct AtmegaMcuClass {
+ /*< private >*/
+ SysBusDeviceClass parent_class;
+ /*< public >*/
+ const char *uc_name;
+ const char *cpu_type;
+ size_t flash_size;
+ size_t eeprom_size;
+ size_t sram_size;
+ size_t io_size;
+ size_t gpio_count;
+ size_t adc_count;
+ const uint8_t *irq;
+ const struct peripheral_cfg *dev;
+};
+
struct AtmegaMcuState {
/*< private >*/
SysBusDevice parent_obj;
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index d4fc9c4aee..96e36743bc 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -36,7 +36,7 @@ enum AtmegaPeripheral {
#define TIMER(n) (n + TIMER0)
#define POWER(n) (n + POWER0)
-typedef struct {
+typedef struct peripheral_cfg {
uint16_t addr;
enum AtmegaPeripheral power_index;
uint8_t power_bit;
@@ -46,26 +46,6 @@ typedef struct {
bool is_timer16;
} peripheral_cfg;
-struct AtmegaMcuClass {
- /*< private >*/
- SysBusDeviceClass parent_class;
- /*< public >*/
- const char *uc_name;
- const char *cpu_type;
- size_t flash_size;
- size_t eeprom_size;
- size_t sram_size;
- size_t io_size;
- size_t gpio_count;
- size_t adc_count;
- const uint8_t *irq;
- const peripheral_cfg *dev;
-};
-typedef struct AtmegaMcuClass AtmegaMcuClass;
-
-DECLARE_CLASS_CHECKERS(AtmegaMcuClass, ATMEGA_MCU,
- TYPE_ATMEGA_MCU)
-
static const peripheral_cfg dev168_328[PERIFMAX] = {
[USART0] = { 0xc0, POWER0, 1 },
[TIMER2] = { 0xb0, POWER0, 6, 0x70, 0x37, false },
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 16/17] hw/avr: Move AtmegaMcuClass to atmega.h
2025-03-23 17:37 ` [PATCH 16/17] hw/avr: Move AtmegaMcuClass to atmega.h Richard Henderson
@ 2025-03-25 1:22 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:22 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> hw/avr/atmega.h | 20 ++++++++++++++++++++
> hw/avr/atmega.c | 22 +---------------------
> 2 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
> index a99ee15c7e..f031e6c10a 100644
> --- a/hw/avr/atmega.h
> +++ b/hw/avr/atmega.h
> @@ -23,6 +23,10 @@
> #define TYPE_ATMEGA1280_MCU "ATmega1280"
> #define TYPE_ATMEGA2560_MCU "ATmega2560"
>
> +typedef struct AtmegaMcuClass AtmegaMcuClass;
> +DECLARE_CLASS_CHECKERS(AtmegaMcuClass, ATMEGA_MCU,
> + TYPE_ATMEGA_MCU)
> +
> typedef struct AtmegaMcuState AtmegaMcuState;
> DECLARE_INSTANCE_CHECKER(AtmegaMcuState, ATMEGA_MCU,
> TYPE_ATMEGA_MCU)
> @@ -32,6 +36,22 @@ DECLARE_INSTANCE_CHECKER(AtmegaMcuState, ATMEGA_MCU,
> #define TIMER_MAX 6
> #define GPIO_MAX 12
>
> +struct AtmegaMcuClass {
> + /*< private >*/
> + SysBusDeviceClass parent_class;
> + /*< public >*/
> + const char *uc_name;
> + const char *cpu_type;
> + size_t flash_size;
> + size_t eeprom_size;
> + size_t sram_size;
> + size_t io_size;
> + size_t gpio_count;
> + size_t adc_count;
> + const uint8_t *irq;
> + const struct peripheral_cfg *dev;
> +};
> +
> struct AtmegaMcuState {
> /*< private >*/
> SysBusDevice parent_obj;
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index d4fc9c4aee..96e36743bc 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -36,7 +36,7 @@ enum AtmegaPeripheral {
> #define TIMER(n) (n + TIMER0)
> #define POWER(n) (n + POWER0)
>
> -typedef struct {
> +typedef struct peripheral_cfg {
> uint16_t addr;
> enum AtmegaPeripheral power_index;
> uint8_t power_bit;
> @@ -46,26 +46,6 @@ typedef struct {
> bool is_timer16;
> } peripheral_cfg;
>
> -struct AtmegaMcuClass {
> - /*< private >*/
> - SysBusDeviceClass parent_class;
> - /*< public >*/
> - const char *uc_name;
> - const char *cpu_type;
> - size_t flash_size;
> - size_t eeprom_size;
> - size_t sram_size;
> - size_t io_size;
> - size_t gpio_count;
> - size_t adc_count;
> - const uint8_t *irq;
> - const peripheral_cfg *dev;
> -};
> -typedef struct AtmegaMcuClass AtmegaMcuClass;
> -
> -DECLARE_CLASS_CHECKERS(AtmegaMcuClass, ATMEGA_MCU,
> - TYPE_ATMEGA_MCU)
> -
> static const peripheral_cfg dev168_328[PERIFMAX] = {
> [USART0] = { 0xc0, POWER0, 1 },
> [TIMER2] = { 0xb0, POWER0, 6, 0x70, 0x37, false },
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 17/17] target/avr: Enable TARGET_PAGE_BITS_VARY
2025-03-23 17:37 [PATCH 00/17] target/avr: Increase page size Richard Henderson
` (15 preceding siblings ...)
2025-03-23 17:37 ` [PATCH 16/17] hw/avr: Move AtmegaMcuClass to atmega.h Richard Henderson
@ 2025-03-23 17:37 ` Richard Henderson
2025-03-25 1:24 ` Pierrick Bouvier
16 siblings, 1 reply; 45+ messages in thread
From: Richard Henderson @ 2025-03-23 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: mrolnik, philmd, pierrick.bouvier
Increase TARGET_PHYS_ADDR_SPACE_BITS to allow flexibility in the page
size without triggering an assert. Select the page size based on the
size of sram. This leaves sram on exactly one page and minimizes the
number of pages required to span the flash.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/cpu-param.h | 11 +++++++++--
hw/avr/arduino.c | 15 +++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/target/avr/cpu-param.h b/target/avr/cpu-param.h
index f5248ce9e7..a18bf39bb9 100644
--- a/target/avr/cpu-param.h
+++ b/target/avr/cpu-param.h
@@ -21,8 +21,15 @@
#ifndef AVR_CPU_PARAM_H
#define AVR_CPU_PARAM_H
-#define TARGET_PAGE_BITS 10
-#define TARGET_PHYS_ADDR_SPACE_BITS 24
+#define TARGET_PAGE_BITS_VARY
+#define TARGET_PAGE_BITS_MIN 10
+
+/*
+ * The real value for TARGET_PHYS_ADDR_SPACE_BITS is 24, but selecting
+ * an overly small value will assert in tb-maint.c when selecting the
+ * shape of the page_table tree. This allows an 8k page size.
+ */
+#define TARGET_PHYS_ADDR_SPACE_BITS 28
#define TARGET_VIRT_ADDR_SPACE_BITS 24
#define TCG_GUEST_DEFAULT_MO 0
diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
index 29cb776848..f309aa5597 100644
--- a/hw/avr/arduino.c
+++ b/hw/avr/arduino.c
@@ -71,9 +71,24 @@ static void arduino_machine_class_init(ObjectClass *oc, void *data)
static void arduino_machine_class_base_init(ObjectClass *oc, void *data)
{
+ MachineClass *mc = MACHINE_CLASS(oc);
ArduinoMachineClass *amc = ARDUINO_MACHINE_CLASS(oc);
+ AtmegaMcuClass *acc;
+ int page_bits;
amc->mcu_type = data;
+
+ /* Find the mcu class that we will instantiate. */
+ acc = ATMEGA_MCU_CLASS(object_class_by_name(amc->mcu_type));
+
+ /*
+ * Select a page size based on the size of sram.
+ * This will result in a page size between 1k and 8k
+ * and minimize the number of pages to span flash.
+ */
+ page_bits = ctz32(acc->sram_size);
+ assert(page_bits >= TARGET_PAGE_BITS_MIN && page_bits <= 13);
+ mc->minimum_page_bits = page_bits;
}
static void arduino_duemilanove_class_init(ObjectClass *oc, void *data)
--
2.43.0
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH 17/17] target/avr: Enable TARGET_PAGE_BITS_VARY
2025-03-23 17:37 ` [PATCH 17/17] target/avr: Enable TARGET_PAGE_BITS_VARY Richard Henderson
@ 2025-03-25 1:24 ` Pierrick Bouvier
0 siblings, 0 replies; 45+ messages in thread
From: Pierrick Bouvier @ 2025-03-25 1:24 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: mrolnik, philmd
On 3/23/25 10:37, Richard Henderson wrote:
> Increase TARGET_PHYS_ADDR_SPACE_BITS to allow flexibility in the page
> size without triggering an assert. Select the page size based on the
> size of sram. This leaves sram on exactly one page and minimizes the
> number of pages required to span the flash.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/avr/cpu-param.h | 11 +++++++++--
> hw/avr/arduino.c | 15 +++++++++++++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 45+ messages in thread