RE: [PATCH] Hexagon (target/hexagon) implement mutability mask for GPRs
> -Original Message- > From: Richard Henderson ... > It might be clearer, and easier to initialize, if you invert the sense of the > mask: Ok -- thanks for the suggestions! I'll give 'em all a try. -Brian
Re: [PATCH] RISC-V: Add support for Ztso
On 9/2/22 04:44, Palmer Dabbelt wrote:
-#define TCG_GUEST_DEFAULT_MO 0
+/*
+ * RISC-V has two memory models: TSO is a bit weaker than Intel (MMIO and
+ * fetch), and WMO is approximately equivilant to Arm MCA. Rather than
+ * enforcing orderings on most accesses, just default to the target memory
+ * order.
+ */
+#ifdef TCG_TARGET_SUPPORTS_MCTCG_RVTSO
+# define TCG_GUEST_DEFAULT_MO (TCG_MO_ALL & ~TCG_MO_ST_LD)
+#else
+# define TCG_GUEST_DEFAULT_MO (0)
+#endif
TCG_GUEST_DEFAULT_MO should be allowed to be variable. Since I've not tried that, it may
not work, but making sure that it does would be the first thing to do.
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -236,6 +236,7 @@ static inline void tb_target_set_jmp_target(uintptr_t
tc_ptr, uintptr_t jmp_rx,
#include "tcg/tcg-mo.h"
#define TCG_TARGET_DEFAULT_MO (TCG_MO_ALL & ~TCG_MO_ST_LD)
+#define TCG_TARGET_SUPPORTS_MCTCG_RVTSO 1
Um, no. There's no need for this hackery...
+#ifdef TCG_TARGET_SUPPORTS_MCTCG_RVTSO
+/*
+ * We only support Ztso on targets that themselves are already TSO, which
+ * means there's no way to provide just RVWMO on those targets. Instead
+ * just default to telling the guest that Ztso is enabled.:
+ */
+DEFINE_PROP_BOOL("ztso", RISCVCPU, cfg.ext_ztso, true),
+#endif
... you can just as well define the property at runtime, with a runtime check on
TCG_TARGET_DEFAULT_MO.
Though, honestly, I've had patches to add the required barriers sitting around for the
last few releases, to better support things like x86 on aarch64. I should just finish
that up.
r~
Re: [PATCH] Hexagon (target/hexagon) implement mutability mask for GPRs
On 9/1/22 22:29, Brian Cain wrote:
+void gen_masked_reg_write(TCGv cur_val, TCGv in_val, TCGv out_val,
+target_ulong reg_mask) {
+TCGv set_bits = tcg_temp_new();
+TCGv cleared_bits = tcg_temp_new();
+
+/*
+ * set_bits = in_val & reg_mask
+ * cleared_bits = (~in_val) & reg_mask
+ */
+tcg_gen_andi_tl(set_bits, in_val, reg_mask);
+tcg_gen_not_tl(cleared_bits, in_val);
+tcg_gen_andi_tl(cleared_bits, cleared_bits, reg_mask);
+
+/*
+ * result = (reg_cur | set_bits) & (~cleared_bits)
+ */
+tcg_gen_not_tl(cleared_bits, cleared_bits);
+tcg_gen_or_tl(set_bits, set_bits, cur_val);
+tcg_gen_and_tl(out_val, set_bits, cleared_bits);
This is overly complicated. It should be
out = (in & mask) | (cur & ~mask)
which is only 3 operations instead of 6:
tcg_gen_andi_tl(t1, in_val, reg_mask);
tcg_gen_andi_tl(t2, cur_val, ~reg_mask);
tcg_gen_or_tl(out_val, t1, t2);
I'm surprised about new files for this simple operation. Surely a subroutine within
genptr.c would be sufficient.
+static const hexagon_mut_entry gpr_mut_masks[HEX_REG_LAST_VALUE] = {
+[HEX_REG_PC] = {true, 0x},
+[HEX_REG_GP] = {true, 0xffc0},
+[HEX_REG_USR] = {true, 0x3ecfff3f},
+[HEX_REG_UTIMERLO] = {true, 0x},
+[HEX_REG_UTIMERHI] = {true, 0x},
+};
...
static inline void gen_log_reg_write(int rnum, TCGv val)
{
-tcg_gen_mov_tl(hex_new_value[rnum], val);
+const hexagon_mut_entry entry = gpr_mut_masks[rnum];
+if (entry.present) {
+gen_masked_reg_write(hex_gpr[rnum], val, hex_new_value[rnum],
+entry.mask);
+} else {
+tcg_gen_mov_tl(hex_new_value[rnum], val);
+}
You could avoid the structure and .present flag by initializing all other entries to
UINT32_MAX. E.g.
static const target_ulong gpr_mut_masks[HEX_REG_LAST_VALUE] = {
[0 ... HEX_REG_LAST_VALUE - 1] = UINT32_MAX,
[HEX_REG_PC] = 0
...
};
It might be clearer, and easier to initialize, if you invert the sense of the mask: only
set bits that are immutable, so that you get
static const target_ulong gpr_immutable_masks[HEX_REG_LAST_VALUE] = {
[HEX_REG_PC] = UINT32_MAX,
[HEX_REG_GP] = 0x3f,
...
};
r~
[PULL v2 16/20] accel/tcg: Add fast path for translator_ld*
Cache the translation from guest to host address, so we may
use direct loads when we hit on the primary translation page.
Look up the second translation page only once, during translation.
This obviates another lookup of the second page within tb_gen_code
after translation.
Fixes a bug in that plugin_insn_append should be passed the bytes
in the original memory order, not bswapped by pieces.
Acked-by: Ilya Leoshkevich
Tested-by: Ilya Leoshkevich
Signed-off-by: Richard Henderson
---
include/exec/translator.h | 63 +++
accel/tcg/translate-all.c | 23 +++
accel/tcg/translator.c| 126 +-
3 files changed, 141 insertions(+), 71 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 69db0f5c21..3b77f5f4aa 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -81,24 +81,14 @@ typedef enum DisasJumpType {
* Architecture-agnostic disassembly context.
*/
typedef struct DisasContextBase {
-const TranslationBlock *tb;
+TranslationBlock *tb;
target_ulong pc_first;
target_ulong pc_next;
DisasJumpType is_jmp;
int num_insns;
int max_insns;
bool singlestep_enabled;
-#ifdef CONFIG_USER_ONLY
-/*
- * Guest address of the last byte of the last protected page.
- *
- * Pages containing the translated instructions are made non-writable in
- * order to achieve consistency in case another thread is modifying the
- * code while translate_insn() fetches the instruction bytes piecemeal.
- * Such writer threads are blocked on mmap_lock() in page_unprotect().
- */
-target_ulong page_protect_end;
-#endif
+void *host_addr[2];
} DisasContextBase;
/**
@@ -183,24 +173,43 @@ bool translator_use_goto_tb(DisasContextBase *db,
target_ulong dest);
* the relevant information at translation time.
*/
-#define GEN_TRANSLATOR_LD(fullname, type, load_fn, swap_fn) \
-type fullname ## _swap(CPUArchState *env, DisasContextBase *dcbase, \
- abi_ptr pc, bool do_swap); \
-static inline type fullname(CPUArchState *env, \
-DisasContextBase *dcbase, abi_ptr pc) \
-{ \
-return fullname ## _swap(env, dcbase, pc, false); \
+uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
+uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
+uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
+uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
+
+static inline uint16_t
+translator_lduw_swap(CPUArchState *env, DisasContextBase *db,
+ abi_ptr pc, bool do_swap)
+{
+uint16_t ret = translator_lduw(env, db, pc);
+if (do_swap) {
+ret = bswap16(ret);
}
+return ret;
+}
-#define FOR_EACH_TRANSLATOR_LD(F) \
-F(translator_ldub, uint8_t, cpu_ldub_code, /* no swap */) \
-F(translator_lduw, uint16_t, cpu_lduw_code, bswap16)\
-F(translator_ldl, uint32_t, cpu_ldl_code, bswap32) \
-F(translator_ldq, uint64_t, cpu_ldq_code, bswap64)
+static inline uint32_t
+translator_ldl_swap(CPUArchState *env, DisasContextBase *db,
+abi_ptr pc, bool do_swap)
+{
+uint32_t ret = translator_ldl(env, db, pc);
+if (do_swap) {
+ret = bswap32(ret);
+}
+return ret;
+}
-FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
-
-#undef GEN_TRANSLATOR_LD
+static inline uint64_t
+translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
+abi_ptr pc, bool do_swap)
+{
+uint64_t ret = translator_ldq(env, db, pc);
+if (do_swap) {
+ret = bswap64(ret);
+}
+return ret;
+}
/*
* Return whether addr is on the same page as where disassembly started.
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 587886aa4e..f5e8592d4a 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1385,8 +1385,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
{
CPUArchState *env = cpu->env_ptr;
TranslationBlock *tb, *existing_tb;
-tb_page_addr_t phys_pc, phys_page2;
-target_ulong virt_page2;
+tb_page_addr_t phys_pc;
tcg_insn_unit *gen_code_buf;
int gen_code_size, search_size, max_insns;
#ifdef CONFIG_PROFILER
@@ -1429,6 +1428,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
tb->flags = flags;
tb->cflags = cflags;
tb->trace_vcpu_dstate = *cpu->trace_dstate;
+tb->page_addr[0] = phys_pc;
+tb->page_addr[1] = -1;
tcg_ctx->tb_cflags = cflags;
tb_overflow:
@@ -1622,13 +1623,11 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
}
/*
- * If the TB is not associated with a physical RAM page then
-
[PULL v2 00/20] tcg patch queue
v2: Fix incorretly resolved rebase conflict in patch 16. r~ The following changes since commit 61fd710b8da8aedcea9b4f197283dc38638e4b60: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2022-09-02 13:24:28 -0400) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20220904 for you to fetch changes up to cc64de1fdeb81bc1ab8bb6c7c24bfd4fc9b28ef2: target/riscv: Make translator stop before the end of a page (2022-09-03 09:27:05 +0100) Respect PROT_EXEC in user-only mode. Fix s390x, i386 and riscv for translations crossing a page. Ilya Leoshkevich (4): linux-user: Clear translations on mprotect() accel/tcg: Introduce is_same_page() target/s390x: Make translator stop before the end of a page target/i386: Make translator stop before the end of a page Richard Henderson (16): linux-user/arm: Mark the commpage executable linux-user/hppa: Allocate page zero as a commpage linux-user/x86_64: Allocate vsyscall page as a commpage linux-user: Honor PT_GNU_STACK tests/tcg/i386: Move smc_code2 to an executable section accel/tcg: Properly implement get_page_addr_code for user-only accel/tcg: Unlock mmap_lock after longjmp accel/tcg: Make tb_htable_lookup static accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c accel/tcg: Use probe_access_internal for softmmu get_page_addr_code_hostp accel/tcg: Document the faulting lookup in tb_lookup_cmp accel/tcg: Remove translator_ldsw accel/tcg: Add pc and host_pc params to gen_intermediate_code accel/tcg: Add fast path for translator_ld* target/riscv: Add MAX_INSN_LEN and insn_len target/riscv: Make translator stop before the end of a page include/elf.h | 1 + include/exec/cpu-common.h | 1 + include/exec/exec-all.h | 89 include/exec/translator.h | 96 - linux-user/arm/target_cpu.h | 4 +- linux-user/qemu.h | 1 + accel/tcg/cpu-exec.c | 143 -- accel/tcg/cputlb.c| 93 +++-- accel/tcg/translate-all.c | 29 accel/tcg/translator.c| 135 ++- accel/tcg/user-exec.c | 17 - linux-user/elfload.c | 82 -- linux-user/mmap.c | 6 +- softmmu/physmem.c | 12 target/alpha/translate.c | 5 +- target/arm/translate.c| 5 +- target/avr/translate.c| 5 +- target/cris/translate.c | 5 +- target/hexagon/translate.c| 6 +- target/hppa/translate.c | 5 +- target/i386/tcg/translate.c | 71 +++ target/loongarch/translate.c | 6 +- target/m68k/translate.c | 5 +- target/microblaze/translate.c | 5 +- target/mips/tcg/translate.c | 5 +- target/nios2/translate.c | 5 +- target/openrisc/translate.c | 6 +- target/ppc/translate.c| 5 +- target/riscv/translate.c | 32 +++-- target/rx/translate.c | 5 +- target/s390x/tcg/translate.c | 20 -- target/sh4/translate.c| 5 +- target/sparc/translate.c | 5 +- target/tricore/translate.c| 6 +- target/xtensa/translate.c | 6 +- tests/tcg/i386/test-i386.c| 2 +- tests/tcg/riscv64/noexec.c| 79 + tests/tcg/s390x/noexec.c | 106 tests/tcg/x86_64/noexec.c | 75 tests/tcg/multiarch/noexec.c.inc | 139 tests/tcg/riscv64/Makefile.target | 1 + tests/tcg/s390x/Makefile.target | 1 + tests/tcg/x86_64/Makefile.target | 3 +- 43 files changed, 966 insertions(+), 367 deletions(-) create mode 100644 tests/tcg/riscv64/noexec.c create mode 100644 tests/tcg/s390x/noexec.c create mode 100644 tests/tcg/x86_64/noexec.c create mode 100644 tests/tcg/multiarch/noexec.c.inc
Re: [PATCH v5 06/14] hw/ppc: set machine->fdt in sam460ex_load_device_tree()
On Sat, 3 Sep 2022, Daniel Henrique Barboza wrote:
This will enable support for 'dumpdtb' QMP/HMP command for the sam460ex
machine.
Cc: BALATON Zoltan
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/sam460ex.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 850bb3b817..fa6f125fda 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -137,6 +137,7 @@ static int sam460ex_load_device_tree(hwaddr addr,
hwaddr initrd_size,
const char *kernel_cmdline)
{
+MachineState *machine = MACHINE(qdev_get_machine());
Sorry for not noticing this earlier but this is ugly, I think you could
just change the prototype of this function to take MachineState *machine
instead of the ramsize and kernel_cmdline params which then can also be
get from machine so they are not needed as separate params any more.
uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
char *filename;
int fdt_size;
@@ -208,7 +209,9 @@ static int sam460ex_load_device_tree(hwaddr addr,
EBC_FREQ);
rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
You've changed this and a few other places to the short comment but left
the long version in pegasos2 and other machines. Could you change all of
them to make it more consistent and less lines altogether?
Regards,
BALATON Zoltan
+machine->fdt = fdt;
return fdt_size;
}
[PATCH v5 10/14] hw/ppc: set machine->fdt in spapr machine
The pSeries machine never bothered with the common machine->fdt attribute. We do all the FDT related work using spapr->fdt_blob. We're going to introduce a QMP/HMP command to dump the FDT, which will rely on setting machine->fdt properly to work across all machine archs/types. Let's set machine->fdt in two places where we manipulate the FDT: spapr_machine_reset() and CAS. There are other places where the FDT is manipulated in the pSeries machines, most notably the hotplug/unplug path. For now we'll acknowledge that we won't have the most accurate representation of the FDT, depending on the current machine state, when using this QMP/HMP fdt command. Making the internal FDT representation always match the actual FDT representation that the guest is using is a problem for another day. spapr->fdt_blob is left untouched for now. To replace it with machine->fdt, since we're migrating spapr->fdt_blob, we would need to migrate machine->fdt as well. This is something that we would like to to do keep our code simpler but it's also a work we'll leave for later. Cc: Cédric Le Goater Cc: qemu-...@nongnu.org Reviewed-by: David Gibson Signed-off-by: Daniel Henrique Barboza --- hw/ppc/spapr.c | 6 ++ hw/ppc/spapr_hcall.c | 8 2 files changed, 14 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index fb790b61e4..f0e5144d83 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1713,6 +1713,12 @@ static void spapr_machine_reset(MachineState *machine) spapr->fdt_initial_size = spapr->fdt_size; spapr->fdt_blob = fdt; +/* + * Set the common machine->fdt pointer to enable support + * for the 'dumpdtb' QMP/HMP command. + */ +machine->fdt = fdt; + /* Set up the entry state */ first_ppc_cpu->env.gpr[5] = 0; diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index a8d4a6bcf0..891206e893 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1256,6 +1256,14 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu, spapr->fdt_initial_size = spapr->fdt_size; spapr->fdt_blob = fdt; +/* + * Set the machine->fdt pointer again since we just freed + * it above (by freeing spapr->fdt_blob). We set this + * pointer to enable support for the 'dumpdtb' QMP/HMP + * command. + */ +MACHINE(spapr)->fdt = fdt; + return H_SUCCESS; } -- 2.37.2
[PATCH v5 07/14] hw/ppc: set machine->fdt in xilinx_load_device_tree()
This will enable support for 'dumpdtb' QMP/HMP command for the
virtex_ml507 machine.
Cc: Edgar E. Iglesias
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/virtex_ml507.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 493ea0c19f..bb21b2a309 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -45,6 +45,8 @@
#include "hw/qdev-properties.h"
#include "ppc405.h"
+#include
+
#define EPAPR_MAGIC(0x45504150)
#define FLASH_SIZE (16 * MiB)
@@ -150,6 +152,7 @@ static int xilinx_load_device_tree(hwaddr addr,
hwaddr initrd_size,
const char *kernel_cmdline)
{
+MachineState *machine = MACHINE(qdev_get_machine());
char *path;
int fdt_size;
void *fdt = NULL;
@@ -194,7 +197,13 @@ static int xilinx_load_device_tree(hwaddr addr,
if (r < 0)
fprintf(stderr, "couldn't set /chosen/bootargs\n");
cpu_physical_memory_write(addr, fdt, fdt_size);
-g_free(fdt);
+
+/*
+ * Update the machine->fdt pointer to enable support for
+ * 'dumpdtb'QMP/HMP command.
+ */
+machine->fdt = fdt;
+
return fdt_size;
}
--
2.37.2
[PATCH v5 06/14] hw/ppc: set machine->fdt in sam460ex_load_device_tree()
This will enable support for 'dumpdtb' QMP/HMP command for the sam460ex
machine.
Cc: BALATON Zoltan
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/sam460ex.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 850bb3b817..fa6f125fda 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -137,6 +137,7 @@ static int sam460ex_load_device_tree(hwaddr addr,
hwaddr initrd_size,
const char *kernel_cmdline)
{
+MachineState *machine = MACHINE(qdev_get_machine());
uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
char *filename;
int fdt_size;
@@ -208,7 +209,9 @@ static int sam460ex_load_device_tree(hwaddr addr,
EBC_FREQ);
rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
return fdt_size;
}
--
2.37.2
[PATCH v5 14/14] qmp/hmp, device_tree.c: introduce dumpdtb
To save the FDT blob we have the '-machine dumpdtb=' property.
With this property set, the machine saves the FDT in and exit.
The created file can then be converted to plain text dts format using
'dtc'.
There's nothing particularly sophisticated into saving the FDT that
can't be done with the machine at any state, as long as the machine has
a valid FDT to be saved.
The 'dumpdtb' command receives a 'filename' paramenter and, if a valid
FDT is available, it'll save it in a file 'filename'. In short, this is
a '-machine dumpdtb' that can be fired on demand via QMP/HMP.
A valid FDT consists of a FDT that was created using libfdt being
retrieved via 'current_machine->fdt' in device_tree.c. This condition is
met by most FDT users in QEMU.
This command will always be executed in-band (i.e. holding BQL),
avoiding potential race conditions with machines that might change the
FDT during runtime (e.g. PowerPC 'pseries' machine).
Cc: Dr. David Alan Gilbert
Cc: Markus Armbruster
Reviewed-by: Alistair Francis
Signed-off-by: Daniel Henrique Barboza
---
hmp-commands.hx | 15 +++
include/sysemu/device_tree.h | 1 +
monitor/misc.c | 1 +
qapi/machine.json| 18 ++
softmmu/device_tree.c| 31 +++
5 files changed, 66 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 182e639d14..9a3e57504f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1800,3 +1800,18 @@ ERST
"\n\t\t\t\t\t limit on a specified virtual cpu",
.cmd= hmp_cancel_vcpu_dirty_limit,
},
+
+#if defined(CONFIG_FDT)
+SRST
+``dumpdtb`` *filename*
+ Save the FDT in the 'filename' file to be decoded using dtc.
+ Requires 'libfdt' support.
+ERST
+{
+.name = "dumpdtb",
+.args_type = "filename:F",
+.params = "filename",
+.help = "save the FDT in the 'filename' file to be decoded using
dtc",
+.cmd= hmp_dumpdtb,
+},
+#endif
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ef060a9759..e7c5441f56 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -136,6 +136,7 @@ int qemu_fdt_add_path(void *fdt, const char *path);
} while (0)
void qemu_fdt_dumpdtb(void *fdt, int size);
+void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
/**
* qemu_fdt_setprop_sized_cells_from_array:
diff --git a/monitor/misc.c b/monitor/misc.c
index 3d2312ba8d..e7dd63030b 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -49,6 +49,7 @@
#include "sysemu/blockdev.h"
#include "sysemu/sysemu.h"
#include "sysemu/tpm.h"
+#include "sysemu/device_tree.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qstring.h"
diff --git a/qapi/machine.json b/qapi/machine.json
index 6afd1936b0..f968a5d343 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1664,3 +1664,21 @@
'*size': 'size',
'*max-size': 'size',
'*slots': 'uint64' } }
+
+##
+# @dumpdtb:
+#
+# Save the FDT in dtb format. Requires 'libfdt' support.
+#
+# @filename: name of the FDT file to be created
+#
+# Since: 7.2
+#
+# Example:
+# {"execute": "dumpdtb"}
+#"arguments": { "filename": "/tmp/fdt.dtb" } }
+#
+##
+{ 'command': 'dumpdtb',
+ 'data': { 'filename': 'str' },
+ 'if': 'CONFIG_FDT' }
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 6ca3fad285..cdd41b6de6 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -26,6 +26,9 @@
#include "hw/loader.h"
#include "hw/boards.h"
#include "qemu/config-file.h"
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/qmp/qdict.h"
+#include "monitor/hmp.h"
#include
@@ -643,3 +646,31 @@ out:
g_free(propcells);
return ret;
}
+
+void qmp_dumpdtb(const char *filename, Error **errp)
+{
+g_autoptr(GError) err = NULL;
+int size;
+
+if (!current_machine->fdt) {
+error_setg(errp, "Unable to find the machine FDT");
+return;
+}
+
+size = fdt_totalsize(current_machine->fdt);
+
+if (!g_file_set_contents(filename, current_machine->fdt, size, )) {
+error_setg(errp, "Error saving FDT to file %s: %s",
+ filename, err->message);
+}
+}
+
+void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
+{
+const char *filename = qdict_get_str(qdict, "filename");
+Error *local_err = NULL;
+
+qmp_dumpdtb(filename, _err);
+
+hmp_handle_error(mon, local_err);
+}
--
2.37.2
[PATCH v5 04/14] hw/ppc: set machine->fdt in ppce500_load_device_tree()
This will enable support for 'dumpdtb' QMP/HMP command for the e500 machine. Cc: Cédric Le Goater Signed-off-by: Daniel Henrique Barboza --- hw/ppc/e500.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 32495d0123..ea5f947824 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -47,6 +47,8 @@ #include "hw/i2c/i2c.h" #include "hw/irq.h" +#include + #define EPAPR_MAGIC(0x45504150) #define DTC_LOAD_PAD 0x180 #define DTC_PAD_MASK 0xF @@ -600,7 +602,16 @@ done: cpu_physical_memory_write(addr, fdt, fdt_size); } ret = fdt_size; -g_free(fdt); + +/* + * Update the machine->fdt pointer to enable support for the + * 'dumpdtb' QMP/HMP command. + * + * The FDT is re-created during reset, so free machine->fdt + * to avoid leaking the old FDT. + */ +g_free(machine->fdt); +machine->fdt = fdt; out: g_free(pci_map); -- 2.37.2
[PATCH v5 12/14] hw/riscv: set machine->fdt in spike_board_init()
This will enable support for the 'dumpdtb' QMP/HMP command for the spike
machine.
Cc: Palmer Dabbelt
Cc: Alistair Francis
Cc: Bin Meng
Reviewed-by: Alistair Francis
Signed-off-by: Daniel Henrique Barboza
---
hw/riscv/spike.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index e41b6aa9f0..1fa41164b3 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -40,6 +40,8 @@
#include "sysemu/device_tree.h"
#include "sysemu/sysemu.h"
+#include
+
static const MemMapEntry spike_memmap[] = {
[SPIKE_MROM] = { 0x1000, 0xf000 },
[SPIKE_HTIF] = { 0x100, 0x1000 },
@@ -304,6 +306,13 @@ static void spike_board_init(MachineState *machine)
/* Compute the fdt load address in dram */
fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
machine->ram_size, s->fdt);
+
+/*
+ * Update the machine->fdt pointer to enable support for
+ * the 'dumpdtb' QMP/HMP command.
+ */
+machine->fdt = s->fdt;
+
/* load the reset vector */
riscv_setup_rom_reset_vec(machine, >soc[0], memmap[SPIKE_DRAM].base,
memmap[SPIKE_MROM].base,
--
2.37.2
[PATCH v5 13/14] hw/xtensa: set machine->fdt in xtfpga_init()
This will enable support for the 'dumpdtb' QMP/HMP command for all
xtensa machines that uses a FDT.
Signed-off-by: Daniel Henrique Barboza
---
hw/xtensa/meson.build | 2 +-
hw/xtensa/xtfpga.c| 9 -
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/xtensa/meson.build b/hw/xtensa/meson.build
index 1d5835df4b..ebba51cc74 100644
--- a/hw/xtensa/meson.build
+++ b/hw/xtensa/meson.build
@@ -6,6 +6,6 @@ xtensa_ss.add(files(
))
xtensa_ss.add(when: 'CONFIG_XTENSA_SIM', if_true: files('sim.c'))
xtensa_ss.add(when: 'CONFIG_XTENSA_VIRT', if_true: files('virt.c'))
-xtensa_ss.add(when: 'CONFIG_XTENSA_XTFPGA', if_true: files('xtfpga.c'))
+xtensa_ss.add(when: 'CONFIG_XTENSA_XTFPGA', if_true: [files('xtfpga.c'), fdt])
hw_arch += {'xtensa': xtensa_ss}
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 2a5556a35f..138453628a 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -50,6 +50,8 @@
#include "hw/xtensa/mx_pic.h"
#include "migration/vmstate.h"
+#include
+
typedef struct XtfpgaFlashDesc {
hwaddr base;
size_t size;
@@ -377,7 +379,12 @@ static void xtfpga_init(const XtfpgaBoardDesc *board,
MachineState *machine)
cur_tagptr = put_tag(cur_tagptr, BP_TAG_FDT,
sizeof(dtb_addr), _addr);
cur_lowmem = QEMU_ALIGN_UP(cur_lowmem + fdt_size, 4 * KiB);
-g_free(fdt);
+
+/*
+ * Update the machine->fdt pointer to enable support for
+ * the 'dumpdtb' QMP/HMP command.
+ */
+machine->fdt = fdt;
}
#else
if (dtb_filename) {
--
2.37.2
[PATCH v5 11/14] hw/riscv: set machine->fdt in sifive_u_machine_init()
This will enable support for 'dumpdtb' QMP/HMP command for the sifive_u
machine.
Cc: Alistair Francis
Cc: Bin Meng
Cc: Palmer Dabbelt
Reviewed-by: Alistair Francis
Signed-off-by: Daniel Henrique Barboza
---
hw/riscv/sifive_u.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..07e25d0740 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -634,6 +634,12 @@ static void sifive_u_machine_init(MachineState *machine)
start_addr_hi32 = (uint64_t)start_addr >> 32;
}
+/*
+ * Update the machine->fdt pointer to enable support for
+ * the 'dumpdtb' QMP/HMP command.
+ */
+machine->fdt = s->fdt;
+
/* reset vector */
uint32_t reset_vec[12] = {
s->msel, /* MSEL pin state */
--
2.37.2
[PATCH v5 02/14] hw/microblaze: set machine->fdt in microblaze_load_dtb()
This will enable support for 'dumpdtb' QMP/HMP command for all
microblaze machines that uses microblaze_load_dtb().
Cc: Edgar E. Iglesias
Signed-off-by: Daniel Henrique Barboza
---
hw/microblaze/boot.c | 8 +++-
hw/microblaze/meson.build | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 8b92a9801a..c8eff7b6fc 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -39,6 +39,8 @@
#include "boot.h"
+#include
+
static struct
{
void (*machine_cpu_reset)(MicroBlazeCPU *);
@@ -72,6 +74,7 @@ static int microblaze_load_dtb(hwaddr addr,
const char *kernel_cmdline,
const char *dtb_filename)
{
+MachineState *machine = MACHINE(qdev_get_machine());
int fdt_size;
void *fdt = NULL;
int r;
@@ -100,7 +103,10 @@ static int microblaze_load_dtb(hwaddr addr,
}
cpu_physical_memory_write(addr, fdt, fdt_size);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
return fdt_size;
}
diff --git a/hw/microblaze/meson.build b/hw/microblaze/meson.build
index bb9e4eb8f4..a38a397872 100644
--- a/hw/microblaze/meson.build
+++ b/hw/microblaze/meson.build
@@ -1,5 +1,5 @@
microblaze_ss = ss.source_set()
-microblaze_ss.add(files('boot.c'))
+microblaze_ss.add(files('boot.c'), fdt)
microblaze_ss.add(when: 'CONFIG_PETALOGIX_S3ADSP1800', if_true:
files('petalogix_s3adsp1800_mmu.c'))
microblaze_ss.add(when: 'CONFIG_PETALOGIX_ML605', if_true:
files('petalogix_ml605_mmu.c'))
microblaze_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true:
files('xlnx-zynqmp-pmu.c'))
--
2.37.2
[PATCH v5 09/14] hw/ppc: set machine->fdt in pnv_reset()
This will enable support for the 'dumpdtb' QMP/HMP command for all powernv machines. Cc: Cédric Le Goater Cc: Frederic Barrat Signed-off-by: Daniel Henrique Barboza --- hw/ppc/pnv.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 354aa289d1..afd90d261b 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -678,7 +678,13 @@ static void pnv_reset(MachineState *machine) qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); -g_free(fdt); +/* + * Update the machine->fdt pointer to enable support for + * 'dumpdtb' QMP/HMP command. Free the existing machine->fdt + * to avoid leaking it during a reset. + */ +g_free(machine->fdt); +machine->fdt = fdt; } static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) -- 2.37.2
[PATCH v5 01/14] hw/arm: do not free machine->fdt in arm_load_dtb()
At this moment, arm_load_dtb() can free machine->fdt when
binfo->dtb_filename is NULL. If there's no 'dtb_filename', 'fdt' will be
retrieved by binfo->get_dtb(). If get_dtb() returns machine->fdt, as is
the case of machvirt_dtb() from hw/arm/virt.c, fdt now has a pointer to
machine->fdt. And, in that case, the existing g_free(fdt) at the end of
arm_load_dtb() will make machine->fdt point to an invalid memory region.
This is not an issue right now because there's no code that access
machine->fdt after arm_load_dtb(), but we're going to add a QMP/HMP
FDT command that will rely on machine->fdt being valid.
Instead of freeing 'fdt' at the end of arm_load_dtb(), assign it to
machine->fdt. This will allow the FDT of ARM machines that relies on
arm_load_dtb() to be accessed later on.
Since all ARM machines allocates the FDT only once, we don't need to
worry about leaking the existing FDT during a machine reset (which is
something that other machines have to look after, e.g. the ppc64 pSeries
machine).
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Signed-off-by: Daniel Henrique Barboza
---
hw/arm/boot.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..60bbfba37f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -684,7 +684,8 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info
*binfo,
*/
rom_add_blob_fixed_as("dtb", fdt, size, addr, as);
-g_free(fdt);
+/* Set ms->fdt for 'dumpdtb' QMP/HMP command */
+ms->fdt = fdt;
return size;
--
2.37.2
[PATCH v5 08/14] hw/ppc: set machine->fdt in pegasos2_machine_reset()
We'll introduce a QMP/HMP command that requires machine->fdt to be set properly. Cc: BALATON Zoltan Cc: qemu-...@nongnu.org Signed-off-by: Daniel Henrique Barboza --- hw/ppc/pegasos2.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c index 61f4263953..d7ea139ff4 100644 --- a/hw/ppc/pegasos2.c +++ b/hw/ppc/pegasos2.c @@ -331,6 +331,13 @@ static void pegasos2_machine_reset(MachineState *machine) vof_build_dt(fdt, pm->vof); vof_client_open_store(fdt, pm->vof, "/chosen", "stdout", "/failsafe"); + +/* + * Set the common machine->fdt pointer to enable support + * for 'dumpdtb' QMP/HMP command. + */ +machine->fdt = fdt; + pm->cpu->vhyp = PPC_VIRTUAL_HYPERVISOR(machine); } -- 2.37.2
[PATCH v5 05/14] hw/ppc: set machine->fdt in bamboo_load_device_tree()
This will enable support for 'dumpdtb' QMP/HMP command for the bamboo
machine.
Cc: Cédric Le Goater
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/ppc440_bamboo.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index ea945a1c99..aa5c901b09 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -34,6 +34,8 @@
#include "hw/qdev-properties.h"
#include "qapi/error.h"
+#include
+
#define BINARY_DEVICE_TREE_FILE "bamboo.dtb"
/* from u-boot */
@@ -62,6 +64,7 @@ static int bamboo_load_device_tree(hwaddr addr,
hwaddr initrd_size,
const char *kernel_cmdline)
{
+MachineState *machine = MACHINE(qdev_get_machine());
int ret = -1;
uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
char *filename;
@@ -119,7 +122,13 @@ static int bamboo_load_device_tree(hwaddr addr,
tb_freq);
rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-g_free(fdt);
+
+/*
+ * Update the machine->fdt pointer to enable support for
+ * 'dumpdtb' QMP/HMP command.
+ */
+machine->fdt = fdt;
+
return 0;
}
--
2.37.2
[PATCH v5 03/14] hw/nios2: set machine->fdt in nios2_load_dtb()
This will enable support for 'dumpdtb' QMP/HMP command for all nios2
machines that uses nios2_load_dtb().
Cc: Chris Wulff
Cc: Marek Vasut
Signed-off-by: Daniel Henrique Barboza
---
hw/nios2/boot.c | 8 +++-
hw/nios2/meson.build | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 21cb47..b30a7b1efb 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -43,6 +43,8 @@
#include "boot.h"
+#include
+
#define NIOS2_MAGIC0x534f494e
static struct nios2_boot_info {
@@ -81,6 +83,7 @@ static uint64_t translate_kernel_address(void *opaque,
uint64_t addr)
static int nios2_load_dtb(struct nios2_boot_info bi, const uint32_t ramsize,
const char *kernel_cmdline, const char *dtb_filename)
{
+MachineState *machine = MACHINE(qdev_get_machine());
int fdt_size;
void *fdt = NULL;
int r;
@@ -113,7 +116,10 @@ static int nios2_load_dtb(struct nios2_boot_info bi, const
uint32_t ramsize,
}
cpu_physical_memory_write(bi.fdt, fdt, fdt_size);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
return fdt_size;
}
diff --git a/hw/nios2/meson.build b/hw/nios2/meson.build
index 6c58e8082b..22277bd6c5 100644
--- a/hw/nios2/meson.build
+++ b/hw/nios2/meson.build
@@ -1,5 +1,5 @@
nios2_ss = ss.source_set()
-nios2_ss.add(files('boot.c'))
+nios2_ss.add(files('boot.c'), fdt)
nios2_ss.add(when: 'CONFIG_NIOS2_10M50', if_true: files('10m50_devboard.c'))
nios2_ss.add(when: 'CONFIG_NIOS2_GENERIC_NOMMU', if_true:
files('generic_nommu.c'))
--
2.37.2
[PATCH v5 00/14] QMP/HMP: introduce 'dumpdtb'
Hi, In this version I removed the 'info fdt' command. It didn't get enough traction/acceptance to justify the amount of code to implement it. Aside from that, all other changes are based on Markus' feedback on patch 14. Changes from v4: - patches 15-21: removed - patch 14: - dumpdtb is now dependent on 'if: CONFIG_FDT' in qapi/machine.json - dumpdtb is now dependent on 'if defined(CONFIG_FDT)' in hmp-commands.hx - moved qmp_dumpdtb() and hmp_dumpdtb() to device_tree.c - added a GError pointer to g_file_set_contents() to report errors caught - hmp_handle_error() is now called unconditionally - v4 link: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg04115.html Daniel Henrique Barboza (14): hw/arm: do not free machine->fdt in arm_load_dtb() 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() qmp/hmp, device_tree.c: introduce dumpdtb hmp-commands.hx | 15 +++ hw/arm/boot.c| 3 ++- hw/microblaze/boot.c | 8 +++- hw/microblaze/meson.build| 2 +- hw/nios2/boot.c | 8 +++- hw/nios2/meson.build | 2 +- hw/ppc/e500.c| 13 - hw/ppc/pegasos2.c| 7 +++ hw/ppc/pnv.c | 8 +++- hw/ppc/ppc440_bamboo.c | 11 ++- hw/ppc/sam460ex.c| 5 - hw/ppc/spapr.c | 6 ++ hw/ppc/spapr_hcall.c | 8 hw/ppc/virtex_ml507.c| 11 ++- hw/riscv/sifive_u.c | 6 ++ hw/riscv/spike.c | 9 + hw/xtensa/meson.build| 2 +- hw/xtensa/xtfpga.c | 9 - include/sysemu/device_tree.h | 1 + monitor/misc.c | 1 + qapi/machine.json| 18 ++ softmmu/device_tree.c| 31 +++ 22 files changed, 172 insertions(+), 12 deletions(-) -- 2.37.2
[PATCH 2/2] vvfat: allow spaces in file names
In R/W mode, files with spaces were never created on host side.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1176
Fixes: c79e243ed67683d6d06692bd7040f7394da178b0
Signed-off-by: Hervé Poussineau
---
block/vvfat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 35057a51c67..9d877028573 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -499,7 +499,7 @@ static bool valid_filename(const unsigned char *name)
(c >= 'A' && c <= 'Z') ||
(c >= 'a' && c <= 'z') ||
c > 127 ||
- strchr("$%'-_@~`!(){}^#&.+,;=[]", c) != NULL))
+ strchr(" $%'-_@~`!(){}^#&.+,;=[]", c) != NULL))
{
return false;
}
--
2.36.2
[PATCH 0/2] Fix some problems with vvfat in R/W mode
Hi, When testing vvfat in read-write mode, I came across some blocking problems when using Windows guests. This patchset is not here to fix all problems of vvfat, but only the main ones I encountered. First patch allows setting/resetting the 'volume dirty' flag on boosector, and the second one allows creating file names with spaces. Hervé Hervé Poussineau (2): vvfat: allow some writes to bootsector vvfat: allow spaces in file names block/vvfat.c | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) -- 2.36.2
[PATCH 1/2] vvfat: allow some writes to bootsector
'reserved1' field in bootsector is used to mark volume dirty, or need to verify.
Allow writes to bootsector which only changes the 'reserved1' field.
This fixes I/O errors on Windows guests.
Resolves: https://bugs.launchpad.net/qemu/+bug/1889421
Signed-off-by: Hervé Poussineau
---
block/vvfat.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index d6dd919683d..35057a51c67 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2993,11 +2993,27 @@ DLOG(checkpoint());
vvfat_close_current_file(s);
+if (sector_num == s->offset_to_bootsector && nb_sectors == 1) {
+/*
+ * Write on bootsector. Allow only changing the reserved1 field,
+ * used to mark volume dirtiness
+ */
+const unsigned char *initial = s->first_sectors
+ + s->offset_to_bootsector * 0x200;
+for (i = 0; i < 0x200; i++) {
+if (i != offsetof(bootsector_t, u.fat16.reserved1) &&
+initial[i] != buf[i]) {
+fprintf(stderr, "Tried to write to protected bootsector\n");
+return -1;
+}
+}
+return 0;
+}
+
/*
* Some sanity checks:
* - do not allow writing to the boot sector
*/
-
if (sector_num < s->offset_to_fat)
return -1;
--
2.36.2
[PATCH] 9pfs: use GHashMap for fid table
The previous implementation would iterate over the fid table for
lookup operations, resulting in an operation with O(n) complexity on
the number of open files and poor cache locality -- for nearly every
open, stat, read, write, etc operation.
This change uses a hashtable for this instead, significantly improving
the performance of the 9p filesystem. The runtime of NixOS's simple
installer test, which copies ~122k files totalling ~1.8GiB from 9p,
decreased by a factor of about 10.
Signed-off-by: Linus Heckemann
---
hw/9pfs/9p.c | 130 +++
hw/9pfs/9p.h | 2 +-
2 files changed, 69 insertions(+), 63 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index aebadeaa03..ff466afe39 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -282,33 +282,31 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu,
int32_t fid)
V9fsFidState *f;
V9fsState *s = pdu->s;
-QSIMPLEQ_FOREACH(f, >fid_list, next) {
-BUG_ON(f->clunked);
-if (f->fid == fid) {
-/*
- * Update the fid ref upfront so that
- * we don't get reclaimed when we yield
- * in open later.
- */
-f->ref++;
-/*
- * check whether we need to reopen the
- * file. We might have closed the fd
- * while trying to free up some file
- * descriptors.
- */
-err = v9fs_reopen_fid(pdu, f);
-if (err < 0) {
-f->ref--;
-return NULL;
-}
-/*
- * Mark the fid as referenced so that the LRU
- * reclaim won't close the file descriptor
- */
-f->flags |= FID_REFERENCED;
-return f;
+f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
+if (f) {
+/*
+ * Update the fid ref upfront so that
+ * we don't get reclaimed when we yield
+ * in open later.
+ */
+f->ref++;
+/*
+ * check whether we need to reopen the
+ * file. We might have closed the fd
+ * while trying to free up some file
+ * descriptors.
+ */
+err = v9fs_reopen_fid(pdu, f);
+if (err < 0) {
+f->ref--;
+return NULL;
}
+/*
+ * Mark the fid as referenced so that the LRU
+ * reclaim won't close the file descriptor
+ */
+f->flags |= FID_REFERENCED;
+return f;
}
return NULL;
}
@@ -317,12 +315,9 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *f;
-QSIMPLEQ_FOREACH(f, >fid_list, next) {
+if (g_hash_table_contains(s->fids, GINT_TO_POINTER(fid))) {
/* If fid is already there return NULL */
-BUG_ON(f->clunked);
-if (f->fid == fid) {
-return NULL;
-}
+return NULL;
}
f = g_new0(V9fsFidState, 1);
f->fid = fid;
@@ -333,7 +328,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
* reclaim won't close the file descriptor
*/
f->flags |= FID_REFERENCED;
-QSIMPLEQ_INSERT_TAIL(>fid_list, f, next);
+g_hash_table_insert(s->fids, GINT_TO_POINTER(fid), f);
v9fs_readdir_init(s->proto_version, >fs.dir);
v9fs_readdir_init(s->proto_version, >fs_reclaim.dir);
@@ -424,12 +419,11 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *fidp;
-QSIMPLEQ_FOREACH(fidp, >fid_list, next) {
-if (fidp->fid == fid) {
-QSIMPLEQ_REMOVE(>fid_list, fidp, V9fsFidState, next);
-fidp->clunked = true;
-return fidp;
-}
+fidp = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
+if (fidp) {
+g_hash_table_remove(s->fids, GINT_TO_POINTER(fid));
+fidp->clunked = true;
+return fidp;
}
return NULL;
}
@@ -439,10 +433,15 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
int reclaim_count = 0;
V9fsState *s = pdu->s;
V9fsFidState *f;
+
+GHashTableIter iter;
+gpointer fid;
+g_hash_table_iter_init(, s->fids);
+
QSLIST_HEAD(, V9fsFidState) reclaim_list =
QSLIST_HEAD_INITIALIZER(reclaim_list);
-QSIMPLEQ_FOREACH(f, >fid_list, next) {
+while (g_hash_table_iter_next(, , (void **) )) {
/*
* Unlink fids cannot be reclaimed. Check
* for them and skip them. Also skip fids
@@ -518,12 +517,12 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU
*pdu, V9fsPath *path)
{
int err;
V9fsState *s = pdu->s;
-V9fsFidState *fidp, *fidp_next;
+V9fsFidState *fidp;
+gpointer fid;
+
+GHashTableIter iter;
+g_hash_table_iter_init(, s->fids);
-fidp = QSIMPLEQ_FIRST(>fid_list);
-if (!fidp) {
-return 0;
-}
/*
* v9fs_reopen_fid() can yield : a reference on the fid must be held
@@ -534,7 +533,13 @@ static int
Re: [PATCH 33/51] tests/qtest: {ahci, ide}-test: Use relative path for temporary files
On Thu, Sep 1, 2022 at 4:58 PM Marc-André Lureau
wrote:
>
>
>
> On Wed, Aug 24, 2022 at 2:55 PM Bin Meng wrote:
>>
>> From: Bin Meng
>>
>> These test cases uses "blkdebug:path/to/config:path/to/image" for
>> testing. On Windows, absolute file paths contain the delimiter ':'
>> which causes the blkdebug filename parser fail to parse filenames.
>>
>
> hmm.. maybe it should learn to escape paths..
>
>
>> Signed-off-by: Bin Meng
>> ---
>>
>> tests/qtest/ahci-test.c | 19 ---
>> tests/qtest/ide-test.c | 18 --
>> 2 files changed, 32 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
>> index 0e88cd0eef..bce9ff770c 100644
>> --- a/tests/qtest/ahci-test.c
>> +++ b/tests/qtest/ahci-test.c
>> @@ -1848,7 +1848,7 @@ static void create_ahci_io_test(enum IOMode type, enum
>> AddrMode addr,
>>
>> int main(int argc, char **argv)
>> {
>> -const char *arch;
>> +const char *arch, *base;
>> int ret;
>> int fd;
>> int c;
>> @@ -1886,8 +1886,21 @@ int main(int argc, char **argv)
>> return 0;
>> }
>>
>> +/*
>> + * "base" stores the starting point where we create temporary files.
>> + *
>> + * On Windows, this is set to the relative path of current working
>> + * directory, because the absolute path causes the blkdebug filename
>> + * parser fail to parse "blkdebug:path/to/config:path/to/image".
>> + */
>> +#ifndef _WIN32
>> +base = g_get_tmp_dir();
>> +#else
>> +base = ".";
>> +#endif
>
>
> Meanwhile, that seems reasonable. Perhaps chdir() to the temporary directory
> first? (assuming other paths are absolute)
Other paths in the QEMU command line indeed are absolute, however the
QEMU executable path is set to a relative path from meson.build thus
we cannot chdir() to the temporary directory here.
>
>>
>> +
>> /* Create a temporary image */
>> -tmp_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir());
>> +tmp_path = g_strdup_printf("%s/qtest.XX", base);
>> fd = mkstemp(tmp_path);
>> g_assert(fd >= 0);
>> if (have_qemu_img()) {
>> @@ -1905,7 +1918,7 @@ int main(int argc, char **argv)
>> close(fd);
>>
>> /* Create temporary blkdebug instructions */
>> -debug_path = g_strdup_printf("%s/qtest-blkdebug.XX",
>> g_get_tmp_dir());
>> +debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
>> fd = mkstemp(debug_path);
>> g_assert(fd >= 0);
>> close(fd);
>> diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
>> index ebbf8e0126..c5cad6c0be 100644
>> --- a/tests/qtest/ide-test.c
>> +++ b/tests/qtest/ide-test.c
>> @@ -1011,17 +1011,31 @@ static void test_cdrom_dma(void)
>>
>> int main(int argc, char **argv)
>> {
>> +const char *base;
>> int fd;
>> int ret;
>>
>> +/*
>> + * "base" stores the starting point where we create temporary files.
>> + *
>> + * On Windows, this is set to the relative path of current working
>> + * directory, because the absolute path causes the blkdebug filename
>> + * parser fail to parse "blkdebug:path/to/config:path/to/image".
>> + */
>> +#ifndef _WIN32
>> +base = g_get_tmp_dir();
>> +#else
>> +base = ".";
>> +#endif
>> +
>> /* Create temporary blkdebug instructions */
>> -debug_path = g_strdup_printf("%s/qtest-blkdebug.XX",
>> g_get_tmp_dir());
>> +debug_path = g_strdup_printf("%s/qtest-blkdebug.XX", base);
>> fd = mkstemp(debug_path);
>> g_assert(fd >= 0);
>> close(fd);
>>
>> /* Create a temporary raw image */
>> -tmp_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir());
>> +tmp_path = g_strdup_printf("%s/qtest.XX", base);
>> fd = mkstemp(tmp_path);
>> g_assert(fd >= 0);
>> ret = ftruncate(fd, TEST_IMAGE_SIZE);
>> --
>> 2.34.1
Regards,
Bin
Re: [PATCH] tests/tcg/x86_64: add cross-modifying code test
Ilya Leoshkevich writes:
> commit f025692c992c ("accel/tcg: Clear PAGE_WRITE before translation")
> fixed cross-modifying code handling, but did not add a test. The
> changed code was further improved recently [1], and I was not sure
> whether these modifications were safe (spoiler: they were fine).
>
> Add a test to make sure there are no regressions.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00034.html
>
> Signed-off-by: Ilya Leoshkevich
> ---
> tests/tcg/x86_64/Makefile.target| 6 +-
> tests/tcg/x86_64/cross-modifying-code.c | 80 +
> 2 files changed, 85 insertions(+), 1 deletion(-)
> create mode 100644 tests/tcg/x86_64/cross-modifying-code.c
>
> diff --git a/tests/tcg/x86_64/Makefile.target
> b/tests/tcg/x86_64/Makefile.target
> index b71a6bcd5e..58e7bfd681 100644
> --- a/tests/tcg/x86_64/Makefile.target
> +++ b/tests/tcg/x86_64/Makefile.target
> @@ -10,6 +10,7 @@ include $(SRC_PATH)/tests/tcg/i386/Makefile.target
>
> ifeq ($(filter %-linux-user, $(TARGET)),$(TARGET))
> X86_64_TESTS += vsyscall
> +X86_64_TESTS += cross-modifying-code
> TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
> else
> TESTS=$(MULTIARCH_TESTS)
> @@ -20,5 +21,8 @@ test-x86_64: LDFLAGS+=-lm -lc
> test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
> $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
>
> -vsyscall: $(SRC_PATH)/tests/tcg/x86_64/vsyscall.c
> +%: $(SRC_PATH)/tests/tcg/x86_64/%.c
> $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
You shouldn't need to redefine the default rule when you can tweak the flags
> +
> +smc: CFLAGS+=-pthread
> +smc: LDFLAGS+=-pthread
I think this must be from an old iteration because:
make[1]: Entering directory
'/home/alex/lsrc/qemu.git/builds/all/tests/tcg/x86_64-linux-user'
cc -Wall -Werror -O0 -g -fno-strict-aliasing
/home/alex/lsrc/qemu.git/tests/tcg/x86_64/cross-modifying-code.c -o
cross-modifying-code -static
/usr/bin/ld: /tmp/ccK05RAk.o: in function `main':
/home/alex/lsrc/qemu.git/tests/tcg/x86_64/cross-modifying-code.c:64: undefined
reference to `pthread_create'
/usr/bin/ld:
/home/alex/lsrc/qemu.git/tests/tcg/x86_64/cross-modifying-code.c:73: undefined
reference to `pthread_join'
collect2: error: ld returned 1 exit status
make[1]: *** [/home/alex/lsrc/qemu.git/tests/tcg/x86_64/Makefile.target:25:
cross-modifying-code] Error 1
make[1]: Leaving directory
'/home/alex/lsrc/qemu.git/builds/all/tests/tcg/x86_64-linux-user'
make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:52:
build-tcg-tests-x86_64-linux-user] Error 2
> diff --git a/tests/tcg/x86_64/cross-modifying-code.c
> b/tests/tcg/x86_64/cross-modifying-code.c
> new file mode 100644
> index 00..2704df6061
> --- /dev/null
> +++ b/tests/tcg/x86_64/cross-modifying-code.c
> @@ -0,0 +1,80 @@
> +/*
> + * Test patching code, running in one thread, from another thread.
> + *
> + * Intel SDM calls this "cross-modifying code" and recommends a special
> + * sequence, which requires both threads to cooperate.
> + *
> + * Linux kernel uses a different sequence that does not require cooperation
> and
> + * involves patching the first byte with int3.
> + *
> + * Finally, there is user-mode software out there that simply uses atomics,
> and
> + * that seems to be good enough in practice. Test that QEMU has no problems
> + * with this as well.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +void add1_or_nop(long *x);
> +asm(".pushsection .rwx,\"awx\",@progbits\n"
> +".globl add1_or_nop\n"
> +/* addq $0x1,(%rdi) */
> +"add1_or_nop: .byte 0x48, 0x83, 0x07, 0x01\n"
> +"ret\n"
> +".popsection\n");
> +
> +#define THREAD_WAIT 0
> +#define THREAD_PATCH 1
> +#define THREAD_STOP 2
> +
> +static void *thread_func(void *arg)
> +{
> +int val = 0x0026748d; /* nop */
> +
> +while (true) {
> +switch (__atomic_load_n((int *)arg, __ATOMIC_SEQ_CST)) {
> +case THREAD_WAIT:
> +break;
> +case THREAD_PATCH:
> +val = __atomic_exchange_n((int *)_or_nop, val,
> + __ATOMIC_SEQ_CST);
> +break;
> +case THREAD_STOP:
> +return NULL;
> +default:
> +assert(false);
> +__builtin_unreachable();
> +}
> +}
> +}
> +
> +#define INITIAL 42
> +#define COUNT 100
> +
> +int main(void)
> +{
> +int command = THREAD_WAIT;
> +pthread_t thread;
> +long x = 0;
> +int err;
> +int i;
> +
> +err = pthread_create(, NULL, _func, );
> +assert(err == 0);
> +
> +__atomic_store_n(, THREAD_PATCH, __ATOMIC_SEQ_CST);
> +for (i = 0; i < COUNT; i++) {
> +add1_or_nop();
> +}
> +__atomic_store_n(, THREAD_STOP, __ATOMIC_SEQ_CST);
> +
> +err = pthread_join(thread, NULL);
> +assert(err == 0);
> +
> +assert(x >= INITIAL);
> +assert(x <= INITIAL + COUNT);
> +
> +return EXIT_SUCCESS;
> +}
--
Alex Bennée
