[v2] hw: misc: edu: fix 2 off-by-one errors
From: Christopher Friedt
In the case that size1 was zero, because of the explicit
'end1 > addr' check, the range check would fail and the error
message would read as shown below. The correct comparison
is 'end1 >= addr' (or 'addr <= end1').
EDU: DMA range 0x4-0x3 out of bounds (0x4-0x40fff)!
At the opposite end, in the case that size1 was 4096, within()
would fail because of the non-inclusive check 'end1 < end2',
which should have been 'end1 <= end2'. The error message would
previously say
EDU: DMA range 0x4-0x40fff out of bounds (0x4-0x40fff)!
This change
1. renames local variables to be more less ambiguous
2. fixes the two off-by-one errors described above.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1254
Signed-off-by: Christopher Friedt
---
hw/misc/edu.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index e935c418d4..52afbd792a 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -103,25 +103,24 @@ static void edu_lower_irq(EduState *edu, uint32_t val)
}
}
-static bool within(uint64_t addr, uint64_t start, uint64_t end)
+static void edu_check_range(uint64_t xfer_start, uint64_t xfer_size,
+uint64_t dma_start, uint64_t dma_size)
{
-return start <= addr && addr < end;
-}
-
-static void edu_check_range(uint64_t addr, uint64_t size1, uint64_t start,
-uint64_t size2)
-{
-uint64_t end1 = addr + size1;
-uint64_t end2 = start + size2;
-
-if (within(addr, start, end2) &&
-end1 > addr && within(end1, start, end2)) {
+uint64_t xfer_end = xfer_start + xfer_size;
+uint64_t dma_end = dma_start + dma_size;
+
+/*
+ * 1. ensure we aren't overflowing
+ * 2. ensure that xfer is within dma address range
+ */
+if (dma_end >= dma_start && xfer_end >= xfer_start &&
+xfer_start >= dma_start && xfer_end <= dma_end) {
return;
}
hw_error("EDU: DMA range 0x%016"PRIx64"-0x%016"PRIx64
" out of bounds (0x%016"PRIx64"-0x%016"PRIx64")!",
-addr, end1 - 1, start, end2 - 1);
+xfer_start, xfer_end - 1, dma_start, dma_end - 1);
}
static dma_addr_t edu_clamp_addr(const EduState *edu, dma_addr_t addr)
--
2.36.1
Re: [PATCH v3] tcg/loongarch64: Add direct jump support
On 10/14/22 11:40, Qi Hu wrote:
Similar to the ARM64, LoongArch has PC-relative instructions such as
PCADDU18I. These instructions can be used to support direct jump for
LoongArch. Additionally, if instruction "B offset" can cover the target
address(target is within ±128MB range), a single "B offset" plus a nop
will be used by "tb_target_set_jump_target".
Signed-off-by: Qi Hu
---
tcg/loongarch64/tcg-target.c.inc | 56 +---
tcg/loongarch64/tcg-target.h | 5 ++-
2 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index f5a214a17f..2a068a52cc 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -1031,6 +1031,35 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg
*args)
#endif
}
+/* LoongArch use `andi zero, zero, 0` as NOP. */
"uses"
+#define NOP OPC_ANDI
+static void tcg_out_nop(TCGContext *s)
+{
+tcg_out32(s, NOP);
+}
+
+void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx,
+ uintptr_t jmp_rw, uintptr_t addr)
+{
+tcg_insn_unit i1, i2;
+ptrdiff_t upper, lower;
+ptrdiff_t offset = (addr - jmp_rx) >> 2;
+
+if (offset == sextreg(offset, 0, 28)) {
You've already shifted by 2 above, so here you should check if `offset`
fits in 26 bits instead.
+i1 = encode_sd10k16_insn(OPC_B, offset);
+i2 = NOP;
+} else {
Could also add an assertion that `offset` fits in 36 bits, as we're now
bounded by `MAX_CODEGEN_BUFFER_SIZE` and don't really have the
capability to go larger than that any more, unlike when indirect jumps
were used.
+lower = (int16_t)offset;
+upper = (offset - lower) >> 16;
+
+i1 = encode_dsj20_insn(OPC_PCADDU18I, TCG_REG_T0, upper);
+i2 = encode_djsk16_insn(OPC_JIRL, TCG_REG_ZERO, TCG_REG_T0, lower);
Instead of hard-coding T0, what about simply using the TMP0 that's
explicitly reserved as a scratch register?
+}
+uint64_t pair = ((uint64_t)i2 << 32) | i1;
+qatomic_set((uint64_t *)jmp_rw, pair);
+flush_idcache_range(jmp_rx, jmp_rw, 8);
+}
+
/*
* Entry-points
*/
@@ -1058,11 +1087,28 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
break;
case INDEX_op_goto_tb:
-assert(s->tb_jmp_insn_offset == 0);
-/* indirect jump method */
-tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
- (uintptr_t)(s->tb_jmp_target_addr + a0));
-tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+if (s->tb_jmp_insn_offset != NULL) {
+/* TCG_TARGET_HAS_direct_jump */
+/*
+ * Ensure that "patch area" are 8-byte aligned so that an
"Ensure that the patch area is"
+ * atomic write can be used to patch the target address.
+ */
+if ((uintptr_t)s->code_ptr & 7) {
+tcg_out_nop(s);
+}
+s->tb_jmp_insn_offset[a0] = tcg_current_code_size(s);
+/*
+ * actual branch destination will be patched by
+ * tb_target_set_jmp_target later
+ */
+tcg_out_opc_pcaddu18i(s, TCG_REG_TMP0, 0);
+tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+} else {
+/* !TCG_TARGET_HAS_direct_jump */
+tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
+(uintptr_t)(s->tb_jmp_target_addr + a0));
+tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
Why not remove the else block altogether? Removing the respective unused
code for aarch64 could be considered a separate logical change and I
think we can do the loongarch64 part independently of whether it's
appropriate/accepted for aarch64.
+}
set_jmp_reset_offset(s, a0);
break;
diff --git a/tcg/loongarch64/tcg-target.h b/tcg/loongarch64/tcg-target.h
index 67380b2432..ee207ec32c 100644
--- a/tcg/loongarch64/tcg-target.h
+++ b/tcg/loongarch64/tcg-target.h
@@ -42,7 +42,7 @@
#define TCG_TARGET_INSN_UNIT_SIZE 4
#define TCG_TARGET_NB_REGS 32
-#define MAX_CODE_GEN_BUFFER_SIZE SIZE_MAX
+#define MAX_CODE_GEN_BUFFER_SIZE (128 * GiB)
If I were you I'd put a comment above saying the 128GiB limit is due to
the PCADDU18I + JIRL sequence giving a total of 20 + 16 + 2 = 38 bits
for signed offsets, which is +/- 128GiB. But I'm fine without it too.
typedef enum {
TCG_REG_ZERO,
@@ -123,7 +123,7 @@ typedef enum {
#define TCG_TARGET_HAS_clz_i32 1
#define TCG_TARGET_HAS_ctz_i32 1
#define TCG_TARGET_HAS_ctpop_i320
-#define TCG_TARGET_HAS_direct_jump 0
+#define TCG_TARGET_HAS_direct_jump 1
#define TCG_TARGET_HAS_brcond2 0
#define TCG_TARGET_HAS_setcond2 0
#define TCG_TARGET_HAS_qemu_st8_i32 0
@@ -166,7 +166,6 @@ typedef enum {
#define TCG_TARGET_HAS_muluh_i641
#define T
Re: [PATCH v3 6/8] openrisc: re-randomize rng-seed on reboot
Hi Jason,
On Thu, Oct 13, 2022 at 08:16:51PM -0600, Jason A. Donenfeld wrote:
> When the system reboots, the rng-seed that the FDT has should be
> re-randomized, so that the new boot gets a new seed. Since the FDT is in
> the ROM region at this point, we add a hook right after the ROM has been
> added, so that we have a pointer to that copy of the FDT.
This looks good to me.
Acked-by: Stafford Horne
> Cc: Stafford Horne
> Signed-off-by: Jason A. Donenfeld
> ---
> hw/openrisc/boot.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/openrisc/boot.c b/hw/openrisc/boot.c
> index 128ccbcba2..007e80cd5a 100644
> --- a/hw/openrisc/boot.c
> +++ b/hw/openrisc/boot.c
> @@ -14,6 +14,7 @@
> #include "hw/openrisc/boot.h"
> #include "sysemu/device_tree.h"
> #include "sysemu/qtest.h"
> +#include "sysemu/reset.h"
>
> #include
>
> @@ -111,6 +112,8 @@ uint32_t openrisc_load_fdt(void *fdt, hwaddr load_start,
>
> rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
>&address_space_memory);
> +qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
> +rom_ptr_for_as(&address_space_memory, fdt_addr,
> fdtsize));
>
> return fdt_addr;
> }
> --
> 2.37.3
>
Re: [PATCH v4] tcg/loongarch64: Add direct jump support
On 10/15/22 17:27, Qi Hu wrote: Similar to the ARM64, LoongArch has PC-relative instructions such as PCADDU18I. These instructions can be used to support direct jump for LoongArch. Additionally, if instruction "B offset" can cover the target address(target is within ±128MB range), a single "B offset" plus a nop will be used by "tb_target_set_jump_target". Cc: Richard Henderson Signed-off-by: Qi Hu --- Changes since v3: - Fix the offset check error which is pointed by WANG Xuerui. - Use TMP0 instead of T0. - Remove useless block due to direct jump support. - Add some assertions. --- tcg/loongarch64/tcg-target.c.inc | 48 +--- tcg/loongarch64/tcg-target.h | 9 -- 2 files changed, 50 insertions(+), 7 deletions(-) Richard may want to double-check for restoring his R-b, but this looks good to me now. Thanks! Reviewed-by: WANG Xuerui
Re: [PATCH v6 14/25] ppc440_sdram: Move RAM size check to ppc440_sdram_init
On 10/15/22 10:20, BALATON Zoltan wrote:
On Sat, 15 Oct 2022, Daniel Henrique Barboza wrote:
On 10/15/22 08:40, BALATON Zoltan wrote:
On Sat, 15 Oct 2022, Daniel Henrique Barboza wrote:
On 10/14/22 19:52, BALATON Zoltan wrote:
On Fri, 14 Oct 2022, Daniel Henrique Barboza wrote:
Zoltan,
Gitlab didn't like this patch. It broke all 32 bits builds due to an overflow
down there:
On 9/24/22 09:28, BALATON Zoltan wrote:
Move the check for valid memory sizes from board to sdram controller
init. This adds the missing valid memory sizes of 4 GiB, 16 and 8 MiB
to the DoC and the board now only checks for additional restrictions
imposed by its firmware then sdram init checks for valid sizes for SoC.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440.h | 4 ++--
hw/ppc/ppc440_uc.c | 15 +++
hw/ppc/sam460ex.c | 32 +---
3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
index 01d76b8000..29f6f14ed7 100644
--- a/hw/ppc/ppc440.h
+++ b/hw/ppc/ppc440.h
@@ -11,13 +11,13 @@
#ifndef PPC440_H
#define PPC440_H
-#include "hw/ppc/ppc4xx.h"
+#include "hw/ppc/ppc.h"
void ppc4xx_l2sram_init(CPUPPCState *env);
void ppc4xx_cpr_init(CPUPPCState *env);
void ppc4xx_sdr_init(CPUPPCState *env);
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks);
+ MemoryRegion *ram);
void ppc4xx_ahb_init(CPUPPCState *env);
void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
void ppc460ex_pcie_init(CPUPPCState *env);
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index edd0781eb7..2b9d666b71 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -487,7 +487,7 @@ void ppc4xx_sdr_init(CPUPPCState *env)
typedef struct ppc440_sdram_t {
uint32_t addr;
uint32_t mcopt2;
- int nbanks;
+ int nbanks; /* Banks to use from the 4, e.g. when board has less slots */
Ppc4xxSdramBank bank[4];
} ppc440_sdram_t;
@@ -733,18 +733,17 @@ static void sdram_ddr2_reset(void *opaque)
}
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks)
+ MemoryRegion *ram)
{
ppc440_sdram_t *s;
- int i;
+ const ram_addr_t valid_bank_sizes[] = {
+ 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * MiB,
^ here. ram_addr_t will be a 32 bit var in a 32 bit host, and assigning 4 * GiB
will
overflow it back to zero.
Here's the Gitlab error from the 'cross-win32-system' runner:
FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj
2725i686-w64-mingw32-gcc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/i686-w64-mingw32/sys-root/mingw/include/pixman-1 -I/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0 -I/usr/i686-w64-mingw32/sys-root/mingw/lib/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -mms-bitfields -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-pie -no-pie -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -c ../hw/ppc/ppc440_uc.c
2726../hw/ppc/ppc440_uc.c: In function 'ppc4xx_sdram_ddr2_realize':
2727../hw/ppc/ppc440_uc.c:729:9: error: unsigned conversion from 'long long
int' to 'unsigned int' changes value from '4294967296' to '0' [-Werror=overflow]
2728 729 | 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB,
64 * MiB,
2729 | ^
2730cc1: all warnings being treated as errors
2731
A quick fix that I can make in-tree is to avoid the overflow by doing (4 * GiB)
- 1.
But since this might affect some logic in the model I figured I should ask you
first.
I think in that case we can just drop the 4*GiB value from the
valid_bank_sizes[] array for now because while it's valid for the SoC the
sam460ex firmware also has problems with it so having 2 GiB as largest value is
OK.
Got it.
Can you change the patch accordingly or should I send an updated version with
this change?
I'll fix it in-tree, no need to re-send. I'll also amend the commit msg
accordingly.
Re: [PATCH v6 14/25] ppc440_sdram: Move RAM size check to ppc440_sdram_init
On Sat, 15 Oct 2022, Daniel Henrique Barboza wrote:
On 10/15/22 08:40, BALATON Zoltan wrote:
On Sat, 15 Oct 2022, Daniel Henrique Barboza wrote:
On 10/14/22 19:52, BALATON Zoltan wrote:
On Fri, 14 Oct 2022, Daniel Henrique Barboza wrote:
Zoltan,
Gitlab didn't like this patch. It broke all 32 bits builds due to an
overflow
down there:
On 9/24/22 09:28, BALATON Zoltan wrote:
Move the check for valid memory sizes from board to sdram controller
init. This adds the missing valid memory sizes of 4 GiB, 16 and 8 MiB
to the DoC and the board now only checks for additional restrictions
imposed by its firmware then sdram init checks for valid sizes for SoC.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440.h | 4 ++--
hw/ppc/ppc440_uc.c | 15 +++
hw/ppc/sam460ex.c | 32 +---
3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
index 01d76b8000..29f6f14ed7 100644
--- a/hw/ppc/ppc440.h
+++ b/hw/ppc/ppc440.h
@@ -11,13 +11,13 @@
#ifndef PPC440_H
#define PPC440_H
-#include "hw/ppc/ppc4xx.h"
+#include "hw/ppc/ppc.h"
void ppc4xx_l2sram_init(CPUPPCState *env);
void ppc4xx_cpr_init(CPUPPCState *env);
void ppc4xx_sdr_init(CPUPPCState *env);
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks);
+ MemoryRegion *ram);
void ppc4xx_ahb_init(CPUPPCState *env);
void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
void ppc460ex_pcie_init(CPUPPCState *env);
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index edd0781eb7..2b9d666b71 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -487,7 +487,7 @@ void ppc4xx_sdr_init(CPUPPCState *env)
typedef struct ppc440_sdram_t {
uint32_t addr;
uint32_t mcopt2;
- int nbanks;
+ int nbanks; /* Banks to use from the 4, e.g. when board has less
slots */
Ppc4xxSdramBank bank[4];
} ppc440_sdram_t;
@@ -733,18 +733,17 @@ static void sdram_ddr2_reset(void *opaque)
}
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks)
+ MemoryRegion *ram)
{
ppc440_sdram_t *s;
- int i;
+ const ram_addr_t valid_bank_sizes[] = {
+ 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64
* MiB,
^ here. ram_addr_t will be a 32 bit var in a 32 bit host, and assigning
4 * GiB will
overflow it back to zero.
Here's the Gitlab error from the 'cross-win32-system' runner:
FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj
2725i686-w64-mingw32-gcc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I..
-Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui
-Iui/shader -I/usr/i686-w64-mingw32/sys-root/mingw/include/pixman-1
-I/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0
-I/usr/i686-w64-mingw32/sys-root/mingw/lib/glib-2.0/include
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
-iquote . -iquote /builds/danielhb/qemu -iquote
/builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386
-mms-bitfields -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-pie -no-pie
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined
-Wimplicit-fallthrough=2 -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong
-DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"'
'-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ
libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -MF
libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj.d -o
libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -c
../hw/ppc/ppc440_uc.c
2726../hw/ppc/ppc440_uc.c: In function 'ppc4xx_sdram_ddr2_realize':
2727../hw/ppc/ppc440_uc.c:729:9: error: unsigned conversion from 'long
long int' to 'unsigned int' changes value from '4294967296' to '0'
[-Werror=overflow]
2728 729 | 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128
* MiB, 64 * MiB,
2729 | ^
2730cc1: all warnings being treated as errors
2731
A quick fix that I can make in-tree is to avoid the overflow by doing (4
* GiB) - 1.
But since this might affect some logic in the model I figured I should
ask you
first.
I think in that case we can just drop the 4*GiB value from the
valid_bank_sizes[] array for now because while it's valid for the SoC the
sam460ex firmware also has problems with it so having 2 GiB as largest
value is OK.
Got it.
Can you change the patch accordingly or should I send an updated version
with this change?
I'll fix it in-tree, no need to re-send. I'll also amend the commit msg
accordingly.
Thank you for t
Re: [RFC v2 00/10] Introduce an extensible static analyzer
On Freitag, 14. Oktober 2022 00:00:58 CEST Paolo Bonzini wrote:
[...]
> However, it seems like a lost battle. :( Some of the optimizations are
> stuff that you should just not have to do, for example only invoking
> "x.kind" once (because it's a property not a field). Another issue is
> that the bindings are incomplete, for example if you have a ForStmt
> you just get a Cursor and you are not able to access individual
> expressions. As a result, this for example is wrong in the
> return-value-never-used test:
>
> static int f(void) { return 42; }
> static void g(void) {
> for (f(); ; ) { } /* should warn, it doesn't */
> }
>
> and I couldn't fix it without breaking "for (; f(); )" because AFAICT
> the two are indistinguishable.
You mean accessing the `for` loop's init expression, condition expression,
increment statement? AFAICS those are accessible already by calling
get_children() on the `for` statement cursor:
https://github.com/llvm/llvm-project/blob/main/clang/bindings/python/clang/cindex.py#L1833
I just made a quick test with a short .c file and their demo script:
https://github.com/llvm/llvm-project/blob/main/clang/bindings/python/examples/cindex/cindex-dump.py
And I got all those components of the `for` loop as children of the `for`
cursor.
Best regards,
Christian Schoenebeck
Re: [PATCH v3 7/9] target/arm: Add PMSAv8r registers
Thank you very much for the review!
I have a few questions:
On 27.09.22 15:50, Peter Maydell wrote:
On Sat, 20 Aug 2022 at 15:19, wrote:
From: Tobias Röhmel
Signed-off-by: Tobias Röhmel
---
target/arm/cpu.h| 10 +++
target/arm/helper.c | 171
2 files changed, 181 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 86e06116a9..632d0d13c6 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -726,8 +726,18 @@ typedef struct CPUArchState {
*/
uint32_t *rbar[M_REG_NUM_BANKS];
uint32_t *rlar[M_REG_NUM_BANKS];
+uint32_t prbarn[255];
+uint32_t prlarn[255];
+uint32_t hprbarn[255];
+uint32_t hprlarn[255];
Don't use magic constants, please. In fact, don't use
fixed size arrays at all here. The v8R PRBAR/PRLAR
register arrays are exactly the same format as the v8M
pmsav8.rbar[] and pmsav8.rlar[], so please use the same
state fields that does. (You'll need to add equivalent
new arrays to handle the HPRBAR/HPRLAR.)
Is there a way to find out whether I'm in secure mode while accessing a
co-processor register?
The banks for rbar etc. are used to model the secure non-secure banks.
The access mechanism
here is via a device which allows the use of the mmu index to find out
if we are in secure mode.
I'm struggling to find a good solution in the co-processor register case.
uint32_t mair0[M_REG_NUM_BANKS];
uint32_t mair1[M_REG_NUM_BANKS];
+uint32_t prbar;
+uint32_t prlar;
You should not need separate prbar/prlar fields, as those
registers only indirectly access the state for thecurrently
selected region. Similarly hprbar, hprlar below.
+uint32_t prselr;
PRSELR is just a renamed PMSAv7 RGNR, for which we already
have a state field, pmsav7.rnr[M_REG_NS] (and indeed a cpreg
struct).
I changed it to use the rnr field, but I think I can't reuse the cpreg
since it has a different encoding.
+uint32_t hprbar;
+uint32_t hprlar;
+uint32_t hprselr;
} pmsav8;
Some of this new state must be handled for migration.
Where state is directly accessed via a coprocessor
register that will be migrated for you. However, where
there is state that is not directly accessible, i.e. for
the rbar/rlar arrays, you need to add code/vmstate structs
to target/arm/machine.c to migrate them. vmstate_pmsav8
already does this for rbar and rlar, but you'll need to
add something similar for the hyp versions. (Watch out that
you maintain migration compat for the existing CPUs -- you
can't just add new fields to existing VMStateDescription
structs. Ask if you need advice.)
I need some help here. Do I need new subsections?
The arrays will also need explicit handling in reset.
Again, look at how PMSAv7 is doing it.
/* v8M SAU */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 23461397e0..1730383f28 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7422,6 +7422,78 @@ static CPAccessResult access_joscr_jmcr(CPUARMState *env,
return CP_ACCESS_OK;
}
+static void prbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+env->pmsav8.prbarn[env->pmsav8.prselr] = value;
+}
+
+static void prlar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+env->pmsav8.prlarn[env->pmsav8.prselr] = value;
+}
For writes you will need to do some kind of tlb flush.
Compare pmsav7_write().
+
+static uint64_t prbar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env->pmsav8.prbarn[env->pmsav8.prselr];
+}
+
+static uint64_t prlar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+return env->pmsav8.prlarn[env->pmsav8.prselr];
+}
+
+static void hprbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+env->pmsav8.hprbarn[env->pmsav8.hprselr] = value;
+}
+
+static void hprlar_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+env->pmsav8.hprlarn[env->pmsav8.hprselr] = value;
+}
+
+static void hprenr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+uint32_t n;
+ARMCPU *cpu = env_archcpu(env);
+for (n = 0; n < (int)cpu->pmsav7_dregion; ++n) {
What's the cast for here ?
The architecture allows EL1 and EL2 MPUs to have different
numbers of regions, so this loop bound shouldn't be on
pmsav7_dregion, which is (I assume) the number of
EL1 MPU regions.
You need to also bound n to less than 32, to avoid
undefined behaviour.
+if (value & (1 << n)) {
+env->pmsav8.hprlarn[n] |= 0x1;
+} else {
+env->pmsav8.hprlarn[n] &= (~0x1);
Brackets unnecessary.
+}
Consider replacing this if() with
bit = extract32(value, n, 1);
env->pmsav8.hprlarn[n] = deposit32(env->pmsav8.hprlarn[n], 0, 1, bit);
+}
Re: [PATCH v8 00/16] QMP/HMP: introduce 'dumpdtb'
Patches 1, 6 and 8-15 applied to ppc-next. Thanks, Daniel On 9/26/22 14:38, Daniel Henrique Barboza wrote: Hi, This new version contains all changes proposed during the review process, all of them done in the patch that introduces dumpdtb. Other changes made: - Patch 14/14, the one that introduces the command, is now patch 1. This change is to make the other machine patches referencing 'dumpdtb QMP/HMP' to reference an existing command. - added two new patches based on Philippe's feedback: patch 2 and patch 4. Mandatory patch pending review: patch 2 Optional machine patches pending review: 3, 4, 5, 7, 16 Changes from v7: - patch 14: switched to start of the series, now patch 1 - patch 1: - changed hmp-commands.hx help to: "dump the FDT in dtb format to 'filename'" - changed 'filename' to *filename* - changed filename description in machine.json to "name of the binary FDT file to be created" - changed 'size' to uint32_t - added a g_assert() for FDT size == zero - added a success message in hmp_dumpdtb() - patch 2 (new): - free ms->fdt in machine_finalize() - patch 4 (new): - assign ms->fdt in boston_mach_init() - v7 link: https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01350.html Daniel Henrique Barboza (16): qmp/hmp, device_tree.c: introduce dumpdtb hw/core: free ms->fdt in machine_finalize() hw/arm: do not free machine->fdt in arm_load_dtb() hw/mips: set machine->fdt in boston_mach_init() hw/microblaze: set machine->fdt in microblaze_load_dtb() hw/nios2: set machine->fdt in nios2_load_dtb() hw/ppc: set machine->fdt in ppce500_load_device_tree() hw/ppc: set machine->fdt in bamboo_load_device_tree() hw/ppc: set machine->fdt in sam460ex_load_device_tree() hw/ppc: set machine->fdt in xilinx_load_device_tree() hw/ppc: set machine->fdt in pegasos2_machine_reset() hw/ppc: set machine->fdt in pnv_reset() hw/ppc: set machine->fdt in spapr machine hw/riscv: set machine->fdt in sifive_u_machine_init() hw/riscv: set machine->fdt in spike_board_init() hw/xtensa: set machine->fdt in xtfpga_init() hmp-commands.hx | 15 +++ hw/arm/boot.c| 3 ++- hw/core/machine.c| 1 + hw/microblaze/boot.c | 8 +++- hw/microblaze/meson.build| 2 +- hw/mips/boston.c | 5 - hw/nios2/boot.c | 8 +++- hw/nios2/meson.build | 2 +- hw/ppc/e500.c| 13 - hw/ppc/pegasos2.c| 4 hw/ppc/pnv.c | 8 +++- hw/ppc/ppc440_bamboo.c | 25 +--- hw/ppc/sam460ex.c| 21 ++-- hw/ppc/spapr.c | 3 +++ hw/ppc/spapr_hcall.c | 8 hw/ppc/virtex_ml507.c| 25 +--- hw/riscv/sifive_u.c | 3 +++ hw/riscv/spike.c | 6 ++ hw/xtensa/meson.build| 2 +- hw/xtensa/xtfpga.c | 6 +- include/sysemu/device_tree.h | 1 + monitor/misc.c | 1 + qapi/machine.json| 18 ++ softmmu/device_tree.c| 37 24 files changed, 183 insertions(+), 42 deletions(-)
Re: [PATCH v6 14/25] ppc440_sdram: Move RAM size check to ppc440_sdram_init
On 10/15/22 08:40, BALATON Zoltan wrote:
On Sat, 15 Oct 2022, Daniel Henrique Barboza wrote:
On 10/14/22 19:52, BALATON Zoltan wrote:
On Fri, 14 Oct 2022, Daniel Henrique Barboza wrote:
Zoltan,
Gitlab didn't like this patch. It broke all 32 bits builds due to an overflow
down there:
On 9/24/22 09:28, BALATON Zoltan wrote:
Move the check for valid memory sizes from board to sdram controller
init. This adds the missing valid memory sizes of 4 GiB, 16 and 8 MiB
to the DoC and the board now only checks for additional restrictions
imposed by its firmware then sdram init checks for valid sizes for SoC.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440.h | 4 ++--
hw/ppc/ppc440_uc.c | 15 +++
hw/ppc/sam460ex.c | 32 +---
3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
index 01d76b8000..29f6f14ed7 100644
--- a/hw/ppc/ppc440.h
+++ b/hw/ppc/ppc440.h
@@ -11,13 +11,13 @@
#ifndef PPC440_H
#define PPC440_H
-#include "hw/ppc/ppc4xx.h"
+#include "hw/ppc/ppc.h"
void ppc4xx_l2sram_init(CPUPPCState *env);
void ppc4xx_cpr_init(CPUPPCState *env);
void ppc4xx_sdr_init(CPUPPCState *env);
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks);
+ MemoryRegion *ram);
void ppc4xx_ahb_init(CPUPPCState *env);
void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
void ppc460ex_pcie_init(CPUPPCState *env);
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index edd0781eb7..2b9d666b71 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -487,7 +487,7 @@ void ppc4xx_sdr_init(CPUPPCState *env)
typedef struct ppc440_sdram_t {
uint32_t addr;
uint32_t mcopt2;
- int nbanks;
+ int nbanks; /* Banks to use from the 4, e.g. when board has less slots */
Ppc4xxSdramBank bank[4];
} ppc440_sdram_t;
@@ -733,18 +733,17 @@ static void sdram_ddr2_reset(void *opaque)
}
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks)
+ MemoryRegion *ram)
{
ppc440_sdram_t *s;
- int i;
+ const ram_addr_t valid_bank_sizes[] = {
+ 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * MiB,
^ here. ram_addr_t will be a 32 bit var in a 32 bit host, and assigning 4 * GiB
will
overflow it back to zero.
Here's the Gitlab error from the 'cross-win32-system' runner:
FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj
2725i686-w64-mingw32-gcc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/i686-w64-mingw32/sys-root/mingw/include/pixman-1 -I/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0 -I/usr/i686-w64-mingw32/sys-root/mingw/lib/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -mms-bitfields -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-pie -no-pie -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -c ../hw/ppc/ppc440_uc.c
2726../hw/ppc/ppc440_uc.c: In function 'ppc4xx_sdram_ddr2_realize':
2727../hw/ppc/ppc440_uc.c:729:9: error: unsigned conversion from 'long long
int' to 'unsigned int' changes value from '4294967296' to '0' [-Werror=overflow]
2728 729 | 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB,
64 * MiB,
2729 | ^
2730cc1: all warnings being treated as errors
2731
A quick fix that I can make in-tree is to avoid the overflow by doing (4 * GiB)
- 1.
But since this might affect some logic in the model I figured I should ask you
first.
I think in that case we can just drop the 4*GiB value from the
valid_bank_sizes[] array for now because while it's valid for the SoC the
sam460ex firmware also has problems with it so having 2 GiB as largest value is
OK.
Got it.
Can you change the patch accordingly or should I send an updated version with
this change?
I'll fix it in-tree, no need to re-send. I'll also amend the commit msg
accordingly.
Thank you for taking care of it.
Do you want a TODO marker in that line mentioning that we're
Re: [PATCH v6 14/25] ppc440_sdram: Move RAM size check to ppc440_sdram_init
On Sat, 15 Oct 2022, Daniel Henrique Barboza wrote:
On 10/14/22 19:52, BALATON Zoltan wrote:
On Fri, 14 Oct 2022, Daniel Henrique Barboza wrote:
Zoltan,
Gitlab didn't like this patch. It broke all 32 bits builds due to an
overflow
down there:
On 9/24/22 09:28, BALATON Zoltan wrote:
Move the check for valid memory sizes from board to sdram controller
init. This adds the missing valid memory sizes of 4 GiB, 16 and 8 MiB
to the DoC and the board now only checks for additional restrictions
imposed by its firmware then sdram init checks for valid sizes for SoC.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440.h | 4 ++--
hw/ppc/ppc440_uc.c | 15 +++
hw/ppc/sam460ex.c | 32 +---
3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
index 01d76b8000..29f6f14ed7 100644
--- a/hw/ppc/ppc440.h
+++ b/hw/ppc/ppc440.h
@@ -11,13 +11,13 @@
#ifndef PPC440_H
#define PPC440_H
-#include "hw/ppc/ppc4xx.h"
+#include "hw/ppc/ppc.h"
void ppc4xx_l2sram_init(CPUPPCState *env);
void ppc4xx_cpr_init(CPUPPCState *env);
void ppc4xx_sdr_init(CPUPPCState *env);
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks);
+ MemoryRegion *ram);
void ppc4xx_ahb_init(CPUPPCState *env);
void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
void ppc460ex_pcie_init(CPUPPCState *env);
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index edd0781eb7..2b9d666b71 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -487,7 +487,7 @@ void ppc4xx_sdr_init(CPUPPCState *env)
typedef struct ppc440_sdram_t {
uint32_t addr;
uint32_t mcopt2;
- int nbanks;
+ int nbanks; /* Banks to use from the 4, e.g. when board has less
slots */
Ppc4xxSdramBank bank[4];
} ppc440_sdram_t;
@@ -733,18 +733,17 @@ static void sdram_ddr2_reset(void *opaque)
}
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks)
+ MemoryRegion *ram)
{
ppc440_sdram_t *s;
- int i;
+ const ram_addr_t valid_bank_sizes[] = {
+ 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 *
MiB,
^ here. ram_addr_t will be a 32 bit var in a 32 bit host, and assigning 4
* GiB will
overflow it back to zero.
Here's the Gitlab error from the 'cross-win32-system' runner:
FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj
2725i686-w64-mingw32-gcc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I..
-Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui
-Iui/shader -I/usr/i686-w64-mingw32/sys-root/mingw/include/pixman-1
-I/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0
-I/usr/i686-w64-mingw32/sys-root/mingw/lib/glib-2.0/include
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
-iquote . -iquote /builds/danielhb/qemu -iquote
/builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386
-mms-bitfields -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-pie -no-pie
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined
-Wimplicit-fallthrough=2 -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -DNEED_CPU_H
'-DCONFIG_TARGET="ppc64-softmmu-config-target.h"'
'-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ
libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -MF
libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj.d -o
libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -c ../hw/ppc/ppc440_uc.c
2726../hw/ppc/ppc440_uc.c: In function 'ppc4xx_sdram_ddr2_realize':
2727../hw/ppc/ppc440_uc.c:729:9: error: unsigned conversion from 'long
long int' to 'unsigned int' changes value from '4294967296' to '0'
[-Werror=overflow]
2728 729 | 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 *
MiB, 64 * MiB,
2729 | ^
2730cc1: all warnings being treated as errors
2731
A quick fix that I can make in-tree is to avoid the overflow by doing (4 *
GiB) - 1.
But since this might affect some logic in the model I figured I should ask
you
first.
I think in that case we can just drop the 4*GiB value from the
valid_bank_sizes[] array for now because while it's valid for the SoC the
sam460ex firmware also has problems with it so having 2 GiB as largest
value is OK.
Got it.
Can you change the patch accordingly or should I send an updated version
with this change?
I'll fix it in-tree, no need to re-send. I'll also amend the commit msg
accordingly.
Thank you for taking care of it.
Do you want a TODO marker in that line mentioning that we're pending
support
Re: [PATCH v6 14/25] ppc440_sdram: Move RAM size check to ppc440_sdram_init
On 10/14/22 19:52, BALATON Zoltan wrote:
On Fri, 14 Oct 2022, Daniel Henrique Barboza wrote:
Zoltan,
Gitlab didn't like this patch. It broke all 32 bits builds due to an overflow
down there:
On 9/24/22 09:28, BALATON Zoltan wrote:
Move the check for valid memory sizes from board to sdram controller
init. This adds the missing valid memory sizes of 4 GiB, 16 and 8 MiB
to the DoC and the board now only checks for additional restrictions
imposed by its firmware then sdram init checks for valid sizes for SoC.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440.h | 4 ++--
hw/ppc/ppc440_uc.c | 15 +++
hw/ppc/sam460ex.c | 32 +---
3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
index 01d76b8000..29f6f14ed7 100644
--- a/hw/ppc/ppc440.h
+++ b/hw/ppc/ppc440.h
@@ -11,13 +11,13 @@
#ifndef PPC440_H
#define PPC440_H
-#include "hw/ppc/ppc4xx.h"
+#include "hw/ppc/ppc.h"
void ppc4xx_l2sram_init(CPUPPCState *env);
void ppc4xx_cpr_init(CPUPPCState *env);
void ppc4xx_sdr_init(CPUPPCState *env);
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks);
+ MemoryRegion *ram);
void ppc4xx_ahb_init(CPUPPCState *env);
void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
void ppc460ex_pcie_init(CPUPPCState *env);
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index edd0781eb7..2b9d666b71 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -487,7 +487,7 @@ void ppc4xx_sdr_init(CPUPPCState *env)
typedef struct ppc440_sdram_t {
uint32_t addr;
uint32_t mcopt2;
- int nbanks;
+ int nbanks; /* Banks to use from the 4, e.g. when board has less slots */
Ppc4xxSdramBank bank[4];
} ppc440_sdram_t;
@@ -733,18 +733,17 @@ static void sdram_ddr2_reset(void *opaque)
}
void ppc440_sdram_init(CPUPPCState *env, int nbanks,
- Ppc4xxSdramBank *ram_banks)
+ MemoryRegion *ram)
{
ppc440_sdram_t *s;
- int i;
+ const ram_addr_t valid_bank_sizes[] = {
+ 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * MiB,
^ here. ram_addr_t will be a 32 bit var in a 32 bit host, and assigning 4 * GiB
will
overflow it back to zero.
Here's the Gitlab error from the 'cross-win32-system' runner:
FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj
2725i686-w64-mingw32-gcc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/i686-w64-mingw32/sys-root/mingw/include/pixman-1 -I/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0 -I/usr/i686-w64-mingw32/sys-root/mingw/lib/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -mms-bitfields -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-pie -no-pie -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_ppc440_uc.c.obj -c ../hw/ppc/ppc440_uc.c
2726../hw/ppc/ppc440_uc.c: In function 'ppc4xx_sdram_ddr2_realize':
2727../hw/ppc/ppc440_uc.c:729:9: error: unsigned conversion from 'long long
int' to 'unsigned int' changes value from '4294967296' to '0' [-Werror=overflow]
2728 729 | 4 * GiB, 2 * GiB, 1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB,
64 * MiB,
2729 | ^
2730cc1: all warnings being treated as errors
2731
A quick fix that I can make in-tree is to avoid the overflow by doing (4 * GiB)
- 1.
But since this might affect some logic in the model I figured I should ask you
first.
I think in that case we can just drop the 4*GiB value from the
valid_bank_sizes[] array for now because while it's valid for the SoC the
sam460ex firmware also has problems with it so having 2 GiB as largest value is
OK.
Got it.
Can you change the patch accordingly or should I send an updated version with
this change?
I'll fix it in-tree, no need to re-send. I'll also amend the commit msg
accordingly.
Do you want a TODO marker in that line mentioning that we're pending
support for the 4GiB value?
And thanks for the quick reply!
Daniel
Regards,
BALATON Zoltan
Let me know if this is
[PATCH v4] tcg/loongarch64: Add direct jump support
Similar to the ARM64, LoongArch has PC-relative instructions such as
PCADDU18I. These instructions can be used to support direct jump for
LoongArch. Additionally, if instruction "B offset" can cover the target
address(target is within ±128MB range), a single "B offset" plus a nop
will be used by "tb_target_set_jump_target".
Cc: Richard Henderson
Signed-off-by: Qi Hu
---
Changes since v3:
- Fix the offset check error which is pointed by WANG Xuerui.
- Use TMP0 instead of T0.
- Remove useless block due to direct jump support.
- Add some assertions.
---
tcg/loongarch64/tcg-target.c.inc | 48 +---
tcg/loongarch64/tcg-target.h | 9 --
2 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index f5a214a17f..8facd78137 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -1031,6 +1031,36 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg
*args)
#endif
}
+/* LoongArch uses `andi zero, zero, 0` as NOP. */
+#define NOP OPC_ANDI
+static void tcg_out_nop(TCGContext *s)
+{
+tcg_out32(s, NOP);
+}
+
+void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx,
+ uintptr_t jmp_rw, uintptr_t addr)
+{
+tcg_insn_unit i1, i2;
+ptrdiff_t upper, lower;
+ptrdiff_t offset = (ptrdiff_t)(addr - jmp_rx) >> 2;
+
+if (offset == sextreg(offset, 0, 26)) {
+i1 = encode_sd10k16_insn(OPC_B, offset);
+i2 = NOP;
+} else {
+tcg_debug_assert(offset == sextreg(offset, 0, 36));
+lower = (int16_t)offset;
+upper = (offset - lower) >> 16;
+
+i1 = encode_dsj20_insn(OPC_PCADDU18I, TCG_REG_TMP0, upper);
+i2 = encode_djsk16_insn(OPC_JIRL, TCG_REG_ZERO, TCG_REG_TMP0, lower);
+}
+uint64_t pair = ((uint64_t)i2 << 32) | i1;
+qatomic_set((uint64_t *)jmp_rw, pair);
+flush_idcache_range(jmp_rx, jmp_rw, 8);
+}
+
/*
* Entry-points
*/
@@ -1058,10 +1088,20 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
break;
case INDEX_op_goto_tb:
-assert(s->tb_jmp_insn_offset == 0);
-/* indirect jump method */
-tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
- (uintptr_t)(s->tb_jmp_target_addr + a0));
+tcg_debug_assert(s->tb_jmp_insn_offset != NULL);
+/*
+ * Ensure that patch area is 8-byte aligned so that an
+ * atomic write can be used to patch the target address.
+ */
+if ((uintptr_t)s->code_ptr & 7) {
+tcg_out_nop(s);
+}
+s->tb_jmp_insn_offset[a0] = tcg_current_code_size(s);
+/*
+ * actual branch destination will be patched by
+ * tb_target_set_jmp_target later
+ */
+tcg_out_opc_pcaddu18i(s, TCG_REG_TMP0, 0);
tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
set_jmp_reset_offset(s, a0);
break;
diff --git a/tcg/loongarch64/tcg-target.h b/tcg/loongarch64/tcg-target.h
index 67380b2432..ba05ba552e 100644
--- a/tcg/loongarch64/tcg-target.h
+++ b/tcg/loongarch64/tcg-target.h
@@ -42,7 +42,11 @@
#define TCG_TARGET_INSN_UNIT_SIZE 4
#define TCG_TARGET_NB_REGS 32
-#define MAX_CODE_GEN_BUFFER_SIZE SIZE_MAX
+/*
+ * PCADDU18I + JIRL sequence can give 20 + 16 + 2 = 38 bits
+ * signed offset, which is +/- 128 GiB.
+ */
+#define MAX_CODE_GEN_BUFFER_SIZE (128 * GiB)
typedef enum {
TCG_REG_ZERO,
@@ -123,7 +127,7 @@ typedef enum {
#define TCG_TARGET_HAS_clz_i32 1
#define TCG_TARGET_HAS_ctz_i32 1
#define TCG_TARGET_HAS_ctpop_i320
-#define TCG_TARGET_HAS_direct_jump 0
+#define TCG_TARGET_HAS_direct_jump 1
#define TCG_TARGET_HAS_brcond2 0
#define TCG_TARGET_HAS_setcond2 0
#define TCG_TARGET_HAS_qemu_st8_i32 0
@@ -166,7 +170,6 @@ typedef enum {
#define TCG_TARGET_HAS_muluh_i641
#define TCG_TARGET_HAS_mulsh_i641
-/* not defined -- call should be eliminated at compile time */
void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t, uintptr_t);
#define TCG_TARGET_DEFAULT_MO (0)
--
2.38.0
