Re: [PATCH] target/mips: fetch code with translator_ld

[Philippe Mathieu-Daudé]

On 1/18/21 6:40 PM, Richard Henderson wrote:
> On 1/16/21 8:13 AM, Philippe Mathieu-Daudé wrote:
>> +++ b/target/mips/tlb_helper.c
>> @@ -21,7 +21,7 @@
>>  #include "cpu.h"
>>  #include "internal.h"
>>  #include "exec/exec-all.h"
>> -#include "exec/cpu_ldst.h"
>> +#include "exec/translator.h"
>>  #include "exec/log.h"
>>  #include "hw/mips/cpudevs.h"
>>  
>> @@ -526,9 +526,9 @@ static bool get_pte(CPUMIPSState *env, uint64_t vaddr, 
>> int entry_size,
>>  return false;
>>  }
>>  if (entry_size == 64) {
>> -*pte = cpu_ldq_code(env, vaddr);
>> +*pte = translator_ldq(env, vaddr);
>>  } else {
>> -*pte = cpu_ldl_code(env, vaddr);
>> +*pte = translator_ldl(env, vaddr);
>>  }
>>  return true;
>>  }
> 
> NACK.  This is not within the translator.

Oops...

Thanks for catching this mistake,

Phil.

RE: [PATCH v7 15/35] Hexagon (target/hexagon/arch.[ch]) utility functions

[Taylor Simpson]

> -Original Message-
> From: Philippe Mathieu-Daudé  On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Friday, January 22, 2021 12:09 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; alex.ben...@linaro.org;
> laur...@vivier.eu; a...@rev.ng; Brian Cain 
> Subject: Re: [PATCH v7 15/35] Hexagon (target/hexagon/arch.[ch]) utility
> functions
>
> Hi Taylor,
>
> On 1/20/21 4:28 AM, Taylor Simpson wrote:
> > Signed-off-by: Taylor Simpson 
> > ---
> >  target/hexagon/arch.h |  35 ++
> >  target/hexagon/arch.c | 294
> ++
> >  2 files changed, 329 insertions(+)
> >  create mode 100644 target/hexagon/arch.h
> >  create mode 100644 target/hexagon/arch.c
> >
> > diff --git a/target/hexagon/arch.h b/target/hexagon/arch.h
> > new file mode 100644
> > index 000..a8374a3
> > --- /dev/null
> > +++ b/target/hexagon/arch.h
>
> Maybe rename "arch_utils.[ch]"?

Any particular reason?

>
> > +extern int arch_sf_invsqrt_common(float32 *Rs, float32 *Rd, int *adjust,
> > +  float_status *fp_status);
>
> (Again, no need for 'extern').

OK, I will change these.

> > diff --git a/target/hexagon/arch.c b/target/hexagon/arch.c
> > new file mode 100644
> > index 000..c59cad5
> > --- /dev/null
> > +++ b/target/hexagon/arch.c
> ...
>
> > +#define RAISE_FP_EXCEPTION \
> > +do {} while (0)/* Not modelled in qemu user mode */
>
> I don't understand why... Can you explain please?

Our Linux kernel only sets the relevant bits in USR (user status register).  
The exception isn't raised to user mode.

Re: [PATCH v7 04/11] slirp: feature detection for smbd

[Peter Maydell]

On Fri, 22 Jan 2021 at 20:16, Joelle van Dyne  wrote:
>
> Replace Windows specific macro with a more generic feature detection
> macro. Allows slirp smb feature to be disabled manually as well.
>
> Signed-off-by: Joelle van Dyne 
> ---


> +if test "$slirp_smbd" = "yes" ; then
> +  echo "CONFIG_SLIRP_SMBD=y" >> $config_host_mak
> +  echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
> +fi

This sets "CONFIG_SLIRP_SMBD" and "CONFIG_SMBD_COMMAND"...

>  if test "$vde" = "yes" ; then
>echo "CONFIG_VDE=y" >> $config_host_mak
>echo "VDE_LIBS=$vde_libs" >> $config_host_mak
> diff --git a/meson.build b/meson.build
> index 6c3ee7f8ca..9577138d7f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2331,7 +2331,7 @@ summary_info += {'sphinx-build':  
> sphinx_build.found()}
>  summary_info += {'genisoimage':   config_host['GENISOIMAGE']}
>  # TODO: add back version
>  summary_info += {'slirp support': slirp_opt == 'disabled' ? false : 
> slirp_opt}
> -if slirp_opt != 'disabled'
> +if slirp_opt != 'disabled' and 'HAVE_HOST_SMBD' in config_host

...but this is looking for "HAVE_HOST_SMBD". Should it be something else?

>summary_info += {'smbd':config_host['CONFIG_SMBD_COMMAND']}
>  endif
>  summary_info += {'module support':config_host.has_key('CONFIG_MODULES')}

thanks
-- PMM

Re: [PATCH v7 05/11] osdep: build with non-working system() function

[Peter Maydell]

On Fri, 22 Jan 2021 at 20:13, Joelle van Dyne  wrote:
>
> Build without error on hosts without a working system(). An assertion
> will trigger if system() is called.
>
> Signed-off-by: Joelle van Dyne 

 configure| 19 +++

Can we do the "does system() exist?" check in meson.build ?
Untested, but looking at the existing check for "does gettid() exist?"
it should be two lines:

has_system = cc.has_function('system')

and then later:

config_host_data.set('HAVE_SYSTEM_FUNCTION', has_system)

> +/**
> + * Platforms which do not support system() gets an assertion failure.
> + */
> +#ifndef HAVE_SYSTEM_FUNCTION
> +#define system platform_does_not_support_system
> +static inline int platform_does_not_support_system(const char *command)
> +{
> +assert(0);
> +}
> +#endif /* !HAVE_SYSTEM_FUNCTION */

I think we should make this return an error code rather than assert:

errno = ENOSYS;
return -1;

In particular, the arm, m68k and nios2 semihosting ABIs presented
to the guest include 'SYSTEM' semihosting calls which we implement
as "call system() with the string the guest hands us". On a
platform without a system() function we want to return an
error to the guest there, not assert.

The other possible approach would be to find all the places
which want to call system() and add suitable ifdeffery to handle
platforms without system:
 * a win32-specific part of the guest-agent (no action needed)
 * net/slirp.c (already handled by the smbd patch in this series)
 * code in tests/ (5 instances)
 * the 3 semihosting uses

But I think providing an always-fails system() is fine.

thanks
-- PMM

Re: [PATCH 1/1] os_find_datadir: search as in version 4.2

[Brian Norris]

Just to follow-up here, since nobody followed up for months...

On Mon, Aug 10, 2020 at 2:41 PM Brian Norris  wrote:
> On Mon, Aug 10, 2020 at 12:29 AM Marc-André Lureau
>  wrote:
> > On Sat, Aug 8, 2020 at 7:34 PM Peter Maydell  
> > wrote:
> > > On Sat, 8 Aug 2020 at 02:35, Brian Norris  
> > > wrote:
> > > It's just missed 5.1, unfortunately :-(

And it missed 5.2 too :(

> > > Marc-André, did you want to review it ?
> >
> > I tried an alternative approach, and ack his version instead:
> >
> > https://patchew.org/QEMU/20200716141100.398296-1-marcandre.lur...@redhat.com/
> >
> > (I am going to do this in this thread instead)
>
> FWIW, you already provided your Review a month ago:
> https://lore.kernel.org/qemu-devel/caj+f1cjmmv6py6r0p6uknza_q+w6ylvaxekgnusgxyuuip6...@mail.gmail.com/
>
> But I see you've now repeated it ;)
> https://lore.kernel.org/qemu-devel/caj+f1cjdho7r9rnmod1clezsylfsfvvjcar5ucsyfgfcw3z...@mail.gmail.com/
>
> In any case, thanks! We'll likely carry this patch in Chrome OS, until
> it gets applied to a proper release.

It turns out that Paolo inadvertently (?) fixed this issue by
refactoring, in v5.2.0:
ea1edcd7da1a vl: relocate paths to data directories
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=ea1edcd7da1a375ef7ccf8aa93b72827b518ad8e;hp=63c4db4c2e6d221cecb5aafa365934bb05724cb4

I tested that out here, and the new find_datadir() is able to track
relocations properly, by looking for a common directory ancestor of
the running executable. Thanks Paolo!

Brian

[PATCH v8 11/11] darwin: remove 64-bit build detection on 32-bit OS

[Joelle van Dyne]

A workaround added in early days of 64-bit OSX forced x86_64 if the
host machine had 64-bit support. This creates issues when cross-
compiling for ARM64. Additionally, the user can always use --cpu=* to
manually set the host CPU and therefore this workaround should be
removed.

Reviewed-by: Peter Maydell 
Signed-off-by: Joelle van Dyne 
---
 configure | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/configure b/configure
index 70061e195d..5f23f5b907 100755
--- a/configure
+++ b/configure
@@ -626,13 +626,6 @@ fi
 # the correct CPU with the --cpu option.
 case $targetos in
 Darwin)
-  # on Leopard most of the system is 32-bit, so we have to ask the kernel if 
we can
-  # run 64-bit userspace code.
-  # If the user didn't specify a CPU explicitly and the kernel says this is
-  # 64 bit hw, then assume x86_64. Otherwise fall through to the usual 
detection code.
-  if test -z "$cpu" && test "$(sysctl -n hw.optional.x86_64)" = "1"; then
-cpu="x86_64"
-  fi
   HOST_DSOSUF=".dylib"
   ;;
 SunOS)
@@ -776,10 +769,6 @@ OpenBSD)
 Darwin)
   bsd="yes"
   darwin="yes"
-  if [ "$cpu" = "x86_64" ] ; then
-QEMU_CFLAGS="-arch x86_64 $QEMU_CFLAGS"
-QEMU_LDFLAGS="-arch x86_64 $QEMU_LDFLAGS"
-  fi
   audio_drv_list="try-coreaudio try-sdl"
   audio_possible_drivers="coreaudio sdl"
   # Disable attempts to use ObjectiveC features in os/object.h since they
-- 
2.28.0

Re: [PATCH v7 34/35] Hexagon build infrastructure

[Philippe Mathieu-Daudé]

On 1/22/21 11:41 PM, Philippe Mathieu-Daudé wrote:
> On 1/22/21 11:34 PM, Philippe Mathieu-Daudé wrote:
>> On 1/20/21 4:29 AM, Taylor Simpson wrote:
>>> Add file to default-configs
>>> Add hexagon to meson.build
>>> Add hexagon to target/meson.build
>>> Add target/hexagon/meson.build
>>> Change scripts/qemu-binfmt-conf.sh
>>>
>>> We can build a hexagon-linux-user target and run programs on the Hexagon
>>> scalar core.  With hexagon-linux-clang installed, "make check-tcg" will
>>> pass.
>>>
>>> Signed-off-by: Taylor Simpson 
>>> ---
>>>  default-configs/targets/hexagon-linux-user.mak |   1 +
>>>  meson.build|   1 +
>>>  scripts/qemu-binfmt-conf.sh|   6 +-
>>>  target/hexagon/meson.build | 187 
>>> +
>>>  target/meson.build |   1 +
>>>  5 files changed, 195 insertions(+), 1 deletion(-)
>>>  create mode 100644 default-configs/targets/hexagon-linux-user.mak
>>>  create mode 100644 target/hexagon/meson.build
>> ...
> 
> BTW you should test your branch on gitlab-ci, I'm pretty sure
> various jobs fail.

Forgot to paste this link:
https://wiki.qemu.org/Testing/CI/GitLabCI

[Bug 1912857] Re: virtio-serial blocks hostfwd ssh on windows 10 host

[Ven Karri]

** Description changed:

- qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
- user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1 -->
- WORKS - meaning I can ssh into the vm via port 
+ qemu-system-x86_64 
+   -display none 
+   -hda archlinux.qcow2 
+   -m 4G 
+   -netdev user,id=n1,hostfwd=tcp::-:22 
+   -device virtio-net-pci,netdev=n1 
  
- qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
- user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
- -device virtio-serial -device virtserialport,chardev=cid0 -chardev
- socket,id=cid0,host:localhost,port:55298,server,nowait --> DOES NOT WORK
- - meaning I cannot ssh into the vm
+ --> THIS WORKS - meaning I can ssh into the vm via port 
+ 
+ qemu-system-x86_64 
+   -display none 
+   -hda archlinux.qcow2 
+   -m 4G 
+   -netdev user,id=n1,hostfwd=tcp::-:22 
+   -device virtio-net-pci,netdev=n1 
+   -device virtio-serial 
+   -device virtserialport,chardev=cid0 
+   -chardev socket,id=cid0,host:localhost,port:55298,server,nowait 
+ 
+ --> DOES NOT WORK - meaning I cannot ssh into the vm
  
  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.
  
  The following doesn't work either:
  
- qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
- user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
- -device virtio-serial -device virtserialport,chardev=cid0 -chardev
- file,id=cid0,path=temp,server,nowait
+ qemu-system-x86_64 
+   -display none 
+   -hda archlinux.qcow2 
+   -m 4G 
+   -netdev user,id=n1,hostfwd=tcp::-:22 
+   -device virtio-net-pci,netdev=n1 
+   -device virtio-serial -device virtserialport,chardev=cid0 
+   -chardev file,id=cid0,path=temp,server,nowait
  
  No matter which character device I use for my virtserialport
  communication (socket or udp or file or pipe), the hostfwd doesn't work.
  
  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

** Description changed:

- qemu-system-x86_64 
-   -display none 
-   -hda archlinux.qcow2 
-   -m 4G 
-   -netdev user,id=n1,hostfwd=tcp::-:22 
-   -device virtio-net-pci,netdev=n1 
+ qemu-system-x86_64
+   -display none
+   -hda archlinux.qcow2
+   -m 4G
+   -netdev user,id=n1,hostfwd=tcp::-:22
+   -device virtio-net-pci,netdev=n1
  
  --> THIS WORKS - meaning I can ssh into the vm via port 
  
- qemu-system-x86_64 
-   -display none 
-   -hda archlinux.qcow2 
-   -m 4G 
-   -netdev user,id=n1,hostfwd=tcp::-:22 
-   -device virtio-net-pci,netdev=n1 
-   -device virtio-serial 
-   -device virtserialport,chardev=cid0 
-   -chardev socket,id=cid0,host:localhost,port:55298,server,nowait 
+ qemu-system-x86_64
+   -display none
+   -hda archlinux.qcow2
+   -m 4G
+   -netdev user,id=n1,hostfwd=tcp::-:22
+   -device virtio-net-pci,netdev=n1
+   -device virtio-serial
+   -device virtserialport,chardev=cid0
+   -chardev socket,id=cid0,host=localhost,port=55298,server,nowait
  
  --> DOES NOT WORK - meaning I cannot ssh into the vm
  
  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.
  
  The following doesn't work either:
  
- qemu-system-x86_64 
-   -display none 
-   -hda archlinux.qcow2 
-   -m 4G 
-   -netdev user,id=n1,hostfwd=tcp::-:22 
-   -device virtio-net-pci,netdev=n1 
-   -device virtio-serial -device virtserialport,chardev=cid0 
-   -chardev file,id=cid0,path=temp,server,nowait
+ qemu-system-x86_64
+   -display none
+   -hda archlinux.qcow2
+   -m 4G
+   -netdev user,id=n1,hostfwd=tcp::-:22
+   -device virtio-net-pci,netdev=n1
+   -device virtio-serial -device virtserialport,chardev=cid0
+   -chardev file,id=cid0,path=temp,server,nowait
  
  No matter which character device I use for my virtserialport
  communication (socket or udp or file or pipe), the hostfwd doesn't work.
  
  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

** Description changed:

  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
  
  --> THIS WORKS - meaning I can ssh into the vm via port 
  
  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
    -device virtio-serial
    -device virtserialport,chardev=cid0
    -chardev socket,id=cid0,host=localhost,port=55298,server,nowait
  
  --> DOES NOT WORK - meaning I cannot ssh into the vm
  
  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.
  
  The following doesn't work either:
  
  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
-   -device virtio-serial -device virtserialport,chardev=cid0
+   -device virtio-serial 
+   -device 

Re: [PATCH v2 1/3] target/arm: Remove PSTATE_SS from cpsr and move it into env->pstate.

[Rebecca Cran]

On 1/22/21 2:03 PM, Richard Henderson wrote:

On 1/21/21 6:45 PM, Rebecca Cran wrote:



  cpsr_write(env, spsr, mask, CPSRWriteRaw);
-if (!arm_singlestep_active(env)) {
-env->uncached_cpsr &= ~PSTATE_SS;
-}
+env->pstate &= ~PSTATE_SS;


Why are you removing the singlestep check?



-env->uncached_cpsr &= ~PSTATE_SS;
-env->spsr = cpsr_read(env);
+env->pstate &= ~PSTATE_SS;
+env->spsr &= ~PSTATE_SS;


This loses the saving of cpsr into spsr.


Oh, right. I've fixed both this and the above issue in the next revision 
which I'll send out early next week (giving a chance for any extra 
feedback).


Thanks.
--
Rebecca Cran

Re: [PATCH v2 1/1] spapr_caps.c: check user input before warning about TCG only caps

[David Gibson]

On Wed, Jan 20, 2021 at 07:54:06AM -0300, Daniel Henrique Barboza wrote:
> Commit 006e9d361869 added warning messages for cap-cfpc, cap-ibs and
> cap-sbbc when enabled under TCG. Commit 8ff43ee404d3 did the same thing
> when introducing cap-ccf-assist.
> 
> These warning messages, although benign to the machine launch, can make
> users a bit confused. E.g:
> 
> $ sudo ./ppc64-softmmu/qemu-system-ppc64
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-cfpc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-sbbc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-ibs=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-ccf-assist=on
> 
> We're complaining about "TCG doesn't support requested feature" when the
> user didn't request any of those caps in the command line.
> 
> Check if these caps were set in the command line before sending an user
> warning.
> 
> Signed-off-by: Daniel Henrique Barboza 

Oof.  I have real mixed feelings about this.

So, yes, the warnings are annoying, but they're not meaningless.  They
are really indicating that the guest environment is different from the
one you requested (implicitly, via the machine version). The fact that
they are only warnings, not hard errors, is already a compromise
because otherwise there would be no real way to use TCG at all with
current machines.

In short, the warnings are scary because they're *meant* to be scary.
TCG will not, and cannot, supply the Spectre mitigations that are
expected on a current machine type.

I agree that the current behaviour is pretty irritating, but I don't
know that silently pretending TCG can do what's normally expected of
that command line is a great option either.


> ---
>  hw/ppc/spapr_caps.c | 47 ++---
>  1 file changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9341e9782a..629c24a96d 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -244,9 +244,15 @@ static void cap_safe_cache_apply(SpaprMachineState 
> *spapr, uint8_t val,
>  uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
>  
>  if (tcg_enabled() && val) {
> -/* TCG only supports broken, allow other values and print a warning 
> */
> -warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> -cap_cfpc_possible.vals[val]);
> +/*
> + * TCG only supports broken, allow other values and print a warning
> + * in case the user attempted to set a different value in the command
> + * line.
> + */
> +if (spapr->cmd_line_caps[SPAPR_CAP_CFPC] != SPAPR_CAP_BROKEN) {
> +warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> +cap_cfpc_possible.vals[val]);
> +}
>  } else if (kvm_enabled() && (val > kvm_val)) {
>  error_setg(errp,
> "Requested safe cache capability level not supported by 
> KVM");
> @@ -269,9 +275,15 @@ static void 
> cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
>  uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
>  
>  if (tcg_enabled() && val) {
> -/* TCG only supports broken, allow other values and print a warning 
> */
> -warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> -cap_sbbc_possible.vals[val]);
> +/*
> + * TCG only supports broken, allow other values and print a warning
> + * in case the user attempted to set a different value in the command
> + * line.
> + */
> +if (spapr->cmd_line_caps[SPAPR_CAP_SBBC] != SPAPR_CAP_BROKEN) {
> +warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> +cap_sbbc_possible.vals[val]);
> +}
>  } else if (kvm_enabled() && (val > kvm_val)) {
>  error_setg(errp,
>  "Requested safe bounds check capability level not supported by KVM");
> @@ -297,9 +309,15 @@ static void 
> cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
>  uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
>  
>  if (tcg_enabled() && val) {
> -/* TCG only supports broken, allow other values and print a warning 
> */
> -warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> -cap_ibs_possible.vals[val]);
> +/*
> + * TCG only supports broken, allow other values and print a warning
> + * in case the user attempted to set a different value in the command
> + * line.
> + */
> +if (spapr->cmd_line_caps[SPAPR_CAP_IBS] != SPAPR_CAP_BROKEN) {
> +warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> +cap_ibs_possible.vals[val]);
> +}
>  } else if (kvm_enabled() && 

Re: [PATCH v7 11/11] darwin: remove 64-bit build detection on 32-bit OS

[Peter Maydell]

On Fri, 22 Jan 2021 at 20:16, Joelle van Dyne  wrote:
>
> A workaround added in early days of 64-bit OSX forced x86_64 if the
> host machine had 64-bit support. This creates issues when cross-
> compiling for ARM64. Additionally, the user can always use --cpu=* to
> manually set the host CPU and therefore this workaround should be
> removed.
>
> Signed-off-by: Joelle van Dyne 
> ---
>  configure | 11 ---
>  1 file changed, 11 deletions(-)
>
> diff --git a/configure b/configure
> index fb671258e6..c7fbda22b9 100755
> --- a/configure
> +++ b/configure
> @@ -626,13 +626,6 @@ fi
>  # the correct CPU with the --cpu option.
>  case $targetos in
>  Darwin)
> -  # on Leopard most of the system is 32-bit, so we have to ask the kernel if 
> we can
> -  # run 64-bit userspace code.
> -  # If the user didn't specify a CPU explicitly and the kernel says this is
> -  # 64 bit hw, then assume x86_64. Otherwise fall through to the usual 
> detection code.
> -  if test -z "$cpu" && test "$(sysctl -n hw.optional.x86_64)" = "1"; then
> -cpu="x86_64"
> -  fi
>HOST_DSOSUF=".dylib"
>;;
>  SunOS)

I was just thinking the other day that we could remove this hack...

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Bug 1912857] Re: virtio-serial blocks hostfwd ssh on windows 10 host

[Ven Karri]

** Description changed:

  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
  
  --> THIS WORKS - meaning I can ssh into the vm via port 
  
  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
    -device virtio-serial
    -device virtserialport,chardev=cid0
    -chardev socket,id=cid0,host=localhost,port=55298,server,nowait
  
  --> DOES NOT WORK - meaning I cannot ssh into the vm
  
  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.
  
  The following doesn't work either:
  
  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
    -device virtio-serial
    -device virtserialport,chardev=cid0
    -chardev file,id=cid0,path=mypath
  
  No matter which character device I use for my virtserialport
  communication (socket or udp or file or pipe), the hostfwd doesn't work.
  
  Also, if I enable the display, I am unable to type anything in the
  emulator window when I use virtserialport.
  
  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2
+ 
+ The same thing works just fine on a Mac OS X host (tested on Big Sur)

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1912857

Title:
  virtio-serial blocks hostfwd ssh on windows 10 host

Status in QEMU:
  New

Bug description:
  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1

  --> THIS WORKS - meaning I can ssh into the vm via port 

  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
    -device virtio-serial
    -device virtserialport,chardev=cid0
    -chardev socket,id=cid0,host=localhost,port=55298,server,nowait

  --> DOES NOT WORK - meaning I cannot ssh into the vm

  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.

  The following doesn't work either:

  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
    -device virtio-serial
    -device virtserialport,chardev=cid0
    -chardev file,id=cid0,path=mypath

  No matter which character device I use for my virtserialport
  communication (socket or udp or file or pipe), the hostfwd doesn't
  work.

  Also, if I enable the display, I am unable to type anything in the
  emulator window when I use virtserialport.

  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

  The same thing works just fine on a Mac OS X host (tested on Big Sur)

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1912857/+subscriptions

Re: [PATCH] coroutine-sigaltstack: Keep SIGUSR2 handler up

[Laszlo Ersek]

On 01/22/21 22:26, Laszlo Ersek wrote:

> I'm drifting towards an overhaul of coroutine-sigaltstack, based on my
> personal understanding of POSIX, but given that I can absolutely not
> *test* coroutine-sigaltstack on the platforms where it actually matters,
> an "overhaul" by me would be reckless.
> 
> I didn't expect these skeletons when I first read Max's "Thread safety
> of coroutine-sigaltstack" email :/
> 
> Max, after having worked on top of your patch for a few hours, I
> officially endorse your mutex approach. I can't encourage you or myself
> to touch this code, in good conscience. It's not that it's "bad"; it's
> inexplicable and (to me) untestable.

I'm attaching a patch (based on 0e3246263068). I'm not convinced that I
should take responsibility for this, given the lack of testability on my
end. So I'm not posting it stand-alone even as an RFC. I've built it and
have booted one of my existent domains with it, but that's all.

Thanks
Laszlo
>From c6c05052961e6066d36f5c7c6e32d36ea9f17dff Mon Sep 17 00:00:00 2001
From: Laszlo Ersek 
Date: Fri, 22 Jan 2021 11:20:41 +0100
Subject: [PATCH] coroutine-sigaltstack: overhaul SIGUSR2 treatment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

(1) Disposition (action) for any given signal is global for the process.
When two threads run coroutine-sigaltstack's qemu_coroutine_new()
concurrently, they may interfere with each other: one of them may
revert the SIGUSR2 handler to SIG_DFL, between the other thread (a)
setting up coroutine_trampoline() as the handler and (b) raising
SIGUSR2. That SIGUSR2 will then terminate the QEMU process abnormally.

Outside of coroutine-sigaltstack, qemu does not use SIGUSR2 [*]. So
move the pthread_sigmask() and sigaction() calls from
qemu_coroutine_new() to coroutine_init(). This will keep the handler
installed all the time, while also ensuring that all threads block
SIGUSR2 all the time.

[*] In user-mode emulation, the guest can register signal handlers for
any signal but SIGSEGV and SIGBUS, so if it registers a SIGUSR2
handler, that will interfere with coroutine-sigaltstack. However,
we do not use coroutines for user-mode emulation, so that is fine.

(2) The temporary unblocking of SIGUSR2 in qemu_coroutine_new() with
sigsuspend(), which implements the synchronous delivery of SIGUSR2 to
the thread, is needlessly complicated. Remove the "tr_called"-based
loop around sigsuspend(), as by the time we reach sigsuspend(),
SIGUSR2 is certainly pending.

(3) Relatedly, the top of the signal handler can only be entered via the
sigsuspend() in qemu_coroutine_new(). Express this fact in the signal
handler by abort()ing on (tr_handler==NULL).

First, even if another process sends a SIGUSR2 to the QEMU process
asynchronously, SIGUSR2 will only be unblocked by sigsuspend() in the
next qemu_coroutine_new() execution, and by that time, the thread in
question will have raised SIGUSR2 anyway.

Second, there is no reason for sigsuspend() *not* to be both a
compiler barrier and a memory barrier.

(4) Finally, the "tr_handler" field should be more strongly typed; it only
ever points to a CoroutineSigAltStack object.

Based on Max's original patch.

Cc: "Daniel P. Berrangé" 
Cc: Eric Blake 
Cc: Kevin Wolf 
Cc: Markus Armbruster 
Cc: Max Reitz 
Cc: Paolo Bonzini 
Cc: Peter Maydell 
Cc: Stefan Hajnoczi 
Signed-off-by: Max Reitz 
Signed-off-by: Laszlo Ersek 
---
 util/coroutine-sigaltstack.c | 89 +---
 1 file changed, 52 insertions(+), 37 deletions(-)

diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index aade82afb8c0..a59513367532 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -44,21 +44,22 @@ typedef struct {
 /**
  * Per-thread coroutine bookkeeping
  */
 typedef struct {
 /** Currently executing coroutine */
 Coroutine *current;
 
 /** The default coroutine */
 CoroutineSigAltStack leader;
 
 /** Information for the signal handler (trampoline) */
 sigjmp_buf tr_reenter;
-volatile sig_atomic_t tr_called;
-void *tr_handler;
+CoroutineSigAltStack *tr_handler;
 } CoroutineThreadState;
 
 static pthread_key_t thread_state_key;
 
+static void coroutine_trampoline(int signal);
+
 static CoroutineThreadState *coroutine_get_thread_state(void)
 {
 CoroutineThreadState *s = pthread_getspecific(thread_state_key);
@@ -81,16 +82,51 @@ static void qemu_coroutine_thread_cleanup(void *opaque)
 static void __attribute__((constructor)) coroutine_init(void)
 {
 int ret;
+sigset_t sigs;
+struct sigaction sa;
 
 ret = pthread_key_create(_state_key, qemu_coroutine_thread_cleanup);
 if (ret != 0) {
 fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
 abort();
 }
+
+/*
+ * This constructor function is running in 

[Bug 1800156] Re: windows 8.1 loose grab/leave window on windowed

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1800156

Title:
  windows 8.1 loose grab/leave window on windowed

Status in QEMU:
  Expired

Bug description:
  Hello, i am new to QEMU and i encounter that annoying issue (windowed)
  when i move the mouse a bit too much then it leave the window.

  Windows 8.1, Latest QEMU (Windows binaries).

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1800156/+subscriptions

Re: [PATCH v7 06/11] darwin: remove redundant dependency declaration

[Peter Maydell]

On Fri, 22 Jan 2021 at 20:20, Joelle van Dyne  wrote:
>
> Meson will find CoreFoundation, IOKit, and Cocoa as needed.
>
> Signed-off-by: Joelle van Dyne 
> ---
>  configure | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/configure b/configure
> index 82ce28c660..4c485dd962 100755
> --- a/configure
> +++ b/configure
> @@ -781,7 +781,6 @@ Darwin)
>fi
>audio_drv_list="coreaudio try-sdl"
>audio_possible_drivers="coreaudio sdl"
> -  QEMU_LDFLAGS="-framework CoreFoundation -framework IOKit $QEMU_LDFLAGS"
># Disable attempts to use ObjectiveC features in os/object.h since they
># won't work when we're compiling with gcc as a C compiler.
>QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
> --

Reviewed-by: Peter Maydell 

I notice that configure also sets
  coreaudio_libs="-framework CoreAudio"
but that looks like it's something that hasn't yet been moved
into meson.build, so we can't remove it yet.

thanks
-- PMM

Re: Thread safety of coroutine-sigaltstack

[Laszlo Ersek]

On 01/22/21 11:14, Peter Maydell wrote:
> On Fri, 22 Jan 2021 at 08:50, Max Reitz  wrote:
>>
>> On 20.01.21 18:25, Laszlo Ersek wrote:
>>
>> [...]
>>
>>> A simple grep for SIGUSR2 seems to indicate that SIGUSR2 is not used by
>>> system emulation for anything else, in practice. Is it possible to
>>> dedicate SIGUSR2 explicitly to coroutine-sigaltstack, and set up the
>>> action beforehand, from some init function that executes on a "central"
>>> thread, before qemu_coroutine_new() is ever called?
>>
>> I wrote a patch to that effect, but just before sending I wondered
>> whether SIGUSR2 cannot be registered by the “guest” in user-mode
>> emulation, and whether that would then break coroutines from there on.
>>
>> (I have no experience dealing with user-mode emulation, but it does look
>> like the guest can just register handlers for any signal but SIGSEGV and
>> SIGBUS.)
> 
> Yes, SIGUSR2 is for the guest in user-emulation mode. OTOH do we
> even use the coroutine code in user-emulation mode? Looking at
> the meson.build files, we only add the coroutine_*.c to util_ss
> if 'have_block', and we set have_block = have_system or have_tools.
> I think (but have not checked) that that means we will build and
> link the object file into the user-mode binaries if you happen
> to build them in the same run as system-mode binaries,

I did that, first running

 ./configure \
--enable-debug \
--target-list==x86_64-softmmu,x86_64-linux-user \
--with-coroutine=sigaltstack

Then I checked the "qemu-system-x86_64" and "qemu-x86_64" binaries with
"nm". Only the former contains "coroutine_init":

009725e4 t coroutine_init

So I believe the coroutine object file(s) are not even linked into the
user-mode emulators. (coroutine_init() is a constructor function, so I
think it would be preserved otherwise, even if it had no explicit caller.)

I tried a different approach too: an #error in
"coroutine-sigaltstack.c", if CONFIG_LINUX_USER were #defined. But that
aborted the build, due to CONFIG_LINUX_USER being poisoned in the first
place. Maybe that result was already enough to answer the question, but
I wasn't sure, hence the check with "nm".

Thanks,
Laszlo

> but won't
> build them in if you built the user-mode binaries as a separate
> build. Which is odd and probably worth fixing, but does mean we
> know that we aren't actually using coroutines in user-mode.
> (Also user-mode really means Linux or BSD and I think both of
> those have working ucontext.)
> 
> thanks
> -- PMM
> 

Re: [PATCH v7 34/35] Hexagon build infrastructure

[Philippe Mathieu-Daudé]

On 1/20/21 4:29 AM, Taylor Simpson wrote:
> Add file to default-configs
> Add hexagon to meson.build
> Add hexagon to target/meson.build
> Add target/hexagon/meson.build
> Change scripts/qemu-binfmt-conf.sh
> 
> We can build a hexagon-linux-user target and run programs on the Hexagon
> scalar core.  With hexagon-linux-clang installed, "make check-tcg" will
> pass.
> 
> Signed-off-by: Taylor Simpson 
> ---
>  default-configs/targets/hexagon-linux-user.mak |   1 +
>  meson.build|   1 +
>  scripts/qemu-binfmt-conf.sh|   6 +-
>  target/hexagon/meson.build | 187 
> +
>  target/meson.build |   1 +
>  5 files changed, 195 insertions(+), 1 deletion(-)
>  create mode 100644 default-configs/targets/hexagon-linux-user.mak
>  create mode 100644 target/hexagon/meson.build
...

> +++ b/target/hexagon/meson.build
> @@ -0,0 +1,187 @@
> +##
> +##  Copyright(c) 2020-2021 Qualcomm Innovation Center, Inc. All Rights 
> Reserved.
> +##
> +##  This program is free software; you can redistribute it and/or modify
> +##  it under the terms of the GNU General Public License as published by
> +##  the Free Software Foundation; either version 2 of the License, or
> +##  (at your option) any later version.
> +##
> +##  This program is distributed in the hope that it will be useful,
> +##  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +##  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +##  GNU General Public License for more details.
> +##
> +##  You should have received a copy of the GNU General Public License
> +##  along with this program; if not, see .
> +##
> +
> +hexagon_ss = ss.source_set()
> +
> +prog_python = import('python').find_installation('python3')
> +
> +hex_common_py = 'hex_common.py'
> +attribs_def_h = meson.current_source_dir() / 'attribs_def.h'
> +gen_tcg_h = meson.current_source_dir() / 'gen_tcg.h'
> +
> +#
> +#  Step 1
> +#  We use a C program to create semantics_generated.pyinc
> +#
> +gen_semantics = executable('gen_semantics', 'gen_semantics.c')
> +
> +semantics = custom_target(
> +'semantics_generated.pyinc',
> +output: 'semantics_generated.pyinc',
> +input: gen_semantics,
> +command: ['@INPUT@', '@OUTPUT@'],
> +)
> +hexagon_ss.add(semantics)

Is something missing here?

$ make -j8
[316/1048] Generating semantics_generated.pyinc with a custom command
FAILED: target/hexagon/semantics_generated.pyinc
target/hexagon/gen_semantics target/hexagon/semantics_generated.pyinc
/bin/sh: 1: target/hexagon/gen_semantics: not found
ninja: build stopped: subcommand failed.

$ make target/hexagon/semantics_generated.pyinc V=1
/usr/bin/ninja -v   -j1  target/hexagon/semantics_generated.pyinc | cat
[1/1] target/hexagon/gen_semantics target/hexagon/semantics_generated.pyinc
FAILED: target/hexagon/semantics_generated.pyinc
target/hexagon/gen_semantics target/hexagon/semantics_generated.pyinc
/bin/sh: 1: target/hexagon/gen_semantics: not found
ninja: build stopped: subcommand failed.
make: *** [Makefile:172: run-ninja] Error 1

OK, I'm cross-compiling, target/hexagon/gen_semantics has been generated
but with as target, and we want it linked for the host...

Cc'ing Paolo in case he figures the issue simply looking at this patch
:)

Phil.

Re: [PATCH v7 34/35] Hexagon build infrastructure

[Philippe Mathieu-Daudé]

On 1/22/21 11:34 PM, Philippe Mathieu-Daudé wrote:
> On 1/20/21 4:29 AM, Taylor Simpson wrote:
>> Add file to default-configs
>> Add hexagon to meson.build
>> Add hexagon to target/meson.build
>> Add target/hexagon/meson.build
>> Change scripts/qemu-binfmt-conf.sh
>>
>> We can build a hexagon-linux-user target and run programs on the Hexagon
>> scalar core.  With hexagon-linux-clang installed, "make check-tcg" will
>> pass.
>>
>> Signed-off-by: Taylor Simpson 
>> ---
>>  default-configs/targets/hexagon-linux-user.mak |   1 +
>>  meson.build|   1 +
>>  scripts/qemu-binfmt-conf.sh|   6 +-
>>  target/hexagon/meson.build | 187 
>> +
>>  target/meson.build |   1 +
>>  5 files changed, 195 insertions(+), 1 deletion(-)
>>  create mode 100644 default-configs/targets/hexagon-linux-user.mak
>>  create mode 100644 target/hexagon/meson.build
> ...
> 
>> +++ b/target/hexagon/meson.build
>> @@ -0,0 +1,187 @@
>> +##
>> +##  Copyright(c) 2020-2021 Qualcomm Innovation Center, Inc. All Rights 
>> Reserved.
>> +##
>> +##  This program is free software; you can redistribute it and/or modify
>> +##  it under the terms of the GNU General Public License as published by
>> +##  the Free Software Foundation; either version 2 of the License, or
>> +##  (at your option) any later version.
>> +##
>> +##  This program is distributed in the hope that it will be useful,
>> +##  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +##  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +##  GNU General Public License for more details.
>> +##
>> +##  You should have received a copy of the GNU General Public License
>> +##  along with this program; if not, see .
>> +##
>> +
>> +hexagon_ss = ss.source_set()
>> +
>> +prog_python = import('python').find_installation('python3')
>> +
>> +hex_common_py = 'hex_common.py'
>> +attribs_def_h = meson.current_source_dir() / 'attribs_def.h'
>> +gen_tcg_h = meson.current_source_dir() / 'gen_tcg.h'
>> +
>> +#
>> +#  Step 1
>> +#  We use a C program to create semantics_generated.pyinc
>> +#
>> +gen_semantics = executable('gen_semantics', 'gen_semantics.c')
>> +
>> +semantics = custom_target(
>> +'semantics_generated.pyinc',
>> +output: 'semantics_generated.pyinc',
>> +input: gen_semantics,
>> +command: ['@INPUT@', '@OUTPUT@'],
>> +)
>> +hexagon_ss.add(semantics)
> 
> Is something missing here?
> 
> $ make -j8
> [316/1048] Generating semantics_generated.pyinc with a custom command
> FAILED: target/hexagon/semantics_generated.pyinc
> target/hexagon/gen_semantics target/hexagon/semantics_generated.pyinc
> /bin/sh: 1: target/hexagon/gen_semantics: not found
> ninja: build stopped: subcommand failed.
> 
> $ make target/hexagon/semantics_generated.pyinc V=1
> /usr/bin/ninja -v   -j1  target/hexagon/semantics_generated.pyinc | cat
> [1/1] target/hexagon/gen_semantics target/hexagon/semantics_generated.pyinc
> FAILED: target/hexagon/semantics_generated.pyinc
> target/hexagon/gen_semantics target/hexagon/semantics_generated.pyinc
> /bin/sh: 1: target/hexagon/gen_semantics: not found
> ninja: build stopped: subcommand failed.
> make: *** [Makefile:172: run-ninja] Error 1
> 
> OK, I'm cross-compiling, target/hexagon/gen_semantics has been generated
> but with as target, and we want it linked for the host...

So I compiled it manually using:

$ gcc -o target/hexagon/gen_semantics
~/source/qemu/target/hexagon/gen_semantics.c

Then same story:

[14/68] Generating iset.py with a custom command
FAILED: target/hexagon/iset.py
target/hexagon/gen_dectree_import target/hexagon/iset.py
/bin/sh: 1: target/hexagon/gen_dectree_import: not found
ninja: build stopped: subcommand failed.

$ gcc -o target/hexagon/gen_dectree_import
~/source/qemu/target/hexagon/gen_dectree_import.c
target/hexagon/gen_dectree_import.c:24:10: fatal error: qemu/osdep.h: No
such file or directory
 #include "qemu/osdep.h"
  ^~

It is late here, so enough testing for today. TBC ;)

BTW you should test your branch on gitlab-ci, I'm pretty sure
various jobs fail.

Regards,

Phil.

[Bug 1912857] Re: virtio-serial blocks hostfwd ssh on windows 10 host

[Ven Karri]

** Description changed:

  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1 -->
  WORKS - meaning I can ssh into the vm via port 
  
  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
  -device virtio-serial -device virtserialport,chardev=cid0 -chardev
- socket,id=cid0,{socket_addr_serial},server,nowait --> DOES NOT WORK -
- meaning I cannot ssh into the vm
+ socket,id=cid0,host:localhost,port:55298,server,nowait --> DOES NOT WORK
+ - meaning I cannot ssh into the vm
  
  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.
  
+ The following doesn't work either:
+ 
+ qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
+ user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
+ -device virtio-serial -device virtserialport,chardev=cid0 -chardev
+ file,id=cid0,path=temp,server,nowait
+ 
+ No matter which character device I use for my virtio-serial-port
+ communication (socket or udp or file or pipe), the hostfwd doesn't work.
+ 
  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

** Description changed:

  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1 -->
  WORKS - meaning I can ssh into the vm via port 
  
  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
  -device virtio-serial -device virtserialport,chardev=cid0 -chardev
  socket,id=cid0,host:localhost,port:55298,server,nowait --> DOES NOT WORK
  - meaning I cannot ssh into the vm
  
  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.
  
  The following doesn't work either:
  
  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
  -device virtio-serial -device virtserialport,chardev=cid0 -chardev
  file,id=cid0,path=temp,server,nowait
  
- No matter which character device I use for my virtio-serial-port
+ No matter which character device I use for my virtserialport
  communication (socket or udp or file or pipe), the hostfwd doesn't work.
  
  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1912857

Title:
  virtio-serial blocks hostfwd ssh on windows 10 host

Status in QEMU:
  New

Bug description:
  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1

  --> THIS WORKS - meaning I can ssh into the vm via port 

  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
    -device virtio-serial
    -device virtserialport,chardev=cid0
    -chardev socket,id=cid0,host=localhost,port=55298,server,nowait

  --> DOES NOT WORK - meaning I cannot ssh into the vm

  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.

  The following doesn't work either:

  qemu-system-x86_64
    -display none
    -hda archlinux.qcow2
    -m 4G
    -netdev user,id=n1,hostfwd=tcp::-:22
    -device virtio-net-pci,netdev=n1
    -device virtio-serial 
-device virtserialport,chardev=cid0
    -chardev file,id=cid0,path=temp,server,nowait

  No matter which character device I use for my virtserialport
  communication (socket or udp or file or pipe), the hostfwd doesn't
  work.

  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1912857/+subscriptions

Re: [PATCHv9 0/3] arm-virt: add secure pl061 for reset/power down

[Peter Maydell]

On Fri, 22 Jan 2021 at 21:52, Maxim Uvarov  wrote:
>
>  v9: - cosmetic changes (move if from patch2 to patch3, rename function name
>and define).
>  v8: - use gpio 0 and 1, align dtb with kernel gpio-restart, gpio-poweroff,
>change define names, trigger on upper front. (Peter Maydell).
>  v7: - same as v6, but resplit patches: patch 2 no function changes and 
> refactor
> gpio setup for virt platfrom and patch 3 adds secure gpio.
>  v6: - 64k align gpio memory region (Andrew Jones)
>  - adjusted memory region to map this address in the corresponding atf 
> patch
>  v5: - removed vms flag, added fdt  (Andrew Jones)
>  - added patch3 to combine secure and non secure pl061. It has to be
>more easy to review if this changes are in the separate patch.
>  v4: rework patches accodring to Peter Maydells comments:
> - split patches on gpio-pwr driver and arm-virt integration.
> - start secure gpio only from virt-6.0.
> - rework qemu interface for gpio-pwr to use 2 named gpio.
> - put secure gpio to secure name space.
>  v3: added missed include qemu/log.h for qemu_log(..
>  v2: replace printf with qemu_log (Philippe Mathieu-Daudé)
>
> This patch works together with ATF patch:
> 
> https://github.com/muvarov/arm-trusted-firmware/commit/886965bddb0624bdf85103efb2b39fd4eb73d89b
>
> Maxim Uvarov (3):
>   hw: gpio: implement gpio-pwr driver for qemu reset/poweroff
>   arm-virt: refactor gpios creation
>   arm-virt: add secure pl061 for reset/power down

Applied to target-arm.next, thanks. I realized we forgot the
documentation, so I'm going to squash this change in to patch 3:

--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -43,6 +43,8 @@ The virt board supports:
 - Secure-World-only devices if the CPU has TrustZone:

   - A second PL011 UART
+  - A second PL061 GPIO controller, with GPIO lines for triggering
+a system reset or system poweroff
   - A secure flash memory
   - 16MB of secure RAM

-- PMM

Re: [PATCH v2 3/3] target/arm: Set ID_AA64PFR0.DIT and ID_PFR0.DIT to 1 for "max" AA64 CPU

[Rebecca Cran]

On 1/22/21 2:06 PM, Richard Henderson wrote:

On 1/21/21 6:45 PM, Rebecca Cran wrote:

Enable FEAT_DIT for the "max" AARCH64 CPU.

Signed-off-by: Rebecca Cran 
Reviewed-by: Richard Henderson 
---
  target/arm/cpu64.c | 5 +
  1 file changed, 5 insertions(+)


There is also a 32-bit "max" cpu in cpu.c.


Thanks. I've fixed it in the next revision.

Re: [PATCH 8/8] configure: automatically parse command line for meson -D options

[Yonggang Luo]

On Sat, Jan 23, 2021 at 4:44 AM Paolo Bonzini  wrote:
>
>
>
> Il ven 22 gen 2021, 09:00 罗勇刚(Yonggang Luo)  ha
scritto:
>>
>> Hi Paolo, as python and meson are required dependencies to building qemu
now,
>> can we detecting python/meson at the very begining of configure,
>> even before the --help parameter.
>
>
> We could and I did it in the first version. However it's ugly that the
user has to use --python on some setups in order to get a full help message.

  Yeap, but finally configure should gone, so I think --python are
acceptable by user, just need make sure to be noticed when the default
python
are not python3

>
> Paolo
>
>>
>> On Wed, Jan 13, 2021 at 6:08 AM Paolo Bonzini 
wrote:
>> >
>> > On 13/01/21 11:31, Daniel P. Berrangé wrote:
>> > >>   meson-buildoptions.json | 717

>> > > I'm not a fan of seeing this file introduced as it has significant
>> > > overlap with meson_options.txt.I feel like the latter has enough
>> > > information present to do an acceptable job for help output. After
>> > > all that's sufficient if we were using meson directly.
>> >
>> > Sorry, I missed this remark.  meson-buildoptions.json is not
>> > hand-written.  It is the result of Meson's own parsing
meson_options.txt
>> > exported as JSON.
>> >
>> > In the commit message "because we parse command-line options before
>> > meson is available, the introspection output is stored in the source
>> > tree.  This is the reason for the unattractive diffstat; the number of
>> > JSON lines added is higher than the number of configure lines removed.
>> > Of course the latter are code that must be maintained manually and the
>> > former is not".
>> >
>> > Paolo
>> >
>> >
>>
>>
>> --
>>  此致
>> 礼
>> 罗勇刚
>> Yours
>> sincerely,
>> Yonggang Luo



--
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

[PATCH v8 04/11] slirp: feature detection for smbd

[Joelle van Dyne]

Replace Windows specific macro with a more generic feature detection
macro. Allows slirp smb feature to be disabled manually as well.

Signed-off-by: Joelle van Dyne 
---
 configure   | 22 +-
 meson.build |  2 +-
 net/slirp.c | 16 
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 8d8a4733d7..d72ab22da5 100755
--- a/configure
+++ b/configure
@@ -464,6 +464,7 @@ fuse="auto"
 fuse_lseek="auto"
 
 malloc_trim="auto"
+slirp_smbd="auto"
 
 # parse CC options second
 for opt do
@@ -845,7 +846,18 @@ do
 fi
 done
 
+# Check for smbd dupport
 : ${smbd=${SMBD-/usr/sbin/smbd}}
+if test "$slirp_smbd" != "no" ; then
+  if test "$mingw32" = "yes" ; then
+if test "$slirp_smbd" = "yes" ; then
+  error_exit "Host smbd not supported on this platform."
+fi
+slirp_smbd=no
+  else
+slirp_smbd=yes
+  fi
+fi
 
 # Default objcc to clang if available, otherwise use CC
 if has clang; then
@@ -1560,6 +1572,10 @@ for opt do
   ;;
   --disable-fuse-lseek) fuse_lseek="disabled"
   ;;
+  --enable-slirp-smbd) slirp_smbd=yes
+  ;;
+  --disable-slirp-smbd) slirp_smbd=no
+  ;;
   *)
   echo "ERROR: unknown option $opt"
   echo "Try '$0 --help' for more information"
@@ -1899,6 +1915,7 @@ disabled with --disable-FEATURE, default is enabled if 
available
   libdaxctl   libdaxctl support
   fuseFUSE block device export
   fuse-lseek  SEEK_HOLE/SEEK_DATA support for FUSE exports
+  slirp-smbd  use smbd (at path --smbd=*) in slirp networking
 
 NOTE: The object files are built at the place where configure is launched
 EOF
@@ -5573,7 +5590,10 @@ fi
 if test "$guest_agent" = "yes" ; then
   echo "CONFIG_GUEST_AGENT=y" >> $config_host_mak
 fi
-echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
+if test "$slirp_smbd" = "yes" ; then
+  echo "CONFIG_SLIRP_SMBD=y" >> $config_host_mak
+  echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
+fi
 if test "$vde" = "yes" ; then
   echo "CONFIG_VDE=y" >> $config_host_mak
   echo "VDE_LIBS=$vde_libs" >> $config_host_mak
diff --git a/meson.build b/meson.build
index 6818d97df5..f1e67b8cd1 100644
--- a/meson.build
+++ b/meson.build
@@ -2336,7 +2336,7 @@ summary_info += {'sphinx-build':  
sphinx_build.found()}
 summary_info += {'genisoimage':   config_host['GENISOIMAGE']}
 # TODO: add back version
 summary_info += {'slirp support': slirp_opt == 'disabled' ? false : 
slirp_opt}
-if slirp_opt != 'disabled'
+if slirp_opt != 'disabled' and 'CONFIG_SLIRP_SMBD' in config_host
   summary_info += {'smbd':config_host['CONFIG_SMBD_COMMAND']}
 endif
 summary_info += {'module support':config_host.has_key('CONFIG_MODULES')}
diff --git a/net/slirp.c b/net/slirp.c
index 8350c6d45f..4348e74805 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -27,7 +27,7 @@
 #include "net/slirp.h"
 
 
-#ifndef _WIN32
+#if defined(CONFIG_SLIRP_SMBD)
 #include 
 #include 
 #endif
@@ -90,7 +90,7 @@ typedef struct SlirpState {
 Slirp *slirp;
 Notifier poll_notifier;
 Notifier exit_notifier;
-#ifndef _WIN32
+#if defined(CONFIG_SLIRP_SMBD)
 gchar *smb_dir;
 #endif
 GSList *fwd;
@@ -103,7 +103,7 @@ static QTAILQ_HEAD(, SlirpState) slirp_stacks =
 static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp);
 static int slirp_guestfwd(SlirpState *s, const char *config_str, Error **errp);
 
-#ifndef _WIN32
+#if defined(CONFIG_SLIRP_SMBD)
 static int slirp_smb(SlirpState *s, const char *exported_dir,
  struct in_addr vserver_addr, Error **errp);
 static void slirp_smb_cleanup(SlirpState *s);
@@ -367,7 +367,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 struct in6_addr ip6_prefix;
 struct in6_addr ip6_host;
 struct in6_addr ip6_dns;
-#ifndef _WIN32
+#if defined(CONFIG_SLIRP_SMBD)
 struct in_addr smbsrv = { .s_addr = 0 };
 #endif
 NetClientState *nc;
@@ -477,7 +477,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 return -1;
 }
 
-#ifndef _WIN32
+#if defined(CONFIG_SLIRP_SMBD)
 if (vsmbserver && !inet_aton(vsmbserver, )) {
 error_setg(errp, "Failed to parse SMB address");
 return -1;
@@ -592,7 +592,7 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 }
 }
 }
-#ifndef _WIN32
+#if defined(CONFIG_SLIRP_SMBD)
 if (smb_export) {
 if (slirp_smb(s, smb_export, smbsrv, errp) < 0) {
 goto error;
@@ -784,7 +784,7 @@ void hmp_hostfwd_add(Monitor *mon, const QDict *qdict)
 
 }
 
-#ifndef _WIN32
+#if defined(CONFIG_SLIRP_SMBD)
 
 /* automatic user mode samba server configuration */
 static void slirp_smb_cleanup(SlirpState *s)
@@ -899,7 +899,7 @@ static int slirp_smb(SlirpState* s, const char 
*exported_dir,
 return 0;
 }
 
-#endif /* !defined(_WIN32) */
+#endif /* defined(CONFIG_SLIRP_SMBD) */
 
 static int guestfwd_can_read(void *opaque)
 {
-- 
2.28.0

[PATCH v8 02/11] configure: cross-compiling with empty cross_prefix

[Joelle van Dyne]

The iOS toolchain does not use the host prefix naming convention. So we
need to enable cross-compile options while allowing the PREFIX to be
blank.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Joelle van Dyne 
---
 configure | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 6f6a319c2f..8d8a4733d7 100755
--- a/configure
+++ b/configure
@@ -238,6 +238,7 @@ cpu=""
 iasl="iasl"
 interp_prefix="/usr/gnemul/qemu-%M"
 static="no"
+cross_compile="no"
 cross_prefix=""
 audio_drv_list=""
 block_drv_rw_whitelist=""
@@ -469,6 +470,7 @@ for opt do
   optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)')
   case "$opt" in
   --cross-prefix=*) cross_prefix="$optarg"
+cross_compile="yes"
   ;;
   --cc=*) CC="$optarg"
   ;;
@@ -1696,7 +1698,7 @@ $(echo Deprecated targets: $deprecated_targets_list | \
   --target-list-exclude=LIST exclude a set of targets from the default 
target-list
 
 Advanced options (experts only):
-  --cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]
+  --cross-prefix=PREFIXuse PREFIX for compile tools, PREFIX can be blank 
[$cross_prefix]
   --cc=CC  use C compiler CC [$cc]
   --iasl=IASL  use ACPI compiler IASL [$iasl]
   --host-cc=CC use C compiler CC [$host_cc] for code run at
@@ -6390,7 +6392,7 @@ if has $sdl2_config; then
 fi
 echo "strip = [$(meson_quote $strip)]" >> $cross
 echo "windres = [$(meson_quote $windres)]" >> $cross
-if test -n "$cross_prefix"; then
+if test "$cross_compile" = "yes"; then
 cross_arg="--cross-file config-meson.cross"
 echo "[host_machine]" >> $cross
 if test "$mingw32" = "yes" ; then
-- 
2.28.0

[PATCH v8 01/11] block: feature detection for host block support

[Joelle van Dyne]

On Darwin (iOS), there are no system level APIs for directly accessing
host block devices. We detect this at configure time.

Signed-off-by: Joelle van Dyne 
---
 meson.build  |  6 +-
 qapi/block-core.json | 10 +++---
 block/file-posix.c   | 33 ++---
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/meson.build b/meson.build
index af2bc89741..27110075df 100644
--- a/meson.build
+++ b/meson.build
@@ -180,7 +180,7 @@ if targetos == 'windows'
   include_directories: 
include_directories('.'))
 elif targetos == 'darwin'
   coref = dependency('appleframeworks', modules: 'CoreFoundation')
-  iokit = dependency('appleframeworks', modules: 'IOKit')
+  iokit = dependency('appleframeworks', modules: 'IOKit', required: false)
 elif targetos == 'sunos'
   socket = [cc.find_library('socket'),
 cc.find_library('nsl'),
@@ -1023,6 +1023,9 @@ if get_option('cfi')
   add_project_link_arguments(cfi_flags, native: false, language: ['c', 'cpp', 
'objc'])
 endif
 
+have_host_block_device = (targetos != 'darwin' or
+cc.has_header('IOKit/storage/IOMedia.h'))
+
 #
 # config-host.h #
 #
@@ -1113,6 +1116,7 @@ config_host_data.set('HAVE_DRM_H', 
cc.has_header('libdrm/drm.h'))
 config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
 config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
 config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
+config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
 
 ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
 arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 
'CONFIG_BDRV_RO_WHITELIST']
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3484986d1c..1a9576de8d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -959,7 +959,8 @@
   'discriminator': 'driver',
   'data': {
   'file': 'BlockStatsSpecificFile',
-  'host_device': 'BlockStatsSpecificFile',
+  'host_device': { 'type': 'BlockStatsSpecificFile',
+   'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
   'nvme': 'BlockStatsSpecificNvme' } }
 
 ##
@@ -2827,7 +2828,9 @@
 { 'enum': 'BlockdevDriver',
   'data': [ 'blkdebug', 'blklogwrites', 'blkreplay', 'blkverify', 'bochs',
 'cloop', 'compress', 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps',
-'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi',
+'gluster', 'host_cdrom',
+{'name': 'host_device', 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+'http', 'https', 'iscsi',
 'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
 'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
 { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
@@ -4012,7 +4015,8 @@
   'ftps':   'BlockdevOptionsCurlFtps',
   'gluster':'BlockdevOptionsGluster',
   'host_cdrom': 'BlockdevOptionsFile',
-  'host_device':'BlockdevOptionsFile',
+  'host_device': { 'type': 'BlockdevOptionsFile',
+   'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
   'http':   'BlockdevOptionsCurlHttp',
   'https':  'BlockdevOptionsCurlHttps',
   'iscsi':  'BlockdevOptionsIscsi',
diff --git a/block/file-posix.c b/block/file-posix.c
index 00cdaaa2d4..11d2021346 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -42,6 +42,8 @@
 #include "scsi/constants.h"
 
 #if defined(__APPLE__) && (__MACH__)
+#include 
+#if defined(HAVE_HOST_BLOCK_DEVICE)
 #include 
 #include 
 #include 
@@ -52,6 +54,7 @@
 //#include 
 #include 
 #include 
+#endif /* defined(HAVE_HOST_BLOCK_DEVICE) */
 #endif
 
 #ifdef __sun__
@@ -181,7 +184,17 @@ typedef struct BDRVRawReopenState {
 bool check_cache_dropped;
 } BDRVRawReopenState;
 
-static int fd_open(BlockDriverState *bs);
+static int fd_open(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
+
+/* this is just to ensure s->fd is sane (its called by io ops) */
+if (s->fd >= 0) {
+return 0;
+}
+return -EIO;
+}
+
 static int64_t raw_getlength(BlockDriverState *bs);
 
 typedef struct RawPosixAIOData {
@@ -3014,6 +3027,7 @@ static BlockStatsSpecific 
*raw_get_specific_stats(BlockDriverState *bs)
 return stats;
 }
 
+#if defined(HAVE_HOST_BLOCK_DEVICE)
 static BlockStatsSpecific *hdev_get_specific_stats(BlockDriverState *bs)
 {
 BlockStatsSpecific *stats = g_new(BlockStatsSpecific, 1);
@@ -3023,6 +3037,7 @@ static BlockStatsSpecific 
*hdev_get_specific_stats(BlockDriverState *bs)
 
 return stats;
 }
+#endif /* HAVE_HOST_BLOCK_DEVICE */
 
 static QemuOptsList raw_create_opts = {
 .name = "raw-create-opts",
@@ -3247,6 +3262,8 @@ BlockDriver bdrv_file = {
 /***/
 /* host device */
 
+#if defined(HAVE_HOST_BLOCK_DEVICE)
+
 #if defined(__APPLE__) && defined(__MACH__)
 

[PATCH v8 00/11] iOS and Apple Silicon host support

[Joelle van Dyne]

These set of changes brings QEMU TCG to iOS devices and future Apple Silicon
devices. They were originally developed last year and have been working in the
UTM app. Recently, we ported the changes to master, re-wrote a lot of the build
script changes for meson, and broke up the patches into more distinct units.

The bulk of the changes allow for cross-compiling for both iOS and macOS running
Apple Silicon and adds feature detection for parts of QEMU that are not
compatible with iOS.

Since v8:

* Moved some feature checks to meson.build
* system() stub return error instead of assertion

Since v7:

* Removed libucontext (will be submitted in another patchset)
* Removed slirp build flags update (superseded by subproject patchset)
* Reworked all patches to use feature detection instead of #ifdef CONFIG_IOS
* Added feature detection for CoreAudio
* Fix various cross compiling issues on macOS

Since v6:

* Dropped the Apple Silicon JIT support patch (superseded by another patchset)
* Changed libucontext to be a Meson subproject
* Cache availablity check for preadv/pwritev on macOS 11 and iOS 14

Since v5:

* Fixed some more instances of QAPI define of CONFIG_HOST_BLOCK_DEVICE
* Fixed libucontext build on newer version of GCC

Since v4:

* Updated QAPI schema for CONFIG_HOST_BLOCK_DEVICE
* Updated maintainers file for iOS host support
* Moved system() changes to osdep.h
* Fixed typo in libucontext meson.build change

Since v3:

* Moved mirror JIT support to a different patch set.
* Removed dependency on `pthread_jit_write_protect_np` because it was redundent
  and also crashes if called on a non-jailbroken iOS device.
* Removed `--enable-cross-compile` option
* Fixed checkpatch errors
* Fixed iOS build on master due to new test recently added which calls system()

Since v2:

* Changed getting mirror pointer from a macro to inline functions
* Split constification of TCG code pointers to separate patch
* Removed slirp updates (will send future patch once slirp changes are in)
* Removed shared library patch (will send future patch)

-j

Joelle van Dyne (11):
  block: feature detection for host block support
  configure: cross-compiling with empty cross_prefix
  configure: check for sys/disk.h
  slirp: feature detection for smbd
  osdep: build with non-working system() function
  darwin: remove redundant dependency declaration
  darwin: fix cross-compiling for Darwin
  configure: cross compile should use x86_64 cpu_family
  block: check availablity for preadv/pwritev on mac
  darwin: detect CoreAudio for build
  darwin: remove 64-bit build detection on 32-bit OS

 configure| 104 +++
 meson.build  |   9 +++-
 qapi/block-core.json |  10 +++--
 include/qemu/osdep.h |  12 +
 block.c  |   2 +-
 block/file-posix.c   |  68 +++-
 net/slirp.c  |  16 +++
 7 files changed, 177 insertions(+), 44 deletions(-)

-- 
2.28.0

RE: [PATCH v7 12/35] Hexagon (target/hexagon) instruction attributes

[Taylor Simpson]

> -Original Message-
> From: Philippe Mathieu-Daudé  On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Friday, January 22, 2021 11:54 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; alex.ben...@linaro.org;
> laur...@vivier.eu; a...@rev.ng; Brian Cain 
> Subject: Re: [PATCH v7 12/35] Hexagon (target/hexagon) instruction
> attributes
>
> On 1/20/21 4:28 AM, Taylor Simpson wrote:
> > Signed-off-by: Taylor Simpson 
> > ---
> >  target/hexagon/attribs.h | 30 ++
> >  target/hexagon/attribs_def.h | 95
> 
> >  2 files changed, 125 insertions(+)
> >  create mode 100644 target/hexagon/attribs.h
> >  create mode 100644 target/hexagon/attribs_def.h
> >
> > diff --git a/target/hexagon/attribs.h b/target/hexagon/attribs.h
> > new file mode 100644
> > index 000..e88e5eb
> > --- /dev/null
> > +++ b/target/hexagon/attribs.h
> > @@ -0,0 +1,30 @@
> > +
> > +enum {
> > +#define DEF_ATTRIB(NAME, ...) A_##NAME,
> > +#include "attribs_def.h"
>
> Per QEMU conventions, this file has to be named "attribs_def.h.inc".

Didn't know that.  Which files should end in .inc?

>
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé 

Thanks!!

[Bug 1745895] Re: Unable to migrate vhost-net to virtio-1.0-capable kernel

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1745895

Title:
  Unable to migrate vhost-net to virtio-1.0-capable kernel

Status in QEMU:
  Expired

Bug description:
  I am running QEMU 2.11 (from upstream source, not Red Hat package) on
  stock RHEL 6 and RHEL 7 kernels. Only the RHEL 7 kernel supports
  VIRTIO_F_VERSION_1 in its vhost-net driver.

  When migrating a guest using vhost-net from the RHEL 6 host to RHEL 7,
  the PCI config is rejected by QEMU on the target machine.

  A simple test case:

  1. On the RHEL 7 host, prepare for an incoming migration:

rhel7# qemu-system-x86_64 -S -accel kvm -nographic -monitor stdio
  -nodefaults -netdev tap,id=net0,vhost=on,script=no,downscript=no
  -device virtio-net-pci,netdev=net0,mac=54:52:00:ff:ff:ff -incoming
  tcp:0.0.0.0:12345

  2. On the RHEL 6 host, start a guest and migrate it to the RHEL 7
  host:

rhel6# qemu-system-x86_64 -S -accel kvm -nographic -monitor stdio 
-nodefaults -netdev tap,id=net0,vhost=on,script=no,downscript=no -device 
virtio-net-pci,netdev=net0,mac=54:52:00:ff:ff:ff
  QEMU 2.11.0 monitor - type 'help' for more information
(qemu) migrate tcp:rhel7:12345

  The RHEL 7 QEMU errors out:

qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x20 read: 0 
device: c cmask: ff wmask: 0 w1cmask:0
qemu-system-x86_64: Failed to load PCIDevice:config
qemu-system-x86_64: Failed to load virtio-net:virtio
qemu-system-x86_64: error while loading state for instance 0x0 of device 
':00:02.0/virtio-net'
qemu-system-x86_64: load of migration failed: Invalid argument

  If I start the source QEMU with vhost=off, or the target QEMU with
  disable-modern=true, the migration is successful.

  My hunch here is that the target QEMU prepares the PCI device to
  support VIRTIO_F_VERSION_1, as that's available in the kernel there,
  but then fails to (or does not know to) disable this during the
  migration.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1745895/+subscriptions

[PATCH v8 10/11] darwin: detect CoreAudio for build

[Joelle van Dyne]

On iOS there is no CoreAudio, so we should not assume Darwin always
has it.

Signed-off-by: Joelle van Dyne 
---
 configure | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index b8ae4609fd..70061e195d 100755
--- a/configure
+++ b/configure
@@ -319,6 +319,7 @@ fdt="auto"
 netmap="no"
 sdl="auto"
 sdl_image="auto"
+coreaudio="auto"
 virtiofsd="auto"
 virtfs="auto"
 libudev="auto"
@@ -779,7 +780,7 @@ Darwin)
 QEMU_CFLAGS="-arch x86_64 $QEMU_CFLAGS"
 QEMU_LDFLAGS="-arch x86_64 $QEMU_LDFLAGS"
   fi
-  audio_drv_list="coreaudio try-sdl"
+  audio_drv_list="try-coreaudio try-sdl"
   audio_possible_drivers="coreaudio sdl"
   # Disable attempts to use ObjectiveC features in os/object.h since they
   # won't work when we're compiling with gcc as a C compiler.
@@ -3162,6 +3163,24 @@ EOF
   fi
 fi
 
+##
+# detect CoreAudio
+if test "$coreaudio" != "no" ; then
+  coreaudio_libs="-framework CoreAudio"
+  cat > $TMPC << EOF
+#include 
+int main(void)
+{
+  return (int)AudioGetCurrentHostTime();
+}
+EOF
+  if compile_prog "" "$coreaudio_libs" ; then
+coreaudio=yes
+  else
+coreaudio=no
+  fi
+fi
+
 ##
 # Sound support libraries probe
 
@@ -3218,8 +3237,20 @@ for drv in $audio_drv_list; do
 fi
 ;;
 
-coreaudio)
+coreaudio | try-coreaudio)
+if test "$coreaudio" = "no"; then
+  if test "$drv" = "try-coreaudio"; then
+audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-coreaudio//')
+  else
+error_exit "$drv check failed" \
+"Make sure to have the $drv is available."
+  fi
+else
   coreaudio_libs="-framework CoreAudio"
+  if test "$drv" = "try-coreaudio"; then
+audio_drv_list=$(echo "$audio_drv_list" | sed -e 
's/try-coreaudio/coreaudio/')
+  fi
+fi
 ;;
 
 dsound)
-- 
2.28.0

Re: [PATCH v7 07/11] darwin: fix cross-compiling for Darwin

[Peter Maydell]

On Fri, 22 Jan 2021 at 20:18, Joelle van Dyne  wrote:
>
> Add objc to the Meson cross file as well as detection of Darwin.
>
> Signed-off-by: Joelle van Dyne 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v7 04/11] slirp: feature detection for smbd

[Joelle van Dyne]

On Fri, Jan 22, 2021 at 2:49 PM Peter Maydell  wrote:
>
> On Fri, 22 Jan 2021 at 20:16, Joelle van Dyne  wrote:
> >
> > Replace Windows specific macro with a more generic feature detection
> > macro. Allows slirp smb feature to be disabled manually as well.
> >
> > Signed-off-by: Joelle van Dyne 
> > ---
>
>
> > +if test "$slirp_smbd" = "yes" ; then
> > +  echo "CONFIG_SLIRP_SMBD=y" >> $config_host_mak
> > +  echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
> > +fi
>
> This sets "CONFIG_SLIRP_SMBD" and "CONFIG_SMBD_COMMAND"...
>
> >  if test "$vde" = "yes" ; then
> >echo "CONFIG_VDE=y" >> $config_host_mak
> >echo "VDE_LIBS=$vde_libs" >> $config_host_mak
> > diff --git a/meson.build b/meson.build
> > index 6c3ee7f8ca..9577138d7f 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -2331,7 +2331,7 @@ summary_info += {'sphinx-build':  
> > sphinx_build.found()}
> >  summary_info += {'genisoimage':   config_host['GENISOIMAGE']}
> >  # TODO: add back version
> >  summary_info += {'slirp support': slirp_opt == 'disabled' ? false : 
> > slirp_opt}
> > -if slirp_opt != 'disabled'
> > +if slirp_opt != 'disabled' and 'HAVE_HOST_SMBD' in config_host
>
> ...but this is looking for "HAVE_HOST_SMBD". Should it be something else?
Yes, it is a typo, will fix.

-j
>
> >summary_info += {'smbd':config_host['CONFIG_SMBD_COMMAND']}
> >  endif
> >  summary_info += {'module support':
> > config_host.has_key('CONFIG_MODULES')}
>
> thanks
> -- PMM

[PATCH] hvf: Fetch cr4 before evaluating CPUID(1)

[Alexander Graf]

The CPUID function 1 has a bit called OSXSAVE which tells user space the
status of the CR4.OSXSAVE bit. Our generic CPUID function injects that bit
based on the status of CR4.

With Hypervisor.framework, we do not synchronize full CPU state often enough
for this function to see the CR4 update before guest user space asks for it.

To be on the save side, let's just always synchronize it when we receive a
CPUID(1) request. That way we can set the bit with real confidence.

Reported-by: Asad Ali 
Signed-off-by: Alexander Graf 
---
 target/i386/hvf/hvf.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 08b4adecd9..f660b829ac 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -426,6 +426,10 @@ int hvf_vcpu_exec(CPUState *cpu)
 uint32_t rcx = (uint32_t)rreg(cpu->hvf->fd, HV_X86_RCX);
 uint32_t rdx = (uint32_t)rreg(cpu->hvf->fd, HV_X86_RDX);
 
+if (rax == 1) {
+/* CPUID1.ecx.OSXSAVE needs to know CR4 */
+env->cr[4] = rvmcs(cpu->hvf->fd, VMCS_GUEST_CR4);
+}
 cpu_x86_cpuid(env, rax, rcx, , , , );
 
 wreg(cpu->hvf->fd, HV_X86_RAX, rax);
-- 
2.24.3 (Apple Git-128)

Re: [PATCH V5 4/6] hw/block/nvme: support for multi-controller in subsystem

[Minwoo Im]

On 21-01-22 10:42:36, Keith Busch wrote:
> On Fri, Jan 22, 2021 at 09:07:34PM +0900, Minwoo Im wrote:
> > index b525fca14103..3dedefb8ebba 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -4435,6 +4435,9 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
> > *pci_dev)
> >  strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
> >  strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
> >  strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
> > +
> > +id->cntlid = n->cntlid;
> 
> cpu_to_le16()? It might be okay to not do that since the only
> requirement is that this is a unique value, but it would be confusing
> for decoding commands that have a controller id field.

Agreed.

Yes, cntlids are allocated in unique values so that functionality has no
problem here.  But, even if so, we should make it have proper value in
Identify data structure with the policy it has to avoid confusing.

Thanks Keith! will fix it :)

[PATCH v8 09/11] block: check availablity for preadv/pwritev on mac

[Joelle van Dyne]

macOS 11/iOS 14 added preadv/pwritev APIs. Due to weak linking, configure
will succeed with CONFIG_PREADV even when targeting a lower OS version.
We therefore need to check at run time if we can actually use these APIs.

Signed-off-by: Joelle van Dyne 
---
 block/file-posix.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 666d3e7504..6473f84db8 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1386,17 +1386,50 @@ static int handle_aiocb_flush(void *opaque)
 #ifdef CONFIG_PREADV
 
 static bool preadv_present = true;
+static bool preadv_checked;
 
 static ssize_t
 qemu_preadv(int fd, const struct iovec *iov, int nr_iov, off_t offset)
 {
+#ifdef CONFIG_DARWIN /* preadv introduced in macOS 11 */
+if (unlikely(!preadv_checked)) {
+if (__builtin_available(macOS 11, iOS 14, watchOS 7, tvOS 14, *)) {
+preadv_checked = true;
+} else {
+preadv_present = false;
+return -ENOSYS;
+}
+}
+/* Now we suppress the availability warning since we use the cached check 
*/
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunguarded-availability-new"
+return preadv(fd, iov, nr_iov, offset);
+#pragma clang diagnostic pop
+#else /* CONFIG_DARWIN */
 return preadv(fd, iov, nr_iov, offset);
+#endif
 }
 
 static ssize_t
 qemu_pwritev(int fd, const struct iovec *iov, int nr_iov, off_t offset)
 {
+#ifdef CONFIG_DARWIN /* preadv introduced in macOS 11 */
+if (unlikely(!preadv_checked)) {
+if (__builtin_available(macOS 11, iOS 14, watchOS 7, tvOS 14, *)) {
+preadv_checked = true;
+} else {
+preadv_present = false;
+return -ENOSYS;
+}
+}
+/* Now we suppress the availability warning since we use the cached check 
*/
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunguarded-availability-new"
+return pwritev(fd, iov, nr_iov, offset);
+#pragma clang diagnostic pop
+#else /* CONFIG_DARWIN */
 return pwritev(fd, iov, nr_iov, offset);
+#endif
 }
 
 #else
-- 
2.28.0

[PATCH v8 08/11] configure: cross compile should use x86_64 cpu_family

[Joelle van Dyne]

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Joelle van Dyne 
---
 configure | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index d4588ed892..b8ae4609fd 100755
--- a/configure
+++ b/configure
@@ -6445,9 +6445,12 @@ if test "$cross_compile" = "yes"; then
 echo "system = 'darwin'" >> $cross
 fi
 case "$ARCH" in
-i386|x86_64)
+i386)
 echo "cpu_family = 'x86'" >> $cross
 ;;
+x86_64)
+echo "cpu_family = 'x86_64'" >> $cross
+;;
 ppc64le)
 echo "cpu_family = 'ppc64'" >> $cross
 ;;
-- 
2.28.0

Re: [PATCH v7 03/11] configure: check for sys/disk.h

[Peter Maydell]

On Fri, 22 Jan 2021 at 20:13, Joelle van Dyne  wrote:
>
> Some BSD platforms do not have this header.
>
> Signed-off-by: Joelle van Dyne 
> ---
>  configure  | 9 +
>  block.c| 2 +-
>  block/file-posix.c | 2 +-
>  3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 32be5d225d..951de427bb 100755
> --- a/configure
> +++ b/configure
> @@ -5295,6 +5295,12 @@ else
>have_host_block_device=no
>  fi
>
> +if check_include "sys/disk.h" ; then
> +  sys_disk_h=yes
> +else
> +  sys_disk_h=no
> +fi
> +
>  ##
>  # End of CC checks
>  # After here, no more $cc or $ld runs
> @@ -5528,6 +5534,9 @@ echo "ARCH=$ARCH" >> $config_host_mak
>  if test "$have_host_block_device" = "yes" ; then
>echo "HAVE_HOST_BLOCK_DEVICE=y" >> $config_host_mak
>  fi
> +if test "$sys_disk_h" = "yes" ; then
> +  echo "HAVE_SYS_DISK_H=y" >> $config_host_mak
> +fi
>  if test "$debug_tcg" = "yes" ; then
>echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
>  fi

We should do this check in meson.build, where it is a one-liner:

config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))

(compare the existing HAVE_PTY_H etc).

thanks
-- PMM

[Bug 1912857] Re: virtio-serial blocks hostfwd ssh on windows 10 host

[Ven Karri]

** Description changed:

  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1 -->
  WORKS - meaning I can ssh into the vm via port 
  
  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
- -device virtio-serial -serial tcp:localhost:55298,server,nowait --> DOES
- NOT WORK - meaning I cannot ssh into the vm
+ -device virtio-serial -device virtserialport,chardev=cid0 -chardev
+ socket,id=cid0,{socket_addr_serial},server,nowait --> DOES NOT WORK -
+ meaning I cannot ssh into the vm
  
  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.
  
  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1912857

Title:
  virtio-serial blocks hostfwd ssh on windows 10 host

Status in QEMU:
  New

Bug description:
  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1 -->
  WORKS - meaning I can ssh into the vm via port 

  qemu-system-x86_64 -display none -hda archlinux.qcow2 -m 4G -netdev
  user,id=n1,hostfwd=tcp::-:22 -device virtio-net-pci,netdev=n1
  -device virtio-serial -device virtserialport,chardev=cid0 -chardev
  socket,id=cid0,{socket_addr_serial},server,nowait --> DOES NOT WORK -
  meaning I cannot ssh into the vm

  Not only does the port  not work, but I am not able to perform any
  serial transfer on port 55298 as well.

  Host: Windows 10
  Guest: archlinux
  QEMU version 5.2

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1912857/+subscriptions

Re: [PATCH v7 05/11] osdep: build with non-working system() function

[Peter Maydell]

On Fri, 22 Jan 2021 at 23:12, Peter Maydell  wrote:
>
> On Fri, 22 Jan 2021 at 20:13, Joelle van Dyne  wrote:
> >
> > Build without error on hosts without a working system(). An assertion
> > will trigger if system() is called.
> >
> > Signed-off-by: Joelle van Dyne 
>
>  configure| 19 +++
>
> Can we do the "does system() exist?" check in meson.build ?
> Untested, but looking at the existing check for "does gettid() exist?"
> it should be two lines:
>
> has_system = cc.has_function('system')
>
> and then later:
>
> config_host_data.set('HAVE_SYSTEM_FUNCTION', has_system)

...looking at how we do the HAVE_FOO_H settings, I think we
can just collapse this into one line:

config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system'))

thanks
-- PMM

Re: [PATCH] spapr: Adjust firmware path of PCI devices

[Alexey Kardashevskiy]

On 23/01/2021 04:01, Greg Kurz wrote:

It is currently not possible to perform a strict boot from USB storage:

$ qemu-system-ppc64 -accel kvm -nodefaults -nographic -serial stdio \
-boot strict=on \
-device qemu-xhci \
-device usb-storage,drive=disk,bootindex=0 \
-blockdev driver=file,node-name=disk,filename=fedora-ppc64le.qcow2


SLOF **
QEMU Starting
  Build Date = Jul 17 2020 11:15:24
  FW Version = git-e18ddad8516ff2cf
  Press "s" to enter Open Firmware.

Populating /vdevice methods
Populating /vdevice/vty@7100
Populating /vdevice/nvram@7101
Populating /pci@8002000
  00  (D) : 1b36 000dserial bus [ usb-xhci ]
No NVRAM common partition, re-initializing...
Scanning USB
   XHCI: Initializing
 USB Storage
SCSI: Looking for devices
   101 DISK : "QEMU QEMU HARDDISK2.5+"
Using default console: /vdevice/vty@7100

   Welcome to Open Firmware

   Copyright (c) 2004, 2017 IBM Corporation All rights reserved.
   This program and the accompanying materials are made available
   under the terms of the BSD License available at
   http://www.opensource.org/licenses/bsd-license.php


Trying to load:  from: 
/pci@8002000/usb@0/storage@1/disk@101 ...
E3405: No such device

E3407: Load failed

   Type 'boot' and press return to continue booting the system.
   Type 'reset-all' and press return to reboot the system.


Ready!
0 >

The device tree handed over by QEMU to SLOF indeed contains:

qemu,boot-list =
"/pci@8002000/usb@0/storage@1/disk@101 HALT";

but the device node is named usb-xhci@0, not usb@0.



I'd expect it to be a return of qdev_fw_name() so in this case something 
like "nec-usb-xhci" (which would still be broken) but seeing a plain 
"usb" is a bit strange.


While your patch works, I wonder if we should assign fw_name to all pci 
nodes to avoid similar problems in the future? Thanks,







This happens because the firmware names of PCI devices returned
by get_boot_devices_list() come from pcibus_get_fw_dev_path(),
while the sPAPR PHB code uses a different naming scheme for
device nodes. This inconsistency has always been there but it was
hidden for a long time because SLOF used to rename USB device
nodes, until this commit, merged in QEMU 4.2.0 :

commit 85164ad4ed9960cac842fa4cc067c6b6699b0994
Author: Alexey Kardashevskiy 
Date:   Wed Sep 11 16:24:32 2019 +1000

 pseries: Update SLOF firmware image

 This fixes USB host bus adapter name in the device tree to match QEMU's
 one.

 Signed-off-by: Alexey Kardashevskiy 
 Signed-off-by: David Gibson 

Fortunately, sPAPR implements the firmware path provider interface.
This provides a way to override the default firmware paths.

Just factor out the sPAPR PHB naming logic from spapr_dt_pci_device()
to a helper, and use it in the sPAPR firmware path provider hook.

Fixes: 85164ad4ed99 ("pseries: Update SLOF firmware image")
Signed-off-by: Greg Kurz 
---
  include/hw/pci-host/spapr.h |  2 ++
  hw/ppc/spapr.c  |  5 +
  hw/ppc/spapr_pci.c  | 33 ++---
  3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index bd014823a933..5b03a7b0eb3f 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -210,4 +210,6 @@ static inline unsigned 
spapr_phb_windows_supported(SpaprPhbState *sphb)
  return sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
  }
  
+char *spapr_pci_fw_dev_name(PCIDevice *dev);

+
  #endif /* PCI_HOST_SPAPR_H */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6ab27ea269d5..632502c2ecf8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3048,6 +3048,7 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, 
BusState *bus,
  SCSIDevice *d = CAST(SCSIDevice,  dev, TYPE_SCSI_DEVICE);
  SpaprPhbState *phb = CAST(SpaprPhbState, dev, TYPE_SPAPR_PCI_HOST_BRIDGE);
  VHostSCSICommon *vsc = CAST(VHostSCSICommon, dev, TYPE_VHOST_SCSI_COMMON);
+PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
  
  if (d) {

  void *spapr = CAST(void, bus->parent, "spapr-vscsi");
@@ -3121,6 +3122,10 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, 
BusState *bus,
  return g_strdup_printf("pci@%x", PCI_SLOT(pcidev->devfn));
  }
  
+if (pcidev) {

+return spapr_pci_fw_dev_name(pcidev);
+}
+
  return NULL;
  }
  
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c

index 76d7c91e9c64..da6eb58724c8 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1334,15 +1334,29 @@ static int spapr_dt_pci_bus(SpaprPhbState *sphb, PCIBus 
*bus,
  return offset;
  }
  
+char *spapr_pci_fw_dev_name(PCIDevice *dev)

+{
+const gchar *basename;
+int slot = PCI_SLOT(dev->devfn);
+int func = 

Re: [PATCH v7 05/11] osdep: build with non-working system() function

[Joelle van Dyne]

Unfortunately, this doesn't work for iOS, which defines system() but
throws a compile time error if you try to call it.

-j

On Fri, Jan 22, 2021 at 3:17 PM Peter Maydell  wrote:
>
> On Fri, 22 Jan 2021 at 23:12, Peter Maydell  wrote:
> >
> > On Fri, 22 Jan 2021 at 20:13, Joelle van Dyne  wrote:
> > >
> > > Build without error on hosts without a working system(). An assertion
> > > will trigger if system() is called.
> > >
> > > Signed-off-by: Joelle van Dyne 
> >
> >  configure| 19 +++
> >
> > Can we do the "does system() exist?" check in meson.build ?
> > Untested, but looking at the existing check for "does gettid() exist?"
> > it should be two lines:
> >
> > has_system = cc.has_function('system')
> >
> > and then later:
> >
> > config_host_data.set('HAVE_SYSTEM_FUNCTION', has_system)
>
> ...looking at how we do the HAVE_FOO_H settings, I think we
> can just collapse this into one line:
>
> config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system'))
>
> thanks
> -- PMM

[PATCH v8 06/11] darwin: remove redundant dependency declaration

[Joelle van Dyne]

Meson will find CoreFoundation, IOKit, and Cocoa as needed.

Reviewed-by: Peter Maydell 
Signed-off-by: Joelle van Dyne 
---
 configure | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure b/configure
index de7487a0c7..0fd3f14c5e 100755
--- a/configure
+++ b/configure
@@ -781,7 +781,6 @@ Darwin)
   fi
   audio_drv_list="coreaudio try-sdl"
   audio_possible_drivers="coreaudio sdl"
-  QEMU_LDFLAGS="-framework CoreFoundation -framework IOKit $QEMU_LDFLAGS"
   # Disable attempts to use ObjectiveC features in os/object.h since they
   # won't work when we're compiling with gcc as a C compiler.
   QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
-- 
2.28.0

[PATCH v8 05/11] osdep: build with non-working system() function

[Joelle van Dyne]

Build without error on hosts without a working system(). If system()
is called, return -1 with ENOSYS.

Signed-off-by: Joelle van Dyne 
---
 configure| 20 
 include/qemu/osdep.h | 12 
 2 files changed, 32 insertions(+)

diff --git a/configure b/configure
index d72ab22da5..de7487a0c7 100755
--- a/configure
+++ b/configure
@@ -5302,6 +5302,22 @@ but not implemented on your system"
 fi
 fi
 
+##
+# check for system()
+# make sure there is no compile error
+
+have_system_function=no
+cat > $TMPC << EOF
+#include 
+int main(void) {
+return system("");
+}
+EOF
+if compile_prog "" "" ; then
+have_system_function=yes
+fi
+
+
 ##
 # End of CC checks
 # After here, no more $cc or $ld runs
@@ -6200,6 +6216,10 @@ if test "$secret_keyring" = "yes" ; then
   echo "CONFIG_SECRET_KEYRING=y" >> $config_host_mak
 fi
 
+if test "$have_system_function" = "yes" ; then
+  echo "HAVE_SYSTEM_FUNCTION=y" >> $config_host_mak
+fi
+
 echo "ROMS=$roms" >> $config_host_mak
 echo "MAKE=$make" >> $config_host_mak
 echo "PYTHON=$python" >> $config_host_mak
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index a434382c58..5bd1a67769 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -682,4 +682,16 @@ char *qemu_get_host_name(Error **errp);
  */
 size_t qemu_get_host_physmem(void);
 
+/**
+ * Platforms which do not support system() return ENOSYS
+ */
+#ifndef HAVE_SYSTEM_FUNCTION
+#define system platform_does_not_support_system
+static inline int platform_does_not_support_system(const char *command)
+{
+errno = ENOSYS;
+return -1;
+}
+#endif /* !HAVE_SYSTEM_FUNCTION */
+
 #endif
-- 
2.28.0

[PATCH v8 03/11] configure: check for sys/disk.h

[Joelle van Dyne]

Some BSD platforms do not have this header.

Signed-off-by: Joelle van Dyne 
---
 meson.build| 1 +
 block.c| 2 +-
 block/file-posix.c | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 27110075df..6818d97df5 100644
--- a/meson.build
+++ b/meson.build
@@ -1117,6 +1117,7 @@ config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
 config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
 config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
 config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
+config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
 
 ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
 arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 
'CONFIG_BDRV_RO_WHITELIST']
diff --git a/block.c b/block.c
index 8b9d457546..c4cf391dea 100644
--- a/block.c
+++ b/block.c
@@ -54,7 +54,7 @@
 #ifdef CONFIG_BSD
 #include 
 #include 
-#ifndef __DragonFly__
+#if defined(HAVE_SYS_DISK_H)
 #include 
 #endif
 #endif
diff --git a/block/file-posix.c b/block/file-posix.c
index 11d2021346..666d3e7504 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2320,7 +2320,7 @@ again:
 }
 if (size == 0)
 #endif
-#if defined(__APPLE__) && defined(__MACH__)
+#if defined(HAVE_SYS_DISK_H) && defined(__APPLE__) && defined(__MACH__)
 {
 uint64_t sectors = 0;
 uint32_t sector_size = 0;
-- 
2.28.0

[PATCH v8 07/11] darwin: fix cross-compiling for Darwin

[Joelle van Dyne]

Add objc to the Meson cross file as well as detection of Darwin.

Reviewed-by: Peter Maydell 
Signed-off-by: Joelle van Dyne 
---
 configure | 4 
 1 file changed, 4 insertions(+)

diff --git a/configure b/configure
index 0fd3f14c5e..d4588ed892 100755
--- a/configure
+++ b/configure
@@ -6422,6 +6422,7 @@ echo "cpp_link_args = [${LDFLAGS:+$(meson_quote 
$LDFLAGS)}]" >> $cross
 echo "[binaries]" >> $cross
 echo "c = [$(meson_quote $cc)]" >> $cross
 test -n "$cxx" && echo "cpp = [$(meson_quote $cxx)]" >> $cross
+test -n "$objcc" && echo "objc = [$(meson_quote $objcc)]" >> $cross
 echo "ar = [$(meson_quote $ar)]" >> $cross
 echo "nm = [$(meson_quote $nm)]" >> $cross
 echo "pkgconfig = [$(meson_quote $pkg_config_exe)]" >> $cross
@@ -6440,6 +6441,9 @@ if test "$cross_compile" = "yes"; then
 if test "$linux" = "yes" ; then
 echo "system = 'linux'" >> $cross
 fi
+if test "$darwin" = "yes" ; then
+echo "system = 'darwin'" >> $cross
+fi
 case "$ARCH" in
 i386|x86_64)
 echo "cpu_family = 'x86'" >> $cross
-- 
2.28.0

Re: [PATCH] linux-user/mmap: Avoid asserts for out of range mremap calls

[Richard Purdie]

On Fri, 2021-01-08 at 17:42 +, Richard Purdie wrote:
> If mremap() is called without the MREMAP_MAYMOVE flag with a start address
> just before the end of memory (reserved_va) where new_size would exceed 
> it (and GUEST_ADDR_MAX), the assert(end - 1 <= GUEST_ADDR_MAX) in 
> page_set_flags() would trigger.
> 
> Add an extra guard to the guest_range_valid() checks to prevent this and
> avoid asserting binaries when reserved_va is set.
> 
> This meant a bug I was seeing locally now gives the same behaviour 
> regardless of whether reserved_va is set or not.
> 
> Signed-off-by: Richard Purdie  
> Index: qemu-5.2.0/linux-user/mmap.c
> ===
> --- qemu-5.2.0.orig/linux-user/mmap.c
> +++ qemu-5.2.0/linux-user/mmap.c
> @@ -727,7 +727,9 @@ abi_long target_mremap(abi_ulong old_add
>  
> 
>  if (!guest_range_valid(old_addr, old_size) ||
>  ((flags & MREMAP_FIXED) &&
> - !guest_range_valid(new_addr, new_size))) {
> + !guest_range_valid(new_addr, new_size)) ||
> +((flags & MREMAP_MAYMOVE) == 0 &&
> + !guest_range_valid(old_addr, new_size))) {
>  errno = ENOMEM;
>  return -1;
>  }
> 
> 

Any comments on this? I believe its a valid issue that needs fixing and
multiple distros appear to be carrying fixes in this area related to
this.

Cheers,

Richard

Re: [RFC PATCH] linux-user/mmap: Return EFAULT for invalid addresses

[Richard Purdie]

On Fri, 2021-01-08 at 17:46 +, Richard Purdie wrote:
> When using qemu-i386 to run gobject introspection parts of a webkitgtk 
> build using musl as libc on a 64 bit host, it sits in an infinite loop 
> of mremap calls of ever decreasing/increasing addresses.
> 
> I suspect something in the musl memory allocation code loops indefinitely
> if it only sees ENOMEM and only exits when it hits EFAULT.
> 
> According to the docs, trying to mremap outside the address space
> can/should return EFAULT and changing this allows the build to succeed.
> 
> There was previous discussion of this as it used to work before qemu 2.11
> and we've carried hacks to work around it since, this appears to be a
> better fix of the real issue?
> 
> Signed-off-by: Richard Purdie  
> Index: qemu-5.2.0/linux-user/mmap.c
> ===
> --- qemu-5.2.0.orig/linux-user/mmap.c
> +++ qemu-5.2.0/linux-user/mmap.c
> @@ -727,7 +727,7 @@ abi_long target_mremap(abi_ulong old_add
>   !guest_range_valid(new_addr, new_size)) ||
>  ((flags & MREMAP_MAYMOVE) == 0 &&
>   !guest_range_valid(old_addr, new_size))) {
> -errno = ENOMEM;
> +errno = EFAULT;
>  return -1;
>  }

Any comments on this? I believe its a valid issue that needs fixing and
multiple distros appear to be carrying fixes in this area related to
this.

Cheers,

Richard

Re: [PATCH 8/8] configure: automatically parse command line for meson -D options

[Yonggang Luo]

Hi Paolo, as python and meson are required dependencies to building qemu
now,
can we detecting python/meson at the very begining of configure,
even before the --help parameter.

On Wed, Jan 13, 2021 at 6:08 AM Paolo Bonzini  wrote:
>
> On 13/01/21 11:31, Daniel P. Berrangé wrote:
> >>   meson-buildoptions.json | 717

> > I'm not a fan of seeing this file introduced as it has significant
> > overlap with meson_options.txt.I feel like the latter has enough
> > information present to do an acceptable job for help output. After
> > all that's sufficient if we were using meson directly.
>
> Sorry, I missed this remark.  meson-buildoptions.json is not
> hand-written.  It is the result of Meson's own parsing meson_options.txt
> exported as JSON.
>
> In the commit message "because we parse command-line options before
> meson is available, the introspection output is stored in the source
> tree.  This is the reason for the unattractive diffstat; the number of
> JSON lines added is higher than the number of configure lines removed.
> Of course the latter are code that must be maintained manually and the
> former is not".
>
> Paolo
>
>


--
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

Re: [RFC PATCH 2/2] gitlab-ci: Add a job building TCI with Clang

[Thomas Huth]

On 21/01/2021 21.46, Wainer dos Santos Moschetta wrote:


On 1/21/21 3:28 PM, Thomas Huth wrote:

On 21/01/2021 19.13, Daniel P. Berrangé wrote:

On Thu, Jan 21, 2021 at 03:05:43PM -0300, Wainer dos Santos Moschetta wrote:

Hi,

On 1/21/21 7:08 AM, Thomas Huth wrote:

On 10/01/2021 17.27, Philippe Mathieu-Daudé wrote:

Split the current GCC build-tci job in 2, and use Clang
compiler in the new job.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC in case someone have better idea to optimize can respin this patch.

   .gitlab-ci.yml | 22 --
   1 file changed, 20 insertions(+), 2 deletions(-)


I'm not quite sure whether we should go down this road ... if we wanted
to have full test coverage for clang, we'd need to duplicate *all* jobs
to run them once with gcc and once with clang. And that would be just
overkill.


I agree with Thomas.




I think we already catch most clang-related problems with the clang jobs
that we already have in our CI, so problems like the ones that you've
tried to address here should be very, very rare. So I'd rather vote for
not splitting the job here.


We got only one clang job on GitLab CI...

   build-clang:
 <<: *native_build_job_definition
 variables:
   IMAGE: fedora
   CONFIGURE_ARGS: --cc=clang --cxx=clang++
   TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
 ppc-softmmu s390x-softmmu arm-linux-user
   MAKE_CHECK_ARGS: check

... and others on Travis:

   "Clang (user)"

   "Clang (main-softmmu)"

   "Clang (other-softmmu)"


I guess these three overlap partially with the build-clang job.


   "[s390x] Clang (disable-tcg)"


Don't forget the  Cirrus CI jobs for freebsd and macOS will
be using  CLang too.


Right... we should work towards getting cirrus-run into the QEMU-CI, too, 
to finally have these in the gitlab-ci dashboard, too.




So I've some questions:

  * Can we move those first three Travis jobs to Gitlab CI? (I can work on
that)


Yeah, if we move those three travis jobs they can replace the existing
build-clang job. We don't neccesssarily need to keep them as three
separate jobs - that split was just due to the Travis time limits.
If a different split works better on GitLab we can do that.


Well, if we really want to increase the amount clang jobs, one of them 
should likely use TCI, as Phillippe suggested.

Ok, got it. I won't touch on those jobs.


I didn't meant my comment as a "no", but rather as a "needs further 
discussion first" ...


So here's my suggestion:

- Keep the current build-tci job as it is

- Add a build-clang-user job that compiles with clang and
  --disable-system

- Add a "build-clang-system 2" job that compiles with clang
  and --enable-tcg-interpreter and uses more or less the same
  targets as the "build-tci" job. Maybe add the avr-softmmu
  target here now, too, since it now also has a qtest with
  TCG (boot-serial-test)

- Rename the current "build-clang" job to "build-clang-system 1"
  and remove the arm-linux-user target and all the targets that
  are already part of the "build-clang-system 2" / "build-tci"
  from that job. Then add some other softmmu targets to that job
  (but take care that it does not exceed the 1h run time limit,
  so likely not all remaining targets can be added here)

- If we feel confident that we got enough test coverage there
  (together with the clang-based jobs on Cirrus-CI), finally
  remove the three x86 clang jobs from travis.yml.

What do you think? Could you work on such a patch (or patch series), Wainer?

 Thomas

[PATCH 0/2] meson: Try to clarify TCG / TCI options for new users

[Philippe Mathieu-Daudé]

Some new users get confused between 'TCG' and 'TCI' and enable
TCI when TCG is better for they needs. Try to clarify it is
better to not use TCI when native backend is available.

Note, before Meson, warnings were summarized at the end of
./configure. Now they are displayed earlier, and likely
missed IMHO. No clue how to improve that :/

Based-on: <20210121095616.1471869-1-phi...@redhat.com>

Philippe Mathieu-Daudé (2):
  meson: Explicit TCG backend used
  meson: Warn when TCI is selected but TCG backend is available

 meson.build | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.26.2

[PATCH 2/2] meson: Warn when TCI is selected but TCG backend is available

[Philippe Mathieu-Daudé]

Some new users get confused with 'TCG' and 'TCI', and enable TCI
support expecting to enable TCG.

Emit a warning when native TCG backend is available on the
host architecture, mentioning this is a suboptimal configuration.

Signed-off-by: Philippe Mathieu-Daudé 
---
 meson.build | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meson.build b/meson.build
index 0a645e54662..7aa52d306c6 100644
--- a/meson.build
+++ b/meson.build
@@ -234,6 +234,9 @@
   error('Unsupported CPU @0@, try --enable-tcg-interpreter'.format(cpu))
 endif
   endif
+  if 'CONFIG_TCG_INTERPRETER' in config_host and cpu in supported_cpus
+warning('Experimental TCI requested while native TCG is available on @0@, 
suboptimal performance expected'.format(cpu))
+  endif
   accelerators += 'CONFIG_TCG'
   config_host += { 'CONFIG_TCG': 'y' }
 endif
-- 
2.26.2

Re: [PATCHv8 3/3] arm-virt: add secure pl061 for reset/power down

[Andrew Jones]

On Wed, Jan 20, 2021 at 12:27:48PM +0300, Maxim Uvarov wrote:
> Add secure pl061 for reset/power down machine from
> the secure world (Arm Trusted Firmware). Connect it
> with gpio-pwr driver.
> 
> Signed-off-by: Maxim Uvarov 
> ---
>  hw/arm/Kconfig|  1 +
>  hw/arm/virt.c | 47 +++
>  include/hw/arm/virt.h |  2 ++
>  3 files changed, 50 insertions(+)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 0a242e4c5d..13cc42dcc8 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -17,6 +17,7 @@ config ARM_VIRT
>  select PL011 # UART
>  select PL031 # RTC
>  select PL061 # GPIO
> +select GPIO_PWR
>  select PLATFORM_BUS
>  select SMBIOS
>  select VIRTIO_MMIO
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index c427ce5f81..060a5f492e 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = {
>  [VIRT_ACPI_GED] =   { 0x0908, ACPI_GED_EVT_SEL_LEN },
>  [VIRT_NVDIMM_ACPI] ={ 0x0909, NVDIMM_ACPI_IO_LEN},
>  [VIRT_PVTIME] = { 0x090a, 0x0001 },
> +[VIRT_SECURE_GPIO] ={ 0x090b, 0x1000 },
>  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
>  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size 
> */
>  [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
> @@ -841,6 +842,43 @@ static void create_gpio_keys(const VirtMachineState *vms,
> "gpios", phandle, 3, 0);
>  }
>  
> +#define SECURE_GPIO_POWEROFF 0
> +#define SECURE_GPIO_REBOOT   1
> +
> +static void create_gpio_pwr(const VirtMachineState *vms,

This function is specific to the secure view. I think it should have
"secure" in its name.

> +DeviceState *pl061_dev,
> +uint32_t phandle)
> +{
> +DeviceState *gpio_pwr_dev;
> +
> +/* gpio-pwr */
> +gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL);

Should this device be in secure memory?

> +
> +/* connect secure pl061 to gpio-pwr */
> +qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_REBOOT,
> +  qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
> +qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_POWEROFF,
> +  qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 
> 0));
> +
> +qemu_fdt_add_subnode(vms->fdt, "/gpio-poweroff");
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "compatible",
> +"gpio-poweroff");
> +qemu_fdt_setprop_cells(vms->fdt, "/gpio-poweroff",
> +   "gpios", phandle, SECURE_GPIO_POWEROFF, 0);
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "status", 
> "disabled");
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "secure-status",
> +"okay");
> +
> +qemu_fdt_add_subnode(vms->fdt, "/gpio-restart");
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "compatible",
> +"gpio-restart");
> +qemu_fdt_setprop_cells(vms->fdt, "/gpio-restart",
> +   "gpios", phandle, SECURE_GPIO_REBOOT, 0);
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "status", "disabled");
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "secure-status",
> +"okay");
> +}
> +
>  static void create_gpio_devices(const VirtMachineState *vms, int gpio,
>  MemoryRegion *mem)
>  {
> @@ -883,6 +921,8 @@ static void create_gpio_devices(const VirtMachineState 
> *vms, int gpio,
>  /* Child gpio devices */
>  if (gpio == VIRT_GPIO) {
>  create_gpio_keys(vms, pl061_dev, phandle);
> +} else {
> +create_gpio_pwr(vms, pl061_dev, phandle);
>  }
>  }
>  
> @@ -2015,6 +2055,10 @@ static void machvirt_init(MachineState *machine)
>  create_gpio_devices(vms, VIRT_GPIO, sysmem);
>  }
>  
> +if (vms->secure && !vmc->no_secure_gpio) {
> +create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem);
> +}
> +
>   /* connect powerdown request */
>   vms->powerdown_notifier.notify = virt_powerdown_req;
>   qemu_register_powerdown_notifier(>powerdown_notifier);
> @@ -2630,8 +2674,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 0)
>  
>  static void virt_machine_5_2_options(MachineClass *mc)
>  {
> +VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>  virt_machine_6_0_options(mc);
>  compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
> +vmc->no_secure_gpio = true;
>  }
>  DEFINE_VIRT_MACHINE(5, 2)
>  
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index abf54fab49..6f6c85ffcf 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -81,6 +81,7 @@ enum {
>  VIRT_GPIO,
>  VIRT_SECURE_UART,
>  VIRT_SECURE_MEM,
> +VIRT_SECURE_GPIO,
>  

Re: [PULL 10/11] vnc: move initialization to framebuffer_update_request

[Gerd Hoffmann]

> This patch breaks QEMU for me.

> The symptom is the following: in virt-manager, the display remains dead
> (black), when I start an OVMF guest. At the same time, unusually high
> CPU load can be seen on the host; it makes me think that virt-manager is
> trying, in a busy loop, to complete the VNC handshake, or some such.
> Ultimately, although the guest boots up fine, virt-manager gives up, and
> the display window says "Viewer was disconnected".

It is the vnc_colordepth() call. Seems gtk-vnc sends a update request
with incremental=0 as response to the VNC_ENCODING_WMVi message.  So
sending that as response to an incremental=0 update request creates an
endless loop ...

take care,
  Gerd

diff --git a/ui/vnc.c b/ui/vnc.c
index d429bfee5a65..0a3e2f4aa98c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2041,7 +2041,6 @@ static void framebuffer_update_request(VncState *vs, int 
incremental,
 } else {
 vs->update = VNC_STATE_UPDATE_FORCE;
 vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
-vnc_colordepth(vs);
 vnc_desktop_resize(vs);
 vnc_led_state_change(vs);
 vnc_cursor_define(vs);

[PATCH] vnc: drop vnc_colordepth() call.

[Gerd Hoffmann]

With gtk-vnc (which supports VNC_FEATURE_WMVI) this results in
sending a VNC_ENCODING_WMVi message to the client.  Which in turn
seems to make gtk-vnc respond with another non-incremental update
request.  Hello endless loop ...

Drop the call.

Fixes: 9e1632ad07ca ("vnc: move initialization to framebuffer_update_request")
Reported-by: Laszlo Ersek 
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index d429bfee5a65..0a3e2f4aa98c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2041,7 +2041,6 @@ static void framebuffer_update_request(VncState *vs, int 
incremental,
 } else {
 vs->update = VNC_STATE_UPDATE_FORCE;
 vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
-vnc_colordepth(vs);
 vnc_desktop_resize(vs);
 vnc_led_state_change(vs);
 vnc_cursor_define(vs);
-- 
2.29.2

Re: [PATCH 1/2] meson: Explicit TCG backend used

[Thomas Huth]

On 22/01/2021 10.22, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  meson.build | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 8535a83fb70..0a645e54662 100644
--- a/meson.build
+++ b/meson.build
@@ -2419,8 +2419,12 @@
  endif
  summary_info += {'TCG support':   config_all.has_key('CONFIG_TCG')}
  if config_all.has_key('CONFIG_TCG')
+  if config_host.has_key('CONFIG_TCG_INTERPRETER')
+summary_info += {'TCG backend':   'TCG interpreter (experimental)'}


maybe say "experimental and slow" in the parentheses?


+  else
+summary_info += {'TCG backend':   'native (@0@)'.format(cpu)}
+  endif
summary_info += {'TCG debug enabled': 
config_host.has_key('CONFIG_DEBUG_TCG')}
-  summary_info += {'TCG interpreter':   
config_host.has_key('CONFIG_TCG_INTERPRETER')}
  endif
  summary_info += {'target list':   ' '.join(target_dirs)}
  if have_system



Reviewed-by: Thomas Huth 

Re: [PATCH 2/2] meson: Warn when TCI is selected but TCG backend is available

[Thomas Huth]

On 22/01/2021 10.22, Philippe Mathieu-Daudé wrote:

Some new users get confused with 'TCG' and 'TCI', and enable TCI
support expecting to enable TCG.

Emit a warning when native TCG backend is available on the
host architecture, mentioning this is a suboptimal configuration.

Signed-off-by: Philippe Mathieu-Daudé 
---
  meson.build | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/meson.build b/meson.build
index 0a645e54662..7aa52d306c6 100644
--- a/meson.build
+++ b/meson.build
@@ -234,6 +234,9 @@
error('Unsupported CPU @0@, try --enable-tcg-interpreter'.format(cpu))
  endif
endif
+  if 'CONFIG_TCG_INTERPRETER' in config_host and cpu in supported_cpus
+warning('Experimental TCI requested while native TCG is available on @0@, 
suboptimal performance expected'.format(cpu))
+  endif
accelerators += 'CONFIG_TCG'
config_host += { 'CONFIG_TCG': 'y' }
  endif



Reviewed-by: Thomas Huth 

... maybe you could also add some wording to the help text of the configure 
script? Or maybe we could simply rename the "--enable-tcg-interpreter" 
option into "--enable-tci" to avoid confusion with the normal TCG ?

Re: [PATCHv8 2/3] arm-virt: refactor gpios creation

[Andrew Jones]

On Wed, Jan 20, 2021 at 12:27:47PM +0300, Maxim Uvarov wrote:
> No functional change. Just refactor code to better
> support secure and normal world gpios.
> 
> Signed-off-by: Maxim Uvarov 
> ---
>  hw/arm/virt.c | 64 ++-
>  1 file changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 96985917d3..c427ce5f81 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -820,17 +820,43 @@ static void virt_powerdown_req(Notifier *n, void 
> *opaque)
>  }
>  }
>  
> -static void create_gpio(const VirtMachineState *vms)
> +static void create_gpio_keys(const VirtMachineState *vms,
> + DeviceState *pl061_dev,
> + uint32_t phandle)
> +{
> +gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> +qdev_get_gpio_in(pl061_dev, 3));
> +
> +qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", 
> "gpio-keys");
> +qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
> +qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
> +
> +qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
> +qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
> +"label", "GPIO Key Poweroff");
> +qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
> +  KEY_POWER);
> +qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
> +   "gpios", phandle, 3, 0);
> +}
> +
> +static void create_gpio_devices(const VirtMachineState *vms, int gpio,
> +MemoryRegion *mem)
>  {
>  char *nodename;
>  DeviceState *pl061_dev;
> -hwaddr base = vms->memmap[VIRT_GPIO].base;
> -hwaddr size = vms->memmap[VIRT_GPIO].size;
> -int irq = vms->irqmap[VIRT_GPIO];
> +hwaddr base = vms->memmap[gpio].base;
> +hwaddr size = vms->memmap[gpio].size;
> +int irq = vms->irqmap[gpio];
>  const char compat[] = "arm,pl061\0arm,primecell";
> +SysBusDevice *s;
>  
> -pl061_dev = sysbus_create_simple("pl061", base,
> - qdev_get_gpio_in(vms->gic, irq));
> +pl061_dev = qdev_new("pl061");
> +s = SYS_BUS_DEVICE(pl061_dev);
> +sysbus_realize_and_unref(s, _fatal);
> +memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0));
> +sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
>  
>  uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt);
>  nodename = g_strdup_printf("/pl061@%" PRIx64, base);
> @@ -847,21 +873,17 @@ static void create_gpio(const VirtMachineState *vms)
>  qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk");
>  qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
>  
> -gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> -qdev_get_gpio_in(pl061_dev, 3));
> -qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
> -qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", 
> "gpio-keys");
> -qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
> -qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
> -
> -qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
> -qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
> -"label", "GPIO Key Poweroff");
> -qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
> -  KEY_POWER);
> -qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
> -   "gpios", phandle, 3, 0);
> +if (gpio != VIRT_GPIO) {
> +/* Mark as not usable by the normal world */
> +qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled");
> +qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay");
> +}

nit: The above if-block could/should have waited until the next patch to
be added.

>  g_free(nodename);
> +
> +/* Child gpio devices */
> +if (gpio == VIRT_GPIO) {

Same nit as above, the next patch is where we should start conditionally
doing stuff based on the gpio type.

> +create_gpio_keys(vms, pl061_dev, phandle);
> +}
>  }
>  
>  static void create_virtio_devices(const VirtMachineState *vms)
> @@ -1990,7 +2012,7 @@ static void machvirt_init(MachineState *machine)
>  if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
>  vms->acpi_dev = create_acpi_ged(vms);
>  } else {
> -create_gpio(vms);
> +create_gpio_devices(vms, VIRT_GPIO, sysmem);
>  }
>  
>   /* connect powerdown request */
> -- 
> 2.17.1
> 
>

Reviewed-by: Andrew Jones 

Thanks,
drew

Re: [PATCH 04/25] keyval: accept escaped commas in implied option

[Markus Armbruster]

Paolo Bonzini  writes:

[...]
> diff --git a/util/keyval.c b/util/keyval.c
> index be34928813..eb9b9c55ec 100644
> --- a/util/keyval.c
> +++ b/util/keyval.c
> @@ -16,8 +16,8 @@
>   *   key-vals = [ key-val { ',' key-val } [ ',' ] ]
>   *   key-val  = key '=' val | help
>   *   key  = key-fragment { '.' key-fragment }
> - *   key-fragment = / [^=,.]+ /
> - *   val  = { / [^,]+ / | ',,' }
> + *   key-fragment = { / [^=,.] / | ',,' }
> + *   val  = { / [^,] / | ',,' }
>   *   help = 'help' | '?'
>   *
>   * Semantics defined by reduction to JSON:
> @@ -78,13 +78,13 @@
>   * Alternative syntax for use with an implied key:
>   *
>   *   key-vals = [ key-val-1st { ',' key-val } [ ',' ] ]
> - *   key-val-1st  = val-no-key | key-val
> - *   val-no-key   = / [^=,]+ / - help
> + *   key-val-1st  = (val-no-key - help) | key-val
> + *   val-no-key   = { / [^=,] / | ',,' }

I finally remembered why I made val-no-key non-empty: to avoid amiguity.

Before the patch, "" can only be parsed as empty key-vals.  Results in
an empty QDict.

Afterwards, the grammar is ambiguous: "" can also be parsed as empty
val-no-key, reduced via key-val-1st to non-empty key-vals.  Results in a
QDict with one entry mapping the implied key to "".

I'm a bit concerned I similarly forgot something that made me avoid ',,'
escapes in val-no-key.

Even if we brushed off the ambiguous grammar issue (and we should not!),
desugaring "" into "implied=" feels unwise, and ",k=v" into
"implied=,k=v" only slightly less so.

Let's keep val-no-key non-empty.

Ripple effect...  I made val-no-key match key (almost):

val-no-key   = / [^=,]+ /

key  = key-fragment { '.' key-fragment }
key-fragment = / [^=,.]+ /

The only difference is val-no-key accepts consecutive '.'.

Commit 8bf12c4f75 "keyval: Parse help options" muddied the waters a bit
by adding '- help' to val-no-key.

Your commit moves it to key-val-1st (good).  It also permits empty
key-fragment, and thus consecutive '.' (good because it makes val-no-key
match key exactly, but also possibly confusing because key-fragment
can't actually be empty due to the "Key-fragments must be valid QAPI
names or consist only of decimal digits" condition).  Okay.

It also changed both val-no-key and key to accept empty.  We need to
keep *both* non-empty.

Your change to val is not wrong, but I prefer the old version, because
it's closer to how the code works.

>   *
>   * where val-no-key is syntactic sugar for implied-key=val-no-key.
>   *
> - * Note that you can't use the sugared form when the value contains
> - * '=' or ','.
> + * Note that you can't use the sugared form when the value is empty

You can with your grammar change, unless the code doesn't match the
grammar.  Which would be a bug.

> + * or contains '='.
>   */
[...]

I apologize for sitting on this patch for so long.  Something felt
wrong, but I couldn't put a finger on it.  Now I can at least for the
empty val-no-key part.

Re: Thread safety of coroutine-sigaltstack

[Max Reitz]

On 20.01.21 18:25, Laszlo Ersek wrote:

[...]


A simple grep for SIGUSR2 seems to indicate that SIGUSR2 is not used by
system emulation for anything else, in practice. Is it possible to
dedicate SIGUSR2 explicitly to coroutine-sigaltstack, and set up the
action beforehand, from some init function that executes on a "central"
thread, before qemu_coroutine_new() is ever called?


I wrote a patch to that effect, but just before sending I wondered 
whether SIGUSR2 cannot be registered by the “guest” in user-mode 
emulation, and whether that would then break coroutines from there on.


(I have no experience dealing with user-mode emulation, but it does look 
like the guest can just register handlers for any signal but SIGSEGV and 
SIGBUS.)


Max

[PATCH 1/2] meson: Explicit TCG backend used

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 meson.build | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 8535a83fb70..0a645e54662 100644
--- a/meson.build
+++ b/meson.build
@@ -2419,8 +2419,12 @@
 endif
 summary_info += {'TCG support':   config_all.has_key('CONFIG_TCG')}
 if config_all.has_key('CONFIG_TCG')
+  if config_host.has_key('CONFIG_TCG_INTERPRETER')
+summary_info += {'TCG backend':   'TCG interpreter (experimental)'}
+  else
+summary_info += {'TCG backend':   'native (@0@)'.format(cpu)}
+  endif
   summary_info += {'TCG debug enabled': 
config_host.has_key('CONFIG_DEBUG_TCG')}
-  summary_info += {'TCG interpreter':   
config_host.has_key('CONFIG_TCG_INTERPRETER')}
 endif
 summary_info += {'target list':   ' '.join(target_dirs)}
 if have_system
-- 
2.26.2

Re: [PATCH 2/2] meson: Warn when TCI is selected but TCG backend is available

[Thomas Huth]

On 22/01/2021 10.46, Philippe Mathieu-Daudé wrote:

On Fri, Jan 22, 2021 at 10:43 AM Thomas Huth  wrote:

On 22/01/2021 10.22, Philippe Mathieu-Daudé wrote:

Some new users get confused with 'TCG' and 'TCI', and enable TCI
support expecting to enable TCG.

Emit a warning when native TCG backend is available on the
host architecture, mentioning this is a suboptimal configuration.

Signed-off-by: Philippe Mathieu-Daudé 
---
   meson.build | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/meson.build b/meson.build
index 0a645e54662..7aa52d306c6 100644
--- a/meson.build
+++ b/meson.build
@@ -234,6 +234,9 @@
 error('Unsupported CPU @0@, try --enable-tcg-interpreter'.format(cpu))
   endif
 endif
+  if 'CONFIG_TCG_INTERPRETER' in config_host and cpu in supported_cpus
+warning('Experimental TCI requested while native TCG is available on @0@, 
suboptimal performance expected'.format(cpu))
+  endif
 accelerators += 'CONFIG_TCG'
 config_host += { 'CONFIG_TCG': 'y' }
   endif



Reviewed-by: Thomas Huth 

... maybe you could also add some wording to the help text of the configure
script? Or maybe we could simply rename the "--enable-tcg-interpreter"
option into "--enable-tci" to avoid confusion with the normal TCG ?


I also thought about renaming as --enable-experimental-tci but then doubt that
would help, some users just want to enable everything :)


Then we should rename it into --disable-native-tcg ;-)

 Thomas

Re: [PATCH 2/2] meson: Warn when TCI is selected but TCG backend is available

[Philippe Mathieu-Daudé]

On Fri, Jan 22, 2021 at 10:43 AM Thomas Huth  wrote:
> On 22/01/2021 10.22, Philippe Mathieu-Daudé wrote:
> > Some new users get confused with 'TCG' and 'TCI', and enable TCI
> > support expecting to enable TCG.
> >
> > Emit a warning when native TCG backend is available on the
> > host architecture, mentioning this is a suboptimal configuration.
> >
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> >   meson.build | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/meson.build b/meson.build
> > index 0a645e54662..7aa52d306c6 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -234,6 +234,9 @@
> > error('Unsupported CPU @0@, try 
> > --enable-tcg-interpreter'.format(cpu))
> >   endif
> > endif
> > +  if 'CONFIG_TCG_INTERPRETER' in config_host and cpu in supported_cpus
> > +warning('Experimental TCI requested while native TCG is available on 
> > @0@, suboptimal performance expected'.format(cpu))
> > +  endif
> > accelerators += 'CONFIG_TCG'
> > config_host += { 'CONFIG_TCG': 'y' }
> >   endif
> >
>
> Reviewed-by: Thomas Huth 
>
> ... maybe you could also add some wording to the help text of the configure
> script? Or maybe we could simply rename the "--enable-tcg-interpreter"
> option into "--enable-tci" to avoid confusion with the normal TCG ?

I also thought about renaming as --enable-experimental-tci but then doubt that
would help, some users just want to enable everything :)

Re: Thread safety of coroutine-sigaltstack

[Max Reitz]

On 22.01.21 11:14, Peter Maydell wrote:

On Fri, 22 Jan 2021 at 08:50, Max Reitz  wrote:


On 20.01.21 18:25, Laszlo Ersek wrote:

[...]


A simple grep for SIGUSR2 seems to indicate that SIGUSR2 is not used by
system emulation for anything else, in practice. Is it possible to
dedicate SIGUSR2 explicitly to coroutine-sigaltstack, and set up the
action beforehand, from some init function that executes on a "central"
thread, before qemu_coroutine_new() is ever called?


I wrote a patch to that effect, but just before sending I wondered
whether SIGUSR2 cannot be registered by the “guest” in user-mode
emulation, and whether that would then break coroutines from there on.

(I have no experience dealing with user-mode emulation, but it does look
like the guest can just register handlers for any signal but SIGSEGV and
SIGBUS.)


Yes, SIGUSR2 is for the guest in user-emulation mode. OTOH do we
even use the coroutine code in user-emulation mode? Looking at
the meson.build files, we only add the coroutine_*.c to util_ss
if 'have_block', and we set have_block = have_system or have_tools.
I think (but have not checked) that that means we will build and
link the object file into the user-mode binaries if you happen
to build them in the same run as system-mode binaries, but won't
build them in if you built the user-mode binaries as a separate
build. Which is odd and probably worth fixing, but does mean we
know that we aren't actually using coroutines in user-mode.
(Also user-mode really means Linux or BSD and I think both of
those have working ucontext.)


OK, great.  Thanks for looking that up.

Max

Re: [PATCH] util/log: flush TB cache when log level changes

[Philippe Mathieu-Daudé]

Hi Pavel,

On 1/22/21 11:03 AM, Pavel Dovgalyuk wrote:
> Sometimes we need to collect the translation logs starting
> from some point of the execution. Some TB listings may
> be missed in this case, when blocks were translated before.
> This patch clears TB cache to allow re-translation of such
> code blocks.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  accel/tcg/translate-all.c |8 
>  include/sysemu/tcg.h  |1 +
>  stubs/meson.build |1 +
>  stubs/tcg.c   |   12 
>  util/log.c|3 +++
>  5 files changed, 25 insertions(+)
>  create mode 100644 stubs/tcg.c
...

>  /*
>   * Formerly ifdef DEBUG_TB_CHECK. These debug functions are user-mode-only,
>   * so in order to prevent bit rot we compile them unconditionally in 
> user-mode,
> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
> index 00349fb18a..7415f11022 100644
> --- a/include/sysemu/tcg.h
> +++ b/include/sysemu/tcg.h
> @@ -9,6 +9,7 @@
>  #define SYSEMU_TCG_H
>  
>  void tcg_exec_init(unsigned long tb_size, int splitwx);
> +void tb_flush_all(void);

Why not declare in "exec/exec-all.h"?

Re: [PATCH v2 2/8] nbd: allow reconnect on open, with corresponding new options

[Vladimir Sementsov-Ogievskiy]

21.01.2021 04:44, Eric Blake wrote:

On 11/30/20 7:40 AM, Vladimir Sementsov-Ogievskiy wrote:

Note: currently, using new option with long timeout in qmp command
blockdev-add is not good idea, as qmp interface is blocking, so,
don't add it now, let's add it later after
"monitor: Optionally run handlers in coroutines" series merged.


If I'm not mistaken, that landed as of eb94b81a94.  Is it just the
commit message that needs an update, or does this patch need a respin?


Oh yes, you are right. I think the most reasonable thing is to keep this patch
in separate (for simple backporting to downstream without Kevin's series), and
add qmp support for the feature as additional new patch. Will do it on respin.





Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block/nbd.c | 115 +---
  1 file changed, 92 insertions(+), 23 deletions(-)




@@ -474,6 +484,11 @@ nbd_co_establish_connection(BlockDriverState *bs, Error 
**errp)
  s->wait_connect = true;
  qemu_coroutine_yield();
  
+if (!s->connect_thread) {

+error_setg(errp, "Connection attempt cancelled by other operation");
+return NULL;
+}


Does this need to use atomics for proper access to s->connect_thread
across threads?  Or are all the operations done by other coroutines but
within the same thread, so we are safe?


s->connect_thread is not accessed from connect_thread_func, so in this way we 
are safe. And variables shared between connect_thread_func and other driver code 
are protected by mutex.

What about accessing nbd bds from different threads.. In my observation, all the 
code is written in assumption that everything inside block-driver may be called 
from different coroutines but from one thread.. And we have a lot of s->* 
variables that are not atomic and not protected by mutexes, and all this works 
somehow:)

I remember Paolo answered me somewhere in mailing list, that actually, 
everything in block drivers and block/io must be thread-safe.. But I don't see 
this thread-safety in current code, so don't introduce it for new variables.





@@ -624,10 +645,15 @@ static coroutine_fn void 
nbd_reconnect_attempt(BDRVNBDState *s)
  bdrv_inc_in_flight(s->bs);
  
  out:

-s->connect_status = ret;
-error_free(s->connect_err);
-s->connect_err = NULL;
-error_propagate(>connect_err, local_err);
+if (s->connect_status == -ETIMEDOUT) {
+/* Don't rewrite timeout error by following cancel-provoked error */


Maybe:

/* Don't propagate a timeout error caused by a job cancellation. */


No, we want to keep ETIMEOUT





+static void open_timer_cb(void *opaque)
+{
+BDRVNBDState *s = opaque;
+
+if (!s->connect_status) {
+/* First attempt was not finished. We should set an error */
+s->connect_status = -ETIMEDOUT;
+error_setg(>connect_err, "First connection attempt is cancelled by "
+   "timeout");
+}
+
+nbd_teardown_connection_async(s->bs);
+open_timer_del(s);
+}
+
+static void open_timer_init(BDRVNBDState *s, uint64_t expire_time_ns)
+{
+assert(!s->open_timer && s->state == NBD_CLIENT_OPENING);
+s->open_timer = aio_timer_new(bdrv_get_aio_context(s->bs),
+  QEMU_CLOCK_REALTIME,
+  SCALE_NS,
+  open_timer_cb, s);
+timer_mod(s->open_timer, expire_time_ns);
+}
+




@@ -2180,6 +2235,14 @@ static QemuOptsList nbd_runtime_opts = {
  "future requests before a successful reconnect will "
  "immediately fail. Default 0",
  },
+{
+.name = "open-timeout",
+.type = QEMU_OPT_NUMBER,
+.help = "In seconds. If zero, nbd driver tries to establish "
+"connection only once, on fail open fails. If non-zero, "


If zero, the nbd driver tries the connection only once, and fails to
open if the connection fails.


+"nbd driver may do several attempts until success or "
+"@open-timeout seconds passed. Default 0",


If non-zero, the nbd driver will repeat connection attempts until
successful or until @open-timeout seconds have elapsed.


+},


Where is the QMP counterpart for setting this option?


Absent (as described in commit msg). Will do in a separate patch.




  { /* end of list */ }
  },
  };
@@ -2235,6 +2298,7 @@ static int nbd_process_options(BlockDriverState *bs, 
QDict *options,
  }
  
  s->reconnect_delay = qemu_opt_get_number(opts, "reconnect-delay", 0);

+s->open_timeout = qemu_opt_get_number(opts, "open-timeout", 0);
  
  ret = 0;
  
@@ -2268,6 +2332,11 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,

  bdrv_inc_in_flight(bs);
  aio_co_schedule(bdrv_get_aio_context(bs), s->connection_co);
  
+if (s->open_timeout) {

+open_timer_init(s, 

Re: [Qemu-devel] [PATCH v2 07/11] chardev: Let IOReadHandler use unsigned type

[Richard Purdie]

On Fri, 2018-10-12 at 02:22 +0200, Philippe Mathieu-Daudé wrote:
> The number of bytes can not be negative nor zero.
> 
> Fixed 2 format string:
> - hw/char/spapr_vty.c
> - hw/usb/ccid-card-passthru.c
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Philippe Mathieu-Daudé 
> Acked-by: Alberto Garcia 

Sorry to drag up an old patch series. As far as I can see this series
was never applied. I suspect a better way of solving the issue may have
been found? If so can anyone point me at that change?

I ask since CVE-2018-18438 is marked as affecting all qemu versions
(https://nvd.nist.gov/vuln/detail/CVE-2018-18438).

If it was fixed, the version mask could be updated. If the fix wasn't
deemed worthwhile for some reason that is also fine and I can mark this
one as such in our system. I'm being told we only need one of the
patches in this series which I also don't believe as I suspect we
either need the set or none of them!

Any info would be most welcome.

Cheers,

Richard

Re: [PATCH v2 06/36] block: BdrvChildClass: add .get_parent_aio_context handler

[Vladimir Sementsov-Ogievskiy]

22.01.2021 14:18, Kevin Wolf wrote:

Am 22.01.2021 um 12:04 hat Vladimir Sementsov-Ogievskiy geschrieben:

19.01.2021 19:38, Kevin Wolf wrote:

Am 18.01.2021 um 18:36 hat Vladimir Sementsov-Ogievskiy geschrieben:

18.01.2021 18:13, Kevin Wolf wrote:

Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add new handler to get aio context and implement it in all child
classes. Add corresponding public interface to be used soon.

Signed-off-by: Vladimir Sementsov-Ogievskiy 


Hm, are you going to introduce cases where parent and child context
don't match, or why is this a useful function?

Even if so, I feel it shouldn't be any of the child's business what
AioContext the parent uses.

Well, maybe the rest of the series will answer this.


It's for the following patch, to not pass parent (as opaque) with it's
class, and with its ctx in separate. Just to simplify the interface of
the function, we are going to work with a lot.


In a way, the result is nicer because we already assume that ctx is the
parent context when we move things to different AioContexts. So it's
more consistent to just directly take it from the parent.

At the same time, it also complicates things a bit because now we need a
defined state in the middle of an operation that adds, removes or
replaces a child.

I guess I still to make more progress in the review of this series until
I see how you're using the interface.


Hm. I'll take this opportunity to say, that the terminology that calls
graph edge "BdrvChild" always leads to the mess between parents and
children.. "child_class" is a class of parent.. list of parents is
list of children... It all would be a lot simpler to understand if
BdrvChild was BdrvEdge or something like this.


Yeah, that would probably be clearer now that we actually use it to
work with both ends of the edge. And BdrvNode instead of
BlockDriverState, I guess.


Do you think, we can just rename them? Or it would be too painful for 
developers,
who get used to current names? I can make patches


I think getting used to new names wouldn't be too bad. I would be more
afraid of the merge conflicts.

Not sure, but it might in the category that we would do it differently
if we were starting over, but maybe not worth changing now.



Hmm yes, such rename will add a year of uncomfortable patch backporting..


--
Best regards,
Vladimir

Re: [RFC PATCH v2 4/4] configure: Reword --enable-tcg-interpreter as --disable-native-tcg

[Thomas Huth]

On 22/01/2021 11.58, Philippe Mathieu-Daudé wrote:

Users might want to enable all features, without realizing some
features have negative effect. Rename '--enable-tcg-interpreter'
as '--disable-native-tcg' to avoid user selecting this feature
without understanding it. '--enable-tcg-interpreter' is kept in
for backward compability with scripts.

Suggested-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
RFC so it can be discarded from the series

  configure | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 71bdc523aa0..5e56fa76499 100755
--- a/configure
+++ b/configure
@@ -1121,7 +1121,8 @@ for opt do
;;
--disable-tcg-interpreter) tcg_interpreter="no"
;;
-  --enable-tcg-interpreter) tcg_interpreter="yes"
+  --enable-tcg-interpreter) # backward compatibility
+  --disable-native-tcg) tcg_interpreter="yes"
;;
--disable-cap-ng)  cap_ng="disabled"
;;
@@ -1753,7 +1754,7 @@ Advanced options (experts only):
--with-trace-file=NAME   Full PATH,NAME of file to store traces
 Default:trace-
--disable-slirp  disable SLIRP userspace network connectivity
-  --enable-tcg-interpreter enable TCG with bytecode interpreter (experimental 
and slow)
+  --disable-native-tcg enable TCG with bytecode interpreter (experimental 
and slow)


The more I think about it, the more I like the idea.

Reviewed-by: Thomas Huth 

Re: [PATCH v7 07/11] iotests: add findtests.py

[Kevin Wolf]

Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> +if 'disabled' not in groups and 'disabled' not in exclude_groups:
> +# Don't want to modify function argument, so create new list.
> +exclude_groups = exclude_groups + ['disabled']

Oops, forgot the other comment I wanted to make:

This would only have been needed if you had turned exclude_groups into a
Sequence. Now that it's still a list, copying the list isn't strictly
necessary.

Kevin

Re: [PATCH v3 12/21] linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE

[Peter Maydell]

On Fri, 22 Jan 2021 at 11:53, Peter Maydell  wrote:
> The kernel's "untagged_addr()" implementation:
> https://elixir.bootlin.com/linux/latest/source/arch/arm64/include/asm/memory.h#L203
> slightly confusingly does "untag the addr if it's in the userspace
> half, leave the tag bits alone if in the kernel half".

...and a kernel person has just explained to me the rationale:
TBI is always enabled for userspace and never for the kernel,
so "always clear tag bits for a userspace address, never clear
them for a kernel address" is the right behaviour. I think we
should have the same logic.

-- PMM

[PATCH V5 4/6] hw/block/nvme: support for multi-controller in subsystem

[Minwoo Im]

We have nvme-subsys and nvme devices mapped together.  To support
multi-controller scheme to this setup, controller identifier(id) has to
be managed.  Earlier, cntlid(controller id) used to be always 0 because
we didn't have any subsystem scheme that controller id matters.

This patch introduced 'cntlid' attribute to the nvme controller
instance(NvmeCtrl) and make it allocated by the nvme-subsys device
mapped to the controller.  If nvme-subsys is not given to the
controller, then it will always be 0 as it was.

Added 'ctrls' array in the nvme-subsys instance to manage attached
controllers to the subsystem with a limit(32).  This patch didn't take
list for the controllers to make it seamless with nvme-ns device.

Signed-off-by: Minwoo Im 
---
 hw/block/nvme-subsys.c | 21 +
 hw/block/nvme-subsys.h |  4 
 hw/block/nvme.c| 29 +
 hw/block/nvme.h|  1 +
 4 files changed, 55 insertions(+)

diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index aa82911b951c..e9d61c993c90 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -22,6 +22,27 @@
 #include "nvme.h"
 #include "nvme-subsys.h"
 
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
+{
+NvmeSubsystem *subsys = n->subsys;
+int cntlid;
+
+for (cntlid = 0; cntlid < ARRAY_SIZE(subsys->ctrls); cntlid++) {
+if (!subsys->ctrls[cntlid]) {
+break;
+}
+}
+
+if (cntlid == ARRAY_SIZE(subsys->ctrls)) {
+error_setg(errp, "no more free controller id");
+return -1;
+}
+
+subsys->ctrls[cntlid] = n;
+
+return cntlid;
+}
+
 static void nvme_subsys_setup(NvmeSubsystem *subsys)
 {
 snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 40f06a4c7db0..4eba50d96a1d 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -20,6 +20,10 @@ typedef struct NvmeNamespace NvmeNamespace;
 typedef struct NvmeSubsystem {
 DeviceState parent_obj;
 uint8_t subnqn[256];
+
+NvmeCtrl*ctrls[NVME_SUBSYS_MAX_CTRLS];
 } NvmeSubsystem;
 
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
+
 #endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index b525fca14103..3dedefb8ebba 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4435,6 +4435,9 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
 strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
 strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
+
+id->cntlid = n->cntlid;
+
 id->rab = 6;
 id->ieee[0] = 0x00;
 id->ieee[1] = 0x02;
@@ -4481,6 +4484,10 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 id->psd[0].enlat = cpu_to_le32(0x10);
 id->psd[0].exlat = cpu_to_le32(0x4);
 
+if (n->subsys) {
+id->cmic |= NVME_CMIC_MULTI_CTRL;
+}
+
 NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
 NVME_CAP_SET_CQR(n->bar.cap, 1);
 NVME_CAP_SET_TO(n->bar.cap, 0xf);
@@ -4495,6 +4502,24 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 n->bar.intmc = n->bar.intms = 0;
 }
 
+static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
+{
+int cntlid;
+
+if (!n->subsys) {
+return 0;
+}
+
+cntlid = nvme_subsys_register_ctrl(n, errp);
+if (cntlid < 0) {
+return -1;
+}
+
+n->cntlid = cntlid;
+
+return 0;
+}
+
 static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 {
 NvmeCtrl *n = NVME(pci_dev);
@@ -4515,6 +4540,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
 return;
 }
 
+if (nvme_init_subsys(n, errp)) {
+error_propagate(errp, local_err);
+return;
+}
 nvme_init_ctrl(n, pci_dev);
 
 /* setup a namespace if the controller drive property was given */
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 04d4684601fd..b8f5f2d6ffb8 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -134,6 +134,7 @@ typedef struct NvmeCtrl {
 NvmeBus  bus;
 BlockConfconf;
 
+uint16_tcntlid;
 boolqs_created;
 uint32_tpage_size;
 uint16_tpage_bits;
-- 
2.17.1

[PATCH V5 6/6] hw/block/nvme: support for shared namespace in subsystem

[Minwoo Im]

nvme-ns device is registered to a nvme controller device during the
initialization in nvme_register_namespace() in case that 'bus' property
is given which means it's mapped to a single controller.

This patch introduced a new property 'subsys' just like the controller
device instance did to map a namespace to a NVMe subsystem.

If 'subsys' property is given to the nvme-ns device, it will belong to
the specified subsystem and will be attached to all controllers in that
subsystem by enabling shared namespace capability in NMIC(Namespace
Multi-path I/O and Namespace Capabilities) in Identify Namespace.

Usage:

  -device nvme-subsys,id=subsys0
  -device nvme,serial=foo,id=nvme0,subsys=subsys0
  -device nvme,serial=bar,id=nvme1,subsys=subsys0
  -device nvme,serial=baz,id=nvme2,subsys=subsys0
  -device nvme-ns,id=ns1,drive=,nsid=1,subsys=subsys0  # Shared
  -device nvme-ns,id=ns2,drive=,nsid=2,bus=nvme2   # Non-shared

  In the above example, 'ns1' will be shared to 'nvme0' and 'nvme1' in
  the same subsystem.  On the other hand, 'ns2' will be attached to the
  'nvme2' only as a private namespace in that subsystem.

All the namespace with 'subsys' parameter will attach all controllers in
the subsystem to the namespace by default.

Signed-off-by: Minwoo Im 
---
 hw/block/nvme-ns.c | 23 ++-
 hw/block/nvme-ns.h |  7 +++
 hw/block/nvme-subsys.c | 25 +
 hw/block/nvme-subsys.h |  3 +++
 hw/block/nvme.c| 10 +-
 5 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 62b25cf69bfa..9b493f2ead03 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -63,6 +63,10 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
 id_ns->npda = id_ns->npdg = npdg - 1;
 
+if (nvme_ns_shared(ns)) {
+id_ns->nmic |= NVME_NMIC_NS_SHARED;
+}
+
 return 0;
 }
 
@@ -365,16 +369,25 @@ static void nvme_ns_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-if (nvme_register_namespace(n, ns, errp)) {
-error_propagate_prepend(errp, local_err,
-"could not register namespace: ");
-return;
+if (ns->subsys) {
+if (nvme_subsys_register_ns(ns, errp)) {
+error_propagate_prepend(errp, local_err,
+"could not setup namespace to subsys: ");
+return;
+}
+} else {
+if (nvme_register_namespace(n, ns, errp)) {
+error_propagate_prepend(errp, local_err,
+"could not register namespace: ");
+return;
+}
 }
-
 }
 
 static Property nvme_ns_props[] = {
 DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
+DEFINE_PROP_LINK("subsys", NvmeNamespace, subsys, TYPE_NVME_SUBSYS,
+ NvmeSubsystem *),
 DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
 DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
 DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index 293ac990e3f6..929e78861903 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -47,6 +47,8 @@ typedef struct NvmeNamespace {
 const uint32_t *iocs;
 uint8_t  csi;
 
+NvmeSubsystem   *subsys;
+
 NvmeIdNsZoned   *id_ns_zoned;
 NvmeZone*zone_array;
 QTAILQ_HEAD(, NvmeZone) exp_open_zones;
@@ -77,6 +79,11 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
 return -1;
 }
 
+static inline bool nvme_ns_shared(NvmeNamespace *ns)
+{
+return !!ns->subsys;
+}
+
 static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
 {
 NvmeIdNs *id_ns = >id_ns;
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index e9d61c993c90..641de33e99fc 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -43,6 +43,31 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
 return cntlid;
 }
 
+int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp)
+{
+NvmeSubsystem *subsys = ns->subsys;
+NvmeCtrl *n;
+int i;
+
+if (subsys->namespaces[nvme_nsid(ns)]) {
+error_setg(errp, "namespace %d already registerd to subsy %s",
+   nvme_nsid(ns), subsys->parent_obj.id);
+return -1;
+}
+
+subsys->namespaces[nvme_nsid(ns)] = ns;
+
+for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
+n = subsys->ctrls[i];
+
+if (n && nvme_register_namespace(n, ns, errp)) {
+return -1;
+}
+}
+
+return 0;
+}
+
 static void nvme_subsys_setup(NvmeSubsystem *subsys)
 {
 snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 4eba50d96a1d..ccf6a71398d3 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -14,6 +14,7 @@
 OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
 
 #define NVME_SUBSYS_MAX_CTRLS   

[PATCH V5 2/6] hw/block/nvme: support to map controller to a subsystem

[Minwoo Im]

nvme controller(nvme) can be mapped to a NVMe subsystem(nvme-subsys).
This patch maps a controller to a subsystem by adding a parameter
'subsys' to the nvme device.

To map a controller to a subsystem, we need to put nvme-subsys first and
then maps the subsystem to the controller:

  -device nvme-subsys,id=subsys0
  -device nvme,serial=foo,id=nvme0,subsys=subsys0

If 'subsys' property is not given to the nvme controller, then subsystem
NQN will be created with serial (e.g., 'foo' in above example),
Otherwise, it will be based on subsys id (e.g., 'subsys0' in above
example).

Signed-off-by: Minwoo Im 
---
 hw/block/nvme.c | 30 +-
 hw/block/nvme.h |  3 +++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index aabccdf36f4b..b525fca14103 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -22,7 +22,8 @@
  *  [pmrdev=,] \
  *  max_ioqpairs=, \
  *  aerl=, aer_max_queued=, \
- *  mdts=,zoned.append_size_limit= \
+ *  mdts=,zoned.append_size_limit=, \
+ *  subsys= \
  *  -device nvme-ns,drive=,bus=,nsid=,\
  *  zoned=
  *  -device nvme-subsys,id=
@@ -44,6 +45,13 @@
  *
  * nvme device parameters
  * ~~
+ * - `subsys`
+ *   NVM Subsystem device. If given, a subsystem NQN will be initialized with
+ *given. Otherwise,  will be taken for subsystem NQN.
+ *   Also, it will enable multi controller capability represented in Identify
+ *   Controller data structure in CMIC (Controller Multi-path I/O and Namesapce
+ *   Sharing Capabilities), if given.
+ *
  * - `aerl`
  *   The Asynchronous Event Request Limit (AERL). Indicates the maximum number
  *   of concurrently outstanding Asynchronous Event Request commands support
@@ -4404,11 +4412,23 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice 
*pci_dev, Error **errp)
 return 0;
 }
 
+static void nvme_init_subnqn(NvmeCtrl *n)
+{
+NvmeSubsystem *subsys = n->subsys;
+NvmeIdCtrl *id = >id_ctrl;
+
+if (!subsys) {
+snprintf((char *)id->subnqn, sizeof(id->subnqn),
+ "nqn.2019-08.org.qemu:%s", n->params.serial);
+} else {
+pstrcpy((char *)id->subnqn, sizeof(id->subnqn), (char*)subsys->subnqn);
+}
+}
+
 static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
 {
 NvmeIdCtrl *id = >id_ctrl;
 uint8_t *pci_conf = pci_dev->config;
-char *subnqn;
 
 id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
 id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
@@ -4455,9 +4475,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN |
NVME_CTRL_SGLS_BITBUCKET);
 
-subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", n->params.serial);
-strpadcpy((char *)id->subnqn, sizeof(id->subnqn), subnqn, '\0');
-g_free(subnqn);
+nvme_init_subnqn(n);
 
 id->psd[0].mp = cpu_to_le16(0x9c4);
 id->psd[0].enlat = cpu_to_le32(0x10);
@@ -4545,6 +4563,8 @@ static Property nvme_props[] = {
 DEFINE_BLOCK_PROPERTIES(NvmeCtrl, namespace.blkconf),
 DEFINE_PROP_LINK("pmrdev", NvmeCtrl, pmr.dev, TYPE_MEMORY_BACKEND,
  HostMemoryBackend *),
+DEFINE_PROP_LINK("subsys", NvmeCtrl, subsys, TYPE_NVME_SUBSYS,
+ NvmeSubsystem *),
 DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
 DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
 DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index dee6092bd45f..04d4684601fd 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -2,6 +2,7 @@
 #define HW_NVME_H
 
 #include "block/nvme.h"
+#include "nvme-subsys.h"
 #include "nvme-ns.h"
 
 #define NVME_MAX_NAMESPACES 256
@@ -170,6 +171,8 @@ typedef struct NvmeCtrl {
 
 uint8_t zasl;
 
+NvmeSubsystem   *subsys;
+
 NvmeNamespace   namespace;
 NvmeNamespace   *namespaces[NVME_MAX_NAMESPACES];
 NvmeSQueue  **sq;
-- 
2.17.1

[PATCH 1/4] hw/riscv: Drop 'struct MemmapEntry'

[Bin Meng]

From: Bin Meng 

There is already a MemMapEntry type defined in hwaddr.h. Let's drop
the RISC-V defined `struct MemmapEntry` and use the existing one.

Signed-off-by: Bin Meng 
---

 hw/riscv/microchip_pfsoc.c |  9 +++--
 hw/riscv/opentitan.c   |  9 +++--
 hw/riscv/sifive_e.c|  9 +++--
 hw/riscv/sifive_u.c| 11 ---
 hw/riscv/spike.c   |  9 +++--
 hw/riscv/virt.c|  9 +++--
 6 files changed, 19 insertions(+), 37 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index e952b49e8c..266f1c3342 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -86,10 +86,7 @@
  *   - Register Map/PF_SoC_RegMap_V1_1/MPFS250T/mpfs250t_ioscb_memmap_dri.htm
  * describes the complete IOSCB modules memory maps
  */
-static const struct MemmapEntry {
-hwaddr base;
-hwaddr size;
-} microchip_pfsoc_memmap[] = {
+static const MemMapEntry microchip_pfsoc_memmap[] = {
 [MICROCHIP_PFSOC_RSVD0] =   {0x0,  0x100 },
 [MICROCHIP_PFSOC_DEBUG] =   {  0x100,  0xf00 },
 [MICROCHIP_PFSOC_E51_DTIM] ={  0x100, 0x2000 },
@@ -182,7 +179,7 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
 MicrochipPFSoCState *s = MICROCHIP_PFSOC(dev);
-const struct MemmapEntry *memmap = microchip_pfsoc_memmap;
+const MemMapEntry *memmap = microchip_pfsoc_memmap;
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *rsvd0_mem = g_new(MemoryRegion, 1);
 MemoryRegion *e51_dtim_mem = g_new(MemoryRegion, 1);
@@ -451,7 +448,7 @@ type_init(microchip_pfsoc_soc_register_types)
 static void microchip_icicle_kit_machine_init(MachineState *machine)
 {
 MachineClass *mc = MACHINE_GET_CLASS(machine);
-const struct MemmapEntry *memmap = microchip_pfsoc_memmap;
+const MemMapEntry *memmap = microchip_pfsoc_memmap;
 MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(machine);
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *mem_low = g_new(MemoryRegion, 1);
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index af3456932f..e168bffe69 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -28,10 +28,7 @@
 #include "qemu/units.h"
 #include "sysemu/sysemu.h"
 
-static const struct MemmapEntry {
-hwaddr base;
-hwaddr size;
-} ibex_memmap[] = {
+static const MemMapEntry ibex_memmap[] = {
 [IBEX_DEV_ROM] ={  0x8000, 16 * KiB },
 [IBEX_DEV_RAM] ={  0x1000,  0x1 },
 [IBEX_DEV_FLASH] =  {  0x2000,  0x8 },
@@ -66,7 +63,7 @@ static const struct MemmapEntry {
 
 static void opentitan_board_init(MachineState *machine)
 {
-const struct MemmapEntry *memmap = ibex_memmap;
+const MemMapEntry *memmap = ibex_memmap;
 OpenTitanState *s = g_new0(OpenTitanState, 1);
 MemoryRegion *sys_mem = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
@@ -114,7 +111,7 @@ static void lowrisc_ibex_soc_init(Object *obj)
 
 static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
 {
-const struct MemmapEntry *memmap = ibex_memmap;
+const MemMapEntry *memmap = ibex_memmap;
 MachineState *ms = MACHINE(qdev_get_machine());
 LowRISCIbexSoCState *s = RISCV_IBEX_SOC(dev_soc);
 MemoryRegion *sys_mem = get_system_memory();
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 59bac4cc9a..f939bcf9ea 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -50,10 +50,7 @@
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 
-static const struct MemmapEntry {
-hwaddr base;
-hwaddr size;
-} sifive_e_memmap[] = {
+static MemMapEntry sifive_e_memmap[] = {
 [SIFIVE_E_DEV_DEBUG] ={0x0, 0x1000 },
 [SIFIVE_E_DEV_MROM] = { 0x1000, 0x2000 },
 [SIFIVE_E_DEV_OTP] =  {0x2, 0x2000 },
@@ -77,7 +74,7 @@ static const struct MemmapEntry {
 
 static void sifive_e_machine_init(MachineState *machine)
 {
-const struct MemmapEntry *memmap = sifive_e_memmap;
+const MemMapEntry *memmap = sifive_e_memmap;
 
 SiFiveEState *s = RISCV_E_MACHINE(machine);
 MemoryRegion *sys_mem = get_system_memory();
@@ -187,7 +184,7 @@ static void sifive_e_soc_init(Object *obj)
 static void sifive_e_soc_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-const struct MemmapEntry *memmap = sifive_e_memmap;
+const MemMapEntry *memmap = sifive_e_memmap;
 SiFiveESoCState *s = RISCV_E_SOC(dev);
 MemoryRegion *sys_mem = get_system_memory();
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 59b61cea01..51e4132fc4 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -60,10 +60,7 @@
 
 #include 
 
-static const struct MemmapEntry {
-hwaddr base;
-hwaddr size;
-} 

[PATCH 0/4] hw/riscv: Clean-ups and map high mmio for PCIe of 'virt' machine

[Bin Meng]

From: Bin Meng 

This series does the following clean-ups:
- Drop 'struct MemmapEntry'
- virt: Drop the 'link_up' parameter of gpex_pcie_init()

It also adds the following small enhancement to 'virt' machine:
- Limit RAM size in a 32-bit system
- Map high mmio for PCIe


Bin Meng (4):
  hw/riscv: Drop 'struct MemmapEntry'
  hw/riscv: virt: Drop the 'link_up' parameter of gpex_pcie_init()
  hw/riscv: virt: Limit RAM size in a 32-bit system
  hw/riscv: virt: Map high mmio for PCIe

 hw/riscv/microchip_pfsoc.c |  9 ++---
 hw/riscv/opentitan.c   |  9 ++---
 hw/riscv/sifive_e.c|  9 ++---
 hw/riscv/sifive_u.c| 11 +++---
 hw/riscv/spike.c   |  9 ++---
 hw/riscv/virt.c| 72 ++
 6 files changed, 73 insertions(+), 46 deletions(-)

-- 
2.25.1

Re: [PATCH 2/2] meson: Warn when TCI is selected but TCG backend is available

[Stefan Weil]

Am 22.01.21 um 10:47 schrieb Thomas Huth:


On 22/01/2021 10.46, Philippe Mathieu-Daudé wrote:

On Fri, Jan 22, 2021 at 10:43 AM Thomas Huth  wrote:

On 22/01/2021 10.22, Philippe Mathieu-Daudé wrote:

Some new users get confused with 'TCG' and 'TCI', and enable TCI
support expecting to enable TCG.

Emit a warning when native TCG backend is available on the
host architecture, mentioning this is a suboptimal configuration.

Signed-off-by: Philippe Mathieu-Daudé 
---
   meson.build | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/meson.build b/meson.build
index 0a645e54662..7aa52d306c6 100644
--- a/meson.build
+++ b/meson.build
@@ -234,6 +234,9 @@
 error('Unsupported CPU @0@, try 
--enable-tcg-interpreter'.format(cpu))

   endif
 endif
+  if 'CONFIG_TCG_INTERPRETER' in config_host and cpu in 
supported_cpus
+    warning('Experimental TCI requested while native TCG is 
available on @0@, suboptimal performance expected'.format(cpu))

+  endif
 accelerators += 'CONFIG_TCG'
 config_host += { 'CONFIG_TCG': 'y' }
   endif



Reviewed-by: Thomas Huth 

... maybe you could also add some wording to the help text of the 
configure

script? Or maybe we could simply rename the "--enable-tcg-interpreter"
option into "--enable-tci" to avoid confusion with the normal TCG ?


I also thought about renaming as --enable-experimental-tci but then 
doubt that

would help, some users just want to enable everything :)


Then we should rename it into --disable-native-tcg ;-)

 Thomas



Both patches are fine (also optionally with the suggested addition of 
"slow"), so


Reviewed-by: Stefan Weil 

I think that --enable-tci would increase the TCI/TCG confusion and 
suggest to keep the current --enable-tcg-interpreter as most experts 
know that an interpreter is usually something which is slow.


As soon as time permits I also plan to make a new effort to integrate 
TCI as a run time option. Some years ago it was not possible to have 
code which supports both native and interpreted TCG, but this might have 
changed now. If it is possible, this would simplify CI a lot, and users 
could select the interpreter via a command line argument.


Stefan

Re: [PATCH] gitlab-ci.yml: Use the whole tree as artifacts to speed up the CI

[Daniel P . Berrangé]

On Fri, Jan 22, 2021 at 10:18:33AM +, Daniel P. Berrangé wrote:
> On Fri, Jan 22, 2021 at 11:07:22AM +0100, Thomas Huth wrote:
> > Currently, our check-system-* jobs are recompiling the whole sources
> > again. This happens due to the fact that the jobs are checking out
> > the whole source tree and required submodules again, and only try
> > to use the "build" directory with the binaries and object files
> > as an artifact from the previous stage - which simply does not work
> > anymore (with the current version of meson). Due to some changed
> > time stamps, meson is always trying to rebuild the whole tree.
> 
> This used to work in the past didn't it ? Did something change in
> meson to break this, or have we just not noticed before.

For to ask, could we address it by using  'meson test --no-rebuild'
perhaps ?

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH] gitlab-ci.yml: Use the whole tree as artifacts to speed up the CI

[Philippe Mathieu-Daudé]

On 1/22/21 11:18 AM, Daniel P. Berrangé wrote:
> On Fri, Jan 22, 2021 at 11:07:22AM +0100, Thomas Huth wrote:
>> Currently, our check-system-* jobs are recompiling the whole sources
>> again. This happens due to the fact that the jobs are checking out
>> the whole source tree and required submodules again, and only try
>> to use the "build" directory with the binaries and object files
>> as an artifact from the previous stage - which simply does not work
>> anymore (with the current version of meson). Due to some changed
>> time stamps, meson is always trying to rebuild the whole tree.
> 
> This used to work in the past didn't it ? Did something change in
> meson to break this, or have we just not noticed before.

Likely https://github.com/mesonbuild/meson/pull/7900/

Kludge:
https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg05491.html

[PATCH v2 3/4] configure: Improve TCI feature description

[Philippe Mathieu-Daudé]

Users might want to enable all features, without realizing some
features have negative effect. Mention the TCI feature is slow
and experimental, hoping it will be selected knowingly.

Suggested-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 9f016b06b54..71bdc523aa0 100755
--- a/configure
+++ b/configure
@@ -1753,7 +1753,7 @@ Advanced options (experts only):
   --with-trace-file=NAME   Full PATH,NAME of file to store traces
Default:trace-
   --disable-slirp  disable SLIRP userspace network connectivity
-  --enable-tcg-interpreter enable TCG with bytecode interpreter (TCI)
+  --enable-tcg-interpreter enable TCG with bytecode interpreter (experimental 
and slow)
   --enable-malloc-trim enable libc malloc_trim() for memory optimization
   --oss-libpath to OSS library
   --cpu=CPUBuild for host CPU [$cpu]
-- 
2.26.2

Re: [PATCH v7 07/11] iotests: add findtests.py

[Kevin Wolf]

Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add python script with new logic of searching for tests:
> 
> Current ./check behavior:
>  - tests are named [0-9][0-9][0-9]
>  - tests must be registered in group file (even if test doesn't belong
>to any group, like 142)
> 
> Behavior of findtests.py:
>  - group file is dropped
>  - tests are all files in tests/ subdirectory (except for .out files),
>so it's not needed more to "register the test", just create it with
>appropriate name in tests/ subdirectory. Old names like
>[0-9][0-9][0-9] (in root iotests directory) are supported too, but
>not recommended for new tests
>  - groups are parsed from '# group: ' line inside test files
>  - optional file group.local may be used to define some additional
>groups for downstreams
>  - 'disabled' group is used to temporary disable tests. So instead of
>commenting tests in old 'group' file you now can add them to
>disabled group with help of 'group.local' file
>  - selecting test ranges like 5-15 are not supported more
>(to support restarting failed ./check command from the middle of the
> process, new argument is added: --start-from)
> 
> Benefits:
>  - no rebase conflicts in group file on patch porting from branch to
>branch
>  - no conflicts in upstream, when different series want to occupy same
>test number
>  - meaningful names for test files
>For example, with digital number, when some person wants to add some
>test about block-stream, he most probably will just create a new
>test. But if there would be test-block-stream test already, he will
>at first look at it and may be just add a test-case into it.
>And anyway meaningful names are better.
> 
> This commit don't update check behavior (which will be done in further
> commit), still, the documentation changed like new behavior is already
> here.  Let's live with this small inconsistency for the following few
> commits, until final change.
> 
> The file findtests.py is self-executable and may be used for debugging
> purposes.

As Eric mentioned, this isn't accurate any more.

You mentioned using it as a way to debug things. I assume this is now
covered by the dry run option?

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  docs/devel/testing.rst  |  50 +-
>  tests/qemu-iotests/findtests.py | 159 
>  2 files changed, 208 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemu-iotests/findtests.py

> +def add_group_file(self, fname: str) -> None:
> +with open(fname) as f:
> +for line in f:
> +line = line.strip()
> +
> +if (not line) or line[0] == '#':
> +continue
> +
> +words = line.split()
> +test_file = self.parse_test_name(words[0])
> +groups = words[1:]

The previous version still had this:

+if test_file not in self.all_tests:
+print(f'Warning: {fname}: "{test_file}" test is not found.'
+  ' Skip.')
+continue

Why did you remove it? I found this useful when I had a wrong test name
in my group.local. Now it's silently ignored.

> +for g in groups:
> +self.groups[g].add(test_file)

Kevin

Re: [PATCH v3 12/21] linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE

[Peter Maydell]

On Fri, 15 Jan 2021 at 22:47, Richard Henderson
 wrote:
>
> This is the prctl bit that controls whether syscalls accept tagged
> addresses.  See Documentation/arm64/tagged-address-abi.rst in the
> linux kernel.

> +#ifdef TARGET_TAGGED_ADDRESSES
> +/**
> + * cpu_untagged_addr:
> + * @cs: CPU context
> + * @x: tagged address
> + *
> + * Remove any address tag from @x.  This is explicitly related to the
> + * linux syscall TIF_TAGGED_ADDR setting, not TBI in general.
> + *
> + * There should be a better place to put this, but we need this in
> + * include/exec/cpu_ldst.h, and not some place linux-user specific.
> + */
> +static inline target_ulong cpu_untagged_addr(CPUState *cs, target_ulong x)
> +{
> +ARMCPU *cpu = ARM_CPU(cs);
> +return x & cpu->env.untagged_addr_mask;
> +}
> +#endif

Forgot to mention: this only does the right thing on addresses
in the lower half of the address space. I guess that's mostly
OK for our purposes? It probably means that if a guest program
deliberately dereferences a bad address in the top half of the
address space we'll report the wrong (ie different to what a real
kernel reports) address value to it in the SEGV signal handler.

The kernel's "untagged_addr()" implementation:
https://elixir.bootlin.com/linux/latest/source/arch/arm64/include/asm/memory.h#L203
slightly confusingly does "untag the addr if it's in the userspace
half, leave the tag bits alone if in the kernel half".

thanks
-- PMM

[PATCH V5 1/6] hw/block/nvme: introduce nvme-subsys device

[Minwoo Im]

To support multi-path in QEMU NVMe device model, We need to have NVMe
subsystem hierarchy to map controllers and namespaces to a NVMe
subsystem.

This patch introduced a simple nvme-subsys device model.  The subsystem
will be prepared with subsystem NQN with  provided in
nvme-subsys device:

  ex) -device nvme-subsys,id=subsys0: nqn.2019-08.org.qemu:subsys0

Signed-off-by: Minwoo Im 
---
 hw/block/meson.build   |  2 +-
 hw/block/nvme-subsys.c | 60 ++
 hw/block/nvme-subsys.h | 25 ++
 hw/block/nvme.c|  3 +++
 4 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 hw/block/nvme-subsys.c
 create mode 100644 hw/block/nvme-subsys.h

diff --git a/hw/block/meson.build b/hw/block/meson.build
index 602ca6c8541d..83ea2d37978d 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -13,7 +13,7 @@ softmmu_ss.add(when: 'CONFIG_SSI_M25P80', if_true: 
files('m25p80.c'))
 softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
 softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('tc58128.c'))
-softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c'))
+softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c', 
'nvme-subsys.c'))
 
 specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
 specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: 
files('vhost-user-blk.c'))
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
new file mode 100644
index ..aa82911b951c
--- /dev/null
+++ b/hw/block/nvme-subsys.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im 
+ *
+ * This code is licensed under the GNU GPL v2.  Refer COPYING.
+ */
+
+#include "qemu/units.h"
+#include "qemu/osdep.h"
+#include "qemu/uuid.h"
+#include "qemu/iov.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-core.h"
+#include "hw/block/block.h"
+#include "block/aio.h"
+#include "block/accounting.h"
+#include "sysemu/sysemu.h"
+#include "hw/pci/pci.h"
+#include "nvme.h"
+#include "nvme-subsys.h"
+
+static void nvme_subsys_setup(NvmeSubsystem *subsys)
+{
+snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
+ "nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
+}
+
+static void nvme_subsys_realize(DeviceState *dev, Error **errp)
+{
+NvmeSubsystem *subsys = NVME_SUBSYS(dev);
+
+nvme_subsys_setup(subsys);
+}
+
+static void nvme_subsys_class_init(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+
+dc->realize = nvme_subsys_realize;
+dc->desc = "Virtual NVMe subsystem";
+}
+
+static const TypeInfo nvme_subsys_info = {
+.name = TYPE_NVME_SUBSYS,
+.parent = TYPE_DEVICE,
+.class_init = nvme_subsys_class_init,
+.instance_size = sizeof(NvmeSubsystem),
+};
+
+static void nvme_subsys_register_types(void)
+{
+type_register_static(_subsys_info);
+}
+
+type_init(nvme_subsys_register_types)
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
new file mode 100644
index ..40f06a4c7db0
--- /dev/null
+++ b/hw/block/nvme-subsys.h
@@ -0,0 +1,25 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im 
+ *
+ * This code is licensed under the GNU GPL v2.  Refer COPYING.
+ */
+
+#ifndef NVME_SUBSYS_H
+#define NVME_SUBSYS_H
+
+#define TYPE_NVME_SUBSYS "nvme-subsys"
+#define NVME_SUBSYS(obj) \
+OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
+
+#define NVME_SUBSYS_MAX_CTRLS   32
+
+typedef struct NvmeCtrl NvmeCtrl;
+typedef struct NvmeNamespace NvmeNamespace;
+typedef struct NvmeSubsystem {
+DeviceState parent_obj;
+uint8_t subnqn[256];
+} NvmeSubsystem;
+
+#endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 21aec90637fa..aabccdf36f4b 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -25,6 +25,7 @@
  *  mdts=,zoned.append_size_limit= \
  *  -device nvme-ns,drive=,bus=,nsid=,\
  *  zoned=
+ *  -device nvme-subsys,id=
  *
  * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
  * offset 0 in BAR2 and supports only WDS, RDS and SQS for now. By default, the
@@ -38,6 +39,8 @@
  *
  * The PMR will use BAR 4/5 exclusively.
  *
+ * To place controller(s) and namespace(s) to a subsystem, then provide
+ * nvme-subsys device as above.
  *
  * nvme device parameters
  * ~~
-- 
2.17.1

[PATCH V5 0/6] hw/block/nvme: support multi-path for ctrl/ns

[Minwoo Im]

Hello,

Here's fifth patch series for the support of NVMe subsystem scheme with
multi-controller and namespace sharing in a subsystem.

This series has applied review comments from the previous series,
mostly from Keith's review.  Thanks Keith!

Here's test result with a simple 'nvme list -v' command from this model
with adding a ZNS example with subsys.

  -device nvme-subsys,id=subsys0 \
  -device nvme,serial=foo,id=nvme0,subsys=subsys0 \
  -device nvme,serial=bar,id=nvme1,subsys=subsys0 \
  -device nvme,serial=baz,id=nvme2,subsys=subsys0 \
  -device nvme-ns,id=ns1,drive=drv10,nsid=1,subsys=subsys0 \
  -device nvme-ns,id=ns2,drive=drv11,nsid=2,bus=nvme2 \
  \
  -device nvme,serial=qux,id=nvme3 \
  -device nvme-ns,id=ns3,drive=drv12,nsid=3,bus=nvme3 \
  \
  -device nvme-subsys,id=subsys1 \
  -device nvme,serial=quux,id=nvme4,subsys=subsys1 \
  -device nvme-ns,id=ns4,drive=drv13,nsid=1,subsys=subsys1,zoned=true \

  root@vm:~/work# nvme list -v
  NVM Express Subsystems

  SubsystemSubsystem-NQN
Controllers
   

 
  nvme-subsys1 nqn.2019-08.org.qemu:subsys0 
nvme0, nvme1, nvme2
  nvme-subsys3 nqn.2019-08.org.qemu:qux 
nvme3
  nvme-subsys4 nqn.2019-08.org.qemu:subsys1 
nvme4

  NVM Express Controllers

  Device   SN   MN   FR 
  TxPort AddressSubsystemNamespaces
     
 -- --  
  nvme0foo  QEMU NVMe Ctrl   1.0
  pcie   :00:06.0   nvme-subsys1 nvme1c0n1
  nvme1bar  QEMU NVMe Ctrl   1.0
  pcie   :00:07.0   nvme-subsys1 nvme1c1n1
  nvme2baz  QEMU NVMe Ctrl   1.0
  pcie   :00:08.0   nvme-subsys1 nvme1c2n1, nvme1c2n2
  nvme3qux  QEMU NVMe Ctrl   1.0
  pcie   :00:09.0   nvme-subsys3 nvme3n1
  nvme4quux QEMU NVMe Ctrl   1.0
  pcie   :00:0a.0   nvme-subsys4 nvme4c4n1

  NVM Express Namespaces

  Device   NSID Usage  Format   Controllers
    --  

  nvme1n1  1134.22  MB / 134.22  MB512   B +  0 B   nvme0, 
nvme1, nvme2
  nvme1n2  2268.44  MB / 268.44  MB512   B +  0 B   nvme2
  nvme3n1  3268.44  MB / 268.44  MB512   B +  0 B   nvme3
  nvme4n1  1268.44  MB / 268.44  MB512   B +  0 B   nvme4

Thanks,

Since V4:
  - Code clean-up to snprintf rather than duplicating it and copy.
(Keith)
  - Documentation for 'subsys' clean-up.  (Keith)
  - Remove 'cntlid' param from nvme_init_ctrl().  (Keith)
  - Put error_propagate() in nvme_realize().  (Keith)

Since RFC V3:
  - Exclude 'deatched' scheme from this series.  This will be covered in
the next series by covering all the ns-related admin commands
including ZNS and ns-mgmt. (Niklas)
  - Rebased on nvme-next.
  - Remove RFC tag from this V4.

Since RFC V2:
  - Rebased on nvme-next branch with trivial patches from the previous
version(V2) applied. (Klaus)
  - Fix enumeration type name convention with NvmeIdNs prefix. (Klaus)
  - Put 'cntlid' to NvmeCtrl instance in nvme_init_ctrl() which was
missed in V2.
  - Added 'detached' parameter to nvme-ns device to decide whether to
attach or not to controller(s) in the subsystem. (Klaus)
  - Implemented Identify Active Namespace ID List aprt from Identify
Allocated Namespace ID List by removing fall-thru statement.

Since RFC V1:
  - Updated namespace sharing scheme to be based on nvme-subsys
hierarchy.

Minwoo Im (6):
  hw/block/nvme: introduce nvme-subsys device
  hw/block/nvme: support to map controller to a subsystem
  hw/block/nvme: add CMIC enum value for Identify Controller
  hw/block/nvme: support for multi-controller in subsystem
  hw/block/nvme: add NMIC enum value for Identify Namespace
  hw/block/nvme: support for shared namespace in subsystem

 hw/block/meson.build   |   2 +-
 hw/block/nvme-ns.c |  23 +++--
 hw/block/nvme-ns.h |   7 +++
 hw/block/nvme-subsys.c | 106 +
 hw/block/nvme-subsys.h |  32 +
 hw/block/nvme.c|  72 +---
 hw/block/nvme.h|   4 ++
 include/block/nvme.h   |   8 
 8 files changed, 242 insertions(+), 12 deletions(-)
 create 

Re: [PATCH] gitlab-ci.yml: Use the whole tree as artifacts to speed up the CI

[Daniel P . Berrangé]

On Fri, Jan 22, 2021 at 11:07:22AM +0100, Thomas Huth wrote:
> Currently, our check-system-* jobs are recompiling the whole sources
> again. This happens due to the fact that the jobs are checking out
> the whole source tree and required submodules again, and only try
> to use the "build" directory with the binaries and object files
> as an artifact from the previous stage - which simply does not work
> anymore (with the current version of meson). Due to some changed
> time stamps, meson is always trying to rebuild the whole tree.

This used to work in the past didn't it ? Did something change in
meson to break this, or have we just not noticed before.

> So instead of trying to marry a freshly checked out source tree
> with the pre-built binaries in these jobs, let's simply pass the
> whole source including the submodules and the build tree as artifact
> to the test jobs. That way timestamps get preserved and there is
> no rebuild of the sources anymore. This saves ca. 15 - 20 minutes
> of precious CI cycles in each run.

I'm a little worried we might end up hitting the artifact size
limit which is supposedly 1GB on gitlab.com.  Im guessing this
must be measuring the compressed size though, as a src checkout
with build dir  and .git dir is already way over 1GB.

> 
> Signed-off-by: Thomas Huth 
> ---
>  This is how a job looked like before my patch, running for 42 minutes:
>  https://gitlab.com/huth/qemu/-/jobs/978432757
> 
>  And this is how it looks like afterwards - it just took 18 minutes:
>  https://gitlab.com/huth/qemu/-/jobs/979500316
> 
>  .gitlab-ci.d/containers.yml |  1 +
>  .gitlab-ci.yml  | 40 +
>  2 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index e2f9c99e27..d55280661f 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -39,7 +39,6 @@ include:
>image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
>script:
>  - cd build
> -- find . -type f -exec touch {} +
>  - make $MAKE_CHECK_ARGS
>  
>  .acceptance_template: _definition
> @@ -83,8 +82,7 @@ build-system-alpine:
>artifacts:
>  expire_in: 2 days
>  paths:
> -  - .git-submodule-status
> -  - build
> +  - "*"
>  
>  check-system-alpine:
><<: *native_test_job_definition
> @@ -92,6 +90,7 @@ check-system-alpine:
>  - job: build-system-alpine
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: alpine
>  MAKE_CHECK_ARGS: check
>  
> @@ -101,6 +100,7 @@ acceptance-system-alpine:
>  - job: build-system-alpine
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: alpine
>  MAKE_CHECK_ARGS: check-acceptance
><<: *acceptance_definition
> @@ -116,7 +116,7 @@ build-system-ubuntu:
>artifacts:
>  expire_in: 2 days
>  paths:
> -  - build
> +  - "*"
>  
>  check-system-ubuntu:
><<: *native_test_job_definition
> @@ -124,6 +124,7 @@ check-system-ubuntu:
>  - job: build-system-ubuntu
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: ubuntu2004
>  MAKE_CHECK_ARGS: check
>  
> @@ -133,6 +134,7 @@ acceptance-system-ubuntu:
>  - job: build-system-ubuntu
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: ubuntu2004
>  MAKE_CHECK_ARGS: check-acceptance
><<: *acceptance_definition
> @@ -148,7 +150,7 @@ build-system-debian:
>artifacts:
>  expire_in: 2 days
>  paths:
> -  - build
> +  - "*"
>  
>  check-system-debian:
><<: *native_test_job_definition
> @@ -156,6 +158,7 @@ check-system-debian:
>  - job: build-system-debian
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: debian-amd64
>  MAKE_CHECK_ARGS: check
>  
> @@ -170,7 +173,7 @@ build-tools-and-docs-debian:
>artifacts:
>  expire_in: 2 days
>  paths:
> -  - build
> +  - "*"
>  
>  acceptance-system-debian:
><<: *native_test_job_definition
> @@ -178,6 +181,7 @@ acceptance-system-debian:
>  - job: build-system-debian
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: debian-amd64
>  MAKE_CHECK_ARGS: check-acceptance
><<: *acceptance_definition
> @@ -194,7 +198,7 @@ build-system-fedora:
>artifacts:
>  expire_in: 2 days
>  paths:
> -  - build
> +  - "*"
>  
>  check-system-fedora:
><<: *native_test_job_definition
> @@ -202,6 +206,7 @@ check-system-fedora:
>  - job: build-system-fedora
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: fedora
>  MAKE_CHECK_ARGS: check
>  
> @@ -211,6 +216,7 @@ acceptance-system-fedora:
>  - job: build-system-fedora
>artifacts: true
>variables:
> +GIT_CHECKOUT: "false"
>  IMAGE: fedora
>  MAKE_CHECK_ARGS: check-acceptance
><<: *acceptance_definition
> @@ -226,7 +232,7 @@ build-system-centos:
>artifacts:
>  

Re: [PULL 0/9] s390x updates

[Peter Maydell]

On Thu, 21 Jan 2021 at 12:16, Cornelia Huck  wrote:
>
> The following changes since commit 954b83f13236d21b4116b93a726ea36b5dc2d303:
>
>   Merge remote-tracking branch 
> 'remotes/huth-gitlab/tags/pull-request-2021-01-20' into staging (2021-01-20 
> 17:44:31 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/cohuck/qemu.git tags/s390x-20210121
>
> for you to fetch changes up to e6a80232f4087e8c7ec253f573319f69165b859d:
>
>   s390x: Use strpadcpy for copying vm name (2021-01-21 11:19:45 +0100)
>
> 
> s390x updates:
> - headers update to Linux 5.11-rc2
> - fix tcg emulation for some instructions that are generated by
>   clang Linux kernel builds
> - vfio-ccw: wire up the device unplug notification mechanism
> - fix a gcc 11 warning
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM

Re: [PATCH] replay: fix replay of the interrupts

[Philippe Mathieu-Daudé]

On 1/19/21 1:39 PM, Pavel Dovgalyuk wrote:
> Sometimes interrupt event comes at the same time with
> the virtual timers. In this case replay tries to proceed
> the timers, because deadline for them is zero.
> This patch allows processing interrupts and exceptions
> by entering the vCPU execution loop, when deadline is zero,
> but checkpoint associated with virtual timers is not ready
> to be replayed.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  accel/tcg/tcg-cpus-icount.c |8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/tcg/tcg-cpus-icount.c b/accel/tcg/tcg-cpus-icount.c
> index 9f45432275..a6d2bb8a88 100644
> --- a/accel/tcg/tcg-cpus-icount.c
> +++ b/accel/tcg/tcg-cpus-icount.c
> @@ -81,7 +81,13 @@ void icount_handle_deadline(void)
>  int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
>QEMU_TIMER_ATTR_ALL);
>  
> -if (deadline == 0) {
> +/*
> + * Instructions, interrupts, and exceptions are processed in cpu-exec.
> + * Don't interrupt cpu thread, when these events are waiting
> + * (i.e., there is no checkpoint)
> + */
> +if (deadline == 0
> +&& (replay_mode == REPLAY_MODE_RECORD || replay_has_checkpoint())) {

LGTM, but Cc'ing Peter/Alex just in case :)

>  icount_notify_aio_contexts();
>  }
>  }
> 
> 

Re: [PATCH v7 00/11] Rework iotests/check

[Vladimir Sementsov-Ogievskiy]

22.01.2021 14:27, Kevin Wolf wrote:

Am 20.01.2021 um 21:52 hat Eric Blake geschrieben:

On 1/16/21 7:44 AM, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

These series has 3 goals:

  - get rid of group file (to forget about rebase and in-list conflicts)
  - introduce human-readable names for tests
  - rewrite check into python

v7:
   - fix wording and grammar
   - satisfy python linters
   - move argv interfaces all into one in new check script
   - support '-n' == '--dry-run' option
   - update check-block to run check with correct PYTHON


I'd really like a second reviewer on 7-11, but I'm queueing 1-6 on my
NBD tree now.


Oh, you already sent a pull request? Having 6 in without the rest is not
a good state. We now have the group info duplicated and one of them is
ignored, but will become the meaningful copy later. We need to be
careful to not let them diverge now.

I hope the rest is fine so we can switch over quickly, otherwise I'd
prefer to revert 6 and redo it from the then current state when we merge
the whole series.



I can make a new version now with tiny fixes suggested by Eric if it is 
convenient. (and keep --color suggestion for a separate follow-up)



--
Best regards,
Vladimir

Re: [PATCH] util/log: flush TB cache when log level changes

[Alex Bennée]

Pavel Dovgalyuk  writes:

> Sometimes we need to collect the translation logs starting
> from some point of the execution. Some TB listings may
> be missed in this case, when blocks were translated before.
> This patch clears TB cache to allow re-translation of such
> code blocks.
>
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  accel/tcg/translate-all.c |8 
>  include/sysemu/tcg.h  |1 +
>  stubs/meson.build |1 +
>  stubs/tcg.c   |   12 
>  util/log.c|3 +++
>  5 files changed, 25 insertions(+)
>  create mode 100644 stubs/tcg.c
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index e9de6ff9dd..3acb227c57 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1461,6 +1461,14 @@ void tb_flush(CPUState *cpu)
>  }
>  }
>  
> +void tb_flush_all(void)
> +{
> +CPUState *cpu;
> +CPU_FOREACH(cpu) {
> +tb_flush(cpu);
> +}
> +}
> +

This isn't needed - tb_flush flushes all translations although it does
need to be executed in a CPU context to do so.

>  /*
>   * Formerly ifdef DEBUG_TB_CHECK. These debug functions are user-mode-only,
>   * so in order to prevent bit rot we compile them unconditionally in 
> user-mode,
> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
> index 00349fb18a..7415f11022 100644
> --- a/include/sysemu/tcg.h
> +++ b/include/sysemu/tcg.h
> @@ -9,6 +9,7 @@
>  #define SYSEMU_TCG_H
>  
>  void tcg_exec_init(unsigned long tb_size, int splitwx);
> +void tb_flush_all(void);
>  
>  #ifdef CONFIG_TCG
>  extern bool tcg_allowed;
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 80b1d81a31..95e70f8542 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -38,6 +38,7 @@ stub_ss.add(files('set-fd-handler.c'))
>  stub_ss.add(files('sysbus.c'))
>  stub_ss.add(files('target-get-monitor-def.c'))
>  stub_ss.add(files('target-monitor-defs.c'))
> +stub_ss.add(files('tcg.c'))
>  stub_ss.add(files('tpm.c'))
>  stub_ss.add(files('trace-control.c'))
>  stub_ss.add(files('uuid.c'))
> diff --git a/stubs/tcg.c b/stubs/tcg.c
> new file mode 100644
> index 00..775a748c77
> --- /dev/null
> +++ b/stubs/tcg.c
> @@ -0,0 +1,12 @@
> +/*
> + * TCG stubs
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "sysemu/tcg.h"
> +
> +void tb_flush_all(void)
> +{
> +}
> diff --git a/util/log.c b/util/log.c
> index 2ee1500bee..2ff342a91b 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -26,6 +26,7 @@
>  #include "trace/control.h"
>  #include "qemu/thread.h"
>  #include "qemu/lockable.h"
> +#include "sysemu/tcg.h"
>  
>  static char *logfilename;
>  static QemuMutex qemu_logfile_mutex;
> @@ -84,6 +85,8 @@ void qemu_set_log(int log_flags)
>  #ifdef CONFIG_TRACE_LOG
>  qemu_loglevel |= LOG_TRACE;
>  #endif
> +tb_flush_all();
> +

I would call tb_flush(current_cpu) or first_cpu here. But two things:

 - I'm not sure you have a CPU at all times qemu_set_log is called
 - It seems overly aggressive to throw away all translations every time
   the log level is changed. I would define a mask in log.h and have
   something like:

  if (log_flags & LOG_TRANSLATION) {
  tb_flush();
  }

>  /*
>   * In all cases we only log if qemu_loglevel is set.
>   * Also:


-- 
Alex Bennée

Re: [Qemu-devel] [PATCH v2 07/11] chardev: Let IOReadHandler use unsigned type

[P J P]

+-- On Fri, 22 Jan 2021, Richard Purdie wrote --+
| If so can anyone point me at that change?
| 
| I ask since CVE-2018-18438 is marked as affecting all qemu versions
| (https://nvd.nist.gov/vuln/detail/CVE-2018-18438).
| 
| If it was fixed, the version mask could be updated. If the fix wasn't deemed 
| worthwhile for some reason that is also fine and I can mark this one as such 
| in our system. I'm being told we only need one of the patches in this series 
| which I also don't believe as I suspect we either need the set or none of 
| them!
| 
| Any info would be most welcome.

  -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02239.html
  -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02231.html

* Yes, the type change fix had come up during patch reviews above, and this 
  series implemented the change.

* Series is required IIUC, didn't realise it's not merged.


Thank you. 
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D

Re: [PATCH v7 07/11] iotests: add findtests.py

[Vladimir Sementsov-Ogievskiy]

22.01.2021 14:49, Kevin Wolf wrote:

Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:

+if 'disabled' not in groups and 'disabled' not in exclude_groups:
+# Don't want to modify function argument, so create new list.
+exclude_groups = exclude_groups + ['disabled']


Oops, forgot the other comment I wanted to make:

This would only have been needed if you had turned exclude_groups into a
Sequence. Now that it's still a list, copying the list isn't strictly
necessary.



I just think that such side effects (changing function arguments when it is not 
part of function intention) are better to avoid.


--
Best regards,
Vladimir

[PATCH V5 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace

[Minwoo Im]

Added Namespace Multi-path I/O and Namespace Sharing Capabilities (NMIC)
field to support shared namespace from controller(s).

This field is in Identify Namespace data structure in [30].

Signed-off-by: Minwoo Im 
---
 include/block/nvme.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index d6415a869c1c..ad68cdc2b92d 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1203,6 +1203,10 @@ enum NvmeNsIdentifierType {
 NVME_NIDT_CSI   = 0x04,
 };
 
+enum NvmeIdNsNmic {
+NVME_NMIC_NS_SHARED = 1 << 0,
+};
+
 enum NvmeCsi {
 NVME_CSI_NVM= 0x00,
 NVME_CSI_ZONED  = 0x02,
-- 
2.17.1

[PATCH] hw/mips: loongson3: Drop 'struct MemmapEntry'

[Bin Meng]

From: Bin Meng 

There is already a MemMapEntry type defined in hwaddr.h. Let's drop
the loongson3 defined `struct MemmapEntry` and use the existing one.

Signed-off-by: Bin Meng 
---

 hw/mips/loongson3_bootp.h | 7 +--
 hw/mips/loongson3_virt.c  | 6 +++---
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/mips/loongson3_bootp.h b/hw/mips/loongson3_bootp.h
index 09f8480abf..d525ab745a 100644
--- a/hw/mips/loongson3_bootp.h
+++ b/hw/mips/loongson3_bootp.h
@@ -228,12 +228,7 @@ enum {
 LOADER_PARAM,
 };
 
-struct MemmapEntry {
-hwaddr base;
-hwaddr size;
-};
-
-extern const struct MemmapEntry virt_memmap[];
+extern const MemMapEntry virt_memmap[];
 void init_loongson_params(struct loongson_params *lp, void *p,
   uint64_t cpu_freq, uint64_t ram_size);
 void init_reset_system(struct efi_reset_system_t *reset);
diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index d4a82fa536..b15071defc 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -72,7 +72,7 @@
 #define RTC_IRQ 1
 #define PCIE_IRQ_BASE   2
 
-const struct MemmapEntry virt_memmap[] = {
+const MemMapEntry virt_memmap[] = {
 [VIRT_LOWMEM] =  { 0x,0x1000 },
 [VIRT_PM] =  { 0x1008, 0x100 },
 [VIRT_FW_CFG] =  { 0x10080100, 0x100 },
@@ -86,13 +86,13 @@ const struct MemmapEntry virt_memmap[] = {
 [VIRT_HIGHMEM] = { 0x8000,   0x0 }, /* Variable */
 };
 
-static const struct MemmapEntry loader_memmap[] = {
+static const MemMapEntry loader_memmap[] = {
 [LOADER_KERNEL] ={ 0x, 0x400 },
 [LOADER_INITRD] ={ 0x0400,   0x0 }, /* Variable */
 [LOADER_CMDLINE] =   { 0x0ff0,  0x10 },
 };
 
-static const struct MemmapEntry loader_rommap[] = {
+static const MemMapEntry loader_rommap[] = {
 [LOADER_BOOTROM] =   { 0x1fc0,0x1000 },
 [LOADER_PARAM] = { 0x1fc01000,   0x1 },
 };
-- 
2.25.1

[PATCH] gitlab-ci.yml: Use the whole tree as artifacts to speed up the CI

[Thomas Huth]

Currently, our check-system-* jobs are recompiling the whole sources
again. This happens due to the fact that the jobs are checking out
the whole source tree and required submodules again, and only try
to use the "build" directory with the binaries and object files
as an artifact from the previous stage - which simply does not work
anymore (with the current version of meson). Due to some changed
time stamps, meson is always trying to rebuild the whole tree.

So instead of trying to marry a freshly checked out source tree
with the pre-built binaries in these jobs, let's simply pass the
whole source including the submodules and the build tree as artifact
to the test jobs. That way timestamps get preserved and there is
no rebuild of the sources anymore. This saves ca. 15 - 20 minutes
of precious CI cycles in each run.

Signed-off-by: Thomas Huth 
---
 This is how a job looked like before my patch, running for 42 minutes:
 https://gitlab.com/huth/qemu/-/jobs/978432757

 And this is how it looks like afterwards - it just took 18 minutes:
 https://gitlab.com/huth/qemu/-/jobs/979500316

 .gitlab-ci.d/containers.yml |  1 +
 .gitlab-ci.yml  | 40 +
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e2f9c99e27..d55280661f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -39,7 +39,6 @@ include:
   image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
   script:
 - cd build
-- find . -type f -exec touch {} +
 - make $MAKE_CHECK_ARGS
 
 .acceptance_template: _definition
@@ -83,8 +82,7 @@ build-system-alpine:
   artifacts:
 expire_in: 2 days
 paths:
-  - .git-submodule-status
-  - build
+  - "*"
 
 check-system-alpine:
   <<: *native_test_job_definition
@@ -92,6 +90,7 @@ check-system-alpine:
 - job: build-system-alpine
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: alpine
 MAKE_CHECK_ARGS: check
 
@@ -101,6 +100,7 @@ acceptance-system-alpine:
 - job: build-system-alpine
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: alpine
 MAKE_CHECK_ARGS: check-acceptance
   <<: *acceptance_definition
@@ -116,7 +116,7 @@ build-system-ubuntu:
   artifacts:
 expire_in: 2 days
 paths:
-  - build
+  - "*"
 
 check-system-ubuntu:
   <<: *native_test_job_definition
@@ -124,6 +124,7 @@ check-system-ubuntu:
 - job: build-system-ubuntu
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: ubuntu2004
 MAKE_CHECK_ARGS: check
 
@@ -133,6 +134,7 @@ acceptance-system-ubuntu:
 - job: build-system-ubuntu
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: ubuntu2004
 MAKE_CHECK_ARGS: check-acceptance
   <<: *acceptance_definition
@@ -148,7 +150,7 @@ build-system-debian:
   artifacts:
 expire_in: 2 days
 paths:
-  - build
+  - "*"
 
 check-system-debian:
   <<: *native_test_job_definition
@@ -156,6 +158,7 @@ check-system-debian:
 - job: build-system-debian
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: debian-amd64
 MAKE_CHECK_ARGS: check
 
@@ -170,7 +173,7 @@ build-tools-and-docs-debian:
   artifacts:
 expire_in: 2 days
 paths:
-  - build
+  - "*"
 
 acceptance-system-debian:
   <<: *native_test_job_definition
@@ -178,6 +181,7 @@ acceptance-system-debian:
 - job: build-system-debian
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: debian-amd64
 MAKE_CHECK_ARGS: check-acceptance
   <<: *acceptance_definition
@@ -194,7 +198,7 @@ build-system-fedora:
   artifacts:
 expire_in: 2 days
 paths:
-  - build
+  - "*"
 
 check-system-fedora:
   <<: *native_test_job_definition
@@ -202,6 +206,7 @@ check-system-fedora:
 - job: build-system-fedora
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: fedora
 MAKE_CHECK_ARGS: check
 
@@ -211,6 +216,7 @@ acceptance-system-fedora:
 - job: build-system-fedora
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: fedora
 MAKE_CHECK_ARGS: check-acceptance
   <<: *acceptance_definition
@@ -226,7 +232,7 @@ build-system-centos:
   artifacts:
 expire_in: 2 days
 paths:
-  - build
+  - "*"
 
 check-system-centos:
   <<: *native_test_job_definition
@@ -234,6 +240,7 @@ check-system-centos:
 - job: build-system-centos
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: centos8
 MAKE_CHECK_ARGS: check
 
@@ -243,6 +250,7 @@ acceptance-system-centos:
 - job: build-system-centos
   artifacts: true
   variables:
+GIT_CHECKOUT: "false"
 IMAGE: centos8
 MAKE_CHECK_ARGS: check-acceptance
   <<: *acceptance_definition
@@ -257,7 +265,7 @@ build-system-opensuse:
   artifacts:
 expire_in: 2 days
 paths:
-  - build
+  - "*"
 
 check-system-opensuse:
   <<: *native_test_job_definition
@@ -265,6 +273,7 @@ 

Re: Thread safety of coroutine-sigaltstack

[Peter Maydell]

On Fri, 22 Jan 2021 at 08:50, Max Reitz  wrote:
>
> On 20.01.21 18:25, Laszlo Ersek wrote:
>
> [...]
>
> > A simple grep for SIGUSR2 seems to indicate that SIGUSR2 is not used by
> > system emulation for anything else, in practice. Is it possible to
> > dedicate SIGUSR2 explicitly to coroutine-sigaltstack, and set up the
> > action beforehand, from some init function that executes on a "central"
> > thread, before qemu_coroutine_new() is ever called?
>
> I wrote a patch to that effect, but just before sending I wondered
> whether SIGUSR2 cannot be registered by the “guest” in user-mode
> emulation, and whether that would then break coroutines from there on.
>
> (I have no experience dealing with user-mode emulation, but it does look
> like the guest can just register handlers for any signal but SIGSEGV and
> SIGBUS.)

Yes, SIGUSR2 is for the guest in user-emulation mode. OTOH do we
even use the coroutine code in user-emulation mode? Looking at
the meson.build files, we only add the coroutine_*.c to util_ss
if 'have_block', and we set have_block = have_system or have_tools.
I think (but have not checked) that that means we will build and
link the object file into the user-mode binaries if you happen
to build them in the same run as system-mode binaries, but won't
build them in if you built the user-mode binaries as a separate
build. Which is odd and probably worth fixing, but does mean we
know that we aren't actually using coroutines in user-mode.
(Also user-mode really means Linux or BSD and I think both of
those have working ucontext.)

thanks
-- PMM

[PATCH] coroutine-sigaltstack: Keep SIGUSR2 handler up

[Max Reitz]

Modifying signal handlers is a process-global operation.  When two
threads run coroutine-sigaltstack's qemu_coroutine_new() concurrently,
they may interfere with each other: One of them may revert the SIGUSR2
handler back to the default between the other thread setting up
coroutine_trampoline() as the handler and raising SIGUSR2.  That SIGUSR2
will then lead to the process exiting.

Outside of coroutine-sigaltstack, qemu does not use SIGUSR2.  We can
thus keep the signal handler installed all the time.
CoroutineThreadState.tr_handler tells coroutine_trampoline() whether its
stack is set up so a new coroutine is to be launched (i.e., it should
invoke sigsetjmp()), or not (i.e., the signal came from an external
source and we should just perform the default action, which is to exit
the process).

Note that in user-mode emulation, the guest can register signal handlers
for any signal but SIGSEGV and SIGBUS, so if it registers a SIGUSR2
handler, sigaltstack coroutines will break from then on.  However, we do
not use coroutines for user-mode emulation, so that is fine.

Suggested-by: Laszlo Ersek 
Signed-off-by: Max Reitz 
---
 util/coroutine-sigaltstack.c | 56 +++-
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index aade82afb8..2d32afc322 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -59,6 +59,8 @@ typedef struct {
 
 static pthread_key_t thread_state_key;
 
+static void coroutine_trampoline(int signal);
+
 static CoroutineThreadState *coroutine_get_thread_state(void)
 {
 CoroutineThreadState *s = pthread_getspecific(thread_state_key);
@@ -80,6 +82,7 @@ static void qemu_coroutine_thread_cleanup(void *opaque)
 
 static void __attribute__((constructor)) coroutine_init(void)
 {
+struct sigaction sa;
 int ret;
 
 ret = pthread_key_create(_state_key, qemu_coroutine_thread_cleanup);
@@ -87,6 +90,20 @@ static void __attribute__((constructor)) coroutine_init(void)
 fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
 abort();
 }
+
+/*
+ * Establish the SIGUSR2 signal handler.  This is a process-wide
+ * operation, and so will apply to all threads from here on.
+ */
+sa = (struct sigaction) {
+.sa_handler = coroutine_trampoline,
+.sa_flags   = SA_ONSTACK,
+};
+
+if (sigaction(SIGUSR2, , NULL) != 0) {
+perror("Unable to install SIGUSR2 handler");
+abort();
+}
 }
 
 /* "boot" function
@@ -121,7 +138,17 @@ static void coroutine_trampoline(int signal)
 /* Get the thread specific information */
 coTS = coroutine_get_thread_state();
 self = coTS->tr_handler;
+
+if (!self) {
+/*
+ * This SIGUSR2 came from an external source, not from
+ * qemu_coroutine_new(), so perform the default action.
+ */
+exit(0);
+}
+
 coTS->tr_called = 1;
+coTS->tr_handler = NULL;
 co = >base;
 
 /*
@@ -150,12 +177,9 @@ Coroutine *qemu_coroutine_new(void)
 {
 CoroutineSigAltStack *co;
 CoroutineThreadState *coTS;
-struct sigaction sa;
-struct sigaction osa;
 stack_t ss;
 stack_t oss;
 sigset_t sigs;
-sigset_t osigs;
 sigjmp_buf old_env;
 
 /* The way to manipulate stack is with the sigaltstack function. We
@@ -172,24 +196,6 @@ Coroutine *qemu_coroutine_new(void)
 co->stack = qemu_alloc_stack(>stack_size);
 co->base.entry_arg = _env; /* stash away our jmp_buf */
 
-coTS = coroutine_get_thread_state();
-coTS->tr_handler = co;
-
-/*
- * Preserve the SIGUSR2 signal state, block SIGUSR2,
- * and establish our signal handler. The signal will
- * later transfer control onto the signal stack.
- */
-sigemptyset();
-sigaddset(, SIGUSR2);
-pthread_sigmask(SIG_BLOCK, , );
-sa.sa_handler = coroutine_trampoline;
-sigfillset(_mask);
-sa.sa_flags = SA_ONSTACK;
-if (sigaction(SIGUSR2, , ) != 0) {
-abort();
-}
-
 /*
  * Set the new stack.
  */
@@ -207,6 +213,8 @@ Coroutine *qemu_coroutine_new(void)
  * signal can be delivered the first time sigsuspend() is
  * called.
  */
+coTS = coroutine_get_thread_state();
+coTS->tr_handler = co;
 coTS->tr_called = 0;
 pthread_kill(pthread_self(), SIGUSR2);
 sigfillset();
@@ -230,12 +238,6 @@ Coroutine *qemu_coroutine_new(void)
 sigaltstack(, NULL);
 }
 
-/*
- * Restore the old SIGUSR2 signal handler and mask
- */
-sigaction(SIGUSR2, , NULL);
-pthread_sigmask(SIG_SETMASK, , NULL);
-
 /*
  * Now enter the trampoline again, but this time not as a signal
  * handler. Instead we jump into it directly. The functionally
-- 
2.29.2

Re: [PATCH v2 06/36] block: BdrvChildClass: add .get_parent_aio_context handler

[Vladimir Sementsov-Ogievskiy]

19.01.2021 19:38, Kevin Wolf wrote:

Am 18.01.2021 um 18:36 hat Vladimir Sementsov-Ogievskiy geschrieben:

18.01.2021 18:13, Kevin Wolf wrote:

Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add new handler to get aio context and implement it in all child
classes. Add corresponding public interface to be used soon.

Signed-off-by: Vladimir Sementsov-Ogievskiy 


Hm, are you going to introduce cases where parent and child context
don't match, or why is this a useful function?

Even if so, I feel it shouldn't be any of the child's business what
AioContext the parent uses.

Well, maybe the rest of the series will answer this.


It's for the following patch, to not pass parent (as opaque) with it's
class, and with its ctx in separate. Just to simplify the interface of
the function, we are going to work with a lot.


In a way, the result is nicer because we already assume that ctx is the
parent context when we move things to different AioContexts. So it's
more consistent to just directly take it from the parent.

At the same time, it also complicates things a bit because now we need a
defined state in the middle of an operation that adds, removes or
replaces a child.

I guess I still to make more progress in the review of this series until
I see how you're using the interface.


Hm. I'll take this opportunity to say, that the terminology that calls
graph edge "BdrvChild" always leads to the mess between parents and
children.. "child_class" is a class of parent.. list of parents is
list of children... It all would be a lot simpler to understand if
BdrvChild was BdrvEdge or something like this.


Yeah, that would probably be clearer now that we actually use it to
work with both ends of the edge. And BdrvNode instead of
BlockDriverState, I guess.


Do you think, we can just rename them? Or it would be too painful for 
developers,
who get used to current names? I can make patches


--
Best regards,
Vladimir

Re: [PATCH v7 07/11] iotests: add findtests.py

[Vladimir Sementsov-Ogievskiy]

22.01.2021 14:48, Kevin Wolf wrote:

Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add python script with new logic of searching for tests:

Current ./check behavior:
  - tests are named [0-9][0-9][0-9]
  - tests must be registered in group file (even if test doesn't belong
to any group, like 142)

Behavior of findtests.py:
  - group file is dropped
  - tests are all files in tests/ subdirectory (except for .out files),
so it's not needed more to "register the test", just create it with
appropriate name in tests/ subdirectory. Old names like
[0-9][0-9][0-9] (in root iotests directory) are supported too, but
not recommended for new tests
  - groups are parsed from '# group: ' line inside test files
  - optional file group.local may be used to define some additional
groups for downstreams
  - 'disabled' group is used to temporary disable tests. So instead of
commenting tests in old 'group' file you now can add them to
disabled group with help of 'group.local' file
  - selecting test ranges like 5-15 are not supported more
(to support restarting failed ./check command from the middle of the
 process, new argument is added: --start-from)

Benefits:
  - no rebase conflicts in group file on patch porting from branch to
branch
  - no conflicts in upstream, when different series want to occupy same
test number
  - meaningful names for test files
For example, with digital number, when some person wants to add some
test about block-stream, he most probably will just create a new
test. But if there would be test-block-stream test already, he will
at first look at it and may be just add a test-case into it.
And anyway meaningful names are better.

This commit don't update check behavior (which will be done in further
commit), still, the documentation changed like new behavior is already
here.  Let's live with this small inconsistency for the following few
commits, until final change.

The file findtests.py is self-executable and may be used for debugging
purposes.


As Eric mentioned, this isn't accurate any more.

You mentioned using it as a way to debug things. I assume this is now
covered by the dry run option?


yes




Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  docs/devel/testing.rst  |  50 +-
  tests/qemu-iotests/findtests.py | 159 
  2 files changed, 208 insertions(+), 1 deletion(-)
  create mode 100644 tests/qemu-iotests/findtests.py



+def add_group_file(self, fname: str) -> None:
+with open(fname) as f:
+for line in f:
+line = line.strip()
+
+if (not line) or line[0] == '#':
+continue
+
+words = line.split()
+test_file = self.parse_test_name(words[0])
+groups = words[1:]


The previous version still had this:

+if test_file not in self.all_tests:
+print(f'Warning: {fname}: "{test_file}" test is not found.'
+  ' Skip.')
+continue

Why did you remove it? I found this useful when I had a wrong test name
in my group.local. Now it's silently ignored.



Because now we use parse_test_name which will raise ValueError, so we will not 
go to this if anyway.

So, wrong name will not be silently ignored, check will fail, and you'll have 
to fix group file.. It is suitable?




+for g in groups:
+self.groups[g].add(test_file)


Kevin




--
Best regards,
Vladimir

Re: [PATCH v3 17/21] linux-user/aarch64: Signal SEGV_MTESERR for sync tag check fault

[Peter Maydell]

On Fri, 15 Jan 2021 at 22:47, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/aarch64/target_signal.h | 2 ++
>  linux-user/aarch64/cpu_loop.c  | 3 +++
>  2 files changed, 5 insertions(+)
>
> diff --git a/linux-user/aarch64/target_signal.h 
> b/linux-user/aarch64/target_signal.h
> index ddd73169f0..777fb667fe 100644
> --- a/linux-user/aarch64/target_signal.h
> +++ b/linux-user/aarch64/target_signal.h
> @@ -21,5 +21,7 @@ typedef struct target_sigaltstack {
>
>  #include "../generic/signal.h"
>
> +#define TARGET_SEGV_MTESERR  9  /* Synchronous ARM MTE exception */
> +
>  #define TARGET_ARCH_HAS_SETUP_FRAME
>  #endif /* AARCH64_TARGET_SIGNAL_H */
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index 7811440c68..6867f0db2b 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -133,6 +133,9 @@ void cpu_loop(CPUARMState *env)
>  case 0x0d ... 0x0f: /* Permission fault, level {1-3} */
>  info.si_code = TARGET_SEGV_ACCERR;
>  break;
> +case 0x11: /* Synchronous Tag Check Fault */
> +info.si_code = TARGET_SEGV_MTESERR;
> +break;
>  default:
>  g_assert_not_reached();
>  }
> --
> 2.25.1

Reviewed-by: Peter Maydell 

thanks
-- PMM

[PATCH 4/4] hw/riscv: virt: Map high mmio for PCIe

[Bin Meng]

From: Bin Meng 

Some peripherals require 64-bit PCI address, so let's map the high
mmio space for PCIe.

For RV32, the address is hardcoded to below 4 GiB from the highest
accessible physical address. For RV64, the base address depends on
top of RAM and is aligned to its size which is using 16 GiB for now.

Signed-off-by: Bin Meng 

---

 hw/riscv/virt.c | 36 ++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4f44509360..4ab3b35cc7 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -59,6 +59,15 @@ static const MemMapEntry virt_memmap[] = {
 [VIRT_DRAM] ={ 0x8000,   0x0 },
 };
 
+/* PCIe high mmio is fixed for RV32 */
+#define VIRT32_HIGH_PCIE_MMIO_BASE  0x3ULL
+#define VIRT32_HIGH_PCIE_MMIO_SIZE  (4 * GiB)
+
+/* PCIe high mmio for RV64, size is fixed but base depends on top of RAM */
+#define VIRT64_HIGH_PCIE_MMIO_SIZE  (16 * GiB)
+
+static MemMapEntry virt_high_pcie_memmap;
+
 #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
 
 static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
@@ -371,7 +380,11 @@ static void create_fdt(RISCVVirtState *s, const 
MemMapEntry *memmap,
 2, memmap[VIRT_PCIE_PIO].base, 2, memmap[VIRT_PCIE_PIO].size,
 1, FDT_PCI_RANGE_MMIO,
 2, memmap[VIRT_PCIE_MMIO].base,
-2, memmap[VIRT_PCIE_MMIO].base, 2, memmap[VIRT_PCIE_MMIO].size);
+2, memmap[VIRT_PCIE_MMIO].base, 2, memmap[VIRT_PCIE_MMIO].size,
+1, FDT_PCI_RANGE_MMIO_64BIT,
+2, virt_high_pcie_memmap.base,
+2, virt_high_pcie_memmap.base, 2, virt_high_pcie_memmap.size);
+
 create_pcie_irq_map(fdt, name, plic_pcie_phandle);
 g_free(name);
 
@@ -448,12 +461,14 @@ update_bootargs:
 static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
   hwaddr ecam_base, hwaddr ecam_size,
   hwaddr mmio_base, hwaddr mmio_size,
+  hwaddr high_mmio_base,
+  hwaddr high_mmio_size,
   hwaddr pio_base,
   DeviceState *plic)
 {
 DeviceState *dev;
 MemoryRegion *ecam_alias, *ecam_reg;
-MemoryRegion *mmio_alias, *mmio_reg;
+MemoryRegion *mmio_alias, *high_mmio_alias, *mmio_reg;
 qemu_irq irq;
 int i;
 
@@ -473,6 +488,13 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion 
*sys_mem,
  mmio_reg, mmio_base, mmio_size);
 memory_region_add_subregion(get_system_memory(), mmio_base, mmio_alias);
 
+/* Map high MMIO space */
+high_mmio_alias = g_new0(MemoryRegion, 1);
+memory_region_init_alias(high_mmio_alias, OBJECT(dev), "pcie-mmio-high",
+ mmio_reg, high_mmio_base, high_mmio_size);
+memory_region_add_subregion(get_system_memory(), high_mmio_base,
+high_mmio_alias);
+
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, pio_base);
 
 for (i = 0; i < GPEX_NUM_IRQS; i++) {
@@ -601,6 +623,14 @@ static void virt_machine_init(MachineState *machine)
 machine->ram_size = 10 * GiB;
 error_report("Limitting RAM size to 10 GiB");
 }
+
+virt_high_pcie_memmap.base = VIRT32_HIGH_PCIE_MMIO_BASE;
+virt_high_pcie_memmap.size = VIRT32_HIGH_PCIE_MMIO_SIZE;
+} else {
+virt_high_pcie_memmap.size = VIRT64_HIGH_PCIE_MMIO_SIZE;
+virt_high_pcie_memmap.base = memmap[VIRT_DRAM].base + 
machine->ram_size;
+virt_high_pcie_memmap.base =
+ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
 }
 
 /* register system main memory (actual RAM) */
@@ -686,6 +716,8 @@ static void virt_machine_init(MachineState *machine)
memmap[VIRT_PCIE_ECAM].size,
memmap[VIRT_PCIE_MMIO].base,
memmap[VIRT_PCIE_MMIO].size,
+   virt_high_pcie_memmap.base,
+   virt_high_pcie_memmap.size,
memmap[VIRT_PCIE_PIO].base,
DEVICE(pcie_plic));
 
-- 
2.25.1

Re: [PATCHv8 3/3] arm-virt: add secure pl061 for reset/power down

[Peter Maydell]

On Fri, 22 Jan 2021 at 08:29, Andrew Jones  wrote:
>
> On Wed, Jan 20, 2021 at 12:27:48PM +0300, Maxim Uvarov wrote:
> > Add secure pl061 for reset/power down machine from
> > the secure world (Arm Trusted Firmware). Connect it
> > with gpio-pwr driver.
> >
> > Signed-off-by: Maxim Uvarov 
> > ---
> >  hw/arm/Kconfig|  1 +
> >  hw/arm/virt.c | 47 +++
> >  include/hw/arm/virt.h |  2 ++
> >  3 files changed, 50 insertions(+)
> >
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 0a242e4c5d..13cc42dcc8 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -17,6 +17,7 @@ config ARM_VIRT
> >  select PL011 # UART
> >  select PL031 # RTC
> >  select PL061 # GPIO
> > +select GPIO_PWR
> >  select PLATFORM_BUS
> >  select SMBIOS
> >  select VIRTIO_MMIO
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index c427ce5f81..060a5f492e 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = {
> >  [VIRT_ACPI_GED] =   { 0x0908, ACPI_GED_EVT_SEL_LEN },
> >  [VIRT_NVDIMM_ACPI] ={ 0x0909, NVDIMM_ACPI_IO_LEN},
> >  [VIRT_PVTIME] = { 0x090a, 0x0001 },
> > +[VIRT_SECURE_GPIO] ={ 0x090b, 0x1000 },
> >  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
> >  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that 
> > size */
> >  [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
> > @@ -841,6 +842,43 @@ static void create_gpio_keys(const VirtMachineState 
> > *vms,
> > "gpios", phandle, 3, 0);
> >  }
> >
> > +#define SECURE_GPIO_POWEROFF 0
> > +#define SECURE_GPIO_REBOOT   1
> > +
> > +static void create_gpio_pwr(const VirtMachineState *vms,
>
> This function is specific to the secure view. I think it should have
> "secure" in its name.
>
> > +DeviceState *pl061_dev,
> > +uint32_t phandle)
> > +{
> > +DeviceState *gpio_pwr_dev;
> > +
> > +/* gpio-pwr */
> > +gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL);
>
> Should this device be in secure memory?

It's not in any memory at all -- -1 as the address argument
to sysbus_create_simple() means "no MMIO regions to map". The
only way it's connected to the rest of the system is via  the
secure-only PL061, so the NS world can't get at it.

(sysbus_create_simple("device", -1, NULL) is equivalent to:
 dev = qdev_new("device");
 sysbus_realize_and_unref(SYSBUS_DEVICE(dev), _fatal);
)

thanks
-- PMM

Re: [RFC PATCH] linux-user/mmap: Return EFAULT for invalid addresses

[Philippe Mathieu-Daudé]

Cc'ing maintainer

On 1/22/21 10:37 AM, Richard Purdie wrote:
> On Fri, 2021-01-08 at 17:46 +, Richard Purdie wrote:
>> When using qemu-i386 to run gobject introspection parts of a webkitgtk 
>> build using musl as libc on a 64 bit host, it sits in an infinite loop 
>> of mremap calls of ever decreasing/increasing addresses.
>>
>> I suspect something in the musl memory allocation code loops indefinitely
>> if it only sees ENOMEM and only exits when it hits EFAULT.
>>
>> According to the docs, trying to mremap outside the address space
>> can/should return EFAULT and changing this allows the build to succeed.
>>
>> There was previous discussion of this as it used to work before qemu 2.11
>> and we've carried hacks to work around it since, this appears to be a
>> better fix of the real issue?
>>
>> Signed-off-by: Richard Purdie >
>> Index: qemu-5.2.0/linux-user/mmap.c
>> ===
>> --- qemu-5.2.0.orig/linux-user/mmap.c
>> +++ qemu-5.2.0/linux-user/mmap.c
>> @@ -727,7 +727,7 @@ abi_long target_mremap(abi_ulong old_add
>>   !guest_range_valid(new_addr, new_size)) ||
>>  ((flags & MREMAP_MAYMOVE) == 0 &&
>>   !guest_range_valid(old_addr, new_size))) {
>> -errno = ENOMEM;
>> +errno = EFAULT;
>>  return -1;
>>  }
> 
> Any comments on this? I believe its a valid issue that needs fixing and
> multiple distros appear to be carrying fixes in this area related to
> this.
> 
> Cheers,
> 
> Richard
> 
>