Re: [PATCH v2 6/8] x86: add support for KVM_CAP_XSAVE2 and AMX state migration

[Yang Zhong]

On Mon, Feb 21, 2022 at 01:25:53PM +, David Edmondson wrote:
> On Wednesday, 2022-02-16 at 22:04:32 -08, Yang Zhong wrote:
> 
> > From: Jing Liu 
> >
> > When dynamic xfeatures (e.g. AMX) are used by the guest, the xsave
> > area would be larger than 4KB. KVM_GET_XSAVE2 and KVM_SET_XSAVE
> > under KVM_CAP_XSAVE2 works with a xsave buffer larger than 4KB.
> > Always use the new ioctls under KVM_CAP_XSAVE2 when KVM supports it.
> >
> > Signed-off-by: Jing Liu 
> > Signed-off-by: Zeng Guang 
> > Signed-off-by: Wei Wang 
> > Signed-off-by: Yang Zhong 
> > ---
> >  target/i386/cpu.h  |  4 
> >  target/i386/kvm/kvm.c  | 42 --
> >  target/i386/xsave_helper.c | 33 ++
> >  3 files changed, 64 insertions(+), 15 deletions(-)
> >
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index f7fc2e97a6..de9da38e42 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1528,6 +1528,10 @@ typedef struct CPUX86State {
> >  uint64_t opmask_regs[NB_OPMASK_REGS];
> >  YMMReg zmmh_regs[CPU_NB_REGS];
> >  ZMMReg hi16_zmm_regs[CPU_NB_REGS];
> > +#ifdef TARGET_X86_64
> > +uint8_t xtilecfg[64];
> > +uint8_t xtiledata[8192];
> > +#endif
> 
> Can we have defined constants for these sizes? They also appear in patch
> 2.

  David, the constants we used here are mainly consistent with other members
  in this struct and file.  thanks!

  Yang

Re: [PATCH v2 4/8] x86: Add XFD faulting bit for state components

[Yang Zhong]

On Mon, Feb 21, 2022 at 01:00:41PM +, David Edmondson wrote:
> On Wednesday, 2022-02-16 at 22:04:30 -08, Yang Zhong wrote:
> 
> > From: Jing Liu 
> >
> > Intel introduces XFD faulting mechanism for extended
> > XSAVE features to dynamically enable the features in
> > runtime. If CPUID (EAX=0Dh, ECX=n, n>1).ECX[2] is set
> > as 1, it indicates support for XFD faulting of this
> > state component.
> >
> > Signed-off-by: Jing Liu 
> > Signed-off-by: Yang Zhong 
> 
> Small comment below...
> 
> Reviewed-by: David Edmondson 
> 
> > ---
> >  target/i386/cpu.h | 2 ++
> >  target/i386/cpu.c | 3 ++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index d4ad0f56bd..f7fc2e97a6 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -558,8 +558,10 @@ typedef enum X86Seg {
> >  #define ARCH_REQ_XCOMP_GUEST_PERM   0x1025
> >
> >  #define ESA_FEATURE_ALIGN64_BIT 1
> > +#define ESA_FEATURE_XFD_BIT 2
> >
> >  #define ESA_FEATURE_ALIGN64_MASK(1U << ESA_FEATURE_ALIGN64_BIT)
> > +#define ESA_FEATURE_XFD_MASK(1U << ESA_FEATURE_XFD_BIT)
> >
> >  /* CPUID feature words */
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 377d993438..5a7ee8c7e1 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -5497,7 +5497,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> > uint32_t count,
> >  const ExtSaveArea *esa = _ext_save_areas[count];
> >  *eax = esa->size;
> >  *ebx = esa->offset;
> > -*ecx = esa->ecx & ESA_FEATURE_ALIGN64_MASK;
> > +*ecx = (esa->ecx & ESA_FEATURE_ALIGN64_MASK) |
> > +   (esa->ecx & ESA_FEATURE_XFD_MASK);
> 
> Is:
> 
> *ecx = esa->ecx &
>(ESA_FEATURE_ALIGN64_MASK | ESA_FEATURE_XFD_MASK);
> 
> not more usual?


  Thanks David, I will update this in next version.

  Yang

> 
> >  }
> >  }
> >  break;
> 
> dme.
> -- 
> All of us, we're going out tonight. We're gonna walk all over your cars.

Re: [PATCH v2] Added parameter to take screenshot with screendump as PNG

[Kshitij Suri]

On 24/02/22 9:48 pm, Daniel P. Berrangé wrote:

On Thu, Feb 24, 2022 at 11:59:08AM +, Kshitij Suri wrote:

Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.

Example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image", 
"format":"png" } }

Resolves:https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718=DwIBaQ=s883GpUCOChKOHiocYtGcg=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w=Hu1QTP-aSF7FeXYQcdODcxg2wW1sBEpxaD4jWHYF5kxD2Z6ihhXkxRFOkovubo-f=YwpDKYWnYlYBM7aE1jNrISGXxP9nKm5f9Kfotxm5rZ4=  


Signed-off-by: Kshitij Suri
---
diff to v1:
   - Removed repeated alpha conversion operation.
   - Modified logic to mirror png conversion in vnc-enc-tight.c file.
   - Added a new CONFIG_PNG parameter for libpng support.
   - Changed input format to enum instead of string.

  hmp-commands.hx| 11 +++---
  meson.build| 13 +--
  meson_options.txt  |  2 +
  monitor/hmp-cmds.c |  8 +++-
  qapi/ui.json   | 24 ++--
  ui/console.c   | 97 --
  6 files changed, 139 insertions(+), 16 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..e43c9954b5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,17 +244,18 @@ ERST
  
  {

  .name   = "screendump",
-.args_type  = "filename:F,device:s?,head:i?",
-.params = "filename [device [head]]",
-.help   = "save screen from head 'head' of display device 'device' 
"
-  "into PPM image 'filename'",
+.args_type  = "filename:F,device:s?,head:i?,format:f?",
+.params = "filename [device [head]] [format]",
+.help   = "save screen from head 'head' of display device 'device'"
+  "in specified format 'format' as image 'filename'."
+  "Currently only 'png' and 'ppm' formats are supported.",
  .cmd= hmp_screendump,
  .coroutine  = true,
  },
  
  SRST

  ``screendump`` *filename*
-  Save screen into PPM image *filename*.
+  Save screen as an image *filename*.
  ERST
  
  {

diff --git a/meson.build b/meson.build
index 8df40bfac4..fd550c01ec 100644
--- a/meson.build
+++ b/meson.build
@@ -1112,13 +1112,16 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
 kwargs: static_kwargs)
  endif
-vnc = not_found
  png = not_found
+png = dependency('libpng', required: get_option('png'),
+   method: 'pkg-config', kwargs: static_kwargs)
+vnc = not_found
+vnc_png = not_found
  jpeg = not_found
  sasl = not_found
  if get_option('vnc').allowed() and have_system
vnc = declare_dependency() # dummy dependency
-  png = dependency('libpng', required: get_option('vnc_png'),
+  vnc_png = dependency('libpng', required: get_option('vnc_png'),
 method: 'pkg-config', kwargs: static_kwargs)
jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'),
  method: 'pkg-config', kwargs: static_kwargs)
@@ -1537,9 +1540,10 @@ config_host_data.set('CONFIG_TPM', have_tpm)
  config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
  config_host_data.set('CONFIG_VDE', vde.found())
  config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', 
have_vhost_user_blk_server)
+config_host_data.set('CONFIG_PNG', png.found())
  config_host_data.set('CONFIG_VNC', vnc.found())
  config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
-config_host_data.set('CONFIG_VNC_PNG', png.found())
+config_host_data.set('CONFIG_VNC_PNG', vnc_png.found())


I think we should be removing  CONFIG_VNC_PNG entirely - the VNC
code should just use  CONFIG_PNG.

I'd suggest splitting ths into two patches.  The first patch
updates meson.build to look for libpng unconditionally and
rename to CONFIG_PNG.  The second patch introduces the PNG
support for screendump.
Yes can remove entirely with a combination of CONFIG_VNC and CONFIG_PNG 
as follows


#ifdef CONFIG_VNC_PNG -> #if defined(CONFIG_VNC) && defined(CONFIG_PNG)

But won't removing the vnc_png option cause problems in the build 
scripts of users with the current
version? Instead, can we use the new png meson-option to denote PNG 
format for VNC as well while maintaining backward compatibility? The 
change would look like


vnc_png = dependency('libpng', required: get_option('vnc_png'), method: 
'pkg-config', kwargs: static_kwargs)


gets changed to

vnc_png = dependency('libpng', required: get_option('vnc_png') || 
get_option('png'),
method: 'pkg-config', kwargs: static_kwargs)


  

Re: [PATCH v2] Added parameter to take screenshot with screendump as PNG

[Kshitij Suri]

On 24/02/22 9:32 pm, Eric Blake wrote:

On Thu, Feb 24, 2022 at 11:59:08AM +, Kshitij Suri wrote:

Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.

Example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image", 
"format":"png" } }

Resolves: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718=DwIBAg=s883GpUCOChKOHiocYtGcg=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w=qNJ3bItOdAVHt5dAMXQLkxPL-RWabpnjdw53Hqk9lMbMhTF5Z2PmCjhWiBIiyiII=HjICgN7yF07YFTdi0rumN8vhm3-EwLgjHTuHhZXVc5w=

Signed-off-by: Kshitij Suri 
---
diff to v1:
   - Removed repeated alpha conversion operation.
   - Modified logic to mirror png conversion in vnc-enc-tight.c file.
   - Added a new CONFIG_PNG parameter for libpng support.
   - Changed input format to enum instead of string.

+++ b/qapi/ui.json
@@ -73,12 +73,27 @@
  ##
  { 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
  
+##

+# @ImageFormat:
+#
+# Available list of supported types.
+#
+# @png: PNG format
+#
+# @ppm: PPM format
+#
+# Since: 6.3

The next release is 7.0, not 6.3.


+#
+##
+{ 'enum': 'ImageFormat',
+  'data': ['ppm', 'png'] }
+
  ##
  # @screendump:
  #
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
  #
-# @filename: the path of a new PPM file to store the image
+# @filename: the path of a new file to store the image
  #
  # @device: ID of the display device that should be dumped. If this parameter
  #  is missing, the primary display will be used. (Since 2.12)
@@ -87,6 +102,9 @@
  #parameter is missing, head #0 will be used. Also note that the head
  #can only be specified in conjunction with the device ID. (Since 2.12)
  #
+# @format: image format for screendump is specified. Currently only PNG and
+# PPM are supported.

The second sentence is no longer necessary, as the documentation for
the enum type covers that information now.  Missing a '(since 7.0)' tag.


+#
  # Returns: Nothing on success
  #
  # Since: 0.14
@@ -99,7 +117,7 @@
  #
  ##
  { 'command': 'screendump',
-  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+  'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format': 
'ImageFormat'},

Please wrap the long line.
Thank you for your prompt review! Will update the suggested changes in 
the following patch.


Regards,
Kshitij

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Michael S. Tsirkin]

On Thu, Feb 24, 2022 at 08:34:40PM +, Joao Martins wrote:
> On 2/24/22 20:12, Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2022 at 08:04:48PM +, Joao Martins wrote:
> >> On 2/24/22 19:54, Michael S. Tsirkin wrote:
> >>> On Thu, Feb 24, 2022 at 07:44:26PM +, Joao Martins wrote:
>  On 2/24/22 18:30, Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
> >> On 2/24/22 17:23, Michael S. Tsirkin wrote:
> >>> On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
>  On 2/23/22 23:35, Joao Martins wrote:
> > On 2/23/22 21:22, Michael S. Tsirkin wrote:
> >>> +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
> >>> +  uint64_t 
> >>> pci_hole64_size)
> >>> +{
> >>> +X86MachineState *x86ms = X86_MACHINE(pcms);
> >>> +uint32_t eax, vendor[3];
> >>> +
> >>> +host_cpuid(0x0, 0, , [0], [2], [1]);
> >>> +if (!IS_AMD_VENDOR(vendor)) {
> >>> +return;
> >>> +}
> >>
> >> Wait a sec, should this actually be tying things to the host CPU 
> >> ID?
> >> It's really about what we present to the guest though,
> >> isn't it?
> >
> > It was the easier catch all to use cpuid without going into
> > Linux UAPI specifics. But it doesn't have to tie in there, it is 
> > only
> > for systems with an IOMMU present.
> >
> >> Also, can't we tie this to whether the AMD IOMMU is present?
> >>
> > I think so, I can add that. Something like a amd_iommu_exists() 
> > helper
> > in util/vfio-helpers.c which checks if there's any sysfs child 
> > entries
> > that start with ivhd in /sys/class/iommu/. Given that this HT 
> > region is
> > hardcoded in iommu reserved regions since >=4.11 (to latest) I 
> > don't think it's
> > even worth checking the range exists in:
> >
> > /sys/kernel/iommu_groups/0/reserved_regions
> >
> > (Also that sysfs ABI is >= 4.11 only)
> 
>  Here's what I have staged in local tree, to address your comment.
> 
>  Naturally the first chunk is what's affected by this patch the rest 
>  is a
>  precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to 
>  pass
>  all the tests and what not.
> 
>  I am not entirely sure this is the right place to put such a helper, 
>  open
>  to suggestions. wrt to the naming of the helper, I tried to follow 
>  the rest
>  of the file's style.
> 
>  diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>  index a9be5d33a291..2ea4430d5dcc 100644
>  --- a/hw/i386/pc.c
>  +++ b/hw/i386/pc.c
>  @@ -868,10 +868,8 @@ static void 
>  x86_update_above_4g_mem_start(PCMachineState *pcms,
> uint64_t pci_hole64_size)
>   {
>   X86MachineState *x86ms = X86_MACHINE(pcms);
>  -uint32_t eax, vendor[3];
> 
>  -host_cpuid(0x0, 0, , [0], [2], [1]);
>  -if (!IS_AMD_VENDOR(vendor)) {
>  +if (!qemu_amd_iommu_is_present()) {
>   return;
>   }
> 
>  diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>  index 7bcce3bceb0f..eb4ea071ecec 100644
>  --- a/include/qemu/osdep.h
>  +++ b/include/qemu/osdep.h
>  @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
>    */
>   size_t qemu_get_host_physmem(void);
> 
>  +/**
>  + * qemu_amd_iommu_is_present:
>  + *
>  + * Operating system agnostic way of querying if an AMD IOMMU
>  + * is present.
>  + *
>  + */
>  +bool qemu_amd_iommu_is_present(void);
>  +
>   /*
>    * Toggle write/execute on the pages marked MAP_JIT
>    * for the current thread.
>  diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>  index f2be7321c59f..54cef21217c4 100644
>  --- a/util/oslib-posix.c
>  +++ b/util/oslib-posix.c
>  @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
>   #endif
>   return 0;
>   }
>  +
>  +bool qemu_amd_iommu_is_present(void)
>  +{
>  +bool found = false;
>  +#ifdef CONFIG_LINUX
>  +struct dirent *entry;
>  +char *path;
>  +DIR *dir;
>  +
>  +path = g_strdup_printf("/sys/class/iommu");
>  +dir = opendir(path);
>  +if (!dir) {
>  +g_free(path);
>  +return found;
>  +}
>  

Fix a potential Use-after-free in test_blockjob_common_drain_node() (v6.2.0).

[wliang]

Hi all,

I find a potential Use-after-free in QEMU 6.2.0, which is in 
test_blockjob_common_drain_node() (./tests/unit/test-bdrv-drain.c).

Specifically, at line 880, the variable 'scr' is released by the bdrv_unref(). 
However, at line 881, it is subsequently used as the 1st parameter of the 
function bdrv_set_backing_hd(). As a result, an UAF bug may be triggered.





880bdrv_unref(src);


881bdrv_set_backing_hd(src, src_backing, _abort);





I believe that the problem can be fixed by invoking bdrv_unref() after the call 
of bdrv_set_backing_hd() rather than before it.


---bdrv_unref(src);
881bdrv_set_backing_hd(src, src_backing, _abort);
+++bdrv_unref(src);


It is a test program, so I could't get a mail-list to send. So I send it to 
you. Hope you can help me.
I'm looking forward to your confirmation.

Sincerely Thanks,
Wentao

From 0d631c66441be73666f4ce959fa00754820cd4ea Mon Sep 17 00:00:00 2001
From: Wentao_Liang 
Date: Fri, 25 Feb 2022 12:12:16 +0800
Subject: [PATCH] Fix a potential Use-after-free in
 test_blockjob_common_drain_node()

Signed-off-by: Wentao_Liang 
---
 tests/unit/test-bdrv-drain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 36be84ae55..0e988badc1 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -877,8 +877,8 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
BDRV_O_RDWR, _abort);
 
 bdrv_set_backing_hd(src_overlay, src, _abort);
-bdrv_unref(src);
 bdrv_set_backing_hd(src, src_backing, _abort);
+bdrv_unref(src);
 bdrv_unref(src_backing);
 
 blk_src = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
-- 
2.25.1

Re: [RFC PATCH 3/4] hw/ppc: Take nested guest into account when saving timebase

[David Gibson]

On Thu, Feb 24, 2022 at 03:58:16PM -0300, Fabiano Rosas wrote:
> When saving the guest "timebase" we look to the first_cpu for its
> tb_offset. If that CPU happens to be running a nested guest at this
> time, the tb_offset will have the nested guest value.
> 
> This was caught by code inspection.

This doesn't seem right.  Isn't the real problem that nested_tb_offset
isn't being migrated?  If you migrate that, shouldn't everything be
fixed up when the L1 cpu leaves the nested guest on the destination
host?

> 
> Signed-off-by: Fabiano Rosas 
> ---
>  hw/ppc/ppc.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 9e99625ea9..093cd87014 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -36,6 +36,7 @@
>  #include "kvm_ppc.h"
>  #include "migration/vmstate.h"
>  #include "trace.h"
> +#include "hw/ppc/spapr_cpu_core.h"
>  
>  static void cpu_ppc_tb_stop (CPUPPCState *env);
>  static void cpu_ppc_tb_start (CPUPPCState *env);
> @@ -961,19 +962,33 @@ static void timebase_save(PPCTimebase *tb)
>  {
>  uint64_t ticks = cpu_get_host_ticks();
>  PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
> +int64_t tb_offset;
>  
>  if (!first_ppc_cpu->env.tb_env) {
>  error_report("No timebase object");
>  return;
>  }
>  
> +tb_offset = first_ppc_cpu->env.tb_env->tb_offset;
> +
> +if (first_ppc_cpu->vhyp && vhyp_cpu_in_nested(first_ppc_cpu)) {
> +SpaprCpuState *spapr_cpu = spapr_cpu_state(first_ppc_cpu);
> +
> +/*
> + * If the first_cpu happens to be running a nested guest at
> + * this time, tb_env->tb_offset will contain the nested guest
> + * offset.
> + */
> +tb_offset -= spapr_cpu->nested_tb_offset;
> +}
> +
>  /* not used anymore, we keep it for compatibility */
>  tb->time_of_the_day_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST);
>  /*
>   * tb_offset is only expected to be changed by QEMU so
>   * there is no need to update it from KVM here
>   */
> -tb->guest_timebase = ticks + first_ppc_cpu->env.tb_env->tb_offset;
> +tb->guest_timebase = ticks + tb_offset;
>  
>  tb->runstate_paused =
>  runstate_check(RUN_STATE_PAUSED) || 
> runstate_check(RUN_STATE_SAVE_VM);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH] aio-posix: fix spurious ->poll_ready() callbacks in main loop

[Jason Wang]

在 2022/2/23 下午11:57, Stefan Hajnoczi 写道:

When ->poll() succeeds the AioHandler is placed on the ready list with
revents set to the magic value 0. This magic value causes
aio_dispatch_handler() to invoke ->poll_ready() instead of ->io_read()
for G_IO_IN or ->io_write() for G_IO_OUT.

This magic value 0 hack works for the IOThread where AioHandlers are
placed on ->ready_list and processed by aio_dispatch_ready_handlers().
It does not work for the main loop where all AioHandlers are processed
by aio_dispatch_handlers(), even those that are not ready and have a
revents value of 0.

As a result the main loop invokes ->poll_ready() on AioHandlers that are
not ready. These spurious ->poll_ready() calls waste CPU cycles and
could lead to crashes if the code assumes ->poll() must have succeeded
before ->poll_ready() is called (a reasonable asumption but I haven't
seen it in practice).

Stop using revents to track whether ->poll_ready() will be called on an
AioHandler. Introduce a separate AioHandler->poll_ready field instead.
This eliminates spurious ->poll_ready() calls in the main loop.

Fixes: 826cc32423db2a99d184dbf4f507c737d7e7a4ae ("aio-posix: split poll check from 
ready handler")
Signed-off-by: Stefan Hajnoczi 



Reported-by: Jason Wang 

Tested-by: Jason Wang 

Thanks



---
  util/aio-posix.h |  1 +
  util/aio-posix.c | 32 ++--
  2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/util/aio-posix.h b/util/aio-posix.h
index 7f2c37a684..80b927c7f4 100644
--- a/util/aio-posix.h
+++ b/util/aio-posix.h
@@ -37,6 +37,7 @@ struct AioHandler {
  unsigned flags; /* see fdmon-io_uring.c */
  #endif
  int64_t poll_idle_timeout; /* when to stop userspace polling */
+bool poll_ready; /* has polling detected an event? */
  bool is_external;
  };
  
diff --git a/util/aio-posix.c b/util/aio-posix.c

index 7b9f629218..be0182a3c6 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -23,15 +23,6 @@
  #include "trace.h"
  #include "aio-posix.h"
  
-/*

- * G_IO_IN and G_IO_OUT are not appropriate revents values for polling, since
- * the handler may not need to access the file descriptor. For example, the
- * handler doesn't need to read from an EventNotifier if it polled a memory
- * location and a read syscall would be slow. Define our own unique revents
- * value to indicate that polling determined this AioHandler is ready.
- */
-#define REVENTS_POLL_READY 0
-
  /* Stop userspace polling on a handler if it isn't active for some time */
  #define POLL_IDLE_INTERVAL_NS (7 * NANOSECONDS_PER_SECOND)
  
@@ -49,6 +40,14 @@ void aio_add_ready_handler(AioHandlerList *ready_list,

  QLIST_INSERT_HEAD(ready_list, node, node_ready);
  }
  
+static void aio_add_poll_ready_handler(AioHandlerList *ready_list,

+   AioHandler *node)
+{
+QLIST_SAFE_REMOVE(node, node_ready); /* remove from nested parent's list */
+node->poll_ready = true;
+QLIST_INSERT_HEAD(ready_list, node, node_ready);
+}
+
  static AioHandler *find_aio_handler(AioContext *ctx, int fd)
  {
  AioHandler *node;
@@ -76,6 +75,7 @@ static bool aio_remove_fd_handler(AioContext *ctx, AioHandler 
*node)
  }
  
  node->pfd.revents = 0;

+node->poll_ready = false;
  
  /* If the fd monitor has already marked it deleted, leave it alone */

  if (QLIST_IS_INSERTED(node, node_deleted)) {
@@ -247,7 +247,7 @@ static bool poll_set_started(AioContext *ctx, 
AioHandlerList *ready_list,
  
  /* Poll one last time in case ->io_poll_end() raced with the event */

  if (!started && node->io_poll(node->opaque)) {
-aio_add_ready_handler(ready_list, node, REVENTS_POLL_READY);
+aio_add_poll_ready_handler(ready_list, node);
  progress = true;
  }
  }
@@ -282,6 +282,7 @@ bool aio_pending(AioContext *ctx)
  QLIST_FOREACH_RCU(node, >aio_handlers, node) {
  int revents;
  
+/* TODO should this check poll ready? */

  revents = node->pfd.revents & node->pfd.events;
  if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR) && node->io_read &&
  aio_node_check(ctx, node->is_external)) {
@@ -323,11 +324,15 @@ static void aio_free_deleted_handlers(AioContext *ctx)
  static bool aio_dispatch_handler(AioContext *ctx, AioHandler *node)
  {
  bool progress = false;
+bool poll_ready;
  int revents;
  
  revents = node->pfd.revents & node->pfd.events;

  node->pfd.revents = 0;
  
+poll_ready = node->poll_ready;

+node->poll_ready = false;
+
  /*
   * Start polling AioHandlers when they become ready because activity is
   * likely to continue.  Note that starvation is theoretically possible 
when
@@ -344,7 +349,7 @@ static bool aio_dispatch_handler(AioContext *ctx, 
AioHandler *node)
  QLIST_INSERT_HEAD(>poll_aio_handlers, node, node_poll);
  }
  if (!QLIST_IS_INSERTED(node, node_deleted) &&
-revents == 0 &&
+   

Re: Re: Fix a potential Use-after-free bug in handle_simd_shift_fpint_conv() (v6.2.0).

[wliang]

> 
> The fix is correct.  We just need the submission formatted properly, with 
> your 
> Signed-off-by tag.  When re-formatting, you can add my
> 
> Reviewed-by: Richard Henderson 
> 

> r~

Hi guys,

Thank you for waiting for me.

Here is a new patch with Signed-off-by tags.

Best,
Wentao
From 15129e2cec483a8416738b266bc3b36d56959f69 Mon Sep 17 00:00:00 2001
From: Wentao_Liang 
Date: Fri, 25 Feb 2022 12:01:42 +0800
Subject: [PATCH] Fix a potential Use-after-free bug in
 handle_simd_shift_fpint_conv()

Signed-off-by: Wentao_Liang 
---
 target/arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 5a1df25f91..d1a59fad9c 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -9045,9 +9045,9 @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar,
 }
 }
 
-tcg_temp_free_ptr(tcg_fpstatus);
 tcg_temp_free_i32(tcg_shift);
 gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus);
+tcg_temp_free_ptr(tcg_fpstatus);
 tcg_temp_free_i32(tcg_rmode);
 }
 
-- 
2.25.1

Re: [PATCH v6 01/19] configure, meson: override C compiler for cmake

[Jag Raman]

> On Feb 24, 2022, at 12:52 PM, Paolo Bonzini  wrote:
> 
> On 2/22/22 20:05, Jag Raman wrote:
>>> -defaults[prefix + 'COMPILER'] = exe_list
>>> +defaults[f'{prefix}COMPILER'] = [exe_list[0]]
>>> +for i in range(1, len(exe_list)):
>>> +defaults[f'{prefix}COMPILER_ARG{i}'] = [exe_list[i]]
>>> +
>>> if comp_obj.get_id() == 'clang-cl':
>>> defaults['CMAKE_LINKER'] = comp_obj.get_linker_exelist()
>> This fix works at my end.
> 
> Would you please check that -m64 and -mcx16 are passed indeed to the compiler?

Hi Paolo,

Yes, I’m able to see that -m64 and -mcx16 are passed to the compiler.

# cat ./subprojects/libvfio-user/__CMake_build/CMakeMesonToolchainFile.cmake
…
set(CMAKE_C_COMPILER "/opt/rh/devtoolset-9/root/usr/bin/cc")
set(CMAKE_C_COMPILER_ARG1 "-m64")
set(CMAKE_C_COMPILER_ARG2 "-mcx16")
…

Full log here: https://pastebin.com/PEwNSWMn

Thank you!
--
Jag

> 
> Paolo

Re: Fix a potential Use-after-free in virtio_iommu_handle_command() (v6.2.0).

[wliang]

Hi all,

Here is a new patch with Signed-off-by tags.
The old one is wrong for it did't have Signed-off-by tags.
I am looking forward to your confirmation.

Thanks,
Wentao
From 8ece42bda1099a9a0df584cac2478ec5a6e83924 Mon Sep 17 00:00:00 2001
From: Wentao_Liang 
Date: Fri, 25 Feb 2022 11:49:54 +0800
Subject: [PATCH] Fix a potential Use-after-free in
 virtio_iommu_handle_command() (v6.2.0).

Signed-off-by: Wentao_Liang 
---
 hw/virtio/virtio-iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index aa9c16a17b..a394901347 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -657,6 +657,7 @@ out:
 virtio_notify(vdev, vq);
 g_free(elem);
 g_free(buf);
+buf = NULL;
 }
 }
 
-- 
2.25.1

Re: [RFC PATCH 0/4] ppc: nested TCG migration (KVM-on-TCG)

[David Gibson]

On Thu, Feb 24, 2022 at 09:00:24PM +, Mark Cave-Ayland wrote:
> On 24/02/2022 18:58, Fabiano Rosas wrote:
> 
> > This series implements the migration for a TCG pseries guest running a
> > nested KVM guest. This is just like migrating a pseries TCG guest, but
> > with some extra state to allow a nested guest to continue to run on
> > the destination.
> > 
> > Unfortunately the regular TCG migration scenario (not nested) is not
> > fully working so I cannot be entirely sure the nested migration is
> > correct. I have included a couple of patches for the general migration
> > case that (I think?) improve the situation a bit, but I'm still seeing
> > hard lockups and other issues with more than 1 vcpu.
> > 
> > This is more of an early RFC to see if anyone spots something right
> > away. I haven't made much progress in debugging the general TCG
> > migration case so if anyone has any input there as well I'd appreciate
> > it.
> > 
> > Thanks
> > 
> > Fabiano Rosas (4):
> >target/ppc: TCG: Migrate tb_offset and decr
> >spapr: TCG: Migrate spapr_cpu->prod
> >hw/ppc: Take nested guest into account when saving timebase
> >spapr: Add KVM-on-TCG migration support
> > 
> >   hw/ppc/ppc.c| 17 +++-
> >   hw/ppc/spapr.c  | 19 
> >   hw/ppc/spapr_cpu_core.c | 77 +
> >   include/hw/ppc/spapr_cpu_core.h |  2 +-
> >   target/ppc/machine.c| 61 ++
> >   5 files changed, 174 insertions(+), 2 deletions(-)
> 
> FWIW I noticed there were some issues with migrating the decrementer on Mac
> machines a while ago which causes a hang on the destination with TCG (for
> MacOS on a x86 host in my case). Have a look at the following threads for
> reference:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00546.html
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04622.html
> 
> IIRC there is code that assumes any migration in PPC is being done live, and
> so adjusts the timebase on the destination to reflect wall clock time by
> recalculating tb_offset. I haven't looked at the code for a while but I
> think the outcome was that there needs to be 2 phases in migration: the
> first is to migrate the timebase as-is for guests that are paused during
> migration, whilst the second is to notify hypervisor-aware guest OSs such as
> Linux to make the timebase adjustment if required if the guest is running.

Whether the timebase is adjusted for the migration downtime depends
whether the guest clock is pinned to wall clock time or not.  Usually
it should be (because you don't want your clocks to go wrong on
migration of a production system).  However in neither case should be
the guest be involved.

There may be guest side code related to this in Linux, but that's
probably for migration under pHyp, which is a guest aware migration
system.  That's essentially unrelated to migration under qemu/kvm,
which is a guest unaware system.

Guest aware migration has some nice-sounding advantages; in particular
itcan allow migrations across a heterogenous cluster with differences
between hosts that the hypervisor can't hide, or can't efficiently
hide.  However, it is IMO, a deeply broken approach, because it can
allow an un-cooperative guest to indefinitely block migration, and for
it to be reliably correct it requires *much* more pinning down of
exactly what host system changes the guest can and can't be expected
to cope with than PAPR has ever bothered to do.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [RFC PATCH 4/4] spapr: Add KVM-on-TCG migration support

[David Gibson]

On Thu, Feb 24, 2022 at 03:58:17PM -0300, Fabiano Rosas wrote:
> This adds migration support for TCG pseries machines running a KVM-HV
> guest.
> 
> The state that needs to be migrated is:
> 
> - the nested PTCR value;
> - the in_nested flag;
> - the nested_tb_offset.
> - the saved host CPUPPCState structure;
> 
> Signed-off-by: Fabiano Rosas 
> 
> ---
> (this migrates just fine with L2 running stress and 1 VCPU in L1. With
> 32 VCPUs in L1 there's crashes which I still don't understand. They might
> be related to L1 migration being flaky right now)
> ---
>  hw/ppc/spapr.c  | 19 +++
>  hw/ppc/spapr_cpu_core.c | 76 +
>  target/ppc/machine.c| 44 
>  3 files changed, 139 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f0b75b22bb..6e87c515db 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1934,6 +1934,13 @@ static bool spapr_patb_entry_needed(void *opaque)
>  return !!spapr->patb_entry;
>  }
>  
> +static bool spapr_nested_ptcr_needed(void *opaque)
> +{
> +SpaprMachineState *spapr = opaque;
> +
> +return !!spapr->nested_ptcr;
> +}
> +
>  static const VMStateDescription vmstate_spapr_patb_entry = {
>  .name = "spapr_patb_entry",
>  .version_id = 1,
> @@ -1945,6 +1952,17 @@ static const VMStateDescription 
> vmstate_spapr_patb_entry = {
>  },
>  };
>  
> +static const VMStateDescription vmstate_spapr_nested_ptcr = {
> +.name = "spapr_nested_ptcr",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = spapr_nested_ptcr_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT64(nested_ptcr, SpaprMachineState),
> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
>  static bool spapr_irq_map_needed(void *opaque)
>  {
>  SpaprMachineState *spapr = opaque;
> @@ -2069,6 +2087,7 @@ static const VMStateDescription vmstate_spapr = {
>  _spapr_cap_fwnmi,
>  _spapr_fwnmi,
>  _spapr_cap_rpt_invalidate,
> +_spapr_nested_ptcr,

Ok, the nested_ptcr stuff looks good.

>  NULL
>  }
>  };
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index efda7730f1..3ec13c0660 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -25,6 +25,7 @@
>  #include "sysemu/reset.h"
>  #include "sysemu/hw_accel.h"
>  #include "qemu/error-report.h"
> +#include "migration/cpu.h"
>  
>  static void spapr_reset_vcpu(PowerPCCPU *cpu)
>  {
> @@ -174,6 +175,80 @@ static const VMStateDescription vmstate_spapr_cpu_vpa = {
>  }
>  };
>  
> +static bool nested_needed(void *opaque)
> +{
> +SpaprCpuState *spapr_cpu = opaque;
> +
> +return spapr_cpu->in_nested;
> +}
> +
> +static int nested_state_pre_save(void *opaque)
> +{
> +CPUPPCState *env = opaque;
> +
> +env->spr[SPR_LR] = env->lr;
> +env->spr[SPR_CTR] = env->ctr;
> +env->spr[SPR_XER] = cpu_read_xer(env);
> +env->spr[SPR_CFAR] = env->cfar;
> +return 0;
> +}
> +
> +static int nested_state_post_load(void *opaque, int version_id)
> +{
> +CPUPPCState *env = opaque;
> +
> +env->lr = env->spr[SPR_LR];
> +env->ctr = env->spr[SPR_CTR];
> +cpu_write_xer(env, env->spr[SPR_XER]);
> +env->cfar = env->spr[SPR_CFAR];
> +
> +return 0;
> +}
> +
> +static const VMStateDescription vmstate_nested_host_state = {
> +.name = "spapr_nested_host_state",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.pre_save = nested_state_pre_save,
> +.post_load = nested_state_post_load,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINTTL_ARRAY(gpr, CPUPPCState, 32),
> +VMSTATE_UINTTL_ARRAY(spr, CPUPPCState, 1024),
> +VMSTATE_UINT32_ARRAY(crf, CPUPPCState, 8),
> +VMSTATE_UINTTL(nip, CPUPPCState),
> +VMSTATE_UINTTL(msr, CPUPPCState),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static int nested_cpu_pre_load(void *opaque)
> +{
> +SpaprCpuState *spapr_cpu = opaque;
> +
> +spapr_cpu->nested_host_state = g_try_malloc(sizeof(CPUPPCState));
> +if (!spapr_cpu->nested_host_state) {
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +static const VMStateDescription vmstate_spapr_cpu_nested = {
> +.name = "spapr_cpu/nested",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = nested_needed,
> +.pre_load = nested_cpu_pre_load,
> +.fields = (VMStateField[]) {
> +VMSTATE_BOOL(in_nested, SpaprCpuState),
> +VMSTATE_INT64(nested_tb_offset, SpaprCpuState),
> +VMSTATE_STRUCT_POINTER_V(nested_host_state, SpaprCpuState, 1,
> + vmstate_nested_host_state, CPUPPCState),
> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
>  static const VMStateDescription vmstate_spapr_cpu_state = {
>  .name = "spapr_cpu",
>  .version_id = 1,
> @@ -184,6 +259,7 @@ static const VMStateDescription vmstate_spapr_cpu_state = 
> {
>  },
>  .subsections = (const 

Re: [RFC PATCH 1/4] target/ppc: TCG: Migrate tb_offset and decr

[David Gibson]

On Thu, Feb 24, 2022 at 03:58:14PM -0300, Fabiano Rosas wrote:
> These two were not migrated so the remote end was starting with the
> decrementer expired.
> 
> I am seeing less frequent crashes with this patch (tested with -smp 4
> and -smp 32). It certainly doesn't fix all issues but it looks like it
> helps.
> 
> Signed-off-by: Fabiano Rosas 

Nack.  This completely breaks migration compatibility for all ppc
machines.  In order to maintain compatibility as Richard says new info
has to go into a subsection, with a needed function that allows
migration of existing machine types both to and from a new qemu
version.

There are also some other problems.

> ---
>  target/ppc/machine.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 1b63146ed1..7ee1984500 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -9,6 +9,7 @@
>  #include "qemu/main-loop.h"
>  #include "kvm_ppc.h"
>  #include "power8-pmu.h"
> +#include "hw/ppc/ppc.h"
>  
>  static void post_load_update_msr(CPUPPCState *env)
>  {
> @@ -666,6 +667,18 @@ static const VMStateDescription vmstate_compat = {
>  }
>  };
>  
> +static const VMStateDescription vmstate_tb_env = {
> +.name = "cpu/env/tb_env",

Because spapr requires that all cpus have synchronized timebases, we
migrate the timebase state from vmstate_spapr, not from each cpu
individually, to make sure it stays synchronized across migration.  If
that's not working right, that's a bug, but it needs to be fixed
there, not just plastered over with extra information transmitted at
cpu level.

> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_INT64(tb_offset, ppc_tb_t),

tb_offset isn't a good thing to put directly in the migration stream.
tb_offset has kind of non-obvious semantics to begin with: when we're
dealing with TCG (which is your explicit case), there is no host TB,
so what's this an offset from?  That's why vmstate_ppc_timebase uses
an explicit guest timebase value matched with a time of day in real
units.  Again, if there's a bug, that needs fixing there.

> +VMSTATE_UINT64(decr_next, ppc_tb_t),
> +VMSTATE_TIMER_PTR(decr_timer, ppc_tb_t),

You're attempting to migrate decr_next and decr_timer, but not the
actual DECR value, which makes me suspect that *is* being migrated.
In which case you should be able to reconstruct the next tick and
timer state in a post_load function on receive.  If that's buggy, it
needs to be fixed there.

> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
>  const VMStateDescription vmstate_ppc_cpu = {
>  .name = "cpu",
>  .version_id = 5,
> @@ -696,12 +709,16 @@ const VMStateDescription vmstate_ppc_cpu = {
>  /* Backward compatible internal state */
>  VMSTATE_UINTTL(env.hflags_compat_nmsr, PowerPCCPU),
>  
> +VMSTATE_STRUCT_POINTER_V(env.tb_env, PowerPCCPU, 1,
> + vmstate_tb_env, ppc_tb_t),
> +
>  /* Sanity checking */
>  VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
>  VMSTATE_UINT64_TEST(mig_insns_flags, PowerPCCPU, 
> cpu_pre_2_8_migration),
>  VMSTATE_UINT64_TEST(mig_insns_flags2, PowerPCCPU,
>  cpu_pre_2_8_migration),
>  VMSTATE_UINT32_TEST(mig_nb_BATs, PowerPCCPU, cpu_pre_2_8_migration),
> +
>  VMSTATE_END_OF_LIST()
>  },
>  .subsections = (const VMStateDescription*[]) {

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [RFC PATCH 2/4] spapr: TCG: Migrate spapr_cpu->prod

[David Gibson]

On Thu, Feb 24, 2022 at 03:58:15PM -0300, Fabiano Rosas wrote:
> I'm seeing some stack traces in the migrated guest going through cede
> and some hangs at the plpar_hcall_norets so let's make sure everything
> related to cede/prod is being migrated just in case.

This is a poor approach in general.  Migration becomes even harder to
maintain than it already is if you don't pare down the set of migrated
data to something minimal and non-redundant.

If you want to migrate prod, you have to give a case for why you
*need* it, not "just in case".

Also, you have to put this in a subsection with a needed function in
order not to break compatibility.

> 
> Signed-off-by: Fabiano Rosas 
> ---
>  hw/ppc/spapr_cpu_core.c | 1 +
>  include/hw/ppc/spapr_cpu_core.h | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index ed84713960..efda7730f1 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -179,6 +179,7 @@ static const VMStateDescription vmstate_spapr_cpu_state = 
> {
>  .version_id = 1,
>  .minimum_version_id = 1,
>  .fields = (VMStateField[]) {
> +VMSTATE_BOOL(prod, SpaprCpuState),
>  VMSTATE_END_OF_LIST()
>  },
>  .subsections = (const VMStateDescription * []) {
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index b560514560..2772689c84 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -45,7 +45,7 @@ typedef struct SpaprCpuState {
>  uint64_t vpa_addr;
>  uint64_t slb_shadow_addr, slb_shadow_size;
>  uint64_t dtl_addr, dtl_size;
> -bool prod; /* not migrated, only used to improve dispatch latencies */
> +bool prod;
>  struct ICPState *icp;
>  struct XiveTCTX *tctx;
>  

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Jason Wang]

On Fri, Feb 25, 2022 at 2:30 AM Michael S. Tsirkin  wrote:
>
> On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
> > On 2/24/22 17:23, Michael S. Tsirkin wrote:
> > > On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
> > >> On 2/23/22 23:35, Joao Martins wrote:
> > >>> On 2/23/22 21:22, Michael S. Tsirkin wrote:
> > > +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
> > > +  uint64_t pci_hole64_size)
> > > +{
> > > +X86MachineState *x86ms = X86_MACHINE(pcms);
> > > +uint32_t eax, vendor[3];
> > > +
> > > +host_cpuid(0x0, 0, , [0], [2], [1]);
> > > +if (!IS_AMD_VENDOR(vendor)) {
> > > +return;
> > > +}
> > 
> >  Wait a sec, should this actually be tying things to the host CPU ID?
> >  It's really about what we present to the guest though,
> >  isn't it?
> > >>>
> > >>> It was the easier catch all to use cpuid without going into
> > >>> Linux UAPI specifics. But it doesn't have to tie in there, it is only
> > >>> for systems with an IOMMU present.
> > >>>
> >  Also, can't we tie this to whether the AMD IOMMU is present?
> > 
> > >>> I think so, I can add that. Something like a amd_iommu_exists() helper
> > >>> in util/vfio-helpers.c which checks if there's any sysfs child entries
> > >>> that start with ivhd in /sys/class/iommu/. Given that this HT region is
> > >>> hardcoded in iommu reserved regions since >=4.11 (to latest) I don't 
> > >>> think it's
> > >>> even worth checking the range exists in:
> > >>>
> > >>>   /sys/kernel/iommu_groups/0/reserved_regions
> > >>>
> > >>> (Also that sysfs ABI is >= 4.11 only)
> > >>
> > >> Here's what I have staged in local tree, to address your comment.
> > >>
> > >> Naturally the first chunk is what's affected by this patch the rest is a
> > >> precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to pass
> > >> all the tests and what not.
> > >>
> > >> I am not entirely sure this is the right place to put such a helper, open
> > >> to suggestions. wrt to the naming of the helper, I tried to follow the 
> > >> rest
> > >> of the file's style.
> > >>
> > >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > >> index a9be5d33a291..2ea4430d5dcc 100644
> > >> --- a/hw/i386/pc.c
> > >> +++ b/hw/i386/pc.c
> > >> @@ -868,10 +868,8 @@ static void 
> > >> x86_update_above_4g_mem_start(PCMachineState *pcms,
> > >>uint64_t pci_hole64_size)
> > >>  {
> > >>  X86MachineState *x86ms = X86_MACHINE(pcms);
> > >> -uint32_t eax, vendor[3];
> > >>
> > >> -host_cpuid(0x0, 0, , [0], [2], [1]);
> > >> -if (!IS_AMD_VENDOR(vendor)) {
> > >> +if (!qemu_amd_iommu_is_present()) {
> > >>  return;
> > >>  }
> > >>
> > >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > >> index 7bcce3bceb0f..eb4ea071ecec 100644
> > >> --- a/include/qemu/osdep.h
> > >> +++ b/include/qemu/osdep.h
> > >> @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
> > >>   */
> > >>  size_t qemu_get_host_physmem(void);
> > >>
> > >> +/**
> > >> + * qemu_amd_iommu_is_present:
> > >> + *
> > >> + * Operating system agnostic way of querying if an AMD IOMMU
> > >> + * is present.
> > >> + *
> > >> + */
> > >> +bool qemu_amd_iommu_is_present(void);
> > >> +
> > >>  /*
> > >>   * Toggle write/execute on the pages marked MAP_JIT
> > >>   * for the current thread.
> > >> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> > >> index f2be7321c59f..54cef21217c4 100644
> > >> --- a/util/oslib-posix.c
> > >> +++ b/util/oslib-posix.c
> > >> @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
> > >>  #endif
> > >>  return 0;
> > >>  }
> > >> +
> > >> +bool qemu_amd_iommu_is_present(void)
> > >> +{
> > >> +bool found = false;
> > >> +#ifdef CONFIG_LINUX
> > >> +struct dirent *entry;
> > >> +char *path;
> > >> +DIR *dir;
> > >> +
> > >> +path = g_strdup_printf("/sys/class/iommu");
> > >> +dir = opendir(path);
> > >> +if (!dir) {
> > >> +g_free(path);
> > >> +return found;
> > >> +}
> > >> +
> > >> +do {
> > >> +entry = readdir(dir);
> > >> +if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
> > >> +found = true;
> > >> +break;
> > >> +}
> > >> +} while (entry);
> > >> +
> > >> +g_free(path);
> > >> +closedir(dir);
> > >> +#endif
> > >> +return found;
> > >> +}
> > >
> > > why are we checking whether an AMD IOMMU is present
> > > on the host?
> > > Isn't what we care about whether it's
> > > present in the VM? What we are reserving after all
> > > is a range of GPAs, not actual host PA's ...
> > >
> > Right.
> >
> > But any DMA map done by VFIO/IOMMU using those GPA ranges will fail,
> > and so we need to not map that portion of address space entirely
> > and use some other portion of the GPA-space. This has to
> > do with 

Re: [PATCH v2] target/arm: Support PSCI 1.1 and SMCCC 1.0

[Akihiko Odaki]

On 2022/02/24 21:53, Peter Maydell wrote:

On Sun, 13 Feb 2022 at 03:58, Akihiko Odaki  wrote:


Support the latest PSCI on TCG and HVF. A 64-bit function called from
AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC
Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since
they do not implement mandatory functions.

Signed-off-by: Akihiko Odaki 
---


Applied, thanks.

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

(I noticed while reviewing this that we report KVM's PSCI via
the DTB as only 0.2 even if KVM's actually implementing better
than that; I'll write a patch to clean that up.)

-- PMM


I don't have an account on https://wiki.qemu.org/ so can you create one? 
I'll update the changelog once I get access to the account.


Regards,
Akihiko Odaki

Re: Fix a potential memory leak bug in write_boot_rom() (v6.2.0).

[wliang]

> 
> yes. Could you please send a patch using  g_autofree ?
> 
> Thanks,
> 
> C.


Here is the new patch.

Thanks,
WentaoFrom 8ed76446f78ab1b4152403fdb9dd6f349d6fd52e Mon Sep 17 00:00:00 2001
From: Wentao_Liang 
Date: Fri, 25 Feb 2022 11:17:33 +0800
Subject: [PATCH] Fix a potential memory leak bug in write_boot_rom() (v6.2.0).

Signed-off-by: Wentao_Liang 
---
 hw/arm/aspeed.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d911dc904f..9da5f87429 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -276,6 +276,7 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
 storage = g_new0(uint8_t, rom_size);
 if (blk_pread(blk, 0, storage, rom_size) < 0) {
 error_setg(errp, "failed to read the initial flash content");
+g_free(storage);
 return;
 }
 
-- 
2.25.1

RE: [PATCH] vl: transform QemuOpts device to JSON syntax device

[Duan, Zhenzhong]

>-Original Message-
>From: Kevin Wolf 
>Sent: Thursday, February 24, 2022 7:31 PM
>To: Duan, Zhenzhong 
>Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; ebl...@redhat.com;
>m...@redhat.com; pkre...@redhat.com; ler...@redhat.com
>Subject: Re: [PATCH] vl: transform QemuOpts device to JSON syntax device
>
>Am 24.02.2022 um 07:06 hat Zhenzhong Duan geschrieben:
>> While there are mixed use of traditional -device option and JSON
>> syntax option, QEMU reports conflict, e.x:
>>
>> /usr/libexec/qemu-kvm -nodefaults \
>>   -device '{"driver":"virtio-scsi-
>pci","id":"scsi0","bus":"pci.0","addr":"0x02.0"}' \
>>   -device virtio-scsi-pci,id=scsi1,bus=pci.0
>>
>> It breaks with:
>>
>> qemu-kvm: -device
>> {"driver":"virtio-scsi-pci","id":"scsi0","bus":"pci.0","addr":"0x02.0"
>> }: PCI: slot 2 function 0 not available for virtio-scsi-pci, in use by
>> virtio-scsi-pci
>>
>> But if we reformat first -device same as the second, so only same kind
>> of option for all the devices, it succeeds, vice versa. e.x:
>>
>> /usr/libexec/qemu-kvm -nodefaults \
>>   -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=02.0 \
>>   -device virtio-scsi-pci,id=scsi1,bus=pci.0
>>
>> Succeed!
>>
>> Because both kind of options are inserted into their own list and
>> break the order in QEMU command line during BDF auto assign. Fix it by
>> transform QemuOpts into JSON syntax and insert in JSON device list, so
>> the order in QEMU command line kept.
>>
>> Signed-off-by: Zhenzhong Duan 
>
>This patch is incorrect and breaks several cases, which are the reason why the
>QemuOpts path hasn't been changed yet.
>
>For example, after this patch, help doesn't work any more:
>
>$ build/qemu-system-x86_64 -device help
>qemu-system-x86_64: -device help: 'help' is not a valid device model name
>
>Any non-string property doesn't work any more in non-JSON syntax:
>
>$ $ build/qemu-system-x86_64 -blockdev null-co,node-name=disk -device
>virtio-blk,drive=disk,physical_block_size=4096
>qemu-system-x86_64: -device virtio-blk,drive=disk,physical_block_size=4096:
>Parameter 'physical_block_size' expects uint64
>
>There may be more cases that are broken with this patch.

Ah, yes. Thanks for point out. I should have learned more before writing this 
patch. Sorry for the noise. And we will try the libvirt maintainer suggested 
way on this issue.

Regards
Zhenzhong

Re: [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt

[Richard Henderson]

On 2/24/22 14:23, Richard Henderson wrote:

On 2/14/22 03:26, Luc Michel wrote:

In some cases, cpu->exit_request can be false after handling the
interrupt, leading to another TB being executed instead of returning
to the main loop.

Fix this by returning true unconditionally when in single-step mode.

Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166

Signed-off-by: Luc Michel 
---
Coming back on this issue I worked on with Richard in 2020. The issue is
that when debugging the guest with GDB, the first instruction of the IRQ
handler is missed by GDB (it's still executed though).

It happened to me again in TCG RR mode (but not in MTTCG). It seems that
cpu->exit_request can be false in RR mode when returning from
cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
returning false and the next TB being executed, instead of the EXCP_DEBUG
being handled.
---
  accel/tcg/cpu-exec.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 8b4cd6c59d..74d7f83f34 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
  /*
   * After processing the interrupt, ensure an EXCP_DEBUG is
   * raised when single-stepping so that GDB doesn't miss the
   * next instruction.
   */
-    cpu->exception_index =
-    (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
-    *last_tb = NULL;
+    if (unlikely(cpu->singlestep_enabled)) {
+    cpu->exception_index = EXCP_DEBUG;
+    return true;


By returning here, you also need to qemu_mutex_unlock_iothread().


+    } else {


You can remove the else after the return.
Otherwise this looks good; sorry for the delay.


I'll just fix this up when applying, it's minor enough.

r~

Re: [RFC PATCH 4/4] spapr: Add KVM-on-TCG migration support

[Nicholas Piggin]

Excerpts from Fabiano Rosas's message of February 25, 2022 4:58 am:
> This adds migration support for TCG pseries machines running a KVM-HV
> guest.
> 
> The state that needs to be migrated is:
> 
> - the nested PTCR value;
> - the in_nested flag;
> - the nested_tb_offset.
> - the saved host CPUPPCState structure;
> 
> Signed-off-by: Fabiano Rosas 

The series generally looks good to me, I guess patches 1 and 2 are
fixes that could go ahead.

Main thing about this is I was thinking of cutting down the CPUPPCState
structure for saving the host state when in the L2, and making a
specific structure for that that only contains what is required.

This patch could easily switch to that so it's no big deal AFAIKS.

> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 7ee1984500..ae09b1bcfe 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -10,6 +10,7 @@
>  #include "kvm_ppc.h"
>  #include "power8-pmu.h"
>  #include "hw/ppc/ppc.h"
> +#include "hw/ppc/spapr_cpu_core.h"
>  
>  static void post_load_update_msr(CPUPPCState *env)
>  {
> @@ -679,6 +680,48 @@ static const VMStateDescription vmstate_tb_env = {
>  }
>  };
>  
> +static const VMStateDescription vmstate_hdecr = {
> +.name = "cpu/hdecr",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT64(hdecr_next, ppc_tb_t),
> +VMSTATE_TIMER_PTR(hdecr_timer, ppc_tb_t),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static bool nested_needed(void *opaque)
> +{
> +PowerPCCPU *cpu = opaque;
> +SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +return spapr_cpu->in_nested;
> +}

I don't know the migration code -- are you assured of having a
spapr CPU here?

Maybe this could call a helper function located near the spapr/nested
code like 'return ppc_cpu_need_hdec_migrate(cpu)' ?

Thanks,
Nick

Re: [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt

[Richard Henderson]

On 2/14/22 03:26, Luc Michel wrote:

In some cases, cpu->exit_request can be false after handling the
interrupt, leading to another TB being executed instead of returning
to the main loop.

Fix this by returning true unconditionally when in single-step mode.

Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166

Signed-off-by: Luc Michel 
---
Coming back on this issue I worked on with Richard in 2020. The issue is
that when debugging the guest with GDB, the first instruction of the IRQ
handler is missed by GDB (it's still executed though).

It happened to me again in TCG RR mode (but not in MTTCG). It seems that
cpu->exit_request can be false in RR mode when returning from
cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
returning false and the next TB being executed, instead of the EXCP_DEBUG
being handled.
---
  accel/tcg/cpu-exec.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 8b4cd6c59d..74d7f83f34 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
  /*
   * After processing the interrupt, ensure an EXCP_DEBUG is
   * raised when single-stepping so that GDB doesn't miss the
   * next instruction.
   */
-cpu->exception_index =
-(cpu->singlestep_enabled ? EXCP_DEBUG : -1);
-*last_tb = NULL;
+if (unlikely(cpu->singlestep_enabled)) {
+cpu->exception_index = EXCP_DEBUG;
+return true;


By returning here, you also need to qemu_mutex_unlock_iothread().


+} else {


You can remove the else after the return.
Otherwise this looks good; sorry for the delay.


r~

Re: [PATCH v2 14/14] target: Use ArchCPU as interface to target CPU

[Richard Henderson]

On 2/14/22 08:31, Philippe Mathieu-Daudé wrote:

ArchCPU is our interface with target-specific code. Use it as
a forward-declared opaque pointer (abstract type), having its
structure defined by each target.

Signed-off-by: Philippe Mathieu-Daudé
---
  include/hw/core/cpu.h   | 4 ++--
  include/qemu/typedefs.h | 1 +
  target/alpha/cpu.h  | 2 +-
  target/arm/cpu.h| 2 +-
  target/avr/cpu.h| 2 +-
  target/cris/cpu.h   | 2 +-
  target/hexagon/cpu.h| 2 +-
  target/hppa/cpu.h   | 2 +-
  target/i386/cpu.h   | 2 +-
  target/m68k/cpu.h   | 2 +-
  target/microblaze/cpu.h | 2 +-
  target/mips/cpu.h   | 2 +-
  target/nios2/cpu.h  | 2 +-
  target/openrisc/cpu.h   | 2 +-
  target/ppc/cpu.h| 2 +-
  target/riscv/cpu.h  | 2 +-
  target/rx/cpu.h | 2 +-
  target/s390x/cpu.h  | 2 +-
  target/sh4/cpu.h| 2 +-
  target/sparc/cpu.h  | 2 +-
  target/tricore/cpu.h| 2 +-
  target/xtensa/cpu.h | 2 +-
  22 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 2a0893b1dc..0efc6153ed 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -70,8 +70,8 @@ DECLARE_CLASS_CHECKERS(CPUClass, CPU,
   * The object struct and class struct need to be declared manually.
   */
  #define OBJECT_DECLARE_CPU_TYPE(CpuInstanceType, CpuClassType, 
CPU_MODULE_OBJ_NAME) \
-OBJECT_DECLARE_TYPE(CpuInstanceType, CpuClassType, CPU_MODULE_OBJ_NAME); \
-typedef CpuInstanceType ArchCPU;
+typedef struct ArchCPU CpuInstanceType; \
+OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
  


Nice.
Reviewed-by: Richard Henderson 


r~

Re: [PATCH v2 13/14] target: Introduce and use OBJECT_DECLARE_CPU_TYPE() macro

[Richard Henderson]

On 2/14/22 08:31, Philippe Mathieu-Daudé wrote:

Replace the boilerplate code to declare CPU QOM types
and macros, and forward-declare the CPU instance type.

Signed-off-by: Philippe Mathieu-Daudé
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 10/14] target/hexagon: Add missing 'hw/core/cpu.h' include

[Richard Henderson]

On 2/14/22 08:31, Philippe Mathieu-Daudé wrote:

HexagonCPU field parent_class is of type CPUClass, which
is declared in "hw/core/cpu.h".

Signed-off-by: Philippe Mathieu-Daudé
---
  target/hexagon/cpu.h | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 2/4] target/nios2: Exteral Interrupt Controller (EIC)

[Richard Henderson]

On 2/24/22 03:48, Amir Gonnen wrote:

+static void nios2_cpu_set_eic_irq(Nios2CPU *cpu, int level)
+{
+CPUNios2State *env = >env;
+CPUState *cs = CPU(cpu);
+
+env->irq_pending = level;
+
+if (env->irq_pending && nios2_take_eic_irq(cpu)) {
+env->irq_pending = 0;
+cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+} else if (!env->irq_pending) {
+cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+}
+}


This looks wrong.  Of course, so does nios2_cpu_set_irq, from which you've 
cribbed this.

(1) The unset of irq_pending looks wrong, though come to think of it, it's existence also 
looks wrong.  I think that it's completely redundant with cpu->interrupt_request, and that 
you should only use cpu_interrupt/cpu_reset_interrupt from this level.


Which also means that nios2_check_interrupts and callers all the way back up are also 
incorrect.  All that should be required from wrctl status is the return to the main loop 
that you get from DISAS_UPDATE and tcg_gen_exit_tb.


Which also means that ipending is implemented incorrectly.  The current manipulation of 
CR_IPENDING in nios2_cpu_set_irc should instead be manipulating an internal "external hw 
request", per Figure 3-2 in NII51003.


For our purposes, I think simply re-using env->regs[CR_IPENDING] as the external hw 
request word is the right thing to do.   But we need to update RDCTL to compute the 
correct value from CR_IPENDING & CR_IENABLE, and update WRCTL to ignore writes.



(2) Checking nios2_take_eic_irq here, or CR_STATUS_PIE in nios2_cpu_set_irq is incorrect. 
 If you check this here, then you'll miss the interrupt when the interrupt enable bit is 
reset.  These checks belong in nios2_cpu_exec_interrupt.



+if (cpu->rnmi) {
+return !(env->regs[CR_STATUS] & CR_STATUS_NMI);
+}


I think this should be a separate

#define CPU_INTERRUPT_NMI  CPU_INTERRUPT_TGT_EXT_0


r~

Re: [PATCH v2 1/4] target/nios2: Shadow register set

[Richard Henderson]

On 2/24/22 03:48, Amir Gonnen wrote:

+void helper_eret(CPUNios2State *env, uint32_t new_pc)
+{
+uint32_t crs = cpu_get_crs(env);
+if (crs == 0) {
+env->regs[CR_STATUS] = env->regs[CR_ESTATUS];
+} else {
+env->regs[CR_STATUS] = env->regs[R_SSTATUS];
+}
+cpu_change_reg_set(env, crs, cpu_get_crs(env));


Hmm.  This could probably use a comment that the second computation of cpu_get_crs is 
using the value just restored into CR_STATUS.




+void helper_wrprs(CPUNios2State *env, uint32_t reg_index, uint32_t value)
+{
+uint32_t prs = cpu_get_prs(env);
+env->shadow_regs[prs][reg_index] = value;
+}
+
+uint32_t helper_rdprs(CPUNios2State *env, uint32_t reg_index)
+{
+uint32_t prs = cpu_get_prs(env);
+return env->shadow_regs[prs][reg_index];
+}


These are fairly easy to compute inline, e.g. for rdprs:

TCGv_i32 t = tcg_temp_i32_new();
TCGv_ptr p = tcg_temp_ptr_new();

tcg_gen_extract_i32(t, cpu_R[CR_STATUS],
R_CR_STATUS_CRS_SHIFT,
R_CR_STATUS_CRS_LENGTH);
tcg_gen_muli_i32(t, t, sizeof(uint32_t) * NUM_GP_REGS);
tcg_gen_ext_i32_ptr(p, t);

tcg_gen_add_ptr(p, p, cpu_env);
tcg_gen_ld_i32(t, p, offsetof(CPUNios2State, shadow_regs)
+ sizeof(uint32_t) * instr.a);
tcg_gen_addi_i32(cpu_R[instr.b], t, instr.imm16.s);

tcg_temp_free_ptr(p);
tcg_temp_free_i32(o);


+static void rdprs(DisasContext *dc, uint32_t code, uint32_t flags)
+{
+I_TYPE(instr, code);
+TCGv t = tcg_temp_new();
+gen_helper_rdprs(t, cpu_env, tcg_const_i32(instr.a));


You're missing a gen_check_supervisor here and in wrprs.


 static void eret(DisasContext *dc, uint32_t code, uint32_t flags)
 {
-tcg_gen_mov_tl(cpu_R[CR_STATUS], cpu_R[CR_ESTATUS]);
-tcg_gen_mov_tl(cpu_R[R_PC], cpu_R[R_EA]);
+gen_helper_eret(cpu_env, cpu_R[R_EA]);


As an existing bug to be fixed by a separate patch, eret should also check for 
supervisor.



The contents of this email message and any attachments are intended solely for 
the addressee(s) and may contain confidential and/or privileged information and 
may be legally protected from disclosure. If you are not the intended recipient 
of this message or their agent, or if this message has been addressed to you in 
error, please immediately alert the sender by reply email and then delete this 
message and any attachments. If you are not the intended recipient, you are 
hereby notified that any use, dissemination, copying, or storage of this 
message or its attachments is strictly prohibited.



You really need to suppress these footers when posting to a public mailing list.


r~

Re: [PATCH] libvhost-user: Fix wrong type of argument to formatting function (reported by LGTM)

[Philippe Mathieu-Daudé]

On 24/2/22 22:22, Stefan Weil wrote:

Am 07.01.22 um 16:49 schrieb Stefan Weil:


Signed-off-by: Stefan Weil 
---

LGTM has some more alerts which need attention:
https://lgtm.com/projects/g/qemu/qemu/

Regards,
Stefan

  subprojects/libvhost-user/libvhost-user.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c

index 787f4d2d4f..6eb72c4200 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,7 @@ generate_faults(VuDev *dev) {
  if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, _struct)) {
  vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%zx offset: %zx: 
(ufd=%d)%s\n",

   __func__, i,
   dev_region->mmap_addr,
   dev_region->size, dev_region->mmap_offset,



Up to now I did not see any response to this patch, and it is also still 
missing in the latest code.


dev_region->mmap_addr is an uint64_t value, so the current format string 
"%p" won't work on any platform where pointers are not 64 bit value.


Stefan





Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 1/4] target/nios2: Shadow register set

[Richard Henderson]

On 2/24/22 03:48, Amir Gonnen wrote:

@@ -88,7 +93,9 @@ struct Nios2CPUClass {
  #define   CR_STATUS_IH   (1 << 3)
  #define   CR_STATUS_IL   (63 << 4)
  #define   CR_STATUS_CRS  (63 << 10)
+#define   CR_STATUS_CRS_OFFSET 10
  #define   CR_STATUS_PRS  (63 << 16)
+#define   CR_STATUS_PRS_OFFSET 16
  #define   CR_STATUS_NMI  (1 << 22)
  #define   CR_STATUS_RSIE (1 << 23)
  #define CR_ESTATUS   (CR_BASE + 1)


It would be preferable to use hw/registerfields.h:

FIELD(CR_STATUS, IL, 4, 6)
FIELD(CR_STATUS, CRS, 10, 6)
FIELD(CR_STATUS, PRS, 16, 6)


+static inline uint32_t cpu_get_crs(const CPUNios2State *env)
+{
+return (env->regs[CR_STATUS] & CR_STATUS_CRS)
+>> CR_STATUS_CRS_OFFSET;
+}


This becomes

return FIELD_EX32(env->regs[CR_STATUS], CR_STATUS, CRS);


+env->regs[CR_STATUS] = (env->regs[CR_STATUS] & (~CR_STATUS_PRS))
+   | ((prev_set << CR_STATUS_PRS_OFFSET) & CR_STATUS_PRS);


This becomes

env->regs[CR_STATUS] = FIELD_DP32(env->regs[CR_STATUS],
  CR_STATUS, PRS, prev_set);

etc.


r~

Re: [PATCH 05/12] compiler.h: drop __printf__ macro MinGW/glib workaround

[Marc-André Lureau]

Hi

On Fri, Feb 25, 2022 at 1:41 AM Stefan Weil  wrote:

> Am 24.02.22 um 20:12 schrieb Peter Maydell:
>
> > On Thu, 24 Feb 2022 at 18:38,  wrote:
> >> From: Marc-André Lureau 
> >>
> >> This workaround was added in commit 95df51a4 ("w32: Always use standard
> >> instead of native format strings"), as it claimed glib was using
> >> __printf__ attribute. This is surprising, since glib has always used
> >> G_GNUC_PRINTF which, as the name implies, uses __gnu_printf__ when
> >> possible.
> > This was not always true: before this commit from 2018
> >
> https://github.com/GNOME/glib/commit/98a0ab929d8c59ee27e5f470f11d077bb6a56749
> > G_GNUC_PRINTF used always used __printf__.
> > I think that change only landed in glib 2.58, so since our current
> > minimum glib version is 2.56 we need to retain this workaround.
> >
> >> Apparently, the workaound is no longer relevant though, I don't see
> >> the warnings.
> > You're probably building with a newer glib, and possibly also
> > a newer mingw.
> >
> > I've cc'd Stefan Weil who might know whether we can drop this
> > workaround as far as the mingw part is concerned.
>
>
> My latest builds of QEMU for Windows still used glib 2.54 because that
> still is the "newest" version which is provided by Cygwin's mingw64:
>
> https://cygwin.com/cgi-bin2/package-grep.cgi?grep=mingw64-.*-glib2.0
>
> So I even had to downgrade the minimum glib version.
>
> A hard requirement of a newer glib would mean that Cygwin mingw64
> packages can no longer be used for building QEMU unless someone updates
> those packages.
>

That sounds doable, I can take a look.

Why not build with msys2 though? It is quite actively maintained and most
people recommended it these days. My understanding is that msys2 is closer
to native Windows (whereas cygwin tries to bring more POSIX compatiblity:
https://www.msys2.org/wiki/How-does-MSYS2-differ-from-Cygwin/)

-- 
Marc-André Lureau

Re: [PATCH v13 4/4] target/ppc: trigger PERFM EBBs from power8-pmu.c

[Richard Henderson]

On 2/24/22 10:19, Daniel Henrique Barboza wrote:

+void helper_ebb_perfm_excp(CPUPPCState *env)


Please reserve the prefix "helper_" for something that is called from tcg generated code, 
which this is not.



r~

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Alex Williamson]

On Thu, 24 Feb 2022 20:34:40 +
Joao Martins  wrote:

> On 2/24/22 20:12, Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2022 at 08:04:48PM +, Joao Martins wrote:  
> >> On 2/24/22 19:54, Michael S. Tsirkin wrote:  
> >>> On Thu, Feb 24, 2022 at 07:44:26PM +, Joao Martins wrote:  
>  On 2/24/22 18:30, Michael S. Tsirkin wrote:  
> > On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:  
> >> On 2/24/22 17:23, Michael S. Tsirkin wrote:  
> >>> On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:  
>  On 2/23/22 23:35, Joao Martins wrote:  
> > On 2/23/22 21:22, Michael S. Tsirkin wrote:  
> >>> +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
> >>> +  uint64_t 
> >>> pci_hole64_size)
> >>> +{
> >>> +X86MachineState *x86ms = X86_MACHINE(pcms);
> >>> +uint32_t eax, vendor[3];
> >>> +
> >>> +host_cpuid(0x0, 0, , [0], [2], [1]);
> >>> +if (!IS_AMD_VENDOR(vendor)) {
> >>> +return;
> >>> +}  
> >>
> >> Wait a sec, should this actually be tying things to the host CPU 
> >> ID?
> >> It's really about what we present to the guest though,
> >> isn't it?  
> >
> > It was the easier catch all to use cpuid without going into
> > Linux UAPI specifics. But it doesn't have to tie in there, it is 
> > only
> > for systems with an IOMMU present.
> >  
> >> Also, can't we tie this to whether the AMD IOMMU is present?
> >>  
> > I think so, I can add that. Something like a amd_iommu_exists() 
> > helper
> > in util/vfio-helpers.c which checks if there's any sysfs child 
> > entries
> > that start with ivhd in /sys/class/iommu/. Given that this HT 
> > region is
> > hardcoded in iommu reserved regions since >=4.11 (to latest) I 
> > don't think it's
> > even worth checking the range exists in:
> >
> > /sys/kernel/iommu_groups/0/reserved_regions
> >
> > (Also that sysfs ABI is >= 4.11 only)  
> 
>  Here's what I have staged in local tree, to address your comment.
> 
>  Naturally the first chunk is what's affected by this patch the rest 
>  is a
>  precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to 
>  pass
>  all the tests and what not.
> 
>  I am not entirely sure this is the right place to put such a helper, 
>  open
>  to suggestions. wrt to the naming of the helper, I tried to follow 
>  the rest
>  of the file's style.
> 
>  diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>  index a9be5d33a291..2ea4430d5dcc 100644
>  --- a/hw/i386/pc.c
>  +++ b/hw/i386/pc.c
>  @@ -868,10 +868,8 @@ static void 
>  x86_update_above_4g_mem_start(PCMachineState *pcms,
> uint64_t pci_hole64_size)
>   {
>   X86MachineState *x86ms = X86_MACHINE(pcms);
>  -uint32_t eax, vendor[3];
> 
>  -host_cpuid(0x0, 0, , [0], [2], [1]);
>  -if (!IS_AMD_VENDOR(vendor)) {
>  +if (!qemu_amd_iommu_is_present()) {
>   return;
>   }
> 
>  diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>  index 7bcce3bceb0f..eb4ea071ecec 100644
>  --- a/include/qemu/osdep.h
>  +++ b/include/qemu/osdep.h
>  @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
>    */
>   size_t qemu_get_host_physmem(void);
> 
>  +/**
>  + * qemu_amd_iommu_is_present:
>  + *
>  + * Operating system agnostic way of querying if an AMD IOMMU
>  + * is present.
>  + *
>  + */
>  +bool qemu_amd_iommu_is_present(void);
>  +
>   /*
>    * Toggle write/execute on the pages marked MAP_JIT
>    * for the current thread.
>  diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>  index f2be7321c59f..54cef21217c4 100644
>  --- a/util/oslib-posix.c
>  +++ b/util/oslib-posix.c
>  @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
>   #endif
>   return 0;
>   }
>  +
>  +bool qemu_amd_iommu_is_present(void)
>  +{
>  +bool found = false;
>  +#ifdef CONFIG_LINUX
>  +struct dirent *entry;
>  +char *path;
>  +DIR *dir;
>  +
>  +path = g_strdup_printf("/sys/class/iommu");
>  +dir = opendir(path);
>  +if (!dir) {
>  +g_free(path);
>  +return found;
> 

Re: [PATCH 05/12] compiler.h: drop __printf__ macro MinGW/glib workaround

[Stefan Weil]

Am 24.02.22 um 20:12 schrieb Peter Maydell:


On Thu, 24 Feb 2022 at 18:38,  wrote:

From: Marc-André Lureau 

This workaround was added in commit 95df51a4 ("w32: Always use standard
instead of native format strings"), as it claimed glib was using
__printf__ attribute. This is surprising, since glib has always used
G_GNUC_PRINTF which, as the name implies, uses __gnu_printf__ when
possible.

This was not always true: before this commit from 2018
https://github.com/GNOME/glib/commit/98a0ab929d8c59ee27e5f470f11d077bb6a56749
G_GNUC_PRINTF used always used __printf__.
I think that change only landed in glib 2.58, so since our current
minimum glib version is 2.56 we need to retain this workaround.


Apparently, the workaound is no longer relevant though, I don't see
the warnings.

You're probably building with a newer glib, and possibly also
a newer mingw.

I've cc'd Stefan Weil who might know whether we can drop this
workaround as far as the mingw part is concerned.



My latest builds of QEMU for Windows still used glib 2.54 because that 
still is the "newest" version which is provided by Cygwin's mingw64:


https://cygwin.com/cgi-bin2/package-grep.cgi?grep=mingw64-.*-glib2.0

So I even had to downgrade the minimum glib version.

A hard requirement of a newer glib would mean that Cygwin mingw64 
packages can no longer be used for building QEMU unless someone updates 
those packages.


Stefan

Re: [PATCH v13 1/4] target/ppc: make power8-pmu.c CONFIG_TCG only

[Richard Henderson]

On 2/24/22 10:18, Daniel Henrique Barboza wrote:

This is an exclusive TCG helper. Gating it with CONFIG_TCG and changing
meson.build accordingly will prevent problems --disable-tcg and
--disable-linux-user later on.

We're also changing the uses of !kvm_enabled() to tcg_enabled() to avoid
adding "defined(CONFIG_TCG)" ifdefs, since tcg_enabled() will be
defaulted to false with --disable-tcg and the block will always be
skipped.

Signed-off-by: Daniel Henrique Barboza
---
  target/ppc/cpu_init.c   | 16 +++-
  target/ppc/machine.c|  6 +-
  target/ppc/meson.build  |  2 +-
  target/ppc/power8-pmu.h |  4 ++--
  4 files changed, 15 insertions(+), 13 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v4 24/47] target/ppc: move vrl[bhwd]nm/vrl[bhwd]mi to decodetree

[Richard Henderson]

On 2/24/22 10:23, Matheus K. Ferst wrote:

You don't want to use tcg_gen_rotlv_vec directly, but tcg_gen_rotlv_vec.



I guess there is a typo here. Did you mean tcg_gen_gvec_rotlv? Or 
tcg_gen_rotlv_mod_vec?


Dangit.  Paste-paste error.  The first: tcg_gen_gvec_rotlv.


r~

Re: [PATCH v4 38/47] target/ppc: Refactor VSX_SCALAR_CMP_DP

[Richard Henderson]

On 2/24/22 09:16, Víctor Colombo wrote:

Could you please elaborate more on how do you think using
float*_compare and its FloatRelation result would work here?
I noticed do_scalar_cmp modifies CR and sets FPCC flag, which
is not what VSX_SCALAR_CMP do. Using that function would require a
rework.

An option I though would be to bring into VSX_SCALAR_CMP the
important necessary parts, something like this:

#define VSX_SCALAR_CMP(op, tp, cmp, fld, svxvc, expr)   ...
     r = tp##_compare(xa->fld, xb->fld, >fp_status);    \
     if (expr) {    \
     memset(, 0xFF, sizeof(t.fld));    \
     } else if (r == float_relation_unordered) {    \
     if (env->fp_status.float_exception_flags & float_flag_invalid_snan) { \
     float_invalid_op_vxsnan(env, GETPC());    \
     if (fpscr_ve == 0 && svxvc) {    \
     float_invalid_op_vxvc(env, 0, GETPC());    \
     }    \
     } else if (svxvc) {    \
     if (tp##_is_quiet_nan(xa->fld, >fp_status) ||    \
     tp##_is_quiet_nan(xb->fld, >fp_status)) {    \
     float_invalid_op_vxvc(env, 0, GETPC());    \
     }    \
     }    \
     }    \
...
VSX_SCALAR_CMP(XSCMPEQDP, float64, eq, VsrD(0), 0, r == float_relation_equal)
VSX_SCALAR_CMP(XSCMPGEDP, float64, le, VsrD(0), 1, \
     r == float_relation_equal || r == float_relation_greater)
VSX_SCALAR_CMP(XSCMPGTDP, float64, lt, VsrD(0), 1, r == float_relation_greater)


I was thinking along the lines of:

bool r;
int flags;

helper_reset_fpstatus(env);
if (svxvc) {
r = tp##cmp(...);
} else {
r = tp##cmp##_quiet(...);
}

flags = get_float_exception_flags(>fp_status);
if (unlikely(flags & float_flag_invalid)) {
bool vxvc = svxvc;
if (flags & float_flag_invalid_snan)) {
float_invalid_op_vxsnan(...);
vxvc &= fpscr_ve == 0;
}
if (vxvc) {
float_invalid_op_vxvc(...);
}
}

memset(xt, 0, sizeof(*xt));
memset(>fld, -r, sizeof(xt->fld));
do_float_check_status(...);


r~

Re: [PATCH] libvhost-user: Fix wrong type of argument to formatting function (reported by LGTM)

[Stefan Weil]

Am 07.01.22 um 16:49 schrieb Stefan Weil:


Signed-off-by: Stefan Weil 
---

LGTM has some more alerts which need attention:
https://lgtm.com/projects/g/qemu/qemu/

Regards,
Stefan

  subprojects/libvhost-user/libvhost-user.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index 787f4d2d4f..6eb72c4200 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,7 @@ generate_faults(VuDev *dev) {
  
  if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, _struct)) {

  vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%zx offset: %zx: (ufd=%d)%s\n",
   __func__, i,
   dev_region->mmap_addr,
   dev_region->size, dev_region->mmap_offset,



Up to now I did not see any response to this patch, and it is also still 
missing in the latest code.


dev_region->mmap_addr is an uint64_t value, so the current format string 
"%p" won't work on any platform where pointers are not 64 bit value.


Stefan

Re: [RFC PATCH 0/4] ppc: nested TCG migration (KVM-on-TCG)

[Mark Cave-Ayland]

On 24/02/2022 18:58, Fabiano Rosas wrote:


This series implements the migration for a TCG pseries guest running a
nested KVM guest. This is just like migrating a pseries TCG guest, but
with some extra state to allow a nested guest to continue to run on
the destination.

Unfortunately the regular TCG migration scenario (not nested) is not
fully working so I cannot be entirely sure the nested migration is
correct. I have included a couple of patches for the general migration
case that (I think?) improve the situation a bit, but I'm still seeing
hard lockups and other issues with more than 1 vcpu.

This is more of an early RFC to see if anyone spots something right
away. I haven't made much progress in debugging the general TCG
migration case so if anyone has any input there as well I'd appreciate
it.

Thanks

Fabiano Rosas (4):
   target/ppc: TCG: Migrate tb_offset and decr
   spapr: TCG: Migrate spapr_cpu->prod
   hw/ppc: Take nested guest into account when saving timebase
   spapr: Add KVM-on-TCG migration support

  hw/ppc/ppc.c| 17 +++-
  hw/ppc/spapr.c  | 19 
  hw/ppc/spapr_cpu_core.c | 77 +
  include/hw/ppc/spapr_cpu_core.h |  2 +-
  target/ppc/machine.c| 61 ++
  5 files changed, 174 insertions(+), 2 deletions(-)


FWIW I noticed there were some issues with migrating the decrementer on Mac machines 
a while ago which causes a hang on the destination with TCG (for MacOS on a x86 host 
in my case). Have a look at the following threads for reference:


https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00546.html
https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04622.html

IIRC there is code that assumes any migration in PPC is being done live, and so 
adjusts the timebase on the destination to reflect wall clock time by recalculating 
tb_offset. I haven't looked at the code for a while but I think the outcome was that 
there needs to be 2 phases in migration: the first is to migrate the timebase as-is 
for guests that are paused during migration, whilst the second is to notify 
hypervisor-aware guest OSs such as Linux to make the timebase adjustment if required 
if the guest is running.



ATB,

Mark.

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Joao Martins]

On 2/24/22 20:12, Michael S. Tsirkin wrote:
> On Thu, Feb 24, 2022 at 08:04:48PM +, Joao Martins wrote:
>> On 2/24/22 19:54, Michael S. Tsirkin wrote:
>>> On Thu, Feb 24, 2022 at 07:44:26PM +, Joao Martins wrote:
 On 2/24/22 18:30, Michael S. Tsirkin wrote:
> On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
>> On 2/24/22 17:23, Michael S. Tsirkin wrote:
>>> On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
 On 2/23/22 23:35, Joao Martins wrote:
> On 2/23/22 21:22, Michael S. Tsirkin wrote:
>>> +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
>>> +  uint64_t pci_hole64_size)
>>> +{
>>> +X86MachineState *x86ms = X86_MACHINE(pcms);
>>> +uint32_t eax, vendor[3];
>>> +
>>> +host_cpuid(0x0, 0, , [0], [2], [1]);
>>> +if (!IS_AMD_VENDOR(vendor)) {
>>> +return;
>>> +}
>>
>> Wait a sec, should this actually be tying things to the host CPU ID?
>> It's really about what we present to the guest though,
>> isn't it?
>
> It was the easier catch all to use cpuid without going into
> Linux UAPI specifics. But it doesn't have to tie in there, it is only
> for systems with an IOMMU present.
>
>> Also, can't we tie this to whether the AMD IOMMU is present?
>>
> I think so, I can add that. Something like a amd_iommu_exists() helper
> in util/vfio-helpers.c which checks if there's any sysfs child entries
> that start with ivhd in /sys/class/iommu/. Given that this HT region 
> is
> hardcoded in iommu reserved regions since >=4.11 (to latest) I don't 
> think it's
> even worth checking the range exists in:
>
>   /sys/kernel/iommu_groups/0/reserved_regions
>
> (Also that sysfs ABI is >= 4.11 only)

 Here's what I have staged in local tree, to address your comment.

 Naturally the first chunk is what's affected by this patch the rest is 
 a
 precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to 
 pass
 all the tests and what not.

 I am not entirely sure this is the right place to put such a helper, 
 open
 to suggestions. wrt to the naming of the helper, I tried to follow the 
 rest
 of the file's style.

 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index a9be5d33a291..2ea4430d5dcc 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -868,10 +868,8 @@ static void 
 x86_update_above_4g_mem_start(PCMachineState *pcms,
uint64_t pci_hole64_size)
  {
  X86MachineState *x86ms = X86_MACHINE(pcms);
 -uint32_t eax, vendor[3];

 -host_cpuid(0x0, 0, , [0], [2], [1]);
 -if (!IS_AMD_VENDOR(vendor)) {
 +if (!qemu_amd_iommu_is_present()) {
  return;
  }

 diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
 index 7bcce3bceb0f..eb4ea071ecec 100644
 --- a/include/qemu/osdep.h
 +++ b/include/qemu/osdep.h
 @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
   */
  size_t qemu_get_host_physmem(void);

 +/**
 + * qemu_amd_iommu_is_present:
 + *
 + * Operating system agnostic way of querying if an AMD IOMMU
 + * is present.
 + *
 + */
 +bool qemu_amd_iommu_is_present(void);
 +
  /*
   * Toggle write/execute on the pages marked MAP_JIT
   * for the current thread.
 diff --git a/util/oslib-posix.c b/util/oslib-posix.c
 index f2be7321c59f..54cef21217c4 100644
 --- a/util/oslib-posix.c
 +++ b/util/oslib-posix.c
 @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
  #endif
  return 0;
  }
 +
 +bool qemu_amd_iommu_is_present(void)
 +{
 +bool found = false;
 +#ifdef CONFIG_LINUX
 +struct dirent *entry;
 +char *path;
 +DIR *dir;
 +
 +path = g_strdup_printf("/sys/class/iommu");
 +dir = opendir(path);
 +if (!dir) {
 +g_free(path);
 +return found;
 +}
 +
 +do {
 +entry = readdir(dir);
 +if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
 +found = true;
 +break;
 +}
 +} while (entry);
 +
 +g_free(path);
 +closedir(dir);
 +#endif
 +   

Re: [PATCH v4 24/47] target/ppc: move vrl[bhwd]nm/vrl[bhwd]mi to decodetree

[Matheus K. Ferst]

On 23/02/2022 19:19, Richard Henderson wrote:

On 2/23/22 11:43, Matheus K. Ferst wrote:

Note that rotlv does the masking itself:

/*
  * Expand D = A << (B % element bits)
  *
  * Unlike scalar shifts, where it is easy for the target front end
  * to include the modulo as part of the expansion.  If the target
  * naturally includes the modulo as part of the operation, great!
  * If the target has some other behaviour from out-of-range shifts,
  * then it could not use this function anyway, and would need to
  * do it's own expansion with custom functions.
  */



Using tcg_gen_rotlv_vec(vece, vrt, vra, vrb) works on PPC but fails on 
x86. It looks like
a problem on the i386 backend. It's using VPS[RL]LV[DQ], but instead 
of this modulo
behavior, these instructions write zero to the element[1]. I'm not 
sure how to fix that.


You don't want to use tcg_gen_rotlv_vec directly, but tcg_gen_rotlv_vec.



I guess there is a typo here. Did you mean tcg_gen_gvec_rotlv? Or 
tcg_gen_rotlv_mod_vec?



The generic modulo is being applied here:

static void tcg_gen_rotlv_mod_vec(unsigned vece, TCGv_vec d,
   TCGv_vec a, TCGv_vec b)
{
     TCGv_vec t = tcg_temp_new_vec_matching(d);
     TCGv_vec m = tcg_constant_vec_matching(d, vece, (8 << vece) - 1);

     tcg_gen_and_vec(vece, t, b, m);
     tcg_gen_rotlv_vec(vece, d, a, t);
     tcg_temp_free_vec(t);
}


I can see that this method is called when we use tcg_gen_gvec_rotlv to 
implement vrl[bhwd], and they are working as expected. For vrl[wd]nm and 
vrl[wd]mi, however, we can't call tcg_gen_rotlv_mod_vec directly in the 
.fniv implementation because it is not exposed in tcg-op.h. Is there any 
other way to use this method? Should we add it to the header file?


Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO 
Analista de Software
Aviso Legal - Disclaimer 

[PATCH v13 4/4] target/ppc: trigger PERFM EBBs from power8-pmu.c

[Daniel Henrique Barboza]

This patch adds the EBB exception support that are triggered by
Performance Monitor alerts. This happens when a Performance Monitor
alert occurs and MMCR0_EBE, BESCR_PME and BESCR_GE are set.

fire_PMC_interrupt() will execute a new ebb_perfm_excp() helper that
will check for MMCR0_EBE, BESCR_PME and BESCR_GE bits. If all bits are
set, do_ebb() will attempt to trigger a PERFM EBB event.

If the EBB facility is enabled in both FSCR and HFSCR we consider that
the EBB is valid and set BESCR_PMEO. After that, if we're running in
problem state, fire a POWERPC_EXCP_PERM_EBB immediately. Otherwise we'll
queue a PPC_INTERRUPT_EBB.

Signed-off-by: Daniel Henrique Barboza 
---
 target/ppc/excp_helper.c | 48 
 target/ppc/helper.h  |  1 +
 target/ppc/power8-pmu.c  |  3 +--
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 5e7d29ae00..89b7b3ac00 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2066,6 +2066,54 @@ void helper_rfebb(CPUPPCState *env, target_ulong s)
 env->spr[SPR_BESCR] &= ~BESCR_GE;
 }
 }
+
+/*
+ * Triggers or queues an 'ebb_excp' EBB exception. All checks
+ * but FSCR, HFSCR and msr_pr must be done beforehand.
+ *
+ * PowerISA v3.1 isn't clear about whether an EBB should be
+ * postponed or cancelled if the EBB facility is unavailable.
+ * Our assumption here is that the EBB is cancelled if both
+ * FSCR and HFSCR EBB facilities aren't available.
+ */
+static void do_ebb(CPUPPCState *env, int ebb_excp)
+{
+PowerPCCPU *cpu = env_archcpu(env);
+CPUState *cs = CPU(cpu);
+
+/*
+ * FSCR_EBB and FSCR_IC_EBB are the same bits used with
+ * HFSCR.
+ */
+helper_fscr_facility_check(env, FSCR_EBB, 0, FSCR_IC_EBB);
+helper_hfscr_facility_check(env, FSCR_EBB, "EBB", FSCR_IC_EBB);
+
+if (ebb_excp == POWERPC_EXCP_PERFM_EBB) {
+env->spr[SPR_BESCR] |= BESCR_PMEO;
+} else if (ebb_excp == POWERPC_EXCP_EXTERNAL_EBB) {
+env->spr[SPR_BESCR] |= BESCR_EEO;
+}
+
+if (msr_pr == 1) {
+powerpc_excp(cpu, ebb_excp);
+} else {
+env->pending_interrupts |= 1 << PPC_INTERRUPT_EBB;
+cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+}
+}
+
+void helper_ebb_perfm_excp(CPUPPCState *env)
+{
+bool perfm_ebb_enabled = env->spr[SPR_POWER_MMCR0] & MMCR0_EBE &&
+ env->spr[SPR_BESCR] & BESCR_PME &&
+ env->spr[SPR_BESCR] & BESCR_GE;
+
+if (!perfm_ebb_enabled) {
+return;
+}
+
+do_ebb(env, POWERPC_EXCP_PERFM_EBB);
+}
 #endif
 
 /*/
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index ab008c9d4e..8c2c03fd48 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -18,6 +18,7 @@ DEF_HELPER_1(rfid, void, env)
 DEF_HELPER_1(rfscv, void, env)
 DEF_HELPER_1(hrfid, void, env)
 DEF_HELPER_2(rfebb, void, env, tl)
+DEF_HELPER_1(ebb_perfm_excp, void, env)
 DEF_HELPER_2(store_lpcr, void, env, tl)
 DEF_HELPER_2(store_pcr, void, env, tl)
 DEF_HELPER_2(store_mmcr0, void, env, tl)
diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
index d245663158..38e1ecb782 100644
--- a/target/ppc/power8-pmu.c
+++ b/target/ppc/power8-pmu.c
@@ -307,8 +307,7 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
 env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
 }
 
-/* PMC interrupt not implemented yet */
-return;
+helper_ebb_perfm_excp(env);
 }
 
 /* This helper assumes that the PMC is running. */
-- 
2.35.1

[PATCH v13 1/4] target/ppc: make power8-pmu.c CONFIG_TCG only

[Daniel Henrique Barboza]

This is an exclusive TCG helper. Gating it with CONFIG_TCG and changing
meson.build accordingly will prevent problems --disable-tcg and
--disable-linux-user later on.

We're also changing the uses of !kvm_enabled() to tcg_enabled() to avoid
adding "defined(CONFIG_TCG)" ifdefs, since tcg_enabled() will be
defaulted to false with --disable-tcg and the block will always be
skipped.

Signed-off-by: Daniel Henrique Barboza 
---
 target/ppc/cpu_init.c   | 16 +++-
 target/ppc/machine.c|  6 +-
 target/ppc/meson.build  |  2 +-
 target/ppc/power8-pmu.h |  4 ++--
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 61d36b11a0..544e052290 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5698,12 +5698,10 @@ static void register_power9_mmu_sprs(CPUPPCState *env)
  */
 static void init_tcg_pmu_power8(CPUPPCState *env)
 {
-#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
 /* Init PMU overflow timers */
-if (!kvm_enabled()) {
+if (tcg_enabled()) {
 cpu_ppc_pmu_init(env);
 }
-#endif
 }
 
 static void init_proc_book3s_common(CPUPPCState *env)
@@ -7167,14 +7165,14 @@ static void ppc_cpu_reset(DeviceState *dev)
 
 #if !defined(CONFIG_USER_ONLY)
 env->nip = env->hreset_vector | env->excp_prefix;
-#if defined(CONFIG_TCG)
-if (env->mmu_model != POWERPC_MMU_REAL) {
-ppc_tlb_invalidate_all(env);
+
+if (tcg_enabled()) {
+if (env->mmu_model != POWERPC_MMU_REAL) {
+ppc_tlb_invalidate_all(env);
+}
+pmu_update_summaries(env);
 }
-#endif /* CONFIG_TCG */
 #endif
-
-pmu_update_summaries(env);
 hreg_compute_hflags(env);
 env->reserve_addr = (target_ulong)-1ULL;
 /* Be sure no exception or interrupt is pending */
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 1b63146ed1..e673944597 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -2,6 +2,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "sysemu/kvm.h"
+#include "sysemu/tcg.h"
 #include "helper_regs.h"
 #include "mmu-hash64.h"
 #include "migration/cpu.h"
@@ -20,7 +21,10 @@ static void post_load_update_msr(CPUPPCState *env)
  */
 env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
 ppc_store_msr(env, msr);
-pmu_update_summaries(env);
+
+if (tcg_enabled()) {
+pmu_update_summaries(env);
+}
 }
 
 static int get_avr(QEMUFile *f, void *pv, size_t size,
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index a49a8911e0..79beaff147 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -16,6 +16,7 @@ ppc_ss.add(when: 'CONFIG_TCG', if_true: files(
   'misc_helper.c',
   'timebase_helper.c',
   'translate.c',
+  'power8-pmu.c',
 ))
 
 ppc_ss.add(libdecnumber)
@@ -51,7 +52,6 @@ ppc_softmmu_ss.add(when: 'TARGET_PPC64', if_true: files(
   'mmu-book3s-v3.c',
   'mmu-hash64.c',
   'mmu-radix64.c',
-  'power8-pmu.c',
 ))
 
 target_arch += {'ppc': ppc_ss}
diff --git a/target/ppc/power8-pmu.h b/target/ppc/power8-pmu.h
index a839199561..256d90f523 100644
--- a/target/ppc/power8-pmu.h
+++ b/target/ppc/power8-pmu.h
@@ -13,11 +13,11 @@
 #ifndef POWER8_PMU
 #define POWER8_PMU
 
-void cpu_ppc_pmu_init(CPUPPCState *env);
-
 #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+void cpu_ppc_pmu_init(CPUPPCState *env);
 void pmu_update_summaries(CPUPPCState *env);
 #else
+static inline void cpu_ppc_pmu_init(CPUPPCState *env) { }
 static inline void pmu_update_summaries(CPUPPCState *env) { }
 #endif
 
-- 
2.35.1

[PATCH v13 2/4] target/ppc: finalize pre-EBB PMU logic

[Daniel Henrique Barboza]

There are still PMU exclusive bits to handle in fire_PMC_interrupt()
before implementing the EBB support. Let's finalize it now to avoid
dealing with PMU and EBB logic at the same time in the next patches.

fire_PMC_interrupt() will fire an Performance Monitor alert depending on
MMCR0_PMAE. If we are required to freeze the timers (MMCR0_FCECE) we'll
also need to update summaries and delete the existing overflow timers.
In all cases we're going to update the cycle counters.

Signed-off-by: Daniel Henrique Barboza 
---
 target/ppc/power8-pmu.c | 36 ++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
index 236e8e66e9..d245663158 100644
--- a/target/ppc/power8-pmu.c
+++ b/target/ppc/power8-pmu.c
@@ -222,6 +222,20 @@ static void pmu_update_overflow_timers(CPUPPCState *env)
 }
 }
 
+static void pmu_delete_timers(CPUPPCState *env)
+{
+QEMUTimer *pmc_overflow_timer;
+int sprn;
+
+for (sprn = SPR_POWER_PMC1; sprn <= SPR_POWER_PMC6; sprn++) {
+pmc_overflow_timer = get_cyc_overflow_timer(env, sprn);
+
+if (pmc_overflow_timer) {
+timer_del(pmc_overflow_timer);
+}
+}
+}
+
 void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
 {
 bool hflags_pmcc0 = (value & MMCR0_PMCC0) != 0;
@@ -271,8 +285,26 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
 {
 CPUPPCState *env = >env;
 
-if (!(env->spr[SPR_POWER_MMCR0] & MMCR0_EBE)) {
-return;
+pmu_update_cycles(env);
+
+if (env->spr[SPR_POWER_MMCR0] & MMCR0_FCECE) {
+env->spr[SPR_POWER_MMCR0] &= ~MMCR0_FCECE;
+env->spr[SPR_POWER_MMCR0] |= MMCR0_FC;
+
+/* Changing MMCR0_FC requires a new HFLAGS_INSN_CNT calc */
+pmu_update_summaries(env);
+
+/*
+ * Delete all pending timers if we need to freeze
+ * the PMC. We'll restart them when the PMC starts
+ * running again.
+ */
+pmu_delete_timers(env);
+}
+
+if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMAE) {
+env->spr[SPR_POWER_MMCR0] &= ~MMCR0_PMAE;
+env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
 }
 
 /* PMC interrupt not implemented yet */
-- 
2.35.1

[PATCH v13 3/4] target/ppc: add PPC_INTERRUPT_EBB and EBB exceptions

[Daniel Henrique Barboza]

PPC_INTERRUPT_EBB is a new interrupt that will be used to deliver EBB
exceptions that had to be postponed because the thread wasn't in problem
state at the time the event-based branch was supposed to occur.

ISA 3.1 also defines two EBB exceptions: Performance Monitor EBB
exception and External EBB exception. They are being added as
POWERPC_EXCP_PERFM_EBB and POWERPC_EXCP_EXTERNAL_EBB.

PPC_INTERRUPT_EBB will check BESCR bits to see the EBB type that
occurred and trigger the appropriate exception. Both exceptions are
doing the same thing in this first implementation: clear BESCR_GE and
enter the branch with env->nip retrieved from SPR_EBBHR.

The checks being done by the interrupt code are msr_pr and BESCR_GE
states. All other checks (EBB facility check, BESCR_PME bit, specific
bits related to the event type) must be done beforehand.

Reviewed-by: Cédric Le Goater 
Signed-off-by: Daniel Henrique Barboza 
---
 target/ppc/cpu.h |  5 -
 target/ppc/cpu_init.c|  4 
 target/ppc/excp_helper.c | 33 +
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5b01d409b3..79375e8df4 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -127,8 +127,10 @@ enum {
 /* ISA 3.00 additions */
 POWERPC_EXCP_HVIRT= 101,
 POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception 
*/
+POWERPC_EXCP_PERFM_EBB = 103,/* Performance Monitor EBB Exception*/
+POWERPC_EXCP_EXTERNAL_EBB = 104, /* External EBB Exception   */
 /* EOL   */
-POWERPC_EXCP_NB   = 103,
+POWERPC_EXCP_NB   = 105,
 /* QEMU exceptions: special cases we want to stop translation*/
 POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only  */
 };
@@ -2434,6 +2436,7 @@ enum {
 PPC_INTERRUPT_HMI,/* Hypervisor Maintenance interrupt*/
 PPC_INTERRUPT_HDOORBELL,  /* Hypervisor Doorbell interrupt*/
 PPC_INTERRUPT_HVIRT,  /* Hypervisor virtualization interrupt  */
+PPC_INTERRUPT_EBB,/* Event-based Branch exception */
 };
 
 /* Processor Compatibility mask (PCR) */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 544e052290..073fd10168 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -2060,6 +2060,10 @@ static void init_excp_POWER8(CPUPPCState *env)
 env->excp_vectors[POWERPC_EXCP_FU]   = 0x0F60;
 env->excp_vectors[POWERPC_EXCP_HV_FU]= 0x0F80;
 env->excp_vectors[POWERPC_EXCP_SDOOR_HV] = 0x0E80;
+
+/* Userland exceptions without vector value in PowerISA v3.1 */
+env->excp_vectors[POWERPC_EXCP_PERFM_EBB] = 0x0;
+env->excp_vectors[POWERPC_EXCP_EXTERNAL_EBB] = 0x0;
 #endif
 }
 
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 6538c56ab0..5e7d29ae00 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1554,6 +1554,21 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 new_msr |= (target_ulong)MSR_HVB;
 new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
 break;
+case POWERPC_EXCP_PERFM_EBB:/* Performance Monitor EBB Exception  
*/
+case POWERPC_EXCP_EXTERNAL_EBB: /* External EBB Exception 
*/
+env->spr[SPR_BESCR] &= ~BESCR_GE;
+
+/*
+ * Save NIP for rfebb insn in SPR_EBBRR. Next nip is
+ * stored in the EBB Handler SPR_EBBHR.
+ */
+env->spr[SPR_EBBRR] = env->nip;
+powerpc_set_excp_state(cpu, env->spr[SPR_EBBHR], env->msr);
+
+/*
+ * This exception is handled in userspace. No need to proceed.
+ */
+return;
 case POWERPC_EXCP_THERM: /* Thermal interrupt*/
 case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt   */
 case POWERPC_EXCP_VPUA:  /* Vector assist exception  */
@@ -1797,6 +1812,24 @@ static void ppc_hw_interrupt(CPUPPCState *env)
 powerpc_excp(cpu, POWERPC_EXCP_THERM);
 return;
 }
+/* EBB exception */
+if (env->pending_interrupts & (1 << PPC_INTERRUPT_EBB)) {
+/*
+ * EBB exception must be taken in problem state and
+ * with BESCR_GE set.
+ */
+if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) {
+env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
+
+if (env->spr[SPR_BESCR] & BESCR_PMEO) {
+powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
+} else if (env->spr[SPR_BESCR] & BESCR_EEO) {
+powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL_EBB);
+}
+
+return;
+}
+}
 }
 
 if (env->resume_as_sreset) {
-- 
2.35.1

[PATCH v13 0/4] PMU-EBB support for PPC64 TCG

[Daniel Henrique Barboza]

Hi,

This new version contains a change in patch 1 (former 2) that was
proposed by Richard in the v12 review.

Changes from v12:
- former patch 1: dropped, no longer applicable
- patch 1 (former 2):
  * use tcg_enabled() instead of !kvm_enabled() to avoid defined(CONFIG_TCG)
  ifdefs
- v12 link: https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg03622.html

Daniel Henrique Barboza (4):
  target/ppc: make power8-pmu.c CONFIG_TCG only
  target/ppc: finalize pre-EBB PMU logic
  target/ppc: add PPC_INTERRUPT_EBB and EBB exceptions
  target/ppc: trigger PERFM EBBs from power8-pmu.c

 target/ppc/cpu.h |  5 ++-
 target/ppc/cpu_init.c| 20 +-
 target/ppc/excp_helper.c | 81 
 target/ppc/helper.h  |  1 +
 target/ppc/machine.c |  6 ++-
 target/ppc/meson.build   |  2 +-
 target/ppc/power8-pmu.c  | 39 +--
 target/ppc/power8-pmu.h  |  4 +-
 8 files changed, 140 insertions(+), 18 deletions(-)

-- 
2.35.1

Re: [PATCH 05/12] compiler.h: drop __printf__ macro MinGW/glib workaround

[Peter Maydell]

On Thu, 24 Feb 2022 at 19:50, Marc-André Lureau
 wrote:
> On Thu, Feb 24, 2022 at 11:23 PM Peter Maydell  
> wrote:
>> You're probably building with a newer glib, and possibly also
>> a newer mingw.
>>
>> I've cc'd Stefan Weil who might know whether we can drop this
>> workaround as far as the mingw part is concerned.
>
>
> Probably safer to keep it until we bump glib dependency to >=2.58.
>
> I would move it to glib-compat.h though, and leave a note there, as it is (or 
> should be ) an old glib specific workaround.

We can only move it to glib-compat if we confirm that only the
glib-related part of the workaround is still relevant and the
mingw side is now no longer needed, though.

-- PMM

Re: [PATCH v12 2/5] target/ppc: make power8-pmu.c CONFIG_TCG only

[Daniel Henrique Barboza]

On 2/17/22 19:17, Richard Henderson wrote:

On 2/16/22 21:10, Daniel Henrique Barboza wrote:

  static void init_tcg_pmu_power8(CPUPPCState *env)
  {
-#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+#if defined(CONFIG_TCG)
  /* Init PMU overflow timers */
  if (!kvm_enabled()) {
  cpu_ppc_pmu_init(env);
@@ -7872,10 +7872,9 @@ static void ppc_cpu_reset(DeviceState *dev)
  if (env->mmu_model != POWERPC_MMU_REAL) {
  ppc_tlb_invalidate_all(env);
  }
+    pmu_update_summaries(env);
  #endif /* CONFIG_TCG */
  #endif
-
-    pmu_update_summaries(env);


It looks like you could remove all of the ifdefs if you simply use 
tcg_enabled() rather than !kvm_enabled().  If !defined(CONFIG_TCG), 
tcg_enabled() will be constant false, and the block will be optimized away.



Just tested and it works. Thanks for the tip.

I'll re-send with this change.


Thanks,


Daniel




r~

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Michael S. Tsirkin]

On Thu, Feb 24, 2022 at 08:04:48PM +, Joao Martins wrote:
> 
> 
> On 2/24/22 19:54, Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2022 at 07:44:26PM +, Joao Martins wrote:
> >> On 2/24/22 18:30, Michael S. Tsirkin wrote:
> >>> On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
>  On 2/24/22 17:23, Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
> >> On 2/23/22 23:35, Joao Martins wrote:
> >>> On 2/23/22 21:22, Michael S. Tsirkin wrote:
> > +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
> > +  uint64_t pci_hole64_size)
> > +{
> > +X86MachineState *x86ms = X86_MACHINE(pcms);
> > +uint32_t eax, vendor[3];
> > +
> > +host_cpuid(0x0, 0, , [0], [2], [1]);
> > +if (!IS_AMD_VENDOR(vendor)) {
> > +return;
> > +}
> 
>  Wait a sec, should this actually be tying things to the host CPU ID?
>  It's really about what we present to the guest though,
>  isn't it?
> >>>
> >>> It was the easier catch all to use cpuid without going into
> >>> Linux UAPI specifics. But it doesn't have to tie in there, it is only
> >>> for systems with an IOMMU present.
> >>>
>  Also, can't we tie this to whether the AMD IOMMU is present?
> 
> >>> I think so, I can add that. Something like a amd_iommu_exists() helper
> >>> in util/vfio-helpers.c which checks if there's any sysfs child entries
> >>> that start with ivhd in /sys/class/iommu/. Given that this HT region 
> >>> is
> >>> hardcoded in iommu reserved regions since >=4.11 (to latest) I don't 
> >>> think it's
> >>> even worth checking the range exists in:
> >>>
> >>>   /sys/kernel/iommu_groups/0/reserved_regions
> >>>
> >>> (Also that sysfs ABI is >= 4.11 only)
> >>
> >> Here's what I have staged in local tree, to address your comment.
> >>
> >> Naturally the first chunk is what's affected by this patch the rest is 
> >> a
> >> precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to 
> >> pass
> >> all the tests and what not.
> >>
> >> I am not entirely sure this is the right place to put such a helper, 
> >> open
> >> to suggestions. wrt to the naming of the helper, I tried to follow the 
> >> rest
> >> of the file's style.
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index a9be5d33a291..2ea4430d5dcc 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -868,10 +868,8 @@ static void 
> >> x86_update_above_4g_mem_start(PCMachineState *pcms,
> >>uint64_t pci_hole64_size)
> >>  {
> >>  X86MachineState *x86ms = X86_MACHINE(pcms);
> >> -uint32_t eax, vendor[3];
> >>
> >> -host_cpuid(0x0, 0, , [0], [2], [1]);
> >> -if (!IS_AMD_VENDOR(vendor)) {
> >> +if (!qemu_amd_iommu_is_present()) {
> >>  return;
> >>  }
> >>
> >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> >> index 7bcce3bceb0f..eb4ea071ecec 100644
> >> --- a/include/qemu/osdep.h
> >> +++ b/include/qemu/osdep.h
> >> @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
> >>   */
> >>  size_t qemu_get_host_physmem(void);
> >>
> >> +/**
> >> + * qemu_amd_iommu_is_present:
> >> + *
> >> + * Operating system agnostic way of querying if an AMD IOMMU
> >> + * is present.
> >> + *
> >> + */
> >> +bool qemu_amd_iommu_is_present(void);
> >> +
> >>  /*
> >>   * Toggle write/execute on the pages marked MAP_JIT
> >>   * for the current thread.
> >> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> >> index f2be7321c59f..54cef21217c4 100644
> >> --- a/util/oslib-posix.c
> >> +++ b/util/oslib-posix.c
> >> @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
> >>  #endif
> >>  return 0;
> >>  }
> >> +
> >> +bool qemu_amd_iommu_is_present(void)
> >> +{
> >> +bool found = false;
> >> +#ifdef CONFIG_LINUX
> >> +struct dirent *entry;
> >> +char *path;
> >> +DIR *dir;
> >> +
> >> +path = g_strdup_printf("/sys/class/iommu");
> >> +dir = opendir(path);
> >> +if (!dir) {
> >> +g_free(path);
> >> +return found;
> >> +}
> >> +
> >> +do {
> >> +entry = readdir(dir);
> >> +if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
> >> +found = true;
> >> +break;
> >> +}
> >> +} while (entry);
> >> +
> >> +g_free(path);
> >> +closedir(dir);
> >> +#endif
> >> +return found;
> >> +}
> >
> 

Re: [RFC PATCH 1/4] target/ppc: TCG: Migrate tb_offset and decr

[Richard Henderson]

On 2/24/22 08:58, Fabiano Rosas wrote:

These two were not migrated so the remote end was starting with the
decrementer expired.

I am seeing less frequent crashes with this patch (tested with -smp 4
and -smp 32). It certainly doesn't fix all issues but it looks like it
helps.

Signed-off-by: Fabiano Rosas 
---
  target/ppc/machine.c | 17 +
  1 file changed, 17 insertions(+)

diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 1b63146ed1..7ee1984500 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -9,6 +9,7 @@
  #include "qemu/main-loop.h"
  #include "kvm_ppc.h"
  #include "power8-pmu.h"
+#include "hw/ppc/ppc.h"
  
  static void post_load_update_msr(CPUPPCState *env)

  {
@@ -666,6 +667,18 @@ static const VMStateDescription vmstate_compat = {
  }
  };
  
+static const VMStateDescription vmstate_tb_env = {

+.name = "cpu/env/tb_env",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_INT64(tb_offset, ppc_tb_t),
+VMSTATE_UINT64(decr_next, ppc_tb_t),
+VMSTATE_TIMER_PTR(decr_timer, ppc_tb_t),
+VMSTATE_END_OF_LIST()
+}
+};
+
  const VMStateDescription vmstate_ppc_cpu = {
  .name = "cpu",
  .version_id = 5,
@@ -696,12 +709,16 @@ const VMStateDescription vmstate_ppc_cpu = {
  /* Backward compatible internal state */
  VMSTATE_UINTTL(env.hflags_compat_nmsr, PowerPCCPU),
  
+VMSTATE_STRUCT_POINTER_V(env.tb_env, PowerPCCPU, 1,

+ vmstate_tb_env, ppc_tb_t),
+
  /* Sanity checking */
  VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
  VMSTATE_UINT64_TEST(mig_insns_flags, PowerPCCPU, 
cpu_pre_2_8_migration),
  VMSTATE_UINT64_TEST(mig_insns_flags2, PowerPCCPU,
  cpu_pre_2_8_migration),
  VMSTATE_UINT32_TEST(mig_nb_BATs, PowerPCCPU, cpu_pre_2_8_migration),
+
  VMSTATE_END_OF_LIST()
  },
  .subsections = (const VMStateDescription*[]) {


I think the new struct should go into a subsection.

r~

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Joao Martins]

On 2/24/22 19:54, Michael S. Tsirkin wrote:
> On Thu, Feb 24, 2022 at 07:44:26PM +, Joao Martins wrote:
>> On 2/24/22 18:30, Michael S. Tsirkin wrote:
>>> On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
 On 2/24/22 17:23, Michael S. Tsirkin wrote:
> On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
>> On 2/23/22 23:35, Joao Martins wrote:
>>> On 2/23/22 21:22, Michael S. Tsirkin wrote:
> +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
> +  uint64_t pci_hole64_size)
> +{
> +X86MachineState *x86ms = X86_MACHINE(pcms);
> +uint32_t eax, vendor[3];
> +
> +host_cpuid(0x0, 0, , [0], [2], [1]);
> +if (!IS_AMD_VENDOR(vendor)) {
> +return;
> +}

 Wait a sec, should this actually be tying things to the host CPU ID?
 It's really about what we present to the guest though,
 isn't it?
>>>
>>> It was the easier catch all to use cpuid without going into
>>> Linux UAPI specifics. But it doesn't have to tie in there, it is only
>>> for systems with an IOMMU present.
>>>
 Also, can't we tie this to whether the AMD IOMMU is present?

>>> I think so, I can add that. Something like a amd_iommu_exists() helper
>>> in util/vfio-helpers.c which checks if there's any sysfs child entries
>>> that start with ivhd in /sys/class/iommu/. Given that this HT region is
>>> hardcoded in iommu reserved regions since >=4.11 (to latest) I don't 
>>> think it's
>>> even worth checking the range exists in:
>>>
>>> /sys/kernel/iommu_groups/0/reserved_regions
>>>
>>> (Also that sysfs ABI is >= 4.11 only)
>>
>> Here's what I have staged in local tree, to address your comment.
>>
>> Naturally the first chunk is what's affected by this patch the rest is a
>> precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to pass
>> all the tests and what not.
>>
>> I am not entirely sure this is the right place to put such a helper, open
>> to suggestions. wrt to the naming of the helper, I tried to follow the 
>> rest
>> of the file's style.
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index a9be5d33a291..2ea4430d5dcc 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -868,10 +868,8 @@ static void 
>> x86_update_above_4g_mem_start(PCMachineState *pcms,
>>uint64_t pci_hole64_size)
>>  {
>>  X86MachineState *x86ms = X86_MACHINE(pcms);
>> -uint32_t eax, vendor[3];
>>
>> -host_cpuid(0x0, 0, , [0], [2], [1]);
>> -if (!IS_AMD_VENDOR(vendor)) {
>> +if (!qemu_amd_iommu_is_present()) {
>>  return;
>>  }
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index 7bcce3bceb0f..eb4ea071ecec 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
>>   */
>>  size_t qemu_get_host_physmem(void);
>>
>> +/**
>> + * qemu_amd_iommu_is_present:
>> + *
>> + * Operating system agnostic way of querying if an AMD IOMMU
>> + * is present.
>> + *
>> + */
>> +bool qemu_amd_iommu_is_present(void);
>> +
>>  /*
>>   * Toggle write/execute on the pages marked MAP_JIT
>>   * for the current thread.
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index f2be7321c59f..54cef21217c4 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
>>  #endif
>>  return 0;
>>  }
>> +
>> +bool qemu_amd_iommu_is_present(void)
>> +{
>> +bool found = false;
>> +#ifdef CONFIG_LINUX
>> +struct dirent *entry;
>> +char *path;
>> +DIR *dir;
>> +
>> +path = g_strdup_printf("/sys/class/iommu");
>> +dir = opendir(path);
>> +if (!dir) {
>> +g_free(path);
>> +return found;
>> +}
>> +
>> +do {
>> +entry = readdir(dir);
>> +if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
>> +found = true;
>> +break;
>> +}
>> +} while (entry);
>> +
>> +g_free(path);
>> +closedir(dir);
>> +#endif
>> +return found;
>> +}
>
> why are we checking whether an AMD IOMMU is present
> on the host? 
> Isn't what we care about whether it's
> present in the VM? What we are reserving after all
> is a range of GPAs, not actual host PA's ...
>
 Right.

 But any DMA map done by VFIO/IOMMU using those GPA ranges will fail,
 and so we need to not map 

Re: [PATCH 11/12] util: remove the net/net.h dependency

[Richard Henderson]

On 2/24/22 08:37, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

Move qemu_ether_ntoa() which is only needed in net/.

Signed-off-by: Marc-André Lureau
---
  include/qemu-common.h |  1 -
  net/announce.c| 13 +
  util/cutils.c | 14 --
  3 files changed, 13 insertions(+), 15 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 12/12] qapi: remove needless include

[Richard Henderson]

On 2/24/22 08:37, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

Signed-off-by: Marc-André Lureau
---
  qapi/qmp-dispatch.c | 1 -
  1 file changed, 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 10/12] util: remove needless includes

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

Signed-off-by: Marc-André Lureau
---
  util/cutils.c | 2 --
  1 file changed, 2 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 03/12] osdep.h: move qemu_build_not_reached()

[Marc-André Lureau]

Hi

On Thu, Feb 24, 2022 at 11:43 PM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau
> >
> > Move the macro and declaration so it can use glib in the following
> > patch.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> >   include/qemu/compiler.h | 16 
> >   include/qemu/osdep.h| 16 
> >   2 files changed, 16 insertions(+), 16 deletions(-)
>
> Would this be obviated by a change to _Noreturn?
>

Yes. Or we could just switch this one to _Noreturn if we decide to use
G_NORETURN elsewhere.

-- 
Marc-André Lureau

Re: [PATCH 08/12] Move HOST_LONG_BITS to compiler.h

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

This will help to make common code independent.

Signed-off-by: Marc-André Lureau
---
  include/qemu/compiler.h | 3 +++
  include/qemu/osdep.h| 3 ---
  2 files changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 06/12] Replace config-time define HOST_WORDS_BIGENDIAN

[Marc-André Lureau]

Hi

On Thu, Feb 24, 2022 at 11:41 PM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau
> >
> > Replace a config-time define with a compile time condition
> > define (compatible with clang and gcc) that must be declared prior to
> > its usage. This avoids having a global configure time define, but also
> > prevents from bad usage, if the config header wasn't included before.
> >
> > This can help to make some code independent from qemu too.
> >
> > gcc supports __BYTE_ORDER__ from about 4.6 and clang from 3.2.
> >
> > Signed-off-by: Marc-André Lureau
>
> Looks ok.  I'd like HOST_WORDS_BIGENDIAN to be poisoned, so that we're
> assured that we've
> caught all uses and new ones don't creep back in.
>
>
ack

-- 
Marc-André Lureau

Re: [PATCH 07/12] Simplify HOST_LONG_BITS

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

-#if UINTPTR_MAX == UINT32_MAX
-# define HOST_LONG_BITS 32
-#elif UINTPTR_MAX == UINT64_MAX
-# define HOST_LONG_BITS 64
-#else
-# error Unknown pointer size
-#endif
+#define HOST_LONG_BITS (__SIZEOF_POINTER__ * 8)


I guess.  I'll note that there are 128-bit pointers on the horizon, but that UINTPTR_MAX 
would not necessarily change to match __SIZEOF_POINTER__ [1].


Acked-by: Richard Henderson 


r~


[1] https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-947.pdf

Re: [PATCH 04/12] compiler.h: replace QEMU_NORETURN with G_NORETURN

[Marc-André Lureau]

Hi

On Thu, Feb 24, 2022 at 11:37 PM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > G_NORETURN was introduced in glib 2.68, fallback to G_GNUC_NORETURN in
> > glib-compat.
> >
> > Note that this attribute must be placed before the function declaration
> > (bringing a bit of consistency in qemu codebase usage).
> >
> > Signed-off-by: Marc-André Lureau 
>
> For C11, G_NORETURN is a wrapper for _Noreturn.
> Since we're using C11, we should just use _Noreturn.
>

G_NORETURN has several flavours, one of them is the c++ [[noreturn]], but
also MSVC specific etc.

Might be worth considering if we think about making code usable in
different contexts.


>
> >   void xtensa_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
> >   MMUAccessType access_type, int
> mmu_idx,
> > -uintptr_t retaddr) QEMU_NORETURN;
> > +uintptr_t retaddr) G_NORETURN;
>
> Incorrect placement.  I didn't scan the whole patch; I assume that using
> _Noreturn will
> flag this up as an error.
>

Right, bad regexp search, will fix.


>
> > -static void QEMU_NORETURN dump_core_and_abort(int target_sig)
> > +G_NORETURN static void dump_core_and_abort(int target_sig)
>
> I guess this can go either place, but I think I prefer the scope specifier
> first.
>

Ok, I'll change it to "static G_NORETURN"

-- 
Marc-André Lureau

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Michael S. Tsirkin]

On Thu, Feb 24, 2022 at 07:44:26PM +, Joao Martins wrote:
> On 2/24/22 18:30, Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
> >> On 2/24/22 17:23, Michael S. Tsirkin wrote:
> >>> On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
>  On 2/23/22 23:35, Joao Martins wrote:
> > On 2/23/22 21:22, Michael S. Tsirkin wrote:
> >>> +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
> >>> +  uint64_t pci_hole64_size)
> >>> +{
> >>> +X86MachineState *x86ms = X86_MACHINE(pcms);
> >>> +uint32_t eax, vendor[3];
> >>> +
> >>> +host_cpuid(0x0, 0, , [0], [2], [1]);
> >>> +if (!IS_AMD_VENDOR(vendor)) {
> >>> +return;
> >>> +}
> >>
> >> Wait a sec, should this actually be tying things to the host CPU ID?
> >> It's really about what we present to the guest though,
> >> isn't it?
> >
> > It was the easier catch all to use cpuid without going into
> > Linux UAPI specifics. But it doesn't have to tie in there, it is only
> > for systems with an IOMMU present.
> >
> >> Also, can't we tie this to whether the AMD IOMMU is present?
> >>
> > I think so, I can add that. Something like a amd_iommu_exists() helper
> > in util/vfio-helpers.c which checks if there's any sysfs child entries
> > that start with ivhd in /sys/class/iommu/. Given that this HT region is
> > hardcoded in iommu reserved regions since >=4.11 (to latest) I don't 
> > think it's
> > even worth checking the range exists in:
> >
> > /sys/kernel/iommu_groups/0/reserved_regions
> >
> > (Also that sysfs ABI is >= 4.11 only)
> 
>  Here's what I have staged in local tree, to address your comment.
> 
>  Naturally the first chunk is what's affected by this patch the rest is a
>  precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to pass
>  all the tests and what not.
> 
>  I am not entirely sure this is the right place to put such a helper, open
>  to suggestions. wrt to the naming of the helper, I tried to follow the 
>  rest
>  of the file's style.
> 
>  diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>  index a9be5d33a291..2ea4430d5dcc 100644
>  --- a/hw/i386/pc.c
>  +++ b/hw/i386/pc.c
>  @@ -868,10 +868,8 @@ static void 
>  x86_update_above_4g_mem_start(PCMachineState *pcms,
> uint64_t pci_hole64_size)
>   {
>   X86MachineState *x86ms = X86_MACHINE(pcms);
>  -uint32_t eax, vendor[3];
> 
>  -host_cpuid(0x0, 0, , [0], [2], [1]);
>  -if (!IS_AMD_VENDOR(vendor)) {
>  +if (!qemu_amd_iommu_is_present()) {
>   return;
>   }
> 
>  diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>  index 7bcce3bceb0f..eb4ea071ecec 100644
>  --- a/include/qemu/osdep.h
>  +++ b/include/qemu/osdep.h
>  @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
>    */
>   size_t qemu_get_host_physmem(void);
> 
>  +/**
>  + * qemu_amd_iommu_is_present:
>  + *
>  + * Operating system agnostic way of querying if an AMD IOMMU
>  + * is present.
>  + *
>  + */
>  +bool qemu_amd_iommu_is_present(void);
>  +
>   /*
>    * Toggle write/execute on the pages marked MAP_JIT
>    * for the current thread.
>  diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>  index f2be7321c59f..54cef21217c4 100644
>  --- a/util/oslib-posix.c
>  +++ b/util/oslib-posix.c
>  @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
>   #endif
>   return 0;
>   }
>  +
>  +bool qemu_amd_iommu_is_present(void)
>  +{
>  +bool found = false;
>  +#ifdef CONFIG_LINUX
>  +struct dirent *entry;
>  +char *path;
>  +DIR *dir;
>  +
>  +path = g_strdup_printf("/sys/class/iommu");
>  +dir = opendir(path);
>  +if (!dir) {
>  +g_free(path);
>  +return found;
>  +}
>  +
>  +do {
>  +entry = readdir(dir);
>  +if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
>  +found = true;
>  +break;
>  +}
>  +} while (entry);
>  +
>  +g_free(path);
>  +closedir(dir);
>  +#endif
>  +return found;
>  +}
> >>>
> >>> why are we checking whether an AMD IOMMU is present
> >>> on the host? 
> >>> Isn't what we care about whether it's
> >>> present in the VM? What we are reserving after all
> >>> is a range of GPAs, not actual host PA's ...
> >>>
> >> Right.
> >>
> >> But any DMA map done by VFIO/IOMMU using those GPA ranges will fail,
> >> and so we need to not map that portion of address space entirely
> >> and use 

Re: [PATCH 05/12] compiler.h: drop __printf__ macro MinGW/glib workaround

[Marc-André Lureau]

Hi Peter

On Thu, Feb 24, 2022 at 11:23 PM Peter Maydell 
wrote:

> On Thu, 24 Feb 2022 at 18:38,  wrote:
> >
> > From: Marc-André Lureau 
> >
> > This workaround was added in commit 95df51a4 ("w32: Always use standard
> > instead of native format strings"), as it claimed glib was using
> > __printf__ attribute. This is surprising, since glib has always used
> > G_GNUC_PRINTF which, as the name implies, uses __gnu_printf__ when
> > possible.
>
> This was not always true: before this commit from 2018
>
> https://github.com/GNOME/glib/commit/98a0ab929d8c59ee27e5f470f11d077bb6a56749
> G_GNUC_PRINTF used always used __printf__.
> I think that change only landed in glib 2.58, so since our current
> minimum glib version is 2.56 we need to retain this workaround.
>
>
Oops


> > Apparently, the workaound is no longer relevant though, I don't see
> > the warnings.
>
> You're probably building with a newer glib, and possibly also
> a newer mingw.
>
> I've cc'd Stefan Weil who might know whether we can drop this
> workaround as far as the mingw part is concerned.
>

Probably safer to keep it until we bump glib dependency to >=2.58.

I would move it to glib-compat.h though, and leave a note there, as it is
(or should be ) an old glib specific workaround.


>
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  include/qemu/compiler.h | 8 
> >  1 file changed, 8 deletions(-)
> >
> > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> > index 2704c314dcac..eb29b72c14d7 100644
> > --- a/include/qemu/compiler.h
> > +++ b/include/qemu/compiler.h
> > @@ -73,14 +73,6 @@
> >  #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x))
> - \
> > sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
> >
> > -#if !defined(__clang__) && defined(_WIN32)
> > -/*
> > - * Map __printf__ to __gnu_printf__ because we want standard format
> strings even
> > - * when MinGW or GLib include files use __printf__.
> > - */
> > -# define __printf__ __gnu_printf__
> > -#endif
> > -
> >  #ifndef __has_warning
> >  #define __has_warning(x) 0 /* compatibility with non-clang compilers */
> >  #endif
> > --
> > 2.35.1.273.ge6ebfd0e8cbb
>
> thanks
> -- PMM
>
>

-- 
Marc-André Lureau

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Joao Martins]

On 2/24/22 18:30, Michael S. Tsirkin wrote:
> On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
>> On 2/24/22 17:23, Michael S. Tsirkin wrote:
>>> On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
 On 2/23/22 23:35, Joao Martins wrote:
> On 2/23/22 21:22, Michael S. Tsirkin wrote:
>>> +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
>>> +  uint64_t pci_hole64_size)
>>> +{
>>> +X86MachineState *x86ms = X86_MACHINE(pcms);
>>> +uint32_t eax, vendor[3];
>>> +
>>> +host_cpuid(0x0, 0, , [0], [2], [1]);
>>> +if (!IS_AMD_VENDOR(vendor)) {
>>> +return;
>>> +}
>>
>> Wait a sec, should this actually be tying things to the host CPU ID?
>> It's really about what we present to the guest though,
>> isn't it?
>
> It was the easier catch all to use cpuid without going into
> Linux UAPI specifics. But it doesn't have to tie in there, it is only
> for systems with an IOMMU present.
>
>> Also, can't we tie this to whether the AMD IOMMU is present?
>>
> I think so, I can add that. Something like a amd_iommu_exists() helper
> in util/vfio-helpers.c which checks if there's any sysfs child entries
> that start with ivhd in /sys/class/iommu/. Given that this HT region is
> hardcoded in iommu reserved regions since >=4.11 (to latest) I don't 
> think it's
> even worth checking the range exists in:
>
>   /sys/kernel/iommu_groups/0/reserved_regions
>
> (Also that sysfs ABI is >= 4.11 only)

 Here's what I have staged in local tree, to address your comment.

 Naturally the first chunk is what's affected by this patch the rest is a
 precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to pass
 all the tests and what not.

 I am not entirely sure this is the right place to put such a helper, open
 to suggestions. wrt to the naming of the helper, I tried to follow the rest
 of the file's style.

 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index a9be5d33a291..2ea4430d5dcc 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -868,10 +868,8 @@ static void 
 x86_update_above_4g_mem_start(PCMachineState *pcms,
uint64_t pci_hole64_size)
  {
  X86MachineState *x86ms = X86_MACHINE(pcms);
 -uint32_t eax, vendor[3];

 -host_cpuid(0x0, 0, , [0], [2], [1]);
 -if (!IS_AMD_VENDOR(vendor)) {
 +if (!qemu_amd_iommu_is_present()) {
  return;
  }

 diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
 index 7bcce3bceb0f..eb4ea071ecec 100644
 --- a/include/qemu/osdep.h
 +++ b/include/qemu/osdep.h
 @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
   */
  size_t qemu_get_host_physmem(void);

 +/**
 + * qemu_amd_iommu_is_present:
 + *
 + * Operating system agnostic way of querying if an AMD IOMMU
 + * is present.
 + *
 + */
 +bool qemu_amd_iommu_is_present(void);
 +
  /*
   * Toggle write/execute on the pages marked MAP_JIT
   * for the current thread.
 diff --git a/util/oslib-posix.c b/util/oslib-posix.c
 index f2be7321c59f..54cef21217c4 100644
 --- a/util/oslib-posix.c
 +++ b/util/oslib-posix.c
 @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
  #endif
  return 0;
  }
 +
 +bool qemu_amd_iommu_is_present(void)
 +{
 +bool found = false;
 +#ifdef CONFIG_LINUX
 +struct dirent *entry;
 +char *path;
 +DIR *dir;
 +
 +path = g_strdup_printf("/sys/class/iommu");
 +dir = opendir(path);
 +if (!dir) {
 +g_free(path);
 +return found;
 +}
 +
 +do {
 +entry = readdir(dir);
 +if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
 +found = true;
 +break;
 +}
 +} while (entry);
 +
 +g_free(path);
 +closedir(dir);
 +#endif
 +return found;
 +}
>>>
>>> why are we checking whether an AMD IOMMU is present
>>> on the host? 
>>> Isn't what we care about whether it's
>>> present in the VM? What we are reserving after all
>>> is a range of GPAs, not actual host PA's ...
>>>
>> Right.
>>
>> But any DMA map done by VFIO/IOMMU using those GPA ranges will fail,
>> and so we need to not map that portion of address space entirely
>> and use some other portion of the GPA-space. This has to
>> do with host IOMMU which is where the DMA mapping of guest PA takes
>> place. So, if you do not have an host IOMMU, you can't
>> service guest DMA/PCIe services via VFIO through the host IOMMU, therefore 
>> you
>> don't need this problem.
>>
>> Whether one uses a guest IOMMU or not does not 

Re: [PATCH 03/12] osdep.h: move qemu_build_not_reached()

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

Move the macro and declaration so it can use glib in the following
patch.

Signed-off-by: Marc-André Lureau
---
  include/qemu/compiler.h | 16 
  include/qemu/osdep.h| 16 
  2 files changed, 16 insertions(+), 16 deletions(-)


Would this be obviated by a change to _Noreturn?


r~

Re: [PATCH 06/12] Replace config-time define HOST_WORDS_BIGENDIAN

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

Replace a config-time define with a compile time condition
define (compatible with clang and gcc) that must be declared prior to
its usage. This avoids having a global configure time define, but also
prevents from bad usage, if the config header wasn't included before.

This can help to make some code independent from qemu too.

gcc supports __BYTE_ORDER__ from about 4.6 and clang from 3.2.

Signed-off-by: Marc-André Lureau


Looks ok.  I'd like HOST_WORDS_BIGENDIAN to be poisoned, so that we're assured that we've 
caught all uses and new ones don't creep back in.



r~

Re: [PATCH 02/12] compiler.h: replace QEMU_SENTINEL with G_GNUC_NULL_TERMINATED

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

One less qemu-specific macro. It also helps to make some headers/units
only depend on glib, and thus moved in standalone projects eventually.

Signed-off-by: Marc-André Lureau
---
  include/qemu/compiler.h| 2 --
  include/qom/object.h   | 6 +++---
  scripts/cocci-macro-file.h | 2 +-
  scripts/checkpatch.pl  | 2 +-
  4 files changed, 5 insertions(+), 7 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 04/12] compiler.h: replace QEMU_NORETURN with G_NORETURN

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

G_NORETURN was introduced in glib 2.68, fallback to G_GNUC_NORETURN in
glib-compat.

Note that this attribute must be placed before the function declaration
(bringing a bit of consistency in qemu codebase usage).

Signed-off-by: Marc-André Lureau 


For C11, G_NORETURN is a wrapper for _Noreturn.
Since we're using C11, we should just use _Noreturn.


  void xtensa_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
  MMUAccessType access_type, int mmu_idx,
-uintptr_t retaddr) QEMU_NORETURN;
+uintptr_t retaddr) G_NORETURN;


Incorrect placement.  I didn't scan the whole patch; I assume that using _Noreturn will 
flag this up as an error.



-static void QEMU_NORETURN dump_core_and_abort(int target_sig)
+G_NORETURN static void dump_core_and_abort(int target_sig)


I guess this can go either place, but I think I prefer the scope specifier 
first.


r~

Re: [PATCH 01/12] compiler.h: replace QEMU_WARN_UNUSED_RESULT with G_GNUC_WARN_UNUSED_RESULT

[Richard Henderson]

On 2/24/22 08:36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau

One less qemu-specific macro. It also helps to make some headers/units
only depend on glib, and thus moved in standalone projects eventually.

Signed-off-by: Marc-André Lureau
---
  include/qemu-common.h  |  2 +-
  include/qemu/compiler.h|  2 --
  include/qemu/range.h   |  4 ++--
  scripts/cocci-macro-file.h |  2 +-
  block/qcow2-refcount.c | 20 +++-
  scripts/checkpatch.pl  |  2 +-
  6 files changed, 16 insertions(+), 16 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 0/5] Fixups for PMBus and new sensors

[Patrick Venture]

On Thu, Jan 6, 2022 at 3:09 PM Titus Rwantare  wrote:

> This patch series contains updates to PMBus in QEMU along with some PMBus
> device models for Renesas regulators.
> I have also added myself to MAINTAINERS as this code is in use daily,
> where I am responsible for it.
>
> Shengtan Mao (1):
>   hw/i2c: Added linear mode translation for pmbus devices
>
> Titus Rwantare (4):
>   hw/i2c: pmbus updates
>   hw/sensor: add Intersil ISL69260 device model
>   hw/sensor: add Renesas raa229004 PMBus device
>   hw/misc: add Renesas raa228000 device
>
>  MAINTAINERS   |  15 +-
>  hw/arm/Kconfig|   1 +
>  hw/i2c/pmbus_device.c | 106 +++-
>  hw/sensor/Kconfig |   5 +
>  hw/sensor/isl_pmbus.c | 278 
>  hw/sensor/meson.build |   1 +
>  include/hw/i2c/pmbus_device.h |  23 +-
>  include/hw/sensor/isl_pmbus.h |  52 
>  tests/qtest/isl_pmbus-test.c  | 460 ++
>  tests/qtest/meson.build   |   1 +
>  10 files changed, 930 insertions(+), 12 deletions(-)
>  create mode 100644 hw/sensor/isl_pmbus.c
>  create mode 100644 include/hw/sensor/isl_pmbus.h
>  create mode 100644 tests/qtest/isl_pmbus-test.c
>
>
Friendly ping - I believe I saw some of these have picked up Reviewer tags,
but ideally this will get into 7.0 before next month's soft-freeze.


> --
> 2.34.1.448.ga2b2bfdf31-goog
>
>

Re: [PATCH v4 38/47] target/ppc: Refactor VSX_SCALAR_CMP_DP

[Víctor Colombo]

On 22/02/2022 21:20, Richard Henderson wrote:> On 2/22/22 04:36, 
matheus.fe...@eldorado.org.br wrote:

From: Víctor Colombo 

Refactor VSX_SCALAR_CMP_DP, changing its name to VSX_SCALAR_CMP and
prepare the helper to be used for quadword comparisons.

Signed-off-by: Víctor Colombo 
Signed-off-by: Matheus Ferst 
---
  target/ppc/fpu_helper.c | 31 ++-
  1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 9b034d1fe4..5ebbcfe3b7 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2265,28 +2265,30 @@ VSX_MADDQ(XSNMSUBQP, NMSUB_FLGS, 0)
  VSX_MADDQ(XSNMSUBQPO, NMSUB_FLGS, 0)

  /*
- * VSX_SCALAR_CMP_DP - VSX scalar floating point compare double 
precision

+ * VSX_SCALAR_CMP - VSX scalar floating point compare
   *   op    - instruction mnemonic
+ *   tp    - type
   *   cmp   - comparison operation
   *   exp   - expected result of comparison
+ *   fld   - vsr_t field
   *   svxvc - set VXVC bit
   */
-#define VSX_SCALAR_CMP_DP(op, cmp, exp, 
svxvc)    \
+#define VSX_SCALAR_CMP(op, tp, cmp, fld, exp, 
svxvc)  \
  void helper_##op(CPUPPCState *env, ppc_vsr_t 
*xt, \
   ppc_vsr_t *xa, ppc_vsr_t 
*xb)    \
  
{ 
\
-    ppc_vsr_t t = 
*xt;    \
+    ppc_vsr_t t = { 
};    \
  bool vxsnan_flag = false, vxvc_flag = false, vex_flag = 
false;    \

\
-    if (float64_is_signaling_nan(xa->VsrD(0), >fp_status) 
|| \
-    float64_is_signaling_nan(xb->VsrD(0), >fp_status)) 
{ \
+    if (tp##_is_signaling_nan(xa->fld, >fp_status) 
||    \
+    tp##_is_signaling_nan(xb->fld, >fp_status)) 
{    \
  vxsnan_flag = 
true;   \
  if (fpscr_ve == 0 && svxvc) 
{ \
  vxvc_flag = 
true; \
  
} \
  } else if (svxvc) 
{   \
-    vxvc_flag = float64_is_quiet_nan(xa->VsrD(0), 
>fp_status) || \
-    float64_is_quiet_nan(xb->VsrD(0), 
>fp_status);   \
+    vxvc_flag = tp##_is_quiet_nan(xa->fld, >fp_status) 
||    \
+    tp##_is_quiet_nan(xb->fld, 
>fp_status);  \

  }


Note that this can be simplified further, using the full FloatRelation 
result and

float_flag_invalid_snan.

Note that do_scalar_cmp gets half-way there, only checking for NaNs once 
we have
float_relation_unordered as a comparision result.  But it could go 
further and check

float_flag_invalid_snan and drop all of the other checks vs snan and qnan.


r~


Hello Richard! Thanks for your review

Could you please elaborate more on how do you think using
float*_compare and its FloatRelation result would work here?
I noticed do_scalar_cmp modifies CR and sets FPCC flag, which
is not what VSX_SCALAR_CMP do. Using that function would require a
rework.

An option I though would be to bring into VSX_SCALAR_CMP the
important necessary parts, something like this:

#define VSX_SCALAR_CMP(op, tp, cmp, fld, svxvc, expr) 
  ...
r = tp##_compare(xa->fld, xb->fld, >fp_status); 
   \
if (expr) { 
   \
memset(, 0xFF, sizeof(t.fld)); 
   \
} else if (r == float_relation_unordered) { 
   \
if (env->fp_status.float_exception_flags & 
float_flag_invalid_snan) { \
float_invalid_op_vxsnan(env, GETPC()); 
   \
if (fpscr_ve == 0 && svxvc) { 
   \
float_invalid_op_vxvc(env, 0, GETPC()); 
   \
} 
   \
} else if (svxvc) { 
   \
if (tp##_is_quiet_nan(xa->fld, >fp_status) || 
   \
tp##_is_quiet_nan(xb->fld, >fp_status)) { 
   \
float_invalid_op_vxvc(env, 0, GETPC()); 
   \
} 
   \
} 
   \
} 
   \

...
VSX_SCALAR_CMP(XSCMPEQDP, float64, eq, VsrD(0), 0, r == 
float_relation_equal)

VSX_SCALAR_CMP(XSCMPGEDP, float64, le, VsrD(0), 1, \
r == float_relation_equal || r == float_relation_greater)
VSX_SCALAR_CMP(XSCMPGTDP, float64, lt, VsrD(0), 1, r == 
float_relation_greater)


But this still looks convoluted. Another option I came with would be:

ppc_vsr_t t = { }; 
   \


   \
helper_reset_fpstatus(env); 
   \


   \
if (tp##_##cmp##_quiet(xb->fld, xa->fld, >fp_status)) { 
   \
memset(, 0xFF, sizeof(t.fld)); 
   

Re: [PATCH 11/12] util: remove the net/net.h dependency

[Peter Maydell]

On Thu, 24 Feb 2022 at 18:39,  wrote:
>
> From: Marc-André Lureau 
>
> Move qemu_ether_ntoa() which is only needed in net/.
>
> Signed-off-by: Marc-André Lureau 
> ---

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2] hw/i2c: flatten pca954x mux device

[Patrick Venture]

On Thu, Feb 24, 2022 at 2:56 AM Peter Maydell 
wrote:

> On Wed, 2 Feb 2022 at 17:57, Patrick Venture  wrote:
> >
> > Previously this device created N subdevices which each owned an i2c bus.
> > Now this device simply owns the N i2c busses directly.
> >
> > Tested: Verified devices behind mux are still accessible via qmp and i2c
> > from within an arm32 SoC.
> >
> > Reviewed-by: Hao Wu 
> > Signed-off-by: Patrick Venture 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > Tested-by: Philippe Mathieu-Daudé 
> > ---
> > v2: explicitly create an incrementing name for the i2c busses (channels).
> > ---
>
> Applied to target-arm.next, thanks.
>
> Apologies for not picking this up earlier, the v2 got lost in the
> side-conversation about spam filtering. (Blame gmail for not
> doing email threading properly if you like :-))
>

Thanks, and no problem.  This v2 is what we have downstream for this.

I'm working on a further improvement to it (separate feature change)
that'll allow setting an id on the device so that all its channels have
that the id embedded in them.  This'll handle some of the situations we're
observing where the qdev paths aren't great for command line added i2c
devices.  There's a side conversation going on about how best to accomplish
this.


>
> -- PMM
>

[RESEND PATCH v3 1/1] multifd: Remove some redundant code

[Li Zhang]

Clean up some unnecessary code

Signed-off-by: Li Zhang 
---
 migration/multifd.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 3242f688e5..d44cc6670f 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -854,19 +854,15 @@ static void multifd_new_send_channel_async(QIOTask *task, 
gpointer opaque)
 Error *local_err = NULL;
 
 trace_multifd_new_send_channel_async(p->id);
-if (qio_task_propagate_error(task, _err)) {
-goto cleanup;
-} else {
+if (!qio_task_propagate_error(task, _err)) {
 p->c = QIO_CHANNEL(sioc);
 qio_channel_set_delay(p->c, false);
 p->running = true;
-if (!multifd_channel_connect(p, sioc, local_err)) {
-goto cleanup;
+if (multifd_channel_connect(p, sioc, local_err)) {
+return;
 }
-return;
 }
 
-cleanup:
 multifd_new_send_channel_cleanup(p, sioc, local_err);
 }
 
@@ -1078,10 +1074,7 @@ static void *multifd_recv_thread(void *opaque)
 
 ret = qio_channel_read_all_eof(p->c, (void *)p->packet,
p->packet_len, _err);
-if (ret == 0) {   /* EOF */
-break;
-}
-if (ret == -1) {   /* Error */
+if (ret == 0 || ret == -1) {   /* 0: EOF  -1: Error */
 break;
 }
 
-- 
2.31.1

Re: [PATCH 05/12] compiler.h: drop __printf__ macro MinGW/glib workaround

[Peter Maydell]

On Thu, 24 Feb 2022 at 18:38,  wrote:
>
> From: Marc-André Lureau 
>
> This workaround was added in commit 95df51a4 ("w32: Always use standard
> instead of native format strings"), as it claimed glib was using
> __printf__ attribute. This is surprising, since glib has always used
> G_GNUC_PRINTF which, as the name implies, uses __gnu_printf__ when
> possible.

This was not always true: before this commit from 2018
https://github.com/GNOME/glib/commit/98a0ab929d8c59ee27e5f470f11d077bb6a56749
G_GNUC_PRINTF used always used __printf__.
I think that change only landed in glib 2.58, so since our current
minimum glib version is 2.56 we need to retain this workaround.

> Apparently, the workaound is no longer relevant though, I don't see
> the warnings.

You're probably building with a newer glib, and possibly also
a newer mingw.

I've cc'd Stefan Weil who might know whether we can drop this
workaround as far as the mingw part is concerned.

> Signed-off-by: Marc-André Lureau 
> ---
>  include/qemu/compiler.h | 8 
>  1 file changed, 8 deletions(-)
>
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 2704c314dcac..eb29b72c14d7 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -73,14 +73,6 @@
>  #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \
> sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
>
> -#if !defined(__clang__) && defined(_WIN32)
> -/*
> - * Map __printf__ to __gnu_printf__ because we want standard format strings 
> even
> - * when MinGW or GLib include files use __printf__.
> - */
> -# define __printf__ __gnu_printf__
> -#endif
> -
>  #ifndef __has_warning
>  #define __has_warning(x) 0 /* compatibility with non-clang compilers */
>  #endif
> --
> 2.35.1.273.ge6ebfd0e8cbb

thanks
-- PMM

Re: [PATCH 0/5] Fixups for PMBus and new sensors

[Corey Minyard]

On Thu, Feb 24, 2022 at 10:58:52AM -0800, Patrick Venture wrote:
> On Thu, Jan 6, 2022 at 3:09 PM Titus Rwantare  wrote:
> 
> > This patch series contains updates to PMBus in QEMU along with some PMBus
> > device models for Renesas regulators.
> > I have also added myself to MAINTAINERS as this code is in use daily,
> > where I am responsible for it.
> >
> > Shengtan Mao (1):
> >   hw/i2c: Added linear mode translation for pmbus devices
> >
> > Titus Rwantare (4):
> >   hw/i2c: pmbus updates
> >   hw/sensor: add Intersil ISL69260 device model
> >   hw/sensor: add Renesas raa229004 PMBus device
> >   hw/misc: add Renesas raa228000 device
> >
> >  MAINTAINERS   |  15 +-
> >  hw/arm/Kconfig|   1 +
> >  hw/i2c/pmbus_device.c | 106 +++-
> >  hw/sensor/Kconfig |   5 +
> >  hw/sensor/isl_pmbus.c | 278 
> >  hw/sensor/meson.build |   1 +
> >  include/hw/i2c/pmbus_device.h |  23 +-
> >  include/hw/sensor/isl_pmbus.h |  52 
> >  tests/qtest/isl_pmbus-test.c  | 460 ++
> >  tests/qtest/meson.build   |   1 +
> >  10 files changed, 930 insertions(+), 12 deletions(-)
> >  create mode 100644 hw/sensor/isl_pmbus.c
> >  create mode 100644 include/hw/sensor/isl_pmbus.h
> >  create mode 100644 tests/qtest/isl_pmbus-test.c
> >
> >
> Friendly ping - I believe I saw some of these have picked up Reviewer tags,
> but ideally this will get into 7.0 before next month's soft-freeze.

Did you split up patch 1 as Peter requested?

-corey

> 
> 
> > --
> > 2.34.1.448.ga2b2bfdf31-goog
> >
> >

Re: [PATCH V7 05/29] vl: start on wakeup request

[Dr. David Alan Gilbert]

* Steve Sistare (steven.sist...@oracle.com) wrote:
> If qemu starts and loads a VM in the suspended state, then a later wakeup
> request will set the state to running, which is not sufficient to initialize
> the vm, as vm_start was never called during this invocation of qemu.  See
> qemu_system_wakeup_request().
> 
> Define the start_on_wakeup_requested() hook to cause vm_start() to be called
> when processing the wakeup request.
> 
> Signed-off-by: Steve Sistare 
> ---
>  include/sysemu/runstate.h |  1 +
>  softmmu/runstate.c| 17 -
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
> index a535691..b655c7b 100644
> --- a/include/sysemu/runstate.h
> +++ b/include/sysemu/runstate.h
> @@ -51,6 +51,7 @@ void qemu_system_reset_request(ShutdownCause reason);
>  void qemu_system_suspend_request(void);
>  void qemu_register_suspend_notifier(Notifier *notifier);
>  bool qemu_wakeup_suspend_enabled(void);
> +void qemu_system_start_on_wakeup_request(void);
>  void qemu_system_wakeup_request(WakeupReason reason, Error **errp);
>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>  void qemu_register_wakeup_notifier(Notifier *notifier);
> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
> index 10d9b73..3d344c9 100644
> --- a/softmmu/runstate.c
> +++ b/softmmu/runstate.c
> @@ -115,6 +115,8 @@ static const RunStateTransition 
> runstate_transitions_def[] = {
>  { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
>  { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
>  { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
> +{ RUN_STATE_PRELAUNCH, RUN_STATE_SUSPENDED },
> +{ RUN_STATE_PRELAUNCH, RUN_STATE_PAUSED },

This seems separate? Is this the bit that allows you to load the VM into
suspended?
But I note you're allowing PAUSED or SUSPENDED here, but the wake up
code only handles suspended - is that expected?

>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
> @@ -335,6 +337,7 @@ void vm_state_notify(bool running, RunState state)
>  }
>  }
>  
> +static bool start_on_wakeup_requested;
>  static ShutdownCause reset_requested;
>  static ShutdownCause shutdown_requested;
>  static int shutdown_signal;
> @@ -562,6 +565,11 @@ void qemu_register_suspend_notifier(Notifier *notifier)
>  notifier_list_add(_notifiers, notifier);
>  }
>  
> +void qemu_system_start_on_wakeup_request(void)
> +{
> +start_on_wakeup_requested = true;
> +}

Markus: Is this OK, or should this actually be another runstate
(PRELAUNCH_SUSPENDED??? or the like??) - is there an interaction here
with the commandline change ideas for a build-the-guest at runtime?

Dave

>  void qemu_system_wakeup_request(WakeupReason reason, Error **errp)
>  {
>  trace_system_wakeup_request(reason);
> @@ -574,7 +582,14 @@ void qemu_system_wakeup_request(WakeupReason reason, 
> Error **errp)
>  if (!(wakeup_reason_mask & (1 << reason))) {
>  return;
>  }
> -runstate_set(RUN_STATE_RUNNING);
> +
> +if (start_on_wakeup_requested) {
> +start_on_wakeup_requested = false;
> +vm_start();
> +} else {
> +runstate_set(RUN_STATE_RUNNING);
> +}
> +
>  wakeup_reason = reason;
>  qemu_notify_event();
>  }
> -- 
> 1.8.3.1
> 
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[RFC PATCH 4/4] spapr: Add KVM-on-TCG migration support

[Fabiano Rosas]

This adds migration support for TCG pseries machines running a KVM-HV
guest.

The state that needs to be migrated is:

- the nested PTCR value;
- the in_nested flag;
- the nested_tb_offset.
- the saved host CPUPPCState structure;

Signed-off-by: Fabiano Rosas 

---
(this migrates just fine with L2 running stress and 1 VCPU in L1. With
32 VCPUs in L1 there's crashes which I still don't understand. They might
be related to L1 migration being flaky right now)
---
 hw/ppc/spapr.c  | 19 +++
 hw/ppc/spapr_cpu_core.c | 76 +
 target/ppc/machine.c| 44 
 3 files changed, 139 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f0b75b22bb..6e87c515db 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1934,6 +1934,13 @@ static bool spapr_patb_entry_needed(void *opaque)
 return !!spapr->patb_entry;
 }
 
+static bool spapr_nested_ptcr_needed(void *opaque)
+{
+SpaprMachineState *spapr = opaque;
+
+return !!spapr->nested_ptcr;
+}
+
 static const VMStateDescription vmstate_spapr_patb_entry = {
 .name = "spapr_patb_entry",
 .version_id = 1,
@@ -1945,6 +1952,17 @@ static const VMStateDescription vmstate_spapr_patb_entry 
= {
 },
 };
 
+static const VMStateDescription vmstate_spapr_nested_ptcr = {
+.name = "spapr_nested_ptcr",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = spapr_nested_ptcr_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UINT64(nested_ptcr, SpaprMachineState),
+VMSTATE_END_OF_LIST()
+},
+};
+
 static bool spapr_irq_map_needed(void *opaque)
 {
 SpaprMachineState *spapr = opaque;
@@ -2069,6 +2087,7 @@ static const VMStateDescription vmstate_spapr = {
 _spapr_cap_fwnmi,
 _spapr_fwnmi,
 _spapr_cap_rpt_invalidate,
+_spapr_nested_ptcr,
 NULL
 }
 };
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index efda7730f1..3ec13c0660 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -25,6 +25,7 @@
 #include "sysemu/reset.h"
 #include "sysemu/hw_accel.h"
 #include "qemu/error-report.h"
+#include "migration/cpu.h"
 
 static void spapr_reset_vcpu(PowerPCCPU *cpu)
 {
@@ -174,6 +175,80 @@ static const VMStateDescription vmstate_spapr_cpu_vpa = {
 }
 };
 
+static bool nested_needed(void *opaque)
+{
+SpaprCpuState *spapr_cpu = opaque;
+
+return spapr_cpu->in_nested;
+}
+
+static int nested_state_pre_save(void *opaque)
+{
+CPUPPCState *env = opaque;
+
+env->spr[SPR_LR] = env->lr;
+env->spr[SPR_CTR] = env->ctr;
+env->spr[SPR_XER] = cpu_read_xer(env);
+env->spr[SPR_CFAR] = env->cfar;
+
+return 0;
+}
+
+static int nested_state_post_load(void *opaque, int version_id)
+{
+CPUPPCState *env = opaque;
+
+env->lr = env->spr[SPR_LR];
+env->ctr = env->spr[SPR_CTR];
+cpu_write_xer(env, env->spr[SPR_XER]);
+env->cfar = env->spr[SPR_CFAR];
+
+return 0;
+}
+
+static const VMStateDescription vmstate_nested_host_state = {
+.name = "spapr_nested_host_state",
+.version_id = 1,
+.minimum_version_id = 1,
+.pre_save = nested_state_pre_save,
+.post_load = nested_state_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_UINTTL_ARRAY(gpr, CPUPPCState, 32),
+VMSTATE_UINTTL_ARRAY(spr, CPUPPCState, 1024),
+VMSTATE_UINT32_ARRAY(crf, CPUPPCState, 8),
+VMSTATE_UINTTL(nip, CPUPPCState),
+VMSTATE_UINTTL(msr, CPUPPCState),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static int nested_cpu_pre_load(void *opaque)
+{
+SpaprCpuState *spapr_cpu = opaque;
+
+spapr_cpu->nested_host_state = g_try_malloc(sizeof(CPUPPCState));
+if (!spapr_cpu->nested_host_state) {
+return -1;
+}
+
+return 0;
+}
+
+static const VMStateDescription vmstate_spapr_cpu_nested = {
+.name = "spapr_cpu/nested",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = nested_needed,
+.pre_load = nested_cpu_pre_load,
+.fields = (VMStateField[]) {
+VMSTATE_BOOL(in_nested, SpaprCpuState),
+VMSTATE_INT64(nested_tb_offset, SpaprCpuState),
+VMSTATE_STRUCT_POINTER_V(nested_host_state, SpaprCpuState, 1,
+ vmstate_nested_host_state, CPUPPCState),
+VMSTATE_END_OF_LIST()
+},
+};
+
 static const VMStateDescription vmstate_spapr_cpu_state = {
 .name = "spapr_cpu",
 .version_id = 1,
@@ -184,6 +259,7 @@ static const VMStateDescription vmstate_spapr_cpu_state = {
 },
 .subsections = (const VMStateDescription * []) {
 _spapr_cpu_vpa,
+_spapr_cpu_nested,
 NULL
 }
 };
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 7ee1984500..ae09b1bcfe 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -10,6 +10,7 @@
 #include "kvm_ppc.h"
 #include "power8-pmu.h"
 #include "hw/ppc/ppc.h"
+#include "hw/ppc/spapr_cpu_core.h"
 
 static void post_load_update_msr(CPUPPCState *env)
 {

[PATCH v4] tests/qtest: add qtests for npcm7xx sdhci

[Hao Wu]

From: Shengtan Mao 

Reviewed-by: Hao Wu 
Reviewed-by: Chris Rauer 
Signed-off-by: Shengtan Mao 
Signed-off-by: Patrick Venture 
Signed-off-by: Hao Wu 
---
v4:
 * use strncmp to compare fixed length strings
v3:
 * fixup compilation from missing macro value
v2:
 * update copyright year
 * check result of open
 * use g_free instead of free
 * move declarations to the top
 * use g_file_open_tmp
---
 tests/qtest/meson.build  |   1 +
 tests/qtest/npcm7xx_sdhci-test.c | 215 +++
 2 files changed, 216 insertions(+)
 create mode 100644 tests/qtest/npcm7xx_sdhci-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 842b1df420..2b566999cd 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -189,6 +189,7 @@ qtests_npcm7xx = \
'npcm7xx_gpio-test',
'npcm7xx_pwm-test',
'npcm7xx_rng-test',
+   'npcm7xx_sdhci-test',
'npcm7xx_smbus-test',
'npcm7xx_timer-test',
'npcm7xx_watchdog_timer-test'] + \
diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
new file mode 100644
index 00..7de28f900b
--- /dev/null
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -0,0 +1,215 @@
+/*
+ * QTests for NPCM7xx SD-3.0 / MMC-4.51 Host Controller
+ *
+ * Copyright (c) 2022 Google LLC
+ *
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sd/npcm7xx_sdhci.h"
+
+#include "libqos/libqtest.h"
+#include "libqtest-single.h"
+#include "libqos/sdhci-cmd.h"
+
+#define NPCM7XX_REG_SIZE 0x100
+#define NPCM7XX_MMC_BA 0xF0842000
+#define NPCM7XX_BLK_SIZE 512
---
 tests/qtest/meson.build  |   1 +
 tests/qtest/npcm7xx_sdhci-test.c | 215 +++
 2 files changed, 216 insertions(+)
 create mode 100644 tests/qtest/npcm7xx_sdhci-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f33d84d19b..721eafad12 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -190,6 +190,7 @@ qtests_npcm7xx = \
'npcm7xx_gpio-test',
'npcm7xx_pwm-test',
'npcm7xx_rng-test',
+   'npcm7xx_sdhci-test',
'npcm7xx_smbus-test',
'npcm7xx_timer-test',
'npcm7xx_watchdog_timer-test'] + \
diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
new file mode 100644
index 00..fbb4e2f2e1
--- /dev/null
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -0,0 +1,215 @@
+/*
+ * QTests for NPCM7xx SD-3.0 / MMC-4.51 Host Controller
+ *
+ * Copyright (c) 2022 Google LLC
+ *
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sd/npcm7xx_sdhci.h"
+
+#include "libqos/libqtest.h"
+#include "libqtest-single.h"
+#include "libqos/sdhci-cmd.h"
+
+#define NPCM7XX_REG_SIZE 0x100
+#define NPCM7XX_MMC_BA 0xF0842000
+#define NPCM7XX_BLK_SIZE 512
+#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30)
+
+char *sd_path;
+
+static QTestState *setup_sd_card(void)
+{
+QTestState *qts = qtest_initf(
+"-machine kudo-bmc "
+"-device sd-card,drive=drive0 "
+"-drive id=drive0,if=none,file=%s,format=raw,auto-read-only=off",
+sd_path);
+
+qtest_writew(qts, NPCM7XX_MMC_BA + SDHC_SWRST, SDHC_RESET_ALL);
+qtest_writew(qts, NPCM7XX_MMC_BA + SDHC_CLKCON,
+ SDHC_CLOCK_SDCLK_EN | SDHC_CLOCK_INT_STABLE |
+ SDHC_CLOCK_INT_EN);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_APP_CMD);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4120, 0, (41 << 8));
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4567, 0,
+   SDHC_SELECT_DESELECT_CARD);
+
+return qts;
+}
+
+static void write_sdread(QTestState *qts, const char *msg)
+{
+int fd, ret;
+size_t len = strlen(msg);
+char *rmsg = g_malloc(len);
+
+/* write message to sd */
+fd = open(sd_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, msg, len);
+close(fd);
+g_assert(ret 

[RFC PATCH 3/4] hw/ppc: Take nested guest into account when saving timebase

[Fabiano Rosas]

When saving the guest "timebase" we look to the first_cpu for its
tb_offset. If that CPU happens to be running a nested guest at this
time, the tb_offset will have the nested guest value.

This was caught by code inspection.

Signed-off-by: Fabiano Rosas 
---
 hw/ppc/ppc.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 9e99625ea9..093cd87014 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -36,6 +36,7 @@
 #include "kvm_ppc.h"
 #include "migration/vmstate.h"
 #include "trace.h"
+#include "hw/ppc/spapr_cpu_core.h"
 
 static void cpu_ppc_tb_stop (CPUPPCState *env);
 static void cpu_ppc_tb_start (CPUPPCState *env);
@@ -961,19 +962,33 @@ static void timebase_save(PPCTimebase *tb)
 {
 uint64_t ticks = cpu_get_host_ticks();
 PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
+int64_t tb_offset;
 
 if (!first_ppc_cpu->env.tb_env) {
 error_report("No timebase object");
 return;
 }
 
+tb_offset = first_ppc_cpu->env.tb_env->tb_offset;
+
+if (first_ppc_cpu->vhyp && vhyp_cpu_in_nested(first_ppc_cpu)) {
+SpaprCpuState *spapr_cpu = spapr_cpu_state(first_ppc_cpu);
+
+/*
+ * If the first_cpu happens to be running a nested guest at
+ * this time, tb_env->tb_offset will contain the nested guest
+ * offset.
+ */
+tb_offset -= spapr_cpu->nested_tb_offset;
+}
+
 /* not used anymore, we keep it for compatibility */
 tb->time_of_the_day_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST);
 /*
  * tb_offset is only expected to be changed by QEMU so
  * there is no need to update it from KVM here
  */
-tb->guest_timebase = ticks + first_ppc_cpu->env.tb_env->tb_offset;
+tb->guest_timebase = ticks + tb_offset;
 
 tb->runstate_paused =
 runstate_check(RUN_STATE_PAUSED) || runstate_check(RUN_STATE_SAVE_VM);
-- 
2.34.1

[PATCH 12/12] qapi: remove needless include

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 qapi/qmp-dispatch.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index d378bccac73b..0990873ec8ec 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -21,7 +21,6 @@
 #include "qapi/qmp/qjson.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qobject-output-visitor.h"
-#include "sysemu/runstate.h"
 #include "qapi/qmp/qbool.h"
 #include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
-- 
2.35.1.273.ge6ebfd0e8cbb

[RFC PATCH 0/4] ppc: nested TCG migration (KVM-on-TCG)

[Fabiano Rosas]

This series implements the migration for a TCG pseries guest running a
nested KVM guest. This is just like migrating a pseries TCG guest, but
with some extra state to allow a nested guest to continue to run on
the destination.

Unfortunately the regular TCG migration scenario (not nested) is not
fully working so I cannot be entirely sure the nested migration is
correct. I have included a couple of patches for the general migration
case that (I think?) improve the situation a bit, but I'm still seeing
hard lockups and other issues with more than 1 vcpu.

This is more of an early RFC to see if anyone spots something right
away. I haven't made much progress in debugging the general TCG
migration case so if anyone has any input there as well I'd appreciate
it.

Thanks

Fabiano Rosas (4):
  target/ppc: TCG: Migrate tb_offset and decr
  spapr: TCG: Migrate spapr_cpu->prod
  hw/ppc: Take nested guest into account when saving timebase
  spapr: Add KVM-on-TCG migration support

 hw/ppc/ppc.c| 17 +++-
 hw/ppc/spapr.c  | 19 
 hw/ppc/spapr_cpu_core.c | 77 +
 include/hw/ppc/spapr_cpu_core.h |  2 +-
 target/ppc/machine.c| 61 ++
 5 files changed, 174 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 11/12] util: remove the net/net.h dependency

[marcandre . lureau]

From: Marc-André Lureau 

Move qemu_ether_ntoa() which is only needed in net/.

Signed-off-by: Marc-André Lureau 
---
 include/qemu-common.h |  1 -
 net/announce.c| 13 +
 util/cutils.c | 14 --
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 79977cb3ec43..e702c5325674 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -108,7 +108,6 @@ void qemu_hexdump(FILE *fp, const char *prefix,
  */
 int parse_debug_env(const char *name, int max, int initial);
 
-const char *qemu_ether_ntoa(const MACAddr *mac);
 void page_size_init(void);
 
 /* returns non-zero if dump is in progress, otherwise zero is
diff --git a/net/announce.c b/net/announce.c
index 26f057f5ee47..3b9e2f1f14e8 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -120,6 +120,19 @@ static int announce_self_create(uint8_t *buf,
 return 60; /* len (FCS will be added by hardware) */
 }
 
+/*
+ * Helper to print ethernet mac address
+ */
+static const char *qemu_ether_ntoa(const MACAddr *mac)
+{
+static char ret[18];
+
+snprintf(ret, sizeof(ret), "%02x:%02x:%02x:%02x:%02x:%02x",
+ mac->a[0], mac->a[1], mac->a[2], mac->a[3], mac->a[4], mac->a[5]);
+
+return ret;
+}
+
 static void qemu_announce_self_iter(NICState *nic, void *opaque)
 {
 AnnounceTimer *timer = opaque;
diff --git a/util/cutils.c b/util/cutils.c
index 53346138c970..0d475ec4cddd 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -27,7 +27,6 @@
 #include 
 
 #include "qemu-common.h"
-#include "net/net.h"
 #include "qemu/ctype.h"
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
@@ -936,19 +935,6 @@ int parse_debug_env(const char *name, int max, int initial)
 return debug;
 }
 
-/*
- * Helper to print ethernet mac address
- */
-const char *qemu_ether_ntoa(const MACAddr *mac)
-{
-static char ret[18];
-
-snprintf(ret, sizeof(ret), "%02x:%02x:%02x:%02x:%02x:%02x",
- mac->a[0], mac->a[1], mac->a[2], mac->a[3], mac->a[4], mac->a[5]);
-
-return ret;
-}
-
 /*
  * Return human readable string for size @val.
  * @val can be anything that uint64_t allows (no more than "16 EiB").
-- 
2.35.1.273.ge6ebfd0e8cbb

[RFC PATCH 2/4] spapr: TCG: Migrate spapr_cpu->prod

[Fabiano Rosas]

I'm seeing some stack traces in the migrated guest going through cede
and some hangs at the plpar_hcall_norets so let's make sure everything
related to cede/prod is being migrated just in case.

Signed-off-by: Fabiano Rosas 
---
 hw/ppc/spapr_cpu_core.c | 1 +
 include/hw/ppc/spapr_cpu_core.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index ed84713960..efda7730f1 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -179,6 +179,7 @@ static const VMStateDescription vmstate_spapr_cpu_state = {
 .version_id = 1,
 .minimum_version_id = 1,
 .fields = (VMStateField[]) {
+VMSTATE_BOOL(prod, SpaprCpuState),
 VMSTATE_END_OF_LIST()
 },
 .subsections = (const VMStateDescription * []) {
diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
index b560514560..2772689c84 100644
--- a/include/hw/ppc/spapr_cpu_core.h
+++ b/include/hw/ppc/spapr_cpu_core.h
@@ -45,7 +45,7 @@ typedef struct SpaprCpuState {
 uint64_t vpa_addr;
 uint64_t slb_shadow_addr, slb_shadow_size;
 uint64_t dtl_addr, dtl_size;
-bool prod; /* not migrated, only used to improve dispatch latencies */
+bool prod;
 struct ICPState *icp;
 struct XiveTCTX *tctx;
 
-- 
2.34.1

Re: [PATCH V7 02/29] migration: fix populate_vfio_info

[Peter Maydell]

On Wed, 22 Dec 2021 at 19:45, Steve Sistare  wrote:
>
> Include CONFIG_DEVICES so that populate_vfio_info is instantiated for
> CONFIG_VFIO.

The commit message says "include CONFIG_DEVICES"...

> Signed-off-by: Steve Sistare 
> ---
>  migration/target.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/migration/target.c b/migration/target.c
> index 907ebf0..4390bf0 100644
> --- a/migration/target.c
> +++ b/migration/target.c
> @@ -8,18 +8,22 @@
>  #include "qemu/osdep.h"
>  #include "qapi/qapi-types-migration.h"
>  #include "migration.h"
> +#include CONFIG_DEVICES

...and the code change does do that, but...

>
>  #ifdef CONFIG_VFIO
> +
>  #include "hw/vfio/vfio-common.h"
> -#endif
>
>  void populate_vfio_info(MigrationInfo *info)
>  {
> -#ifdef CONFIG_VFIO
>  if (vfio_mig_active()) {
>  info->has_vfio = true;
>  info->vfio = g_malloc0(sizeof(*info->vfio));
>  info->vfio->transferred = vfio_mig_bytes_transferred();
>  }
> -#endif
>  }
> +#else
> +
> +void populate_vfio_info(MigrationInfo *info) {}
> +
> +#endif /* CONFIG_VFIO */

...it also seems to be making a no-change-of-behaviour rewrite
of the rest of the file. Is there a reason I'm missing for doing
that ?

thanks
-- PMM

Re: [PATCH v3] tests/qtest: add qtests for npcm7xx sdhci

[Hao Wu]

The problem is probably because we read it in using "strcmp". strcmp
compares two strings that end with "\0". But one of the string is read in
using read() so it didn't read in the ending '\0' character.

We should use strncmp to compare the two strings. It probably avoids the
issue.

On Tue, Feb 22, 2022 at 5:28 PM Patrick Venture  wrote:

>
>
> On Mon, Feb 21, 2022 at 5:30 AM Peter Maydell 
> wrote:
>
>> On Wed, 16 Feb 2022 at 17:30, Peter Maydell 
>> wrote:
>> >
>> > On Tue, 8 Feb 2022 at 18:18, Patrick Venture 
>> wrote:
>> > >
>> > > From: Shengtan Mao 
>> > >
>> > > Reviewed-by: Hao Wu 
>> > > Reviewed-by: Chris Rauer 
>> > > Signed-off-by: Shengtan Mao 
>> > > Signed-off-by: Patrick Venture 
>> > > ---
>> >
>> >
>> >
>> > Applied to target-arm.next, thanks.
>>
>> This hits assertions in some of the CI jobs, eg:
>> https://gitlab.com/qemu-project/qemu/-/jobs/2116932769
>>
>> 258/717 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test INTERRUPT
>> 643.16s killed by signal 6 SIGABRT
>> ― ✀
>> ―
>> stderr:
>> ** Message: 06:06:50.205: /tmp/sdhci_F7ETH1
>> **
>> ERROR:../tests/qtest/npcm7xx_sdhci-test.c:101:sdwrite_read: assertion
>> failed: (!strcmp(rmsg, msg))
>>
>> ――
>> ...terminated.
>>
>> so I've dropped it again.
>>
>
> I'm sorry to hear that, I'll have to pick up some cycles in a week or so
> and see if I can reproduce the issue.
>
>
>>
>> thanks
>> -- PMM
>>
>

[RFC PATCH 1/4] target/ppc: TCG: Migrate tb_offset and decr

[Fabiano Rosas]

These two were not migrated so the remote end was starting with the
decrementer expired.

I am seeing less frequent crashes with this patch (tested with -smp 4
and -smp 32). It certainly doesn't fix all issues but it looks like it
helps.

Signed-off-by: Fabiano Rosas 
---
 target/ppc/machine.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 1b63146ed1..7ee1984500 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -9,6 +9,7 @@
 #include "qemu/main-loop.h"
 #include "kvm_ppc.h"
 #include "power8-pmu.h"
+#include "hw/ppc/ppc.h"
 
 static void post_load_update_msr(CPUPPCState *env)
 {
@@ -666,6 +667,18 @@ static const VMStateDescription vmstate_compat = {
 }
 };
 
+static const VMStateDescription vmstate_tb_env = {
+.name = "cpu/env/tb_env",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_INT64(tb_offset, ppc_tb_t),
+VMSTATE_UINT64(decr_next, ppc_tb_t),
+VMSTATE_TIMER_PTR(decr_timer, ppc_tb_t),
+VMSTATE_END_OF_LIST()
+}
+};
+
 const VMStateDescription vmstate_ppc_cpu = {
 .name = "cpu",
 .version_id = 5,
@@ -696,12 +709,16 @@ const VMStateDescription vmstate_ppc_cpu = {
 /* Backward compatible internal state */
 VMSTATE_UINTTL(env.hflags_compat_nmsr, PowerPCCPU),
 
+VMSTATE_STRUCT_POINTER_V(env.tb_env, PowerPCCPU, 1,
+ vmstate_tb_env, ppc_tb_t),
+
 /* Sanity checking */
 VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
 VMSTATE_UINT64_TEST(mig_insns_flags, PowerPCCPU, 
cpu_pre_2_8_migration),
 VMSTATE_UINT64_TEST(mig_insns_flags2, PowerPCCPU,
 cpu_pre_2_8_migration),
 VMSTATE_UINT32_TEST(mig_nb_BATs, PowerPCCPU, cpu_pre_2_8_migration),
+
 VMSTATE_END_OF_LIST()
 },
 .subsections = (const VMStateDescription*[]) {
-- 
2.34.1

[PATCH 04/12] compiler.h: replace QEMU_NORETURN with G_NORETURN

[marcandre . lureau]

From: Marc-André Lureau 

G_NORETURN was introduced in glib 2.68, fallback to G_GNUC_NORETURN in
glib-compat.

Note that this attribute must be placed before the function declaration
(bringing a bit of consistency in qemu codebase usage).

Signed-off-by: Marc-André Lureau 
---
 accel/tcg/internal.h |  3 +--
 include/exec/exec-all.h  | 20 +--
 include/exec/helper-head.h   |  2 +-
 include/glib-compat.h|  4 
 include/hw/core/cpu.h|  2 +-
 include/hw/core/tcg-cpu-ops.h|  6 +++---
 include/hw/hw.h  |  2 +-
 include/qemu/compiler.h  |  2 --
 include/qemu/osdep.h |  2 +-
 include/qemu/thread.h|  2 +-
 include/tcg/tcg-ldst.h   |  4 ++--
 include/tcg/tcg.h|  2 +-
 linux-user/user-internals.h  |  2 +-
 scripts/cocci-macro-file.h   |  2 +-
 target/alpha/cpu.h   | 10 +-
 target/arm/internals.h   | 12 +--
 target/hppa/cpu.h|  2 +-
 target/i386/tcg/helper-tcg.h | 24 +++---
 target/microblaze/cpu.h  |  6 +++---
 target/mips/tcg/tcg-internal.h   | 16 +++
 target/nios2/cpu.h   |  6 +++---
 target/openrisc/exception.h  |  2 +-
 target/ppc/cpu.h | 14 ++---
 target/ppc/internal.h|  6 +++---
 target/riscv/cpu.h   | 10 +-
 target/s390x/s390x-internal.h|  6 +++---
 target/s390x/tcg/tcg_s390x.h | 12 +--
 target/sh4/cpu.h |  6 +++---
 target/sparc/cpu.h   | 10 +-
 target/xtensa/cpu.h  |  2 +-
 accel/stubs/tcg-stub.c   |  4 ++--
 bsd-user/signal.c|  2 +-
 hw/misc/mips_itu.c   |  2 +-
 linux-user/signal.c  |  2 +-
 monitor/hmp.c|  2 +-
 qemu-img.c   |  9 +
 target/alpha/helper.c| 10 +-
 target/arm/pauth_helper.c|  4 ++--
 target/arm/tlb_helper.c  |  6 +++---
 target/hexagon/op_helper.c   |  8 
 target/hppa/cpu.c|  2 +-
 target/hppa/op_helper.c  |  4 ++--
 target/i386/tcg/bpt_helper.c |  2 +-
 target/i386/tcg/excp_helper.c| 30 ++--
 target/i386/tcg/misc_helper.c|  6 +++---
 target/i386/tcg/sysemu/misc_helper.c |  6 +++---
 target/openrisc/exception.c  |  2 +-
 target/openrisc/exception_helper.c   |  2 +-
 target/riscv/op_helper.c |  4 ++--
 target/rx/op_helper.c| 20 +--
 target/s390x/tcg/excp_helper.c   | 20 +--
 target/sh4/op_helper.c   |  4 ++--
 target/sparc/mmu_helper.c|  8 
 target/tricore/op_helper.c   |  2 +-
 tcg/tcg.c|  2 +-
 tests/fp/fp-bench.c  |  2 +-
 tests/fp/fp-test.c   |  2 +-
 scripts/checkpatch.pl|  2 +-
 58 files changed, 185 insertions(+), 183 deletions(-)

diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index 881bc1ede0b1..3092bfa96430 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -14,8 +14,7 @@
 TranslationBlock *tb_gen_code(CPUState *cpu, target_ulong pc,
   target_ulong cs_base, uint32_t flags,
   int cflags);
-
-void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
+G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 void page_init(void);
 void tb_htable_init(void);
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 35d8e93976f5..d7510411ad7a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -61,10 +61,10 @@ void restore_state_to_opc(CPUArchState *env, 
TranslationBlock *tb,
  */
 bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc, bool will_exit);
 
-void QEMU_NORETURN cpu_loop_exit_noexc(CPUState *cpu);
-void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
-void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
-void QEMU_NORETURN cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc);
+G_NORETURN void cpu_loop_exit_noexc(CPUState *cpu);
+G_NORETURN void cpu_loop_exit(CPUState *cpu);
+G_NORETURN void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
+G_NORETURN void cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc);
 
 /**
  * cpu_loop_exit_requested:
@@ -697,9 +697,9 @@ bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t 
*old_set,
  * Use the TCGCPUOps hook to record cpu state, do guest operating system
  * specific things to raise SIGSEGV, and jump to the main cpu loop.
  */
-void QEMU_NORETURN cpu_loop_exit_sigsegv(CPUState *cpu, target_ulong addr,
- 

[PATCH 10/12] util: remove needless includes

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 util/cutils.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/util/cutils.c b/util/cutils.c
index c9b91e7535a8..53346138c970 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -27,8 +27,6 @@
 #include 
 
 #include "qemu-common.h"
-#include "qemu/sockets.h"
-#include "qemu/iov.h"
 #include "net/net.h"
 #include "qemu/ctype.h"
 #include "qemu/cutils.h"
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 07/12] Simplify HOST_LONG_BITS

[marcandre . lureau]

From: Marc-André Lureau 

Simplify the macro, not depending on headers defines, but compiler
predefined __SIZEOF__POINTER__ only.

Available since gcc 4.3 and clang 2.8.

Signed-off-by: Marc-André Lureau 
---
 include/qemu/osdep.h | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 74473867f3f6..cea7303c78a4 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -261,13 +261,7 @@ G_NORETURN extern void QEMU_ERROR("code path is reachable")
 #endif
 
 /* HOST_LONG_BITS is the size of a native pointer in bits. */
-#if UINTPTR_MAX == UINT32_MAX
-# define HOST_LONG_BITS 32
-#elif UINTPTR_MAX == UINT64_MAX
-# define HOST_LONG_BITS 64
-#else
-# error Unknown pointer size
-#endif
+#define HOST_LONG_BITS (__SIZEOF_POINTER__ * 8)
 
 /* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
  * the wrong type. Our replacement isn't usable in preprocessor
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 03/12] osdep.h: move qemu_build_not_reached()

[marcandre . lureau]

From: Marc-André Lureau 

Move the macro and declaration so it can use glib in the following
patch.

Signed-off-by: Marc-André Lureau 
---
 include/qemu/compiler.h | 16 
 include/qemu/osdep.h| 16 
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 0a5e67fb970e..a2d2d48dcf34 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -151,22 +151,6 @@
 #define QEMU_ALWAYS_INLINE
 #endif
 
-/**
- * qemu_build_not_reached()
- *
- * The compiler, during optimization, is expected to prove that a call
- * to this function cannot be reached and remove it.  If the compiler
- * supports QEMU_ERROR, this will be reported at compile time; otherwise
- * this will be reported at link time due to the missing symbol.
- */
-extern void QEMU_NORETURN QEMU_ERROR("code path is reachable")
-qemu_build_not_reached_always(void);
-#if defined(__OPTIMIZE__) && !defined(__NO_INLINE__)
-#define qemu_build_not_reached()  qemu_build_not_reached_always()
-#else
-#define qemu_build_not_reached()  g_assert_not_reached()
-#endif
-
 /**
  * In most cases, normal "fallthrough" comments are good enough for
  * switch-case statements, but sometimes the compiler has problems
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 7bcce3bceb0f..fb72b0006d5c 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -157,6 +157,22 @@ extern "C" {
 #define assert(x)  g_assert(x)
 #endif
 
+/**
+ * qemu_build_not_reached()
+ *
+ * The compiler, during optimization, is expected to prove that a call
+ * to this function cannot be reached and remove it.  If the compiler
+ * supports QEMU_ERROR, this will be reported at compile time; otherwise
+ * this will be reported at link time due to the missing symbol.
+ */
+extern void QEMU_NORETURN QEMU_ERROR("code path is reachable")
+qemu_build_not_reached_always(void);
+#if defined(__OPTIMIZE__) && !defined(__NO_INLINE__)
+#define qemu_build_not_reached()  qemu_build_not_reached_always()
+#else
+#define qemu_build_not_reached()  g_assert_not_reached()
+#endif
+
 /*
  * According to waitpid man page:
  * WCOREDUMP
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 09/12] scripts/modinfo-collect: remove unused/dead code

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 scripts/modinfo-collect.py | 5 -
 1 file changed, 5 deletions(-)

diff --git a/scripts/modinfo-collect.py b/scripts/modinfo-collect.py
index 61b90688c6dc..4e7584df6676 100755
--- a/scripts/modinfo-collect.py
+++ b/scripts/modinfo-collect.py
@@ -18,13 +18,8 @@ def find_command(src, target, compile_commands):
 
 def process_command(src, command):
 skip = False
-arg = False
 out = []
 for item in shlex.split(command):
-if arg:
-out.append(x)
-arg = False
-continue
 if skip:
 skip = False
 continue
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 08/12] Move HOST_LONG_BITS to compiler.h

[marcandre . lureau]

From: Marc-André Lureau 

This will help to make common code independent.

Signed-off-by: Marc-André Lureau 
---
 include/qemu/compiler.h | 3 +++
 include/qemu/osdep.h| 3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 8c7da00c56ba..553e49aac2ad 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -9,6 +9,9 @@
 
 #define HOST_BIG_ENDIAN (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
 
+/* HOST_LONG_BITS is the size of a native pointer in bits. */
+#define HOST_LONG_BITS (__SIZEOF_POINTER__ * 8)
+
 #if defined __clang_analyzer__ || defined __COVERITY__
 #define QEMU_STATIC_ANALYSIS 1
 #endif
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index cea7303c78a4..4711aad22803 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -260,9 +260,6 @@ G_NORETURN extern void QEMU_ERROR("code path is reachable")
 #define TIME_MAX TYPE_MAXIMUM(time_t)
 #endif
 
-/* HOST_LONG_BITS is the size of a native pointer in bits. */
-#define HOST_LONG_BITS (__SIZEOF_POINTER__ * 8)
-
 /* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
  * the wrong type. Our replacement isn't usable in preprocessor
  * expressions, but it is sufficient for our needs. */
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 06/12] Replace config-time define HOST_WORDS_BIGENDIAN

[marcandre . lureau]

From: Marc-André Lureau 

Replace a config-time define with a compile time condition
define (compatible with clang and gcc) that must be declared prior to
its usage. This avoids having a global configure time define, but also
prevents from bad usage, if the config header wasn't included before.

This can help to make some code independent from qemu too.

gcc supports __BYTE_ORDER__ from about 4.6 and clang from 3.2.

Signed-off-by: Marc-André Lureau 
---
 meson.build |  1 -
 accel/tcg/atomic_template.h |  4 +-
 audio/audio.h   |  2 +-
 hw/display/pl110_template.h |  6 +--
 hw/net/can/ctucan_core.h|  2 +-
 hw/net/vmxnet3.h|  4 +-
 include/exec/cpu-all.h  |  4 +-
 include/exec/cpu-common.h   |  2 +-
 include/exec/memop.h|  2 +-
 include/exec/memory.h   |  2 +-
 include/fpu/softfloat-types.h   |  2 +-
 include/hw/core/cpu.h   |  2 +-
 include/hw/i386/intel_iommu.h   |  6 +--
 include/hw/i386/x86-iommu.h |  4 +-
 include/hw/virtio/virtio-access.h   |  6 +--
 include/hw/virtio/virtio-gpu-bswap.h|  2 +-
 include/libdecnumber/dconfig.h  |  2 +-
 include/net/eth.h   |  2 +-
 include/qemu/bswap.h|  8 ++--
 include/qemu/compiler.h |  2 +
 include/qemu/host-utils.h   |  2 +-
 include/qemu/int128.h   |  2 +-
 include/ui/qemu-pixman.h|  2 +-
 net/util.h  |  2 +-
 target/arm/cpu.h|  8 ++--
 target/arm/translate-a64.h  |  2 +-
 target/arm/vec_internal.h   |  2 +-
 target/i386/cpu.h   |  2 +-
 target/mips/cpu.h   |  2 +-
 target/ppc/cpu.h|  2 +-
 target/s390x/tcg/vec.h  |  2 +-
 target/xtensa/cpu.h |  2 +-
 tests/fp/platform.h |  4 +-
 accel/kvm/kvm-all.c |  4 +-
 audio/dbusaudio.c   |  2 +-
 disas.c |  2 +-
 hw/core/loader.c|  4 +-
 hw/display/artist.c |  6 +--
 hw/display/pxa2xx_lcd.c |  2 +-
 hw/display/vga.c| 12 +++---
 hw/display/virtio-gpu-gl.c  |  2 +-
 hw/s390x/event-facility.c   |  2 +-
 hw/virtio/vhost.c   |  2 +-
 linux-user/arm/nwfpe/double_cpdo.c  |  4 +-
 linux-user/arm/nwfpe/fpa11_cpdt.c   |  4 +-
 linux-user/ppc/signal.c |  3 +-
 linux-user/syscall.c|  6 +--
 net/net.c   |  4 +-
 target/alpha/translate.c|  2 +-
 target/arm/crypto_helper.c  |  2 +-
 target/arm/helper.c |  2 +-
 target/arm/kvm64.c  |  4 +-
 target/arm/neon_helper.c|  2 +-
 target/arm/sve_helper.c |  4 +-
 target/arm/translate-sve.c  |  6 +--
 target/arm/translate-vfp.c  |  2 +-
 target/arm/translate.c  |  2 +-
 target/hppa/translate.c |  2 +-
 target/i386/tcg/translate.c |  2 +-
 target/mips/tcg/lmmi_helper.c   |  2 +-
 target/mips/tcg/msa_helper.c| 54 -
 target/ppc/arch_dump.c  |  2 +-
 target/ppc/int_helper.c | 22 +-
 target/ppc/kvm.c|  4 +-
 target/ppc/mem_helper.c |  2 +-
 target/riscv/vector_helper.c|  2 +-
 target/s390x/tcg/translate.c|  2 +-
 target/sparc/vis_helper.c   |  4 +-
 tcg/tcg-op.c|  4 +-
 tcg/tcg.c   | 12 +++---
 tests/qtest/vhost-user-blk-test.c   |  2 +-
 tests/qtest/virtio-blk-test.c   |  2 +-
 ui/vdagent.c|  2 +-
 ui/vnc.c|  2 +-
 util/bitmap.c   |  2 +-
 util/host-utils.c   |  2 +-
 target/ppc/translate/vmx-impl.c.inc |  4 +-
 target/ppc/translate/vsx-impl.c.inc |  2 +-
 target/riscv/insn_trans/trans_rvv.c.inc |  4 +-
 target/s390x/tcg/translate_vx.c.inc |  2 +-
 tcg/aarch64/tcg-target.c.inc|  4 +-
 tcg/arm/tcg-target.c.inc|  4 +-
 tcg/mips/tcg-target.c.inc   |  2 +-
 tcg/ppc/tcg-target.c.inc| 10 ++---
 tcg/riscv/tcg-target.c.inc  |  4 +-
 85 files changed, 173 insertions(+), 173 deletions(-)

diff --git a/meson.build b/meson.build
index b1d2fcecbdcf..a97c6b0b5dbc 100644
--- a/meson.build
+++ b/meson.build
@@ -1574,7 +1574,6 @@ config_host_data.set('QEMU_VERSION_MICRO', 
meson.project_version().split('.')[2]
 
 

[PATCH 05/12] compiler.h: drop __printf__ macro MinGW/glib workaround

[marcandre . lureau]

From: Marc-André Lureau 

This workaround was added in commit 95df51a4 ("w32: Always use standard
instead of native format strings"), as it claimed glib was using
__printf__ attribute. This is surprising, since glib has always used
G_GNUC_PRINTF which, as the name implies, uses __gnu_printf__ when
possible.

Apparently, the workaound is no longer relevant though, I don't see
the warnings.

Signed-off-by: Marc-André Lureau 
---
 include/qemu/compiler.h | 8 
 1 file changed, 8 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 2704c314dcac..eb29b72c14d7 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -73,14 +73,6 @@
 #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \
sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
 
-#if !defined(__clang__) && defined(_WIN32)
-/*
- * Map __printf__ to __gnu_printf__ because we want standard format strings 
even
- * when MinGW or GLib include files use __printf__.
- */
-# define __printf__ __gnu_printf__
-#endif
-
 #ifndef __has_warning
 #define __has_warning(x) 0 /* compatibility with non-clang compilers */
 #endif
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 02/12] compiler.h: replace QEMU_SENTINEL with G_GNUC_NULL_TERMINATED

[marcandre . lureau]

From: Marc-André Lureau 

One less qemu-specific macro. It also helps to make some headers/units
only depend on glib, and thus moved in standalone projects eventually.

Signed-off-by: Marc-André Lureau 
---
 include/qemu/compiler.h| 2 --
 include/qom/object.h   | 6 +++---
 scripts/cocci-macro-file.h | 2 +-
 scripts/checkpatch.pl  | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 8385e477c18e..0a5e67fb970e 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -19,8 +19,6 @@
 
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 
-#define QEMU_SENTINEL __attribute__((sentinel))
-
 #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
 # define QEMU_PACKED __attribute__((gcc_struct, packed))
 #else
diff --git a/include/qom/object.h b/include/qom/object.h
index fae096f51cce..5f3d5b5bf532 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -616,7 +616,7 @@ Object *object_new_with_props(const char *typename,
   Object *parent,
   const char *id,
   Error **errp,
-  ...) QEMU_SENTINEL;
+  ...) G_GNUC_NULL_TERMINATED;
 
 /**
  * object_new_with_propv:
@@ -676,7 +676,7 @@ void object_apply_compat_props(Object *obj);
  *
  * Returns: %true on success, %false on error.
  */
-bool object_set_props(Object *obj, Error **errp, ...) QEMU_SENTINEL;
+bool object_set_props(Object *obj, Error **errp, ...) G_GNUC_NULL_TERMINATED;
 
 /**
  * object_set_propv:
@@ -728,7 +728,7 @@ void object_initialize(void *obj, size_t size, const char 
*typename);
 bool object_initialize_child_with_props(Object *parentobj,
  const char *propname,
  void *childobj, size_t size, const char *type,
- Error **errp, ...) QEMU_SENTINEL;
+ Error **errp, ...) G_GNUC_NULL_TERMINATED;
 
 /**
  * object_initialize_child_with_propsv:
diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
index 9daec24d7825..3d1e9b50919a 100644
--- a/scripts/cocci-macro-file.h
+++ b/scripts/cocci-macro-file.h
@@ -21,7 +21,7 @@
 /* From qemu/compiler.h */
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 #define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
-#define QEMU_SENTINEL __attribute__((sentinel))
+#define G_GNUC_NULL_TERMINATED __attribute__((sentinel))
 
 #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
 # define QEMU_PACKED __attribute__((gcc_struct, packed))
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 797738a8e839..ddc6003de280 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -225,7 +225,7 @@ our $Attribute  = qr{
volatile|
QEMU_NORETURN|
G_GNUC_WARN_UNUSED_RESULT|
-   QEMU_SENTINEL|
+   G_GNUC_NULL_TERMINATED|
QEMU_PACKED|
G_GNUC_PRINTF
  }x;
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 00/12] Misc cleanups

[marcandre . lureau]

From: Marc-André Lureau 

Hi,

A small collection of patches gleaned while working on different things (more
specifically, I am looking at making qapi code usable outside of qemu, many
things are twisted together, it takes effort but it is hopefully worth it)

Based-on: <20220222194008.610377-1-marcandre.lur...@redhat.com>

Marc-André Lureau (12):
  compiler.h: replace QEMU_WARN_UNUSED_RESULT with
G_GNUC_WARN_UNUSED_RESULT
  compiler.h: replace QEMU_SENTINEL with G_GNUC_NULL_TERMINATED
  osdep.h: move qemu_build_not_reached()
  compiler.h: replace QEMU_NORETURN with G_NORETURN
  compiler.h: drop __printf__ macro MinGW/glib workaround
  Replace config-time define HOST_WORDS_BIGENDIAN
  Simplify HOST_LONG_BITS
  Move HOST_LONG_BITS to compiler.h
  scripts/modinfo-collect: remove unused/dead code
  util: remove needless includes
  util: remove the net/net.h dependency
  qapi: remove needless include

 meson.build |  1 -
 accel/tcg/atomic_template.h |  4 +-
 accel/tcg/internal.h|  3 +-
 audio/audio.h   |  2 +-
 hw/display/pl110_template.h |  6 +--
 hw/net/can/ctucan_core.h|  2 +-
 hw/net/vmxnet3.h|  4 +-
 include/exec/cpu-all.h  |  4 +-
 include/exec/cpu-common.h   |  2 +-
 include/exec/exec-all.h | 20 -
 include/exec/helper-head.h  |  2 +-
 include/exec/memop.h|  2 +-
 include/exec/memory.h   |  2 +-
 include/fpu/softfloat-types.h   |  2 +-
 include/glib-compat.h   |  4 ++
 include/hw/core/cpu.h   |  4 +-
 include/hw/core/tcg-cpu-ops.h   |  6 +--
 include/hw/hw.h |  2 +-
 include/hw/i386/intel_iommu.h   |  6 +--
 include/hw/i386/x86-iommu.h |  4 +-
 include/hw/virtio/virtio-access.h   |  6 +--
 include/hw/virtio/virtio-gpu-bswap.h|  2 +-
 include/libdecnumber/dconfig.h  |  2 +-
 include/net/eth.h   |  2 +-
 include/qemu-common.h   |  3 +-
 include/qemu/bswap.h|  8 ++--
 include/qemu/compiler.h | 35 +++-
 include/qemu/host-utils.h   |  2 +-
 include/qemu/int128.h   |  2 +-
 include/qemu/osdep.h| 25 +++-
 include/qemu/range.h|  4 +-
 include/qemu/thread.h   |  2 +-
 include/qom/object.h|  6 +--
 include/tcg/tcg-ldst.h  |  4 +-
 include/tcg/tcg.h   |  2 +-
 include/ui/qemu-pixman.h|  2 +-
 linux-user/user-internals.h |  2 +-
 net/util.h  |  2 +-
 scripts/cocci-macro-file.h  |  6 +--
 target/alpha/cpu.h  | 10 ++---
 target/arm/cpu.h|  8 ++--
 target/arm/internals.h  | 12 +++---
 target/arm/translate-a64.h  |  2 +-
 target/arm/vec_internal.h   |  2 +-
 target/hppa/cpu.h   |  2 +-
 target/i386/cpu.h   |  2 +-
 target/i386/tcg/helper-tcg.h| 24 +--
 target/microblaze/cpu.h |  6 +--
 target/mips/cpu.h   |  2 +-
 target/mips/tcg/tcg-internal.h  | 16 
 target/nios2/cpu.h  |  6 +--
 target/openrisc/exception.h |  2 +-
 target/ppc/cpu.h| 16 
 target/ppc/internal.h   |  6 +--
 target/riscv/cpu.h  | 10 ++---
 target/s390x/s390x-internal.h   |  6 +--
 target/s390x/tcg/tcg_s390x.h| 12 +++---
 target/s390x/tcg/vec.h  |  2 +-
 target/sh4/cpu.h|  6 +--
 target/sparc/cpu.h  | 10 ++---
 target/xtensa/cpu.h |  4 +-
 tests/fp/platform.h |  4 +-
 accel/kvm/kvm-all.c |  4 +-
 accel/stubs/tcg-stub.c  |  4 +-
 audio/dbusaudio.c   |  2 +-
 block/qcow2-refcount.c  | 20 -
 bsd-user/signal.c   |  2 +-
 disas.c |  2 +-
 hw/core/loader.c|  4 +-
 hw/display/artist.c |  6 +--
 hw/display/pxa2xx_lcd.c |  2 +-
 hw/display/vga.c| 12 +++---
 hw/display/virtio-gpu-gl.c  |  2 +-
 hw/misc/mips_itu.c  |  2 +-
 hw/s390x/event-facility.c   |  2 +-
 hw/virtio/vhost.c   |  2 +-
 linux-user/arm/nwfpe/double_cpdo.c  |  4 +-
 linux-user/arm/nwfpe/fpa11_cpdt.c   |  4 +-
 linux-user/ppc/signal.c |  3 +-
 linux-user/signal.c |  2 +-
 linux-user/syscall.c|  6 +--
 monitor/hmp.c 

[PATCH 01/12] compiler.h: replace QEMU_WARN_UNUSED_RESULT with G_GNUC_WARN_UNUSED_RESULT

[marcandre . lureau]

From: Marc-André Lureau 

One less qemu-specific macro. It also helps to make some headers/units
only depend on glib, and thus moved in standalone projects eventually.

Signed-off-by: Marc-André Lureau 
---
 include/qemu-common.h  |  2 +-
 include/qemu/compiler.h|  2 --
 include/qemu/range.h   |  4 ++--
 scripts/cocci-macro-file.h |  2 +-
 block/qcow2-refcount.c | 20 +++-
 scripts/checkpatch.pl  |  2 +-
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 6969f957b7c3..79977cb3ec43 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -29,7 +29,7 @@ int qemu_main(int argc, char **argv, char **envp);
 void *qemu_oom_check(void *ptr);
 
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
-QEMU_WARN_UNUSED_RESULT;
+G_GNUC_WARN_UNUSED_RESULT;
 
 #ifndef _WIN32
 int qemu_pipe(int pipefd[2]);
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index f2bd050e3b9a..8385e477c18e 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -19,8 +19,6 @@
 
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 
-#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
-
 #define QEMU_SENTINEL __attribute__((sentinel))
 
 #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
diff --git a/include/qemu/range.h b/include/qemu/range.h
index f62b363e0d12..7e2b1cc447af 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -114,8 +114,8 @@ static inline uint64_t range_upb(Range *range)
  * @size may be 0. If the range would overflow, returns -ERANGE, otherwise
  * 0.
  */
-static inline int QEMU_WARN_UNUSED_RESULT range_init(Range *range, uint64_t 
lob,
- uint64_t size)
+G_GNUC_WARN_UNUSED_RESULT
+static inline int range_init(Range *range, uint64_t lob, uint64_t size)
 {
 if (lob + size < lob) {
 return -ERANGE;
diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
index c2fcea8e77a2..9daec24d7825 100644
--- a/scripts/cocci-macro-file.h
+++ b/scripts/cocci-macro-file.h
@@ -20,7 +20,7 @@
 
 /* From qemu/compiler.h */
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
-#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
+#define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
 #define QEMU_SENTINEL __attribute__((sentinel))
 
 #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 461457225274..4c7bf5c2b5db 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -32,9 +32,11 @@
 
 static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size,
 uint64_t max);
-static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
-int64_t offset, int64_t length, uint64_t addend,
-bool decrease, enum qcow2_discard_type type);
+
+G_GNUC_WARN_UNUSED_RESULT
+static int update_refcount(BlockDriverState *bs,
+   int64_t offset, int64_t length, uint64_t addend,
+   bool decrease, enum qcow2_discard_type type);
 
 static uint64_t get_refcount_ro0(const void *refcount_array, uint64_t index);
 static uint64_t get_refcount_ro1(const void *refcount_array, uint64_t index);
@@ -802,12 +804,12 @@ found:
 /* XXX: cache several refcount block clusters ? */
 /* @addend is the absolute value of the addend; if @decrease is set, @addend
  * will be subtracted from the current refcount, otherwise it will be added */
-static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
-   int64_t offset,
-   int64_t length,
-   uint64_t addend,
-   bool decrease,
-   enum qcow2_discard_type 
type)
+static int update_refcount(BlockDriverState *bs,
+   int64_t offset,
+   int64_t length,
+   uint64_t addend,
+   bool decrease,
+   enum qcow2_discard_type type)
 {
 BDRVQcow2State *s = bs->opaque;
 int64_t start, last, cluster_offset;
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a07f0effb540..797738a8e839 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -224,7 +224,7 @@ our $Attribute  = qr{
const|
volatile|
QEMU_NORETURN|
-   QEMU_WARN_UNUSED_RESULT|
+   G_GNUC_WARN_UNUSED_RESULT|
QEMU_SENTINEL|
QEMU_PACKED|
G_GNUC_PRINTF
-- 

Re: [PATCH v3 4/6] i386/pc: relocate 4g start to 1T where applicable

[Michael S. Tsirkin]

On Thu, Feb 24, 2022 at 05:54:58PM +, Joao Martins wrote:
> On 2/24/22 17:23, Michael S. Tsirkin wrote:
> > On Thu, Feb 24, 2022 at 04:07:22PM +, Joao Martins wrote:
> >> On 2/23/22 23:35, Joao Martins wrote:
> >>> On 2/23/22 21:22, Michael S. Tsirkin wrote:
> > +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
> > +  uint64_t pci_hole64_size)
> > +{
> > +X86MachineState *x86ms = X86_MACHINE(pcms);
> > +uint32_t eax, vendor[3];
> > +
> > +host_cpuid(0x0, 0, , [0], [2], [1]);
> > +if (!IS_AMD_VENDOR(vendor)) {
> > +return;
> > +}
> 
>  Wait a sec, should this actually be tying things to the host CPU ID?
>  It's really about what we present to the guest though,
>  isn't it?
> >>>
> >>> It was the easier catch all to use cpuid without going into
> >>> Linux UAPI specifics. But it doesn't have to tie in there, it is only
> >>> for systems with an IOMMU present.
> >>>
>  Also, can't we tie this to whether the AMD IOMMU is present?
> 
> >>> I think so, I can add that. Something like a amd_iommu_exists() helper
> >>> in util/vfio-helpers.c which checks if there's any sysfs child entries
> >>> that start with ivhd in /sys/class/iommu/. Given that this HT region is
> >>> hardcoded in iommu reserved regions since >=4.11 (to latest) I don't 
> >>> think it's
> >>> even worth checking the range exists in:
> >>>
> >>>   /sys/kernel/iommu_groups/0/reserved_regions
> >>>
> >>> (Also that sysfs ABI is >= 4.11 only)
> >>
> >> Here's what I have staged in local tree, to address your comment.
> >>
> >> Naturally the first chunk is what's affected by this patch the rest is a
> >> precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to pass
> >> all the tests and what not.
> >>
> >> I am not entirely sure this is the right place to put such a helper, open
> >> to suggestions. wrt to the naming of the helper, I tried to follow the rest
> >> of the file's style.
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index a9be5d33a291..2ea4430d5dcc 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -868,10 +868,8 @@ static void 
> >> x86_update_above_4g_mem_start(PCMachineState *pcms,
> >>uint64_t pci_hole64_size)
> >>  {
> >>  X86MachineState *x86ms = X86_MACHINE(pcms);
> >> -uint32_t eax, vendor[3];
> >>
> >> -host_cpuid(0x0, 0, , [0], [2], [1]);
> >> -if (!IS_AMD_VENDOR(vendor)) {
> >> +if (!qemu_amd_iommu_is_present()) {
> >>  return;
> >>  }
> >>
> >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> >> index 7bcce3bceb0f..eb4ea071ecec 100644
> >> --- a/include/qemu/osdep.h
> >> +++ b/include/qemu/osdep.h
> >> @@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
> >>   */
> >>  size_t qemu_get_host_physmem(void);
> >>
> >> +/**
> >> + * qemu_amd_iommu_is_present:
> >> + *
> >> + * Operating system agnostic way of querying if an AMD IOMMU
> >> + * is present.
> >> + *
> >> + */
> >> +bool qemu_amd_iommu_is_present(void);
> >> +
> >>  /*
> >>   * Toggle write/execute on the pages marked MAP_JIT
> >>   * for the current thread.
> >> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> >> index f2be7321c59f..54cef21217c4 100644
> >> --- a/util/oslib-posix.c
> >> +++ b/util/oslib-posix.c
> >> @@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
> >>  #endif
> >>  return 0;
> >>  }
> >> +
> >> +bool qemu_amd_iommu_is_present(void)
> >> +{
> >> +bool found = false;
> >> +#ifdef CONFIG_LINUX
> >> +struct dirent *entry;
> >> +char *path;
> >> +DIR *dir;
> >> +
> >> +path = g_strdup_printf("/sys/class/iommu");
> >> +dir = opendir(path);
> >> +if (!dir) {
> >> +g_free(path);
> >> +return found;
> >> +}
> >> +
> >> +do {
> >> +entry = readdir(dir);
> >> +if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
> >> +found = true;
> >> +break;
> >> +}
> >> +} while (entry);
> >> +
> >> +g_free(path);
> >> +closedir(dir);
> >> +#endif
> >> +return found;
> >> +}
> > 
> > why are we checking whether an AMD IOMMU is present
> > on the host? 
> > Isn't what we care about whether it's
> > present in the VM? What we are reserving after all
> > is a range of GPAs, not actual host PA's ...
> > 
> Right.
> 
> But any DMA map done by VFIO/IOMMU using those GPA ranges will fail,
> and so we need to not map that portion of address space entirely
> and use some other portion of the GPA-space. This has to
> do with host IOMMU which is where the DMA mapping of guest PA takes
> place. So, if you do not have an host IOMMU, you can't
> service guest DMA/PCIe services via VFIO through the host IOMMU, therefore you
> don't need this problem.
> 
> Whether one uses a guest IOMMU or not does not affect the result,
> it would be more of a case of 

Re: [PATCH V7 01/29] memory: qemu_check_ram_volatile

[Dr. David Alan Gilbert]

* Steve Sistare (steven.sist...@oracle.com) wrote:
> Add a function that returns an error if any ram_list block represents
> volatile memory.
> 
> Signed-off-by: Steve Sistare 
> ---
>  include/exec/memory.h |  8 
>  softmmu/memory.c  | 26 ++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 20f1b27..137f5f3 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2981,6 +2981,14 @@ bool ram_block_discard_is_disabled(void);
>   */
>  bool ram_block_discard_is_required(void);
>  
> +/**
> + * qemu_ram_check_volatile: return 1 if any memory regions are writable and 
> not
> + * backed by shared memory, else return 0.
> + *
> + * @errp: returned error message identifying the first volatile region found.
> + */
> +int qemu_check_ram_volatile(Error **errp);
> +
>  #endif
>  
>  #endif
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 7340e19..30b2f68 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -2837,6 +2837,32 @@ void memory_global_dirty_log_stop(unsigned int flags)
>  memory_global_dirty_log_do_stop(flags);
>  }
>  
> +static int check_volatile(RAMBlock *rb, void *opaque)
> +{
> +MemoryRegion *mr = rb->mr;
> +
> +if (mr &&
> +memory_region_is_ram(mr) &&
> +!memory_region_is_ram_device(mr) &&
> +!memory_region_is_rom(mr) &&
> +(rb->fd == -1 || !qemu_ram_is_shared(rb))) {
> +*(const char **)opaque = memory_region_name(mr);
> +return -1;
> +}
> +return 0;
> +}
> +
> +int qemu_check_ram_volatile(Error **errp)
> +{
> +char *name;

Does that need to be const char *name for safety since you're casting
it to it below?

Other than that,


Reviewed-by: Dr. David Alan Gilbert 

> +
> +if (qemu_ram_foreach_block(check_volatile, )) {
> +error_setg(errp, "Memory region %s is volatile", name);
> +return -1;
> +}
> +return 0;
> +}
> +
>  static void listener_add_address_space(MemoryListener *listener,
> AddressSpace *as)
>  {
> -- 
> 1.8.3.1
> 
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH V7 04/29] migration: simplify savevm

[Dr. David Alan Gilbert]

* Steve Sistare (steven.sist...@oracle.com) wrote:
> Use qemu_file_open to simplify a few functions in savevm.c.
> No functional change.
> 
> Signed-off-by: Steve Sistare 

So I think this is mostly OK, but a couple of minor tidyups below;
so with the tidies and the renames from the previous patch:

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/savevm.c | 21 +++--
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 0bef031..c71d525 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2910,8 +2910,9 @@ bool save_snapshot(const char *name, bool overwrite, 
> const char *vmstate,
>  void qmp_xen_save_devices_state(const char *filename, bool has_live, bool 
> live,
>  Error **errp)
>  {
> +const char *ioc_name = "migration-xen-save-state";
> +int flags = O_WRONLY | O_CREAT | O_TRUNC;

I don't see why to take these (or the matching ones in load) as separate
variables; just keep them as is, and be parameters.

>  QEMUFile *f;
> -QIOChannelFile *ioc;
>  int saved_vm_running;
>  int ret;
>  
> @@ -2925,14 +2926,10 @@ void qmp_xen_save_devices_state(const char *filename, 
> bool has_live, bool live,
>  vm_stop(RUN_STATE_SAVE_VM);
>  global_state_store_running();
>  
> -ioc = qio_channel_file_new_path(filename, O_WRONLY | O_CREAT | O_TRUNC,
> -0660, errp);
> -if (!ioc) {
> +f = qemu_file_open(filename, flags, 0660, ioc_name, errp);
> +if (!f) {
>  goto the_end;
>  }
> -qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
> -f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
> -object_unref(OBJECT(ioc));
>  ret = qemu_save_device_state(f);
>  if (ret < 0 || qemu_fclose(f) < 0) {
>  error_setg(errp, QERR_IO_ERROR);
> @@ -2960,8 +2957,8 @@ void qmp_xen_save_devices_state(const char *filename, 
> bool has_live, bool live,
>  
>  void qmp_xen_load_devices_state(const char *filename, Error **errp)
>  {
> +const char *ioc_name = "migration-xen-load-state";
>  QEMUFile *f;
> -QIOChannelFile *ioc;
>  int ret;
>  
>  /* Guest must be paused before loading the device state; the RAM state
> @@ -2973,14 +2970,10 @@ void qmp_xen_load_devices_state(const char *filename, 
> Error **errp)
>  }
>  vm_stop(RUN_STATE_RESTORE_VM);
>  
> -ioc = qio_channel_file_new_path(filename, O_RDONLY | O_BINARY, 0, errp);
> -if (!ioc) {
> +f = qemu_file_open(filename, O_RDONLY | O_BINARY, 0, ioc_name, errp);
> +if (!f) {
>  return;
>  }
> -qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
> -f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
> -object_unref(OBJECT(ioc));
> -
>  ret = qemu_loadvm_state(f);
>  qemu_fclose(f);
>  if (ret < 0) {
> -- 
> 1.8.3.1
> 
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH V7 03/29] migration: qemu file wrappers

[Dr. David Alan Gilbert]

* Steve Sistare (steven.sist...@oracle.com) wrote:
> Add qemu_file_open and qemu_fd_open to create QEMUFile objects for unix
> files and file descriptors.
> 
> Signed-off-by: Steve Sistare 
> ---
>  migration/qemu-file-channel.c | 36 
>  migration/qemu-file-channel.h |  6 ++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index bb5a575..afb16d7 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -27,8 +27,10 @@
>  #include "qemu-file.h"
>  #include "io/channel-socket.h"
>  #include "io/channel-tls.h"
> +#include "io/channel-file.h"
>  #include "qemu/iov.h"
>  #include "qemu/yank.h"
> +#include "qapi/error.h"
>  #include "yank_functions.h"
>  
>  
> @@ -192,3 +194,37 @@ QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
>  object_ref(OBJECT(ioc));
>  return qemu_fopen_ops(ioc, _output_ops, true);
>  }
> +
> +QEMUFile *qemu_file_open(const char *path, int flags, int mode,
> + const char *name, Error **errp)

Can you please make that qemu_fopen_file

> +{
> +g_autoptr(QIOChannelFile) fioc = NULL;
> +QIOChannel *ioc;
> +QEMUFile *f;
> +
> +if (flags & O_RDWR) {
> +error_setg(errp, "qemu_file_open %s: O_RDWR not supported", path);
> +return NULL;
> +}
> +
> +fioc = qio_channel_file_new_path(path, flags, mode, errp);
> +if (!fioc) {
> +return NULL;
> +}
> +
> +ioc = QIO_CHANNEL(fioc);
> +qio_channel_set_name(ioc, name);
> +f = (flags & O_WRONLY) ? qemu_fopen_channel_output(ioc) :
> + qemu_fopen_channel_input(ioc);
> +return f;
> +}
> +
> +QEMUFile *qemu_fd_open(int fd, bool writable, const char *name)
> +{

Can you please make that qemu_fopen_fd

> +g_autoptr(QIOChannelFile) fioc = qio_channel_file_new_fd(fd);

Can you use qio_channel_new_fd for that? Then it creates either
a socket or file subclass depending what type of fd is passed
(and gives you a QIOChannel without needing to cast).

> +QIOChannel *ioc = QIO_CHANNEL(fioc);
> +QEMUFile *f = writable ? qemu_fopen_channel_output(ioc) :
> + qemu_fopen_channel_input(ioc);
> +qio_channel_set_name(ioc, name);
> +return f;
> +}
> diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
> index 0028a09..324ae2d 100644
> --- a/migration/qemu-file-channel.h
> +++ b/migration/qemu-file-channel.h
> @@ -29,4 +29,10 @@
>  
>  QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
>  QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
> +
> +QEMUFile *qemu_file_open(const char *path, int flags, int mode,
> + const char *name, Error **errp);
> +
> +QEMUFile *qemu_fd_open(int fd, bool writable, const char *name);
> +
>  #endif
> -- 
> 1.8.3.1
> 
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH] target/arm: Report KVM's actual PSCI version to guest in dtb

[Richard Henderson]

On 2/24/22 03:46, Peter Maydell wrote:

When we're using KVM, the PSCI implementation is provided by the
kernel, but QEMU has to tell the guest about it via the device tree.
Currently we look at the KVM_CAP_ARM_PSCI_0_2 capability to determine
if the kernel is providing at least PSCI 0.2, but if the kernel
provides a newer version than that we will still only tell the guest
it has PSCI 0.2.  (This is fairly harmless; it just means the guest
won't use newer parts of the PSCI API.)

The kernel exposes the specific PSCI version it is implementing via
the ONE_REG API; use this to report in the dtb that the PSCI
implementation is 1.0-compatible if appropriate.  (The device tree
binding currently only distinguishes "pre-0.2", "0.2-compatible" and
"1.0-compatible".)

Signed-off-by: Peter Maydell
---
Based-on:20220213035753.34577-1-akihiko.od...@gmail.com
("[PATCH v2] target/arm: Support PSCI 1.1 and SMCCC 1.0")
though note that to compile on arm hosts you'll need the
bugfix to that patch from which I describe in a reply to it.

  target/arm/kvm-consts.h |  1 +
  hw/arm/boot.c   |  5 ++---
  target/arm/kvm64.c  | 12 
  3 files changed, 15 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~