Re: [PATCH-for-5.1 v2 0/2] stubs: Fix notify-event stub linkage error on MinGW

[Philippe Mathieu-Daudé]

On 8/18/20 8:37 AM, Paolo Bonzini wrote:
> On 05/08/20 10:55, Philippe Mathieu-Daudé wrote:
>> 2 trivial patches to fix the link error reported by Thomas:
>>
>>   LINKtests/test-timed-average.exe
>>  libqemuutil.a(main-loop.o): In function `qemu_notify_event':
>>  util/main-loop.c:139: multiple definition of `qemu_notify_event'
>>  
>> tests/test-timed-average.o:/builds/huth/qemu/tests/../stubs/notify-event.c:5:
>>  first defined here
>>  collect2: error: ld returned 1 exit status
>>  rules.mak:124: recipe for target 'tests/test-timed-average.exe' failed
>>
>> Philippe Mathieu-Daudé (2):
>>   exec: Restrict icount to softmmu
>>   stubs: Remove qemu_notify_event()
>>
>>  include/sysemu/cpus.h  | 4 
>>  exec.c | 4 
>>  softmmu/cpus.c | 7 +++
>>  stubs/cpu-get-icount.c | 2 +-
>>  stubs/notify-event.c   | 6 --
>>  stubs/Makefile.objs| 1 -
>>  6 files changed, 12 insertions(+), 12 deletions(-)
>>  delete mode 100644 stubs/notify-event.c
>>
> 
> Patch 1 will be superseded by Claudio's refactoring.  For patch 2 please
> respin according to the review.

The respin is available as <20200805100126.25583-3-phi...@redhat.com>:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg727616.html

Re: [PATCH 13/41] hvf: Add missing include

[Philippe Mathieu-Daudé]

On 8/14/20 12:25 AM, Eduardo Habkost wrote:
> The sysemu/accel.h header is needed for the ACCEL_CLASS_NAME
> macro.  This will be necessary to allow us to use OBJECT_DEFINE*()
> for TYPE_HVF_ACCEL.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/sysemu/hvf.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> index 6d3ee4fdb7..d3bed80ea8 100644
> --- a/include/sysemu/hvf.h
> +++ b/include/sysemu/hvf.h
> @@ -13,6 +13,8 @@
>  #ifndef HVF_H
>  #define HVF_H
>  
> +#include "sysemu/accel.h"
> +
>  #ifdef CONFIG_HVF
>  uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
>   int reg);
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] qemu-img: Explicit number replaced by a constant

[Stefano Garzarella]

Hi Yi Li,
thanks for this patch! Just a comment below:

On Mon, Aug 17, 2020 at 07:01:13PM +0800, Yi Li wrote:
> Signed-off-by: Yi Li 
> ---
>  qemu-img.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 5308773811..a0fbc2757c 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1181,7 +1181,7 @@ static int64_t find_nonzero(const uint8_t *buf, int64_t 
> n)
>  }
>  
>  /*
> - * Returns true iff the first sector pointed to by 'buf' contains at least
> + * Returns true if the first sector pointed to by 'buf' contains at least

This change seems unrelated, please can you put this in a separate patch?

Thanks,
Stefano

>   * a non-NUL byte.
>   *
>   * 'pnum' is set to the number of sectors (including and immediately 
> following
> @@ -1200,10 +1200,10 @@ static int is_allocated_sectors(const uint8_t *buf, 
> int n, int *pnum,
>  *pnum = 0;
>  return 0;
>  }
> -is_zero = buffer_is_zero(buf, 512);
> +is_zero = buffer_is_zero(buf, BDRV_SECTOR_SIZE);
>  for(i = 1; i < n; i++) {
> -buf += 512;
> -if (is_zero != buffer_is_zero(buf, 512)) {
> +buf += BDRV_SECTOR_SIZE;
> +if (is_zero != buffer_is_zero(buf, BDRV_SECTOR_SIZE)) {
>  break;
>  }
>  }
> @@ -2489,8 +2489,8 @@ static int img_convert(int argc, char **argv)
>  }
>  }
>  
> -qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s.total_sectors * 512,
> -&error_abort);
> +qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> +s.total_sectors * BDRV_SECTOR_SIZE, 
> &error_abort);
>  ret = add_old_style_options(out_fmt, opts, out_baseimg, NULL);
>  if (ret < 0) {
>  goto out;
> -- 
> 2.25.3
> 
> 
> 
> 

Re: hw-display-qxl.so: undefined symbol: qemu_qxl_io_log_semaphore

[Gerd Hoffmann]

  Hi,

> So nothing specific to qxl, seems all modules are busted.
> I tried downgrading gcc to f32 versions, no difference. Same after
> downgrading binutils. Downgrading glibc downgrades the whole distro so I
> didn't attempt it

> Any suggestions?

Try downgrade rpm macros?

take care,
  Gerd

Re: [PATCH v4 4/8] ppc/e500: Use start-powered-off CPUState property

[Philippe Mathieu-Daudé]

On 8/18/20 5:33 AM, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
> the start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
> 
> Also change creation of CPU object from cpu_create() to object_new() and
> qdev_realize() because cpu_create() realizes the CPU and it's not possible
> to set a property after the object is realized.
> 
> Signed-off-by: Thiago Jung Bauermann 
> ---
>  hw/ppc/e500.c | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index ab9884e315..0077aca74d 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
>  
>  cpu_reset(cs);
>  
> -/* Secondary CPU starts in halted state for now. Needs to change when
> -   implementing non-kernel boot. */
> -cs->halted = 1;
>  cs->exception_index = EXCP_HLT;
>  }
>  
> @@ -864,8 +861,9 @@ void ppce500_init(MachineState *machine)
>  PowerPCCPU *cpu;
>  CPUState *cs;
>  qemu_irq *input;
> +Error *err = NULL;
>  
> -cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> +cpu = POWERPC_CPU(object_new(machine->cpu_type));
>  env = &cpu->env;
>  cs = CPU(cpu);
>  
> @@ -897,6 +895,19 @@ void ppce500_init(MachineState *machine)
>  } else {
>  /* Secondary CPUs */
>  qemu_register_reset(ppce500_cpu_reset_sec, cpu);
> +
> +/*
> + * Secondary CPU starts in halted state for now. Needs to change
> + * when implementing non-kernel boot.
> + */
> +object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);

[*]

> +}
> +
> +if (!qdev_realize(DEVICE(cs), NULL, &err)) {
> +error_report_err(err);
> +object_unref(OBJECT(cs));
> +exit(EXIT_FAILURE);
>  }

The last 4 lines are equivalent to:

   qdev_realize(DEVICE(cs), NULL, &error_fatal)) {

This is also the preferred form, as we can not propagate errors
from the machine_init() handler.

Since you use &error_abort in [*], maybe you want to use it here too.

>  }
>  
> 

Re: [PATCH v4 5/8] mips/cps: Use start-powered-off CPUState property

[Philippe Mathieu-Daudé]

On 8/18/20 5:33 AM, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in main_cpu_reset(), use the
> start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
> 
> Also change creation of CPU object from cpu_create() to object_new() and
> qdev_realize() because cpu_create() realizes the CPU and it's not possible to
> set a property after the object is realized.
> 
> Signed-off-by: Thiago Jung Bauermann 
> ---
>  hw/mips/cps.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
> index 615e1a1ad2..be85357dc0 100644
> --- a/hw/mips/cps.c
> +++ b/hw/mips/cps.c
> @@ -52,9 +52,6 @@ static void main_cpu_reset(void *opaque)
>  CPUState *cs = CPU(cpu);
>  
>  cpu_reset(cs);
> -
> -/* All VPs are halted on reset. Leave powering up to CPC. */
> -cs->halted = 1;
>  }
>  
>  static bool cpu_mips_itu_supported(CPUMIPSState *env)
> @@ -76,7 +73,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
>  bool saar_present = false;
>  
>  for (i = 0; i < s->num_vp; i++) {
> -cpu = MIPS_CPU(cpu_create(s->cpu_type));
> +Error *err = NULL;
> +
> +cpu = MIPS_CPU(object_new(s->cpu_type));
>  
>  /* Init internal devices */
>  cpu_mips_irq_init_cpu(cpu);
> @@ -89,7 +88,16 @@ static void mips_cps_realize(DeviceState *dev, Error 
> **errp)
>  env->itc_tag = mips_itu_get_tag_region(&s->itu);
>  env->itu = &s->itu;
>  }
> +/* All VPs are halted on reset. Leave powering up to CPC. */
> +object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> + &error_abort);
>  qemu_register_reset(main_cpu_reset, cpu);
> +
> +if (!qdev_realize(DEVICE(cpu), NULL, &err)) {
> +error_report_err(err);
> +object_unref(OBJECT(cpu));
> +exit(EXIT_FAILURE);
> +}

Here errp is available, so we can propagate the error to the caller:

   if (!qdev_realize(DEVICE(cpu), NULL, errp)) {
   return;
   }

For example in hw/mips/boston.c:

object_initialize_child(OBJECT(machine), "cps", &s->cps, TYPE_MIPS_CPS);
object_property_set_str(OBJECT(&s->cps), "cpu-type", machine->cpu_type,
&error_fatal);
object_property_set_int(OBJECT(&s->cps), "num-vp", machine->smp.cpus,
&error_fatal);
sysbus_realize(SYS_BUS_DEVICE(&s->cps), &error_fatal);

This will be propagated here ---^

>  }
>  
>  cpu = MIPS_CPU(first_cpu);
> 

Re: [PATCH v4 7/8] sparc/sun4m: Use start-powered-off CPUState property

[Philippe Mathieu-Daudé]

On 8/18/20 5:33 AM, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in secondary_cpu_reset(), use the
> start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
> 
> This makes secondary_cpu_reset() unnecessary, so remove it.
> 
> Also remove setting of cs->halted from cpu_devinit(), which seems out of
> place when compared to similar code in other architectures (e.g.,
> ppce500_init() in hw/ppc/e500.c).
> 
> Finally, change creation of CPU object from cpu_create() to object_new()
> and qdev_realize() because cpu_create() realizes the CPU and it's not
> possible to set a property after the object is realized.
> 
> Suggested-by: Philippe Mathieu-Daudé 
> Signed-off-by: Thiago Jung Bauermann 
> ---
>  hw/sparc/sun4m.c | 26 ++
>  1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index f1d92df781..41a7c5fa86 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -218,15 +218,6 @@ static void dummy_cpu_set_irq(void *opaque, int irq, int 
> level)
>  {
>  }
>  
> -static void secondary_cpu_reset(void *opaque)
> -{
> -SPARCCPU *cpu = opaque;
> -CPUState *cs = CPU(cpu);
> -
> -cpu_reset(cs);
> -cs->halted = 1;
> -}
> -
>  static void cpu_halt_signal(void *opaque, int irq, int level)
>  {
>  if (level && current_cpu) {
> @@ -810,21 +801,24 @@ static const TypeInfo ram_info = {
>  static void cpu_devinit(const char *cpu_type, unsigned int id,
>  uint64_t prom_addr, qemu_irq **cpu_irqs)
>  {
> -CPUState *cs;
>  SPARCCPU *cpu;
>  CPUSPARCState *env;
> +Error *err = NULL;
>  
> -cpu = SPARC_CPU(cpu_create(cpu_type));
> +cpu = SPARC_CPU(object_new(cpu_type));
>  env = &cpu->env;
>  
>  cpu_sparc_set_id(env, id);
> -if (id != 0) {
> -qemu_register_reset(secondary_cpu_reset, cpu);
> -cs = CPU(cpu);
> -cs->halted = 1;
> -}
> +object_property_set_bool(OBJECT(cpu), "start-powered-off", id != 0,
> + &error_abort);
>  *cpu_irqs = qemu_allocate_irqs(cpu_set_irq, cpu, MAX_PILS);
>  env->prom_addr = prom_addr;
> +
> +if (!qdev_realize(DEVICE(cpu), NULL, &err)) {
> +error_report_err(err);
> +object_unref(OBJECT(cpu));
> +exit(EXIT_FAILURE);
> +}

Simply use:

   qdev_realize(DEVICE(cpu), NULL, &error_abort);

>  }
>  
>  static void dummy_fdc_tc(void *opaque, int irq, int level)
> 

Re: [PATCH v4 4/8] ppc/e500: Use start-powered-off CPUState property

[Philippe Mathieu-Daudé]

On 8/18/20 9:22 AM, Philippe Mathieu-Daudé wrote:
> On 8/18/20 5:33 AM, Thiago Jung Bauermann wrote:
>> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
>> the start-powered-off property which makes cpu_common_reset() initialize it
>> to 1 in common code.
>>
>> Also change creation of CPU object from cpu_create() to object_new() and
>> qdev_realize() because cpu_create() realizes the CPU and it's not possible
>> to set a property after the object is realized.
>>
>> Signed-off-by: Thiago Jung Bauermann 
>> ---
>>  hw/ppc/e500.c | 19 +++
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index ab9884e315..0077aca74d 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
>>  
>>  cpu_reset(cs);
>>  
>> -/* Secondary CPU starts in halted state for now. Needs to change when
>> -   implementing non-kernel boot. */
>> -cs->halted = 1;
>>  cs->exception_index = EXCP_HLT;
>>  }
>>  
>> @@ -864,8 +861,9 @@ void ppce500_init(MachineState *machine)
>>  PowerPCCPU *cpu;
>>  CPUState *cs;
>>  qemu_irq *input;
>> +Error *err = NULL;
>>  
>> -cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>> +cpu = POWERPC_CPU(object_new(machine->cpu_type));
>>  env = &cpu->env;
>>  cs = CPU(cpu);
>>  
>> @@ -897,6 +895,19 @@ void ppce500_init(MachineState *machine)
>>  } else {
>>  /* Secondary CPUs */
>>  qemu_register_reset(ppce500_cpu_reset_sec, cpu);
>> +
>> +/*
>> + * Secondary CPU starts in halted state for now. Needs to change
>> + * when implementing non-kernel boot.
>> + */
>> +object_property_set_bool(OBJECT(cs), "start-powered-off", true,
>> + &error_abort);
> 
> [*]
> 
>> +}
>> +
>> +if (!qdev_realize(DEVICE(cs), NULL, &err)) {
>> +error_report_err(err);
>> +object_unref(OBJECT(cs));
>> +exit(EXIT_FAILURE);
>>  }
> 
> The last 4 lines are equivalent to:
> 
>qdev_realize(DEVICE(cs), NULL, &error_fatal)) {

I meant:

 qdev_realize(DEVICE(cs), NULL, &error_fatal);

> 
> This is also the preferred form, as we can not propagate errors
> from the machine_init() handler.
> 
> Since you use &error_abort in [*], maybe you want to use it here too.
> 
>>  }
>>  
>>
> 

[Bug 1882851] [PATCH 1/2] drm/virtio: fix unblank

[Gerd Hoffmann]

When going through a disable/enable cycle without changing the
framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio:
skip set_scanout if framebuffer didn't change") causes the screen stay
blank.  Add a bool to force an update to fix that.

v2: use drm_atomic_crtc_needs_modeset() (Daniel).

Cc: 1882...@bugs.launchpad.net
Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't 
change")
Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/virtio/virtgpu_drv.h |  1 +
 drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++
 drivers/gpu/drm/virtio/virtgpu_plane.c   |  4 +++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 9ff9f4ac0522..4ab1b0ba2925 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -138,6 +138,7 @@ struct virtio_gpu_output {
int cur_x;
int cur_y;
bool enabled;
+   bool needs_modeset;
 };
 #define drm_crtc_to_virtio_gpu_output(x) \
container_of(x, struct virtio_gpu_output, crtc)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c 
b/drivers/gpu/drm/virtio/virtgpu_display.c
index 2c2742b8d657..6c26b41f4e0d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -123,6 +123,17 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc 
*crtc,
 static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc,
 struct drm_crtc_state *old_state)
 {
+   struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc);
+
+   /*
+* virtio-gpu can't do modeset and plane update operations
+* independant from each other.  So the actual modeset happens
+* in the plane update callback, and here we just check
+* whenever we must force the modeset.
+*/
+   if (drm_atomic_crtc_needs_modeset(crtc->state)) {
+   output->needs_modeset = true;
+   }
 }
 
 static const struct drm_crtc_helper_funcs virtio_gpu_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c 
b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 52d24179bcec..65757409d9ed 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -163,7 +163,9 @@ static void virtio_gpu_primary_plane_update(struct 
drm_plane *plane,
plane->state->src_w != old_state->src_w ||
plane->state->src_h != old_state->src_h ||
plane->state->src_x != old_state->src_x ||
-   plane->state->src_y != old_state->src_y) {
+   plane->state->src_y != old_state->src_y ||
+   output->needs_modeset) {
+   output->needs_modeset = false;
DRM_DEBUG("handle 0x%x, crtc %dx%d+%d+%d, src %dx%d+%d+%d\n",
  bo->hw_res_handle,
  plane->state->crtc_w, plane->state->crtc_h,
-- 
2.18.4

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

Title:
  QEMU video freezes with "Guest disabled display" (virtio driver)

Status in QEMU:
  New

Bug description:
  I am using Arch Linux as my Guest and Host OS, after starting qemu
  with the following command:

$ qemu-system-x86_64 -enable-kvm -hda arch-zoom.qcow2 -m 4G -vga
  virtio

  and waiting for a screen blank, I get this message:

Guest disabled display

  And nothing happens after that, I can move the mouse or hit any key,
  and the message is still there.

  I can still reboot the VM but that's not optimal.

  I can reproduce this with the latest QEMU release (5.0.0) or git master, 
  I also tried this with older releases (4.0.0, 3.0.0) and the issue is still 
there.

  I can't reproduce this with other video drivers (std, qxl).

  With std/qxl the screen will blank a bit and then continue as normal.

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

Re: [PATCH 1/2] i386/cpu: Clear FEAT_XSAVE_COMP_{LO, HI} when XSAVE is not available

[Paolo Bonzini]

On 16/07/20 10:20, Xiaoyao Li wrote:
> Per Intel SDM vol 1, 13.2, if CPUID.1:ECX.XSAVE[bit 26] is 0, the
> processor provides no further enumeration through CPUID function 0DH.
> 
> Signed-off-by: Xiaoyao Li 
> ---
>  target/i386/cpu.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1e5123251d74..f5f11603e805 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6261,6 +6261,8 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
>  uint64_t mask;
>  
>  if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
> +env->features[FEAT_XSAVE_COMP_LO] = 0;
> +env->features[FEAT_XSAVE_COMP_HI] = 0;
>  return;
>  }
>  
> 

Queued this patch, noting in the commit message that it affects "-cpu
host,-xsave".

Paolo

Re: [PATCH-for-5.2] memory: Add trace events to audit MemoryRegionOps fields

[Philippe Mathieu-Daudé]

On 8/18/20 8:32 AM, Paolo Bonzini wrote:
> On 06/08/20 17:26, Philippe Mathieu-Daudé wrote:
>> Add trace events to audit MemoryRegionOps field such:
>>  - are all the valid/impl fields provided?
>>  - is the region a power of two?
>>
>> These cases are accepted, but it is interesting to list them.
>>
>> Example:
>>
>>   $ qemu-system-i386 -S -trace memory_region_io_check\*
>>   memory_region_io_check_odd_size mr name:'dma-page' size:0x3

(a)

>>   memory_region_io_check_access_size_incomplete mr name:'acpi-tmr' 
>> min/max:[valid:1/4 impl:4/0]
>>   memory_region_io_check_access_size_incomplete mr name:'acpi-evt' 
>> min/max:[valid:1/2 impl:2/0]
>>   memory_region_io_check_access_size_incomplete mr name:'acpi-cnt' 
>> min/max:[valid:1/2 impl:2/0]

(b)

> 
> Can they be detected using Coccinelle instead?

For static declarations, probably.

(a) is not really fixable (because some datasheets don't
count the reserved space in the device address map [1]),
but is interesting to audit.

I believe (b) has to be updated per maintainers preference,
not by an individual developer. IIUC Michael said [2] while
there is no bus information in MemoryRegionOps (and way to
report a bus specific error), it is pointless to blindly fill
the zero access sizes.

Meanwhile I prefer to share my debugging helpers as trace
events instead of ./configure --enable-maintainer and #ifdef'ry.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg431171.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg709171.html

> 
> Paolo
> 
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> Based-on: <20200805130221.24487-1-phi...@redhat.com>
>>   "softmmu: Add missing trace-events file"
>> ---
>>  softmmu/memory.c | 11 +++
>>  softmmu/trace-events |  2 ++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index d030eb6f7c..daa0daf2a8 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -1488,6 +1488,17 @@ void memory_region_init_io(MemoryRegion *mr,
>>  mr->ops = ops ? ops : &unassigned_mem_ops;
>>  mr->opaque = opaque;
>>  mr->terminates = true;
>> +if (size != UINT64_MAX && !is_power_of_2(size)) {
>> +trace_memory_region_io_check_odd_size(name, size);
>> +}
>> +if (ops && (!ops->impl.min_access_size || !ops->impl.max_access_size ||
>> +!ops->valid.min_access_size || 
>> !ops->valid.max_access_size)) {
>> +trace_memory_region_io_check_access_size_incomplete(name,
>> +mr->ops->valid.min_access_size,
>> +mr->ops->valid.max_access_size,
>> +mr->ops->impl.min_access_size,
>> +mr->ops->impl.max_access_size);
>> +}
>>  }
>>  
>>  void memory_region_init_ram_nomigrate(MemoryRegion *mr,
>> diff --git a/softmmu/trace-events b/softmmu/trace-events
>> index b80ca042e1..00eb316aef 100644
>> --- a/softmmu/trace-events
>> +++ b/softmmu/trace-events
>> @@ -18,6 +18,8 @@ memory_region_ram_device_write(int cpu_index, void *mr, 
>> uint64_t addr, uint64_t
>>  flatview_new(void *view, void *root) "%p (root %p)"
>>  flatview_destroy(void *view, void *root) "%p (root %p)"
>>  flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
>> +memory_region_io_check_odd_size(const char *name, uint64_t size) "mr 
>> name:'%s' size:0x%"PRIx64
>> +memory_region_io_check_access_size_incomplete(const char *name, unsigned 
>> vmin, unsigned vmax, unsigned imin, unsigned imax) "mr name:'%s' 
>> min/max:[valid:%u/%u impl:%u/%u]"
>>  
>>  # vl.c
>>  vm_state_notify(int running, int reason, const char *reason_str) "running 
>> %d reason %d (%s)"
>>
> 

[Bug 1890545] Re: (ARM64) qemu-x86_64+schroot(Debian bullseye) can't run chrome and can't load HTML

[Tony.LI]

I wrote an example to  load local HTML:

#include "mainwindow.h"
#include "ui_mainwindow.h"
#include 
MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);

QWebEngineView *webView = new QWebEngineView(this);

webView->load(QUrl("file:home/tony/1.html"));
webView->setFixedSize(this->width(),this->height());
webView->show();
}

MainWindow::~MainWindow()
{
delete ui;
}


At the same time, I found that a process(QtWebEngineProcess) did not start 
properly;
Then,I run:

$ ./QtWebEngineProcess --type=zygote --webengine-schemes=qrc:sLV --lang=zh  
 
qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
 
But,I didn't find any mistakes.Why does the process exit?

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

Title:
  (ARM64) qemu-x86_64+schroot(Debian bullseye) can't run chrome and
  can't load HTML

Status in QEMU:
  New

Bug description:
  First I creat a file system that is debian(bullseye amd64)on arm64
  machine，then I download google-chrome，however, when I ran Google
  browser, some errors occurred.

  $ google-chrome --no-sandbox
  or 
  $ qemu-x86_64-static google-chrome --no-sandbox

  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  [1661:1661:0806/074307.502638:ERROR:nacl_fork_delegate_linux.cc(323)] Bad 
NaCl helper startup ack (0 bytes)
  [1664:1664:0806/074307.504159:ERROR:nacl_fork_delegate_linux.cc(323)] Bad 
NaCl helper startup ack (0 bytes)
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  [1637:1678:0806/074308.337567:ERROR:file_path_watcher_linux.cc(315)] 
inotify_init() failed: Function not implemented (38)
  Fontconfig warning: "/etc/fonts/fonts.conf", line 100: unknown element "blank"
  qemu: unknown option 'type=utility'
  [1637:1680:0806/074313.598432:FATAL:gpu_data_manager_impl_private.cc(439)] 
GPU process isn't usable. Goodbye.
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  Trace/breakpoint trap

  Why?
  And then I run firefox,it can be opened, but it can't load any web pages and 
HTML.
  I really need help！
  Thank.

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

Re: [PATCH] memory: Directly dispatch alias accesses on origin memory region

[Philippe Mathieu-Daudé]

On 8/17/20 6:27 PM, Paolo Bonzini wrote:
> On 16/08/20 19:30, Philippe Mathieu-Daudé wrote:
>> There is an issue when accessing an alias memory region via the
>> memory_region_dispatch_read() / memory_region_dispatch_write()
>> calls:
>>
>> The memory_region_init_alias() flow is:
>>
>>   memory_region_init_alias()
>>   -> memory_region_init()
>>  -> object_initialize(TYPE_MEMORY_REGION)
>> -> memory_region_initfn()
>>-> mr->ops = &unassigned_mem_ops;
>>
>> Later when accessing the alias, the memory_region_dispatch_read()
>> flow is:
>>
>>   memory_region_dispatch_read()
>>   -> memory_region_access_valid(mr)
>>  -> mr->ops->valid.accepts()
>> -> unassigned_mem_accepts()
>> <- false
>>  <- false
>><- MEMTX_DECODE_ERROR
>>
>> The caller gets a MEMTX_DECODE_ERROR while the access is OK.
> 
> What is the path that leads to this call?

Using the interleaver from:
https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg03680.html

#2  0x8162f7b6 in unassigned_mem_read (opaque=0x82ac7330, addr=0,
size=1) at softmmu/memory.c:1261
#3  0x8162fc2f in memory_region_dispatch_read (mr=0x82ac7330, addr=0,
pval=0x7ffe24b9cc08, op=MO_8, attrs=...) at softmmu/memory.c:1417
#4  0x8175488b in interleaver_read (opaque=0x82b9e530, offset=0,
data=0x7ffe24b9cc08, size=1, attrs=...) at hw/misc/interleaver.c:76
#5  0x8162cbdc in memory_region_read_with_attrs_accessor (mr=0x82b9e850,
addr=0, value=0x7ffe24b9cd90, size=1, shift=0, mask=255, attrs=...) at
softmmu/memory.c:456
#6  0x8162cfab in access_with_adjusted_size (addr=0,
value=0x7ffe24b9cd90, size=4, access_size_min=1, access_size_max=1,
access_fn=
0x8162cb7c , mr=0x82b9e850,
attrs=...) at softmmu/memory.c:544
#7  0x8162fb98 in memory_region_dispatch_read1 (mr=0x82b9e850, addr=0,
pval=0x7ffe24b9cd90, size=4, attrs=...) at softmmu/memory.c:1395
#8  0x8162fc5a in memory_region_dispatch_read (mr=0x82b9e850, addr=0,
pval=0x7ffe24b9cd90, op=MO_32, attrs=...) at softmmu/memory.c:1421
#9  0x8153012b in flatview_read_continue (fv=0x82bd0d10, addr=320897024,
attrs=..., ptr=0x7ffe24b9cea0, len=4, addr1=0, l=4, mr=0x82b9e850) at
exec.c:3239
#10 0x8153027e in flatview_read (fv=0x82bd0d10, addr=320897024,
attrs=..., buf=0x7ffe24b9cea0, len=4) at exec.c:3278
#11 0x81530307 in address_space_read_full (as=0x81ec1ac0
, addr=320897024, attrs=..., buf=0x7ffe24b9cea0,
len=4) at exec.c:3291
#12 0x8163761e in address_space_read (len=4, buf=0x7ffe24b9cea0,
attrs=..., addr=320897024, as=0x81ec1ac0 ) at
include/exec/memory.h:2420
#13 qtest_process_command (chr=0x81edcd00 , words=0x82be2b30)
at softmmu/qtest.c:495
#14 0x8163877b in qtest_process_inbuf (chr=0x81edcd00 ,
inbuf=0x82a65220) at softmmu/qtest.c:724
#15 0x8163880c in qtest_read (opaque=0x81edcd00 ,
buf=0x7ffe24b9d1f0 "readl 0x13208000\n 0x76\n", size=17) at
softmmu/qtest.c:736

> 
>> Fix by directly dispatching aliases accesses to its origin region.
>>
>> Fixes: 2cdfcf272d ("memory: assign MemoryRegionOps to all regions")
> 
> I don't think the "Fixes" is okay because you'd have gotten a different
> bug before.

OK I'll reword.

> 
>> +if (mr->alias) {
>> +addr += mr->alias_offset;
>> +mr = mr->alias;
>> +}
> 
> Also, I think this would have to be a while loop.

I haven't thought about this case! I'll add a test for it :)

> 
> Paolo
> 
> 

[Bug 1890545] Re: (ARM64) qemu-x86_64+schroot(Debian bullseye) can't run chrome and can't load HTML

[Tony.LI]

I wrote an example to load local HTML:

#include "mainwindow.h"
#include "ui_mainwindow.h"
#include 
MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);

QWebEngineView *webView = new QWebEngineView(this);

webView->load(QUrl("file:home/tony/1.html"));
webView->setFixedSize(this->width(),this->height());
webView->show();
}

MainWindow::~MainWindow()
{
delete ui;
}

At the same time, I found that a process(QtWebEngineProcess) did not start 
properly;
Then,I run:

$ ./QtWebEngineProcess --type=zygote --webengine-schemes=qrc:sLV 
qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped

But,I didn't find any mistakes.Why does the process exit?

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

Title:
  (ARM64) qemu-x86_64+schroot(Debian bullseye) can't run chrome and
  can't load HTML

Status in QEMU:
  New

Bug description:
  First I creat a file system that is debian(bullseye amd64)on arm64
  machine，then I download google-chrome，however, when I ran Google
  browser, some errors occurred.

  $ google-chrome --no-sandbox
  or 
  $ qemu-x86_64-static google-chrome --no-sandbox

  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  [1661:1661:0806/074307.502638:ERROR:nacl_fork_delegate_linux.cc(323)] Bad 
NaCl helper startup ack (0 bytes)
  [1664:1664:0806/074307.504159:ERROR:nacl_fork_delegate_linux.cc(323)] Bad 
NaCl helper startup ack (0 bytes)
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  [1637:1678:0806/074308.337567:ERROR:file_path_watcher_linux.cc(315)] 
inotify_init() failed: Function not implemented (38)
  Fontconfig warning: "/etc/fonts/fonts.conf", line 100: unknown element "blank"
  qemu: unknown option 'type=utility'
  [1637:1680:0806/074313.598432:FATAL:gpu_data_manager_impl_private.cc(439)] 
GPU process isn't usable. Goodbye.
  qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped
  Trace/breakpoint trap

  Why?
  And then I run firefox,it can be opened, but it can't load any web pages and 
HTML.
  I really need help！
  Thank.

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

Re: [PATCH 0/1] qcow2: Skip copy-on-write when allocating a zero cluster

[Kevin Wolf]

Am 17.08.2020 um 20:18 hat Alberto Garcia geschrieben:
> On Mon 17 Aug 2020 05:53:07 PM CEST, Kevin Wolf wrote:
> > Maybe the difference is in allocating 64k at once instead of doing a
> > separate allocation for every 4k block? But with the extent size hint
> > patches to file-posix, we should allocate 1 MB at once by default now
> > (if your test image was newly created). Can you check whether this is
> > in effect for your image file?
> 
> Ehmm... is that hint supported in ext4 or only in xfs?

Hm, I had understood that ext4 supports this, but looking at the kernel
code, it doesn't look like it. :-(

Kevin

Re: [PATCH] hw: dev-wacom: Support wacom tablet emulation in linux qemu

[Michael Nazzareno Trimarchi]

Hi

On Tue, Aug 18, 2020 at 8:02 AM Gerd Hoffmann  wrote:
>
> On Mon, Aug 17, 2020 at 06:42:02PM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi Gerd
> >
> > Have another small question. Do you know how force show cursor working
> > in this case?
>
> Which display and which vga do you use?

/usr/bin/qemu-system-i386 -M pc \
-enable-kvm -cpu host \
-m 1024 \
-vga std \
-global VGA.vgamem_mb=4 \
-kernel out/w_and_h_emulator/images/bzImage \
-drive file=out/w_and_h_emulator/images/rootfs.ext2,if=virtio,format=raw \
-append "root=/dev/vda console=" \
-net nic,model=virtio \
-net 
user,hostfwd=tcp::2-:22,hostfwd=tcp::1-:1,hostfwd=tcp::10001-:10001
\
-usb -device usb-wacom-tablet \
-usb -device usb-host,vendorid=0x04d8,productid=0x000a -show-cursor

But I need to use the framebuffer. So what happens for instance in
software like ts_calibrate you can not see the cursor

Michael
>
> take care,
>   Gerd
>


-- 
Michael Nazzareno Trimarchi
Amarula Solutions BV
COO Co-Founder
Cruquiuskade 47 Amsterdam 1018 AM NL
T. +31(0)851119172
M. +39(0)3479132170
[`as] https://www.amarulasolutions.com

Re: [RFC PATCH 06/22] qemu-nbd: Use raw block driver for --offset

[Kevin Wolf]

Am 17.08.2020 um 19:19 hat Nir Soffer geschrieben:
> On Thu, Aug 13, 2020 at 7:36 PM Kevin Wolf  wrote:
> 
> > Instead of implementing qemu-nbd --offset in the NBD code, just put a
> > raw block node with the requested offset on top of the user image and
> > rely on that doing the job.
> >
> > This does not only simplify the nbd_export_new() interface and bring it
> > closer to the set of options that the nbd-server-add QMP command offers,
> > but in fact it also eliminates a potential source for bugs in the NBD
> > code which previously had to add the offset manually in all relevant
> > places.
> >
> 
> Just to make sure I understand this correctly -
> 
> qemu-nbd can work with:
> 
> $ qemu-nbd 'json:{"driver": "file", "filename": "test.raw"}'
> 
> And:
> 
> $ qemu-nbd 'json:{"driver": "raw", "file": {"driver": "file",
> "filename": "test.raw"}}'
> 
> I assumed that we always create the raw node?

No, the first form creates only the 'file' node without a 'raw' node on
top. For all practical matters, this should be the same in qemu-img or
qemu-nbd. For actually running VMs, omitting the 'raw' node where it's
not needed can improve performance a little.

What is true is that if you use a filename without specifying the driver
(i.e.  you rely on format probing), you'll get a 'raw' node on top of
the 'file' node.

> oVirt always uses the second form to make it easier to support offset,
> size, and backing.
> https://github.com/oVirt/ovirt-imageio/blob/2021164d064227d7c5e03c8da087adc66e3a577e/daemon/ovirt_imageio/_internal/qemu_nbd.py#L104
> 
> This also seems to be the way libvirt builds the nodes using -blockdev.

libvirt actually has a BZ to avoid the 'raw' node for performance when
it's not needed.

> Do we have a way to visualize the internal node graph used by
> qemu-nbd/qemu-img?

No, but as long as you explicitly specify the driver, you get exactly
what you specified.

For exploring what happens, you can pass the same json: filename to QEMU
(maybe with -hda) and then use the monitor to inspect the state.

Kevin

Re: device compatibility interface for live migration with assigned devices

[Daniel P . Berrangé]

On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:
> 
> On 2020/8/14 下午1:16, Yan Zhao wrote:
> > On Thu, Aug 13, 2020 at 12:24:50PM +0800, Jason Wang wrote:
> > > On 2020/8/10 下午3:46, Yan Zhao wrote:
> > > > > driver is it handled by?
> > > > It looks that the devlink is for network device specific, and in
> > > > devlink.h, it says
> > > > include/uapi/linux/devlink.h - Network physical device Netlink
> > > > interface,
> > > 
> > > Actually not, I think there used to have some discussion last year and the
> > > conclusion is to remove this comment.
> > > 
> > > It supports IB and probably vDPA in the future.
> > > 
> > hmm... sorry, I didn't find the referred discussion. only below discussion
> > regarding to why to add devlink.
> > 
> > https://www.mail-archive.com/netdev@vger.kernel.org/msg95801.html
> > >This doesn't seem to be too much related to networking? Why can't 
> > something
> > >like this be in sysfs?
> > 
> > It is related to networking quite bit. There has been couple of
> > iteration of this, including sysfs and configfs implementations. There
> > has been a consensus reached that this should be done by netlink. I
> > believe netlink is really the best for this purpose. Sysfs is not a good
> > idea
> 
> 
> See the discussion here:
> 
> https://patchwork.ozlabs.org/project/netdev/patch/20191115223355.1277139-1-jeffrey.t.kirs...@intel.com/
> 
> 
> > 
> > https://www.mail-archive.com/netdev@vger.kernel.org/msg96102.html
> > >there is already a way to change eth/ib via
> > >echo 'eth' > /sys/bus/pci/drivers/mlx4_core/:02:00.0/mlx4_port1
> > >
> > >sounds like this is another way to achieve the same?
> > 
> > It is. However the current way is driver-specific, not correct.
> > For mlx5, we need the same, it cannot be done in this way. Do devlink is
> > the correct way to go.
> > 
> > https://lwn.net/Articles/674867/
> > There a is need for some userspace API that would allow to expose things
> > that are not directly related to any device class like net_device of
> > ib_device, but rather chip-wide/switch-ASIC-wide stuff.
> > 
> > Use cases:
> > 1) get/set of port type (Ethernet/InfiniBand)
> > 2) monitoring of hardware messages to and from chip
> > 3) setting up port splitters - split port into multiple ones and squash 
> > again,
> >enables usage of splitter cable
> > 4) setting up shared buffers - shared among multiple ports within one 
> > chip
> > 
> > 
> > 
> > we actually can also retrieve the same information through sysfs, .e.g
> > 
> > |- [path to device]
> >|--- migration
> >| |--- self
> >| |   |---device_api
> >||   |---mdev_type
> >||   |---software_version
> >||   |---device_id
> >||   |---aggregator
> >| |--- compatible
> >| |   |---device_api
> >||   |---mdev_type
> >||   |---software_version
> >||   |---device_id
> >||   |---aggregator
> > 
> 
> Yes but:
> 
> - You need one file per attribute (one syscall for one attribute)
> - Attribute is coupled with kobject
> 
> All of above seems unnecessary.
> 
> Another point, as we discussed in another thread, it's really hard to make
> sure the above API work for all types of devices and frameworks. So having a
> vendor specific API looks much better.

>From the POV of userspace mgmt apps doing device compat checking / migration,
we certainly do NOT want to use different vendor specific APIs. We want to
have an API that can be used / controlled in a standard manner across vendors.



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

Re: device compatibility interface for live migration with assigned devices

[Jason Wang]

  
  


On 2020/8/18 下午4:55, Daniel P. Berrangé
  wrote:


  On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:

  
On 2020/8/14 下午1:16, Yan Zhao wrote:


  On Thu, Aug 13, 2020 at 12:24:50PM +0800, Jason Wang wrote:

  
On 2020/8/10 下午3:46, Yan Zhao wrote:


  
driver is it handled by?

  
  It looks that the devlink is for network device specific, and in
devlink.h, it says
include/uapi/linux/devlink.h - Network physical device Netlink
interface,


Actually not, I think there used to have some discussion last year and the
conclusion is to remove this comment.

It supports IB and probably vDPA in the future.


  
  hmm... sorry, I didn't find the referred discussion. only below discussion
regarding to why to add devlink.

https://www.mail-archive.com/netdev@vger.kernel.org/msg95801.html
	>This doesn't seem to be too much related to networking? Why can't something
	>like this be in sysfs?
	
	It is related to networking quite bit. There has been couple of
	iteration of this, including sysfs and configfs implementations. There
	has been a consensus reached that this should be done by netlink. I
	believe netlink is really the best for this purpose. Sysfs is not a good
	idea



See the discussion here:

https://patchwork.ozlabs.org/project/netdev/patch/20191115223355.1277139-1-jeffrey.t.kirs...@intel.com/




  https://www.mail-archive.com/netdev@vger.kernel.org/msg96102.html
	>there is already a way to change eth/ib via
	>echo 'eth' > /sys/bus/pci/drivers/mlx4_core/:02:00.0/mlx4_port1
	>
	>sounds like this is another way to achieve the same?
	
	It is. However the current way is driver-specific, not correct.
	For mlx5, we need the same, it cannot be done in this way. Do devlink is
	the correct way to go.

https://lwn.net/Articles/674867/
	There a is need for some userspace API that would allow to expose things
	that are not directly related to any device class like net_device of
	ib_device, but rather chip-wide/switch-ASIC-wide stuff.

	Use cases:
	1) get/set of port type (Ethernet/InfiniBand)
	2) monitoring of hardware messages to and from chip
	3) setting up port splitters - split port into multiple ones and squash again,
	   enables usage of splitter cable
	4) setting up shared buffers - shared among multiple ports within one chip



we actually can also retrieve the same information through sysfs, .e.g

|- [path to device]
   |--- migration
   | |--- self
   | |   |---device_api
   |	|   |---mdev_type
   |	|   |---software_version
   |	|   |---device_id
   |	|   |---aggregator
   | |--- compatible
   | |   |---device_api
   |	|   |---mdev_type
   |	|   |---software_version
   |	|   |---device_id
   |	|   |---aggregator



Yes but:

- You need one file per attribute (one syscall for one attribute)
- Attribute is coupled with kobject

All of above seems unnecessary.

Another point, as we discussed in another thread, it's really hard to make
sure the above API work for all types of devices and frameworks. So having a
vendor specific API looks much better.

  
  From the POV of userspace mgmt apps doing device compat checking / migration,
we certainly do NOT want to use different vendor specific APIs. We want to
have an API that can be used / controlled in a standard manner across vendors.



Yes, but it could be hard. E.g vDPA will chose to use devlink
  (there's a long debate on sysfs vs devlink). So if we go with
  sysfs, at least two APIs needs to be supported ...
Thanks




  



Regards,
Daniel

  

Re: [RFC PATCH 06/22] qemu-nbd: Use raw block driver for --offset

[Nir Soffer]

On Tue, Aug 18, 2020 at 11:47 AM Kevin Wolf  wrote:

> Am 17.08.2020 um 19:19 hat Nir Soffer geschrieben:
> > On Thu, Aug 13, 2020 at 7:36 PM Kevin Wolf  wrote:
> >
> > > Instead of implementing qemu-nbd --offset in the NBD code, just put a
> > > raw block node with the requested offset on top of the user image and
> > > rely on that doing the job.
> > >
> > > This does not only simplify the nbd_export_new() interface and bring it
> > > closer to the set of options that the nbd-server-add QMP command
> offers,
> > > but in fact it also eliminates a potential source for bugs in the NBD
> > > code which previously had to add the offset manually in all relevant
> > > places.
> > >
> >
> > Just to make sure I understand this correctly -
> >
> > qemu-nbd can work with:
> >
> > $ qemu-nbd 'json:{"driver": "file", "filename": "test.raw"}'
> >
> > And:
> >
> > $ qemu-nbd 'json:{"driver": "raw", "file": {"driver": "file",
> > "filename": "test.raw"}}'
> >
> > I assumed that we always create the raw node?
>
> No, the first form creates only the 'file' node without a 'raw' node on
> top. For all practical matters, this should be the same in qemu-img or
> qemu-nbd. For actually running VMs, omitting the 'raw' node where it's
> not needed can improve performance a little.
>

We did not check if we have different performance with the extra raw node.
Since in our use case (copying images) small reads/writes are unlikely, I
don't
think it will make a difference.

What is true is that if you use a filename without specifying the driver
> (i.e.  you rely on format probing), you'll get a 'raw' node on top of
> the 'file' node.
>
> > oVirt always uses the second form to make it easier to support offset,
> > size, and backing.
> >
> https://github.com/oVirt/ovirt-imageio/blob/2021164d064227d7c5e03c8da087adc66e3a577e/daemon/ovirt_imageio/_internal/qemu_nbd.py#L104
> >
> > This also seems to be the way libvirt builds the nodes using -blockdev.
>
> libvirt actually has a BZ to avoid the 'raw' node for performance when
> it's not needed.
>
> > Do we have a way to visualize the internal node graph used by
> > qemu-nbd/qemu-img?
>
> No, but as long as you explicitly specify the driver, you get exactly
> what you specified.
>

So this is not really needed then.


> For exploring what happens, you can pass the same json: filename to QEMU
> (maybe with -hda) and then use the monitor to inspect the state.
>
> Kevin
>
>

Re: device compatibility interface for live migration with assigned devices

[Cornelia Huck]

On Tue, 18 Aug 2020 09:55:27 +0100
Daniel P. Berrangé  wrote:

> On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:
> > Another point, as we discussed in another thread, it's really hard to make
> > sure the above API work for all types of devices and frameworks. So having a
> > vendor specific API looks much better.  
> 
> From the POV of userspace mgmt apps doing device compat checking / migration,
> we certainly do NOT want to use different vendor specific APIs. We want to
> have an API that can be used / controlled in a standard manner across vendors.

As we certainly will need to have different things to check for
different device types and vendor drivers, would it still be fine to
have differing (say) attributes, as long as they are presented (and can
be discovered) in a standardized way?

(See e.g. what I came up with for vfio-ccw in a different branch of
this thread.)

E.g.
version=
.type_specific_value0=
.type_specific_value1=
.vendor_driver_specific_value0=

with a type or vendor driver having some kind of
get_supported_attributes method?

Re: [RFC PATCH] os-posix: fix regression for install-less datadir location

[Paolo Bonzini]

On 16/07/20 16:11, Marc-André Lureau wrote:
> os_find_datadir() used to check the ../share/qemu location (regardless
> of CONFIG_QEMU_DATADIR). It turns out that people rely on that location
> for running qemu in an arbitrary "install-less/portable" fashion. Change
> the logic to return that directory as a last resort.
> 
> (this is an alternative to the patch "[PATCH 1/1] os_find_datadir: search
> as in version 4.2" from Joe Slater)
> 
> Fixes: 6dd2dacedd83d12328 ("os-posix: simplify os_find_datadir")
> Cc: Paolo Bonzini 
> Signed-off-by: Marc-André Lureau 

For 5.2 I plan to support fully relocatable installs, so I think this
will not be needed.

The idea is to write a function like

char *get_relocatable_path(const char *dir);

That takes CONFIG_QEMU_*DIR as the argument, turns it into a path
relative to bindir, and tacks it to the end of qemu_get_exec_dir().

So for example all references to CONFIG_QEMU_DATADIR would invoke
get_relocatable_path(CONFIG_QEMU_DATADIR), which would return something
like "/usr/bin/../share/qemu".

Paolo

Re: device compatibility interface for live migration with assigned devices

[Daniel P . Berrangé]

Your mail came through as HTML-only so all the quoting and attribution
is mangled / lost now :-(

On Tue, Aug 18, 2020 at 05:01:51PM +0800, Jason Wang wrote:
>On 2020/8/18 下午4:55, Daniel P. Berrangé wrote:
> 
>  On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:
> 
>  On 2020/8/14 下午1:16, Yan Zhao wrote:
> 
>  On Thu, Aug 13, 2020 at 12:24:50PM +0800, Jason Wang wrote:
> 
>  On 2020/8/10 下午3:46, Yan Zhao wrote:

>  we actually can also retrieve the same information through sysfs, .e.g
> 
>  |- [path to device]
> |--- migration
> | |--- self
> | |   |---device_api
> ||   |---mdev_type
> ||   |---software_version
> ||   |---device_id
> ||   |---aggregator
> | |--- compatible
> | |   |---device_api
> ||   |---mdev_type
> ||   |---software_version
> ||   |---device_id
> ||   |---aggregator
> 
> 
>  Yes but:
> 
>  - You need one file per attribute (one syscall for one attribute)
>  - Attribute is coupled with kobject
> 
>  All of above seems unnecessary.
> 
>  Another point, as we discussed in another thread, it's really hard to make
>  sure the above API work for all types of devices and frameworks. So having a
>  vendor specific API looks much better.
> 
>  From the POV of userspace mgmt apps doing device compat checking / migration,
>  we certainly do NOT want to use different vendor specific APIs. We want to
>  have an API that can be used / controlled in a standard manner across 
> vendors.
> 
>Yes, but it could be hard. E.g vDPA will chose to use devlink (there's a
>long debate on sysfs vs devlink). So if we go with sysfs, at least two
>APIs needs to be supported ...

NB, I was not questioning devlink vs sysfs directly. If devlink is related
to netlink, I can't say I'm enthusiastic as IMKE sysfs is easier to deal
with. I don't know enough about devlink to have much of an opinion though.
The key point was that I don't want the userspace APIs we need to deal with
to be vendor specific.

What I care about is that we have a *standard* userspace API for performing
device compatibility checking / state migration, for use by QEMU/libvirt/
OpenStack, such that we can write code without countless vendor specific
code paths.

If there is vendor specific stuff on the side, that's fine as we can ignore
that, but the core functionality for device compat / migration needs to be
standardized.

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

Re: device compatibility interface for live migration with assigned devices

[Daniel P . Berrangé]

On Tue, Aug 18, 2020 at 11:06:17AM +0200, Cornelia Huck wrote:
> On Tue, 18 Aug 2020 09:55:27 +0100
> Daniel P. Berrangé  wrote:
> 
> > On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:
> > > Another point, as we discussed in another thread, it's really hard to make
> > > sure the above API work for all types of devices and frameworks. So 
> > > having a
> > > vendor specific API looks much better.  
> > 
> > From the POV of userspace mgmt apps doing device compat checking / 
> > migration,
> > we certainly do NOT want to use different vendor specific APIs. We want to
> > have an API that can be used / controlled in a standard manner across 
> > vendors.
> 
> As we certainly will need to have different things to check for
> different device types and vendor drivers, would it still be fine to
> have differing (say) attributes, as long as they are presented (and can
> be discovered) in a standardized way?

Yes, the control API and algorithm to deal with the problem needs to
have standardization, but the data passed in/out of the APIs can vary.

Essentially the key is that vendors should be able to create devices
at the kernel, and those devices should "just work" with the existing
generic userspace migration / compat checking code, without needing
extra vendor specific logic to be added.

Note, I'm not saying that the userspace decisions would be perfectly
optimal based on generic code. They might be making a simplified
decision that while functionally safe, is not the ideal solution.
Adding vendor specific code might be able to optimize the userspace
decisions, but that should be considered just optimization, not a
core must have for any opertion.

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

Re: device compatibility interface for live migration with assigned devices

[Cornelia Huck]

On Tue, 18 Aug 2020 10:16:28 +0100
Daniel P. Berrangé  wrote:

> On Tue, Aug 18, 2020 at 05:01:51PM +0800, Jason Wang wrote:
> >On 2020/8/18 下午4:55, Daniel P. Berrangé wrote:
> > 
> >  On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:
> > 
> >  On 2020/8/14 下午1:16, Yan Zhao wrote:
> > 
> >  On Thu, Aug 13, 2020 at 12:24:50PM +0800, Jason Wang wrote:
> > 
> >  On 2020/8/10 下午3:46, Yan Zhao wrote:  
> 
> >  we actually can also retrieve the same information through sysfs, .e.g
> > 
> >  |- [path to device]
> > |--- migration
> > | |--- self
> > | |   |---device_api
> > ||   |---mdev_type
> > ||   |---software_version
> > ||   |---device_id
> > ||   |---aggregator
> > | |--- compatible
> > | |   |---device_api
> > ||   |---mdev_type
> > ||   |---software_version
> > ||   |---device_id
> > ||   |---aggregator
> > 
> > 
> >  Yes but:
> > 
> >  - You need one file per attribute (one syscall for one attribute)
> >  - Attribute is coupled with kobject

Is that really that bad? You have the device with an embedded kobject
anyway, and you can just put things into an attribute group?

[Also, I think that self/compatible split in the example makes things
needlessly complex. Shouldn't semantic versioning and matching already
cover nearly everything? I would expect very few cases that are more
complex than that. Maybe the aggregation stuff, but I don't think we
need that self/compatible split for that, either.]

> > 
> >  All of above seems unnecessary.
> > 
> >  Another point, as we discussed in another thread, it's really hard to make
> >  sure the above API work for all types of devices and frameworks. So having 
> > a
> >  vendor specific API looks much better.
> > 
> >  From the POV of userspace mgmt apps doing device compat checking / 
> > migration,
> >  we certainly do NOT want to use different vendor specific APIs. We want to
> >  have an API that can be used / controlled in a standard manner across 
> > vendors.
> > 
> >Yes, but it could be hard. E.g vDPA will chose to use devlink (there's a
> >long debate on sysfs vs devlink). So if we go with sysfs, at least two
> >APIs needs to be supported ...  
> 
> NB, I was not questioning devlink vs sysfs directly. If devlink is related
> to netlink, I can't say I'm enthusiastic as IMKE sysfs is easier to deal
> with. I don't know enough about devlink to have much of an opinion though.
> The key point was that I don't want the userspace APIs we need to deal with
> to be vendor specific.

From what I've seen of devlink, it seems quite nice; but I understand
why sysfs might be easier to deal with (especially as there's likely
already a lot of code using it.)

I understand that some users would like devlink because it is already
widely used for network drivers (and some others), but I don't think
the majority of devices used with vfio are network (although certainly
a lot of them are.)

> 
> What I care about is that we have a *standard* userspace API for performing
> device compatibility checking / state migration, for use by QEMU/libvirt/
> OpenStack, such that we can write code without countless vendor specific
> code paths.
> 
> If there is vendor specific stuff on the side, that's fine as we can ignore
> that, but the core functionality for device compat / migration needs to be
> standardized.

To summarize:
- choose one of sysfs or devlink
- have a common interface, with a standardized way to add
  vendor-specific attributes
?

Re: [PATCH] fixed proc myself (linux user) for musl

[Андрей Аладьев]

I've sent 3 separate patches using "git send email", please review.

пн, 17 авг. 2020 г. в 18:12, Laurent Vivier :

> Hi,
>
> it seems your patch does several things.
>
> Could you split it into several patches?
> Could you add a description of what the patches do?
> Could you use a tool like "git send-email" or "git publish" to send your
> series?
>
> For the patch moving code from syscall.c to syscall_proc.c, write a
> patch that moves the code without modifying it. And then you can follow
> with a patch fixing what you want.
>
> Thanks,
> Laurent
>
> Le 16/08/2020 à 21:58, Андрей Аладьев a écrit :
> > From: Andrew Aladjev  > >
> > Date: Sun, 16 Aug 2020 22:50:13 +0300
> > Subject: [PATCH] fixed proc myself (linux user) for musl
> > Buglink: https://bugs.gentoo.org/587230
> > Signed-off-by: Andrew Aladjev  > >
> > ---
> >  linux-user/Makefile.objs  |  5 +-
> >  linux-user/elfload.c  |  7 ++-
> >  linux-user/exit.c |  7 ++-
> >  linux-user/main.c |  2 +-
> >  linux-user/qemu.h |  1 +
> >  linux-user/syscall.c  | 85 +-
> >  linux-user/syscall_proc.c | 96 +++
> >  linux-user/syscall_proc.h |  8 
> >  8 files changed, 150 insertions(+), 61 deletions(-)
> >  create mode 100644 linux-user/syscall_proc.c
> >  create mode 100644 linux-user/syscall_proc.h
> >
> > diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> > index 1940910a73..ad84380738 100644
> > --- a/linux-user/Makefile.objs
> > +++ b/linux-user/Makefile.objs
> > @@ -1,7 +1,8 @@
> >  obj-y = main.o syscall.o strace.o mmap.o signal.o \
> >   elfload.o linuxload.o uaccess.o uname.o \
> > - safe-syscall.o $(TARGET_ABI_DIR)/signal.o \
> > -$(TARGET_ABI_DIR)/cpu_loop.o exit.o fd-trans.o
> > + safe-syscall.o syscall_proc.o \
> > + $(TARGET_ABI_DIR)/cpu_loop.o $(TARGET_ABI_DIR)/signal.o \
> > + exit.o fd-trans.o
> >
> >  obj-$(TARGET_HAS_BFLT) += flatload.o
> >  obj-$(TARGET_I386) += vm86.o
> > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > index 619c054cc4..201db61d91 100644
> > --- a/linux-user/elfload.c
> > +++ b/linux-user/elfload.c
> > @@ -2319,7 +2319,10 @@ exit_errmsg:
> > buffer is sufficiently aligned to present no problems to the host
> > in accessing data at aligned offsets within the buffer.
> >
> > -   On return: INFO values will be filled in, as necessary or available.
>  */
> > +   On return: INFO values will be filled in, as necessary or available.
> > +
> > +   WARNING: this function won't close "image_fd".
> > +*/
> >
> >  static void load_elf_image(const char *image_name, int image_fd,
> > struct image_info *info, char **pinterp_name,
> > @@ -2576,7 +2579,6 @@ static void load_elf_image(const char *image_name,
> > int image_fd,
> >
> >  mmap_unlock();
> >
> > -close(image_fd);
> >  return;
> >
> >   exit_read:
> > @@ -2610,6 +2612,7 @@ static void load_elf_interp(const char *filename,
> > struct image_info *info,
> >  }
> >
> >  load_elf_image(filename, fd, info, NULL, bprm_buf);
> > +close(fd);
> >  return;
> >
> >   exit_perror:
> > diff --git a/linux-user/exit.c b/linux-user/exit.c
> > index 1594015444..f0626fc432 100644
> > --- a/linux-user/exit.c
> > +++ b/linux-user/exit.c
> > @@ -28,12 +28,15 @@ extern void __gcov_dump(void);
> >
> >  void preexit_cleanup(CPUArchState *env, int code)
> >  {
> > +close(execfd);
> > +
> >  #ifdef CONFIG_GPROF
> >  _mcleanup();
> >  #endif
> >  #ifdef CONFIG_GCOV
> >  __gcov_dump();
> >  #endif
> > -gdb_exit(env, code);
> > -qemu_plugin_atexit_cb();
> > +
> > +gdb_exit(env, code);
> > +qemu_plugin_atexit_cb();
> >  }
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 22578b1633..9cc6c1e6da 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -48,6 +48,7 @@
> >  #include "crypto/init.h"
> >
> >  char *exec_path;
> > +int execfd;
> >
> >  int singlestep;
> >  static const char *argv0;
> > @@ -628,7 +629,6 @@ int main(int argc, char **argv, char **envp)
> >  int target_argc;
> >  int i;
> >  int ret;
> > -int execfd;
> >  int log_mask;
> >  unsigned long max_reserved_va;
> >
> > diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> > index 792c74290f..d822f2b9df 100644
> > --- a/linux-user/qemu.h
> > +++ b/linux-user/qemu.h
> > @@ -156,6 +156,7 @@ typedef struct TaskState {
> >  } __attribute__((aligned(16))) TaskState;
> >
> >  extern char *exec_path;
> > +extern int execfd;
> >  void init_task_state(TaskState *ts);
> >  void task_settid(TaskState *);
> >  void stop_all_tasks(void);
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 05f03919ff..483a735c1a 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -122,6 +122,7 @@
> >  #include "qapi/error.h"
> >  #include "fd-trans.h"
> >  #include "tcg/tcg

Re: device compatibility interface for live migration with assigned devices

[Cornelia Huck]

On Tue, 18 Aug 2020 10:24:33 +0100
Daniel P. Berrangé  wrote:

> On Tue, Aug 18, 2020 at 11:06:17AM +0200, Cornelia Huck wrote:
> > On Tue, 18 Aug 2020 09:55:27 +0100
> > Daniel P. Berrangé  wrote:
> >   
> > > On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:  
> > > > Another point, as we discussed in another thread, it's really hard to 
> > > > make
> > > > sure the above API work for all types of devices and frameworks. So 
> > > > having a
> > > > vendor specific API looks much better.
> > > 
> > > From the POV of userspace mgmt apps doing device compat checking / 
> > > migration,
> > > we certainly do NOT want to use different vendor specific APIs. We want to
> > > have an API that can be used / controlled in a standard manner across 
> > > vendors.  
> > 
> > As we certainly will need to have different things to check for
> > different device types and vendor drivers, would it still be fine to
> > have differing (say) attributes, as long as they are presented (and can
> > be discovered) in a standardized way?  
> 
> Yes, the control API and algorithm to deal with the problem needs to
> have standardization, but the data passed in/out of the APIs can vary.
> 
> Essentially the key is that vendors should be able to create devices
> at the kernel, and those devices should "just work" with the existing
> generic userspace migration / compat checking code, without needing
> extra vendor specific logic to be added.
> 
> Note, I'm not saying that the userspace decisions would be perfectly
> optimal based on generic code. They might be making a simplified
> decision that while functionally safe, is not the ideal solution.
> Adding vendor specific code might be able to optimize the userspace
> decisions, but that should be considered just optimization, not a
> core must have for any opertion.

Yes, that sounds reasonable.

Re: [PATCH 2/2] hw: ehci: check return value of 'usb_packet_map'

[Gerd Hoffmann]

On Wed, Aug 12, 2020 at 09:17:27AM -0700, Li Qiang wrote:
> If 'usb_packet_map' fails, we should stop to process the usb
> request.

Queued up all three ehci/xhci patches.

thanks,
  Gerd

Re: [RFC PATCH 1/9] memory: Initialize MemoryRegionOps for RAM memory regions

[P J P]

+-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
| Fix by initializing the MemoryRegionOps to ram_device_mem_ops, this way the 
| memory accesses are properly dispatched using 
| memory_region_ram_device_read() / memory_region_ram_device_write().
| 
| Fixes: 4a2e242bbb ("memory: Don't use memcpy for ram_device regions")
| Signed-off-by: Philippe Mathieu-Daudé 
| ---
| Since v1: Corrected description (PJP)
| Cc: P J P 
| ---
|  softmmu/memory.c | 10 ++
|  1 file changed, 10 insertions(+)
| 
| diff --git a/softmmu/memory.c b/softmmu/memory.c
| index 651705b7d1..8139da1a58 100644
| --- a/softmmu/memory.c
| +++ b/softmmu/memory.c
| @@ -1517,6 +1517,8 @@ void 
memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
|  Error *err = NULL;
|  memory_region_init(mr, owner, name, size);
|  mr->ram = true;
| +mr->ops = &ram_device_mem_ops;
| +mr->opaque = mr;
|  mr->terminates = true;


I wonder if 'mr->ops = &ram_device_mem_ops' could be done in 
memory_region_init() instead?

Otherwise looks okay.

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

Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug

[Ani Sinha]

Igor etc, I just tested this patch using a Windows 2012 R2 image and
it seems to be working. Any feedbacks on this patch?

On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha  wrote:
>
> We introduce a new global flag for PIIX with which we can turn on or off PCI
> device hotplug on the root bus. This flag can be used to prevent all PCI
> devices from getting hotplugged or unplugged from the root PCI bus.
>
> Signed-off-by: Ani Sinha 
> ---
>  hw/acpi/piix4.c  |  3 +++
>  hw/i386/acpi-build.c | 20 
>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 26bac4f..94ec35a 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>
>  AcpiPciHpState acpi_pci_hotplug;
>  bool use_acpi_hotplug_bridge;
> +bool use_acpi_root_pci_hotplug;
>
>  uint8_t disable_s3;
>  uint8_t disable_s4;
> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
>  DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
>  DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
>   use_acpi_hotplug_bridge, true),
> +DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
> + use_acpi_root_pci_hotplug, true),
>  DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>   acpi_memory_hotplug.is_enabled, true),
>  DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbb..a82e5c1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
>  bool s3_disabled;
>  bool s4_disabled;
>  bool pcihp_bridge_en;
> +bool pcihp_root_en;
>  uint8_t s4_val;
>  AcpiFadtData fadt;
>  uint16_t cpu_hp_io_base;
> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, 
> AcpiPmInfo *pm)
>  pm->pcihp_bridge_en =
>  object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
>   NULL);
> +pm->pcihp_root_en =
> +object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
> +
>  }
>
>  static void acpi_get_misc_info(AcpiMiscInfo *info)
> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml 
> *method, int slot)
>  }
>
>  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> - bool pcihp_bridge_en)
> + bool pcihp_bridge_en,
> + bool pcihp_root_en)
>  {
>  Aml *dev, *notify_method = NULL, *method;
>  QObject *bsel;
>  PCIBus *sec;
>  int i;
> +bool root_bus = pci_bus_is_root(bus);
> +bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>
>  bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, 
> NULL);
>  if (bsel) {
> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml 
> *parent_scope, PCIBus *bus,
>  bool bridge_in_acpi;
>
>  if (!pdev) {
> +/* skip if pci hotplug for the root bus is disabled */
> +if (root_pcihp_disabled)
> +continue;
>  if (bsel) { /* add hotplug slots for non present devices */
>  dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
>  aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml 
> *parent_scope, PCIBus *bus,
>  method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
>  aml_append(method, aml_return(aml_int(s3d)));
>  aml_append(dev, method);
> -} else if (hotplug_enabled_dev) {
> +} else if (hotplug_enabled_dev && !root_pcihp_disabled) {
>  /* add _SUN/_EJ0 to make slot hotpluggable  */
>  aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>
> @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml 
> *parent_scope, PCIBus *bus,
>   */
>  PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>
> -build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
> +build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
> + pcihp_root_en);
>  }
>  /* slot descriptor has been composed, add it into parent context */
>  aml_append(parent_scope, dev);
> @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>  if (bus) {
>  Aml *scope = aml_scope("PCI0");
>  /* Scan all PCI buses. Generate tables to support hotplug. */
> -build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
> +build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
> + pm->pcihp_root_en);
>
>  if (TP

Re: [RFC PATCH 1/9] memory: Initialize MemoryRegionOps for RAM memory regions

[Philippe Mathieu-Daudé]

On 8/18/20 11:52 AM, P J P wrote:
> +-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
> | Fix by initializing the MemoryRegionOps to ram_device_mem_ops, this way the 
> | memory accesses are properly dispatched using 
> | memory_region_ram_device_read() / memory_region_ram_device_write().
> | 
> | Fixes: 4a2e242bbb ("memory: Don't use memcpy for ram_device regions")
> | Signed-off-by: Philippe Mathieu-Daudé 
> | ---
> | Since v1: Corrected description (PJP)
> | Cc: P J P 
> | ---
> |  softmmu/memory.c | 10 ++
> |  1 file changed, 10 insertions(+)
> | 
> | diff --git a/softmmu/memory.c b/softmmu/memory.c
> | index 651705b7d1..8139da1a58 100644
> | --- a/softmmu/memory.c
> | +++ b/softmmu/memory.c
> | @@ -1517,6 +1517,8 @@ void 
> memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
> |  Error *err = NULL;
> |  memory_region_init(mr, owner, name, size);
> |  mr->ram = true;
> | +mr->ops = &ram_device_mem_ops;
> | +mr->opaque = mr;
> |  mr->terminates = true;
> 
> 
> I wonder if 'mr->ops = &ram_device_mem_ops' could be done in 
> memory_region_init() instead?

I don't think so, because to use &ram_device_mem_ops handlers use
mr->opaque and mr->ram_block, so alias/io/iommu/rom types would
directly crash accessing these NULL fields.

> 
> Otherwise looks okay.

Thanks!

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

Re: [PATCH v3 000/150] Meson integration for 5.2

[Cornelia Huck]

On Mon, 17 Aug 2020 16:34:53 +0200
Paolo Bonzini  wrote:

> - Simplified/rewrote handling of the modules variable [Howard]
> - Fixed access to gdb-xml files [Howard]
> - Fixed cross-compilation failure due to mingw prefix [Howard]
> - Fixed SDL2 detection on cross compiles [Howard]
> - Fixed sub-makefiles that include config-host.mak
> 
> You can find it at the usual place, https://gitlab.com/bonzini/qemu.git
> branch meson-poc-next.

Builds (on x86 & s390x), runs (this time with the right binary ;), and
passes make check, make check-tcg, and kvm unit tests (s390x) for me.

Re: [PATCH 00/41] qom: Automated conversion of type checking boilerplate

[Roman Bolshakov]

On Thu, Aug 13, 2020 at 06:25:44PM -0400, Eduardo Habkost wrote:
> This is an extension of the series previously submitted by
> Daniel[1], including a script that will convert existing type
> checker macros automatically.
> 

Hi Eduardo,

do you have a repo where it can be checked it out?

Thanks,
Roman

Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug

[Philippe Mathieu-Daudé]

Hi,

On 8/18/20 11:54 AM, Ani Sinha wrote:
> Igor etc, I just tested this patch using a Windows 2012 R2 image and
> it seems to be working. Any feedbacks on this patch?
> 
> On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha  wrote:
>>
>> We introduce a new global flag for PIIX with which we can turn on or off PCI
>> device hotplug on the root bus. This flag can be used to prevent all PCI
>> devices from getting hotplugged or unplugged from the root PCI bus.
>>
>> Signed-off-by: Ani Sinha 
>> ---
>>  hw/acpi/piix4.c  |  3 +++
>>  hw/i386/acpi-build.c | 20 
>>  2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>> index 26bac4f..94ec35a 100644
>> --- a/hw/acpi/piix4.c
>> +++ b/hw/acpi/piix4.c
>> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>>
>>  AcpiPciHpState acpi_pci_hotplug;
>>  bool use_acpi_hotplug_bridge;
>> +bool use_acpi_root_pci_hotplug;
>>
>>  uint8_t disable_s3;
>>  uint8_t disable_s4;
>> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
>>  DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
>>  DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
>>   use_acpi_hotplug_bridge, true),
>> +DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
>> + use_acpi_root_pci_hotplug, true),

>From what I understood here, this file is already pretty twisted
and Igor doesn't want more workarounds:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg690564.html

¯\_(ツ)_/¯

>>  DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>>   acpi_memory_hotplug.is_enabled, true),
>>  DEFINE_PROP_END_OF_LIST(),
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index b7bcbbb..a82e5c1 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
>>  bool s3_disabled;
>>  bool s4_disabled;
>>  bool pcihp_bridge_en;
>> +bool pcihp_root_en;
>>  uint8_t s4_val;
>>  AcpiFadtData fadt;
>>  uint16_t cpu_hp_io_base;
>> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, 
>> AcpiPmInfo *pm)
>>  pm->pcihp_bridge_en =
>>  object_property_get_bool(obj, 
>> "acpi-pci-hotplug-with-bridge-support",
>>   NULL);
>> +pm->pcihp_root_en =
>> +object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
>> +
>>  }
>>
>>  static void acpi_get_misc_info(AcpiMiscInfo *info)
>> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml 
>> *method, int slot)
>>  }
>>
>>  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>> - bool pcihp_bridge_en)
>> + bool pcihp_bridge_en,
>> + bool pcihp_root_en)
>>  {
>>  Aml *dev, *notify_method = NULL, *method;
>>  QObject *bsel;
>>  PCIBus *sec;
>>  int i;
>> +bool root_bus = pci_bus_is_root(bus);
>> +bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>>
>>  bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, 
>> NULL);
>>  if (bsel) {
>> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml 
>> *parent_scope, PCIBus *bus,
>>  bool bridge_in_acpi;
>>
>>  if (!pdev) {
>> +/* skip if pci hotplug for the root bus is disabled */
>> +if (root_pcihp_disabled)
>> +continue;
>>  if (bsel) { /* add hotplug slots for non present devices */
>>  dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
>>  aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml 
>> *parent_scope, PCIBus *bus,
>>  method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
>>  aml_append(method, aml_return(aml_int(s3d)));
>>  aml_append(dev, method);
>> -} else if (hotplug_enabled_dev) {
>> +} else if (hotplug_enabled_dev && !root_pcihp_disabled) {
>>  /* add _SUN/_EJ0 to make slot hotpluggable  */
>>  aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>>
>> @@ -439,7 +449,8 @@ static void build_append_pci_bus_devices(Aml 
>> *parent_scope, PCIBus *bus,
>>   */
>>  PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>>
>> -build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
>> +build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
>> + pcihp_root_en);
>>  }
>>  /* slot descriptor has been composed, add it into parent context */
>>  aml_append(parent_scope, dev);
>> @@ -1818,7 +1829,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>  if (bus) {
>>  Am

Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug

[Ani Sinha]

This is not a workaround, this is a feature we need badly. We have
discussed this at length. Igor suggested we have one additional global flag
to disable all pci hotplug. But then that leads to strange case when pci
hotplug on the bridges are enabled (we have a separate flag for that).
Hence, I think we can simply add another flag to disable hotplug just for
the pci root bus.

On Tue, Aug 18, 2020 at 3:46 PM Philippe Mathieu-Daudé 
wrote:

> Hi,
>
>
>
> On 8/18/20 11:54 AM, Ani Sinha wrote:
>
> > Igor etc, I just tested this patch using a Windows 2012 R2 image and
>
> > it seems to be working. Any feedbacks on this patch?
>
> >
>
> > On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha  wrote:
>
> >>
>
> >> We introduce a new global flag for PIIX with which we can turn on or
> off PCI
>
> >> device hotplug on the root bus. This flag can be used to prevent all PCI
>
> >> devices from getting hotplugged or unplugged from the root PCI bus.
>
> >>
>
> >> Signed-off-by: Ani Sinha 
>
> >> ---
>
> >>  hw/acpi/piix4.c  |  3 +++
>
> >>  hw/i386/acpi-build.c | 20 
>
> >>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> >>
>
> >> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>
> >> index 26bac4f..94ec35a 100644
>
> >> --- a/hw/acpi/piix4.c
>
> >> +++ b/hw/acpi/piix4.c
>
> >> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>
> >>
>
> >>  AcpiPciHpState acpi_pci_hotplug;
>
> >>  bool use_acpi_hotplug_bridge;
>
> >> +bool use_acpi_root_pci_hotplug;
>
> >>
>
> >>  uint8_t disable_s3;
>
> >>  uint8_t disable_s4;
>
> >> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
>
> >>  DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
>
> >>  DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support",
> PIIX4PMState,
>
> >>   use_acpi_hotplug_bridge, true),
>
> >> +DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
>
> >> + use_acpi_root_pci_hotplug, true),
>
>
>
> From what I understood here, this file is already pretty twisted
>
> and Igor doesn't want more workarounds:
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg690564.html
>
>
>
> ¯\_(ツ)_/¯
>
>
>
> >>  DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>
> >>   acpi_memory_hotplug.is_enabled, true),
>
> >>  DEFINE_PROP_END_OF_LIST(),
>
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>
> >> index b7bcbbb..a82e5c1 100644
>
> >> --- a/hw/i386/acpi-build.c
>
> >> +++ b/hw/i386/acpi-build.c
>
> >> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
>
> >>  bool s3_disabled;
>
> >>  bool s4_disabled;
>
> >>  bool pcihp_bridge_en;
>
> >> +bool pcihp_root_en;
>
> >>  uint8_t s4_val;
>
> >>  AcpiFadtData fadt;
>
> >>  uint16_t cpu_hp_io_base;
>
> >> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine,
> AcpiPmInfo *pm)
>
> >>  pm->pcihp_bridge_en =
>
> >>  object_property_get_bool(obj,
> "acpi-pci-hotplug-with-bridge-support",
>
> >>   NULL);
>
> >> +pm->pcihp_root_en =
>
> >> +object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
>
> >> +
>
> >>  }
>
> >>
>
> >>  static void acpi_get_misc_info(AcpiMiscInfo *info)
>
> >> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml
> *method, int slot)
>
> >>  }
>
> >>
>
> >>  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus
> *bus,
>
> >> - bool pcihp_bridge_en)
>
> >> + bool pcihp_bridge_en,
>
> >> + bool pcihp_root_en)
>
> >>  {
>
> >>  Aml *dev, *notify_method = NULL, *method;
>
> >>  QObject *bsel;
>
> >>  PCIBus *sec;
>
> >>  int i;
>
> >> +bool root_bus = pci_bus_is_root(bus);
>
> >> +bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>
> >>
>
> >>  bsel = object_property_get_qobject(OBJECT(bus),
> ACPI_PCIHP_PROP_BSEL, NULL);
>
> >>  if (bsel) {
>
> >> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
>
> >>  bool bridge_in_acpi;
>
> >>
>
> >>  if (!pdev) {
>
> >> +/* skip if pci hotplug for the root bus is disabled */
>
> >> +if (root_pcihp_disabled)
>
> >> +continue;
>
> >>  if (bsel) { /* add hotplug slots for non present devices */
>
> >>  dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
>
> >>  aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
>
> >> @@ -419,7 +429,7 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
>
> >>  method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
>
> >>  aml_append(method, aml_return(aml_int(s3d)));
>
> >>  aml_append(dev, method);
>
> >> -} else if (hotplug_enabled_dev) {
>
> >> +} else if (hotplug_enabled_d

Re: [PATCH v3 000/150] Meson integration for 5.2

[Paolo Bonzini]

On 18/08/20 11:59, Cornelia Huck wrote:
> On Mon, 17 Aug 2020 16:34:53 +0200
> Paolo Bonzini  wrote:
> 
>> - Simplified/rewrote handling of the modules variable [Howard]
>> - Fixed access to gdb-xml files [Howard]
>> - Fixed cross-compilation failure due to mingw prefix [Howard]
>> - Fixed SDL2 detection on cross compiles [Howard]
>> - Fixed sub-makefiles that include config-host.mak
>>
>> You can find it at the usual place, https://gitlab.com/bonzini/qemu.git
>> branch meson-poc-next.
> 
> Builds (on x86 & s390x), runs (this time with the right binary ;), and
> passes make check, make check-tcg, and kvm unit tests (s390x) for me.
> 

Great, I'll do the last tests here and send a pull request.

Paolo

Re: [PATCH 108/150] meson: convert hw/isa

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs |  1 -
>  hw/isa/Makefile.objs | 11 ---
>  hw/isa/meson.build   | 11 +++
>  hw/meson.build   |  1 +
>  4 files changed, 12 insertions(+), 12 deletions(-)
>  delete mode 100644 hw/isa/Makefile.objs
>  create mode 100644 hw/isa/meson.build
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 119/150] meson: convert hw/cpu

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs | 1 -
>  hw/cpu/Makefile.objs | 5 -
>  hw/cpu/meson.build   | 6 ++
>  hw/meson.build   | 1 +
>  4 files changed, 7 insertions(+), 6 deletions(-)
>  delete mode 100644 hw/cpu/Makefile.objs
>  create mode 100644 hw/cpu/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 6fce6a1473..97ad30295e 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -5,7 +5,6 @@ devices-dirs-y += adc/
>  devices-dirs-y += audio/
>  devices-dirs-y += block/
>  devices-dirs-y += char/
> -devices-dirs-y += cpu/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
> deleted file mode 100644
> index 8db9e8a7b3..00
> --- a/hw/cpu/Makefile.objs
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
> -obj-$(CONFIG_REALVIEW) += realview_mpcore.o
> -obj-$(CONFIG_A9MPCORE) += a9mpcore.o
> -obj-$(CONFIG_A15MPCORE) += a15mpcore.o
> -common-obj-y += core.o cluster.o
> diff --git a/hw/cpu/meson.build b/hw/cpu/meson.build
> new file mode 100644
> index 00..9e52fee9e7
> --- /dev/null
> +++ b/hw/cpu/meson.build
> @@ -0,0 +1,6 @@
> +softmmu_ss.add(files('core.c', 'cluster.c'))
> +
> +specific_ss.add(when: 'CONFIG_ARM11MPCORE', if_true: files('arm11mpcore.c'))
> +specific_ss.add(when: 'CONFIG_REALVIEW', if_true: files('realview_mpcore.c'))
> +specific_ss.add(when: 'CONFIG_A9MPCORE', if_true: files('a9mpcore.c'))
> +specific_ss.add(when: 'CONFIG_A15MPCORE', if_true: files('a15mpcore.c'))
> diff --git a/hw/meson.build b/hw/meson.build
> index dacd26c561..103da4840b 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -1,4 +1,5 @@
>  subdir('core')
> +subdir('cpu')
>  subdir('display')
>  subdir('dma')
>  subdir('gpio')
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 123/150] meson: convert hw/adc

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs | 1 -
>  hw/adc/Makefile.objs | 1 -
>  hw/adc/meson.build   | 1 +
>  hw/meson.build   | 1 +
>  4 files changed, 2 insertions(+), 2 deletions(-)
>  delete mode 100644 hw/adc/Makefile.objs
>  create mode 100644 hw/adc/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index c0cbc0f132..4bdb674ef0 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -1,7 +1,6 @@
>  ifeq ($(CONFIG_SOFTMMU), y)
>  devices-dirs-$(call lor,$(CONFIG_VIRTIO_9P),$(call 
> land,$(CONFIG_VIRTFS),$(CONFIG_XEN))) += 9pfs/
>  devices-dirs-y += acpi/
> -devices-dirs-y += adc/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/adc/Makefile.objs b/hw/adc/Makefile.objs
> deleted file mode 100644
> index 2b9dc36c7f..00
> --- a/hw/adc/Makefile.objs
> +++ /dev/null
> @@ -1 +0,0 @@
> -common-obj-$(CONFIG_STM32F2XX_ADC) += stm32f2xx_adc.o
> diff --git a/hw/adc/meson.build b/hw/adc/meson.build
> new file mode 100644
> index 00..0d62ae96ae
> --- /dev/null
> +++ b/hw/adc/meson.build
> @@ -0,0 +1 @@
> +softmmu_ss.add(when: 'CONFIG_STM32F2XX_ADC', if_true: 
> files('stm32f2xx_adc.c'))
> diff --git a/hw/meson.build b/hw/meson.build
> index 624335be90..ffa3f06dc0 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -1,3 +1,4 @@
> +subdir('adc')
>  subdir('audio')
>  subdir('block')
>  subdir('char')
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v7 10/47] mirror-top: Support compressed writes

[Kevin Wolf]

Am 25.06.2020 um 17:21 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz 
> ---
>  block/mirror.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index e8e8844afc..469acf4600 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1480,6 +1480,15 @@ static int coroutine_fn 
> bdrv_mirror_top_pdiscard(BlockDriverState *bs,
>  NULL, 0);
>  }
>  
> +static int coroutine_fn bdrv_mirror_top_pwritev_compressed(BlockDriverState 
> *bs,
> +   uint64_t offset,
> +   uint64_t bytes,
> +   QEMUIOVector 
> *qiov)
> +{
> +return bdrv_mirror_top_pwritev(bs, offset, bytes, qiov,
> +   BDRV_REQ_WRITE_COMPRESSED);
> +}

Hm, not sure if it's a problem, but bdrv_supports_compressed_writes()
will now return true for mirror-top. However, with an active mirror to a
target that doesn't support compression, trying to actually do a
compressed write will always return -ENOTSUP.

So I guess the set of nodes patch 7 looks at still isn't quite complete.
However, it's not obvious how to make it more complete without
delegating to the driver.

Maybe we need to use bs->supported_write_flags, which is set by the
driver, instead of looking at the presence of callbacks.

Of course, in the general case, we also should make sure that graph
changes will be reflected in bs->supported_write_flags, but we already
fail to do this in raw-format, so I guess ignoring it for now is good
enough here, too...

Kevin

Re: [PATCH v4 4/8] ppc/e500: Use start-powered-off CPUState property

[Igor Mammedov]

On Tue, 18 Aug 2020 09:22:05 +0200
Philippe Mathieu-Daudé  wrote:

> On 8/18/20 5:33 AM, Thiago Jung Bauermann wrote:
> > Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
> > the start-powered-off property which makes cpu_common_reset() initialize it
> > to 1 in common code.
> > 
> > Also change creation of CPU object from cpu_create() to object_new() and
> > qdev_realize() because cpu_create() realizes the CPU and it's not possible
> > to set a property after the object is realized.
> > 
> > Signed-off-by: Thiago Jung Bauermann 
> > ---
> >  hw/ppc/e500.c | 19 +++
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> > index ab9884e315..0077aca74d 100644
> > --- a/hw/ppc/e500.c
> > +++ b/hw/ppc/e500.c
> > @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
> >  
> >  cpu_reset(cs);
> >  
> > -/* Secondary CPU starts in halted state for now. Needs to change when
> > -   implementing non-kernel boot. */
> > -cs->halted = 1;
> >  cs->exception_index = EXCP_HLT;
> >  }
> >  
> > @@ -864,8 +861,9 @@ void ppce500_init(MachineState *machine)
> >  PowerPCCPU *cpu;
> >  CPUState *cs;
> >  qemu_irq *input;
> > +Error *err = NULL;
> >  
> > -cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> > +cpu = POWERPC_CPU(object_new(machine->cpu_type));
> >  env = &cpu->env;
> >  cs = CPU(cpu);
> >  
> > @@ -897,6 +895,19 @@ void ppce500_init(MachineState *machine)
> >  } else {
> >  /* Secondary CPUs */
> >  qemu_register_reset(ppce500_cpu_reset_sec, cpu);
> > +
> > +/*
> > + * Secondary CPU starts in halted state for now. Needs to 
> > change
> > + * when implementing non-kernel boot.
> > + */
> > +object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> > + &error_abort);  
> 
> [*]
> 
> > +}
> > +
> > +if (!qdev_realize(DEVICE(cs), NULL, &err)) {
> > +error_report_err(err);
> > +object_unref(OBJECT(cs));
> > +exit(EXIT_FAILURE);
> >  }  
> 
> The last 4 lines are equivalent to:
> 
>qdev_realize(DEVICE(cs), NULL, &error_fatal)) {
> 
> This is also the preferred form, as we can not propagate errors
> from the machine_init() handler.
not exactly, it will leak refference, but we are dying anyways so who cares.

> 
> Since you use &error_abort in [*], maybe you want to use it here too.
> 
> >  }
> >  
> >   
> 
> 

Re: [PATCH 105/150] meson: convert hw/rdma

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs  |  1 -
>  hw/meson.build|  1 +
>  hw/rdma/Makefile.objs |  3 ---
>  hw/rdma/meson.build   | 10 ++
>  4 files changed, 11 insertions(+), 4 deletions(-)
>  delete mode 100644 hw/rdma/Makefile.objs
>  create mode 100644 hw/rdma/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 78860ae9c7..748a718b0f 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -19,7 +19,6 @@ devices-dirs-$(CONFIG_IPMI) += ipmi/
>  devices-dirs-y += isa/
>  devices-dirs-y += misc/
>  devices-dirs-y += net/
> -devices-dirs-y += rdma/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/meson.build b/hw/meson.build
> index 0bdd6a2613..f704e51361 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -6,6 +6,7 @@ subdir('pci')
>  subdir('pci-bridge')
>  subdir('pci-host')
>  subdir('pcmcia')
> +subdir('rdma')
>  subdir('rtc')
>  subdir('scsi')
>  subdir('sd')
> diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
> deleted file mode 100644
> index 819bb12a35..00
> --- a/hw/rdma/Makefile.objs
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -obj-$(CONFIG_VMW_PVRDMA) += rdma_utils.o rdma_backend.o rdma_rm.o rdma.o
> -obj-$(CONFIG_VMW_PVRDMA) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
> - vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
> diff --git a/hw/rdma/meson.build b/hw/rdma/meson.build
> new file mode 100644
> index 00..7325f40c32
> --- /dev/null
> +++ b/hw/rdma/meson.build
> @@ -0,0 +1,10 @@
> +specific_ss.add(when: 'CONFIG_VMW_PVRDMA', if_true: files(
> +  'rdma.c',
> +  'rdma_backend.c',
> +  'rdma_rm.c',
> +  'rdma_utils.c',
> +  'vmw/pvrdma_cmd.c',
> +  'vmw/pvrdma_dev_ring.c',
> +  'vmw/pvrdma_main.c',
> +  'vmw/pvrdma_qp_ops.c',
> +))
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 13/41] hvf: Add missing include

[Roman Bolshakov]

On Thu, Aug 13, 2020 at 06:25:57PM -0400, Eduardo Habkost wrote:
> The sysemu/accel.h header is needed for the ACCEL_CLASS_NAME
> macro.  This will be necessary to allow us to use OBJECT_DEFINE*()
> for TYPE_HVF_ACCEL.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/sysemu/hvf.h | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Roman Bolshakov 

Re: [PATCH 104/150] meson: convert hw/nvram

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs   | 1 -
>  hw/meson.build | 1 +
>  hw/nvram/Makefile.objs | 8 
>  hw/nvram/meson.build   | 9 +
>  4 files changed, 10 insertions(+), 9 deletions(-)
>  delete mode 100644 hw/nvram/Makefile.objs
>  create mode 100644 hw/nvram/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 20832841d2..78860ae9c7 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -20,7 +20,6 @@ devices-dirs-y += isa/
>  devices-dirs-y += misc/
>  devices-dirs-y += net/
>  devices-dirs-y += rdma/
> -devices-dirs-y += nvram/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/meson.build b/hw/meson.build
> index 53c347c395..0bdd6a2613 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -1,6 +1,7 @@
>  subdir('core')
>  subdir('mem')
>  subdir('nubus')
> +subdir('nvram')
>  subdir('pci')
>  subdir('pci-bridge')
>  subdir('pci-host')
> diff --git a/hw/nvram/Makefile.objs b/hw/nvram/Makefile.objs
> deleted file mode 100644
> index f3ad921382..00
> --- a/hw/nvram/Makefile.objs
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -common-obj-$(CONFIG_DS1225Y) += ds1225y.o
> -common-obj-$(CONFIG_NMC93XX_EEPROM) += eeprom93xx.o
> -common-obj-$(CONFIG_AT24C) += eeprom_at24c.o
> -common-obj-y += fw_cfg.o
> -common-obj-$(CONFIG_CHRP_NVRAM) += chrp_nvram.o
> -common-obj-$(CONFIG_MAC_NVRAM) += mac_nvram.o
> -common-obj-$(CONFIG_NRF51_SOC) += nrf51_nvm.o
> -obj-$(CONFIG_PSERIES) += spapr_nvram.o
> diff --git a/hw/nvram/meson.build b/hw/nvram/meson.build
> new file mode 100644
> index 00..ba214558ac
> --- /dev/null
> +++ b/hw/nvram/meson.build
> @@ -0,0 +1,9 @@
> +softmmu_ss.add(files('fw_cfg.c'))
> +softmmu_ss.add(when: 'CONFIG_CHRP_NVRAM', if_true: files('chrp_nvram.c'))
> +softmmu_ss.add(when: 'CONFIG_DS1225Y', if_true: files('ds1225y.c'))
> +softmmu_ss.add(when: 'CONFIG_NMC93XX_EEPROM', if_true: files('eeprom93xx.c'))
> +softmmu_ss.add(when: 'CONFIG_AT24C', if_true: files('eeprom_at24c.c'))
> +softmmu_ss.add(when: 'CONFIG_MAC_NVRAM', if_true: files('mac_nvram.c'))
> +softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_nvm.c'))
> +
> +specific_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr_nvram.c'))
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 103/150] meson: convert hw/pci

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs |  1 -
>  hw/meson.build   |  1 +
>  hw/pci/Makefile.objs | 14 --
>  hw/pci/meson.build   | 19 +++
>  4 files changed, 20 insertions(+), 15 deletions(-)
>  delete mode 100644 hw/pci/Makefile.objs
>  create mode 100644 hw/pci/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 3513040d4a..20832841d2 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -21,7 +21,6 @@ devices-dirs-y += misc/
>  devices-dirs-y += net/
>  devices-dirs-y += rdma/
>  devices-dirs-y += nvram/
> -devices-dirs-y += pci/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/meson.build b/hw/meson.build
> index a7301f21e8..53c347c395 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -1,6 +1,7 @@
>  subdir('core')
>  subdir('mem')
>  subdir('nubus')
> +subdir('pci')
>  subdir('pci-bridge')
>  subdir('pci-host')
>  subdir('pcmcia')
> diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> deleted file mode 100644
> index c78f2fb24b..00
> --- a/hw/pci/Makefile.objs
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -common-obj-$(CONFIG_PCI) += pci.o pci_bridge.o
> -common-obj-$(CONFIG_PCI) += msix.o msi.o
> -common-obj-$(CONFIG_PCI) += shpc.o
> -common-obj-$(CONFIG_PCI) += slotid_cap.o
> -common-obj-$(CONFIG_PCI) += pci_host.o
> -
> -# The functions in these modules can be used by devices too.  Since we
> -# allow plugging PCIe devices into PCI buses, include them even if
> -# CONFIG_PCI_EXPRESS=n.
> -common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o
> -common-obj-$(CONFIG_PCI_EXPRESS) += pcie_port.o pcie_host.o
> -
> -common-obj-$(call lnot,$(CONFIG_PCI)) += pci-stub.o
> -common-obj-$(CONFIG_ALL) += pci-stub.o
> diff --git a/hw/pci/meson.build b/hw/pci/meson.build
> new file mode 100644
> index 00..5c4bbac817
> --- /dev/null
> +++ b/hw/pci/meson.build
> @@ -0,0 +1,19 @@
> +pci_ss = ss.source_set()
> +pci_ss.add(files(
> +  'msi.c',
> +  'msix.c',
> +  'pci.c',
> +  'pci_bridge.c',
> +  'pci_host.c',
> +  'shpc.c',
> +  'slotid_cap.c'
> +))

I'd have written this simply as:

softmmu_ss.add(when: 'CONFIG_PCI', if_true: files(
  'msi.c',
  'msix.c',
  'pci.c',
  'pci_bridge.c',
  'pci_host.c',
  'shpc.c',
  'slotid_cap.c',
  # The functions in these modules can be used by devices too.  Since we
  # allow plugging PCIe devices into PCI buses, include them even if
  # CONFIG_PCI_EXPRESS=n.
  'pcie.c',
  'pcie_aer.c'
))
softmmu_ss.add(when: 'CONFIG_PCI_EXPRESS', if_true: files(
  'pcie_port.c',
  'pcie_host.c'
))

And keep the pci-stub.c lines.

Anyway:
Reviewed-by: Philippe Mathieu-Daudé 

> +# The functions in these modules can be used by devices too.  Since we
> +# allow plugging PCIe devices into PCI buses, include them even if
> +# CONFIG_PCI_EXPRESS=n.
> +pci_ss.add(files('pcie.c', 'pcie_aer.c'))
> +softmmu_ss.add(when: 'CONFIG_PCI_EXPRESS', if_true: files('pcie_port.c', 
> 'pcie_host.c'))
> +softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
> +
> +softmmu_ss.add(when: 'CONFIG_PCI', if_false: files('pci-stub.c'))
> +softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('pci-stub.c'))
> 

Re: [PATCH 098/150] meson: convert hw/sd

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs|  1 -
>  hw/meson.build  |  1 +
>  hw/sd/Makefile.objs | 12 
>  hw/sd/meson.build   | 12 
>  4 files changed, 13 insertions(+), 13 deletions(-)
>  delete mode 100644 hw/sd/Makefile.objs
>  create mode 100644 hw/sd/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index e50eb04635..6f9c0b7983 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -25,7 +25,6 @@ devices-dirs-y += pci/
>  devices-dirs-$(CONFIG_PCI) += pci-bridge/ pci-host/
>  devices-dirs-y += pcmcia/
>  devices-dirs-$(CONFIG_SCSI) += scsi/
> -devices-dirs-y += sd/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/meson.build b/hw/meson.build
> index 75644266f2..a73f4aebde 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -2,6 +2,7 @@ subdir('core')
>  subdir('mem')
>  subdir('nubus')
>  subdir('rtc')
> +subdir('sd')
>  subdir('semihosting')
>  subdir('smbios')
>  subdir('ssi')
> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
> deleted file mode 100644
> index 0d1df1721c..00
> --- a/hw/sd/Makefile.objs
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -common-obj-$(CONFIG_PL181) += pl181.o
> -common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
> -common-obj-$(CONFIG_SD) += sd.o core.o sdmmc-internal.o
> -common-obj-$(CONFIG_SDHCI) += sdhci.o
> -common-obj-$(CONFIG_SDHCI_PCI) += sdhci-pci.o
> -
> -common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-sdhost.o
> -common-obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
> -common-obj-$(CONFIG_OMAP) += omap_mmc.o
> -common-obj-$(CONFIG_PXA2XX) += pxa2xx_mmci.o
> -common-obj-$(CONFIG_RASPI) += bcm2835_sdhost.o
> -common-obj-$(CONFIG_ASPEED_SOC) += aspeed_sdhci.o
> diff --git a/hw/sd/meson.build b/hw/sd/meson.build
> new file mode 100644
> index 00..b43e59bd00
> --- /dev/null
> +++ b/hw/sd/meson.build
> @@ -0,0 +1,12 @@
> +softmmu_ss.add(when: 'CONFIG_PL181', if_true: files('pl181.c'))
> +softmmu_ss.add(when: 'CONFIG_SD', if_true: files('sd.c', 'core.c', 
> 'sdmmc-internal.c'))
> +softmmu_ss.add(when: 'CONFIG_SDHCI', if_true: files('sdhci.c'))
> +softmmu_ss.add(when: 'CONFIG_SDHCI_PCI', if_true: files('sdhci-pci.c'))
> +softmmu_ss.add(when: 'CONFIG_SSI_SD', if_true: files('ssi-sd.c'))
> +
> +softmmu_ss.add(when: 'CONFIG_MILKYMIST', if_true: 
> files('milkymist-memcard.c'))
> +softmmu_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_mmc.c'))
> +softmmu_ss.add(when: 'CONFIG_PXA2XX', if_true: files('pxa2xx_mmci.c'))
> +softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_sdhost.c'))
> +softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_sdhci.c'))
> +softmmu_ss.add(when: 'CONFIG_ALLWINNER_H3', if_true: 
> files('allwinner-sdhost.c'))
> 

Again, this one already has:
Reviewed-by: Philippe Mathieu-Daudé 

https://www.mail-archive.com/qemu-devel@nongnu.org/msg728253.html

Thanks.

Re: [PATCH 086/150] meson: convert hw/semihosting

[Philippe Mathieu-Daudé]

On 8/17/20 4:36 PM, Paolo Bonzini wrote:
> Signed-off-by: Marc-André Lureau 
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs | 1 -
>  hw/meson.build   | 1 +
>  hw/semihosting/Makefile.objs | 2 --
>  hw/semihosting/meson.build   | 4 
>  4 files changed, 5 insertions(+), 3 deletions(-)
>  delete mode 100644 hw/semihosting/Makefile.objs
>  create mode 100644 hw/semihosting/meson.build

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 100/150] meson: convert hw/pcmcia

[Philippe Mathieu-Daudé]

On 8/17/20 4:40 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs| 1 -
>  hw/meson.build  | 1 +
>  hw/pcmcia/Makefile.objs | 2 --
>  hw/pcmcia/meson.build   | 2 ++
>  4 files changed, 3 insertions(+), 3 deletions(-)
>  delete mode 100644 hw/pcmcia/Makefile.objs
>  create mode 100644 hw/pcmcia/meson.build

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 087/150] meson: convert hw/nubus

[Philippe Mathieu-Daudé]

On 8/17/20 4:36 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs   | 1 -
>  hw/meson.build | 1 +
>  hw/nubus/Makefile.objs | 4 
>  hw/nubus/meson.build   | 7 +++
>  4 files changed, 8 insertions(+), 5 deletions(-)
>  delete mode 100644 hw/nubus/Makefile.objs
>  create mode 100644 hw/nubus/meson.build

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 088/150] meson: convert hw/smbios

[Philippe Mathieu-Daudé]

On 8/17/20 4:36 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs|  1 -
>  hw/meson.build  |  1 +
>  hw/smbios/Makefile.objs | 10 --
>  hw/smbios/meson.build   | 13 +
>  4 files changed, 14 insertions(+), 11 deletions(-)
>  delete mode 100644 hw/smbios/Makefile.objs
>  create mode 100644 hw/smbios/meson.build

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH V2] Introduce a new flag for piix to disable root bus PCI hotplug

[Ani Sinha]

> On Aug 18, 2020, at 3:49 PM, Ani Sinha  wrote:
> 
> 
> This is not a workaround, this is a feature we need badly. We have discussed 
> this at length. Igor suggested we have one additional global flag to disable 
> all pci hotplug. But then that leads to strange case when pci hotplug on the 
> bridges are enabled (we have a separate flag for that). Hence, I think we can 
> simply add another flag to disable hotplug just for the pci root bus. 

Relevant thread: 
https://www.mail-archive.com/qemu-devel@nongnu.org/msg729072.html


>> On Tue, Aug 18, 2020 at 3:46 PM Philippe Mathieu-Daudé  
>> wrote:
>> Hi,
>> 
>> 
>> 
>> On 8/18/20 11:54 AM, Ani Sinha wrote:
>> 
>> > Igor etc, I just tested this patch using a Windows 2012 R2 image and
>> 
>> > it seems to be working. Any feedbacks on this patch?
>> 
>> > 
>> 
>> > On Tue, Aug 11, 2020 at 6:42 PM Ani Sinha  wrote:
>> 
>> >>
>> 
>> >> We introduce a new global flag for PIIX with which we can turn on or off 
>> >> PCI
>> 
>> >> device hotplug on the root bus. This flag can be used to prevent all PCI
>> 
>> >> devices from getting hotplugged or unplugged from the root PCI bus.
>> 
>> >>
>> 
>> >> Signed-off-by: Ani Sinha 
>> 
>> >> ---
>> 
>> >>  hw/acpi/piix4.c  |  3 +++
>> 
>> >>  hw/i386/acpi-build.c | 20 
>> 
>> >>  2 files changed, 19 insertions(+), 4 deletions(-)
>> 
>> >>
>> 
>> >> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>> 
>> >> index 26bac4f..94ec35a 100644
>> 
>> >> --- a/hw/acpi/piix4.c
>> 
>> >> +++ b/hw/acpi/piix4.c
>> 
>> >> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
>> 
>> >>
>> 
>> >>  AcpiPciHpState acpi_pci_hotplug;
>> 
>> >>  bool use_acpi_hotplug_bridge;
>> 
>> >> +bool use_acpi_root_pci_hotplug;
>> 
>> >>
>> 
>> >>  uint8_t disable_s3;
>> 
>> >>  uint8_t disable_s4;
>> 
>> >> @@ -635,6 +636,8 @@ static Property piix4_pm_properties[] = {
>> 
>> >>  DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
>> 
>> >>  DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", 
>> >> PIIX4PMState,
>> 
>> >>   use_acpi_hotplug_bridge, true),
>> 
>> >> +DEFINE_PROP_BOOL("acpi-root-pci-hotplug", PIIX4PMState,
>> 
>> >> + use_acpi_root_pci_hotplug, true),
>> 
>> 
>> 
>> From what I understood here, this file is already pretty twisted
>> 
>> and Igor doesn't want more workarounds:
>> 
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg690564.html
>> 
>> 
>> 
>> ¯\_(ツ)_/¯
>> 
>> 
>> 
>> >>  DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>> 
>> >>   acpi_memory_hotplug.is_enabled, true),
>> 
>> >>  DEFINE_PROP_END_OF_LIST(),
>> 
>> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> 
>> >> index b7bcbbb..a82e5c1 100644
>> 
>> >> --- a/hw/i386/acpi-build.c
>> 
>> >> +++ b/hw/i386/acpi-build.c
>> 
>> >> @@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
>> 
>> >>  bool s3_disabled;
>> 
>> >>  bool s4_disabled;
>> 
>> >>  bool pcihp_bridge_en;
>> 
>> >> +bool pcihp_root_en;
>> 
>> >>  uint8_t s4_val;
>> 
>> >>  AcpiFadtData fadt;
>> 
>> >>  uint16_t cpu_hp_io_base;
>> 
>> >> @@ -245,6 +246,9 @@ static void acpi_get_pm_info(MachineState *machine, 
>> >> AcpiPmInfo *pm)
>> 
>> >>  pm->pcihp_bridge_en =
>> 
>> >>  object_property_get_bool(obj, 
>> >> "acpi-pci-hotplug-with-bridge-support",
>> 
>> >>   NULL);
>> 
>> >> +pm->pcihp_root_en =
>> 
>> >> +object_property_get_bool(obj, "acpi-root-pci-hotplug", NULL);
>> 
>> >> +
>> 
>> >>  }
>> 
>> >>
>> 
>> >>  static void acpi_get_misc_info(AcpiMiscInfo *info)
>> 
>> >> @@ -337,12 +341,15 @@ static void build_append_pcihp_notify_entry(Aml 
>> >> *method, int slot)
>> 
>> >>  }
>> 
>> >>
>> 
>> >>  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>> 
>> >> - bool pcihp_bridge_en)
>> 
>> >> + bool pcihp_bridge_en,
>> 
>> >> + bool pcihp_root_en)
>> 
>> >>  {
>> 
>> >>  Aml *dev, *notify_method = NULL, *method;
>> 
>> >>  QObject *bsel;
>> 
>> >>  PCIBus *sec;
>> 
>> >>  int i;
>> 
>> >> +bool root_bus = pci_bus_is_root(bus);
>> 
>> >> +bool root_pcihp_disabled = (root_bus && !pcihp_root_en);
>> 
>> >>
>> 
>> >>  bsel = object_property_get_qobject(OBJECT(bus), 
>> >> ACPI_PCIHP_PROP_BSEL, NULL);
>> 
>> >>  if (bsel) {
>> 
>> >> @@ -361,6 +368,9 @@ static void build_append_pci_bus_devices(Aml 
>> >> *parent_scope, PCIBus *bus,
>> 
>> >>  bool bridge_in_acpi;
>> 
>> >>
>> 
>> >>  if (!pdev) {
>> 
>> >> +/* skip if pci hotplug for the root bus is disabled */
>> 
>> >> +if (root_pcihp_disabled)
>> 
>> >> +continue;
>> 
>> >>  if (bsel) { /* add hotplug slots for non present devices */
>> 
>> >>  dev = aml_device("S%.0

Re: [PATCH 090/150] meson: convert hw/watchdog

[Philippe Mathieu-Daudé]

On 8/17/20 4:36 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs  | 1 -
>  hw/meson.build| 1 +
>  hw/watchdog/Makefile.objs | 7 ---
>  hw/watchdog/meson.build   | 7 +++
>  4 files changed, 8 insertions(+), 8 deletions(-)
>  delete mode 100644 hw/watchdog/Makefile.objs
>  create mode 100644 hw/watchdog/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index df72b25a64..9ebd13d7cd 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -33,7 +33,6 @@ devices-dirs-$(CONFIG_TPM) += tpm/
>  devices-dirs-y += usb/
>  devices-dirs-$(CONFIG_VFIO) += vfio/
>  devices-dirs-y += virtio/
> -devices-dirs-y += watchdog/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/meson.build b/hw/meson.build
> index 542d675618..f2209d900c 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -3,4 +3,5 @@ subdir('mem')
>  subdir('nubus')
>  subdir('semihosting')
>  subdir('smbios')
> +subdir('watchdog')
>  subdir('xen')
> diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs
> deleted file mode 100644
> index 631b711d86..00
> --- a/hw/watchdog/Makefile.objs
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -common-obj-y += watchdog.o
> -common-obj-$(CONFIG_CMSDK_APB_WATCHDOG) += cmsdk-apb-watchdog.o
> -common-obj-$(CONFIG_WDT_IB6300ESB) += wdt_i6300esb.o
> -common-obj-$(CONFIG_WDT_IB700) += wdt_ib700.o
> -common-obj-$(CONFIG_WDT_DIAG288) += wdt_diag288.o
> -common-obj-$(CONFIG_ASPEED_SOC) += wdt_aspeed.o
> -common-obj-$(CONFIG_WDT_IMX2) += wdt_imx2.o
> diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
> new file mode 100644
> index 00..9b8725e642
> --- /dev/null
> +++ b/hw/watchdog/meson.build
> @@ -0,0 +1,7 @@
> +softmmu_ss.add(files('watchdog.c'))
> +softmmu_ss.add(when: 'CONFIG_CMSDK_APB_WATCHDOG', if_true: 
> files('cmsdk-apb-watchdog.c'))
> +softmmu_ss.add(when: 'CONFIG_WDT_IB6300ESB', if_true: 
> files('wdt_i6300esb.c'))
> +softmmu_ss.add(when: 'CONFIG_WDT_IB700', if_true: files('wdt_ib700.c'))
> +softmmu_ss.add(when: 'CONFIG_WDT_DIAG288', if_true: files('wdt_diag288.c'))
> +softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('wdt_aspeed.c'))
> +softmmu_ss.add(when: 'CONFIG_WDT_IMX2', if_true: files('wdt_imx2.c'))

This one is not sorted alphabetically!

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 092/150] meson: convert hw/vfio

[Philippe Mathieu-Daudé]

On 8/17/20 4:36 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs  |  1 -
>  hw/meson.build|  1 +
>  hw/vfio/Makefile.objs |  8 
>  hw/vfio/meson.build   | 18 ++
>  4 files changed, 19 insertions(+), 9 deletions(-)
>  delete mode 100644 hw/vfio/Makefile.objs
>  create mode 100644 hw/vfio/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index a1cfb99334..f6e08f6660 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -31,7 +31,6 @@ devices-dirs-y += ssi/
>  devices-dirs-y += timer/
>  devices-dirs-$(CONFIG_TPM) += tpm/
>  devices-dirs-y += usb/
> -devices-dirs-$(CONFIG_VFIO) += vfio/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/meson.build b/hw/meson.build
> index 8338fc4408..4dafc8a08e 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -3,6 +3,7 @@ subdir('mem')
>  subdir('nubus')
>  subdir('semihosting')
>  subdir('smbios')
> +subdir('vfio')
>  subdir('virtio')
>  subdir('watchdog')
>  subdir('xen')
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> deleted file mode 100644
> index 9bb1c09e84..00
> --- a/hw/vfio/Makefile.objs
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -obj-y += common.o spapr.o
> -obj-$(CONFIG_VFIO_PCI) += pci.o pci-quirks.o display.o
> -obj-$(CONFIG_VFIO_CCW) += ccw.o
> -obj-$(CONFIG_VFIO_PLATFORM) += platform.o
> -obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
> -obj-$(CONFIG_VFIO_AMD_XGBE) += amd-xgbe.o
> -obj-$(CONFIG_VFIO_AP) += ap.o
> -obj-$(CONFIG_VFIO_IGD) += igd.o
> diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
> new file mode 100644
> index 00..37efa74018
> --- /dev/null
> +++ b/hw/vfio/meson.build
> @@ -0,0 +1,18 @@
> +vfio_ss = ss.source_set()
> +vfio_ss.add(files(
> +  'common.c',
> +  'spapr.c',
> +))
> +vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
> +  'display.c',
> +  'pci-quirks.c',
> +  'pci.c',
> +))
> +vfio_ss.add(when: 'CONFIG_VFIO_CCW', if_true: files('ccw.c'))
> +vfio_ss.add(when: 'CONFIG_VFIO_PLATFORM', if_true: files('platform.c'))
> +vfio_ss.add(when: 'CONFIG_VFIO_XGMAC', if_true: files('calxeda-xgmac.c'))
> +vfio_ss.add(when: 'CONFIG_VFIO_AMD_XGBE', if_true: files('amd-xgbe.c'))
> +vfio_ss.add(when: 'CONFIG_VFIO_AP', if_true: files('ap.c'))
> +vfio_ss.add(when: 'CONFIG_VFIO_IGD', if_true: files('igd.c'))

Not sorted :)

Reviewed-by: Philippe Mathieu-Daudé 

> +
> +specific_ss.add_all(when: 'CONFIG_VFIO', if_true: vfio_ss)
> 

[PATCH v5] qapi/opts-visitor: Added missing fallthrough annotations

[Rohit Shinde]

Added fallthrough comment on line 270 to prevent the compiler from
throwing an error while compiling with the -Wimplicit-fallthrough flag

Signed-off-by: Rohit Shinde 
---
 qapi/opts-visitor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 7781c23a42..3422ff265e 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -266,6 +266,7 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
 }
 ov->list_mode = LM_IN_PROGRESS;
 /* range has been completed, fall through in order to pop option */
+/* fallthrough */
 
 case LM_IN_PROGRESS: {
 const QemuOpt *opt;
-- 
2.25.1

Re: [PATCH 094/150] meson: convert hw/tpm

[Philippe Mathieu-Daudé]

Hi Paolo, Marc-André,

On 8/17/20 4:36 PM, Paolo Bonzini wrote:
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/Makefile.objs | 1 -
>  hw/meson.build   | 1 +
>  hw/tpm/Makefile.objs | 6 --
>  hw/tpm/meson.build   | 8 
>  4 files changed, 9 insertions(+), 7 deletions(-)
>  delete mode 100644 hw/tpm/Makefile.objs
>  create mode 100644 hw/tpm/meson.build
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index c2619bbb50..f0e5728d13 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -29,7 +29,6 @@ devices-dirs-$(CONFIG_SCSI) += scsi/
>  devices-dirs-y += sd/
>  devices-dirs-y += ssi/
>  devices-dirs-y += timer/
> -devices-dirs-$(CONFIG_TPM) += tpm/

I suppose you simplified this line due to Kconfig selecting
CONFIG_TPM automatically. Maybe worth mentioning in the
description.

>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> diff --git a/hw/meson.build b/hw/meson.build
> index 89bd6adb70..2ddf6bad33 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -3,6 +3,7 @@ subdir('mem')
>  subdir('nubus')
>  subdir('semihosting')
>  subdir('smbios')
> +subdir('tpm')
>  subdir('usb')
>  subdir('vfio')
>  subdir('virtio')
> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
> deleted file mode 100644
> index 6fc05be67c..00
> --- a/hw/tpm/Makefile.objs
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -obj-$(call lor,$(CONFIG_TPM_TIS),$(CONFIG_TPM_CRB)) += tpm_ppi.o
> -common-obj-$(CONFIG_TPM_TIS_ISA) += tpm_tis_isa.o
> -common-obj-$(CONFIG_TPM_TIS_SYSBUS) += tpm_tis_sysbus.o
> -common-obj-$(CONFIG_TPM_TIS) += tpm_tis_common.o
> -common-obj-$(CONFIG_TPM_CRB) += tpm_crb.o
> -obj-$(CONFIG_TPM_SPAPR) += tpm_spapr.o
> diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> new file mode 100644
> index 00..1c68d81d6a
> --- /dev/null
> +++ b/hw/tpm/meson.build
> @@ -0,0 +1,8 @@
> +softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_tis_common.c'))
> +softmmu_ss.add(when: 'CONFIG_TPM_TIS_ISA', if_true: files('tpm_tis_isa.c'))
> +softmmu_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: 
> files('tpm_tis_sysbus.c'))
> +softmmu_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
> +
> +specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_TIS'], if_true: 
> files('tpm_ppi.c'))
> +specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_CRB'], if_true: 
> files('tpm_ppi.c'))

I don't understand the 'CONFIG_SOFTMMU' use.

Since specific_ss is a source_set(), why not simply use:

 specific_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_ppi.c'))
 specific_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_ppi.c'))

Thanks,

Phil.

> +specific_ss.add(when: 'CONFIG_TPM_SPAPR', if_true: files('tpm_spapr.c'))
> 

Re: [PATCH v4 4/8] ppc/e500: Use start-powered-off CPUState property

[Igor Mammedov]

On Tue, 18 Aug 2020 00:33:19 -0300
Thiago Jung Bauermann  wrote:

[...]

> Also change creation of CPU object from cpu_create() to object_new() and
> qdev_realize() because cpu_create() realizes the CPU and it's not possible
> to set a property after the object is realized.

cpu_create was introduced to remove code duplication in simple cases where
we do not need to set properties on created cpu.

returning back to manual object_new + realize() is fine if it 's only
small set of of boards. If it's tree-wide change then that would bring
back all code duplication that cpu_create() got rid of.

An alternative way is to use 'hotplug' callbacks to let board set
additional properties before cpu's realize is called.

example:
  hw/ppc/spapr.c:
   spapr_machine_class_init()
 mc->get_hotplug_handler = spapr_get_hotplug_handler;   
  
 hc->pre_plug = spapr_machine_device_pre_plug; 
   ...
   static const TypeInfo spapr_machine_info = {
   ...
{ TYPE_HOTPLUG_HANDLER },

that might work in generic case if it is put into generic machine code,
and existing users of mc->get_hotplug_handler/hc->pre_plug were taken care of.
In which case board would only need to set MachineClass:cpu-start-powered-of
to gate property setting.

 
> Signed-off-by: Thiago Jung Bauermann 
> ---
>  hw/ppc/e500.c | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index ab9884e315..0077aca74d 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
>  
>  cpu_reset(cs);
>  
> -/* Secondary CPU starts in halted state for now. Needs to change when
> -   implementing non-kernel boot. */
> -cs->halted = 1;
>  cs->exception_index = EXCP_HLT;
>  }
>  
> @@ -864,8 +861,9 @@ void ppce500_init(MachineState *machine)
>  PowerPCCPU *cpu;
>  CPUState *cs;
>  qemu_irq *input;
> +Error *err = NULL;
>  
> -cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> +cpu = POWERPC_CPU(object_new(machine->cpu_type));
>  env = &cpu->env;
>  cs = CPU(cpu);
>  
> @@ -897,6 +895,19 @@ void ppce500_init(MachineState *machine)
>  } else {
>  /* Secondary CPUs */
>  qemu_register_reset(ppce500_cpu_reset_sec, cpu);
> +
> +/*
> + * Secondary CPU starts in halted state for now. Needs to change
> + * when implementing non-kernel boot.
> + */
> +object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);
> +}
> +
> +if (!qdev_realize(DEVICE(cs), NULL, &err)) {
> +error_report_err(err);
> +object_unref(OBJECT(cs));
> +exit(EXIT_FAILURE);
>  }

btw:
board leaks cpu reference (from cpu_create()/object_new()) since qdev_realize()
adds it's own and the caller of object_new() is suposed to free the original 
one.

in this case qdev_realize_and_unref() fits nicely.


>  }
>  
> 

Re: [PATCH v4 5/8] mips/cps: Use start-powered-off CPUState property

[Philippe Mathieu-Daudé]

On 8/18/20 9:26 AM, Philippe Mathieu-Daudé wrote:
> On 8/18/20 5:33 AM, Thiago Jung Bauermann wrote:
>> Instead of setting CPUState::halted to 1 in main_cpu_reset(), use the
>> start-powered-off property which makes cpu_common_reset() initialize it
>> to 1 in common code.
>>
>> Also change creation of CPU object from cpu_create() to object_new() and
>> qdev_realize() because cpu_create() realizes the CPU and it's not possible to
>> set a property after the object is realized.
>>
>> Signed-off-by: Thiago Jung Bauermann 
>> ---
>>  hw/mips/cps.c | 16 
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
>> index 615e1a1ad2..be85357dc0 100644
>> --- a/hw/mips/cps.c
>> +++ b/hw/mips/cps.c
>> @@ -52,9 +52,6 @@ static void main_cpu_reset(void *opaque)
>>  CPUState *cs = CPU(cpu);
>>  
>>  cpu_reset(cs);
>> -
>> -/* All VPs are halted on reset. Leave powering up to CPC. */
>> -cs->halted = 1;
>>  }
>>  
>>  static bool cpu_mips_itu_supported(CPUMIPSState *env)
>> @@ -76,7 +73,9 @@ static void mips_cps_realize(DeviceState *dev, Error 
>> **errp)
>>  bool saar_present = false;
>>  
>>  for (i = 0; i < s->num_vp; i++) {
>> -cpu = MIPS_CPU(cpu_create(s->cpu_type));
>> +Error *err = NULL;
>> +
>> +cpu = MIPS_CPU(object_new(s->cpu_type));
>>  
>>  /* Init internal devices */
>>  cpu_mips_irq_init_cpu(cpu);
>> @@ -89,7 +88,16 @@ static void mips_cps_realize(DeviceState *dev, Error 
>> **errp)
>>  env->itc_tag = mips_itu_get_tag_region(&s->itu);
>>  env->itu = &s->itu;
>>  }
>> +/* All VPs are halted on reset. Leave powering up to CPC. */
>> +object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
>> + &error_abort);
>>  qemu_register_reset(main_cpu_reset, cpu);
>> +
>> +if (!qdev_realize(DEVICE(cpu), NULL, &err)) {
>> +error_report_err(err);
>> +object_unref(OBJECT(cpu));
>> +exit(EXIT_FAILURE);
>> +}
> 
> Here errp is available, so we can propagate the error to the caller:
> 
>if (!qdev_realize(DEVICE(cpu), NULL, errp)) {

Igor corrected me in the previous patch, to avoid leaking the
reference this snippet misses:

 object_unref(OBJECT(cpu));

>return;
>}
> 
> For example in hw/mips/boston.c:
> 
> object_initialize_child(OBJECT(machine), "cps", &s->cps, TYPE_MIPS_CPS);
> object_property_set_str(OBJECT(&s->cps), "cpu-type", machine->cpu_type,
> &error_fatal);
> object_property_set_int(OBJECT(&s->cps), "num-vp", machine->smp.cpus,
> &error_fatal);
> sysbus_realize(SYS_BUS_DEVICE(&s->cps), &error_fatal);
> 
> This will be propagated here ---^
> 
>>  }
>>  
>>  cpu = MIPS_CPU(first_cpu);
>>

Re: [PATCH v4 5/8] mips/cps: Use start-powered-off CPUState property

[Philippe Mathieu-Daudé]

On 8/18/20 1:03 PM, Philippe Mathieu-Daudé wrote:
> On 8/18/20 9:26 AM, Philippe Mathieu-Daudé wrote:
>> On 8/18/20 5:33 AM, Thiago Jung Bauermann wrote:
>>> Instead of setting CPUState::halted to 1 in main_cpu_reset(), use the
>>> start-powered-off property which makes cpu_common_reset() initialize it
>>> to 1 in common code.
>>>
>>> Also change creation of CPU object from cpu_create() to object_new() and
>>> qdev_realize() because cpu_create() realizes the CPU and it's not possible 
>>> to
>>> set a property after the object is realized.
>>>
>>> Signed-off-by: Thiago Jung Bauermann 
>>> ---
>>>  hw/mips/cps.c | 16 
>>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
>>> index 615e1a1ad2..be85357dc0 100644
>>> --- a/hw/mips/cps.c
>>> +++ b/hw/mips/cps.c
>>> @@ -52,9 +52,6 @@ static void main_cpu_reset(void *opaque)
>>>  CPUState *cs = CPU(cpu);
>>>  
>>>  cpu_reset(cs);
>>> -
>>> -/* All VPs are halted on reset. Leave powering up to CPC. */
>>> -cs->halted = 1;
>>>  }
>>>  
>>>  static bool cpu_mips_itu_supported(CPUMIPSState *env)
>>> @@ -76,7 +73,9 @@ static void mips_cps_realize(DeviceState *dev, Error 
>>> **errp)
>>>  bool saar_present = false;
>>>  
>>>  for (i = 0; i < s->num_vp; i++) {
>>> -cpu = MIPS_CPU(cpu_create(s->cpu_type));
>>> +Error *err = NULL;
>>> +
>>> +cpu = MIPS_CPU(object_new(s->cpu_type));
>>>  
>>>  /* Init internal devices */
>>>  cpu_mips_irq_init_cpu(cpu);
>>> @@ -89,7 +88,16 @@ static void mips_cps_realize(DeviceState *dev, Error 
>>> **errp)
>>>  env->itc_tag = mips_itu_get_tag_region(&s->itu);
>>>  env->itu = &s->itu;
>>>  }
>>> +/* All VPs are halted on reset. Leave powering up to CPC. */
>>> +object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
>>> + &error_abort);
>>>  qemu_register_reset(main_cpu_reset, cpu);
>>> +
>>> +if (!qdev_realize(DEVICE(cpu), NULL, &err)) {
>>> +error_report_err(err);
>>> +object_unref(OBJECT(cpu));
>>> +exit(EXIT_FAILURE);
>>> +}
>>
>> Here errp is available, so we can propagate the error to the caller:
>>
>>if (!qdev_realize(DEVICE(cpu), NULL, errp)) {
> 
> Igor corrected me in the previous patch, to avoid leaking the
> reference this snippet misses:
> 
>  object_unref(OBJECT(cpu));

Well this is simply:

  if (!qdev_realize_and_unref(DEVICE(cpu), NULL, errp)) {
> 
>>return;
>>}
>>
>> For example in hw/mips/boston.c:
>>
>> object_initialize_child(OBJECT(machine), "cps", &s->cps, TYPE_MIPS_CPS);
>> object_property_set_str(OBJECT(&s->cps), "cpu-type", machine->cpu_type,
>> &error_fatal);
>> object_property_set_int(OBJECT(&s->cps), "num-vp", machine->smp.cpus,
>> &error_fatal);
>> sysbus_realize(SYS_BUS_DEVICE(&s->cps), &error_fatal);
>>
>> This will be propagated here ---^
>>
>>>  }
>>>  
>>>  cpu = MIPS_CPU(first_cpu);
>>>
> 

Re: [PATCH v3 000/150] Meson integration for 5.2

[Philippe Mathieu-Daudé]

On 8/18/20 12:25 PM, Paolo Bonzini wrote:
> On 18/08/20 11:59, Cornelia Huck wrote:
>> On Mon, 17 Aug 2020 16:34:53 +0200
>> Paolo Bonzini  wrote:
>>
>>> - Simplified/rewrote handling of the modules variable [Howard]
>>> - Fixed access to gdb-xml files [Howard]
>>> - Fixed cross-compilation failure due to mingw prefix [Howard]
>>> - Fixed SDL2 detection on cross compiles [Howard]
>>> - Fixed sub-makefiles that include config-host.mak
>>>
>>> You can find it at the usual place, https://gitlab.com/bonzini/qemu.git
>>> branch meson-poc-next.
>>
>> Builds (on x86 & s390x), runs (this time with the right binary ;), and
>> passes make check, make check-tcg, and kvm unit tests (s390x) for me.
>>
> 
> Great, I'll do the last tests here and send a pull request.

What about the 2 patchew errors?

../src/meson.build:227:2: ERROR: 'sdl2' is not a config-tool dependency

ERROR: meson setup failed



../src/meson.build:547:0: ERROR: Program 'scripts/tracetool.py' not found

ERROR: meson setup failed

[RFC PATCH 4/9] block/curl: Perform IO in fixed size chunks

[David Edmondson]

Do all IO requests to the remote server in 256kB chunks.

Signed-off-by: David Edmondson 
---
 block/curl.c   | 151 -
 block/trace-events |   2 +
 2 files changed, 109 insertions(+), 44 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index d2f4de46c9..cfc518efda 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -78,6 +78,14 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_BLOCK_OPT_SSLVERIFY_DEFAULT true
 #define CURL_BLOCK_OPT_TIMEOUT_DEFAULT 5
 
+/* Must be a non-zero power of 2. */
+#define CURL_BLOCK_SIZE (256 * 1024)
+
+/* Align "n" to the start of the containing block. */
+#define CURL_BLOCK_ALIGN(n) ((n) & ~(CURL_BLOCK_SIZE - 1))
+/* The offset of "n" within its' block. */
+#define CURL_BLOCK_OFFSET(n) ((n) & (CURL_BLOCK_SIZE - 1))
+
 struct BDRVCURLState;
 struct CURLState;
 
@@ -86,11 +94,18 @@ static bool libcurl_initialized;
 typedef struct CURLAIOCB {
 Coroutine *co;
 QEMUIOVector *qiov;
+uint64_t qiov_offset; /* Offset in qiov to place data. */
 
 uint64_t offset;
 uint64_t bytes;
 int ret;
 
+/*
+ * start and end indicate the subset of the surrounding
+ * CURL_BLOCK_SIZE sized block that is the subject of this
+ * IOCB. They are offsets from the beginning of the underlying
+ * buffer.
+ */
 size_t start;
 size_t end;
 } CURLAIOCB;
@@ -110,7 +125,6 @@ typedef struct CURLState
 char *orig_buf;
 uint64_t buf_start;
 size_t buf_off;
-size_t buf_len;
 char range[128];
 char errmsg[CURL_ERROR_SIZE];
 char in_use;
@@ -259,11 +273,11 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t 
nmemb, void *opaque)
 goto read_end;
 }
 
-if (s->buf_off >= s->buf_len) {
+if (s->buf_off >= CURL_BLOCK_SIZE) {
 /* buffer full, read nothing */
 goto read_end;
 }
-realsize = MIN(realsize, s->buf_len - s->buf_off);
+realsize = MIN(realsize, CURL_BLOCK_SIZE - s->buf_off);
 memcpy(s->orig_buf + s->buf_off, ptr, realsize);
 s->buf_off += realsize;
 
@@ -281,35 +295,44 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t 
start, uint64_t len,
 uint64_t clamped_end = MIN(end, s->len);
 uint64_t clamped_len = clamped_end - start;
 
-for (i=0; istates[i];
-uint64_t buf_end = (state->buf_start + state->buf_off);
-uint64_t buf_fend = (state->buf_start + state->buf_len);
+/* The end of the currently valid data. */
+uint64_t buf_end = state->buf_start + state->buf_off;
+/* The end of the valid data when the IO completes. */
+uint64_t buf_fend = state->buf_start + CURL_BLOCK_SIZE;
 
 if (!state->orig_buf)
 continue;
 if (!state->buf_off)
 continue;
 
-// Does the existing buffer cover our section?
+/*
+ * Does the existing buffer cover our section?
+ */
 if ((start >= state->buf_start) &&
 (start <= buf_end) &&
 (clamped_end >= state->buf_start) &&
 (clamped_end <= buf_end))
 {
-char *buf = state->orig_buf + (start - state->buf_start);
+char *buf = state->orig_buf + CURL_BLOCK_OFFSET(start);
 
 trace_curl_pending_hit(qemu_coroutine_self(),
start, len);
-qemu_iovec_from_buf(acb->qiov, 0, buf, clamped_len);
+qemu_iovec_from_buf(acb->qiov, acb->qiov_offset, buf, clamped_len);
 if (clamped_len < len) {
-qemu_iovec_memset(acb->qiov, clamped_len, 0, len - 
clamped_len);
+qemu_iovec_memset(acb->qiov, acb->qiov_offset + clamped_len,
+  0, len - clamped_len);
 }
 acb->ret = 0;
 return true;
 }
 
-// Wait for unfinished chunks
+/*
+ * If an in-progress IO will provide the required data, wait
+ * for it to complete - the initiator will complete this
+ * aiocb.
+ */
 if (state->in_use &&
 (start >= state->buf_start) &&
 (start <= buf_fend) &&
@@ -320,10 +343,10 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t 
start, uint64_t len,
 
 trace_curl_pending_piggyback(qemu_coroutine_self(),
  start, len);
-acb->start = start - state->buf_start;
+acb->start = CURL_BLOCK_OFFSET(start);
 acb->end = acb->start + clamped_len;
 
-for (j=0; jacb[j]) {
 state->acb[j] = acb;
 return true;
@@ -377,7 +400,7 @@ static void curl_multi_check_completion(BDRVCURLState *s)
 for (i = 0; i < CURL_NUM_ACB; i++) {
 CURLAIOCB *acb = state->acb[i];
 
-if (acb == NULL) {
+if (!acb) {
 continue;
 }
 
@@ -385,14 +408,15 @@ stati

[RFC PATCH 1/9] block/curl: Add an 'offset' parameter, affecting all range requests

[David Edmondson]

A new 'offset' parameter affects all range requests that are sent to
the remote server. The value, in bytes, is simply added to any byte
offset values passed in to the driver.

Signed-off-by: David Edmondson 
---
 block/curl.c  | 12 +++-
 docs/system/device-url-syntax.rst.inc |  4 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 4f907c47be..32ec760f66 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -74,6 +74,7 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
 #define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
 #define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
+#define CURL_BLOCK_OPT_OFFSET "offset"
 
 #define CURL_BLOCK_OPT_READAHEAD_DEFAULT (256 * 1024)
 #define CURL_BLOCK_OPT_SSLVERIFY_DEFAULT true
@@ -135,6 +136,7 @@ typedef struct BDRVCURLState {
 char *password;
 char *proxyusername;
 char *proxypassword;
+size_t offset;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
@@ -658,6 +660,11 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "ID of secret used as password for HTTP proxy auth",
 },
+{
+.name = CURL_BLOCK_OPT_OFFSET,
+.type = QEMU_OPT_SIZE,
+.help = "Offset into underlying content"
+},
 { /* end of list */ }
 },
 };
@@ -769,6 +776,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 }
 
+s->offset = qemu_opt_get_size(opts, CURL_BLOCK_OPT_OFFSET, 0);
+
 trace_curl_open(file);
 qemu_co_queue_init(&s->free_state_waitq);
 s->aio_context = bdrv_get_aio_context(bs);
@@ -899,7 +908,8 @@ static void curl_setup_preadv(BlockDriverState *bs, 
CURLAIOCB *acb)
 }
 state->acb[0] = acb;
 
-snprintf(state->range, 127, "%" PRIu64 "-%" PRIu64, start, end);
+snprintf(state->range, 127, "%" PRIu64 "-%" PRIu64,
+ s->offset + start, s->offset + end);
 trace_curl_setup_preadv(acb->bytes, start, state->range);
 curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
 
diff --git a/docs/system/device-url-syntax.rst.inc 
b/docs/system/device-url-syntax.rst.inc
index 88d7a372a7..33f1ddfe6d 100644
--- a/docs/system/device-url-syntax.rst.inc
+++ b/docs/system/device-url-syntax.rst.inc
@@ -197,6 +197,10 @@ These are specified using a special URL syntax.
   get the size of the image to be downloaded. If not set, the
   default timeout of 5 seconds is used.
 
+   ``offset``
+  Add an offset, in bytes, to all range requests sent to the
+  remote server.
+
Note that when passing options to qemu explicitly, ``driver`` is the
value of .
 
-- 
2.27.0

[RFC PATCH 7/9] block/curl: Allow the user to control the number of cache blocks

[David Edmondson]

Rather than using a fixed number, allow the user to specify the number
of cache blocks allocated. This cannot be less than the number of Curl
states and defaults to that value.

Signed-off-by: David Edmondson 
---
 block/curl.c  | 20 +---
 docs/system/device-url-syntax.rst.inc |  4 
 qapi/block-core.json  |  4 
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 0ea9eedebd..27fa77c351 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -75,14 +75,15 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
 #define CURL_BLOCK_OPT_OFFSET "offset"
 #define CURL_BLOCK_OPT_BLOCKSIZE "blocksize"
+#define CURL_BLOCK_OPT_BLOCKCOUNT "blockcount"
 
 #define CURL_BLOCK_OPT_SSLVERIFY_DEFAULT true
 #define CURL_BLOCK_OPT_TIMEOUT_DEFAULT 5
 /* Must be a non-zero power of 2. */
 #define CURL_BLOCK_OPT_BLOCKSIZE_DEFAULT (256 * 1024)
+/* The defaultnumber of blocks to store in the cache. */
+#define CURL_BLOCK_OPT_BLOCKCOUNT_DEFAULT (CURL_NUM_STATES)
 
-/* The maximum number of blocks to store in the cache. */
-#define CURL_BLOCK_CACHE_MAX_BLOCKS 100
 /* The number of heads in the hash table. */
 #define CURL_BLOCK_CACHE_HASH 37
 
@@ -161,6 +162,7 @@ typedef struct BDRVCURLState {
 char *proxypassword;
 size_t offset;
 size_t blocksize;
+int cache_max;
 int cache_allocated; /* The number of block_t currently allocated. */
 QLIST_HEAD(, block) cache_free;
 QTAILQ_HEAD(, block) cache_lru;
@@ -287,7 +289,7 @@ static block_t *curl_cache_get(BDRVCURLState *s)
 }
 
 /* If not at the limit, try get a new one. */
-if (s->cache_allocated < CURL_BLOCK_CACHE_MAX_BLOCKS) {
+if (s->cache_allocated < s->cache_max) {
 b = curl_cache_alloc(s);
 if (b) {
 b->use++;
@@ -929,6 +931,11 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_SIZE,
 .help = "Block size for IO requests"
 },
+{
+.name = CURL_BLOCK_OPT_BLOCKCOUNT,
+.type = QEMU_OPT_SIZE,
+.help = "Maximum number of cached blocks"
+},
 { /* end of list */ }
 },
 };
@@ -1039,6 +1046,13 @@ static int curl_open(BlockDriverState *bs, QDict 
*options, int flags,
 error_setg(errp, "blocksize must be a non-zero power of two");
 goto out_noclean;
 }
+s->cache_max = qemu_opt_get_size(opts, CURL_BLOCK_OPT_BLOCKCOUNT,
+ CURL_BLOCK_OPT_BLOCKCOUNT_DEFAULT);
+if (s->cache_max < CURL_NUM_STATES) {
+error_setg(errp, "blockcount must be larger than %d",
+CURL_NUM_STATES - 1);
+goto out_noclean;
+}
 
 trace_curl_open(file);
 qemu_co_queue_init(&s->free_state_waitq);
diff --git a/docs/system/device-url-syntax.rst.inc 
b/docs/system/device-url-syntax.rst.inc
index ee504ee41a..56843cb38f 100644
--- a/docs/system/device-url-syntax.rst.inc
+++ b/docs/system/device-url-syntax.rst.inc
@@ -201,6 +201,10 @@ These are specified using a special URL syntax.
   bytes. The value must be a non-zero power of two.  It defaults
   to 256kB.
 
+   ``blockcount``
+  The number of ``blocksize`` blocks that the system may allocate
+  to store data read from the remote server.
+
Note that when passing options to qemu explicitly, ``driver`` is the
value of .
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index cd16197e1e..91888166fa 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3767,11 +3767,15 @@
 # @blocksize: Size of all IO requests sent to the remote server; must
 # be a non-zero power of two (defaults to 1 256kB)
 #
+# @blockcount: The number of IO blocks used to cache data from the
+#  remote server.
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsCurlBase',
   'data': { 'url': 'str',
 '*blocksize': 'int',
+'*blockcount': 'int',
 '*timeout': 'int',
 '*username': 'str',
 '*password-secret': 'str',
-- 
2.27.0

[RFC PATCH 5/9] block/curl: Allow the blocksize to be specified by the user

[David Edmondson]

Rather than a fixed 256kB blocksize, allow the user to specify the
size used. It must be a non-zero power of two, defaulting to 256kB.

Signed-off-by: David Edmondson 
---
 block/curl.c  | 73 +--
 docs/system/device-url-syntax.rst.inc |  7 +++
 qapi/block-core.json  |  4 ++
 3 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index cfc518efda..b2d02818a9 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -74,17 +74,12 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
 #define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
 #define CURL_BLOCK_OPT_OFFSET "offset"
+#define CURL_BLOCK_OPT_BLOCKSIZE "blocksize"
 
 #define CURL_BLOCK_OPT_SSLVERIFY_DEFAULT true
 #define CURL_BLOCK_OPT_TIMEOUT_DEFAULT 5
-
 /* Must be a non-zero power of 2. */
-#define CURL_BLOCK_SIZE (256 * 1024)
-
-/* Align "n" to the start of the containing block. */
-#define CURL_BLOCK_ALIGN(n) ((n) & ~(CURL_BLOCK_SIZE - 1))
-/* The offset of "n" within its' block. */
-#define CURL_BLOCK_OFFSET(n) ((n) & (CURL_BLOCK_SIZE - 1))
+#define CURL_BLOCK_OPT_BLOCKSIZE_DEFAULT (256 * 1024)
 
 struct BDRVCURLState;
 struct CURLState;
@@ -102,7 +97,7 @@ typedef struct CURLAIOCB {
 
 /*
  * start and end indicate the subset of the surrounding
- * CURL_BLOCK_SIZE sized block that is the subject of this
+ * BDRVCURLState.blocksize sized block that is the subject of this
  * IOCB. They are offsets from the beginning of the underlying
  * buffer.
  */
@@ -148,11 +143,24 @@ typedef struct BDRVCURLState {
 char *proxyusername;
 char *proxypassword;
 size_t offset;
+size_t blocksize;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
 static void curl_multi_do(void *arg);
 
+/* Align "n" to the start of the containing block. */
+static inline uint64_t curl_block_align(BDRVCURLState *s, uint64_t n)
+{
+return n & ~(s->blocksize - 1);
+}
+
+/* The offset of "n" within its' block. */
+static inline uint64_t curl_block_offset(BDRVCURLState *s, uint64_t n)
+{
+return n & (s->blocksize - 1);
+}
+
 #ifdef NEED_CURL_TIMER_CALLBACK
 /* Called from curl_multi_do_locked, with s->mutex held.  */
 static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
@@ -264,22 +272,23 @@ static size_t curl_header_cb(void *ptr, size_t size, 
size_t nmemb, void *opaque)
 /* Called from curl_multi_do_locked, with s->mutex held.  */
 static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
-CURLState *s = ((CURLState*)opaque);
+CURLState *state = (CURLState *)opaque;
+BDRVCURLState *s = state->s;
 size_t realsize = size * nmemb;
 
 trace_curl_read_cb(realsize);
 
-if (!s || !s->orig_buf) {
+if (!state || !state->orig_buf) {
 goto read_end;
 }
 
-if (s->buf_off >= CURL_BLOCK_SIZE) {
+if (state->buf_off >= s->blocksize) {
 /* buffer full, read nothing */
 goto read_end;
 }
-realsize = MIN(realsize, CURL_BLOCK_SIZE - s->buf_off);
-memcpy(s->orig_buf + s->buf_off, ptr, realsize);
-s->buf_off += realsize;
+realsize = MIN(realsize, s->blocksize - state->buf_off);
+memcpy(state->orig_buf + state->buf_off, ptr, realsize);
+state->buf_off += realsize;
 
 read_end:
 /* curl will error out if we do not return this value */
@@ -300,7 +309,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, 
uint64_t len,
 /* The end of the currently valid data. */
 uint64_t buf_end = state->buf_start + state->buf_off;
 /* The end of the valid data when the IO completes. */
-uint64_t buf_fend = state->buf_start + CURL_BLOCK_SIZE;
+uint64_t buf_fend = state->buf_start + s->blocksize;
 
 if (!state->orig_buf)
 continue;
@@ -315,7 +324,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, 
uint64_t len,
 (clamped_end >= state->buf_start) &&
 (clamped_end <= buf_end))
 {
-char *buf = state->orig_buf + CURL_BLOCK_OFFSET(start);
+char *buf = state->orig_buf + curl_block_offset(s, start);
 
 trace_curl_pending_hit(qemu_coroutine_self(),
start, len);
@@ -343,7 +352,7 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, 
uint64_t len,
 
 trace_curl_pending_piggyback(qemu_coroutine_self(),
  start, len);
-acb->start = CURL_BLOCK_OFFSET(start);
+acb->start = curl_block_offset(s, start);
 acb->end = acb->start + clamped_len;
 
 for (j = 0; j < CURL_NUM_ACB; j++) {
@@ -688,6 +697,11 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_SIZE,
 .help = "Offset into underlying content"
 },
+{
+.name = CURL_

[RFC PATCH 0/9] block/curl: Add caching of data downloaded from the remote server

[David Edmondson]

When using qemu-img to convert an image that is hosted on an HTTP
server to some faster local (or pseudo-local) storage, the overall
performance can be improved by reading data from the HTTP server in
larger blocks and by caching and re-using blocks already read. This
set of patches implements both of these, and adds a further patch
allowing an offset to be added to all of the HTTP requests.

The first patch (block/curl: Add an 'offset' parameter, affecting all
range requests) allows the user to add an arbitrary offset to all
range requests sent to the HTTP server. This is useful if the image to
be read from the HTTP server is embedded in another file (for example
an uncompressed tar file). It avoids the need to first download the
file containing the source image and extract it (both of which will
require writing the image to local storage). It is logically distinct
from the rest of the patches and somewhat use-case specific.

The remaining patches implement block based retrieval of data from the
HTTP server and, optionally, caching of those blocks in memory.

The existing HTTP implementation simply reads whatever data is
requested by the caller, with the option for a user-specified amount
of readahead. This is poor for performance because most IO requests
(from QCOW2, for example) are for relatively small amounts of data,
typically no more than 64kB. This does not allow the underlying TCP
connections to achieve peak throughput.

The existing readhead mechanism is also intended to work in
conjunction with the HTTP driver's attempt to piggy-back a new IO
request on one that is already in flight. This works, but is often
defeated because it relies on the existing IO request *completely*
satisfying any subsequent request that might piggy-back onto it. This
is rarely the case and, particularly when used with "readahead", can
result in the same data being downloaded repeatedly.

The observed performance will depend greatly on the environment, but
when using qemu-img to retrieve a 1GiB QCOW2 image from an HTTPS
server, the following was observed:

| approach   | time (hh:mm:ss) |
|+-|
| QCOW2 over HTTPS (existing implementation) |00:00:59 |
| 256kB blocks, 8 cached blocks  |00:00:42 |
| 2MB blocks, 100 cached blocks  |00:00:34 |

By way of comparison, aria2c (a dedicated HTTP download client) can
retrieve the same image in 19 seconds. Obviously this is without any
QCOW2 layer.

David Edmondson (9):
  block/curl: Add an 'offset' parameter, affecting all range requests
  block/curl: Remove readahead support
  block/curl: Tracing
  block/curl: Perform IO in fixed size chunks
  block/curl: Allow the blocksize to be specified by the user
  block/curl: Cache downloaded blocks
  block/curl: Allow the user to control the number of cache blocks
  block/curl: Allow 16 sockets/ACB
  block/curl: Add readahead support

 block/curl.c  | 515 ++
 block/io.c|   4 +
 block/linux-aio.c |   6 +
 block/trace-events|  18 +-
 docs/system/device-url-syntax.rst.inc |  15 +
 qapi/block-core.json  |  11 +-
 6 files changed, 488 insertions(+), 81 deletions(-)

-- 
2.27.0

[RFC PATCH 6/9] block/curl: Cache downloaded blocks

[David Edmondson]

In the hope that they will be referred to multiple times, cache the
blocks downloaded from the remote server.

Signed-off-by: David Edmondson 
---
 block/curl.c   | 321 +++--
 block/trace-events |   3 +
 2 files changed, 287 insertions(+), 37 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index b2d02818a9..0ea9eedebd 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -81,11 +81,29 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 /* Must be a non-zero power of 2. */
 #define CURL_BLOCK_OPT_BLOCKSIZE_DEFAULT (256 * 1024)
 
+/* The maximum number of blocks to store in the cache. */
+#define CURL_BLOCK_CACHE_MAX_BLOCKS 100
+/* The number of heads in the hash table. */
+#define CURL_BLOCK_CACHE_HASH 37
+
 struct BDRVCURLState;
 struct CURLState;
 
 static bool libcurl_initialized;
 
+typedef struct block {
+QLIST_ENTRY(block) hash; /* Blocks with the same hash value. */
+QLIST_ENTRY(block) free; /* Block free list. */
+QTAILQ_ENTRY(block) lru; /* LRU list. */
+bool hashed; /* block_t contains data and is hashed. */
+int use; /* Use count. */
+
+uint64_t start; /* Offset of first byte. */
+uint64_t count; /* Valid bytes. */
+
+char *buf;  /* Data. */
+} block_t;
+
 typedef struct CURLAIOCB {
 Coroutine *co;
 QEMUIOVector *qiov;
@@ -117,12 +135,11 @@ typedef struct CURLState
 CURLAIOCB *acb[CURL_NUM_ACB];
 CURL *curl;
 QLIST_HEAD(, CURLSocket) sockets;
-char *orig_buf;
-uint64_t buf_start;
 size_t buf_off;
 char range[128];
 char errmsg[CURL_ERROR_SIZE];
 char in_use;
+block_t *cache_block;
 } CURLState;
 
 typedef struct BDRVCURLState {
@@ -144,11 +161,17 @@ typedef struct BDRVCURLState {
 char *proxypassword;
 size_t offset;
 size_t blocksize;
+int cache_allocated; /* The number of block_t currently allocated. */
+QLIST_HEAD(, block) cache_free;
+QTAILQ_HEAD(, block) cache_lru;
+QLIST_HEAD(, block) * cache_hash;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
 static void curl_multi_do(void *arg);
 
+static void curl_cache_free(BDRVCURLState *s, block_t *b);
+
 /* Align "n" to the start of the containing block. */
 static inline uint64_t curl_block_align(BDRVCURLState *s, uint64_t n)
 {
@@ -161,6 +184,198 @@ static inline uint64_t curl_block_offset(BDRVCURLState 
*s, uint64_t n)
 return n & (s->blocksize - 1);
 }
 
+static uint64_t curl_cache_hash(BDRVCURLState *s, uint64_t n)
+{
+return curl_block_align(s, n) % CURL_BLOCK_CACHE_HASH;
+}
+
+static bool curl_cache_init(BDRVCURLState *s)
+{
+s->cache_allocated = 0;
+
+QLIST_INIT(&s->cache_free);
+QTAILQ_INIT(&s->cache_lru);
+
+s->cache_hash = g_try_malloc(CURL_BLOCK_CACHE_HASH * 
sizeof(s->cache_hash));
+if (!s->cache_hash) {
+return false;
+}
+
+for (int i = 0; i < CURL_BLOCK_CACHE_HASH; i++) {
+QLIST_INIT(&s->cache_hash[i]);
+}
+
+return true;
+}
+
+static void curl_cache_deinit(BDRVCURLState *s)
+{
+block_t *b;
+
+/*
+ * Cache blocks are either in the hash table or on the free list.
+ */
+for (int i = 0; i < CURL_BLOCK_CACHE_HASH; i++) {
+while (!QLIST_EMPTY(&s->cache_hash[i])) {
+b = QLIST_FIRST(&s->cache_hash[i]);
+QLIST_REMOVE(b, hash);
+b->hashed = false;
+curl_cache_free(s, b);
+}
+}
+
+while (!QLIST_EMPTY(&s->cache_free)) {
+b = QLIST_FIRST(&s->cache_free);
+QLIST_REMOVE(b, free);
+curl_cache_free(s, b);
+}
+
+assert(s->cache_allocated == 0);
+
+g_free(s->cache_hash);
+s->cache_hash = NULL;
+}
+
+static block_t *curl_cache_alloc(BDRVCURLState *s)
+{
+block_t *b = g_try_malloc0(sizeof(*b));
+
+if (!b) {
+return NULL;
+}
+
+b->buf = g_try_malloc(s->blocksize);
+if (!b->buf) {
+g_free(b);
+return NULL;
+}
+
+s->cache_allocated++;
+
+trace_curl_cache_alloc(s->cache_allocated);
+
+return b;
+}
+
+static void curl_cache_free(BDRVCURLState *s, block_t *b)
+{
+assert(b->use == 0);
+assert(!b->hashed);
+
+g_free(b->buf);
+g_free(b);
+
+s->cache_allocated--;
+
+trace_curl_cache_free(s->cache_allocated);
+}
+
+static block_t *curl_cache_get(BDRVCURLState *s)
+{
+block_t *b = NULL;
+
+/* If there is one on the free list, use it. */
+if (!QLIST_EMPTY(&s->cache_free)) {
+b = QLIST_FIRST(&s->cache_free);
+QLIST_REMOVE(b, free);
+
+assert(b->use == 0);
+assert(!b->hashed);
+
+b->use++;
+goto done;
+}
+
+/* If not at the limit, try get a new one. */
+if (s->cache_allocated < CURL_BLOCK_CACHE_MAX_BLOCKS) {
+b = curl_cache_alloc(s);
+if (b) {
+b->use++;
+goto done;
+}
+}
+
+/* Take one from the LRU list. */
+if (!QTAILQ_EMPTY(&s->cache_lru)) {
+b = QTAILQ_FIRST(&s->

[RFC PATCH 8/9] block/curl: Allow 16 sockets/ACB

[David Edmondson]

qemu-img allows up to 16 coroutines when performing IO. Ensure that
there is a Curl socket and ACB available to each of them.

Signed-off-by: David Edmondson 
---
 block/curl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 27fa77c351..8ee314739a 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -60,8 +60,8 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
CURLPROTO_FTP | CURLPROTO_FTPS)
 
-#define CURL_NUM_STATES 8
-#define CURL_NUM_ACB8
+#define CURL_NUM_STATES 16
+#define CURL_NUM_ACB16
 #define CURL_TIMEOUT_MAX 1
 
 #define CURL_BLOCK_OPT_URL   "url"
-- 
2.27.0

[RFC PATCH 2/9] block/curl: Remove readahead support

[David Edmondson]

Block based caching and the current readahead support do not interact
well, so remove readahead support before adding block
caching. Readahead will be re-added later.

Signed-off-by: David Edmondson 
---
 block/curl.c  | 23 ---
 docs/system/device-url-syntax.rst.inc |  7 ---
 qapi/block-core.json  |  4 
 3 files changed, 4 insertions(+), 30 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 32ec760f66..d0c74d7de5 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -65,7 +65,6 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_TIMEOUT_MAX 1
 
 #define CURL_BLOCK_OPT_URL   "url"
-#define CURL_BLOCK_OPT_READAHEAD "readahead"
 #define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
 #define CURL_BLOCK_OPT_TIMEOUT "timeout"
 #define CURL_BLOCK_OPT_COOKIE"cookie"
@@ -76,7 +75,6 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
 #define CURL_BLOCK_OPT_OFFSET "offset"
 
-#define CURL_BLOCK_OPT_READAHEAD_DEFAULT (256 * 1024)
 #define CURL_BLOCK_OPT_SSLVERIFY_DEFAULT true
 #define CURL_BLOCK_OPT_TIMEOUT_DEFAULT 5
 
@@ -124,7 +122,6 @@ typedef struct BDRVCURLState {
 uint64_t len;
 CURLState states[CURL_NUM_STATES];
 char *url;
-size_t readahead_size;
 bool sslverify;
 uint64_t timeout;
 char *cookie;
@@ -615,11 +612,6 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "URL to open",
 },
-{
-.name = CURL_BLOCK_OPT_READAHEAD,
-.type = QEMU_OPT_SIZE,
-.help = "Readahead size",
-},
 {
 .name = CURL_BLOCK_OPT_SSLVERIFY,
 .type = QEMU_OPT_BOOL,
@@ -705,14 +697,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto out_noclean;
 }
 
-s->readahead_size = qemu_opt_get_size(opts, CURL_BLOCK_OPT_READAHEAD,
-  CURL_BLOCK_OPT_READAHEAD_DEFAULT);
-if ((s->readahead_size & 0x1ff) != 0) {
-error_setg(errp, "HTTP_READAHEAD_SIZE %zd is not a multiple of 512",
-   s->readahead_size);
-goto out_noclean;
-}
-
 s->timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
  CURL_BLOCK_OPT_TIMEOUT_DEFAULT);
 if (s->timeout > CURL_TIMEOUT_MAX) {
@@ -898,7 +882,7 @@ static void curl_setup_preadv(BlockDriverState *bs, 
CURLAIOCB *acb)
 state->buf_off = 0;
 g_free(state->orig_buf);
 state->buf_start = start;
-state->buf_len = MIN(acb->end + s->readahead_size, s->len - start);
+state->buf_len = MIN(acb->end, s->len - start);
 end = start + state->buf_len - 1;
 state->orig_buf = g_try_malloc(state->buf_len);
 if (state->buf_len && state->orig_buf == NULL) {
@@ -971,8 +955,9 @@ static void curl_refresh_filename(BlockDriverState *bs)
 {
 BDRVCURLState *s = bs->opaque;
 
-/* "readahead" and "timeout" do not change the guest-visible data,
- * so ignore them */
+/*
+ * "timeout" does not change the guest-visible data, so ignore it.
+ */
 if (s->sslverify != CURL_BLOCK_OPT_SSLVERIFY_DEFAULT ||
 s->cookie || s->username || s->password || s->proxyusername ||
 s->proxypassword)
diff --git a/docs/system/device-url-syntax.rst.inc 
b/docs/system/device-url-syntax.rst.inc
index 33f1ddfe6d..bc38b9df38 100644
--- a/docs/system/device-url-syntax.rst.inc
+++ b/docs/system/device-url-syntax.rst.inc
@@ -174,13 +174,6 @@ These are specified using a special URL syntax.
``url``
   The full URL when passing options to the driver explicitly.
 
-   ``readahead``
-  The amount of data to read ahead with each range request to the
-  remote server. This value may optionally have the suffix 'T', 'G',
-  'M', 'K', 'k' or 'b'. If it does not have a suffix, it will be
-  assumed to be in bytes. The value must be a multiple of 512 bytes.
-  It defaults to 256k.
-
``sslverify``
   Whether to verify the remote server's certificate when connecting
   over SSL. It can have the value 'on' or 'off'. It defaults to
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 197bdc1c36..d6f5e91cc3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3752,9 +3752,6 @@
 #
 # @url: URL of the image file
 #
-# @readahead: Size of the read-ahead cache; must be a multiple of
-# 512 (defaults to 256 kB)
-#
 # @timeout: Timeout for connections, in seconds (defaults to 5)
 #
 # @username: Username for authentication (defaults to none)
@@ -3771,7 +3768,6 @@
 ##
 { 'struct': 'BlockdevOptionsCurlBase',
   'data': { 'url': 'str',
-'*readahead': 'int',
 '*timeout': 'int',
 '*username': 'str',
 '*password-secret': 'str',
-- 
2.27.0

[RFC PATCH 3/9] block/curl: Tracing

[David Edmondson]

Add some more trace functions to the IO path.

Signed-off-by: David Edmondson 
---
 block/curl.c   | 10 +-
 block/io.c |  4 
 block/linux-aio.c  |  6 ++
 block/trace-events | 13 -
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index d0c74d7de5..d2f4de46c9 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -299,6 +299,8 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, 
uint64_t len,
 {
 char *buf = state->orig_buf + (start - state->buf_start);
 
+trace_curl_pending_hit(qemu_coroutine_self(),
+   start, len);
 qemu_iovec_from_buf(acb->qiov, 0, buf, clamped_len);
 if (clamped_len < len) {
 qemu_iovec_memset(acb->qiov, clamped_len, 0, len - 
clamped_len);
@@ -316,6 +318,8 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, 
uint64_t len,
 {
 int j;
 
+trace_curl_pending_piggyback(qemu_coroutine_self(),
+ start, len);
 acb->start = start - state->buf_start;
 acb->end = acb->start + clamped_len;
 
@@ -328,6 +332,8 @@ static bool curl_find_buf(BDRVCURLState *s, uint64_t start, 
uint64_t len,
 }
 }
 
+trace_curl_pending_miss(qemu_coroutine_self(), start, len);
+
 return false;
 }
 
@@ -894,7 +900,7 @@ static void curl_setup_preadv(BlockDriverState *bs, 
CURLAIOCB *acb)
 
 snprintf(state->range, 127, "%" PRIu64 "-%" PRIu64,
  s->offset + start, s->offset + end);
-trace_curl_setup_preadv(acb->bytes, start, state->range);
+trace_curl_setup_preadv(qemu_coroutine_self(), start, acb->bytes);
 curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
 
 if (curl_multi_add_handle(s->multi, state->curl) != CURLM_OK) {
@@ -923,10 +929,12 @@ static int coroutine_fn curl_co_preadv(BlockDriverState 
*bs,
 .bytes = bytes
 };
 
+trace_curl_co_preadv(qemu_coroutine_self(), offset, bytes);
 curl_setup_preadv(bs, &acb);
 while (acb.ret == -EINPROGRESS) {
 qemu_coroutine_yield();
 }
+trace_curl_co_preadv_done(qemu_coroutine_self());
 return acb.ret;
 }
 
diff --git a/block/io.c b/block/io.c
index ad3a51ed53..f816a46bf0 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1194,6 +1194,8 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 return -ENOMEDIUM;
 }
 
+trace_bdrv_driver_pwritev(qemu_coroutine_self(), offset, bytes);
+
 if (drv->bdrv_co_pwritev_part) {
 ret = drv->bdrv_co_pwritev_part(bs, offset, bytes, qiov, qiov_offset,
 flags & bs->supported_write_flags);
@@ -1253,6 +1255,8 @@ emulate_flags:
 qemu_iovec_destroy(&local_qiov);
 }
 
+trace_bdrv_driver_pwritev_done(qemu_coroutine_self());
+
 return ret;
 }
 
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 3c0527c2bf..811e9ff94c 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -15,6 +15,7 @@
 #include "qemu/event_notifier.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
+#include "trace.h"
 
 #include 
 
@@ -391,6 +392,8 @@ int coroutine_fn laio_co_submit(BlockDriverState *bs, 
LinuxAioState *s, int fd,
 .qiov   = qiov,
 };
 
+trace_laio_co_submit(qemu_coroutine_self(), offset, qiov->size);
+
 ret = laio_do_submit(fd, &laiocb, offset, type);
 if (ret < 0) {
 return ret;
@@ -399,6 +402,9 @@ int coroutine_fn laio_co_submit(BlockDriverState *bs, 
LinuxAioState *s, int fd,
 if (laiocb.ret == -EINPROGRESS) {
 qemu_coroutine_yield();
 }
+
+trace_laio_co_submit_done(qemu_coroutine_self());
+
 return laiocb.ret;
 }
 
diff --git a/block/trace-events b/block/trace-events
index 9158335061..0b52d2ca1d 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -17,6 +17,8 @@ bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, 
int flags) "bs %p off
 bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t 
cluster_offset, int64_t cluster_bytes) "bs %p offset %"PRId64" bytes %u 
cluster_offset %"PRId64" cluster_bytes %"PRId64
 bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t 
dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset 
%"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
 bdrv_co_copy_range_to(void *src, uint64_t src_offset, void *dst, uint64_t 
dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset 
%"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
+bdrv_driver_pwritev(void *co, uint64_t offset, uint64_t bytes) "co %p writes 
0x%"PRIx64" + 0x%"PRIx64
+bdrv_driver_pwritev_done(void *co) "co %p done"
 
 # stream.c
 stream_one_iteration(void *s, int64_t offset, uint64_t bytes, int 
is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
@@ -196,7 +19

[RFC PATCH 9/9] block/curl: Add readahead support

[David Edmondson]

Re-add support for a readahead parameter, which is the number of bytes
added to the request from the upper layer before breaking the request
into blocks. The default is zero. The number of bytes specified has no
alignment requirements.

Signed-off-by: David Edmondson 
---
 block/curl.c  | 11 +++
 docs/system/device-url-syntax.rst.inc |  7 +++
 qapi/block-core.json  |  3 +++
 3 files changed, 21 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index 8ee314739a..a182a55b93 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -65,6 +65,7 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_TIMEOUT_MAX 1
 
 #define CURL_BLOCK_OPT_URL   "url"
+#define CURL_BLOCK_OPT_READAHEAD "readahead"
 #define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
 #define CURL_BLOCK_OPT_TIMEOUT "timeout"
 #define CURL_BLOCK_OPT_COOKIE"cookie"
@@ -149,6 +150,7 @@ typedef struct BDRVCURLState {
 uint64_t len;
 CURLState states[CURL_NUM_STATES];
 char *url;
+size_t readahead_size;
 bool sslverify;
 uint64_t timeout;
 char *cookie;
@@ -881,6 +883,11 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "URL to open",
 },
+{
+.name = CURL_BLOCK_OPT_READAHEAD,
+.type = QEMU_OPT_SIZE,
+.help = "Readahead size",
+},
 {
 .name = CURL_BLOCK_OPT_SSLVERIFY,
 .type = QEMU_OPT_BOOL,
@@ -976,6 +983,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto out_noclean;
 }
 
+s->readahead_size = qemu_opt_get_size(opts, CURL_BLOCK_OPT_READAHEAD, 0);
+
 s->timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
  CURL_BLOCK_OPT_TIMEOUT_DEFAULT);
 if (s->timeout > CURL_TIMEOUT_MAX) {
@@ -1247,6 +1256,8 @@ static int coroutine_fn curl_co_preadv(BlockDriverState 
*bs,
 
 trace_curl_co_preadv(qemu_coroutine_self(), offset, bytes);
 
+bytes += s->readahead_size;
+
 while (bytes > 0) {
 uint64_t len = MIN(bytes, s->blocksize - curl_block_offset(s, off));
 CURLAIOCB acb = {
diff --git a/docs/system/device-url-syntax.rst.inc 
b/docs/system/device-url-syntax.rst.inc
index 56843cb38f..58245e017c 100644
--- a/docs/system/device-url-syntax.rst.inc
+++ b/docs/system/device-url-syntax.rst.inc
@@ -174,6 +174,13 @@ These are specified using a special URL syntax.
``url``
   The full URL when passing options to the driver explicitly.
 
+   ``readahead``
+  The amount of data to read ahead with each range request to the
+  remote server. This value may optionally have the suffix 'T', 'G',
+  'M', 'K', 'k' or 'b'. If it does not have a suffix, it will be
+  assumed to be in bytes. The value must be a multiple of 512 bytes.
+  It defaults to 256k.
+
``sslverify``
   Whether to verify the remote server's certificate when connecting
   over SSL. It can have the value 'on' or 'off'. It defaults to
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 91888166fa..f4092ccc14 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3752,6 +3752,8 @@
 #
 # @url: URL of the image file
 #
+# @readahead: Amount of read-ahead (defaults to 0)
+#
 # @timeout: Timeout for connections, in seconds (defaults to 5)
 #
 # @username: Username for authentication (defaults to none)
@@ -3776,6 +3778,7 @@
   'data': { 'url': 'str',
 '*blocksize': 'int',
 '*blockcount': 'int',
+'*readahead': 'int',
 '*timeout': 'int',
 '*username': 'str',
 '*password-secret': 'str',
-- 
2.27.0

Re: [PATCH] qemu-img: Explicit number replaced by a constant

[Max Reitz]

On 18.08.20 09:19, Stefano Garzarella wrote:
> Hi Yi Li,
> thanks for this patch! Just a comment below:
> 
> On Mon, Aug 17, 2020 at 07:01:13PM +0800, Yi Li wrote:
>> Signed-off-by: Yi Li 
>> ---
>>  qemu-img.c | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 5308773811..a0fbc2757c 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1181,7 +1181,7 @@ static int64_t find_nonzero(const uint8_t *buf, 
>> int64_t n)
>>  }
>>  
>>  /*
>> - * Returns true iff the first sector pointed to by 'buf' contains at least
>> + * Returns true if the first sector pointed to by 'buf' contains at least
> 
> This change seems unrelated, please can you put this in a separate patch?

It’s also just not a correct change, as “iff” means “if and only if”.

Max



signature.asc
Description: OpenPGP digital signature

Re: hw-display-qxl.so: undefined symbol: qemu_qxl_io_log_semaphore

[Daniel P . Berrangé]

On Wed, Aug 12, 2020 at 11:46:21AM -0400, Cole Robinson wrote:
> On 7/29/20 8:50 AM, Stefan Hajnoczi wrote:
> > On Thu, Jul 16, 2020 at 05:10:26PM -0400, Cole Robinson wrote:
> >> I'm trying to build qemu 5.1.0-rc0 in Fedora. I'm hitting some issues.
> > 
> > For anyone else reading this email thread, this was fixed in QEMU
> > 5.1.0-rc1:
> > 
> >   commit d97df4b84bc42613cf9a03619de453ebd0be30b7
> >   Author: Gerd Hoffmann 
> >   Date:   Mon Jul 20 12:03:50 2020 +0200
> > 
> >   qxl: fix modular builds with dtrace
> > 
> 
> FWIW I'm still hitting issues with qemu-5.1.0 GA but maybe it's
> unrelated to that specific fix. Issues reproduce on fedora 33+, not
> fedora 32.

Gerd's fix here was to remove the reference to the
conditional trace_event_get_state_backends()  checks.

This avoids referencing the systemtap  semaphore symbol.

This works. with systemtap 4.3  or earlier.

In systemtap 4.4 they attempted to support LTO, by changing
the _SDT_PROBE macro so that it *always* references the
semaphore symbol as a hint for LTO.

This broke QEMU again.

While this is obviously a regression in behaviour in systemtap,
I think its possible to argue that QEMU's use of probes is
itself broken.

We have tracepoints in the qxl.so module, but the probes
are all linked into the qemu-system-x86_64 binary, and
not exported for use by the modules.  AFAICT, we've merely
been lucky not to hit this previously, as none of the
modules we had upto now used trace_event_get_state_backends()
checks. Any such use would always have been broken with all
systemtap versions.

IOW, new systemtap 4.4 is exposing a long standing design
flaw in QEMU's probe.

I'm gong to ask the systemtap maintainers for an opinion on
this behaviour change none the less.


If systemtap won't change, then to fix this, for any foo.c
that will be in a module, we need a separate 'foo.trace'
file that generates a .o that is directly linked to the
foo.so, not the qemu-system-x86_64 binary.


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

[PATCH v2 1/7] x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features

[Igor Mammedov]

It will allow firmware to notify QEMU that firmware requires SMI
being triggered on CPU hot[un]plug, so that it would be able to account
for hotplugged CPU and relocate it to new SMM base and/or safely remove
CPU on unplug.

Using negotiated features, follow up patches will insert SMI upcall
into AML code, to make sure that firmware processes hotplug before
guest OS would attempt to use new CPU.

Signed-off-by: Igor Mammedov 
Reviewed-by: Laszlo Ersek 
---
v2:
  - rebase on top of 5.1 (move compat values to 5.1 machine)
  - make "x-smi-cpu-hotunplug" false by default (Laszlo Ersek 
)
---
 include/hw/i386/ich9.h |  2 ++
 include/hw/i386/pc.h   |  3 +++
 hw/i386/pc.c   |  6 +-
 hw/i386/pc_piix.c  |  1 +
 hw/i386/pc_q35.c   |  1 +
 hw/isa/lpc_ich9.c  | 13 +
 6 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index a98d10b252..d1bb3f7bf0 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -247,5 +247,7 @@ typedef struct ICH9LPCState {
 
 /* bit positions used in fw_cfg SMI feature negotiation */
 #define ICH9_LPC_SMI_F_BROADCAST_BIT0
+#define ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT  1
+#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT   2
 
 #endif /* HW_ICH9_H */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 3d7ed3a55e..fe52e165b2 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -193,6 +193,9 @@ void pc_system_firmware_init(PCMachineState *pcms, 
MemoryRegion *rom_memory);
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry);
 
+extern GlobalProperty pc_compat_5_1[];
+extern const size_t pc_compat_5_1_len;
+
 extern GlobalProperty pc_compat_5_0[];
 extern const size_t pc_compat_5_0_len;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 47c5ca3e34..99c6bdbab4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -97,8 +97,12 @@
 #include "fw_cfg.h"
 #include "trace.h"
 
-GlobalProperty pc_compat_5_0[] = {
+GlobalProperty pc_compat_5_1[] = {
+{ "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
 };
+const size_t pc_compat_5_1_len = G_N_ELEMENTS(pc_compat_5_1);
+
+GlobalProperty pc_compat_5_0[] = { };
 const size_t pc_compat_5_0_len = G_N_ELEMENTS(pc_compat_5_0);
 
 GlobalProperty pc_compat_4_2[] = {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b789e83f9a..d56f2e1b96 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -433,6 +433,7 @@ static void pc_i440fx_5_1_machine_options(MachineClass *m)
 m->alias = "pc";
 m->is_default = true;
 pcmc->default_cpu_version = 1;
+compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
 }
 
 DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a3e607a544..0ca1146a59 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -359,6 +359,7 @@ static void pc_q35_5_1_machine_options(MachineClass *m)
 pc_q35_machine_options(m);
 m->alias = "q35";
 pcmc->default_cpu_version = 1;
+compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
 }
 
 DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index cd6e169d47..19f32bed3e 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -373,6 +373,15 @@ static void smi_features_ok_callback(void *opaque)
 /* guest requests invalid features, leave @features_ok at zero */
 return;
 }
+if (!(guest_features & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT)) &&
+guest_features & (BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT) |
+  BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT))) {
+/*
+ * cpu hot-[un]plug with SMI requires SMI broadcast,
+ * leave @features_ok at zero
+ */
+return;
+}
 
 /* valid feature subset requested, lock it down, report success */
 lpc->smi_negotiated_features = guest_features;
@@ -747,6 +756,10 @@ static Property ich9_lpc_properties[] = {
 DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true),
 DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features,
   ICH9_LPC_SMI_F_BROADCAST_BIT, true),
+DEFINE_PROP_BIT64("x-smi-cpu-hotplug", ICH9LPCState, smi_host_features,
+  ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT, true),
+DEFINE_PROP_BIT64("x-smi-cpu-hotunplug", ICH9LPCState, smi_host_features,
+  ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.26.2

[PATCH v2 5/7] tests: acpi: mark to be changed tables in bios-tables-test-allowed-diff

[Igor Mammedov]

... to let tests pass until binary blobs are updated with new AML

Signed-off-by: Igor Mammedov 
---
 tests/qtest/bios-tables-test-allowed-diff.h | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..dba32d5613 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,20 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/DSDT",
+"tests/data/acpi/q35/DSDT",
+"tests/data/acpi/q35/DSDT.tis",
+"tests/data/acpi/q35/DSDT.bridge",
+"tests/data/acpi/q35/DSDT.mmio64",
+"tests/data/acpi/q35/DSDT.ipmibt",
+"tests/data/acpi/q35/DSDT.cphp",
+"tests/data/acpi/q35/DSDT.memhp",
+"tests/data/acpi/q35/DSDT.numamem",
+"tests/data/acpi/q35/DSDT.dimmpxm",
+"tests/data/acpi/q35/DSDT.acpihmat",
+"tests/data/acpi/pc/DSDT.bridge",
+"tests/data/acpi/pc/DSDT.ipmikcs",
+"tests/data/acpi/pc/DSDT.cphp",
+"tests/data/acpi/pc/DSDT.memhp",
+"tests/data/acpi/pc/DSDT.numamem",
+"tests/data/acpi/pc/DSDT.dimmpxm",
+"tests/data/acpi/pc/DSDT.acpihmat",
+"tests/data/acpi/pc/DSDT.acpihmat",
-- 
2.26.2

[PATCH v2 6/7] x68: acpi: trigger SMI before sending hotplug Notify event to OSPM

[Igor Mammedov]

In case firmware has negotiated CPU hotplug SMI feature, generate
AML to describe SMI IO port region and send SMI to firmware
on each CPU hotplug SCI in case new CPUs were hotplugged.

Since new CPUs can be hotplugged while CPU_SCAN_METHOD is running
we can't send SMI before new CPUs are fetched from QEMU as it
could cause sending Notify to a CPU that firmware hasn't seen yet.
So fetch new CPUs into local cache first and then send SMI and
after that sends Notify events to cached CPUs. This should ensure
that Notify is sent only to CPUs which were processed by firmware
first.
Any CPUs that were hotplugged after caching will be processed
by the next CPU_SCAN_METHOD, when pending SCI is handled.

Signed-off-by: Igor Mammedov 
---
v2:
 - clear insert event after firmware has returned
   control from SMI. (Laszlo Ersek )
v1:
 - make sure that Notify is sent only to CPUs seen by FW
 - (Laszlo Ersek )
- use macro instead of x-smi-negotiated-features
- style fixes
- reserve whole SMI block (0xB2, 0xB3)
v0:
 - s/aml_string/aml_eisaid/
   /fixes Windows BSOD, on nonsense value/ (Laszlo Ersek )
 - put SMI device under PCI0 like the rest of hotplug IO port
 - do not generate SMI AML if CPU hotplug SMI feature hasn't been negotiated

Signed-off-by: Igor Mammedov 
---
 include/hw/acpi/cpu.h  |  1 +
 include/hw/i386/ich9.h |  2 ++
 hw/acpi/cpu.c  | 80 +++---
 hw/i386/acpi-build.c   | 35 +-
 hw/isa/lpc_ich9.c  |  3 ++
 5 files changed, 115 insertions(+), 6 deletions(-)

diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 62f0278ba2..0eeedaa491 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -50,6 +50,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
 typedef struct CPUHotplugFeatures {
 bool acpi_1_compatible;
 bool has_legacy_cphp;
+const char *smi_path;
 } CPUHotplugFeatures;
 
 void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index d1bb3f7bf0..0f43ef2481 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -245,6 +245,8 @@ typedef struct ICH9LPCState {
 #define ICH9_SMB_HST_D1 0x06
 #define ICH9_SMB_HOST_BLOCK_DB  0x07
 
+#define ICH9_LPC_SMI_NEGOTIATED_FEAT_PROP "x-smi-negotiated-features"
+
 /* bit positions used in fw_cfg SMI feature negotiation */
 #define ICH9_LPC_SMI_F_BROADCAST_BIT0
 #define ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT  1
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 3d6a500fb7..4864c3b396 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -14,6 +14,8 @@
 #define ACPI_CPU_CMD_DATA_OFFSET_RW 8
 #define ACPI_CPU_CMD_DATA2_OFFSET_R 0
 
+#define OVMF_CPUHP_SMI_CMD 4
+
 enum {
 CPHP_GET_NEXT_CPU_WITH_EVENT_CMD = 0,
 CPHP_OST_EVENT_CMD = 1,
@@ -321,6 +323,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
 #define CPU_NOTIFY_METHOD "CTFY"
 #define CPU_EJECT_METHOD  "CEJ0"
 #define CPU_OST_METHOD"COST"
+#define CPU_ADDED_LIST"CNEW"
 
 #define CPU_ENABLED   "CPEN"
 #define CPU_SELECTOR  "CSEL"
@@ -471,21 +474,61 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
 Aml *dev_chk = aml_int(1);
 Aml *eject_req = aml_int(3);
 Aml *next_cpu_cmd = aml_int(CPHP_GET_NEXT_CPU_WITH_EVENT_CMD);
+Aml *num_added_cpus = aml_local(1);
+Aml *cpu_idx = aml_local(2);
+Aml *uid = aml_local(3);
+Aml *new_cpus = aml_name(CPU_ADDED_LIST);
 
 aml_append(method, aml_acquire(ctrl_lock, 0x));
+
+/* use named package as old Windows don't support it in local var 
*/
+if (arch_ids->len <= 256) {
+/*
+ * For compatibility with Windows Server 2008 (max 256 cpus),
+ * use ACPI1.0 PackageOp which can cache max 255 elements.
+ * Which is fine for 256 CPUs VM. Max 255 can be hotplugged,
+ * sice at least one CPU must be already present.
+ */
+aml_append(method, aml_name_decl(CPU_ADDED_LIST,
+aml_package(arch_ids->len)));
+} else {
+aml_append(method, aml_name_decl(CPU_ADDED_LIST,
+aml_varpackage(arch_ids->len)));
+}
+
+aml_append(method, aml_store(zero, num_added_cpus));
 aml_append(method, aml_store(one, has_event));
-while_ctx = aml_while(aml_equal(has_event, one));
+/* start scan from the 1st CPU */
+aml_append(method, aml_store(zero, uid));
+while_ctx = aml_while(aml_land(aml_equal(has_event, one),
+  aml_lless(uid, aml_int(arch_ids->len;
 {
  /* clear loop exit condition, ins_evt/rm_evt checks
   * will set it to 1 while next_cpu_cmd r

[PATCH v2 0/7] x86: fix cpu hotplug with secure boot

[Igor Mammedov]

v2:
  - AML: clean is_inserted flag only after SMI callback
  - make x-smi-cpu-hotunplug false by default
  - massage error hint on not supported unplug
v1:
  - fix typos and some phrases (Laszlo)
  - add unplug check (Laszlo)
  - redo AML scan logic to avoid race when adding multiple CPUs

CPU hotplug with Secure Boot was not really supported and firmware wasn't aware
of hotplugged CPUs (which might lead to guest crashes). During 4.2 we introduced
locked SMI handler RAM arrea to make sure that guest OS wasn't able to inject
its own SMI handler and OVMF added initial CPU hotplug support.

This series is QEMU part of that support which lets QMVF tell QEMU that
CPU hotplug with SMI broadcast enabled is supported so that QEMU would be able
to prevent hotplug in case it's not supported and trigger SMI on hotplug when
it's necessary.


Igor Mammedov (7):
  x86: lpc9: let firmware negotiate 'CPU hotplug with SMI' features
  x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI is in
use
  x86: cpuhp: refuse cpu hot-unplug request earlier if not supported
  acpi: add aml_land() and aml_break() primitives
  tests: acpi: mark to be changed tables in
bios-tables-test-allowed-diff
  x68: acpi: trigger SMI before sending hotplug Notify event to OSPM
  tests: acpi: update acpi blobs with new AML

 include/hw/acpi/aml-build.h   |   2 +
 include/hw/acpi/cpu.h |   1 +
 include/hw/i386/ich9.h|   4 ++
 include/hw/i386/pc.h  |   3 ++
 hw/acpi/aml-build.c   |  16 ++
 hw/acpi/cpu.c |  80 --
 hw/acpi/ich9.c|  24 -
 hw/i386/acpi-build.c  |  35 -
 hw/i386/pc.c  |  17 ++-
 hw/i386/pc_piix.c |   1 +
 hw/i386/pc_q35.c  |   1 +
 hw/isa/lpc_ich9.c |  16 ++
 tests/data/acpi/pc/DSDT   | Bin 4934 -> 5009 bytes
 tests/data/acpi/pc/DSDT.acpihmat  | Bin 6258 -> 6334 bytes
 tests/data/acpi/pc/DSDT.bridge| Bin 6793 -> 6868 bytes
 tests/data/acpi/pc/DSDT.cphp  | Bin 5397 -> 5473 bytes
 tests/data/acpi/pc/DSDT.dimmpxm   | Bin 6587 -> 6663 bytes
 tests/data/acpi/pc/DSDT.ipmikcs   | Bin 5006 -> 5081 bytes
 tests/data/acpi/pc/DSDT.memhp | Bin 6293 -> 6368 bytes
 tests/data/acpi/pc/DSDT.numamem   | Bin 4940 -> 5015 bytes
 tests/data/acpi/q35/DSDT  | Bin 7678 -> 7753 bytes
 tests/data/acpi/q35/DSDT.acpihmat | Bin 9002 -> 9078 bytes
 tests/data/acpi/q35/DSDT.bridge   | Bin 7695 -> 7770 bytes
 tests/data/acpi/q35/DSDT.cphp | Bin 8141 -> 8217 bytes
 tests/data/acpi/q35/DSDT.dimmpxm  | Bin 9331 -> 9407 bytes
 tests/data/acpi/q35/DSDT.ipmibt   | Bin 7753 -> 7828 bytes
 tests/data/acpi/q35/DSDT.memhp| Bin 9037 -> 9112 bytes
 tests/data/acpi/q35/DSDT.mmio64   | Bin 8808 -> 8883 bytes
 tests/data/acpi/q35/DSDT.numamem  | Bin 7684 -> 7759 bytes
 tests/data/acpi/q35/DSDT.tis  | Bin 8283 -> 8358 bytes
 30 files changed, 192 insertions(+), 8 deletions(-)

-- 
2.26.2

[PATCH v2 7/7] tests: acpi: update acpi blobs with new AML

[Igor Mammedov]

Update CPU hotplug AML with following changes

 Method (CSCN, 0, Serialized)
 {
 Acquire (\_SB.PCI0.PRES.CPLK, 0x)
+Name (CNEW, Package (0x01){})
+Local1 = Zero
 Local0 = One
-While ((Local0 == One))
+Local3 = Zero
+While (((Local0 == One) && (Local3 < One)))
 {
 Local0 = Zero
+\_SB.PCI0.PRES.CSEL = Local3
 \_SB.PCI0.PRES.CCMD = Zero
+If ((\_SB.PCI0.PRES.CDAT < Local3))
+{
+Break
+}
+
+Local3 = \_SB.PCI0.PRES.CDAT
 If ((\_SB.PCI0.PRES.CINS == One))
 {
-CTFY (\_SB.PCI0.PRES.CDAT, One)
-\_SB.PCI0.PRES.CINS = One
+CNEW [Local1] = Local3
+Local1++
 Local0 = One
 }
 ElseIf ((\_SB.PCI0.PRES.CRMV == One))
 {
-CTFY (\_SB.PCI0.PRES.CDAT, 0x03)
+CTFY (Local3, 0x03)
 \_SB.PCI0.PRES.CRMV = One
 Local0 = One
 }
+
+Local3++
+}
+
+Local2 = Zero
+While ((Local2 < Local1))
+{
+CTFY (DerefOf (CNEW [Local2]), One)
+\_SB.PCI0.PRES.CINS = One
+Local2++
 }

Signed-off-by: Igor Mammedov 
---
 tests/qtest/bios-tables-test-allowed-diff.h |  19 ---
 tests/data/acpi/pc/DSDT | Bin 4934 -> 5009 bytes
 tests/data/acpi/pc/DSDT.acpihmat| Bin 6258 -> 6334 bytes
 tests/data/acpi/pc/DSDT.bridge  | Bin 6793 -> 6868 bytes
 tests/data/acpi/pc/DSDT.cphp| Bin 5397 -> 5473 bytes
 tests/data/acpi/pc/DSDT.dimmpxm | Bin 6587 -> 6663 bytes
 tests/data/acpi/pc/DSDT.ipmikcs | Bin 5006 -> 5081 bytes
 tests/data/acpi/pc/DSDT.memhp   | Bin 6293 -> 6368 bytes
 tests/data/acpi/pc/DSDT.numamem | Bin 4940 -> 5015 bytes
 tests/data/acpi/q35/DSDT| Bin 7678 -> 7753 bytes
 tests/data/acpi/q35/DSDT.acpihmat   | Bin 9002 -> 9078 bytes
 tests/data/acpi/q35/DSDT.bridge | Bin 7695 -> 7770 bytes
 tests/data/acpi/q35/DSDT.cphp   | Bin 8141 -> 8217 bytes
 tests/data/acpi/q35/DSDT.dimmpxm| Bin 9331 -> 9407 bytes
 tests/data/acpi/q35/DSDT.ipmibt | Bin 7753 -> 7828 bytes
 tests/data/acpi/q35/DSDT.memhp  | Bin 9037 -> 9112 bytes
 tests/data/acpi/q35/DSDT.mmio64 | Bin 8808 -> 8883 bytes
 tests/data/acpi/q35/DSDT.numamem| Bin 7684 -> 7759 bytes
 tests/data/acpi/q35/DSDT.tis| Bin 8283 -> 8358 bytes
 19 files changed, 19 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dba32d5613..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,20 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/pc/DSDT",
-"tests/data/acpi/q35/DSDT",
-"tests/data/acpi/q35/DSDT.tis",
-"tests/data/acpi/q35/DSDT.bridge",
-"tests/data/acpi/q35/DSDT.mmio64",
-"tests/data/acpi/q35/DSDT.ipmibt",
-"tests/data/acpi/q35/DSDT.cphp",
-"tests/data/acpi/q35/DSDT.memhp",
-"tests/data/acpi/q35/DSDT.numamem",
-"tests/data/acpi/q35/DSDT.dimmpxm",
-"tests/data/acpi/q35/DSDT.acpihmat",
-"tests/data/acpi/pc/DSDT.bridge",
-"tests/data/acpi/pc/DSDT.ipmikcs",
-"tests/data/acpi/pc/DSDT.cphp",
-"tests/data/acpi/pc/DSDT.memhp",
-"tests/data/acpi/pc/DSDT.numamem",
-"tests/data/acpi/pc/DSDT.dimmpxm",
-"tests/data/acpi/pc/DSDT.acpihmat",
-"tests/data/acpi/pc/DSDT.acpihmat",
diff --git a/tests/data/acpi/pc/DSDT b/tests/data/acpi/pc/DSDT
index 
6d0aaf729ac7d64cf966621adf276534de5cc555..716d412a2474e0990136560b0340f808e490b514
 100644
GIT binary patch
delta 304
zcmX@6Hc_3+CDw
z#sY@K0>*>_hU7&~JQF4-FiuSd3MCXI$LO=f2Rp?FIC~ld1i1z~2fO+dFrW)M`?@R;
zpNcN(;uw;=v;dpv0+q>7;d;2io_@iM1<4&C=Oi+ef*iBZe1R!U9b8$EZy2L`YiFmPVoWGo(2IyuEEaEzAg*gStkGI5EXY0af?Kkc5w`uT)-)23N&P)
bH_HMKmdWTUf_%dmap`p6l-k_E$;kx(->We%

diff --git a/tests/data/acpi/pc/DSDT.acpihmat b/tests/data/acpi/pc/DSDT.acpihmat
index 
2e5e02400b1bd2842989d395c573fc593f45503b..198e87dfc2d7df261f2f51d685d90db5c6eb8929
 100644
GIT binary patch
delta 295
zcmexlu+NaoCD>TXu#}TbOnUO=-
z?EilbXFu0)Att5*hQtEKgaU@-Mb11ECMPgXP38iMB@`sb=(EHJJH-b$dm01;xduB2
zyZTI?&mmSXJ{4WY#W5s#X#qCT1uB!#g+2X(84Hp-K(;3`l!EMDXuiM{Ls^h-7^8ED
zTVyg9a{(i=8I$cex%5ht3mB3XX-rK@1gU8To0i1Dh^i9gXrOJSNt5F_#5Tuqitqyf
D&!|=a

delta 215
zcmdmI_{o6FCDZZv3Dca7B(g?o5=^*Rx|k;Zq8-rU}X2;a}IX)%r8akP`UnC5wjePt

diff --git a

[PATCH v2 2/7] x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI is in use

[Igor Mammedov]

There were reports of guest crash on CPU hotplug, when using q35 machine
type and OVMF with SMM, due to hotplugged CPU trying to process SMI at
default SMI handler location without it being relocated by firmware first.

Fix it by refusing hotplug if firmware hasn't negotiated CPU hotplug with
SMI support while SMI broadcast is in use.

Signed-off-by: Igor Mammedov 
Reviewed-by: Laszlo Ersek 
Tested-by: Laszlo Ersek 
---
v1:
   fix typos an use suggested wording in commit and error msg
   s/secure boot/smm/; s/hotplug SMI/hotplug with SMI/
  (Laszlo Ersek )
---
 hw/acpi/ich9.c | 12 +++-
 hw/i386/pc.c   | 11 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 6a19070cec..0acc9a3107 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -408,10 +408,20 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
 
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
-!lpc->pm.acpi_memory_hotplug.is_enabled)
+!lpc->pm.acpi_memory_hotplug.is_enabled) {
 error_setg(errp,
"memory hotplug is not enabled: %s.memory-hotplug-support "
"is not set", object_get_typename(OBJECT(lpc)));
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+uint64_t negotiated = lpc->smi_negotiated_features;
+
+if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) &&
+!(negotiated & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT))) {
+error_setg(errp, "cpu hotplug with SMI wasn't enabled by 
firmware");
+error_append_hint(errp, "update machine type to newer than 5.1 "
+"and firmware that suppors CPU hotplug with SMM");
+}
+}
 }
 
 void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 99c6bdbab4..f07a8ab6c4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1499,6 +1499,17 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 return;
 }
 
+if (pcms->acpi_dev) {
+Error *local_err = NULL;
+
+hotplug_handler_pre_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
+ &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+}
+
 init_topo_info(&topo_info, x86ms);
 
 env->nr_dies = x86ms->smp_dies;
-- 
2.26.2

[PATCH v2 3/7] x86: cpuhp: refuse cpu hot-unplug request earlier if not supported

[Igor Mammedov]

CPU hot-unplug with SMM requires firmware participation to prevent
guest crash (i.e. CPU can be removed only after OS _and_ firmware
were prepared for the action).
Previous patches introduced ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT
feature bit, which is advertised by firmware when it has support
for CPU hot-unplug. Use it to check if guest is able to handle
unplug and make device_del fail gracefully if hot-unplug feature
hasn't been negotiated.

Signed-off-by: Igor Mammedov 
Tested-by: Laszlo Ersek 
Reviewed-by: Laszlo Ersek 
---
v2:
 - fix typo in commit message
 - drop 5.1 version from hint message (Laszlo)
---
 hw/acpi/ich9.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 0acc9a3107..95cb0f935b 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -460,6 +460,18 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler 
*hotplug_dev,
   errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
!lpc->pm.cpu_hotplug_legacy) {
+uint64_t negotiated = lpc->smi_negotiated_features;
+
+if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) &&
+!(negotiated & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT))) {
+error_setg(errp, "cpu hot-unplug with SMI wasn't enabled "
+ "by firmware");
+error_append_hint(errp, "update machine type to a version having "
+"x-smi-cpu-hotunplug=on and firmware that "
+"supports CPU hot-unplug with SMM");
+return;
+}
+
 acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state,
dev, errp);
 } else {
-- 
2.26.2

Re: [PATCH v7 0/8] Introduce 'yank' oob qmp command to recover from hanging qemu

[Lukas Straub]

On Tue, 4 Aug 2020 10:11:22 +0200
Lukas Straub  wrote:

> Hello Everyone,
> In many cases, if qemu has a network connection (qmp, migration, chardev, 
> etc.)
> to some other server and that server dies or hangs, qemu hangs too.
> These patches introduce the new 'yank' out-of-band qmp command to recover from
> these kinds of hangs. The different subsystems register callbacks which get
> executed with the yank command. For example the callback can shutdown() a
> socket. This is intended for the colo use-case, but it can be used for other
> things too of course.
> 
> Regards,
> Lukas Straub
> 
> v7:
>  -yank_register_instance now returns error via Error **errp instead of 
> aborting
>  -dropped "chardev/char.c: Check for duplicate id before  creating chardev"
> 
> v6:
>  -add Reviewed-by and Acked-by tags
>  -rebase on master
>  -lots of changes in nbd due to rebase
>  -only take maintainership of util/yank.c and include/qemu/yank.h (Daniel P. 
> Berrangé)
>  -fix a crash discovered by the newly added chardev test
>  -fix the test itself
> 
> v5:
>  -move yank.c to util/
>  -move yank.h to include/qemu/
>  -add license to yank.h
>  -use const char*
>  -nbd: use atomic_store_release and atomic_load_aqcuire
>  -io-channel: ensure thread-safety and document it
>  -add myself as maintainer for yank
> 
> v4:
>  -fix build errors...
> 
> v3:
>  -don't touch softmmu/vl.c, use __contructor__ attribute instead (Paolo 
> Bonzini)
>  -fix build errors
>  -rewrite migration patch so it actually passes all tests
> 
> v2:
>  -don't touch io/ code anymore
>  -always register yank functions
>  -'yank' now takes a list of instances to yank
>  -'query-yank' returns a list of yankable instances
> 
> Lukas Straub (8):
>   Introduce yank feature
>   block/nbd.c: Add yank feature
>   chardev/char-socket.c: Add yank feature
>   migration: Add yank feature
>   io/channel-tls.c: make qio_channel_tls_shutdown thread-safe
>   io: Document thread-safety of qio_channel_shutdown
>   MAINTAINERS: Add myself as maintainer for yank feature
>   tests/test-char.c: Wait for the chardev to connect in
> char_socket_client_dupid_test
> 
>  MAINTAINERS   |   6 ++
>  block/nbd.c   | 129 +++-
>  chardev/char-socket.c |  31 ++
>  include/io/channel.h  |   2 +
>  include/qemu/yank.h   |  80 +++
>  io/channel-tls.c  |   6 +-
>  migration/channel.c   |  12 +++
>  migration/migration.c |  25 -
>  migration/multifd.c   |  10 ++
>  migration/qemu-file-channel.c |   6 ++
>  migration/savevm.c|   6 ++
>  qapi/misc.json|  45 +
>  tests/Makefile.include|   2 +-
>  tests/test-char.c |   1 +
>  util/Makefile.objs|   1 +
>  util/yank.c   | 184 ++
>  16 files changed, 493 insertions(+), 53 deletions(-)
>  create mode 100644 include/qemu/yank.h
>  create mode 100644 util/yank.c
> 
> --
> 2.20.1

Ping...


pgpRz8K6C6HvG.pgp
Description: OpenPGP digital signature

Re: [PATCH v3 0/7] colo: Introduce resource agent and test suite/CI

[Lukas Straub]

On Tue, 4 Aug 2020 12:46:29 +0200
Lukas Straub  wrote:

> Hello Everyone,
> So here is v3. Patch 1 can already be merged independently of the others.
> Please review.
> 
> Regards,
> Lukas Straub
> 
> Based-on: 
> "Introduce 'yank' oob qmp command to recover from hanging qemu"
> 
> Changes:
> 
> v3:
>  -resource-agent: Don't determine local qemu state by remote master-score, 
> query
>   directly via qmp instead
>  -resource-agent: Add max_queue_size parameter for colo-compare
>  -resource-agent: Fix monitor action on secondary returning error during
>   clean shutdown
>  -resource-agent: Fix stop action setting master-score to 0 on primary on
>   clean shutdown
> 
> v2:
>  -use new yank api
>  -drop disk_size parameter
>  -introduce pick_qemu_util function and use it
> 
> Overview:
> 
> Hello Everyone,
> These patches introduce a resource agent for fully automatic management of 
> colo
> and a test suite building upon the resource agent to extensively test colo.
> 
> Test suite features:
> -Tests failover with peer crashing and hanging and failover during checkpoint
> -Tests network using ssh and iperf3
> -Quick test requires no special configuration
> -Network test for testing colo-compare
> -Stress test: failover all the time with network load
> 
> Resource agent features:
> -Fully automatic management of colo
> -Handles many failures: hanging/crashing qemu, replication error, disk error, 
> ...
> -Recovers from hanging qemu by using the "yank" oob command
> -Tracks which node has up-to-date data
> -Works well in clusters with more than 2 nodes
> 
> Run times on my laptop:
> Quick test: 200s
> Network test: 800s (tagged as slow)
> Stress test: 1300s (tagged as slow)
> 
> For the last two tests, the test suite needs access to a network bridge to
> properly test the network, so some parameters need to be given to the test
> run. See tests/acceptance/colo.py for more information.
> 
> Regards,
> Lukas Straub
> 
> Lukas Straub (7):
>   block/quorum.c: stable children names
>   avocado_qemu: Introduce pick_qemu_util to pick qemu utility binaries
>   boot_linux.py: Use pick_qemu_util
>   colo: Introduce resource agent
>   colo: Introduce high-level test suite
>   configure,Makefile: Install colo resource-agent
>   MAINTAINERS: Add myself as maintainer for COLO resource agent
> 
>  MAINTAINERS   |6 +
>  Makefile  |5 +
>  block/quorum.c|   20 +-
>  configure |   10 +
>  scripts/colo-resource-agent/colo  | 1501 +
>  scripts/colo-resource-agent/crm_master|   44 +
>  scripts/colo-resource-agent/crm_resource  |   12 +
>  tests/acceptance/avocado_qemu/__init__.py |   15 +
>  tests/acceptance/boot_linux.py|   11 +-
>  tests/acceptance/colo.py  |  677 ++
>  10 files changed, 2286 insertions(+), 15 deletions(-)
>  create mode 100755 scripts/colo-resource-agent/colo
>  create mode 100755 scripts/colo-resource-agent/crm_master
>  create mode 100755 scripts/colo-resource-agent/crm_resource
>  create mode 100644 tests/acceptance/colo.py
> 
> --
> 2.20.1

Ping...


pgpj34v89dVQe.pgp
Description: OpenPGP digital signature

[PATCH v2 4/7] acpi: add aml_land() and aml_break() primitives

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
 include/hw/acpi/aml-build.h |  2 ++
 hw/acpi/aml-build.c | 16 
 2 files changed, 18 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index d27da03d64..e213fc554c 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -291,6 +291,7 @@ Aml *aml_store(Aml *val, Aml *target);
 Aml *aml_and(Aml *arg1, Aml *arg2, Aml *dst);
 Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst);
 Aml *aml_lor(Aml *arg1, Aml *arg2);
+Aml *aml_land(Aml *arg1, Aml *arg2);
 Aml *aml_shiftleft(Aml *arg1, Aml *count);
 Aml *aml_shiftright(Aml *arg1, Aml *count, Aml *dst);
 Aml *aml_lless(Aml *arg1, Aml *arg2);
@@ -300,6 +301,7 @@ Aml *aml_increment(Aml *arg);
 Aml *aml_decrement(Aml *arg);
 Aml *aml_index(Aml *arg1, Aml *idx);
 Aml *aml_notify(Aml *arg1, Aml *arg2);
+Aml *aml_break(void);
 Aml *aml_call0(const char *method);
 Aml *aml_call1(const char *method, Aml *arg1);
 Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f6fbc9b95d..14b41b56f0 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -556,6 +556,15 @@ Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst)
 return build_opcode_2arg_dst(0x7D /* OrOp */, arg1, arg2, dst);
 }
 
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLAnd */
+Aml *aml_land(Aml *arg1, Aml *arg2)
+{
+Aml *var = aml_opcode(0x90 /* LandOp */);
+aml_append(var, arg1);
+aml_append(var, arg2);
+return var;
+}
+
 /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLOr */
 Aml *aml_lor(Aml *arg1, Aml *arg2)
 {
@@ -629,6 +638,13 @@ Aml *aml_notify(Aml *arg1, Aml *arg2)
 return var;
 }
 
+/* ACPI 1.0b: 16.2.5.3 Type 1 Opcodes Encoding: DefBreak */
+Aml *aml_break(void)
+{
+Aml *var = aml_opcode(0xa5 /* BreakOp */);
+return var;
+}
+
 /* helper to call method without argument */
 Aml *aml_call0(const char *method)
 {
-- 
2.26.2

Re: [PATCH v3 0/7] colo: Introduce resource agent and test suite/CI

[Philippe Mathieu-Daudé]

On 8/18/20 2:27 PM, Lukas Straub wrote:
> On Tue, 4 Aug 2020 12:46:29 +0200
> Lukas Straub  wrote:
> 
>> Hello Everyone,
>> So here is v3. Patch 1 can already be merged independently of the others.
>> Please review.
>>
>> Regards,
>> Lukas Straub
>>
>> Based-on: 
>> "Introduce 'yank' oob qmp command to recover from hanging qemu"
>>
>> Changes:
>>
>> v3:
>>  -resource-agent: Don't determine local qemu state by remote master-score, 
>> query
>>   directly via qmp instead
>>  -resource-agent: Add max_queue_size parameter for colo-compare
>>  -resource-agent: Fix monitor action on secondary returning error during
>>   clean shutdown
>>  -resource-agent: Fix stop action setting master-score to 0 on primary on
>>   clean shutdown
>>
>> v2:
>>  -use new yank api
>>  -drop disk_size parameter
>>  -introduce pick_qemu_util function and use it
>>
>> Overview:
>>
>> Hello Everyone,
>> These patches introduce a resource agent for fully automatic management of 
>> colo
>> and a test suite building upon the resource agent to extensively test colo.
>>
>> Test suite features:
>> -Tests failover with peer crashing and hanging and failover during checkpoint
>> -Tests network using ssh and iperf3
>> -Quick test requires no special configuration
>> -Network test for testing colo-compare
>> -Stress test: failover all the time with network load
>>
>> Resource agent features:
>> -Fully automatic management of colo
>> -Handles many failures: hanging/crashing qemu, replication error, disk 
>> error, ...
>> -Recovers from hanging qemu by using the "yank" oob command
>> -Tracks which node has up-to-date data
>> -Works well in clusters with more than 2 nodes
>>
>> Run times on my laptop:
>> Quick test: 200s
>> Network test: 800s (tagged as slow)
>> Stress test: 1300s (tagged as slow)
>>
>> For the last two tests, the test suite needs access to a network bridge to
>> properly test the network, so some parameters need to be given to the test
>> run. See tests/acceptance/colo.py for more information.
>>
>> Regards,
>> Lukas Straub
>>
>> Lukas Straub (7):
>>   block/quorum.c: stable children names
>>   avocado_qemu: Introduce pick_qemu_util to pick qemu utility binaries
>>   boot_linux.py: Use pick_qemu_util
>>   colo: Introduce resource agent
>>   colo: Introduce high-level test suite
>>   configure,Makefile: Install colo resource-agent
>>   MAINTAINERS: Add myself as maintainer for COLO resource agent
>>
>>  MAINTAINERS   |6 +
>>  Makefile  |5 +
>>  block/quorum.c|   20 +-
>>  configure |   10 +
>>  scripts/colo-resource-agent/colo  | 1501 +
>>  scripts/colo-resource-agent/crm_master|   44 +
>>  scripts/colo-resource-agent/crm_resource  |   12 +
>>  tests/acceptance/avocado_qemu/__init__.py |   15 +
>>  tests/acceptance/boot_linux.py|   11 +-
>>  tests/acceptance/colo.py  |  677 ++
>>  10 files changed, 2286 insertions(+), 15 deletions(-)
>>  create mode 100755 scripts/colo-resource-agent/colo
>>  create mode 100755 scripts/colo-resource-agent/crm_master
>>  create mode 100755 scripts/colo-resource-agent/crm_resource
>>  create mode 100644 tests/acceptance/colo.py
>>
>> --
>> 2.20.1
> 
> Ping...
> 

Cleber, Wainer, can you have a look at tests/acceptance/colo.py please?

Re: [PATCH] Makefile: Let the 'help' target list the helper targets

[Philippe Mathieu-Daudé]

Hi Mirek,

On 4/23/20 12:43 PM, Philippe Mathieu-Daudé wrote:
> List the name of the helper targets when calling 'make help',
> along with the tool targets:
> 
>   $ make help
>   [...]
> 
>   Helper targets:
> fsdev/virtfs-proxy-helper  - Build virtfs-proxy-helper
> scsi/qemu-pr-helper- Build qemu-pr-helper
> qemu-bridge-helper - Build qemu-bridge-helper
> vhost-user-gpu - Build vhost-user-gpu
> virtiofsd  - Build virtiofsd
> 
>   Tools targets:
> qemu-ga- Build qemu-ga tool
> qemu-keymap- Build qemu-keymap tool
> elf2dmp- Build elf2dmp tool
> ivshmem-client - Build ivshmem-client tool
> ivshmem-server - Build ivshmem-server tool
> qemu-nbd   - Build qemu-nbd tool
> qemu-storage-daemon- Build qemu-storage-daemon tool
> qemu-img   - Build qemu-img tool
> qemu-io- Build qemu-io tool
> qemu-edid  - Build qemu-edid tool
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  configure | 5 +++--
>  Makefile  | 9 +++--
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index 23b5e93752..caf880c38e 100755
> --- a/configure
> +++ b/configure
> @@ -6374,7 +6374,7 @@ if test "$softmmu" = yes ; then
>if test "$linux" = yes; then
>  if test "$virtfs" != no && test "$cap_ng" = yes && test "$attr" = yes ; 
> then
>virtfs=yes
> -  tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
> +  helpers="$helpers fsdev/virtfs-proxy-helper\$(EXESUF)"
>  else
>if test "$virtfs" = yes; then
>  error_exit "VirtFS requires libcap-ng devel and libattr devel"
> @@ -6389,7 +6389,7 @@ if test "$softmmu" = yes ; then
>fi
>mpath=no
>  fi
> -tools="$tools scsi/qemu-pr-helper\$(EXESUF)"
> +helpers="$helpers scsi/qemu-pr-helper\$(EXESUF)"

Paolo said on IRC this patch didn't follow the recommendation
from https://www.mail-archive.com/qemu-devel@nongnu.org/msg524586.html
and moved qemu-pr-helper from /usr/bin to /usr/libexec.

FYI virtfs-proxy-helper might be also affected.

>else
>  if test "$virtfs" = yes; then
>error_exit "VirtFS is supported only on Linux"
> @@ -7630,6 +7630,7 @@ else
>QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
>  fi
>  
> +echo "HELPERS=$helpers" >> $config_host_mak
>  echo "TOOLS=$tools" >> $config_host_mak
>  echo "ROMS=$roms" >> $config_host_mak
>  echo "MAKE=$make" >> $config_host_mak
> diff --git a/Makefile b/Makefile
> index 8a9113e666..021a0cd491 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -336,9 +336,9 @@ $(call set-vpath, $(SRC_PATH))
>  LIBS+=-lz $(LIBS_TOOLS)
>  
>  vhost-user-json-y =
> -HELPERS-y =
> +HELPERS-y = $(HELPERS)
>  
> -HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) = 
> qemu-bridge-helper$(EXESUF)
> +HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) += 
> qemu-bridge-helper$(EXESUF)
>  
>  ifeq ($(CONFIG_LINUX)$(CONFIG_VIRGL)$(CONFIG_GBM)$(CONFIG_TOOLS),)
>  HELPERS-y += vhost-user-gpu$(EXESUF)
> @@ -1255,6 +1255,11 @@ endif
>   $(foreach t, $(TARGET_DIRS), \
>   $(call print-help-run,$(t)/all,Build for $(t));) \
>   echo '')
> + @$(if $(HELPERS-y), \
> + echo 'Helper targets:'; \
> + $(foreach t, $(HELPERS-y), \
> + $(call print-help-run,$(t),Build $(shell basename $(t)));) \
> + echo '')
>   @$(if $(TOOLS), \
>   echo 'Tools targets:'; \
>   $(foreach t, $(TOOLS), \
> 

Re: hw-display-qxl.so: undefined symbol: qemu_qxl_io_log_semaphore

[Daniel P . Berrangé]

On Tue, Aug 18, 2020 at 01:15:21PM +0100, Daniel P. Berrangé wrote:
> IOW, new systemtap 4.4 is exposing a long standing design
> flaw in QEMU's probe.
> 
> I'm gong to ask the systemtap maintainers for an opinion on
> this behaviour change none the less.

Reported to systemtap maintainers here:

  https://bugzilla.redhat.com/show_bug.cgi?id=1869642

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

Re: virtio-vsock requires 'disable-legacy=on' in QEMU 5.1

[Cornelia Huck]

On Mon, 17 Aug 2020 15:11:28 +0200
Stefano Garzarella  wrote:

> On Mon, Aug 17, 2020 at 12:27:46PM +0200, Cornelia Huck wrote:
> > On Thu, 13 Aug 2020 14:04:15 +0200
> > Stefano Garzarella  wrote:
> >   
> > > On Thu, Aug 13, 2020 at 12:37:37PM +0200, Cornelia Huck wrote:  
> > > > On Thu, 13 Aug 2020 12:24:30 +0200
> > > > Stefano Garzarella  wrote:
> > > > 
> > > > > On Thu, Aug 13, 2020 at 11:28:20AM +0200, Cornelia Huck wrote:
> > > > > > We basically have three possible ways to deal with this:
> > > > > > 
> > > > > > - Force it to modern (i.e., what you have been doing; would need the
> > > > > >   equivalent changes in ccw as well.)  
> > > > > 
> > > > > Oo, thanks for pointing out ccw!
> > > > > I don't know ccw well, in this case should we set dev->max_rev to 1 
> > > > > or 2
> > > > > to force to modern?
> > > > 
> > > > No, ->max_rev is the wrong side of the limit :) You want
> > > 
> > > Well :-) Thanks!
> > >   
> > > > 
> > > > ccw_dev->force_revision_1 = true;
> > > > 
> > > > in _instance_init() (see e.g. virtio-ccw-gpu.c).
> > > > 
> > > > > 
> > > > > >   Pro: looks like the cleanest approach.
> > > > > >   Con: not sure if we would need backwards compatibility support,
> > > > > >   which looks hairy.  
> > > > > 
> > > > > Not sure too.
> > > > 
> > > > Yes, I'm not sure at all how to handle user-specified values for
> > > > legacy/modern.  
> > 
> > Thinking a bit more about it, I'm not sure whether we even *can*
> > provide backwards compatibility: we have different autoconfigurations
> > for PCI based upon where it is plugged, and ccw does not have a way to
> > turn legacy on/off, except from within the code.  
> 
> Yes, I discovered today for example that the PCIe bus set auto-legacy
> mode to off.

And vhost-vsock actually really seems to be modern-only, see below.

> 
> >   
> > > > 
> > > > > 
> > > > > > - Add vsock to the list of devices with legacy support.
> > > > > >   Pro: Existing setups continue to work.
> > > > > >   Con: If vsock is really virtio-1-only, we still carry around
> > > > > >   possibly broken legacy support.  
> > > > > 
> > > > > I'm not sure it is virtio-1-only, but virtio-vsock was introduced in
> > > > > 2016, so I supposed it is modern-only.
> > > > 
> > > > Yes, I would guess so as well.
> > > > 
> > > > > 
> > > > > How can I verify that? Maybe forcing legacy mode and run some tests.  
> > > > >   
> > > > 
> > > > Probably yes. The likeliest area with issues is probably endianness, so
> > > > maybe with something big endian in the mix?
> > > > 
> > > 
> > > Yeah, I'll try this setup!

Ok, I tried this now with an x86 host and an s390x guest. Reverted the
checking commit, tried both with a -ccw and a -pci device and your ncat
example.
- When using virtio-1, both devices work fine.
- When using the -pci device with disable-modern=yes, I get "reset by
  peer".
- When using the -ccw device with max_revision=0, I get an instant
  timeout.

Smells like endianness problems (aka weird things are happening).

Also noticed that vhost-vsock-ccw does not have an immediate problem,
even with the commit: The code only checks whether the device has been
forced to legacy, not whether legacy is allowed (which cannot be
controlled by the user anyway). Probably best to address after we've
dealt with the vhost-vsock issue and made sure that there are no other
problems.

> > >   
> > > > > 
> > > > > > - Do nothing, have users force legacy off. Bad idea, as ccw has no 
> > > > > > way
> > > > > >   to do that on the command line.
> > > > > > 
> > > > > > The first option is probably best.  
> > 
> > The first option is now "force modern, but with no backwards
> > compatibility", which is not that great; but "allow legacy, even though
> > it should not exist" is not particularly appealing, either... what a
> > mess :(  
> 
> Yeah, it's a mess :-( anyway I still prefer option 1, it seems a little
> bit more correct to me.

It seems to me that the status before this was "works by accident, but
only if we're not negotiating to legacy, or the guest/host are both
little endian". IOW, no visible breakage for most people (or we'd
probably have heard of it already). Now we have a setup that's correct,
but forces users to adapt their QEMU command lines. Option 1 would
eliminate the need to do that, but would cause possibly
not-really-fixable migration issues (you can probably deal with that
manually, detaching and re-attaching the device as a last resort.)

So, force modern, probably also remove the -transitional device type,
and put a prominent explanation into the change log?

Re: [PATCH 07/11] vfio/platform: Remove dead assignment in vfio_intp_interrupt()

[Stefan Hajnoczi]

On Thu, Aug 13, 2020 at 09:18:59PM +0200, Auger Eric wrote:
> Hi Alex,
> 
> On 8/13/20 9:15 PM, Alex Williamson wrote:
> > On Thu, 13 Aug 2020 20:02:45 +0200
> > Auger Eric  wrote:
> > 
> >> Hi Alex,
> >>
> >> On 8/13/20 6:59 PM, Alex Williamson wrote:
> >>> On Thu, 13 Aug 2020 15:37:08 +0800
> >>> Chen Qun  wrote:
> >>>   
>  Clang static code analyzer show warning:
>  hw/vfio/platform.c:239:9: warning: Value stored to 'ret' is never read
>  ret = event_notifier_test_and_clear(intp->interrupt);
>  ^ ~~
> 
>  Reported-by: Euler Robot 
>  Signed-off-by: Chen Qun 
>  ---
>  Cc: Alex Williamson 
>  Cc: Eric Auger 
>  ---
>   hw/vfio/platform.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
>  diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
>  index ac2cefc9b1..869ed2c39d 100644
>  --- a/hw/vfio/platform.c
>  +++ b/hw/vfio/platform.c
>  @@ -236,7 +236,7 @@ static void vfio_intp_interrupt(VFIOINTp *intp)
>   trace_vfio_intp_interrupt_set_pending(intp->pin);
>   QSIMPLEQ_INSERT_TAIL(&vdev->pending_intp_queue,
>    intp, pqnext);
>  -ret = event_notifier_test_and_clear(intp->interrupt);
>  +event_notifier_test_and_clear(intp->interrupt);
>   return;
>   }  
> >>>
> >>> Testing that an event is pending in our notifier is generally a
> >>> prerequisite to doing anything in the interrupt handler, I don't
> >>> understand why we're just consuming it and ignoring the return value.
> >>> The above is in the delayed handling branch of the function, but the
> >>> normal non-delayed path would only go on to error_report() if the
> >>> notifier is not pending and then inject an interrupt anyway.  This all
> >>> seems rather suspicious and it's a unique pattern among the vfio
> >>> callers of this function.  Is there a more fundamental bug that this
> >>> function should perform this test once and return without doing
> >>> anything if it's called spuriously, ie. without a notifier pending?
> >>> Thanks,  
> >>
> >> Hum that's correct that other VFIO call sites do the check. My
> >> understanding was that this could not fail in this case as, if we
> >> entered the handler there was something to be cleared. In which
> >> situation can this fail?
> > 
> > I'm not sure what the right answer is, I see examples either way
> > looking outside of vfio code.  On one hand, maybe we never get called
> > spuriously, on the other if it's the callee's responsibility to drain
> > events from the fd and we have it readily accessible whether there were
> > any events pending, why would we inject an interrupt if the result that
> > we have in hand shows no pending events?  The overhead of returning
> > based on that result is minuscule.
> 
> I agree
> > 
> > qemu_set_fd_handler() is a wrapper for aio_set_fd_handler().  Stefan is
> > a possible defacto maintainer of some of the aio code.  Stefan, do you
> > have thoughts on whether callbacks from event notifier fds should
> > consider spurious events?  Thanks,
> 
> Indeed I saw that for instance block/nvme.c nvme_handle_event is not
> checking the result.
> 
> Let's wait for Stefan's answer ...

vfio_intp_interrupt() will always read a non-zero eventfd value, based
on these assumptions:

intp->interrupt is "readable" since vfio_intp_interrupt() is called by
the AioContext (event loop). "readable" does not guarantee that data can
actually be read because it also includes error events:

  new_node->pfd.events = (io_read ? G_IO_IN | G_IO_HUP | G_IO_ERR : 0);

However, I think we can exclude the error case for the VFIO interrupt
eventfds because there are no error cases for eventfds (unlike socket
disconnection, for example).

The other important assumption is that only one thread on the host is
monitoring the eventfd for activity.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v2 0/7] x86: fix cpu hotplug with secure boot

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200818122208.1243901-1-imamm...@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTcheck-unit: tests/test-char
Unexpected error in object_property_try_add() at 
/tmp/qemu-test/src/qom/object.c:1181:
attempt to add duplicate property 'serial-id' to object (type 'container')
ERROR test-char - too few tests run (expected 38, got 9)
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs
  TESTiotest-qcow2: 024
  TESTiotest-qcow2: 025
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=f2b52e779ea044a6af8347ae49d4785e', '-u', 
'1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-e651x4a8/src/docker-src.2020-08-18-08.43.04.23321:/var/tmp/qemu:z,ro',
 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=f2b52e779ea044a6af8347ae49d4785e
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-e651x4a8/src'
make: *** [docker-run-test-quick@centos7] Error 2

real13m7.140s
user0m9.328s


The full log is available at
http://patchew.org/logs/20200818122208.1243901-1-imamm...@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v2 0/7] x86: fix cpu hotplug with secure boot

[Philippe Mathieu-Daudé]

On 8/18/20 2:56 PM, no-re...@patchew.org wrote:
> Patchew URL: 
> https://patchew.org/QEMU/20200818122208.1243901-1-imamm...@redhat.com/
> 
> 
> 
> Hi,
> 
> This series failed the docker-quick@centos7 build test. Please find the 
> testing commands and
> their output below. If you have Docker installed, you can probably reproduce 
> it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> make docker-image-centos7 V=1 NETWORK=1
> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
>   TESTcheck-unit: tests/test-char
> Unexpected error in object_property_try_add() at 
> /tmp/qemu-test/src/qom/object.c:1181:
> attempt to add duplicate property 'serial-id' to object (type 'container')

Unrelated error.

> ERROR test-char - too few tests run (expected 38, got 9)
> make: *** [check-unit] Error 1
> make: *** Waiting for unfinished jobs
>   TESTiotest-qcow2: 024
>   TESTiotest-qcow2: 025
> ---
> raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
> '--label', 'com.qemu.instance.uuid=f2b52e779ea044a6af8347ae49d4785e', '-u', 
> '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
> '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', 
> '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
> '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
> '/var/tmp/patchew-tester-tmp-e651x4a8/src/docker-src.2020-08-18-08.43.04.23321:/var/tmp/qemu:z,ro',
>  'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
> status 2.
> filter=--filter=label=com.qemu.instance.uuid=f2b52e779ea044a6af8347ae49d4785e
> make[1]: *** [docker-run] Error 1
> make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-e651x4a8/src'
> make: *** [docker-run-test-quick@centos7] Error 2
> 
> real13m7.140s
> user0m9.328s
> 
> 
> The full log is available at
> http://patchew.org/logs/20200818122208.1243901-1-imamm...@redhat.com/testing.docker-quick@centos7/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-de...@redhat.com
> 

[RFC v4 0/2] memory: Delete assertion in memory_region_unregister_iommu_notifier

[Eugenio Pérez]

I am able to hit this assertion when a Red Hat 7 guest virtio_net device
raises an "Invalidation" of all the TLB entries. This happens in the
guest's startup if 'intel_iommu=on' argument is passed to the guest
kernel and right IOMMU/ATS devices are declared in qemu's command line.

Command line:
/home/qemu/x86_64-softmmu/qemu-system-x86_64 -name \
guest=rhel7-test,debug-threads=on -machine \
pc-q35-5.1,accel=kvm,usb=off,dump-guest-core=off,kernel_irqchip=split \
-cpu \
Broadwell,vme=on,ss=on,vmx=on,f16c=on,rdrand=on,hypervisor=on,arat=on,tsc-adjust=on,umip=on,arch-capabilities=on,xsaveopt=on,pdpe1gb=on,abm=on,skip-l1dfl-vmentry=on,rtm=on,hle=on
 \
-m 8096 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid \
d022ecbf-679e-4755-87ce-eb87fc5bbc5d -display none -no-user-config \
-nodefaults -rtc base=utc,driftfix=slew -global \
kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global \
ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on \
-device intel-iommu,intremap=on,device-iotlb=on -device \
pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x1 
\
-device \
pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \
-device \
pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \
-device \
pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \
-device \
pcie-root-port,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x1.0x4 \
-device \
pcie-root-port,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x1.0x5 \
-device \
pcie-root-port,port=0xe,chassis=7,id=pci.7,bus=pcie.0,addr=0x1.0x6 \
-device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 -device \
virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 -drive \
file=/home/virtio-test2.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \
-device \
virtio-blk-pci,scsi=off,bus=pci.4,addr=0x0,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 \
-netdev tap,id=hostnet0,vhost=on,vhostforce=on -device \
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:0d:1d:f2,bus=pci.1,addr=0x0,iommu_platform=on,ats=on
 \
-device virtio-balloon-pci,id=balloon0,bus=pci.5,addr=0x0 -object \
rng-random,id=objrng0,filename=/dev/urandom -device \
virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.6,addr=0x0 -s -msg \
timestamp=on

#0  0x7521370f in raise () at /lib64/libc.so.6
#1  0x751fdb25 in abort () at /lib64/libc.so.6
#2  0x751fd9f9 in _nl_load_domain.cold.0 () at /lib64/libc.so.6
#3  0x7520bcc6 in .annobin_assert.c_end () at /lib64/libc.so.6
#4  0x55888171 in memory_region_notify_one
(notifier=0x7ffde0487fa8, entry=0x7ffde5dfe200) at
/home/qemu/memory.c:1918
#5  0x55888247 in memory_region_notify_iommu
(iommu_mr=0x56f6c0b0, iommu_idx=0, entry=...) at
/home/qemu/memory.c:1941
#6  0x55951c8d in vtd_process_device_iotlb_desc
(s=0x57609000, inv_desc=0x7ffde5dfe2d0)
at /home/qemu/hw/i386/intel_iommu.c:2468
#7  0x55951e6a in vtd_process_inv_desc (s=0x57609000) at
/home/qemu/hw/i386/intel_iommu.c:2531
#8  0x55951fa5 in vtd_fetch_inv_desc (s=0x57609000) at
/home/qemu/hw/i386/intel_iommu.c:2563
#9  0x559520e5 in vtd_handle_iqt_write (s=0x57609000) at
/home/qemu/hw/i386/intel_iommu.c:2590
#10 0x55952b45 in vtd_mem_write (opaque=0x57609000,
addr=136, val=2688, size=4) at /home/qemu/hw/i386/intel_iommu.c:2837
#11 0x55883e17 in memory_region_write_accessor
(mr=0x57609330, addr=136, value=0x7ffde5dfe478, size=4,
shift=0, mask=4294967295, attrs=...) at /home/qemu/memory.c:483
#12 0x5588401d in access_with_adjusted_size
(addr=136, value=0x7ffde5dfe478, size=4, access_size_min=4,
access_size_max=8, access_fn=
0x55883d38 , mr=0x57609330,
attrs=...) at /home/qemu/memory.c:544
#13 0x55886f37 in memory_region_dispatch_write
(mr=0x57609330, addr=136, data=2688, op=MO_32, attrs=...)
at /home/qemu/memory.c:1476
#14 0x55827a03 in flatview_write_continue
(fv=0x7ffdd8503150, addr=4275634312, attrs=...,
ptr=0x77ff0028, len=4, addr1=136, l=4, mr=0x57609330) at
/home/qemu/exec.c:3146
#15 0x55827b48 in flatview_write (fv=0x7ffdd8503150,
addr=4275634312, attrs=..., buf=0x77ff0028, len=4)
at /home/qemu/exec.c:3186
#16 0x55827e9d in address_space_write
(as=0x567ca640 , addr=4275634312,
attrs=..., buf=0x77ff0028, len=4) at /home/qemu/exec.c:3277
#17 0x55827f0a in address_space_rw
(as=0x567ca640 , addr=4275634312,
attrs=..., buf=0x77ff0028, len=4, is_write=true)
at /home/qemu/exec.c:3287
#18 0x5589b633 in kvm_cpu_exec (cpu=0x56b65640) at
/home/qemu/accel/kvm/kvm-all.c:2511
#19 0x55876ba8 in qemu_kvm_cpu_thread_fn (arg=0x56b65640)
at /home/qemu/cpus.c:1284
#20 0x55dafff1 in qemu_thread_start (args=0x56b8c3b0) at
util/qemu-thread-posix.c:521
#21 0x755a62de in start_thread () at /lib64/libpthread.so.0
#22 0x752d7e83 in clone () at /lib64/

Re: [PATCH v2 4/7] acpi: add aml_land() and aml_break() primitives

[Philippe Mathieu-Daudé]

On 8/18/20 2:22 PM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  include/hw/acpi/aml-build.h |  2 ++
>  hw/acpi/aml-build.c | 16 
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index d27da03d64..e213fc554c 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -291,6 +291,7 @@ Aml *aml_store(Aml *val, Aml *target);
>  Aml *aml_and(Aml *arg1, Aml *arg2, Aml *dst);
>  Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst);
>  Aml *aml_lor(Aml *arg1, Aml *arg2);
> +Aml *aml_land(Aml *arg1, Aml *arg2);
>  Aml *aml_shiftleft(Aml *arg1, Aml *count);
>  Aml *aml_shiftright(Aml *arg1, Aml *count, Aml *dst);
>  Aml *aml_lless(Aml *arg1, Aml *arg2);
> @@ -300,6 +301,7 @@ Aml *aml_increment(Aml *arg);
>  Aml *aml_decrement(Aml *arg);
>  Aml *aml_index(Aml *arg1, Aml *idx);
>  Aml *aml_notify(Aml *arg1, Aml *arg2);
> +Aml *aml_break(void);
>  Aml *aml_call0(const char *method);
>  Aml *aml_call1(const char *method, Aml *arg1);
>  Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index f6fbc9b95d..14b41b56f0 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -556,6 +556,15 @@ Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst)
>  return build_opcode_2arg_dst(0x7D /* OrOp */, arg1, arg2, dst);
>  }
>  
> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLAnd */
> +Aml *aml_land(Aml *arg1, Aml *arg2)
> +{
> +Aml *var = aml_opcode(0x90 /* LandOp */);
> +aml_append(var, arg1);
> +aml_append(var, arg2);
> +return var;
> +}
> +
>  /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLOr */
>  Aml *aml_lor(Aml *arg1, Aml *arg2)
>  {
> @@ -629,6 +638,13 @@ Aml *aml_notify(Aml *arg1, Aml *arg2)
>  return var;
>  }
>  
> +/* ACPI 1.0b: 16.2.5.3 Type 1 Opcodes Encoding: DefBreak */
> +Aml *aml_break(void)
> +{
> +Aml *var = aml_opcode(0xa5 /* BreakOp */);
> +return var;
> +}
> +
>  /* helper to call method without argument */
>  Aml *aml_call0(const char *method)
>  {
> 

[RFC v4 2/2] memory: Skip bad range assertion if notifier is DEVIOTLB type

[Eugenio Pérez]

Signed-off-by: Eugenio Pérez 
---
 hw/virtio/vhost.c |  2 +-
 include/exec/memory.h |  2 ++
 softmmu/memory.c  | 15 +--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1a1384e7a6..6ca168b47e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -729,7 +729,7 @@ static void vhost_iommu_region_add(MemoryListener *listener,
 iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr,
MEMTXATTRS_UNSPECIFIED);
 iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
-IOMMU_NOTIFIER_UNMAP,
+IOMMU_NOTIFIER_DEVIOTLB,
 section->offset_within_region,
 int128_get64(end),
 iommu_idx);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index ed99a80f17..74ab71543a 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -87,6 +87,8 @@ typedef enum {
 IOMMU_NOTIFIER_UNMAP = 0x1,
 /* Notify entry changes (newly created entries) */
 IOMMU_NOTIFIER_MAP = 0x2,
+/* Notify changes on device IOTLB entries */
+IOMMU_NOTIFIER_DEVIOTLB = 0x04,
 } IOMMUNotifierFlag;
 
 #define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP)
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 0043791361..bbc910b129 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1895,6 +1895,7 @@ void memory_region_notify_one_iommu(IOMMUNotifier 
*notifier,
 {
 IOMMUNotifierFlag request_flags;
 hwaddr entry_end = entry->iova + entry->addr_mask;
+IOMMUTLBEntry tmp = *entry;
 
 /*
  * Skip the notification if the notification does not overlap
@@ -1904,16 +1905,26 @@ void memory_region_notify_one_iommu(IOMMUNotifier 
*notifier,
 return;
 }
 
-assert(entry->iova >= notifier->start && entry_end <= notifier->end);
+if (notifier->notifier_flags & IOMMU_NOTIFIER_DEVIOTLB) {
+/* Crop (iova, addr_mask) to range */
+tmp.iova = MAX(tmp.iova, notifier->start);
+tmp.addr_mask = MIN(entry_end, notifier->end) - tmp.iova;
+/* Confirm no underflow */
+assert(MIN(entry_end, notifier->end) >= tmp.iova);
+} else {
+assert(entry->iova >= notifier->start && entry_end <= notifier->end);
+}
 
 if (entry->perm & IOMMU_RW) {
 request_flags = IOMMU_NOTIFIER_MAP;
+} else if (notifier->notifier_flags == IOMMU_NOTIFIER_DEVIOTLB) {
+request_flags = IOMMU_NOTIFIER_DEVIOTLB;
 } else {
 request_flags = IOMMU_NOTIFIER_UNMAP;
 }
 
 if (notifier->notifier_flags & request_flags) {
-notifier->notify(notifier, entry);
+notifier->notify(notifier, &tmp);
 }
 }
 
-- 
2.18.1

[RFC v4 1/2] memory: Rename memory_region_notify_one to memory_region_notify_one_iommu

[Eugenio Pérez]

Signed-off-by: Eugenio Pérez 
---
 hw/arm/smmu-common.c  | 2 +-
 hw/arm/smmuv3.c   | 2 +-
 hw/i386/intel_iommu.c | 4 ++--
 include/exec/memory.h | 6 +++---
 softmmu/memory.c  | 6 +++---
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index e13a5f4a7c..3d7fc87ff8 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -396,7 +396,7 @@ static void smmu_unmap_notifier_range(IOMMUNotifier *n)
 entry.perm = IOMMU_NONE;
 entry.addr_mask = n->end - n->start;
 
-memory_region_notify_one(n, &entry);
+memory_region_notify_one_iommu(n, &entry);
 }
 
 /* Unmap all notifiers attached to @mr */
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 57a79df55b..cc3d4de319 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -838,7 +838,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
 entry.addr_mask = (1 << tt->granule_sz) - 1;
 entry.perm = IOMMU_NONE;
 
-memory_region_notify_one(n, &entry);
+memory_region_notify_one_iommu(n, &entry);
 }
 
 /* invalidate an asid/iova tuple in all mr's */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5284bb68b6..d68e91c729 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3498,7 +3498,7 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, 
IOMMUNotifier *n)
 /* This field is meaningless for unmap */
 entry.translated_addr = 0;
 
-memory_region_notify_one(n, &entry);
+memory_region_notify_one_iommu(n, &entry);
 
 start += mask;
 remain -= mask;
@@ -3536,7 +3536,7 @@ static void vtd_address_space_refresh_all(IntelIOMMUState 
*s)
 
 static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private)
 {
-memory_region_notify_one((IOMMUNotifier *)private, entry);
+memory_region_notify_one_iommu((IOMMUNotifier *)private, entry);
 return 0;
 }
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 307e527835..ed99a80f17 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -226,7 +226,7 @@ enum IOMMUMemoryRegionAttr {
  * The IOMMU implementation must use the IOMMU notifier infrastructure
  * to report whenever mappings are changed, by calling
  * memory_region_notify_iommu() (or, if necessary, by calling
- * memory_region_notify_one() for each registered notifier).
+ * memory_region_notify_one_iommu() for each registered notifier).
  *
  * Conceptually an IOMMU provides a mapping from input address
  * to an output TLB entry. If the IOMMU is aware of memory transaction
@@ -1274,7 +1274,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion 
*iommu_mr,
 IOMMUTLBEntry entry);
 
 /**
- * memory_region_notify_one: notify a change in an IOMMU translation
+ * memory_region_notify_one_iommu: notify a change in an IOMMU translation
  *   entry to a single notifier
  *
  * This works just like memory_region_notify_iommu(), but it only
@@ -1285,7 +1285,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion 
*iommu_mr,
  * replaces all old entries for the same virtual I/O address range.
  * Deleted entries have .@perm == 0.
  */
-void memory_region_notify_one(IOMMUNotifier *notifier,
+void memory_region_notify_one_iommu(IOMMUNotifier *notifier,
   IOMMUTLBEntry *entry);
 
 /**
diff --git a/softmmu/memory.c b/softmmu/memory.c
index af25987518..0043791361 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1890,8 +1890,8 @@ void memory_region_unregister_iommu_notifier(MemoryRegion 
*mr,
 memory_region_update_iommu_notify_flags(iommu_mr, NULL);
 }
 
-void memory_region_notify_one(IOMMUNotifier *notifier,
-  IOMMUTLBEntry *entry)
+void memory_region_notify_one_iommu(IOMMUNotifier *notifier,
+IOMMUTLBEntry *entry)
 {
 IOMMUNotifierFlag request_flags;
 hwaddr entry_end = entry->iova + entry->addr_mask;
@@ -1927,7 +1927,7 @@ void memory_region_notify_iommu(IOMMUMemoryRegion 
*iommu_mr,
 
 IOMMU_NOTIFIER_FOREACH(iommu_notifier, iommu_mr) {
 if (iommu_notifier->iommu_idx == iommu_idx) {
-memory_region_notify_one(iommu_notifier, &entry);
+memory_region_notify_one_iommu(iommu_notifier, &entry);
 }
 }
 }
-- 
2.18.1

Re: [RFC PATCH] os-posix: fix regression for install-less datadir location

[Peter Maydell]

On Tue, 18 Aug 2020 at 10:11, Paolo Bonzini  wrote:
>
> On 16/07/20 16:11, Marc-André Lureau wrote:
> > os_find_datadir() used to check the ../share/qemu location (regardless
> > of CONFIG_QEMU_DATADIR). It turns out that people rely on that location
> > for running qemu in an arbitrary "install-less/portable" fashion. Change
> > the logic to return that directory as a last resort.
> >
> > (this is an alternative to the patch "[PATCH 1/1] os_find_datadir: search
> > as in version 4.2" from Joe Slater)
> >
> > Fixes: 6dd2dacedd83d12328 ("os-posix: simplify os_find_datadir")
> > Cc: Paolo Bonzini 
> > Signed-off-by: Marc-André Lureau 
>
> For 5.2 I plan to support fully relocatable installs, so I think this
> will not be needed.
>
> The idea is to write a function like
>
> char *get_relocatable_path(const char *dir);
>
> That takes CONFIG_QEMU_*DIR as the argument, turns it into a path
> relative to bindir, and tacks it to the end of qemu_get_exec_dir().
>
> So for example all references to CONFIG_QEMU_DATADIR would invoke
> get_relocatable_path(CONFIG_QEMU_DATADIR), which would return something
> like "/usr/bin/../share/qemu".

Unless you have that series ready-to-roll, I think it would
be useful to just fix the regression (and cc qemu-stable on it
so it gets backported to 5.1.1) for the moment.

thanks
-- PMM

Re: [PATCH 00/18] hw/riscv: Add Microchip PolarFire SoC Icicle Kit board support

[via]

On 8/18/20 7:17 AM, Anup Patel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
>
> On Tue, Aug 18, 2020 at 1:23 AM  wrote:
>> On 8/17/20 8:28 PM, Alistair Francis wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
>>> content is safe
>>>
>>> On Mon, Aug 17, 2020 at 11:12 AM via  wrote:
 Hi Anup,

 On 8/17/20 11:30 AM, Bin Meng wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
>
> Hi Anup,
>
> On Sat, Aug 15, 2020 at 1:44 AM Anup Patel  wrote:
>> On Fri, Aug 14, 2020 at 10:12 PM Bin Meng  wrote:
>>> From: Bin Meng 
>>>
>>> This adds support for Microchip PolarFire SoC Icicle Kit board.
>>> The Icicle Kit board integrates a PolarFire SoC, with one SiFive's
>>> E51 plus four U54 cores and many on-chip peripherals and an FPGA.
>> Nice Work !!! This is very helpful.
> Thanks!
>
>> The Microchip HSS is quite convoluted. It has:
>> 1. DDR Init
>> 2. Boot device support
>> 3. SBI support using OpenSBI as library
>> 4. Simple TEE support
>>
>> I think point 1) and 2) above should be part of U-Boot SPL.
>> The point 3) can be OpenSBI FW_DYNAMIC.
>>
>> Lastly,for point 4), we are working on a new OpenSBI feature using
>> which we can run independent Secure OS and Non-Secure OS using
>> U-Boot_SPL+OpenSBI (for both SiFive Unleashed and Microchip
>> PolarFire).
>>
>> Do you have plans for adding U-Boot SPL support for this board ??
> + Cyril Jean from Microchip
>
> I will have to leave this question to Cyril to comment.
>
 I currently do not have a plan to support U-Boot SPL. The idea of the
 HSS is to contain all the silicon specific initialization and
 configuration code within the HSS before jumping to U-Boot S-mode. I
 would rather keep all this within the HSS for the time being. I would
 wait until we reach production silicon before attempting to move this to
 U-Boot SPL as the HSS is likely to contain some opaque silicon related
 changes for another while.
>>> That is unfortunate, a lot of work has gone into making the boot flow
>>> simple and easy to use.
>>>
>>> QEMU now includes OpenSBI by default to make it easy for users to boot
>>> Linux. The Icicle Kit board is now the most difficult QEMU board to
>>> boot Linux on. Not to mention it makes it hard to impossible to
>>> support it in standard tool flows such as meta-riscv.
>>>
>>> Alistair
>> If it is such a problem we can add a U-Boot SPL stage and the HSS can be
>> treated as standard SoC ROM code.
> It's not mandatory for U-Boot SPL to have stable DRAM calibration settings
> from the start itself. The initial U-Boot SPL support for most
> platforms (accross
> architectures) usually include basic working DRAM calibration settings which
> is later on updated with separate patches. Also, we don't need all U-Boot
> drivers to be upstreamed in one-go as this can be done in phases.
>
> The advantage we have with PolarFire SoC Icicle board is that we already
> have a U-Boot S-mode. and we believe the OpenSBI generic platform will
> work fine for PolarFire SoC Icicle board so the only thing missing right now
> is the U-Boot SPL support for OpenSource boot-flow.
>
> It will certainly accelerate open-source development if we have boot-flow
> U-Boot_SPL => OpenSBI (FW_DYNAMIC) => U-Boot_S-mode working
> on PolarFire SoC Icicle board. Initially, we just need DRAM, SD/eMMC,
> and Serial port support for U-Boot SPL and U-Boot S-mode. Later on,
> more patches can add ethernet and other booting device drivers to U-Boot.
>
> Regarding security services of HSS, we are working on a OpenSBI
> feature which will allow HSS security services to run as independent
> binary in M-mode (not linked to OpenSBI) and OpenSBI FW_DYNAMIC
> will be a separate binary acting as a secure monitor.
>
> Regards,
> Anup

What I have in mind is that the external memory will be up and running 
by the time we get to U-Boot SPL. In the case of PolarFire SoC the ROM 
code equivalent brings up the DDR memory interface so we do not need to 
worry about this as part of U-Boot.

Sounds to me the component that needs to be upstreamed next is the eMMC 
support so that it can be used as part of U-Boot SPL. Correct?


Regards,

Cyril.

[PATCH] audio/jack: fix use after free segfault

[Geoffrey McRae]

Due to a ridiculous commit in the Jack library, the client may have been
freed already by a secondary audio device recovering its session.

https://github.com/jackaudio/jack2/issues/627

Until there is a proper fix for this we can not risk using the pointer
at all if we have been notified of a shutdown as it may have been freed
by the jack library, as such the close call is commented out to prevent
a use after free segfault.

At this time, this will not cause a memory leak as the recovery routine
will trigger the "cleanup" code in the jack library, however, if this is
ever corrected in the jack library this will need to be revisited.

Signed-off-by: Geoffrey McRae 
---
 audio/jackaudio.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 72ed7c4929..e8faf1bb89 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -563,7 +563,22 @@ static void qjack_client_fini(QJackClient *c)
 /* fallthrough */
 
 case QJACK_STATE_SHUTDOWN:
-jack_client_close(c->client);
+/*
+ * Due to a rediculous commit in the Jack library, the client may have
+ * been freed already.
+ *
+ * Until there is a proper fix for this we can not risk using the
+ * pointer at all if we have been notified of a shutdown, as such the
+ * below line is commented out to prevent a use after free segfault.
+ * This will not cause a memory leak as the recovery routine will 
trigger
+ * the "cleanup" code in the jack library.
+ *
+ * 
https://github.com/jackaudio/jack2/commit/171a3c4a0ddd18d2afae56f3af6291c8e96ee3ac
+ */
+
+//jack_client_close(c->client);
+c->client = NULL;
+
 /* fallthrough */
 
 case QJACK_STATE_DISCONNECTED:
-- 
2.20.1

Re: [PATCH v2] linux-user: Fix 'semop()' and 'semtimedop()' implementation

[Laurent Vivier]

Le 17/08/2020 à 17:23, Filip Bozuta a écrit :
> The implementations of syscalls 'semop()' and 'semtimedop()' in
> file 'syscall.c' use function 'target_to_host_sembuf()' to convert
> values of 'struct sembuf' from host to target. However, before this
> conversion it should be check whether the number of semaphore operations
> 'nsops' is not bigger than maximum allowed semaphor operations per
> syscall: 'SEMOPM'. In these cases, errno 'E2BIG' ("Arg list too long")
> should be set. But the implementation will set errno 'EFAULT' ("Bad address")
> in this case since the conversion from target to host in this case fails.
> 
> This was confirmed with the LTP test for 'semop()' ('ipc/semop/semop02') in
> test case where 'nsops' is greater than SEMOPM with unaproppriate errno 
> EFAULT:
> 
> semop02.c:130: FAIL: semop failed unexpectedly; expected: E2BIG: EFAULT (14)
> 
> This patch changes this by adding a check whether 'nsops' is bigger than
> 'SEMOPM' before the conversion function 'target_to_host_sembuf()' is called.
> After the changes from this patch, the test works fine along with the other
> LTP testcases for 'semop()'):
> 
> semop02.c:126: PASS: semop failed as expected: E2BIG (7)
> 
> Implementation notes:
> 
> A target value ('TARGET_SEMOPM') was added for 'SEMOPM' as to be sure
> in case the value is not available for some targets.
> 
> Signed-off-by: Filip Bozuta 
> ---
>  linux-user/syscall.c  | 12 ++--
>  linux-user/syscall_defs.h |  2 ++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..70b172740c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3904,7 +3904,7 @@ static inline abi_long do_semtimedop(int semid,
>   unsigned nsops,
>   abi_long timeout)
>  {
> -struct sembuf sops[nsops];
> +struct sembuf *sops = NULL;

No need to set it to NULL.

>  struct timespec ts, *pts = NULL;
>  abi_long ret;
>  
> @@ -3915,8 +3915,15 @@ static inline abi_long do_semtimedop(int semid,
>  }
>  }
>  
> -if (target_to_host_sembuf(sops, ptr, nsops))
> +if (nsops > TARGET_SEMOPM) {
> +return -TARGET_E2BIG;
> +}
> +
> +sops = g_new(struct sembuf, nsops);
> +
> +if (target_to_host_sembuf(sops, ptr, nsops)) {

Add:

g_free(sops);

>  return -TARGET_EFAULT;
> +}
>  
>  ret = -TARGET_ENOSYS;
>  #ifdef __NR_semtimedop

Thanks,
Laurent

Re: [PATCH 00/41] qom: Automated conversion of type checking boilerplate

[Eduardo Habkost]

On Tue, Aug 18, 2020 at 12:59:03PM +0300, Roman Bolshakov wrote:
> On Thu, Aug 13, 2020 at 06:25:44PM -0400, Eduardo Habkost wrote:
> > This is an extension of the series previously submitted by
> > Daniel[1], including a script that will convert existing type
> > checker macros automatically.
> > 
> 
> Hi Eduardo,
> 
> do you have a repo where it can be checked it out?

Yes:
https://github.com/ehabkost/qemu-hacks/tree/work/qom-macros-autoconvert

-- 
Eduardo

Re: [PATCH v3 000/150] Meson integration for 5.2

[Philippe Mathieu-Daudé]

On 8/17/20 4:34 PM, Paolo Bonzini wrote:
> - Simplified/rewrote handling of the modules variable [Howard]
> - Fixed access to gdb-xml files [Howard]
> - Fixed cross-compilation failure due to mingw prefix [Howard]
> - Fixed SDL2 detection on cross compiles [Howard]
> - Fixed sub-makefiles that include config-host.mak
> 
> You can find it at the usual place, https://gitlab.com/bonzini/qemu.git
> branch meson-poc-next.

I'm getting this warning (nothing critical, so can be fix later):

../configure: line 183: shift: shift count out of range

which is in version_ge().

I used:

../configure \
  --enable-trace-backends=log \
  --disable-docs \
  --enable-debug \
  --extra-cflags=-ggdb \
  --audio-drv-list=oss alsa sdl pa'

Thanks,

Phil.

[PATCH v4 2/4] iotests.py: Add wait_for_runstate()

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 717b5b652c..ee93cf22db 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -833,6 +833,10 @@ class VM(qtest.QEMUQtestMachine):
'Found node %s under %s (but expected %s)' % \
(node['name'], path, expected_node)
 
+def wait_for_runstate(self, runstate: str) -> None:
+while self.qmp('query-status')['return']['status'] != runstate:
+time.sleep(0.2)
+
 index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
 
 class QMPTestCase(unittest.TestCase):
-- 
2.26.2

[PATCH v4 3/4] iotests.py: Let wait_migration() return on failure

[Max Reitz]

Let wait_migration() return on failure (with the return value indicating
whether the migration was completed or has failed), so we can use it for
migrations that are expected to fail, too.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ee93cf22db..f39fd580a6 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -729,16 +729,22 @@ class VM(qtest.QEMUQtestMachine):
 }
 ]))
 
-def wait_migration(self, expect_runstate):
+def wait_migration(self, expect_runstate: Optional[str]) -> bool:
 while True:
 event = self.event_wait('MIGRATION')
 log(event, filters=[filter_qmp_event])
-if event['data']['status'] == 'completed':
+if event['data']['status'] in ('completed', 'failed'):
 break
-# The event may occur in finish-migrate, so wait for the expected
-# post-migration runstate
-while self.qmp('query-status')['return']['status'] != expect_runstate:
-pass
+
+if event['data']['status'] == 'completed':
+# The event may occur in finish-migrate, so wait for the expected
+# post-migration runstate
+runstate = None
+while runstate != expect_runstate:
+runstate = self.qmp('query-status')['return']['status']
+return True
+else:
+return False
 
 def node_info(self, node_name):
 nodes = self.qmp('query-named-block-nodes')
-- 
2.26.2