[PATCH for-10.0] docs: Fix some typos (found by codespell and typos)

2025-04-12 Thread Stefan Weil via
Signed-off-by: Stefan Weil 
---
 docs/about/deprecated.rst  | 4 ++--
 docs/devel/codebase.rst| 6 +++---
 docs/devel/qapi-domain.rst | 4 ++--
 include/exec/memory.h  | 4 ++--
 qapi/qdev.json | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 0f41a99c67..05381441a9 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -452,7 +452,7 @@ Backend ``memory`` (since 9.0)
 ``reconnect`` (since 9.2)
 ^
 
-The ``reconnect`` option only allows specifiying second granularity timeouts,
+The ``reconnect`` option only allows specifying second granularity timeouts,
 which is not enough for all types of use cases, use ``reconnect-ms`` instead.
 
 
@@ -462,7 +462,7 @@ Net device options
 Stream ``reconnect`` (since 9.2)
 
 
-The ``reconnect`` option only allows specifiying second granularity timeouts,
+The ``reconnect`` option only allows specifying second granularity timeouts,
 which is not enough for all types of use cases, use ``reconnect-ms`` instead.
 
 VFIO device options
diff --git a/docs/devel/codebase.rst b/docs/devel/codebase.rst
index 1b09953197..ef98578296 100644
--- a/docs/devel/codebase.rst
+++ b/docs/devel/codebase.rst
@@ -5,7 +5,7 @@ Codebase
 This section presents the various parts of QEMU and how the codebase is
 organized.
 
-Beyond giving succint descriptions, the goal is to offer links to various
+Beyond giving succinct descriptions, the goal is to offer links to various
 parts of the documentation/codebase.
 
 Subsystems
@@ -67,7 +67,7 @@ yet, so sometimes the source code is all you have.
 * `chardev `_:
   Various backends used by char devices.
 * `common-user 
`_:
-  User-mode assembly code for dealing with signals occuring during syscalls.
+  User-mode assembly code for dealing with signals occurring during syscalls.
 * `configs `_:
   Makefiles defining configurations to build QEMU.
 * `contrib `_:
@@ -102,7 +102,7 @@ yet, so sometimes the source code is all you have.
 * `.gitlab-ci.d 
`_:
   `CI ` yaml and scripts.
 * `include `_:
-  All headers associated to different subsystems in QEMU. The hierachy used
+  All headers associated to different subsystems in QEMU. The hierarchy used
   mirrors source code organization and naming.
 * `hw `_:
   `Devices ` and boards emulation. Devices are categorized by
diff --git a/docs/devel/qapi-domain.rst b/docs/devel/qapi-domain.rst
index a748529f51..11238723c2 100644
--- a/docs/devel/qapi-domain.rst
+++ b/docs/devel/qapi-domain.rst
@@ -41,7 +41,7 @@ Schema or generating documentation from code that exists. It 
is merely
 the rST syntax used to describe things. For instance, the Sphinx Python
 domain adds syntax like ``:py:func:`` for describing Python functions in
 documentation, but it's the autodoc module that is responsible for
-reading python code and generating such syntax. QAPI is analagous here:
+reading Python code and generating such syntax. QAPI is analogous here:
 qapidoc.py is responsible for reading the QAPI Schema and generating rST
 syntax, and qapi_domain.py is responsible for translating that special
 syntax and providing APIs for Sphinx internals.
@@ -514,7 +514,7 @@ the definition's "fully qualified name", allowing two 
different
 namespaces to create an otherwise identically named definition.
 
 This directive also influences how reference resolution works for any
-references that do not explicity specify a namespace, so this directive
+references that do not explicitly specify a namespace, so this directive
 can be used to nudge references into preferring targets from within that
 namespace.
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index d09af58c97..e1c196a0c2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2162,7 +2162,7 @@ void memory_region_flush_rom_device(MemoryRegion *mr, 
hwaddr addr, hwaddr size);
  * only useful on RAM regions.
  *
  * @mr: the region being updated.
- * @readonly: whether rhe region is to be ROM or RAM.
+ * @readonly: whether the region is to be ROM or RAM.
  */
 void memory_region_set_readonly(MemoryRegion *mr, bool readonly);
 
@@ -2173,7 +2173,7 @@ void memory_region_set_readonly(MemoryRegion *mr, bool 
readonly);
  * only useful on RAM regions.
  *
  * @mr: the region being updated.
- * @nonvolatile: whether rhe region is to be non-volatile.
+ * @nonvolatile: whether the region is to be non-volatile.
  */
 void memory_region_set_nonvolatile(MemoryRegion *mr, bool nonvolatile);
 
di

[PATCH] Fix objdump output parser in "nsis.py"

2025-04-12 Thread Arthur Sengileyev
In msys2 distribution objdump from gcc is using single tab character
prefix, but objdump from clang is using 4 white space characters instead.
The script will not identify any dll dependencies for a QEMU build
generated with clang. This in turn will fail the build, because there
will be no files inside dlldir and no setup file will be created.
Instead of checking for whitespace in prefix use lstrip to accommodate
for differences in outputs.

Signed-off-by: Arthur Sengileyev 
---
 scripts/nsis.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/nsis.py b/scripts/nsis.py
index af4e064819..8f469634eb 100644
--- a/scripts/nsis.py
+++ b/scripts/nsis.py
@@ -23,7 +23,7 @@ def find_deps(exe_or_dll, search_path, analyzed_deps):
 output = subprocess.check_output(["objdump", "-p", exe_or_dll], text=True)
 output = output.split("\n")
 for line in output:
-if not line.startswith("\tDLL Name: "):
+if not line.lstrip().startswith("DLL Name: "):
 continue
 
 dep = line.split("DLL Name: ")[1].strip()
-- 
2.43.0




Re: [PATCH for-10.0] Fix objdump output parser in "nsis.py"

2025-04-12 Thread Stefan Weil via

Am 12.04.25 um 20:08 schrieb Arthur Sengileyev:


In msys2 distribution objdump from gcc is using single tab character
prefix, but objdump from clang is using 4 white space characters instead.
The script will not identify any dll dependencies for a QEMU build
generated with clang. This in turn will fail the build, because there
will be no files inside dlldir and no setup file will be created.
Instead of checking for whitespace in prefix use lstrip to accommodate
for differences in outputs.

Signed-off-by: Arthur Sengileyev 
---
  scripts/nsis.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/nsis.py b/scripts/nsis.py
index af4e064819..8f469634eb 100644
--- a/scripts/nsis.py
+++ b/scripts/nsis.py
@@ -23,7 +23,7 @@ def find_deps(exe_or_dll, search_path, analyzed_deps):
  output = subprocess.check_output(["objdump", "-p", exe_or_dll], text=True)
  output = output.split("\n")
  for line in output:
-if not line.startswith("\tDLL Name: "):
+if not line.lstrip().startswith("DLL Name: "):
  continue
  
  dep = line.split("DLL Name: ")[1].strip()



Thanks. I use nearly the same code `if not line.strip().startswith("DLL 
Name: "):` in my builds for WoA.


@Stefan, can this trivial patch still be applied for 10.0?

I had planned to replace the whole code with objdump by platform 
independent Python code, but that's a larger change, and I missed the 
deadline.



Reviewed-by: Stefan Weil 





target/mips: Fix MIPS16e translation

2025-04-12 Thread Hauke Mehrtens
Fix a wrong conversion to gen_op_addr_addi(). The framesize should be
added like it was done before.

This bug broke booting OpenWrt MIPS32 BE malta Linux system images
generated by OpenWrt.

Fixes: d0b24b7f50e1 ("target/mips: Use gen_op_addr_addi() when possible")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Hauke Mehrtens 
---
 target/mips/tcg/mips16e_translate.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/tcg/mips16e_translate.c.inc 
b/target/mips/tcg/mips16e_translate.c.inc
index a9af8f1e74..97da3456ea 100644
--- a/target/mips/tcg/mips16e_translate.c.inc
+++ b/target/mips/tcg/mips16e_translate.c.inc
@@ -306,7 +306,7 @@ static void gen_mips16_restore(DisasContext *ctx,
 int astatic;
 TCGv t0 = tcg_temp_new();
 
-gen_op_addr_addi(ctx, t0, cpu_gpr[29], -framesize);
+gen_op_addr_addi(ctx, t0, cpu_gpr[29], framesize);
 
 if (do_ra) {
 decr_and_load(ctx, 31, t0);
@@ -386,7 +386,7 @@ static void gen_mips16_restore(DisasContext *ctx,
 }
 }
 
-gen_op_addr_addi(ctx, cpu_gpr[29], cpu_gpr[29], -framesize);
+gen_op_addr_addi(ctx, cpu_gpr[29], cpu_gpr[29], framesize);
 }
 
 #if defined(TARGET_MIPS64)
-- 
2.49.0




Re: [PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

2025-04-12 Thread Guenter Roeck

On 4/11/25 00:40, Nicholas Piggin wrote:

On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:

According to the XHCI specification, ERSTBA should be written in Low-High
order. The Linux kernel writes the high word first. This results in an
initialization failure.


This should probably be reworded, it's not so much that Linux does it,
this kind of implies a Linux bug. It is that the hardware requires it
and Linux works around such quirk.

   According to the XHCI specification, ERSTBA should be written in Low-High
   order, however some controllers have a quirk that requires the low
   word to be written last.



The following information is found in the Linux kernel commit log.

[Synopsys]- The host controller was design to support ERST setting
during the RUN state. But since there is a limitation in controller
in supporting separate ERSTBA_HI and ERSTBA_LO programming,
It is supported when the ERSTBA is programmed in 64bit,
or in 32 bit mode ERSTBA_HI before ERSTBA_LO

[Synopsys]- The internal initialization of event ring fetches
the "Event Ring Segment Table Entry" based on the indication of
ERSTBA_LO written.


Could you include a reference to the commit in the normal form?

The following information is found in the changelog for Linux kernel
commit sha ("blah").



Add property to support writing the high word first.

Signed-off-by: Guenter Roeck 
---
  hw/usb/hcd-xhci.c | 8 +++-
  hw/usb/hcd-xhci.h | 1 +
  2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 64c3a23b9b..8c0ba569c8 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
  } else {
  intr->erstba_low = val & 0xffc0;
  }
+if (xhci->erstba_hi_lo) {
+xhci_er_reset(xhci, v);
+}
  break;
  case 0x14: /* ERSTBA high */
  intr->erstba_high = val;
-xhci_er_reset(xhci, v);
+if (!xhci->erstba_hi_lo) {
+xhci_er_reset(xhci, v);
+}
  break;
  case 0x18: /* ERDP low */
  if (val & ERDP_EHB) {
@@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
  DEFINE_PROP_UINT32("p3",XHCIState, numports_3, 4),
  DEFINE_PROP_LINK("host",XHCIState, hostOpaque, TYPE_DEVICE,
   DeviceState *),
+DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
  };
  
  static void xhci_class_init(ObjectClass *klass, void *data)

diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 9c3974f148..cf3f074261 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -189,6 +189,7 @@ typedef struct XHCIState {
  uint32_t numports_3;
  uint32_t numintrs;
  uint32_t numslots;
+bool erstba_hi_lo;


Could you use the "quirk" prefix for the device and property name?

With those changes,

Reviewed-by: Nicholas Piggin 

With your patch, if the target does do a 64-bit write to the address,
what happens? I wonder if that's something the device is supposed to


Sorry, I have no means to test this, so I have no idea what happens
in this situation.


cope with but doesn't work or just works by luck today... I would say
that's a separate problem though, if you can get Linux working okay
with this approach.



Linux always writes four bytes at a time, so any change in qemu to explicitly
handle 8-byte writes would not make a difference.

Guenter




Re: [PATCH 1/2] system/main: transfer replay mutex ownership from main thread to main loop thread

2025-04-12 Thread Pierrick Bouvier

On 4/11/25 22:30, Nicholas Piggin wrote:

On Fri Apr 11, 2025 at 8:55 AM AEST, Pierrick Bouvier wrote:

On MacOS, UI event loop has to be ran in the main thread of a process.
Because of that restriction, on this platform, qemu main event loop is
ran on another thread [1].

This breaks record/replay feature, which expects thread running qemu_init
to initialize hold this lock, breaking associated functional tests on
MacOS.

Thus, as a generalization, and similar to how BQL is handled, we release
it after init, and reacquire the lock before entering main event loop,
avoiding a special case if a separate thread is used.

Tested on MacOS with:
$ meson test -C build --setup thorough --print-errorlogs \
func-x86_64-x86_64_replay func-arm-arm_replay func-aarch64-aarch64_replay
$ ./build/qemu-system-x86_64 -nographic -icount 
shift=auto,rr=record,rrfile=replay.log
$ ./build/qemu-system-x86_64 -nographic -icount 
shift=auto,rr=replay,rrfile=replay.log

[1] 
https://gitlab.com/qemu-project/qemu/-/commit/f5ab12caba4f1656479c1feb5248beac1c833243

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2907
Signed-off-by: Pierrick Bouvier 
---
  system/main.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/system/main.c b/system/main.c
index ecb12fd397c..1c022067349 100644
--- a/system/main.c
+++ b/system/main.c
@@ -25,6 +25,7 @@
  #include "qemu/osdep.h"
  #include "qemu-main.h"
  #include "qemu/main-loop.h"
+#include "system/replay.h"
  #include "system/system.h"
  
  #ifdef CONFIG_SDL

@@ -44,10 +45,12 @@ static void *qemu_default_main(void *opaque)
  {
  int status;
  
+replay_mutex_lock();

  bql_lock();
  status = qemu_main_loop();
  qemu_cleanup(status);
  bql_unlock();
+replay_mutex_unlock();
  
  exit(status);

  }
@@ -67,6 +70,7 @@ int main(int argc, char **argv)
  {
  qemu_init(argc, argv);
  bql_unlock();
+replay_mutex_unlock();
  if (qemu_main) {
  QemuThread main_loop_thread;
  qemu_thread_create(&main_loop_thread, "qemu_main",


Do we actually need to hold replay mutex (or even bql) over qemu_init()?
Both should get dropped before we return here. But as a simple fix, I
guess this is okay.



For the bql, I don't know the exact reason.
For replay lock, we need to hold it as clock gets saved as soon as the 
devices are initialized, which happens before end of qemu_init.



Reviewed-by: Nicholas Piggin 





[PATCH for-10.0] docs: Document removal of 64-bit on 32-bit emulation

2025-04-12 Thread Richard Henderson
With acce728cbc6c we disallowed configuring 64-bit guests on
32-bit hosts, but forgot to document that in removed-features.

Signed-off-by: Richard Henderson 
---
 docs/about/removed-features.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 2527a91795..790a5e481c 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -858,6 +858,15 @@ QEMU.  Since all recent x86 hardware from the past >10 
years is
 capable of the 64-bit x86 extensions, a corresponding 64-bit OS should
 be used instead.
 
+32-bit hosts for 64-bit guests (removed in 10.0)
+
+
+In general, 32-bit hosts cannot support the memory space or atomicity
+requirements of 64-bit guests.  Prior to 10.0, QEMU attempted to
+work around the atomicity issues in system mode by running all vCPUs
+in a single thread context; in user mode atomicity was simply broken.
+From 10.0, QEMU has disabled configuration of 64-bit guests on 32-bit hosts.
+
 Guest Emulator ISAs
 ---
 
-- 
2.43.0




Re: [PATCH 0/4] target/arm: fix arm_cpu_get_phys_page_attrs_debug

2025-04-12 Thread Richard Henderson

On 4/10/25 14:00, Pierrick Bouvier wrote:

Pierrick Bouvier (4):
   target/arm/ptw: extract arm_mmu_idx_to_security_space
   target/arm/ptw: get current security_space for current mmu_idx
   target/arm/ptw: extract arm_cpu_get_phys_page
   target/arm/ptw: fix arm_cpu_get_phys_page_attrs_debug


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 07/10] tcg: Add a TCG backend for WebAssembly

2025-04-12 Thread Kohei Tokunaga
Hi Philippe, thank you for the feedback.

> > diff --git a/tcg/tcg.c b/tcg/tcg.c
> > index dfd48b8264..154a4dafa7 100644
> > --- a/tcg/tcg.c
> > +++ b/tcg/tcg.c
> > @@ -136,6 +136,10 @@ static void tcg_out_goto_tb(TCGContext *s, int
which);
> >   static void tcg_out_op(TCGContext *s, TCGOpcode opc, TCGType type,
> >  const TCGArg args[TCG_MAX_OP_ARGS],
> >  const int const_args[TCG_MAX_OP_ARGS]);
> > +#if defined(EMSCRIPTEN)
>
> Maybe we can let this independently of EMSCRIPTEN, to reduce #ifdef'ry.

Sure, I'll make this independent of EMSCRIPTEN in the next version of the
series.

> > +static void tcg_out_label_cb(TCGContext *s, TCGLabel *l);
> > +static int tcg_out_tb_end(TCGContext *s);
> > +#endif
> >   #if TCG_TARGET_MAYBE_vec
> >   static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned
vece,
> >   TCGReg dst, TCGReg src);
> > @@ -251,7 +255,7 @@ TCGv_env tcg_env;
> >   const void *tcg_code_gen_epilogue;
> >   uintptr_t tcg_splitwx_diff;
> >
> > -#ifndef CONFIG_TCG_INTERPRETER
> > +#if !defined(CONFIG_TCG_INTERPRETER) && !defined(EMSCRIPTEN)
>
> s/&&/||/ otherwise breaks TCI? (various cases)

Thank you for pointing this out. Let me break it down with a table to
clarify this logic.

  | !defined(CONFIG_TCG_INTERPRETER) | !defined(EMSCRIPTEN)
| && | || |
--+--+--+++
non-emcc + TCI|  F   |   T
 | F  |  T |
non-emcc + non-TCI|  T   |   T
 | T  |  T |
emcc + TCI|  F   |   F
 | F  |  F |
emcc + wasm   |  T   |   F
 | F  |  T |

If we use "||", this condition becomes always true for non-emscripten
builds, regardless of whether TCI is used. That would break TCI's intended
behaviour. On emscripten builds, both of TCI and wasm backends define
tcg_qemu_tb_exec on their own so the declaration here is unnecessary. This
aligns with the behaviour of "&&", not "||". So unless I'm missing
something, "&&" seems to match the intent?

That said, this condition is a bit complex. Parhaps we shoud introduce a
clearer flag (e.g., HAS_TCG_QEMU_TB_EXEC) in tcg-target.h?

> Out of curiosity, have you tried to run a big-endian guest?

I've just tried a s390x guest by booting Linux. It worked after applying a
small bugfix (unrelated to endianness, shown below). I'll include this fix
in the next verison of the patch series.

> @@ -1718,6 +1718,7 @@ static void tcg_wasm_out_sub2_i64(TCGContext *s,
TCGReg retl, TCGReg reth,
>  tcg_wasm_out_op_local_get(s, TMP64_LOCAL_0_IDX);
>  tcg_wasm_out_op_global_get_r(s, al);
>  tcg_wasm_out_op_i64_gt_u(s);
> +tcg_wasm_out_op_i64_extend_i32_s(s);
>
>  tcg_wasm_out_op_local_get(s, TMP64_LOCAL_0_IDX);
>  tcg_wasm_out_op_global_set_r(s, retl);
> @@ -1727,6 +1728,7 @@ static void tcg_wasm_out_sub2_i64(TCGContext *s,
TCGReg retl, TCGReg reth,
>  tcg_wasm_out_op_global_get_r(s, retl);
>  tcg_wasm_out_op_global_get_r(s, al);
>  tcg_wasm_out_op_i64_gt_u(s);
> +tcg_wasm_out_op_i64_extend_i32_s(s);
>  }


Re: [PATCH 07/10] tcg: Add a TCG backend for WebAssembly

2025-04-12 Thread Kohei Tokunaga
Hi Philippe, let me resend the table as it was corrupted in the last mail.

A: !defined(CONFIG_TCG_INTERPRETER)
B: !defined(EMSCRIPTEN)

  | A | B | && | || |
--+---+---+++
non-emcc + TCI| F | T | F  | T  |
non-emcc + non-TCI| T | T | T  | T  |
emcc + TCI| F | F | F  | F  |
emcc + wasm   | T | F | F  | T  |


Re: [PATCH 08/10] hw/9pfs: Allow using hw/9pfs with emscripten

2025-04-12 Thread Christian Schoenebeck
On Saturday, April 12, 2025 12:21:47 PM CEST Christian Schoenebeck wrote:
> On Saturday, April 12, 2025 10:21:47 AM CEST Christian Schoenebeck wrote:
> > On Friday, April 11, 2025 12:47:29 PM CEST Kohei Tokunaga wrote:
[...]
> Let my answer my own question: I just checked the wasi sources. The errno
> values are hard coded by the wasi API, consistent over systems. So the current
> mapping of this patch is wrong. macOS uses a different mapping than the wasi
> API.
> 
> https://github.com/WebAssembly/wasi-libc/blob/main/libc-bottom-half/headers/public/__errno_values.h
> 
> https://github.com/emscripten-core/emscripten/blob/4af36cf80647f9a82be617a0ff32f3e56f220e41/system/include/wasi/api.h#L116
> 
> So please use a correct mapping as defined in that header file.
> 
> /Christian
> 
> > Alternatively 9p2000.u protocol variant could be used for Emscripten. Not
> > ideal, as this 9p protocol version is somewhat a legacy protocol from QEMU
> > perspective, reduced performance, less reliable, but it transmits error
> > strings to client which it can map to correct errno values by itself. Linux 
> > 9p
> > client uses a hash map for this errno translation of 9p2000.u error strings.

Stupid me. That's host errno -> Linux errno translation. So your values are
obviously correct, sorry!

However still worth comparing the Linux vs. wasi header files on this.

And I would avoid duplicating the macOS translation code. Instead I would just
do a one-line change:

#elif defined(CONFIG_DARWIN) || defined(EMSCRIPTEN)
...

And probably leave a comment with a link to the wasi API header file there, so
in case new errno translations are added for macOS, that people also check
whether those macros exist in the wasi header file as well.

/Christian





Re: [PATCH 08/10] hw/9pfs: Allow using hw/9pfs with emscripten

2025-04-12 Thread Christian Schoenebeck
On Saturday, April 12, 2025 10:21:47 AM CEST Christian Schoenebeck wrote:
> On Friday, April 11, 2025 12:47:29 PM CEST Kohei Tokunaga wrote:
> > Hi Christian,
> > 
> > > > Emscripten's fiber does not support submitting coroutines to other
> > > > threads. So this commit modifies hw/9pfs/coth.h to disable this behavior
> > > > when compiled with Emscripten.
> > >
> > > The lack of being able to dispatch a coroutine to a worker thread is one
> > > thing, however it would probably still make sense to use fibers in 9pfs as
> > > replacement of its coroutines mechanism.
> > >
> > > In 9pfs coroutines are used to dispatch blocking fs I/O syscalls from main
> > > thread to worker thread(s):
> > >
> > > https://wiki.qemu.org/Documentation/9p#Control_Flow
> > >
> > > If you just remove the coroutine code entirely, 9p server might hang for
> > good,
> > > and with it QEMU's main thread.
> > >
> > > By using fibers instead, it would not hang, as it seems as if I/O
> > syscalls are
> > > emulated in Emscripten, right?
> > 
> > Thank you for the feedback. Yes, it would be great if Emscripten's fiber
> > could be used to address this limitation. Since Emscripten's fiber is
> > cooperative, I believe a blocking code_block can still block the 9pfs server
> > unless an explicit yield occurs within it. I'll continue exploring better
> > solutions for this. Please let me know if I'm missing anything.
> 
> As far as I understand it, the I/O syscalls are emulated, and when being
> called by fibers, blocking syscalls would imply to yield under the hood,
> without explicit yield by application that is.
> 
> If that's true, it would only require little code changes for this to work.
> 
> > > Missing
> > >
> > > errno = ENOTSUP;
> > 
> > Sure, I'll fix this in the next version of the series.
> > 
> > > Looks like you just copied the macOS errno translation code. That probably
> > > doesn't make sense.
> > 
> > Errno values differ between Emscripten and Linux, so conversion is required
> > here. I've used the same mappings as macOS for now, but I'm happy to add
> > more conversions if needed.
> 
> OK, but why have you chosen macOS errno mapping exactly? Are you testing on a
> macOS host machine? Are errno values of emscripten machine dependent?
> 
> The errno mapping must be correct, otherwise funny things will happen on guest
> side if 9p2000.L client is used, as it blindly expects errno numbers sent by
> 9p server to be in native Linux errno number presentation.

Let my answer my own question: I just checked the wasi sources. The errno
values are hard coded by the wasi API, consistent over systems. So the current
mapping of this patch is wrong. macOS uses a different mapping than the wasi
API.

https://github.com/WebAssembly/wasi-libc/blob/main/libc-bottom-half/headers/public/__errno_values.h

https://github.com/emscripten-core/emscripten/blob/4af36cf80647f9a82be617a0ff32f3e56f220e41/system/include/wasi/api.h#L116

So please use a correct mapping as defined in that header file.

/Christian

> Alternatively 9p2000.u protocol variant could be used for Emscripten. Not
> ideal, as this 9p protocol version is somewhat a legacy protocol from QEMU
> perspective, reduced performance, less reliable, but it transmits error
> strings to client which it can map to correct errno values by itself. Linux 9p
> client uses a hash map for this errno translation of 9p2000.u error strings.
> 
> /Christian





Re: [PATCH 08/10] hw/9pfs: Allow using hw/9pfs with emscripten

2025-04-12 Thread Christian Schoenebeck
On Friday, April 11, 2025 12:47:29 PM CEST Kohei Tokunaga wrote:
> Hi Christian,
> 
> > > Emscripten's fiber does not support submitting coroutines to other
> > > threads. So this commit modifies hw/9pfs/coth.h to disable this behavior
> > > when compiled with Emscripten.
> >
> > The lack of being able to dispatch a coroutine to a worker thread is one
> > thing, however it would probably still make sense to use fibers in 9pfs as
> > replacement of its coroutines mechanism.
> >
> > In 9pfs coroutines are used to dispatch blocking fs I/O syscalls from main
> > thread to worker thread(s):
> >
> > https://wiki.qemu.org/Documentation/9p#Control_Flow
> >
> > If you just remove the coroutine code entirely, 9p server might hang for
> good,
> > and with it QEMU's main thread.
> >
> > By using fibers instead, it would not hang, as it seems as if I/O
> syscalls are
> > emulated in Emscripten, right?
> 
> Thank you for the feedback. Yes, it would be great if Emscripten's fiber
> could be used to address this limitation. Since Emscripten's fiber is
> cooperative, I believe a blocking code_block can still block the 9pfs server
> unless an explicit yield occurs within it. I'll continue exploring better
> solutions for this. Please let me know if I'm missing anything.

As far as I understand it, the I/O syscalls are emulated, and when being
called by fibers, blocking syscalls would imply to yield under the hood,
without explicit yield by application that is.

If that's true, it would only require little code changes for this to work.

> > Missing
> >
> > errno = ENOTSUP;
> 
> Sure, I'll fix this in the next version of the series.
> 
> > Looks like you just copied the macOS errno translation code. That probably
> > doesn't make sense.
> 
> Errno values differ between Emscripten and Linux, so conversion is required
> here. I've used the same mappings as macOS for now, but I'm happy to add
> more conversions if needed.

OK, but why have you chosen macOS errno mapping exactly? Are you testing on a
macOS host machine? Are errno values of emscripten machine dependent?

The errno mapping must be correct, otherwise funny things will happen on guest
side if 9p2000.L client is used, as it blindly expects errno numbers sent by
9p server to be in native Linux errno number presentation.

Alternatively 9p2000.u protocol variant could be used for Emscripten. Not
ideal, as this 9p protocol version is somewhat a legacy protocol from QEMU
perspective, reduced performance, less reliable, but it transmits error
strings to client which it can map to correct errno values by itself. Linux 9p
client uses a hash map for this errno translation of 9p2000.u error strings.

/Christian