Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

[Paolo Bonzini]

On 25/06/2018 21:51, Eduardo Habkost wrote:
>> Before that we should ask what the benefit is in changing the default
>> for qemu-system-*.  Nobody is using it in practice to start QEMU with
>> KVM enabled...
>
> How can you be sure?  If qemu-system-* is installed with KVM
> compiled in, libvirt will probe it using
> "-machine none,accel=kvm:tcg" and run the VM using
> "-machine $MACHINE,accel=kvm".

Right, that needs to be qualified with "without libvirt".

> In either case, I'm not arguing (yet) for changing the default
> upstream.  I'm just arguing for upstream QEMU to not make any
> promises about the default.

It would be a guest ABI breakage for TCG guests, so it would only apply
to new machine types.  I don't think it's worth the complication.

BTW, another thing that needs documenting is ABI promises for HAX and
WHPX.  Both do not support -cpu in any meaningful way, at least WHPX
installs a migration blocker.

Paolo

Re: [Qemu-devel] s390 qemu boot failure in -next

[Thomas Huth]

On 26.06.2018 07:32, Georgi Guninski wrote:
> On Mon, Jun 25, 2018 at 09:27:59AM +0200, Christian Borntraeger wrote:
>> -/* Overwrite parameters in the kernel image, which are "rom" */
>> -strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> 
>> +strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> 
> Why not replace strcpy() with strncpy() or snprintf()?
> strcpy() may overflow.

This will be fixed by
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg04227.html by
adding a check for a valid size.

 Thomas

[Qemu-devel] [PATCH 1/1] block/dirty-bitmap: Useless bitmaps in image can be removed

[13466399134]

From: Yao Xu <13466399...@163.com>

If qemu-kvm quit without saving bitmaps to image(coredump, host kernel panic,
or host pooweroff), bitmaps in image can not be safely used anymore, and also
can not be removed. Useless bitmaps should be removed.

"block-dirty-bitmap-remove" failed, because block_dirty_bitmap_lookup return 
NULL
"block-dirty-bitmap-add" failed, because qcow2_remove_persistent_dirty_bitmap 
can find the same name bitmap in image. As a result, the bitmap can never be 
used, neither can be removed.

Signed-off-by: Yao Xu <13466399...@163.com>
---
diff --git a/blockdev.c b/blockdev.c
index 58d7570932..d2403f4b7f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2837,31 +2837,38 @@ void qmp_block_dirty_bitmap_remove(const char *node, 
const char *name,
 Error *local_err = NULL;
 
 bitmap = block_dirty_bitmap_lookup(node, name, , errp);
-if (!bitmap || !bs) {
+if (!bs) {
 return;
 }
 
-if (bdrv_dirty_bitmap_frozen(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently frozen and cannot be removed",
-   name);
-return;
-} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently locked and cannot be removed",
-   name);
+if (bitmap) {
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently frozen and cannot be removed",
+   name);
+return;
+} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently locked and cannot be removed",
+   name);
+return;
+}
+}
+
+bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
 return;
 }
 
-if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
-bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
-if (local_err != NULL) {
-error_propagate(errp, local_err);
-return;
-}
+if (bitmap) {
+bdrv_release_dirty_bitmap(bs, bitmap);
 }
 
-bdrv_release_dirty_bitmap(bs, bitmap);
+if (*errp) {
+error_free(*errp);
+*errp = NULL;
+}
 }
 
 /**

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

[Thomas Huth]

On 25.06.2018 20:26, Paolo Bonzini wrote:
> On 25/06/2018 19:30, Eduardo Habkost wrote:
 Attentive distros could even replace the wrapper script by a link.
>>> If they are okay with replacing the "KVM only" semantics with "KVM or
>>> TCG", which I think is generally worse.
>>
>> If we can't get agreement on what's the right default for each
>> QEMU binary, I think that's yet another reason to document that
>> upstream QEMU won't guarantee ABI compatibility if -accel is
>> omitted.
> 
> Before that we should ask what the benefit is in changing the default
> for qemu-system-*.  Nobody is using it in practice to start QEMU with
> KVM enabled...

That's certainly not true. I've seen a couple of times already that
people ask on IRC why their guests are running so slow, and if you ask
them about their command line, it's obvious that they simply were not
aware of "-accel" / "-enable-kvm" yet.


Maybe we simply should add a "--verbose" command line option that people
can use to diagnose their problems:

$ qemu-system-x86_64 --verbose
QEMU emulator version 2.12.50
Using 'tcg' accelerator. Use '-accel kvm' to speed things up.
Machine type is 'pc-i440fx-3.0'. Use 'q35' for a more modern machine.



 Cheers,
  Thomas

[Qemu-devel] [Bug 628082] Re: nl-be keymap is wrong

[Launchpad Bug Tracker]

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

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

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

Title:
  nl-be keymap is wrong

Status in QEMU:
  Expired

Bug description:
  As mentioned on
  https://bugs.launchpad.net/ubuntu/+source/kvm/+bug/429965 as well as
  the kvm mailinglist
  (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/14413), the
  nl-be keymap does not work. The number keys above the regular keys
  (non-numeric keypad numbers) as well as vital keys such as slash,
  backslash, dash, ... are not working.

  The nl-be keymap that is presented in the above URLs (and also
  attached to this bug) does work properly.

  Would it be possible to include this keymap rather than the current
  one?

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

[Qemu-devel] [Bug 1660599] Re: v2.8.0 won't compile if g++ compiler doesn't understand "-fstack-protector-strong"

[Launchpad Bug Tracker]

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

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

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

Title:
  v2.8.0 won't compile if g++ compiler doesn't understand "-fstack-
  protector-strong"

Status in QEMU:
  Expired

Bug description:
  For example, Ubuntu Trusty (LTS 14.04) uses g++ v4.8.5.
  Compilation fails with a syntax error saying that the 
""-fstack-protector-strong" option in g++ is unrecognized.
  Instead, under Ubuntu Xenial (LTS 16.04), the g++ compiler is v5.4.0 and the 
compilation goes on smoothly.

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

[Qemu-devel] block/dirty-bitmap: Useless bitmaps in image can be removed

[yaoxu]

If qemu-kvm quit without saving bitmaps to image(coredump, host kernel panic,
or host pooweroff), bitmaps in image can not be safely used anymore, and also
can not be removed. Useless bitmaps should be removed.

Signed-off-by: yaoxu 
---
diff --git a/blockdev.c b/blockdev.c
index 58d7570932..c85056a74b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2837,31 +2837,35 @@ void qmp_block_dirty_bitmap_remove(const char *node, 
const char *name,
 Error *local_err = NULL;
 
 bitmap = block_dirty_bitmap_lookup(node, name, , errp);
-if (!bitmap || !bs) {
+if (!bs) {
 return;
 }
 
-if (bdrv_dirty_bitmap_frozen(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently frozen and cannot be removed",
-   name);
-return;
-} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently locked and cannot be removed",
-   name);
+if (bitmap) {
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently frozen and cannot be removed",
+   name);
+return;
+} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently locked and cannot be removed",
+   name);
+return;
+}
+}
+
+bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
 return;
 }
 
-if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
-bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
-if (local_err != NULL) {
-error_propagate(errp, local_err);
-return;
-}
+if (bitmap) {
+bdrv_release_dirty_bitmap(bs, bitmap);
 }
 
-bdrv_release_dirty_bitmap(bs, bitmap);
+if (*errp) {
+error_free(*errp);
+*errp = NULL;
+}
 }
 
 /**

Re: [Qemu-devel] [PATCH 1/1] target/m68k: correctly disassemble move16

[Thomas Huth]

Am Mon, 25 Jun 2018 22:35:59 +0200
schrieb Laurent Vivier :

> "move16 %a0@+,%a1@" and "fmovel (cpid=3) %a0@-,%fpcr"
> share the same opcode.
> 
> To fix that, backport the fix from binutils:
> 
>   2005-11-10  Andreas Schwab  
> 
>  * m68k-dis.c (print_insn_m68k): Only match FPU insns with
>  coprocessor ID 1.
> 
> Reported-by: Thomas Huth 
> Signed-off-by: Laurent Vivier 
> ---
>  disas/m68k.c | 14 ++
>  1 file changed, 14 insertions(+)

Thank you very much, Laurent, this indeed fixes the problem for me!

Tested-by: Thomas Huth 

[Qemu-devel] block/dirty-bitmap: Useless bitmaps in image can be removed

[yaoxu]

If qemu-kvm quit without saving bitmaps to image(coredump, host kernel panic,
or host pooweroff), bitmaps in image can not be safely used anymore, and also
can not be removed. Useless bitmaps should be removed.

Signed-off-by: yaoxu 
---
diff --git a/blockdev.c b/blockdev.c
index 58d7570932..c85056a74b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2837,31 +2837,35 @@ void qmp_block_dirty_bitmap_remove(const char *node, 
const char *name,
 Error *local_err = NULL;
 
 bitmap = block_dirty_bitmap_lookup(node, name, , errp);
-if (!bitmap || !bs) {
+if (!bs) {
 return;
 }
 
-if (bdrv_dirty_bitmap_frozen(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently frozen and cannot be removed",
-   name);
-return;
-} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently locked and cannot be removed",
-   name);
+if (bitmap) {
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently frozen and cannot be removed",
+   name);
+return;
+} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently locked and cannot be removed",
+   name);
+return;
+}
+}
+
+bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
 return;
 }
 
-if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
-bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
-if (local_err != NULL) {
-error_propagate(errp, local_err);
-return;
-}
+if (bitmap) {
+bdrv_release_dirty_bitmap(bs, bitmap);
 }
 
-bdrv_release_dirty_bitmap(bs, bitmap);
+if (*errp) {
+error_free(*errp);
+*errp = NULL;
+}
 }
 
 /**

Re: [Qemu-devel] [PATCH v2 5/5] ppc/xics: rework the ICS classes inheritance tree

[David Gibson]

On Mon, Jun 25, 2018 at 11:17:18AM +0200, Cédric Le Goater wrote:
> With the previous changes, we can now let the ICS_KVM class inherit
> directly from ICS_BASE class and not from the intermediate ICS_SIMPLE.
> It makes the class hierarchy much cleaner.
> 
> What is left in the top classes is the low level interface to access
> the KVM XICS device in ICS_KVM and the XICS emulating handlers in
> ICS_SIMPLE.
> 
> This should not break migration compatibility.
> 
> Signed-off-by: Cédric Le Goater 

Applied, thanks.

> ---
>  hw/intc/xics_kvm.c | 12 +---
>  hw/ppc/spapr.c |  2 +-
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index b314eb7d1607..30c3769a2084 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -359,12 +359,10 @@ static void ics_kvm_class_init(ObjectClass *klass, void 
> *data)
>  ICSStateClass *icsc = ICS_BASE_CLASS(klass);
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -/*
> - * Use device_class_set_parent_realize() when ics-kvm inherits
> - * directly from ics-base and not from ics-simple anymore.
> - */
> -dc->realize = ics_kvm_realize;
> -dc->reset = ics_kvm_reset;
> +device_class_set_parent_realize(dc, ics_kvm_realize,
> +>parent_realize);
> +device_class_set_parent_reset(dc, ics_kvm_reset,
> +  >parent_reset);
>  
>  icsc->pre_save = ics_get_kvm_state;
>  icsc->post_load = ics_set_kvm_state;
> @@ -373,7 +371,7 @@ static void ics_kvm_class_init(ObjectClass *klass, void 
> *data)
>  
>  static const TypeInfo ics_kvm_info = {
>  .name = TYPE_ICS_KVM,
> -.parent = TYPE_ICS_SIMPLE,
> +.parent = TYPE_ICS_BASE,
>  .instance_size = sizeof(ICSState),
>  .class_init = ics_kvm_class_init,
>  };
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0d032a1ad03c..8cc996d0b822 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -137,7 +137,7 @@ static ICSState *spapr_ics_create(sPAPRMachineState 
> *spapr,
>  goto error;
>  }
>  
> -return ICS_SIMPLE(obj);
> +return ICS_BASE(obj);
>  
>  error:
>  error_propagate(errp, local_err);

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference

[WangJie (Pluto)]

Thanks Stefan, will you push it to master branch?

On 2018/6/18 23:50, Stefan Hajnoczi wrote:
> On Tue, Jun 12, 2018 at 07:26:25AM +0800, Jie Wang wrote:
>> if laio_init create linux_aio failed and return NULL, NULL pointer
>> dereference will occur when laio_attach_aio_context dereference
>> linux_aio in aio_get_linux_aio. Let's avoid it and report error.
>>
>> Signed-off-by: Jie Wang 
>> ---
>>  block/file-posix.c | 19 +--
>>  util/async.c   |  5 -
>>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> If someone wants to split aio_get_linux_aio() into an initialization
> function and a "get" function which doesn't return NULL if init
> succeeded, then we can make this a bit cleaner.  But it doesn't matter
> at the moment since there are few callers and duplicating the NULL check
> isn't too bad.
> 
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
> 
> Stefan
> 

Re: [Qemu-devel] [PATCH v2 3/5] ppx/xics: introduce a parent_reset in ICSStateClass

[David Gibson]

On Mon, Jun 25, 2018 at 11:17:16AM +0200, Cédric Le Goater wrote:
> Just like for the realize handlers, this makes possible to move the
> common ICSState code of the reset handlers in the ics-base class.
> 
> Signed-off-by: Cédric Le Goater 

Applied, thanks.

> ---
>  include/hw/ppc/xics.h |  1 +
>  hw/intc/xics.c| 45 ++---
>  hw/intc/xics_kvm.c| 26 ++
>  3 files changed, 41 insertions(+), 31 deletions(-)
> 
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 44e96e640070..6ac8a9392da6 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -116,6 +116,7 @@ struct ICSStateClass {
>  DeviceClass parent_class;
>  
>  DeviceRealize parent_realize;
> +DeviceReset parent_reset;
>  
>  void (*pre_save)(ICSState *s);
>  int (*post_load)(ICSState *s, int version_id);
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 83340770f7c0..8cfe2231531e 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -537,23 +537,16 @@ static void ics_simple_eoi(ICSState *ics, uint32_t nr)
>  }
>  }
>  
> -static void ics_simple_reset(void *dev)
> +static void ics_simple_reset(DeviceState *dev)
>  {
> -ICSState *ics = ICS_SIMPLE(dev);
> -int i;
> -uint8_t flags[ics->nr_irqs];
> +ICSStateClass *icsc = ICS_BASE_GET_CLASS(dev);
>  
> -for (i = 0; i < ics->nr_irqs; i++) {
> -flags[i] = ics->irqs[i].flags;
> -}
> -
> -memset(ics->irqs, 0, sizeof(ICSIRQState) * ics->nr_irqs);
> +icsc->parent_reset(dev);
> +}
>  
> -for (i = 0; i < ics->nr_irqs; i++) {
> -ics->irqs[i].priority = 0xff;
> -ics->irqs[i].saved_priority = 0xff;
> -ics->irqs[i].flags = flags[i];
> -}
> +static void ics_simple_reset_handler(void *dev)
> +{
> +ics_simple_reset(dev);
>  }
>  
>  static int ics_simple_dispatch_pre_save(void *opaque)
> @@ -625,7 +618,7 @@ static void ics_simple_realize(DeviceState *dev, Error 
> **errp)
>  
>  ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
>  
> -qemu_register_reset(ics_simple_reset, ics);
> +qemu_register_reset(ics_simple_reset_handler, ics);
>  }
>  
>  static void ics_simple_class_init(ObjectClass *klass, void *data)
> @@ -635,6 +628,8 @@ static void ics_simple_class_init(ObjectClass *klass, 
> void *data)
>  
>  device_class_set_parent_realize(dc, ics_simple_realize,
>  >parent_realize);
> +device_class_set_parent_reset(dc, ics_simple_reset,
> +  >parent_reset);
>  
>  dc->vmsd = _ics_simple;
>  isc->reject = ics_simple_reject;
> @@ -650,6 +645,25 @@ static const TypeInfo ics_simple_info = {
>  .class_size = sizeof(ICSStateClass),
>  };
>  
> +static void ics_base_reset(DeviceState *dev)
> +{
> +ICSState *ics = ICS_BASE(dev);
> +int i;
> +uint8_t flags[ics->nr_irqs];
> +
> +for (i = 0; i < ics->nr_irqs; i++) {
> +flags[i] = ics->irqs[i].flags;
> +}
> +
> +memset(ics->irqs, 0, sizeof(ICSIRQState) * ics->nr_irqs);
> +
> +for (i = 0; i < ics->nr_irqs; i++) {
> +ics->irqs[i].priority = 0xff;
> +ics->irqs[i].saved_priority = 0xff;
> +ics->irqs[i].flags = flags[i];
> +}
> +}
> +
>  static void ics_base_realize(DeviceState *dev, Error **errp)
>  {
>  ICSState *ics = ICS_BASE(dev);
> @@ -689,6 +703,7 @@ static void ics_base_class_init(ObjectClass *klass, void 
> *data)
>  
>  dc->realize = ics_base_realize;
>  dc->props = ics_base_properties;
> +dc->reset = ics_base_reset;
>  }
>  
>  static const TypeInfo ics_base_info = {
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 1f27eb497981..b314eb7d1607 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -324,25 +324,18 @@ static void ics_kvm_set_irq(void *opaque, int srcno, 
> int val)
>  }
>  }
>  
> -static void ics_kvm_reset(void *dev)
> +static void ics_kvm_reset(DeviceState *dev)
>  {
> -ICSState *ics = ICS_SIMPLE(dev);
> -int i;
> -uint8_t flags[ics->nr_irqs];
> -
> -for (i = 0; i < ics->nr_irqs; i++) {
> -flags[i] = ics->irqs[i].flags;
> -}
> +ICSStateClass *icsc = ICS_BASE_GET_CLASS(dev);
>  
> -memset(ics->irqs, 0, sizeof(ICSIRQState) * ics->nr_irqs);
> +icsc->parent_reset(dev);
>  
> -for (i = 0; i < ics->nr_irqs; i++) {
> -ics->irqs[i].priority = 0xff;
> -ics->irqs[i].saved_priority = 0xff;
> -ics->irqs[i].flags = flags[i];
> -}
> +ics_set_kvm_state(ICS_KVM(dev), 1);
> +}
>  
> -ics_set_kvm_state(ics, 1);
> +static void ics_kvm_reset_handler(void *dev)
> +{
> +ics_kvm_reset(dev);
>  }
>  
>  static void ics_kvm_realize(DeviceState *dev, Error **errp)
> @@ -358,7 +351,7 @@ static void ics_kvm_realize(DeviceState *dev, Error 
> **errp)
>  }
>  ics->qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics->nr_irqs);
>  
> -

Re: [Qemu-devel] [PATCH v2 2/5] ppc/xics: move the instance_init handler under the ics-base class

[David Gibson]

On Mon, Jun 25, 2018 at 11:17:15AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/intc/xics.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)

Applied, thanks.

> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index d6066d561fdc..83340770f7c0 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -611,13 +611,6 @@ static const VMStateDescription vmstate_ics_simple = {
>  },
>  };
>  
> -static void ics_simple_initfn(Object *obj)
> -{
> -ICSState *ics = ICS_SIMPLE(obj);
> -
> -ics->offset = XICS_IRQ_BASE;
> -}
> -
>  static void ics_simple_realize(DeviceState *dev, Error **errp)
>  {
>  ICSState *ics = ICS_SIMPLE(dev);
> @@ -655,7 +648,6 @@ static const TypeInfo ics_simple_info = {
>  .instance_size = sizeof(ICSState),
>  .class_init = ics_simple_class_init,
>  .class_size = sizeof(ICSStateClass),
> -.instance_init = ics_simple_initfn,
>  };
>  
>  static void ics_base_realize(DeviceState *dev, Error **errp)
> @@ -679,6 +671,13 @@ static void ics_base_realize(DeviceState *dev, Error 
> **errp)
>  ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>  }
>  
> +static void ics_base_instance_init(Object *obj)
> +{
> +ICSState *ics = ICS_BASE(obj);
> +
> +ics->offset = XICS_IRQ_BASE;
> +}
> +
>  static Property ics_base_properties[] = {
>  DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0),
>  DEFINE_PROP_END_OF_LIST(),
> @@ -697,6 +696,7 @@ static const TypeInfo ics_base_info = {
>  .parent = TYPE_DEVICE,
>  .abstract = true,
>  .instance_size = sizeof(ICSState),
> +.instance_init = ics_base_instance_init,
>  .class_init = ics_base_class_init,
>  .class_size = sizeof(ICSStateClass),
>  };

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 1/5] ppc/xics: introduce a parent_realize in ICSStateClass

[David Gibson]

On Mon, Jun 25, 2018 at 11:17:14AM +0200, Cédric Le Goater wrote:
> This makes possible to move the common ICSState code of the realize
> handlers in the ics-base class.
> 
> Signed-off-by: Cédric Le Goater 

Applied to ppc-for-3.0, thanks.

> ---
>  include/hw/ppc/xics.h |  3 ++-
>  hw/intc/xics.c| 37 ++---
>  hw/intc/xics_kvm.c| 20 +++-
>  3 files changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 4b04b295a772..44e96e640070 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -115,7 +115,8 @@ struct PnvICPState {
>  struct ICSStateClass {
>  DeviceClass parent_class;
>  
> -void (*realize)(ICSState *s, Error **errp);
> +DeviceRealize parent_realize;
> +
>  void (*pre_save)(ICSState *s);
>  int (*post_load)(ICSState *s, int version_id);
>  void (*reject)(ICSState *s, uint32_t irq);
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 063491f38712..d6066d561fdc 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -618,30 +618,31 @@ static void ics_simple_initfn(Object *obj)
>  ics->offset = XICS_IRQ_BASE;
>  }
>  
> -static void ics_simple_realize(ICSState *ics, Error **errp)
> +static void ics_simple_realize(DeviceState *dev, Error **errp)
>  {
> -if (!ics->nr_irqs) {
> -error_setg(errp, "Number of interrupts needs to be greater 0");
> +ICSState *ics = ICS_SIMPLE(dev);
> +ICSStateClass *icsc = ICS_BASE_GET_CLASS(ics);
> +Error *local_err = NULL;
> +
> +icsc->parent_realize(dev, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
>  return;
>  }
> -ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
> +
>  ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
>  
>  qemu_register_reset(ics_simple_reset, ics);
>  }
>  
> -static Property ics_simple_properties[] = {
> -DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0),
> -DEFINE_PROP_END_OF_LIST(),
> -};
> -
>  static void ics_simple_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  ICSStateClass *isc = ICS_BASE_CLASS(klass);
>  
> -isc->realize = ics_simple_realize;
> -dc->props = ics_simple_properties;
> +device_class_set_parent_realize(dc, ics_simple_realize,
> +>parent_realize);
> +
>  dc->vmsd = _ics_simple;
>  isc->reject = ics_simple_reject;
>  isc->resend = ics_simple_resend;
> @@ -659,7 +660,6 @@ static const TypeInfo ics_simple_info = {
>  
>  static void ics_base_realize(DeviceState *dev, Error **errp)
>  {
> -ICSStateClass *icsc = ICS_BASE_GET_CLASS(dev);
>  ICSState *ics = ICS_BASE(dev);
>  Object *obj;
>  Error *err = NULL;
> @@ -672,17 +672,24 @@ static void ics_base_realize(DeviceState *dev, Error 
> **errp)
>  }
>  ics->xics = XICS_FABRIC(obj);
>  
> -
> -if (icsc->realize) {
> -icsc->realize(ics, errp);
> +if (!ics->nr_irqs) {
> +error_setg(errp, "Number of interrupts needs to be greater 0");
> +return;
>  }
> +ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>  }
>  
> +static Property ics_base_properties[] = {
> +DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void ics_base_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  
>  dc->realize = ics_base_realize;
> +dc->props = ics_base_properties;
>  }
>  
>  static const TypeInfo ics_base_info = {
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index f511e50a8020..1f27eb497981 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -345,13 +345,17 @@ static void ics_kvm_reset(void *dev)
>  ics_set_kvm_state(ics, 1);
>  }
>  
> -static void ics_kvm_realize(ICSState *ics, Error **errp)
> +static void ics_kvm_realize(DeviceState *dev, Error **errp)
>  {
> -if (!ics->nr_irqs) {
> -error_setg(errp, "Number of interrupts needs to be greater 0");
> +ICSState *ics = ICS_KVM(dev);
> +ICSStateClass *icsc = ICS_BASE_GET_CLASS(ics);
> +Error *local_err = NULL;
> +
> +icsc->parent_realize(dev, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
>  return;
>  }
> -ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>  ics->qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics->nr_irqs);
>  
>  qemu_register_reset(ics_kvm_reset, ics);
> @@ -360,8 +364,14 @@ static void ics_kvm_realize(ICSState *ics, Error **errp)
>  static void ics_kvm_class_init(ObjectClass *klass, void *data)
>  {
>  ICSStateClass *icsc = ICS_BASE_CLASS(klass);
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +/*
> + * Use device_class_set_parent_realize() when ics-kvm inherits
> + * directly from ics-base and not from ics-simple 

Re: [Qemu-devel] [PATCH v2 4/5] ppc/xics: move the vmstate structures under the ics-base class

[David Gibson]

On Mon, Jun 25, 2018 at 11:17:17AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 

Applied, thanks.

> ---
>  hw/intc/xics.c | 112 
> -
>  1 file changed, 56 insertions(+), 56 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 8cfe2231531e..b9f1a3c97214 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -549,61 +549,6 @@ static void ics_simple_reset_handler(void *dev)
>  ics_simple_reset(dev);
>  }
>  
> -static int ics_simple_dispatch_pre_save(void *opaque)
> -{
> -ICSState *ics = opaque;
> -ICSStateClass *info = ICS_BASE_GET_CLASS(ics);
> -
> -if (info->pre_save) {
> -info->pre_save(ics);
> -}
> -
> -return 0;
> -}
> -
> -static int ics_simple_dispatch_post_load(void *opaque, int version_id)
> -{
> -ICSState *ics = opaque;
> -ICSStateClass *info = ICS_BASE_GET_CLASS(ics);
> -
> -if (info->post_load) {
> -return info->post_load(ics, version_id);
> -}
> -
> -return 0;
> -}
> -
> -static const VMStateDescription vmstate_ics_simple_irq = {
> -.name = "ics/irq",
> -.version_id = 2,
> -.minimum_version_id = 1,
> -.fields = (VMStateField[]) {
> -VMSTATE_UINT32(server, ICSIRQState),
> -VMSTATE_UINT8(priority, ICSIRQState),
> -VMSTATE_UINT8(saved_priority, ICSIRQState),
> -VMSTATE_UINT8(status, ICSIRQState),
> -VMSTATE_UINT8(flags, ICSIRQState),
> -VMSTATE_END_OF_LIST()
> -},
> -};
> -
> -static const VMStateDescription vmstate_ics_simple = {
> -.name = "ics",
> -.version_id = 1,
> -.minimum_version_id = 1,
> -.pre_save = ics_simple_dispatch_pre_save,
> -.post_load = ics_simple_dispatch_post_load,
> -.fields = (VMStateField[]) {
> -/* Sanity check */
> -VMSTATE_UINT32_EQUAL(nr_irqs, ICSState, NULL),
> -
> -VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs,
> - vmstate_ics_simple_irq,
> - ICSIRQState),
> -VMSTATE_END_OF_LIST()
> -},
> -};
> -
>  static void ics_simple_realize(DeviceState *dev, Error **errp)
>  {
>  ICSState *ics = ICS_SIMPLE(dev);
> @@ -631,7 +576,6 @@ static void ics_simple_class_init(ObjectClass *klass, 
> void *data)
>  device_class_set_parent_reset(dc, ics_simple_reset,
>>parent_reset);
>  
> -dc->vmsd = _ics_simple;
>  isc->reject = ics_simple_reject;
>  isc->resend = ics_simple_resend;
>  isc->eoi = ics_simple_eoi;
> @@ -692,6 +636,61 @@ static void ics_base_instance_init(Object *obj)
>  ics->offset = XICS_IRQ_BASE;
>  }
>  
> +static int ics_base_dispatch_pre_save(void *opaque)
> +{
> +ICSState *ics = opaque;
> +ICSStateClass *info = ICS_BASE_GET_CLASS(ics);
> +
> +if (info->pre_save) {
> +info->pre_save(ics);
> +}
> +
> +return 0;
> +}
> +
> +static int ics_base_dispatch_post_load(void *opaque, int version_id)
> +{
> +ICSState *ics = opaque;
> +ICSStateClass *info = ICS_BASE_GET_CLASS(ics);
> +
> +if (info->post_load) {
> +return info->post_load(ics, version_id);
> +}
> +
> +return 0;
> +}
> +
> +static const VMStateDescription vmstate_ics_base_irq = {
> +.name = "ics/irq",
> +.version_id = 2,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(server, ICSIRQState),
> +VMSTATE_UINT8(priority, ICSIRQState),
> +VMSTATE_UINT8(saved_priority, ICSIRQState),
> +VMSTATE_UINT8(status, ICSIRQState),
> +VMSTATE_UINT8(flags, ICSIRQState),
> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
> +static const VMStateDescription vmstate_ics_base = {
> +.name = "ics",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.pre_save = ics_base_dispatch_pre_save,
> +.post_load = ics_base_dispatch_post_load,
> +.fields = (VMStateField[]) {
> +/* Sanity check */
> +VMSTATE_UINT32_EQUAL(nr_irqs, ICSState, NULL),
> +
> +VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs,
> + vmstate_ics_base_irq,
> + ICSIRQState),
> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
>  static Property ics_base_properties[] = {
>  DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0),
>  DEFINE_PROP_END_OF_LIST(),
> @@ -704,6 +703,7 @@ static void ics_base_class_init(ObjectClass *klass, void 
> *data)
>  dc->realize = ics_base_realize;
>  dc->props = ics_base_properties;
>  dc->reset = ics_base_reset;
> +dc->vmsd = _ics_base;
>  }
>  
>  static const TypeInfo ics_base_info = {

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!

Re: [Qemu-devel] [PATCH v5 41/46] hw/ppc: Use the IEC binary prefix definitions

[David Gibson]

On Mon, Jun 25, 2018 at 09:42:33AM -0300, Philippe Mathieu-Daudé wrote:
> It eases code review, unit is explicit.
> 
> Patch generated using:
> 
>   $ git grep -n '[<>][<>]= ?[1-5]0'
> 
> and modified manually.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Acked-by: David Gibson 

> ---
>  hw/ppc/sam460ex.c   | 2 +-
>  target/ppc/mmu_helper.c | 8 
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index ffe561fbc1..c7c799b843 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -126,7 +126,7 @@ static void generate_eeprom_spd(uint8_t *eeprom, 
> ram_addr_t ram_size)
>  int i;
>  
>  /* work in terms of MB */
> -ram_size >>= 20;
> +ram_size /= MiB;
>  
>  while ((ram_size >= 4) && (nbanks <= 2)) {
>  int sz_log2 = MIN(31 - clz32(ram_size), 14);
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 98ce17985b..e6739e6c24 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -17,6 +17,7 @@
>   * License along with this library; if not, see 
> .
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "cpu.h"
>  #include "exec/helper-proto.h"
>  #include "sysemu/kvm.h"
> @@ -1090,11 +1091,10 @@ static void mmubooke_dump_mmu(FILE *f, 
> fprintf_function cpu_fprintf,
>  pa = entry->RPN & mask;
>  /* Extend the physical address to 36 bits */
>  pa |= (hwaddr)(entry->RPN & 0xF) << 32;
> -size /= 1024;
> -if (size >= 1024) {
> -snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "M", size / 
> 1024);
> +if (size >= 1 * MiB) {
> +snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "M", size / 
> MiB);
>  } else {
> -snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "k", size);
> +snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "k", size / 
> KiB);
>  }
>  cpu_fprintf(f, "0x%016" PRIx64 " 0x%016" PRIx64 " %s %-5u %08x 
> %08x\n",
>  (uint64_t)ea, (uint64_t)pa, size_buf, 
> (uint32_t)entry->PID,

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block][RFC PATCH] hw/block/nvme: add optional parameter num_queues for nvme device

[Keith Busch]

On Tue, Jun 26, 2018 at 09:44:56AM +0800, Weiping Zhang wrote:
> Add an optional paramter num_queues for device, and set it
> to 64 by default.
> 
> Signed-off-by: Weiping Zhang 

Looks good to me.

Acked-by: Keith Busch 

Re: [Qemu-devel] [virtio-dev] Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net

[Michael S. Tsirkin]

On Mon, Jun 25, 2018 at 10:54:09AM -0700, Samudrala, Sridhar wrote:
> > > > > Might not neccessarily be something wrong, but it's very limited to
> > > > > prohibit the MAC of VF from changing when enslaved by failover.
> > > > You mean guest changing MAC? I'm not sure why we prohibit that.
> > > I think Sridhar and Jiri might be better person to answer it. My
> > > impression was that sync'ing the MAC address change between all 3
> > > devices is challenging, as the failover driver uses MAC address to
> > > match net_device internally.
> 
> Yes. The MAC address is assigned by the hypervisor and it needs to manage the 
> movement
> of the MAC between the PF and VF.  Allowing the guest to change the MAC will 
> require
> synchronization between the hypervisor and the PF/VF drivers. Most of the VF 
> drivers
> don't allow changing guest MAC unless it is a trusted VF.

OK but it's a policy thing. Maybe it's a trusted VF. Who knows?
For example I can see host just
failing VIRTIO_NET_CTRL_MAC_ADDR_SET if it wants to block it.
I'm not sure why VIRTIO_NET_F_STANDBY has to block it in the guest.

-- 
MST

Re: [Qemu-devel] [virtio-dev] Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net

[Michael S. Tsirkin]

On Mon, Jun 25, 2018 at 11:55:12AM +0200, Cornelia Huck wrote:
> On Fri, 22 Jun 2018 22:05:50 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Fri, Jun 22, 2018 at 05:09:55PM +0200, Cornelia Huck wrote:
> > > On Thu, 21 Jun 2018 21:20:13 +0300
> > > "Michael S. Tsirkin"  wrote:
> > >   
> > > > On Thu, Jun 21, 2018 at 04:59:13PM +0200, Cornelia Huck wrote:  
> > > > > OK, so what about the following:
> > > > > 
> > > > > - introduce a new feature bit, VIRTIO_NET_F_STANDBY_UUID that 
> > > > > indicates
> > > > >   that we have a new uuid field in the virtio-net config space
> > > > > - in QEMU, add a property for virtio-net that allows to specify a 
> > > > > uuid,
> > > > >   offer VIRTIO_NET_F_STANDBY_UUID if set
> > > > > - when configuring, set the property to the group UUID of the vfio-pci
> > > > >   device
> > > > > - in the guest, use the uuid from the virtio-net device's config space
> > > > >   if applicable; else, fall back to matching by MAC as done today
> > > > > 
> > > > > That should work for all virtio transports.
> > > > 
> > > > True. I'm a bit unhappy that it's virtio net specific though
> > > > since down the road I expect we'll have a very similar feature
> > > > for scsi (and maybe others).
> > > > 
> > > > But we do not have a way to have fields that are portable
> > > > both across devices and transports, and I think it would
> > > > be a useful addition. How would this work though? Any idea?  
> > > 
> > > Can we introduce some kind of device-independent config space area?
> > > Pushing back the device-specific config space by a certain value if the
> > > appropriate feature is negotiated and use that for things like the uuid?  
> > 
> > So config moves back and forth?
> > Reminds me of the msi vector mess we had with pci.
> 
> Yes, that would be a bit unfortunate.
> 
> > I'd rather have every transport add a new config.
> 
> You mean via different mechanisms?

I guess so.

> > 
> > > But regardless of that, I'm not sure whether extending this approach to
> > > other device types is the way to go. Tying together two different
> > > devices is creating complicated situations at least in the hypervisor
> > > (even if it's fairly straightforward in the guest). [I have not come
> > > around again to look at the "how to handle visibility in QEMU"
> > > questions due to lack of cycles, sorry about that.]
> > > 
> > > So, what's the goal of this approach? Only to allow migration with
> > > vfio-pci, or also to plug in a faster device and use it instead of an
> > > already attached paravirtualized device?  
> > 
> > These are two sides of the same coin, I think the second approach
> > is closer to what we are doing here.
> 
> Thinking about it, do we need any knob to keep the vfio device
> invisible if the virtio device is not present? IOW, how does the
> hypervisor know that the vfio device is supposed to be paired with a
> virtio device? It seems we need an explicit tie-in.

If we are going the way of the bridge, both bridge and
virtio would have some kind of id.

When pairing using mac, I'm less sure. PAss vfio device mac to qemu
as a property?


> > 
> > > What about migration of vfio devices that are not easily replaced by a
> > > paravirtualized device? I'm thinking of vfio-ccw, where our main (and
> > > currently only) supported device is dasd (disks) -- which can do a lot
> > > of specialized things that virtio-blk does not support (and should not
> > > or even cannot support).  
> > 
> > But maybe virtio-scsi can?
> 
> I don't think so. Dasds have some channel commands that don't map
> easily to scsi commands.

There's always a choice of adding these to the spec.
E.g. FC extensions were proposed, I don't remember why they
are still stuck.

> > 
> > > Would it be more helpful to focus on generic
> > > migration support for vfio instead of going about it device by device?
> > > 
> > > This network device approach already seems far along, so it makes sense
> > > to continue with it. But I'm not sure whether we want to spend time and
> > > energy on that for other device types rather than working on a general
> > > solution for vfio migration.  
> > 
> > I'm inclined to say finalizing this feature would be a good first step
> > and will teach us how we can move forward.
> 
> I'm not opposed to figuring out this one, but I'm not sure whether we
> want to extend it to more device types.
> 
> Are people looking into generic migration support? I have it on my
> things-to-look-at list (figuring out what needs to be device specific
> and what can be generic, figuring out how we can support vfio-ccw,
> etc.).

I expect to see more of it if SPDK makes progress.

-- 
MST

Re: [Qemu-devel] [PATCH 1/2] ide/hw/core: fix crash on processing a partial-sector-size DMA xfer

[Amol Surati]

On Mon, Jun 25, 2018 at 05:10:23PM -0400, John Snow wrote:
> 
> 
> On 06/20/2018 12:29 AM, Amol Surati wrote:
> > Fixes: https://bugs.launchpad.net/qemu/+bug/1777315
> > 
> > QEMU's short PRD policy applies to a DMA transfer of size < 512 bytes.
> > But it fails to consider transfers which are >= 512 bytes, but are
> > not a multiple of 512 bytes.
> > 
> > Such transfers are not subject to the short PRD policy. They end up
> > violating the assumptions about the granularity of the IO sizes,
> > upon which depend the verification of the completion of the previous
> > transfer, and the advancement of the offset in preparation of the next.
> > 
> > Those violations result in the crash.
> > 
> > By forcing each transfer to be a multiple of sector size, such
> > transfers are subjected to the policy, and therefore culled before they
> > cause the crash.
> > 
> 
> So now even if the PRDT we get is greater than a sector is not an even
> multiple of 512, we reject it as too short.

Yes. 

When PRDT is greater than a sector but non an even multiple of 512,
we used to crash. Now we do not.

The reason for this patch is to maintain backward compatibility with the
current split-completion design that QEMU adopts, while still avoiding the
crash. The cover letter has the reasons for this patch.


> 
> That doesn't seem correct to me.

https://github.com/asurati/1777315/blob/master/v2.diff has a different
patch, which aligns with what Kevin had suggested. It allows the
IO which you described above, but then it completes the command
as soon as it detects a partial transfer which is larger than 512.

The patch where s->sg.size is used as the offset to start the next
IO would require the dma IOs to be misaligned and of non-512 sizes.
Will that be okay? 

Else, one needs to change each implementation
of prepare_buf to ensure that it always ALIGNS_UP to the sector
size any partial read/write/trim, in anticipation of the next PRDT
which can cover the difference.


> 
> > Signed-off-by: Amol Surati 
> > ---
> >  hw/ide/core.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > index 2c62efc536..14d135224b 100644
> > --- a/hw/ide/core.c
> > +++ b/hw/ide/core.c
> > @@ -836,6 +836,7 @@ static void ide_dma_cb(void *opaque, int ret)
> >  {
> >  IDEState *s = opaque;
> >  int n;
> > +int32_t size_prepared;
> >  int64_t sector_num;
> >  uint64_t offset;
> >  bool stay_active = false;
> > @@ -886,7 +887,9 @@ static void ide_dma_cb(void *opaque, int ret)
> >  n = s->nsector;
> >  s->io_buffer_index = 0;
> >  s->io_buffer_size = n * 512;
> > -if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size) < 
> > 512) {
> > +size_prepared = s->bus->dma->ops->prepare_buf(s->bus->dma,
> > +  s->io_buffer_size);
> > +if (size_prepared <= 0 || size_prepared % 512) {
> >  /* The PRDs were too short. Reset the Active bit, but don't raise 
> > an
> >   * interrupt. */
> >  s->status = READY_STAT | SEEK_STAT;
> > 
> 
> -- 
> —js

Re: [Qemu-devel] [PATCH v5 00/46] Use the IEC binary prefix definitions

[Michael S. Tsirkin]

On Mon, Jun 25, 2018 at 09:41:52AM -0300, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series:
> 
> - split the byte-based definitions from "qemu/cutils.h" to "qemu/units.h"
>   (this now triggers WARNING: 1 added file, does MAINTAINERS need updating?)
> - clean hw/ includes;
> - replace different constants used for byte size with their corresponding
>   IEC binary prefix definitions.

Series:

Acked-by: Michael S. Tsirkin 


> Since v4:
> - fixed checkpatch (do not match the IEC binary prefix as camelcase typedefs)
> - use signed format for size (Richard)
> - fix 32-bit (Thomas)
> - update '[<>][<>]= ?[1-5]0', adding 5 new patches (Eric)
> 
> Since v3:
> - use IEC binary prefix definitions (Stefan Weil)
> - rebased
> - added R-b tags
> 
> Since v2:
> - use "qemu/units.h" instead of "qemu/cunits.h" (Daniel P. Berrangé)
> - avoid rebuilding the world including "qemu/units.h" in "hw/hw.h" (Thomas 
> Huth)
> - more constant replaced
> 
> Since v1:
> - corrected Xen mult -> div (Alan Robinson)
> - tagged 'include "qemu/cunits.h" in "hw/hw.h" as RFC
> - fixed mips_r4k 'params_size' (Thomas Huth)
> - added command used to generate patch in each commit (Alistair Francis)
> - use G_BYTE for the raspi3 machine
> - added Reviewed-by tags
> 
> Regards,
> 
> Phil.
> 
> [] : patches are identical
> [] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, 
> respectively
> 
> 001/46:[0012] [FC] 'include: Add IEC binary prefixes in "qemu/units.h"'
> 002/46:[0005] [FC] 'vdi: Use definitions from "qemu/units.h"'
> 003/46:[] [--] 'x86/cpu: Use definitions from "qemu/units.h"'
> 004/46:[0002] [FC] 'checkpatch: Recognize IEC binary prefix definitions'
> 005/46:[] [-C] 'hw: Use IEC binary prefix definitions from "qemu/units.h"'
> 006/46:[] [--] 'hw: Directly use "qemu/units.h" instead of 
> "qemu/cutils.h"'
> 007/46:[] [--] 'hw/ivshmem: Use the IEC binary prefix definitions'
> 008/46:[] [--] 'hw/ipack: Use the IEC binary prefix definitions'
> 009/46:[] [--] 'hw/scsi: Use the IEC binary prefix definitions'
> 010/46:[] [--] 'hw/smbios: Use the IEC binary prefix definitions'
> 011/46:[0002] [FC] 'hw/xen: Use the IEC binary prefix definitions'
> 012/46:[] [--] 'hw/tpm: Use the IEC binary prefix definitions'
> 013/46:[] [--] 'hw/block: Use the IEC binary prefix definitions'
> 014/46:[0008] [FC] 'hw/display: Use the IEC binary prefix definitions'
> 015/46:[] [--] 'hw/misc: Use the IEC binary prefix definitions'
> 016/46:[] [--] 'hw/riscv: Use the IEC binary prefix definitions'
> 017/46:[] [--] 'hw/m68k: Use the IEC binary prefix definitions'
> 018/46:[0006] [FC] 'hw/sparc: Use the IEC binary prefix definitions'
> 019/46:[0003] [FC] 'hw/s390x: Use the IEC binary prefix definitions'
> 020/46:[0004] [FC] 'hw/hppa: Use the IEC binary prefix definitions'
> 021/46:[0009] [FC] 'hw/xtensa: Use the IEC binary prefix definitions'
> 022/46:[] [--] 'hw/alpha: Use the IEC binary prefix definitions'
> 023/46:[] [--] 'hw/tricore: Use the IEC binary prefix definitions'
> 024/46:[] [--] 'hw/microblaze: Use the IEC binary prefix definitions'
> 025/46:[] [--] 'hw/nios2: Use the IEC binary prefix definitions'
> 026/46:[] [--] 'hw/cris: Use the IEC binary prefix definitions'
> 027/46:[] [--] 'hw/lm32: Use the IEC binary prefix definitions'
> 028/46:[] [--] 'hw/sh4: Use the IEC binary prefix definitions'
> 029/46:[] [--] 'hw/mips/r4k: Constify params_size'
> 030/46:[0012] [FC] 'hw/mips: Use the IEC binary prefix definitions'
> 031/46:[] [--] 'hw/arm: Use the IEC binary prefix definitions'
> 032/46:[0022] [FC] 'hw/ppc: Use the IEC binary prefix definitions'
> 033/46:[0006] [FC] 'hw/i386: Use the IEC binary prefix definitions'
> 034/46:[] [--] 'hw/net: Use the IEC binary prefix definitions'
> 035/46:[0002] [FC] 'hw/usb: Use the IEC binary prefix definitions'
> 036/46:[0002] [FC] 'hw/sd: Use the IEC binary prefix definitions'
> 037/46:[] [--] 'hw/vfio: Use the IEC binary prefix definitions'
> 038/46:[] [--] 'hw/virtio: Use the IEC binary prefix definitions'
> 039/46:[] [--] 'hw/rdma: Use the IEC binary prefix definitions'
> 040/46:[] [--] 'cutils: Do not include "qemu/units.h" directly'
> 041/46:[0148] [FC] 'hw/ppc: Use the IEC binary prefix definitions'
> 042/46:[down] 'monitor: Use the IEC binary prefix definitions'
> 043/46:[down] 'vl: Use the IEC binary prefix definitions'
> 044/46:[down] 'tests/crypto: Use the IEC binary prefix definitions'
> 045/46:[down] 'linux-user: Use the IEC binary prefix definitions'
> 046/46:[down] 'bsd-user: Use the IEC binary prefix definitions'
> 
> Philippe Mathieu-Daudé (46):
>   include: Add IEC binary prefixes in "qemu/units.h"
>   vdi: Use definitions from "qemu/units.h"
>   x86/cpu: Use definitions from "qemu/units.h"
>   checkpatch: Recognize IEC binary prefix definitions
>   hw: Use IEC 

[Qemu-devel] [PULL 1/2] vl.c: do not allow --daemonize in combination with --preconfig CLI option

[Eduardo Habkost]

From: Igor Mammedov 

some users when using --daemonize expect that QEMU will parse CLI options,
initialize VM and only then complete daemonzation by signalling lead
process to exit and start listening on monitor socket. So users treat
parent process exit as sync point to connect to QEMU's monitor.

That however doesn't work when --preconfig options is used, since it
provides monitor before completing daemonization and expects user to
issue exit-preconfig command when additional configuration via monitor
is finished. We also can't move completing daemonization before
preconfig monitor becomes available, since that would imply:
  * partially loosing ability to configure QEMU instance in --preconfig
mode since QEMU might drop privileges, chroot and do other things
when daemonization is completed
  * lead to loss of error messages in case they would happen after
daemonization

Be proactive now and make options mutually exclusive, so users would
get clear error message instead of waiting for lead process exit
indefinitely before connecting to monitor.

PS:
In case someone would come up with usecase where both options should
be enabled at the same time we could drop this restriction as far
as daemonization point is left where it is now (os_setup_post).

Signed-off-by: Igor Mammedov 
Message-Id: <1529501059-163139-1-git-send-email-imamm...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 vl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/vl.c b/vl.c
index d451f45dc5..a37dfe4684 100644
--- a/vl.c
+++ b/vl.c
@@ -4134,6 +4134,12 @@ int main(int argc, char **argv, char **envp)
 }
 
 if (is_daemonized()) {
+if (!preconfig_exit_requested) {
+error_report("'preconfig' and 'daemonize' options are "
+ "mutually exclusive");
+exit(EXIT_FAILURE);
+}
+
 /* According to documentation and historically, -nographic redirects
  * serial port, parallel port and monitor to stdio, which does not work
  * with -daemonize.  We can redirect these to null instead, but since
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 2/2] hw/i386: Deprecate the machine types pc-0.10 and pc-0.11

[Eduardo Habkost]

From: Thomas Huth 

The oldest machine type which is still used in a still maintained distro
is a pc-0.12 based machine type in RHEL6, so everything that is older
than pc-0.12 should not be used anymore. Thus let's deprecate pc-0.10
and pc-0.11 so that we can finally remove them in a future release.

Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
Message-Id: <1529917512-10528-1-git-send-email-th...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Eduardo Habkost 
---
 include/hw/boards.h |  3 +++
 hw/i386/pc_piix.c   |  1 +
 vl.c| 10 --
 qemu-doc.texi   |  5 +
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index ef7457f5dd..79069ddcbe 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -107,6 +107,8 @@ typedef struct {
 
 /**
  * MachineClass:
+ * @deprecation_reason: If set, the machine is marked as deprecated. The
+ *string should provide some clear information about what to use instead.
  * @max_cpus: maximum number of CPUs supported. Default: 1
  * @min_cpus: minimum number of CPUs supported. Default: 1
  * @default_cpus: number of CPUs instantiated if none are specified. Default: 1
@@ -166,6 +168,7 @@ struct MachineClass {
 char *name;
 const char *alias;
 const char *desc;
+const char *deprecation_reason;
 
 void (*init)(MachineState *state);
 void (*reset)(void);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e9b6f064fb..d357907b0b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -956,6 +956,7 @@ static void pc_i440fx_0_11_machine_options(MachineClass *m)
 {
 pc_i440fx_0_12_machine_options(m);
 m->hw_version = "0.11";
+m->deprecation_reason = "use a newer machine type instead";
 SET_MACHINE_COMPAT(m, PC_COMPAT_0_11);
 }
 
diff --git a/vl.c b/vl.c
index a37dfe4684..d26f19b06d 100644
--- a/vl.c
+++ b/vl.c
@@ -2560,8 +2560,9 @@ static gint machine_class_cmp(gconstpointer a, 
gconstpointer b)
 if (mc->alias) {
 printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, 
mc->name);
 }
-printf("%-20s %s%s\n", mc->name, mc->desc,
-   mc->is_default ? " (default)" : "");
+printf("%-20s %s%s%s\n", mc->name, mc->desc,
+   mc->is_default ? " (default)" : "",
+   mc->deprecation_reason ? " (deprecated)" : "");
 }
 }
 
@@ -4263,6 +4264,11 @@ int main(int argc, char **argv, char **envp)
 
 configure_accelerator(current_machine);
 
+if (!qtest_enabled() && machine_class->deprecation_reason) {
+error_report("Machine type '%s' is deprecated: %s",
+ machine_class->name, machine_class->deprecation_reason);
+}
+
 /*
  * Register all the global properties, including accel properties,
  * machine properties, and user-specified ones.
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 282bc3dc35..16fcb47901 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2943,6 +2943,11 @@ support page sizes < 4096 any longer.
 
 @section System emulator machines
 
+@subsection pc-0.10 and pc-0.11 (since 3.0)
+
+These machine types are very old and likely can not be used for live migration
+from old QEMU versions anymore. A newer machine type should be used instead.
+
 @section Device options
 
 @subsection Block device options
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 0/2] Machine queue, 2018-06-25

[Eduardo Habkost]

The following changes since commit 35e238c9330669882487f9929e0aa97900431853:

  Merge remote-tracking branch 
'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
15:25:26 +0100)

are available in the Git repository at:

  git://github.com/ehabkost/qemu.git tags/machine-next-pull-request

for you to fetch changes up to 08fe68244eb44f3c8ccecd35066eca8392d1345a:

  hw/i386: Deprecate the machine types pc-0.10 and pc-0.11 (2018-06-25 14:10:01 
-0300)


Machine queue, 2018-06-25

* Don't support --daemonize and --preconfig together
* Deprecate machine types pc-0.10 and pc-0.11



Queue for Machine Core patches


Igor Mammedov (1):
  vl.c: do not allow --daemonize in combination with --preconfig CLI
option

Thomas Huth (1):
  hw/i386: Deprecate the machine types pc-0.10 and pc-0.11

 include/hw/boards.h |  3 +++
 hw/i386/pc_piix.c   |  1 +
 vl.c| 16 ++--
 qemu-doc.texi   |  5 +
 4 files changed, 23 insertions(+), 2 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH] ehci: Don't fetch a NULL current qtd but advance the queue instead.

[Sebastian Bauer]

Fetching qtd with the NULL address most likely makes no sense so from now
on, we handle it this case similarly as if the terminate (T) bit is not
set, which is already an exception as according to section 3.6 of the EHCI
spec there is no T bit defined for the current_qtd field.

The spec is a bit vague on how an EHCI driver should initialize these
fields: "The general operational model is that the host controller can
detect whether the overlay area contains a description of an active
transfer" (p. 49). QEMU primarily uses the QTD_TOKEN_ACTIVE bit of the
queue header to infer the activity state but there are other ways
conceivable.

This change allows QEMU to boot further into AmigaOS. The public available
version of the EHCI driver recycles queue heads in some rare conditions but
only clears the current_qtd field but not the status field. This works with
many available EHCI PCI cards but e.g., not with the Freescale USB
controller's found on the P5040. On the emulated EHCI controller of QEMU
the consequence is that some garbage was read in, which resulted in a
reset of the controller. This change fixes the problem.

Signed-off-by: Sebastian Bauer 

---

The fix is probably not the best solution. However, it is one that is very
small.

QEMU still warns that an active qh will be changed if the qh is recycled,
so more changes could be done. But I didn't notice any negative
consequences implied by the warning so I would leave a possible fix to this
problem to a further patch.
---
 hw/usb/hcd-ehci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 0134232627..e5acfc5ba5 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1672,7 +1672,8 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int 
async)
 ehci_set_state(ehci, async, EST_HORIZONTALQH);
 
 } else if ((q->qh.token & QTD_TOKEN_ACTIVE) &&
-   (NLPTR_TBIT(q->qh.current_qtd) == 0)) {
+   (NLPTR_TBIT(q->qh.current_qtd) == 0) &&
+   (q->qh.current_qtd != 0)) {
 q->qtdaddr = q->qh.current_qtd;
 ehci_set_state(ehci, async, EST_FETCHQTD);
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2] docs: add guidance on configuring CPU models for x86

[Eduardo Habkost]

On Mon, Jun 18, 2018 at 11:18:47AM +0100, Daniel P. Berrangé wrote:
> With the recent set of CPU hardware vulnerabilities on x86, it is
> increasingly difficult to understand which CPU configurations are
> good to use and what flaws they might be vulnerable to.
> 
> This doc attempts to help management applications and administrators in
> picking sensible CPU configuration on x86 hosts. It outlines which of
> the named CPU models are good choices, and describes which extra CPU
> flags should be enabled to allow the guest to mitigate hardware flaws.
> 
> Signed-off-by: Daniel P. Berrangé 

This fails to build on a RHEL7 host:

# make qemu-doc.html V=1
LC_ALL=C makeinfo --no-split --number-sections -I docs -I . -I . --no-headers 
--html qemu-doc.texi -o qemu-doc.html
qemu-doc.texi:127: node `QEMU PC System emulator' lacks menu item for 
`cpu_models' despite being its Up target
qemu-doc.texi:605: node `cpu_models' lacks menu item for 
`recommendations_cpu_models_x86' despite being its Up target
docs/qemu-cpu-models.texi:40: node `recommendations_cpu_models_x86' lacks menu 
item for `preferred_cpu_models_intel_x86' despite being its Up target
docs/qemu-cpu-models.texi:40: node `recommendations_cpu_models_x86' lacks menu 
item for `important_cpu_features_intel_x86' despite being its Up target
docs/qemu-cpu-models.texi:40: node `recommendations_cpu_models_x86' lacks menu 
item for `preferred_cpu_models_amd_x86' despite being its Up target
docs/qemu-cpu-models.texi:40: node `recommendations_cpu_models_x86' lacks menu 
item for `important_cpu_features_amd_x86' despite being its Up target
docs/qemu-cpu-models.texi:40: node `recommendations_cpu_models_x86' lacks menu 
item for `default_cpu_models_x86' despite being its Up target
docs/qemu-cpu-models.texi:40: node `recommendations_cpu_models_x86' lacks menu 
item for `other_non_recommended_cpu_models_x86' despite being its Up target
qemu-doc.texi:605: node `cpu_models' lacks menu item for 
`cpu_model_syntax_apps' despite being its Up target
docs/qemu-cpu-models.texi:357: node `cpu_model_syntax_apps' lacks menu item for 
`cpu_model_syntax_qemu' despite being its Up target
docs/qemu-cpu-models.texi:357: node `cpu_model_syntax_apps' lacks menu item for 
`cpu_model_syntax_libvirt' despite being its Up target
make: *** [qemu-doc.html] Error 1
# rpm -qf /usr/bin/makeinfo 
texinfo-5.1-5.el7.x86_64

-- 
Eduardo

[Qemu-devel] [PULL 09/12] i386: Allow TOPOEXT to be enabled on older kernels

[Eduardo Habkost]

From: Babu Moger 

Enabling TOPOEXT feature might cause compatibility issues if
older kernels does not set this feature. Lets set this feature
unconditionally.

Signed-off-by: Babu Moger 
Message-Id: <1528939107-17193-2-git-send-email-babu.mo...@amd.com>
[ehabkost: rewrite comment and commit message]
Signed-off-by: Eduardo Habkost 
---
 target/i386/kvm.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 445e0e0b11..2d174f3a91 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -372,6 +372,13 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, 
uint32_t function,
 if (host_tsx_blacklisted()) {
 ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
 }
+} else if (function == 0x8001 && reg == R_ECX) {
+/*
+ * It's safe to enable TOPOEXT even if it's not returned by
+ * GET_SUPPORTED_CPUID.  Unconditionally enabling TOPOEXT here allows
+ * us to keep CPU models including TOPOEXT runnable on older kernels.
+ */
+ret |= CPUID_EXT3_TOPOEXT;
 } else if (function == 0x8001 && reg == R_EDX) {
 /* On Intel, kvm returns cpuid according to the Intel spec,
  * so add missing bits according to the AMD spec:
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 12/12] i386: Remove generic SMT thread check

[Eduardo Habkost]

From: Babu Moger 

Remove generic non-intel check while validating hyperthreading support.
Certain AMD CPUs can support hyperthreading now.

CPU family with TOPOEXT feature can support hyperthreading now.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
Reviewed-by: Eduardo Habkost 
Message-Id: <1529443919-67509-4-git-send-email-babu.mo...@amd.com>
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d6ed29b484..e6c2f8a22a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4985,17 +4985,22 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 
 qemu_init_vcpu(cs);
 
-/* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
- * issue by adjusting CPUID__0001_EBX and CPUID_8000_0008_ECX
- * based on inputs (sockets,cores,threads), it is still better to gives
+/*
+ * Most Intel and certain AMD CPUs support hyperthreading. Even though QEMU
+ * fixes this issue by adjusting CPUID__0001_EBX and 
CPUID_8000_0008_ECX
+ * based on inputs (sockets,cores,threads), it is still better to give
  * users a warning.
  *
  * NOTE: the following code has to follow qemu_init_vcpu(). Otherwise
  * cs->nr_threads hasn't be populated yet and the checking is incorrect.
  */
-if (!IS_INTEL_CPU(env) && cs->nr_threads > 1 && !ht_warned) {
-error_report("AMD CPU doesn't support hyperthreading. Please configure"
- " -smp options properly.");
+ if (IS_AMD_CPU(env) &&
+ !(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
+ cs->nr_threads > 1 && !ht_warned) {
+error_report("This family of AMD CPU doesn't support "
+ "hyperthreading(%d). Please configure -smp "
+ "options properly or try enabling topoext feature.",
+ cs->nr_threads);
 ht_warned = true;
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 11/12] i386: Enable TOPOEXT feature on AMD EPYC CPU

[Eduardo Habkost]

From: Babu Moger 

Enable TOPOEXT feature on EPYC CPU. This is required to support
hyperthreading on VM guests. Also extend xlevel to 0x801E.

Disable topoext on PC_COMPAT_2_12 and keep xlevel 0x800a.

Signed-off-by: Babu Moger 
Message-Id: <1529443919-67509-3-git-send-email-babu.mo...@amd.com>
[ehabkost: Added EPYC-IBPB.xlevel to PC_COMPAT_2_12]
Signed-off-by: Eduardo Habkost 
---
 include/hw/i386/pc.h | 12 
 target/i386/cpu.c| 10 ++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fc8dedca12..316230e570 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -303,6 +303,18 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 .driver   = TYPE_X86_CPU,\
 .property = "legacy-cache",\
 .value= "on",\
+},{\
+.driver   = TYPE_X86_CPU,\
+.property = "topoext",\
+.value= "off",\
+},{\
+.driver   = "EPYC-" TYPE_X86_CPU,\
+.property = "xlevel",\
+.value= stringify(0x800a),\
+},{\
+.driver   = "EPYC-IBPB" TYPE_X86_CPU,\
+.property = "xlevel",\
+.value= stringify(0x800a),\
 },
 
 #define PC_COMPAT_2_11 \
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 130391c840..d6ed29b484 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2579,7 +2579,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .features[FEAT_8000_0001_ECX] =
 CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
 CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
-CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+CPUID_EXT3_TOPOEXT,
 .features[FEAT_7_0_EBX] =
 CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 |
 CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
@@ -2594,7 +2595,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_XSAVE_XGETBV1,
 .features[FEAT_6_EAX] =
 CPUID_6_EAX_ARAT,
-.xlevel = 0x800A,
+.xlevel = 0x801E,
 .model_id = "AMD EPYC Processor",
 .cache_info = _cache_info,
 },
@@ -2624,7 +2625,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .features[FEAT_8000_0001_ECX] =
 CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
 CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
-CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+CPUID_EXT3_TOPOEXT,
 .features[FEAT_8000_0008_EBX] =
 CPUID_8000_0008_EBX_IBPB,
 .features[FEAT_7_0_EBX] =
@@ -2641,7 +2643,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_XSAVE_XGETBV1,
 .features[FEAT_6_EAX] =
 CPUID_6_EAX_ARAT,
-.xlevel = 0x800A,
+.xlevel = 0x801E,
 .model_id = "AMD EPYC Processor (with IBPB)",
 .cache_info = _cache_info,
 },
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 05/12] i386: Remove osxsave CPUID flag name

[Eduardo Habkost]

OSXAVE is not a static feature flag: it changes dynamically at
runtime depending on CR4, and it was never configurable: KVM
never returned OSXSAVE on GET_SUPPORTED_CPUID, and it is not
included in TCG_EXT_FEATURES.

Remove OSXSAVE from the feature name array so users don't try to
configure it manually.

Signed-off-by: Eduardo Habkost 
Message-Id: <20180611203855.13269-1-ehabk...@redhat.com>
Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9da4920421..60deae3100 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -788,7 +788,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 "fma", "cx16", "xtpr", "pdcm",
 NULL, "pcid", "dca", "sse4.1",
 "sse4.2", "x2apic", "movbe", "popcnt",
-"tsc-deadline", "aes", "xsave", "osxsave",
+"tsc-deadline", "aes", "xsave", NULL /* osxsave */,
 "avx", "f16c", "rdrand", "hypervisor",
 },
 .cpuid_eax = 1, .cpuid_reg = R_ECX,
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 07/12] i386: define the AMD 'amd-ssbd' CPUID feature bit

[Eduardo Habkost]

From: Konrad Rzeszutek Wilk 

AMD future CPUs expose _two_ ways to utilize the Intel equivalant
of the Speculative Store Bypass Disable. The first is via
the virtualized VIRT_SPEC CTRL MSR (0xC001_011f) and the second
is via the SPEC_CTRL MSR (0x48). The document titled:
124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf

gives priority of SPEC CTRL MSR over the VIRT SPEC CTRL MSR.

A copy of this document is available at
  https://bugzilla.kernel.org/show_bug.cgi?id=199889

Anyhow, this means that on future AMD CPUs there will be  _two_ ways to
deal with SSBD.

Signed-off-by: Konrad Rzeszutek Wilk 
Message-Id: <20180601153809.15259-2-konrad.w...@oracle.com>
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d7dfefcde0..7234bebfcb 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1009,7 +1009,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = 
{
 "ibpb", NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
-NULL, "virt-ssbd", NULL, NULL,
+"amd-ssbd", "virt-ssbd", NULL, NULL,
 NULL, NULL, NULL, NULL,
 },
 .cpuid_eax = 0x8008,
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [Bug 1778350] Re: Android-x86 4.4-r5 won't boot on QEMU since v2.11.0-rc2

[Michael S. Tsirkin]

On Mon, Jun 25, 2018 at 01:46:16PM -0400, John Snow wrote:
> CC Michael Tsirkin
> 
> On 06/25/2018 06:53 AM, navicrej wrote:
> > After 'git bisect' -ing it I found the commit that is responsible for
> > this:
> > 
> > https://git.qemu.org/?p=qemu.git;a=commitdiff;h=9fa99d2519cbf71f871e46871df12cb446dc1c3e
> > 

Can you pls try to set x-pci-hole64-fix=off and see whether that helps?

-- 
MST

[Qemu-devel] [PULL 04/12] i386: display known CPUID features linewrapped, in alphabetical order

[Eduardo Habkost]

From: Daniel P. Berrangé 

When using '-cpu help' the list of CPUID features is grouped according
to the internal low level CPUID grouping. The data printed results in
very long lines too.

This combines to make it hard for users to read the output and identify
if QEMU knows about the feature they wish to use.

This change gets rid of the grouping of features and treats all flags as
single list. The list is sorted into alphabetical order and the printing
with line wrapping at the 77th column.

Signed-off-by: Daniel P. Berrangé 
Message-Id: <20180606165527.17365-4-berra...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 41 +++--
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 19ac1e6569..9da4920421 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3331,17 +3331,21 @@ static void 
x86_cpu_class_check_missing_features(X86CPUClass *xcc,
 
 /* Print all cpuid feature names in featureset
  */
-static void listflags(FILE *f, fprintf_function print, const char **featureset)
+static void listflags(FILE *f, fprintf_function print, GList *features)
 {
-int bit;
-bool first = true;
-
-for (bit = 0; bit < 32; bit++) {
-if (featureset[bit]) {
-print(f, "%s%s", first ? "" : " ", featureset[bit]);
-first = false;
+size_t len = 0;
+GList *tmp;
+
+for (tmp = features; tmp; tmp = tmp->next) {
+const char *name = tmp->data;
+if ((len + strlen(name) + 1) >= 75) {
+print(f, "\n");
+len = 0;
 }
+print(f, "%s%s", len == 0 ? "  " : " ", name);
+len += strlen(name) + 1;
 }
+print(f, "\n");
 }
 
 /* Sort alphabetically by type name, respecting X86CPUClass::ordering. */
@@ -3392,26 +3396,35 @@ static void x86_cpu_list_entry(gpointer data, gpointer 
user_data)
 /* list available CPU models and flags */
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
-int i;
+int i, j;
 CPUListState s = {
 .file = f,
 .cpu_fprintf = cpu_fprintf,
 };
 GSList *list;
+GList *names = NULL;
 
 (*cpu_fprintf)(f, "Available CPUs:\n");
 list = get_sorted_cpu_model_list();
 g_slist_foreach(list, x86_cpu_list_entry, );
 g_slist_free(list);
 
-(*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
+names = NULL;
 for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
 FeatureWordInfo *fw = _word_info[i];
-
-(*cpu_fprintf)(f, "  ");
-listflags(f, cpu_fprintf, fw->feat_names);
-(*cpu_fprintf)(f, "\n");
+for (j = 0; j < 32; j++) {
+if (fw->feat_names[j]) {
+names = g_list_append(names, (gpointer)fw->feat_names[j]);
+}
+}
 }
+
+names = g_list_sort(names, (GCompareFunc)strcmp);
+
+(*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
+listflags(f, cpu_fprintf, names);
+(*cpu_fprintf)(f, "\n");
+g_list_free(names);
 }
 
 static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 10/12] i386: Fix up the Node id for CPUID_8000_001E

[Eduardo Habkost]

From: Babu Moger 

This is part of topoext support. To keep the compatibility, it is better
we support all the combination of nr_cores and nr_threads currently
supported. By allowing more nr_cores and nr_threads, we might end up with
more nodes than we can actually support with the real hardware. We need to
fix up the node id to make this work. We can achieve this by shifting the
socket_id bits left to address more nodes.

Signed-off-by: Babu Moger 
Message-Id: <1529443919-67509-2-git-send-email-babu.mo...@amd.com>
Reviewed-by: Eduardo Habkost 
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 7a4484bb06..130391c840 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
+#include "qemu/bitops.h"
 
 #include "cpu.h"
 #include "exec/exec-all.h"
@@ -472,6 +473,8 @@ static void encode_topo_cpuid801e(CPUState *cs, X86CPU 
*cpu,
uint32_t *ecx, uint32_t *edx)
 {
 struct core_topology topo = {0};
+unsigned long nodes;
+int shift;
 
 build_core_topology(cs->nr_cores, cpu->core_id, );
 *eax = cpu->apic_id;
@@ -504,7 +507,28 @@ static void encode_topo_cpuid801e(CPUState *cs, X86CPU 
*cpu,
  * 2  Socket id
  *   1:0  Node id
  */
-*ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) | topo.node_id;
+if (topo.num_nodes <= 4) {
+*ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
+topo.node_id;
+} else {
+/*
+ * Node id fix up. Actual hardware supports up to 4 nodes. But with
+ * more than 32 cores, we may end up with more than 4 nodes.
+ * Node id is a combination of socket id and node id. Only requirement
+ * here is that this number should be unique accross the system.
+ * Shift the socket id to accommodate more nodes. We dont expect both
+ * socket id and node id to be big number at the same time. This is not
+ * an ideal config but we need to to support it. Max nodes we can have
+ * is 32 (255/8) with 8 cores per node and 255 max cores. We only need
+ * 5 bits for nodes. Find the left most set bit to represent the total
+ * number of nodes. find_last_bit returns last set bit(0 based). Left
+ * shift(+1) the socket id to represent all the nodes.
+ */
+nodes = topo.num_nodes - 1;
+shift = find_last_bit(, 8);
+*ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << (shift + 1)) |
+topo.node_id;
+}
 *edx = 0;
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 08/12] i386: Define AMD's no SSB mitigation needed.

[Eduardo Habkost]

From: Konrad Rzeszutek Wilk 

AMD future CPUs expose a mechanism to tell the guest that the
Speculative Store Bypass Disable is not needed and that the
CPU is all good.

This is exposed via the CPUID 8000_0008.EBX[26] bit.

See 124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf

A copy of this document is available at
https://bugzilla.kernel.org/show_bug.cgi?id=199889

Signed-off-by: Konrad Rzeszutek Wilk 
Message-Id: <20180601153809.15259-3-konrad.w...@oracle.com>
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 7234bebfcb..7a4484bb06 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1009,7 +1009,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = 
{
 "ibpb", NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
-"amd-ssbd", "virt-ssbd", NULL, NULL,
+"amd-ssbd", "virt-ssbd", "amd-no-ssb", NULL,
 NULL, NULL, NULL, NULL,
 },
 .cpuid_eax = 0x8008,
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 02/12] i386: improve alignment of CPU model listing

[Eduardo Habkost]

From: Daniel P. Berrangé 

Since the addition of the -IBRS CPU model variants, the descriptions
shown by '-cpu help' are not well aligned, as several model names
overflow the space allowed. Right aligning the CPU model names is also
not attractive, because it obscures the common name prefixes of many
models. The CPU model name field needs to be 4 characters larger, and
be left aligned instead.

Signed-off-by: Daniel P. Berrangé 
Message-Id: <20180606165527.17365-2-berra...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 86fb1a4fb8..e1d7157d8c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3380,7 +3380,7 @@ static void x86_cpu_list_entry(gpointer data, gpointer 
user_data)
 desc = cc->cpu_def->model_id;
 }
 
-(*s->cpu_fprintf)(s->file, "x86 %16s  %-48s\n",
+(*s->cpu_fprintf)(s->file, "x86 %-20s  %-48s\n",
   name, desc);
 g_free(name);
 }
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 06/12] i386: Remove ospke CPUID flag name

[Eduardo Habkost]

OSPKE is not a static feature flag: it changes dynamically at
runtime depending on CR4, and it was never configurable: KVM
never returned OSPKE on GET_SUPPORTED_CPUID, and on TCG enables
it automatically if CR4_PKE_MASK is set.

Remove OSPKE from the feature name array so users don't try to
configure it manually.

Signed-off-by: Eduardo Habkost 
Message-Id: <20180611203712.12086-1-ehabk...@redhat.com>
Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 60deae3100..d7dfefcde0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -738,7 +738,8 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t 
vendor1,
   CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
   CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
   CPUID_7_0_EBX_RDSEED */
-#define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE | \
+#define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | \
+  /* CPUID_7_0_ECX_OSPKE is dynamic */ \
   CPUID_7_0_ECX_LA57)
 #define TCG_7_0_EDX_FEATURES 0
 #define TCG_APM_FEATURES 0
@@ -955,7 +956,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 [FEAT_7_0_ECX] = {
 .feat_names = {
 NULL, "avx512vbmi", "umip", "pku",
-"ospke", NULL, "avx512vbmi2", NULL,
+NULL /* ospke */, NULL, "avx512vbmi2", NULL,
 "gfni", "vaes", "vpclmulqdq", "avx512vnni",
 "avx512bitalg", NULL, "avx512-vpopcntdq", NULL,
 "la57", NULL, NULL, NULL,
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 01/12] i386: Add support for CPUID_8000_001E for AMD

[Eduardo Habkost]

From: Babu Moger 

Add support for cpuid leaf CPUID_8000_001E. Build the config that closely
match the underlying hardware. Please refer to the Processor Programming
Reference (PPR) for AMD Family 17h Model for more details.

Signed-off-by: Babu Moger 
Message-Id: <1528498581-131037-2-git-send-email-babu.mo...@amd.com>
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 86 +++
 1 file changed, 86 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1e69e68f25..86fb1a4fb8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -427,6 +427,87 @@ static void encode_cache_cpuid801d(CPUCacheInfo 
*cache, CPUState *cs,
(cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
 }
 
+/* Data structure to hold the configuration info for a given core index */
+struct core_topology {
+/* core complex id of the current core index */
+int ccx_id;
+/*
+ * Adjusted core index for this core in the topology
+ * This can be 0,1,2,3 with max 4 cores in a core complex
+ */
+int core_id;
+/* Node id for this core index */
+int node_id;
+/* Number of nodes in this config */
+int num_nodes;
+};
+
+/*
+ * Build the configuration closely match the EPYC hardware. Using the EPYC
+ * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE)
+ * right now. This could change in future.
+ * nr_cores : Total number of cores in the config
+ * core_id  : Core index of the current CPU
+ * topo : Data structure to hold all the config info for this core index
+ */
+static void build_core_topology(int nr_cores, int core_id,
+struct core_topology *topo)
+{
+int nodes, cores_in_ccx;
+
+/* First get the number of nodes required */
+nodes = nodes_in_socket(nr_cores);
+
+cores_in_ccx = cores_in_core_complex(nr_cores);
+
+topo->node_id = core_id / (cores_in_ccx * MAX_CCX);
+topo->ccx_id = (core_id % (cores_in_ccx * MAX_CCX)) / cores_in_ccx;
+topo->core_id = core_id % cores_in_ccx;
+topo->num_nodes = nodes;
+}
+
+/* Encode cache info for CPUID[801E] */
+static void encode_topo_cpuid801e(CPUState *cs, X86CPU *cpu,
+   uint32_t *eax, uint32_t *ebx,
+   uint32_t *ecx, uint32_t *edx)
+{
+struct core_topology topo = {0};
+
+build_core_topology(cs->nr_cores, cpu->core_id, );
+*eax = cpu->apic_id;
+/*
+ * CPUID_Fn801E_EBX
+ * 31:16 Reserved
+ * 15:8  Threads per core (The number of threads per core is
+ *   Threads per core + 1)
+ *  7:0  Core id (see bit decoding below)
+ *   SMT:
+ *   4:3 node id
+ * 2 Core complex id
+ *   1:0 Core id
+ *   Non SMT:
+ *   5:4 node id
+ * 3 Core complex id
+ *   1:0 Core id
+ */
+if (cs->nr_threads - 1) {
+*ebx = ((cs->nr_threads - 1) << 8) | (topo.node_id << 3) |
+(topo.ccx_id << 2) | topo.core_id;
+} else {
+*ebx = (topo.node_id << 4) | (topo.ccx_id << 3) | topo.core_id;
+}
+/*
+ * CPUID_Fn801E_ECX
+ * 31:11 Reserved
+ * 10:8  Nodes per processor (Nodes per processor is number of nodes + 1)
+ *  7:0  Node id (see bit decoding below)
+ * 2  Socket id
+ *   1:0  Node id
+ */
+*ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) | topo.node_id;
+*edx = 0;
+}
+
 /*
  * Definitions of the hardcoded cache entries we expose:
  * These are legacy cache values. If there is a need to change any
@@ -4120,6 +4201,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 break;
 }
 break;
+case 0x801E:
+assert(cpu->core_id <= 255);
+encode_topo_cpuid801e(cs, cpu,
+  eax, ebx, ecx, edx);
+break;
 case 0xC000:
 *eax = env->cpuid_xlevel2;
 *ebx = 0;
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 03/12] i386: improve sorting of CPU model names

[Eduardo Habkost]

From: Daniel P. Berrangé 

The current list of CPU model names output by "-cpu help" is sorted
alphabetically based on the internal QOM class name. The text that is
displayed, however, uses the CPU model name, which is equivalent to the
QOM class name, minus a suffix. Unfortunately that suffix has an effect
on the sort ordering, for example, causing the various Broadwell
variants to appear reversed:

  x86 486
  x86 Broadwell-IBRSIntel Core Processor (Broadwell, IBRS)
  x86 Broadwell-noTSX-IBRS  Intel Core Processor (Broadwell, no TSX, IBRS
  x86 Broadwell-noTSX   Intel Core Processor (Broadwell, no TSX)
  x86 Broadwell Intel Core Processor (Broadwell)
  x86 ConroeIntel Celeron_4x0 (Conroe/Merom Class Core 2)

By sorting on the actual CPU model name text that is displayed, the
result is

  x86 486
  x86 Broadwell Intel Core Processor (Broadwell)
  x86 Broadwell-IBRSIntel Core Processor (Broadwell, IBRS)
  x86 Broadwell-noTSX   Intel Core Processor (Broadwell, no TSX)
  x86 Broadwell-noTSX-IBRS  Intel Core Processor (Broadwell, no TSX, IBRS)
  x86 ConroeIntel Celeron_4x0 (Conroe/Merom Class Core 2)

This requires extra string allocations during sorting, but this is not a
concern given the usage scenario and the number of CPU models that exist.

Signed-off-by: Daniel P. Berrangé 
Message-Id: <20180606165527.17365-3-berra...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e1d7157d8c..19ac1e6569 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3351,15 +3351,19 @@ static gint x86_cpu_list_compare(gconstpointer a, 
gconstpointer b)
 ObjectClass *class_b = (ObjectClass *)b;
 X86CPUClass *cc_a = X86_CPU_CLASS(class_a);
 X86CPUClass *cc_b = X86_CPU_CLASS(class_b);
-const char *name_a, *name_b;
+char *name_a, *name_b;
+int ret;
 
 if (cc_a->ordering != cc_b->ordering) {
-return cc_a->ordering - cc_b->ordering;
+ret = cc_a->ordering - cc_b->ordering;
 } else {
-name_a = object_class_get_name(class_a);
-name_b = object_class_get_name(class_b);
-return strcmp(name_a, name_b);
+name_a = x86_cpu_class_get_model_name(cc_a);
+name_b = x86_cpu_class_get_model_name(cc_b);
+ret = strcmp(name_a, name_b);
+g_free(name_a);
+g_free(name_b);
 }
+return ret;
 }
 
 static GSList *get_sorted_cpu_model_list(void)
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 00/12] x86 queue, 2018-06-25

[Eduardo Habkost]

The following changes since commit 35e238c9330669882487f9929e0aa97900431853:

  Merge remote-tracking branch 
'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
15:25:26 +0100)

are available in the Git repository at:

  git://github.com/ehabkost/qemu.git tags/x86-next-pull-request

for you to fetch changes up to 0b6e9aa89e02c8b213af019aad816e00ba8243f8:

  Merge branch 'master' of git://git.qemu.org/qemu into x86-next (2018-06-25 
14:10:56 -0300)


x86 queue, 2018-06-25

* Add TOPOEXT feature to EPYC CPU model
* AMD's amd-ssbd and amd-no-ssbd CPUID features
* Removed unused CPUID flag names: ospke, osxsave
* Better formatting of '-cpu help'



Babu Moger (5):
  i386: Add support for CPUID_8000_001E for AMD
  i386: Allow TOPOEXT to be enabled on older kernels
  i386: Fix up the Node id for CPUID_8000_001E
  i386: Enable TOPOEXT feature on AMD EPYC CPU
  i386: Remove generic SMT thread check

Daniel P. Berrangé (3):
  i386: improve alignment of CPU model listing
  i386: improve sorting of CPU model names
  i386: display known CPUID features linewrapped, in alphabetical order

Eduardo Habkost (2):
  i386: Remove osxsave CPUID flag name
  i386: Remove ospke CPUID flag name

Konrad Rzeszutek Wilk (2):
  i386: define the AMD 'amd-ssbd' CPUID feature bit
  i386: Define AMD's no SSB mitigation needed.

 include/hw/i386/pc.h |  12 +++
 target/i386/cpu.c| 203 +++
 target/i386/kvm.c|   7 ++
 3 files changed, 188 insertions(+), 34 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH] fix fdiv instruction

[Programmingkid]

> On Jun 25, 2018, at 5:08 PM, Richard Henderson  
> wrote:
> 
> On Mon, Jun 25, 2018, 08:23 G 3  wrote:
> >
> > Try
> >
> > uint64_t expected_answer = 0xdeadbeef;
> > ...
> > c.i = expected_answer;
> > asm volatile("fdiv %0, %1, %2" : "+f"(c.d) : "f"(1.0), "f"(0.0));
> >
> > to avoid depending on uninitialized data.  (This expected value is
> > an SNaN with a deadbeef marker Just to be Sure.)
> >
> >
> > r~
> 
> 
> Ok I made this program and tried it on my iMac G5 (PowerPC 970).
> 
> #include 
> #include 
> #include 
> 
> // Used to convert unsigned integer <--> double
> union Converter
> {
>  double d;
>  uint64_t i;
> };
> typedef union Converter Converter;
> 
> int main (int argc, const char * argv[]) {
>  Converter answer;
>  answer.i = 0xdeadbeef;
>  //asm volatile("mtfsb1 27"); /* Set ZE bit */
>  asm volatile("fdiv %0, %1, %2" : "=f"(answer.d) : "f"(1.0),  
> "f"(0.0));
> 
> Need +f for inout operand.
> This didn't test what you expected.

What do you mean by inout operand?

If you could send me some sample code I will test it out.

Re: [Qemu-devel] [PATCH 0/2] dirty bitmap fixes

[John Snow]

On 06/25/2018 12:57 PM, Vladimir Sementsov-Ogievskiy wrote:
> Vladimir Sementsov-Ogievskiy (2):
>   block/dirty-bitmap: add bdrv_enable_dirty_bitmap_locked
>   dirty-bitmap: fix double lock on bitmap enabling
> 
>  include/block/dirty-bitmap.h   |  1 +
>  block/dirty-bitmap.c   | 12 +---
>  migration/block-dirty-bitmap.c |  4 ++--
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 

Thanks, applied to my bitmaps tree:

https://github.com/jnsnow/qemu/commits/bitmaps
https://github.com/jnsnow/qemu.git

--js

Re: [Qemu-devel] [PATCH 0/2] dirty bitmap fixes

[John Snow]

On 06/25/2018 12:57 PM, Vladimir Sementsov-Ogievskiy wrote:
> Vladimir Sementsov-Ogievskiy (2):
>   block/dirty-bitmap: add bdrv_enable_dirty_bitmap_locked
>   dirty-bitmap: fix double lock on bitmap enabling
> 
>  include/block/dirty-bitmap.h   |  1 +
>  block/dirty-bitmap.c   | 12 +---
>  migration/block-dirty-bitmap.c |  4 ++--
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 

ACK,

though it looks really odd that we've got enable/disable but only one
corresponding _locked version.

I have a patch somewhere in my local files to create a combined
_set_disabled() that would cut the number of functions we have by a
little bit.

I'll take this first though, as an iotest fix.

--js

Re: [Qemu-devel] [PULL 1/1] ahci: fix FIS I bit and PIO Setup FIS interrupt

[John Snow]

Michael: It's probably much too late to include this in the 2.12.1
roundup, isn't it?

I'd either push for you to include this fix OR to drop the other ATAPI
related fix...

--js

On 06/25/2018 05:11 PM, John Snow wrote:
> From: Paolo Bonzini 
> 
> The "I" bit in PIO Setup and D2H FISes is exclusively a device concept
> and the irqstatus register in the controller does not matter.  The SATA
> spec says when it should be one; for D2H FISes in practice it is always
> set, while the PIO Setup FIS has several subcases that are documented in
> the patch.
> 
> Also, the PIO Setup FIS interrupt is actually generated _after_ data
> has been received.
> 
> Someone should probably spend some time reading the SATA specification and
> figuring out the more obscure fields in the PIO Setup FIS, but this is enough
> to fix SeaBIOS booting from ATAPI CD-ROMs over an AHCI controller.
> 
> Fixes: 956556e131e35f387ac482ad7b41151576fef057
> Reported-by: Gerd Hoffmann 
> Signed-off-by: Paolo Bonzini 
> Reviewed-by: John Snow 
> Message-id: 20180622165159.19863-1-pbonz...@redhat.com
> [Minor edit to avoid ATAPI comment ambiguity. --js]
> Signed-off-by: John Snow 
> ---
>  hw/ide/ahci.c  | 37 +
>  hw/ide/ahci_internal.h |  2 +-
>  tests/libqos/ahci.c| 25 -
>  tests/libqos/ahci.h|  2 +-
>  4 files changed, 43 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 2ec24cad9f..d700ca973b 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -801,7 +801,7 @@ static void ahci_write_fis_sdb(AHCIState *s, 
> NCQTransferState *ncq_tfs)
>  }
>  }
>  
> -static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
> +static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len, bool pio_fis_i)
>  {
>  AHCIPortRegs *pr = >port_regs;
>  uint8_t *pio_fis;
> @@ -814,7 +814,7 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t 
> len)
>  pio_fis = >res_fis[RES_FIS_PSFIS];
>  
>  pio_fis[0] = SATA_FIS_TYPE_PIO_SETUP;
> -pio_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
> +pio_fis[1] = (pio_fis_i ? (1 << 6) : 0);
>  pio_fis[2] = s->status;
>  pio_fis[3] = s->error;
>  
> @@ -842,8 +842,6 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t 
> len)
>  if (pio_fis[2] & ERR_STAT) {
>  ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_TFES);
>  }
> -
> -ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_PSS);
>  }
>  
>  static bool ahci_write_fis_d2h(AHCIDevice *ad)
> @@ -860,7 +858,7 @@ static bool ahci_write_fis_d2h(AHCIDevice *ad)
>  d2h_fis = >res_fis[RES_FIS_RFIS];
>  
>  d2h_fis[0] = SATA_FIS_TYPE_REGISTER_D2H;
> -d2h_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
> +d2h_fis[1] = (1 << 6); /* interrupt bit */
>  d2h_fis[2] = s->status;
>  d2h_fis[3] = s->error;
>  
> @@ -1258,11 +1256,10 @@ static void handle_reg_h2d_fis(AHCIState *s, int port,
>  trace_handle_reg_h2d_fis_dump(s, port, pretty_fis);
>  g_free(pretty_fis);
>  }
> -s->dev[port].done_atapi_packet = false;
>  }
>  
>  ide_state->error = 0;
> -
> +s->dev[port].done_first_drq = false;
>  /* Reset transferred byte counter */
>  cmd->status = 0;
>  
> @@ -1351,13 +1348,23 @@ static void ahci_pio_transfer(IDEDMA *dma)
>  int is_write = opts & AHCI_CMD_WRITE;
>  int is_atapi = opts & AHCI_CMD_ATAPI;
>  int has_sglist = 0;
> +bool pio_fis_i;
>  
> -/* PIO FIS gets written prior to transfer */
> -ahci_write_fis_pio(ad, size);
> +/* The PIO Setup FIS is received prior to transfer, but the interrupt
> + * is only triggered after data is received.
> + *
> + * The device only sets the 'I' bit in the PIO Setup FIS for device->host
> + * requests (see "DPIOI1" in the SATA spec), or for host->device DRQs 
> after
> + * the first (see "DPIOO1").  The latter is consistent with the spec's
> + * description of the PACKET protocol, where the command part of ATAPI 
> requests
> + * ("DPKT0") has the 'I' bit clear, while the data part of PIO ATAPI 
> requests
> + * ("DPKT4a" and "DPKT7") has the 'I' bit set for both directions for 
> all DRQs.
> + */
> +pio_fis_i = ad->done_first_drq || (!is_atapi && !is_write);
> +ahci_write_fis_pio(ad, size, pio_fis_i);
>  
> -if (is_atapi && !ad->done_atapi_packet) {
> +if (is_atapi && !ad->done_first_drq) {
>  /* already prepopulated iobuffer */
> -ad->done_atapi_packet = true;
>  goto out;
>  }
>  
> @@ -1379,9 +1386,15 @@ static void ahci_pio_transfer(IDEDMA *dma)
>  
>  /* Update number of transferred bytes, destroy sglist */
>  dma_buf_commit(s, size);
> +
>  out:
>  /* declare that we processed everything */
>  s->data_ptr = s->data_end;
> +
> +ad->done_first_drq = true;
> +if (pio_fis_i) {
> +ahci_trigger_irq(ad->hba, ad, 

[Qemu-devel] [PULL 0/1] Ide patches

[John Snow]

The following changes since commit 35e238c9330669882487f9929e0aa97900431853:

  Merge remote-tracking branch 
'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
15:25:26 +0100)

are available in the Git repository at:

  https://github.com/jnsnow/qemu.git tags/ide-pull-request

for you to fetch changes up to ae79c2db150e17757ee1be080481be675a15ccea:

  ahci: fix FIS I bit and PIO Setup FIS interrupt (2018-06-25 16:50:48 -0400)


Pull request



Paolo Bonzini (1):
  ahci: fix FIS I bit and PIO Setup FIS interrupt

 hw/ide/ahci.c  | 37 +
 hw/ide/ahci_internal.h |  2 +-
 tests/libqos/ahci.c| 25 -
 tests/libqos/ahci.h|  2 +-
 4 files changed, 43 insertions(+), 23 deletions(-)

-- 
2.14.4

[Qemu-devel] [PULL 1/1] ahci: fix FIS I bit and PIO Setup FIS interrupt

[John Snow]

From: Paolo Bonzini 

The "I" bit in PIO Setup and D2H FISes is exclusively a device concept
and the irqstatus register in the controller does not matter.  The SATA
spec says when it should be one; for D2H FISes in practice it is always
set, while the PIO Setup FIS has several subcases that are documented in
the patch.

Also, the PIO Setup FIS interrupt is actually generated _after_ data
has been received.

Someone should probably spend some time reading the SATA specification and
figuring out the more obscure fields in the PIO Setup FIS, but this is enough
to fix SeaBIOS booting from ATAPI CD-ROMs over an AHCI controller.

Fixes: 956556e131e35f387ac482ad7b41151576fef057
Reported-by: Gerd Hoffmann 
Signed-off-by: Paolo Bonzini 
Reviewed-by: John Snow 
Message-id: 20180622165159.19863-1-pbonz...@redhat.com
[Minor edit to avoid ATAPI comment ambiguity. --js]
Signed-off-by: John Snow 
---
 hw/ide/ahci.c  | 37 +
 hw/ide/ahci_internal.h |  2 +-
 tests/libqos/ahci.c| 25 -
 tests/libqos/ahci.h|  2 +-
 4 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 2ec24cad9f..d700ca973b 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -801,7 +801,7 @@ static void ahci_write_fis_sdb(AHCIState *s, 
NCQTransferState *ncq_tfs)
 }
 }
 
-static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
+static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len, bool pio_fis_i)
 {
 AHCIPortRegs *pr = >port_regs;
 uint8_t *pio_fis;
@@ -814,7 +814,7 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
 pio_fis = >res_fis[RES_FIS_PSFIS];
 
 pio_fis[0] = SATA_FIS_TYPE_PIO_SETUP;
-pio_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
+pio_fis[1] = (pio_fis_i ? (1 << 6) : 0);
 pio_fis[2] = s->status;
 pio_fis[3] = s->error;
 
@@ -842,8 +842,6 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
 if (pio_fis[2] & ERR_STAT) {
 ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_TFES);
 }
-
-ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_PSS);
 }
 
 static bool ahci_write_fis_d2h(AHCIDevice *ad)
@@ -860,7 +858,7 @@ static bool ahci_write_fis_d2h(AHCIDevice *ad)
 d2h_fis = >res_fis[RES_FIS_RFIS];
 
 d2h_fis[0] = SATA_FIS_TYPE_REGISTER_D2H;
-d2h_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
+d2h_fis[1] = (1 << 6); /* interrupt bit */
 d2h_fis[2] = s->status;
 d2h_fis[3] = s->error;
 
@@ -1258,11 +1256,10 @@ static void handle_reg_h2d_fis(AHCIState *s, int port,
 trace_handle_reg_h2d_fis_dump(s, port, pretty_fis);
 g_free(pretty_fis);
 }
-s->dev[port].done_atapi_packet = false;
 }
 
 ide_state->error = 0;
-
+s->dev[port].done_first_drq = false;
 /* Reset transferred byte counter */
 cmd->status = 0;
 
@@ -1351,13 +1348,23 @@ static void ahci_pio_transfer(IDEDMA *dma)
 int is_write = opts & AHCI_CMD_WRITE;
 int is_atapi = opts & AHCI_CMD_ATAPI;
 int has_sglist = 0;
+bool pio_fis_i;
 
-/* PIO FIS gets written prior to transfer */
-ahci_write_fis_pio(ad, size);
+/* The PIO Setup FIS is received prior to transfer, but the interrupt
+ * is only triggered after data is received.
+ *
+ * The device only sets the 'I' bit in the PIO Setup FIS for device->host
+ * requests (see "DPIOI1" in the SATA spec), or for host->device DRQs after
+ * the first (see "DPIOO1").  The latter is consistent with the spec's
+ * description of the PACKET protocol, where the command part of ATAPI 
requests
+ * ("DPKT0") has the 'I' bit clear, while the data part of PIO ATAPI 
requests
+ * ("DPKT4a" and "DPKT7") has the 'I' bit set for both directions for all 
DRQs.
+ */
+pio_fis_i = ad->done_first_drq || (!is_atapi && !is_write);
+ahci_write_fis_pio(ad, size, pio_fis_i);
 
-if (is_atapi && !ad->done_atapi_packet) {
+if (is_atapi && !ad->done_first_drq) {
 /* already prepopulated iobuffer */
-ad->done_atapi_packet = true;
 goto out;
 }
 
@@ -1379,9 +1386,15 @@ static void ahci_pio_transfer(IDEDMA *dma)
 
 /* Update number of transferred bytes, destroy sglist */
 dma_buf_commit(s, size);
+
 out:
 /* declare that we processed everything */
 s->data_ptr = s->data_end;
+
+ad->done_first_drq = true;
+if (pio_fis_i) {
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_PSS);
+}
 }
 
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
@@ -1627,7 +1640,7 @@ static const VMStateDescription vmstate_ahci_device = {
 VMSTATE_UINT32(port_regs.scr_err, AHCIDevice),
 VMSTATE_UINT32(port_regs.scr_act, AHCIDevice),
 VMSTATE_UINT32(port_regs.cmd_issue, AHCIDevice),
-VMSTATE_BOOL(done_atapi_packet, AHCIDevice),
+VMSTATE_BOOL(done_first_drq, AHCIDevice),
 VMSTATE_INT32(busy_slot, AHCIDevice),
 

Re: [Qemu-devel] [PATCH 1/2] ide/hw/core: fix crash on processing a partial-sector-size DMA xfer

[John Snow]

On 06/20/2018 12:29 AM, Amol Surati wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1777315
> 
> QEMU's short PRD policy applies to a DMA transfer of size < 512 bytes.
> But it fails to consider transfers which are >= 512 bytes, but are
> not a multiple of 512 bytes.
> 
> Such transfers are not subject to the short PRD policy. They end up
> violating the assumptions about the granularity of the IO sizes,
> upon which depend the verification of the completion of the previous
> transfer, and the advancement of the offset in preparation of the next.
> 
> Those violations result in the crash.
> 
> By forcing each transfer to be a multiple of sector size, such
> transfers are subjected to the policy, and therefore culled before they
> cause the crash.
> 

So now even if the PRDT we get is greater than a sector is not an even
multiple of 512, we reject it as too short.

That doesn't seem correct to me.

> Signed-off-by: Amol Surati 
> ---
>  hw/ide/core.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 2c62efc536..14d135224b 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -836,6 +836,7 @@ static void ide_dma_cb(void *opaque, int ret)
>  {
>  IDEState *s = opaque;
>  int n;
> +int32_t size_prepared;
>  int64_t sector_num;
>  uint64_t offset;
>  bool stay_active = false;
> @@ -886,7 +887,9 @@ static void ide_dma_cb(void *opaque, int ret)
>  n = s->nsector;
>  s->io_buffer_index = 0;
>  s->io_buffer_size = n * 512;
> -if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size) < 512) 
> {
> +size_prepared = s->bus->dma->ops->prepare_buf(s->bus->dma,
> +  s->io_buffer_size);
> +if (size_prepared <= 0 || size_prepared % 512) {
>  /* The PRDs were too short. Reset the Active bit, but don't raise an
>   * interrupt. */
>  s->status = READY_STAT | SEEK_STAT;
> 

-- 
—js

Re: [Qemu-devel] [PATCH] fix fdiv instruction

[Richard Henderson]

On Mon, Jun 25, 2018, 08:23 G 3  wrote:

> >
> > Try
> >
> > uint64_t expected_answer = 0xdeadbeef;
> > ...
> > c.i = expected_answer;
> > asm volatile("fdiv %0, %1, %2" : "+f"(c.d) : "f"(1.0), "f"(0.0));
> >
> > to avoid depending on uninitialized data.  (This expected value is
> > an SNaN with a deadbeef marker Just to be Sure.)
> >
> >
> > r~
>
>
> Ok I made this program and tried it on my iMac G5 (PowerPC 970).
>
> #include 
> #include 
> #include 
>
> // Used to convert unsigned integer <--> double
> union Converter
> {
>  double d;
>  uint64_t i;
> };
> typedef union Converter Converter;
>
> int main (int argc, const char * argv[]) {
>  Converter answer;
>  answer.i = 0xdeadbeef;
>  //asm volatile("mtfsb1 27"); /* Set ZE bit */
>  asm volatile("fdiv %0, %1, %2" : "=f"(answer.d) : "f"(1.0),
> "f"(0.0));
>

Need +f for inout operand.
This didn't test what you expected.

r~

Re: [Qemu-devel] [PATCH v3 5/5] tests/tcg/aarch64: userspace system register test

[Alex Bennée]

Alex Bennée  writes:

> This tests a bunch of registers that the kernel allows userspace to
> read including the CPUID registers.
>
> Signed-off-by: Alex Bennée 
> ---
>  tests/tcg/aarch64/Makefile.target |  2 +-
>  tests/tcg/aarch64/sysregs.c   | 99 +++
>  2 files changed, 100 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tcg/aarch64/sysregs.c
>
> diff --git a/tests/tcg/aarch64/Makefile.target 
> b/tests/tcg/aarch64/Makefile.target
> index 08c45b8470..cc1a7eb486 100644
> --- a/tests/tcg/aarch64/Makefile.target
> +++ b/tests/tcg/aarch64/Makefile.target
> @@ -7,7 +7,7 @@ VPATH += $(AARCH64_SRC)
>
>  # we don't build any of the ARM tests
>  AARCH64_TESTS=$(filter-out $(ARM_TESTS), $(TESTS))
> -AARCH64_TESTS+=fcvt
> +AARCH64_TESTS+=fcvt sysregs
>  TESTS:=$(AARCH64_TESTS)
>
>  fcvt: LDFLAGS+=-lm
> diff --git a/tests/tcg/aarch64/sysregs.c b/tests/tcg/aarch64/sysregs.c
> new file mode 100644
> index 00..177d1fe33b
> --- /dev/null
> +++ b/tests/tcg/aarch64/sysregs.c
> @@ -0,0 +1,99 @@
> +/*
> + * Check emulated system register access for linux-user mode.
> + *
> + * See: 
> https://www.kernel.org/doc/Documentation/arm64/cpu-feature-registers.txt
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +

> +
> +/* when (getauxval(AT_HWCAP) & HWCAP_CPUID), since 4.11*/
> +if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) {
> +printf("CPUID registers unavailable\n");
> +return 1;
> +} else {
> +printf("Checking CPUID registers\n");
> +}

Annoyingly this fails on qemu:debian-arm64-cross as it uses an older set
of headers than my desktop cross environment:

  aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.3.0-16ubuntu3) 7.3.0

with the libc:

  Source: cross-toolchain-base (25ubuntu6)
  Version: 2.27-3ubuntu1cross1
  Provides: libc6-arm64-dcv1

So I'm thinking an #ifndef HWCAP_CPUID and define it would be acceptable
for a test case.

--
Alex Bennée

[Qemu-devel] [PATCH 1/1] target/m68k: correctly disassemble move16

[Laurent Vivier]

"move16 %a0@+,%a1@" and "fmovel (cpid=3) %a0@-,%fpcr"
share the same opcode.

To fix that, backport the fix from binutils:

  2005-11-10  Andreas Schwab  

 * m68k-dis.c (print_insn_m68k): Only match FPU insns with
 coprocessor ID 1.

Reported-by: Thomas Huth 
Signed-off-by: Laurent Vivier 
---
 disas/m68k.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/disas/m68k.c b/disas/m68k.c
index 61b689ef3e..a687df437c 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -2017,6 +2017,20 @@ print_insn_m68k (bfd_vma memaddr, disassemble_info *info)
}
}
 
+  /* Don't match FPU insns with non-default coprocessor ID.  */
+  if (*d == '\0')
+{
+  for (d = opc->args; *d; d += 2)
+{
+  if (d[0] == 'I')
+{
+  val = fetch_arg (buffer, 'd', 3, info);
+  if (val != 1)
+break;
+}
+}
+}
+
  if (*d == '\0')
if ((val = match_insn_m68k (memaddr, info, opc, & priv)))
  return val;
-- 
2.14.4

[Qemu-devel] [PATCH 0/1] target/m68k: correctly disassemble move16

[Laurent Vivier]

"move16 %a0@+,%a1@" and "fmovel (cpid=3) %a0@-,%fpcr" share the same opcode...
but QEMU executes move16 (and M68040 too).

You can try:

--8<--- move16.S
.data

src:
.long 0x01020304, 0x05060708, 0x090a0b0c, 0x0d0e0f00
dst:
.long 0, 0, 0, 0

.text
.globl _start
_start:
lea src,%a0
lea dst,%a1

.fopt id=3
fmovel -(%a0),%fpcr
move16 (%a0)+, (%a1)+

move.l #0,%d1
move.l #1, %d0
trap #0
--8<--- move16.S

m68k-linux-gnu-gcc -g -m68040 -nostartfiles  -nodefaultlibs \
   -nostdlib -o move16 move16.S

m68k-linux-gnu-objdump -d move16

move16: file format elf32-m68k

Disassembly of section .text:

80b8 <_start>:
80b8:   41f9 8000 20d4  lea 800020d4 ,%a0
80be:   43f9 8000 20e4  lea 800020e4 ,%a1
80c4:   f620 9000   move16 %a0@+,%a1@+
80c8:   f620 9000   move16 %a0@+,%a1@+
80cc:   7200moveq #0,%d1
80ce:   7001moveq #1,%d0
80d0:   4e40trap #0

qemu-m68k -d in_asm ./move16


IN:
0x80b8:  lea 0x800020d4,%a0
0x80be:  lea 0x800020e4,%a1
0x80c4:  fmovel (cpid=3) %a0@-,%fpcr
0x80c8:  fmovel (cpid=3) %a0@-,%fpcr
0x80cc:  moveq #0,%d1
0x80ce:  moveq #1,%d0
0x80d0:  trap #0

This patch backports the fix from binutils to only match FPU instructions
with coprocessor ID 1.

Laurent Vivier (1):
  target/m68k: correctly disassemble move16

 disas/m68k.c | 14 ++
 1 file changed, 14 insertions(+)

-- 
2.14.4

Re: [Qemu-devel] [PATCH] qemu-img: align is_allocated_sectors to 4k

[Peter Lieven]

Am 11.06.2018 um 16:04 schrieb Max Reitz:
> On 2018-06-11 15:59, Peter Lieven wrote:
>> Am 11.06.2018 um 15:30 schrieb Max Reitz:
>>> On 2018-06-07 14:46, Peter Lieven wrote:
 We currently don't enforce that the sparse segments we detect during
 convert are
 aligned. This leads to unnecessary and costly read-modify-write
 cycles either
 internally in Qemu or in the background on the storage device as
 nearly all
 modern filesystems or hardware has a 4k alignment internally.

 As we per default set the min_sparse size to 4k it makes perfectly
 sense to ensure
 that these sparse holes in the file are placed at 4k boundaries.

 The number of RMW cycles when converting an example image [1] to a
 raw device that
 has 4k sector size is about 4600 4k read requests to perform a total
 of about 15000
 write requests. With this path the 4600 additional read requests are
 eliminated.

 [1]
 https://cloud-images.ubuntu.com/releases/16.04/release/ubuntu-16.04-server-cloudimg-amd64-disk1.vmdk


 Signed-off-by: Peter Lieven 
 ---
   qemu-img.c | 21 +++--
   1 file changed, 15 insertions(+), 6 deletions(-)
>>> I like the idea, but it doesn't seem guaranteed that
>>> is_allocated_sectors() is called on aligned offsets, so this alignment
>>> work may still leave things unaligned.
>> I can't image why this should happen. As long as the alignment devides
>> the buffer size we either
>> write or skip aligned bytes. Maybe get_block_status returns an unaligned
>> number of sectors?
> Yes, because the source medium does not need to be the same as the
> destination (so the source may have e.g. 512-byte clusters).
>
>>> Furthermore, we should probably not blindly assume 4k but instead use
>>> some block limit of the target, like pwrite_zeroes_alignment, or
>>> pdiscard_alignment, depending on the case.  (Or probably still
>>> min_sparse, if that's less.)
>>>
>>> Since is_allocated_sectors_min() (the only caller of
>>> is_allocated_sectors()) is called from just a single place, taking those
>>> factors into account should be possible.
>> I also thought of this, but for instance for raw-posix I always get a
>> request_alignment of 1.
> Yes, because request_alignment is a hard requirement.  With caching, you
> can send requests with any alignment, so it's 1.
>
> pwrite_zeroes_alignment and pdiscard_alignment are described as "Optimal
> alignment", so those should contain the values we/you want.  If they are
> 0, then you should probably fall back to opt_transfer instead of
> request_alignment.

I am still trying to figure out what is the best solution. If I take the optima 
into
account I might ending up transfering more data than necessary just to create 
an optimal
request. I just want to avoid unnecessary RMW cycles. And even if modern byte 
interfaces
advertise a request_alignment of 1 someone has to do the RMW cycle. Either the 
OS or the
harddrive itself.

I am thinking about sth like

alignment = MAX(request_alignment, opt_transfer, min_sparse)

as a starting point?

I found that opt_transfer seems to be 0 for everything I found to test.
So maybe even reduce the alignment to MAX(request_alignment, min_sparse).

Peter

[Qemu-devel] [PULL 0/9] Target MIPS queue, 2018-06-25

[Aleksandar Markovic]

From: Aleksandar Markovic 

The following changes since commit 35e238c9330669882487f9929e0aa97900431853:

  Merge remote-tracking branch 
'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
15:25:26 +0100)

are available in the git repository at:

  https://github.com/AMarkovic/qemu.git 

for you to fetch changes up to 2209731f338baaad3a1ef419fab1928e1e51ec17:

  target/mips: Fix gdbstub to read/write 64 bit FP registers (2018-06-25 
21:21:24 +0200)


Aleksandar Markovic (1):
  MAINTAINERS: update target-mips maintainers

Peter Maydell (3):
  hw/mips/boston: don't make flash region 'nomigrate'
  hw/mips/mips_malta: don't make bios region 'nomigrate'
  hw/pci-host/xilinx-pcie: don't make "io" region be RAM

Yongbok Kim (5):
  target/mips: Raise a RI when given fs is n/a from CTC1
  target/mips: Fix microMIPS on reset
  target/mips: Update gen_flt_ldst()
  target/mips: Fix data type for offset
  target/mips: Fix gdbstub to read/write 64 bit FP registers

 MAINTAINERS   |  6 +++---
 hw/mips/boston.c  |  3 +--
 hw/mips/mips_malta.c  |  2 +-
 hw/pci-host/xilinx-pcie.c |  5 ++---
 target/mips/gdbstub.c |  3 ++-
 target/mips/op_helper.c   |  3 +++
 target/mips/translate.c   | 28 
 7 files changed, 28 insertions(+), 22 deletions(-)

-- 
2.7.4

[Qemu-devel] [PULL 3/9] hw/mips/mips_malta: don't make bios region 'nomigrate'

[Aleksandar Markovic]

From: Peter Maydell 

Currently we use memory_region_init_rom_nomigrate() to create
the "bios.1fc" memory region, and we don't manually register
it with vmstate_register_ram(). This currently means that its
contents are migrated but as a ram block whose name is the empty
string; in future it may mean they are not migrated at all. Use
memory_region_init_ram() instead.

Note that this is a a cross-version migration compatibility break
for the "malta" machine.

Signed-off-by: Peter Maydell 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aleksandar Markovic 
---
 hw/mips/mips_malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 494f84e..b9d92bf 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1152,7 +1152,7 @@ void mips_malta_init(MachineState *machine)
  * handled by an overlapping region as the resulting ROM code subpage
  * regions are not executable.
  */
-memory_region_init_ram_nomigrate(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
+memory_region_init_ram(bios_copy, NULL, "bios.1fc", BIOS_SIZE,
_fatal);
 if (!rom_copy(memory_region_get_ram_ptr(bios_copy),
   FLASH_ADDRESS, BIOS_SIZE)) {
-- 
2.7.4

[Qemu-devel] [PULL 5/9] target/mips: Raise a RI when given fs is n/a from CTC1

[Aleksandar Markovic]

From: Yongbok Kim 

Fix to raise a Reserved Instruction exception when given fs is not
available from CTC1.

Signed-off-by: Yongbok Kim 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Aleksandar Markovic 
---
 target/mips/op_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 9025f42..41d3634 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -2627,6 +2627,9 @@ void helper_ctc1(CPUMIPSState *env, target_ulong arg1, 
uint32_t fs, uint32_t rt)
(env->active_fpu.fcr31 & ~(env->active_fpu.fcr31_rw_bitmask));
 break;
 default:
+if (env->insn_flags & ISA_MIPS32R6) {
+do_raise_exception(env, EXCP_RI, GETPC());
+}
 return;
 }
 restore_fp_status(env);
-- 
2.7.4

[Qemu-devel] [PULL 2/9] hw/mips/boston: don't make flash region 'nomigrate'

[Aleksandar Markovic]

From: Peter Maydell 

Currently we use memory_region_init_rom_nomigrate() to create
the "boston.flash" memory region, and we don't manually register
it with vmstate_register_ram(). This currently means that its
contents are migrated but as a ram block whose name is the empty
string; in future it may mean they are not migrated at all. Use
memory_region_init_ram() instead.

Note that this is a a cross-version migration compatibility break
for the "boston" machine.

Signed-off-by: Peter Maydell 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aleksandar Markovic 
---
 hw/mips/boston.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 52cce19..14e6f95 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -471,8 +471,7 @@ static void boston_mach_init(MachineState *machine)
 sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->cps), 0, 0, 1);
 
 flash =  g_new(MemoryRegion, 1);
-memory_region_init_rom_nomigrate(flash, NULL,
- "boston.flash", 128 * M_BYTE, );
+memory_region_init_rom(flash, NULL, "boston.flash", 128 * M_BYTE, );
 memory_region_add_subregion_overlap(sys_mem, 0x1800, flash, 0);
 
 ddr = g_new(MemoryRegion, 1);
-- 
2.7.4

[Qemu-devel] [PULL 1/9] MAINTAINERS: update target-mips maintainers

[Aleksandar Markovic]

From: Aleksandar Markovic 

Yongbok Kim transfers duties of QEMU for target MIPS maintainer to
myself as he leaves MIPS. Many thanks to Yongbok for his substantial
contributing to QEMU for MIPS over many years and taking care of its
maintainance for almost two years.

Signed-off-by: Aleksandar Markovic 
Acked-by: Yongbok Kim 
Reviewed-by: Aleksandar Markovic 
---
 MAINTAINERS | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2874ddc..0f62dca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -187,7 +187,7 @@ F: disas/microblaze.c
 
 MIPS
 M: Aurelien Jarno 
-M: Yongbok Kim 
+M: Aleksandar Markovic 
 S: Maintained
 F: target/mips/
 F: hw/mips/
@@ -706,7 +706,7 @@ S: Maintained
 F: hw/mips/mips_malta.c
 
 Mipssim
-M: Yongbok Kim 
+M: Aleksandar Markovic 
 S: Odd Fixes
 F: hw/mips/mips_mipssim.c
 F: hw/net/mipsnet.c
@@ -717,7 +717,7 @@ S: Maintained
 F: hw/mips/mips_r4k.c
 
 Fulong 2E
-M: Yongbok Kim 
+M: Aleksandar Markovic 
 S: Odd Fixes
 F: hw/mips/mips_fulong2e.c
 F: hw/isa/vt82c686.c
-- 
2.7.4

[Qemu-devel] [PULL 7/9] target/mips: Update gen_flt_ldst()

[Aleksandar Markovic]

From: Yongbok Kim 

Update gen_flt_ldst() in order to reuse the functions for nanoMIPS

Signed-off-by: Yongbok Kim 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Aleksandar Markovic 
---
 target/mips/translate.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 2eb211a..e923d27 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2433,11 +2433,8 @@ static void gen_st_cond (DisasContext *ctx, uint32_t 
opc, int rt,
 
 /* Load and store */
 static void gen_flt_ldst (DisasContext *ctx, uint32_t opc, int ft,
-  int base, int16_t offset)
+  TCGv t0)
 {
-TCGv t0 = tcg_temp_new();
-
-gen_base_offset_addr(ctx, t0, base, offset);
 /* Don't do NOP if destination is zero: we must perform the actual
memory access. */
 switch (opc) {
@@ -2480,15 +2477,15 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t 
opc, int ft,
 default:
 MIPS_INVAL("flt_ldst");
 generate_exception_end(ctx, EXCP_RI);
-goto out;
+break;
 }
- out:
-tcg_temp_free(t0);
 }
 
 static void gen_cop1_ldst(DisasContext *ctx, uint32_t op, int rt,
   int rs, int16_t imm)
 {
+TCGv t0 = tcg_temp_new();
+
 if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
 check_cp1_enabled(ctx);
 switch (op) {
@@ -2497,11 +2494,13 @@ static void gen_cop1_ldst(DisasContext *ctx, uint32_t 
op, int rt,
 check_insn(ctx, ISA_MIPS2);
 /* Fallthrough */
 default:
-gen_flt_ldst(ctx, op, rt, rs, imm);
+gen_base_offset_addr(ctx, t0, rs, imm);
+gen_flt_ldst(ctx, op, rt, t0);
 }
 } else {
 generate_exception_err(ctx, EXCP_CpU, 1);
 }
+tcg_temp_free(t0);
 }
 
 /* Arithmetic with immediate operand */
-- 
2.7.4

[Qemu-devel] [PULL 6/9] target/mips: Fix microMIPS on reset

[Aleksandar Markovic]

From: Yongbok Kim 

Fix to activate microMIPS (and nanoMIPS) on reset when Config3.ISA == {1, 3}

Signed-off-by: Yongbok Kim 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Aleksandar Markovic 
---
 target/mips/translate.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index e57d71e..2eb211a 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20713,6 +20713,11 @@ void cpu_state_reset(CPUMIPSState *env)
 env->CP0_Status |= (1 << CP0St_FR);
 }
 
+if (env->CP0_Config3 & (1 << CP0C3_ISA)) {
+/*  microMIPS on reset when Config3.ISA == {1, 3} */
+env->hflags |= MIPS_HFLAG_M16;
+}
+
 /* MSA */
 if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
 msa_reset(env);
-- 
2.7.4

[Qemu-devel] [PULL 4/9] hw/pci-host/xilinx-pcie: don't make "io" region be RAM

[Aleksandar Markovic]

From: Peter Maydell 

Currently we use memory_region_init_rom_nomigrate() to create
the "io" memory region to pass to pci_register_root_bus().
This is a dummy region, because this PCI controller doesn't
support accesses to PCI IO space.

There is no reason for the dummy region to be a RAM region;
it is only used as a place where PCI BARs can be mapped,
and if you could get a PCI card to do a bus master access
to the IO space it should not get acts-like-RAM behaviour.
Use a simple container memory region instead. (We do have
one PCI card model which can do bus master accesses to IO
space -- the LSI53C895A SCSI adaptor.)

This avoids the oddity of having a memory region which is
RAM but where the RAM is not migrated.

Note that the size of the region we use here has no
effect on behaviour.

Signed-off-by: Peter Maydell 
Reviewed-by: Alistair Francis 
Signed-off-by: Aleksandar Markovic 
---
 hw/pci-host/xilinx-pcie.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 044e312..b0a31b9 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -120,9 +120,8 @@ static void xilinx_pcie_host_realize(DeviceState *dev, 
Error **errp)
 memory_region_init(>mmio, OBJECT(s), "mmio", UINT64_MAX);
 memory_region_set_enabled(>mmio, false);
 
-/* dummy I/O region */
-memory_region_init_ram_nomigrate(>io, OBJECT(s), "io", 16, NULL);
-memory_region_set_enabled(>io, false);
+/* dummy PCI I/O region (not visible to the CPU) */
+memory_region_init(>io, OBJECT(s), "io", 16);
 
 /* interrupt out */
 qdev_init_gpio_out_named(dev, >irq, "interrupt_out", 1);
-- 
2.7.4

[Qemu-devel] [PULL 8/9] target/mips: Fix data type for offset

[Aleksandar Markovic]

From: Yongbok Kim 

Offset can be larger than 16 bit from nanoMIPS,
and immediate field can be larger than 16 bits as well.

Signed-off-by: Yongbok Kim 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Aleksandar Markovic 
---
 target/mips/translate.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index e923d27..20b43c0 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2112,7 +2112,7 @@ OP_ST_ATOMIC(scd,st64,ld64,0x7);
 #undef OP_ST_ATOMIC
 
 static void gen_base_offset_addr (DisasContext *ctx, TCGv addr,
-  int base, int16_t offset)
+  int base, int offset)
 {
 if (base == 0) {
 tcg_gen_movi_tl(addr, offset);
@@ -2140,7 +2140,7 @@ static target_ulong pc_relative_pc (DisasContext *ctx)
 
 /* Load */
 static void gen_ld(DisasContext *ctx, uint32_t opc,
-   int rt, int base, int16_t offset)
+   int rt, int base, int offset)
 {
 TCGv t0, t1, t2;
 int mem_idx = ctx->mem_idx;
@@ -2337,7 +2337,7 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
 
 /* Store */
 static void gen_st (DisasContext *ctx, uint32_t opc, int rt,
-int base, int16_t offset)
+int base, int offset)
 {
 TCGv t0 = tcg_temp_new();
 TCGv t1 = tcg_temp_new();
@@ -2505,7 +2505,7 @@ static void gen_cop1_ldst(DisasContext *ctx, uint32_t op, 
int rt,
 
 /* Arithmetic with immediate operand */
 static void gen_arith_imm(DisasContext *ctx, uint32_t opc,
-  int rt, int rs, int16_t imm)
+  int rt, int rs, int imm)
 {
 target_ulong uimm = (target_long)imm; /* Sign extend to 32/64 bits */
 
-- 
2.7.4

[Qemu-devel] [PULL 9/9] target/mips: Fix gdbstub to read/write 64 bit FP registers

[Aleksandar Markovic]

From: Yongbok Kim 

Fix gdbstub to read/write 64 bit FP registers

Signed-off-by: Yongbok Kim 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Aleksandar Markovic 
---
 target/mips/gdbstub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/mips/gdbstub.c b/target/mips/gdbstub.c
index 6d1fb70..18e0e6d 100644
--- a/target/mips/gdbstub.c
+++ b/target/mips/gdbstub.c
@@ -39,7 +39,7 @@ int mips_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 return gdb_get_regl(mem_buf, (int32_t)env->active_fpu.fcr0);
 default:
 if (env->CP0_Status & (1 << CP0St_FR)) {
-return gdb_get_regl(mem_buf,
+return gdb_get_reg64(mem_buf,
 env->active_fpu.fpr[n - 38].d);
 } else {
 return gdb_get_regl(mem_buf,
@@ -100,6 +100,7 @@ int mips_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 break;
 default:
 if (env->CP0_Status & (1 << CP0St_FR)) {
+uint64_t tmp = ldq_p(mem_buf);
 env->active_fpu.fpr[n - 38].d = tmp;
 } else {
 env->active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
-- 
2.7.4

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

[Eduardo Habkost]

On Mon, Jun 25, 2018 at 08:26:28PM +0200, Paolo Bonzini wrote:
> On 25/06/2018 19:30, Eduardo Habkost wrote:
> >>> Attentive distros could even replace the wrapper script by a link.
> >> If they are okay with replacing the "KVM only" semantics with "KVM or
> >> TCG", which I think is generally worse.
> >
> > If we can't get agreement on what's the right default for each
> > QEMU binary, I think that's yet another reason to document that
> > upstream QEMU won't guarantee ABI compatibility if -accel is
> > omitted.
> 
> Before that we should ask what the benefit is in changing the default
> for qemu-system-*.  Nobody is using it in practice to start QEMU with
> KVM enabled...

How can you be sure?  If qemu-system-* is installed with KVM
compiled in, libvirt will probe it using
"-machine none,accel=kvm:tcg" and run the VM using
"-machine $MACHINE,accel=kvm".

In either case, I'm not arguing (yet) for changing the default
upstream.  I'm just arguing for upstream QEMU to not make any
promises about the default.

-- 
Eduardo

Re: [Qemu-devel] Choosing PCR banks for swtpm's TPM 2

[Stefan Berger]

On 06/25/2018 11:05 AM, Stefan Berger wrote:

Hi!

 I am sending this email to solicit input on the choice of the PCR 
banks to enable for swtpm's TPM 2. I have currently enabled 4 PCR 
banks for SHA{1,256,384,512}. The downside of this is that running the 
TPM 2 with so many PCR banks has a performance impact when the Linux 
integrity measurement architecture is used and has to extend 
measurements into all PCR banks, which Linux does already.


TPM 2 has the PCR_Allocate() command for a user to select the PCR 
banks to use. This command allows to make some PCR banks invisible. 
The change has to be done through the firmware and has the downside 
that the TPM2 does not support TPM2_Shutdown(SU_STATE) after this 
command was used. This prevents suspend/resume from working properly. 
So, it seems that one shouldn't have to use this command, which in 
turn means the number of PCR banks should be small.


Actually that was my interpretation of the specs and from what it looks 
like I was wrong assuming that once PCR_Allocate() was used that 
TPM2_Shutdown(SU_STATE) cannot be used anymore at all. The text is a bit 
ambiguous about it. This command can be sent, but the machine needs to 
be rebooted and with that the TPM 2 reset.


The next issue is that the IBM TSS2 is hard coded for 3 PCR banks and a 
few commands are breaking because of that. Now the solution would be to:


- compile-time disable the SHA512 bank; this will break existing state 
but for as long as it's in preview, I hope this is ok; we cannot easily 
enable SHA512 then in the future.


- swtpm_setup gets a --pcr-banks  option that 
PCR_Allocate()'s the active PCR banks for the swtpm. The default will be 
SHA 1 and SHA 256, which disables the SHA 384 PCR bank; Users can choose 
their banks if they run this command directly. SHA1 and SHA256 seems to 
be a reasonable set of active PCR banks for now.


   Stefan





Another complication with the swtpm is the upgrade path. Suspended VMs 
will expect that the PCR banks that were available before the suspend 
will be available after the resume and a possible swtpm upgrade. This 
in turn means that the PCR banks should be chosen now and we'll have 
to stick with them.


That said, my suggestion would be to enable only PCR banks for SHA256 
for 'now' and SHA512 for the future. Having two PCR banks should 
enable decent performance. If someone wants to have better performance 
he will have to go through the firmware to select the PCR banks at the 
expense of loosing suspend/resume support.


The change of PCR banks for the current 4 PCR banks will break the 
state of all swtpms.


If you have suggestions, please let me know.

Regards,

   Stefan

Re: [Qemu-devel] [PATCH] s390/boot: block uncompressed vmlinux booting attempts

[Guenter Roeck]

On Mon, Jun 25, 2018 at 05:09:19PM +0200, Vasily Gorbik wrote:
> Since uncompressed kernel image "vmlinux" elf file is not bootable under
> qemu anymore, add a check which would report that.
> 
> Qemu users are encouraged to use bzImage or
> arch/s390/boot/compressed/vmlinux instead.
> 
> The check relies on s390 linux entry point ABI definition, which is only
> present in bzImage and arch/s390/boot/compressed/vmlinux.
> 
> Signed-off-by: Vasily Gorbik 

With qemu:

Tested-by: Guenter Roeck 

> ---
>  arch/s390/boot/head.S |  4 ++--
>  arch/s390/include/asm/setup.h |  3 ++-
>  arch/s390/kernel/early.c  | 12 
>  3 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/boot/head.S b/arch/s390/boot/head.S
> index f09e792df495..f721913b73f1 100644
> --- a/arch/s390/boot/head.S
> +++ b/arch/s390/boot/head.S
> @@ -272,14 +272,14 @@ iplstart:
>   .org0x1
>  ENTRY(startup)
>   j   .Lep_startup_normal
> - .org0x10008
> + .orgEP_OFFSET
>  #
>  # This is a list of s390 kernel entry points. At address 0x1000f the number 
> of
>  # valid entry points is stored.
>  #
>  # IMPORTANT: Do not change this table, it is s390 kernel ABI!
>  #
> - .ascii  "S390EP"
> + .ascii  EP_STRING
>   .byte   0x00,0x01
>  #
>  # kdump startup-code at 0x10010, running in 64 bit absolute addressing mode
> diff --git a/arch/s390/include/asm/setup.h b/arch/s390/include/asm/setup.h
> index be02f0558048..1d66016f4170 100644
> --- a/arch/s390/include/asm/setup.h
> +++ b/arch/s390/include/asm/setup.h
> @@ -9,7 +9,8 @@
>  #include 
>  #include 
>  
> -
> +#define EP_OFFSET0x10008
> +#define EP_STRING"S390EP"
>  #define PARMAREA 0x10400
>  #define PARMAREA_END 0x11000
>  
> diff --git a/arch/s390/kernel/early.c b/arch/s390/kernel/early.c
> index 827699eb48fa..45c5be3d8777 100644
> --- a/arch/s390/kernel/early.c
> +++ b/arch/s390/kernel/early.c
> @@ -331,8 +331,20 @@ static void __init setup_boot_command_line(void)
>   append_to_cmdline(append_ipl_scpdata);
>  }
>  
> +static void __init check_image_bootable(void)
> +{
> + if (!memcmp(EP_STRING, (void *)EP_OFFSET, strlen(EP_STRING)))
> + return;
> +
> + sclp_early_printk("The linux kernel boot failure: the image is 
> corrupted or not bootable.\n");
> + sclp_early_printk("Please check that you are using bootable kernel 
> image \"bzImage\".\n");
> + sclp_early_printk("(or alternatively 
> \"arch/s390/boot/compressed/vmlinux\" image for qemu)\n");
> + disabled_wait(0xbadb007);
> +}
> +
>  void __init startup_init(void)
>  {
> + check_image_bootable();
>   time_early_init();
>   init_kernel_storage_key();
>   lockdep_off();
> -- 
> ⣔⢻⣟⢢ 2.18.0.rc2.13.g4da9a5d
> ⣿⢿⡿⣿ pacman edition
> 

Re: [Qemu-devel] [PATCH] Deprecate the -enable-hax option

[Stefan Weil]

Am 25.06.2018 um 20:28 schrieb Paolo Bonzini:
> On 25/06/2018 20:22, Thomas Huth wrote:
>> We currently have got three ways of turning on the HAX accelerator:
>> "-machine accel=hax", "-accel hax" and "-enable-hax". That's really
>> confusing and overloaded. Since "-accel" is our preferred way to enable
>> an accelerator nowadays, and "-accel hax" is even less to type than
>> "-enable-hax", let's deprecate the "-enable-hax" option now.
>>
>> Note: While "-enable-kvm" is available since a long time and can hardly be
>> removed since it is used in a lot of upper layer tools and scripts, the
>> "-enable-hax" option is still rather new and not very widespread yet, so
>> I think that it should be OK if we remove this in a couple of releases again
>> (we'll see whether someone complains after seeing the deprecation message -
>> then we could still reconsider to keep it if there a well-founded reasons).
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  Let's give this a try at least :-)
> 
> Sounds good, queued.
> 
> Paolo
> 

Reviewed-by: Stefan Weil 

I would not mind if --enable-hax were removed soon, as I don't think it
was widely used.

Stefan

[Qemu-devel] [Bug 1331859] Re: QEMU kernel panic on Windows with arithmetic syntax error

[Thomas Huth]

Looking through old bug tickets... can you still reproduce this issue
with the latest version of QEMU? Or could we close this ticket nowadays?


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

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

Title:
  QEMU kernel panic on Windows with arithmetic syntax error

Status in QEMU:
  Incomplete

Bug description:
  During attempts to bring-up QEMU 64-bit ARM support I discovered a
  kernel panics that only occur on Windows but work properly on Linux.

  The issue can be reproduced by running the following command line:

  $ ./arm-softmmu/qemu-system-arm -M versatilepb -kernel
  $IMAGES/vmlinuz-3.2.0-4-versatile -initrd
  $IMAGES/initrd.img-3.2.0-4-versatile -hda
  $IMAGES/debian_wheezy_armel_standard.qcow2 -append "root=/dev/sda1"

  where $IMAGES is the location where the images are downloaded from
  http://people.debian.org/~aurel32/qemu/armel/.

  This was reproduced with both a custom built QEMU as well as the QEMU
  image installed by
  http://qemu.weilnetz.de/w32/qemu_w32-setup-20140617.exe.

  The same command line runs properly on Linux using a custom built
  QEMU.

  The Windows versions of QEMU do appear to work properly using the arm-
  test images available on qemu.org.

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

Re: [Qemu-devel] [PATCH v2] Show values and description when using "qom-list"

[Markus Armbruster]

"Dr. David Alan Gilbert"  writes:

> * Markus Armbruster (arm...@redhat.com) wrote:
>> Markus Armbruster  writes:
>> 
>> > Andreas Färber  writes:
>> >
>> >> Am 08.06.2018 um 11:41 schrieb Dr. David Alan Gilbert:
>> >>> * Andreas Färber (afaer...@suse.de) wrote:
>>  Am 01.06.2018 um 17:39 schrieb Ricardo Perez Blanco:
>> > For debugging purposes it is very useful to:
>> >  - See the description of the field. This information is already filled
>> >in but not shown in "qom-list" command.
>> 
>>  No objection on this part.
>> 
>> >  - Display value of the field.
>> 
>>  That is by definition the qom-get operation, not qom-list. Just like the
>>  ls command does not show file contents, there's cat etc. for that. For
>>  debugging purposes we had a qom-tree (?) command that would combine
>>  both.
>> >>>
>> >>> I'm not too bothered about distinguishing between the two commands;
>> >>> but it would be nice
>> >
>> > When an HMP and QMP both have a command with the same name, they should
>> > do the same.
>> >
>> > HMP may add convenience features that aren't wanted in QMP, but I feel
>> > extending an operation to list objects to also show their contents goes
>> > beyond that.  If we want an HMP command that does both, it should be
>> > named differently.  Perhaps that might even be more appropriate for HMP
>> > than low-level commands qom-list and qom-get, but I leave that to the
>> > HMP maintainer to decide.
>> >
>> >>>  - one reason I'm not too bothered is because we've
>> >>> failed to get a qom-get in multiple years of trying.
>> >
>> > We clearly haven't tried hard enough.
>> >
>> > If we can figure out how to show values in qom-list, surely we can
>> > figure out how to show them in qom-get.
>> >
>>    There might be unmerged patches on qemu-devel related to display
>>  of certain data types.
>> >>> 
>> >>> Which ones?
>> >>
>> >> My original qom-info series needed StringOutputVisitor changes for enums
>> >> (test case: rtc) that did not get accepted immediately and thus some
>> >> part of HMP qom-info/qom-get got stuck due to risking assertions for
>> >> qom-info / otherwise; QMP was not affected IIRC.
>> >
>> > Here's the last try I can find:
>> > [PATCH v2] qom: Implement qom-get HMP command
>> > Message-Id: <1473157086-12062-1-git-send-email-dgilb...@redhat.com>
>> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01041.html
>> 
>> Stalled on output format and consistency with qom-set.  I wrote back
>> then "We can take qom-get as is and improve its output later."  I'd like
>> to encourage you to dust it off.  Perfect's the enemy of good.
>> 
>> Wanted improvements include:
>> 
>> * Prettier output format.  I'd suggest creating a keyval variant of the
>>   output visitor.
>
> I'm not too bothered about pretty at first, as long as we don't stop
> anyway of making it pretty later; especially if non-compound types work
> well.

We've waited so long for "someone" to post a solution that satisfies all
wants.  We should take a solution that is useful and can grow.

>> * Make qom-set input format consistent by switching to the matching
>>   input visitor.
>> 
>> > Its v1 tries a different approach:
>> > [PATCH 0/2] qom-get [for 2.8]
>> > Message-Id: <1472117833-10236-1-git-send-email-dgilb...@redhat.com>
>> > Unfortunately the mailing list archive doesn't show the full thread, so
>> > you get to follow three links:
>> > https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg03815.html
>> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04261.html
>> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04267.html
>> 
>> This one stalled on string visitor limitations.  You didn't feel like
>> addressing them just to get qom-get working.  Understandable.
>
> Have there been any changes in the last 2 years that have helped?

There's been progress, but there are gaps left.

Back then, we had the choice of string input/output visitors, and the
options visitor, all (severely) restricted to certain kinds of data.

Now we have the qobject keyval input visitor, which is almost general.
There is no matching output visitor, yet, simply because we haven't had
the need.

Re: [Qemu-devel] [PATCH] Deprecate the -enable-hax option

[Paolo Bonzini]

On 25/06/2018 20:22, Thomas Huth wrote:
> We currently have got three ways of turning on the HAX accelerator:
> "-machine accel=hax", "-accel hax" and "-enable-hax". That's really
> confusing and overloaded. Since "-accel" is our preferred way to enable
> an accelerator nowadays, and "-accel hax" is even less to type than
> "-enable-hax", let's deprecate the "-enable-hax" option now.
> 
> Note: While "-enable-kvm" is available since a long time and can hardly be
> removed since it is used in a lot of upper layer tools and scripts, the
> "-enable-hax" option is still rather new and not very widespread yet, so
> I think that it should be OK if we remove this in a couple of releases again
> (we'll see whether someone complains after seeing the deprecation message -
> then we could still reconsider to keep it if there a well-founded reasons).
> 
> Signed-off-by: Thomas Huth 
> ---
>  Let's give this a try at least :-)

Sounds good, queued.

Paolo

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

[Paolo Bonzini]

On 25/06/2018 19:30, Eduardo Habkost wrote:
>>> Attentive distros could even replace the wrapper script by a link.
>> If they are okay with replacing the "KVM only" semantics with "KVM or
>> TCG", which I think is generally worse.
>
> If we can't get agreement on what's the right default for each
> QEMU binary, I think that's yet another reason to document that
> upstream QEMU won't guarantee ABI compatibility if -accel is
> omitted.

Before that we should ask what the benefit is in changing the default
for qemu-system-*.  Nobody is using it in practice to start QEMU with
KVM enabled...

Paolo

> If downstream distributions want to keep promising ABI
> compatibility, it will be up to them.

[Qemu-devel] [PATCH] Deprecate the -enable-hax option

[Thomas Huth]

We currently have got three ways of turning on the HAX accelerator:
"-machine accel=hax", "-accel hax" and "-enable-hax". That's really
confusing and overloaded. Since "-accel" is our preferred way to enable
an accelerator nowadays, and "-accel hax" is even less to type than
"-enable-hax", let's deprecate the "-enable-hax" option now.

Note: While "-enable-kvm" is available since a long time and can hardly be
removed since it is used in a lot of upper layer tools and scripts, the
"-enable-hax" option is still rather new and not very widespread yet, so
I think that it should be OK if we remove this in a couple of releases again
(we'll see whether someone complains after seeing the deprecation message -
then we could still reconsider to keep it if there a well-founded reasons).

Signed-off-by: Thomas Huth 
---
 Let's give this a try at least :-)

 qemu-doc.texi   | 4 
 qemu-options.hx | 2 +-
 vl.c| 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 16fcb47..ac5ca34 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2912,6 +2912,10 @@ Option @option{-virtioconsole} has been replaced by
 The @code{-clock} option is ignored since QEMU version 1.7.0. There is no
 replacement since it is not needed anymore.
 
+@subsection -enable-hax (since 3.0.0)
+
+The @option{-enable-hax} option has been replaced by @option{-accel hax}.
+
 @section QEMU Machine Protocol (QMP) commands
 
 @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/qemu-options.hx b/qemu-options.hx
index d5b0c26..d811072 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3421,7 +3421,7 @@ STEXI
 Enable HAX (Hardware-based Acceleration eXecution) support. This option
 is only available if HAX support is enabled when compiling. HAX is only
 applicable to MAC and Windows platform, and thus does not conflict with
-KVM.
+KVM. This option is deprecated, use @option{-accel hax} instead.
 ETEXI
 
 DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
diff --git a/vl.c b/vl.c
index 5d8a9df..c13d589 100644
--- a/vl.c
+++ b/vl.c
@@ -3581,6 +3581,7 @@ int main(int argc, char **argv, char **envp)
 qemu_opts_parse_noisily(olist, "accel=kvm", false);
 break;
 case QEMU_OPTION_enable_hax:
+warn_report("Option is deprecated, use '-accel hax' instead");
 olist = qemu_find_opts("machine");
 qemu_opts_parse_noisily(olist, "accel=hax", false);
 break;
-- 
1.8.3.1

[Qemu-devel] map host MMIO address to guest

[Huaicheng Li]

Hi all,

I'm trying to map a host MMIO region (host PCIe device BAR) into guest
physical address space. The goal is to enable direct control over that host
MMIO region from guest OS by accessing a certain GPA.

I know the address of the host MMIO region (one page). First I map the page
into QEMU process address space and get a QEMU buffer. Then I use
"memory_region_init_ram_ptr();
memory_region_add_subregion_overlap(system_memory, 512MB, my_mr, 1)" to map
the QEMU buffer as part of guest physical address space (starting from
512MB to 512MB+4K).

When I read/write to QEMU buffer, I can observe that correct MMIO region
access is triggered. However, when I try to access the mapped MMIO region
from guest OS (using a guest kernel module to access gpa:512MB directly),
the following host kernel panic will be triggered.

I don't understand why this happens. When I use the same method and map a
host memory page (instead of a host MMIO page) into guest, it works fine. I
appreciate if anyone can help analyze this? Thanks in advance.

Best,
Huaicheng

[  323.844213] BUG: unable to handle kernel paging request at
ea0003faf460
[  323.845671] IP: gup_pgd_range+0x2f5/0x860
[  323.846615] PGD 23f7ed067 P4D 23f7ed067 PUD 23f7ec067 PMD 0
[  323.847848] Oops:  [#1] SMP
[  323.848692] Modules linked in: wpt(O) kvm_intel kvm irqbypass
[  323.850085] CPU: 2 PID: 4994 Comm: qemu-system-x86 Tainted: G
 O 4.15.0-rc4+ #10
[  323.853002] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
[  323.855029] RIP: 0010:gup_pgd_range+0x2f5/0x860
[  323.855792] RSP: 0018:c90004fdbae0 EFLAGS: 00010002
[  323.856648] RAX: 03faf440 RBX: 555b8c6db000 RCX:
3000
[  323.857618] RDX: ea0003faf440 RSI: 88022d4c36d0 RDI:
8000febd1067
[  323.858598] RBP: c90004fdbb7c R08: 0400 R09:
ea00
[  323.859428] R10: 3000 R11: 8000febd1067 R12:
00022d4c3067
[  323.860216] R13: c90004fdbba8 R14: 555b8c6da000 R15:
0007
[  323.860979] FS:  7f4a3a527700() GS:88023fc8()
knlGS:
[  323.861899] CS:  0010 DS:  ES:  CR0: 80050033
[  323.862534] CR2: ea0003faf460 CR3: 000232723001 CR4:
000626e0
[  323.863312] Call Trace:
[  323.863667]  __get_user_pages_fast+0x6b/0x90
[  323.864234]  __gfn_to_pfn_memslot+0xf5/0x3b0 [kvm]
[  323.864858]  ? kvm_irq_delivery_to_apic+0x51/0x2a0 [kvm]
[  323.865502]  try_async_pf+0x53/0x1f0 [kvm]
[  323.866039]  tdp_page_fault+0x112/0x280 [kvm]
[  323.866609]  kvm_mmu_page_fault+0x53/0x130 [kvm]
[  323.867201]  vmx_handle_exit+0x9b/0x1510 [kvm_intel]
[  323.867823]  ? atomic_switch_perf_msrs+0x5f/0x80 [kvm_intel]
[  323.868504]  ? vmx_vcpu_run+0x30a/0x4b0 [kvm_intel]
[  323.869101]  kvm_arch_vcpu_ioctl_run+0xa79/0x1570 [kvm]
[  323.869748]  ? kvm_vcpu_ioctl+0x2eb/0x570 [kvm]
[  323.870332]  kvm_vcpu_ioctl+0x2eb/0x570 [kvm]
[  323.870897]  ? kvm_vm_ioctl+0x142/0x7e0 [kvm]
[  323.871457]  do_vfs_ioctl+0x8f/0x5b0
[  323.871955]  ? native_write_msr+0x6/0x20
[  323.872476]  ? security_file_ioctl+0x3e/0x60
[  323.873024]  SyS_ioctl+0x74/0x80
[  323.873468]  entry_SYSCALL_64_fastpath+0x1a/0x7d
[  323.874043] RIP: 0033:0x7f4a3c897f47
[  323.874506] RSP: 002b:7f4a3a526a78 EFLAGS: 0246 ORIG_RAX:
0010
[  323.875439] RAX: ffda RBX: ae80 RCX:
7f4a3c897f47
[  323.876234] RDX:  RSI: ae80 RDI:
000f
[  323.877093] RBP: 555b8c5ef450 R08: 555b89bc1a70 R09:

[  323.877931] R10: fee0 R11: 0246 R12:

[  323.878760] R13: 7f4a3e2db000 R14: 0006 R15:
555b8c5ef450
[  323.879555] Code: 00 00 d3 e2 85 c2 75 ae 4c 85 c7 0f 85 d0 00 00 00 f7
c7 00 02 00 00 75 9d 48 89 f8 66 66 66 90 4c 21 d0 48 c1 e8 06 4a 8d 14 08
<48> 8b 42 20 4c 8d 58 ff a8 01 4c 0f 44
da 41 8b 43 1c 85 c0 0f
[  323.881713] RIP: gup_pgd_range+0x2f5/0x860 RSP: c90004fdbae0
[  323.882509] CR2: ea0003faf460
[  323.883069] ---[ end trace 2427ffda7b3b2a32 ]---

Re: [Qemu-devel] [PATCH] migration: invalidate cache before source start

[John Snow]

On 06/25/2018 01:50 PM, Dr. David Alan Gilbert wrote:
> * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
>> * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
>>> 15.06.2018 15:06, Dr. David Alan Gilbert wrote:
 * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> Invalidate cache before source start in case of failed migration.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
 Why doesn't the code at the bottom of migration_completion,
 fail_invalidate:   and the code in migrate_fd_cancel   handle this?

 What case did you see it in that those didn't handle?
 (Also I guess it probably should set s->block_inactive = false)
>>>
>>> on source I see:
>>>
>>> 81392@1529065750.766289:migrate_set_state new state 7
>>> 81392@1529065750.766330:migration_thread_file_err
>>> 81392@1529065750.766332:migration_thread_after_loop
>>>
>>> so, we are leaving loop on
>>>     if (qemu_file_get_error(s->to_dst_file)) {
>>>     migrate_set_state(>state, current_active_state,
>>> MIGRATION_STATUS_FAILED);
>>> trace_migration_thread_file_err();
>>> break;
>>>     }
>>>
>>> and skip migration_completion()
>>
>> Yeh, OK; I'd seen soemthing else a few days ago, where a cancellation
>> test that had previously ended with a 'cancelled' state has now ended up
>> in 'failed' (which is the state 7 you have above).
>> I suspect there's something else going on as well; I think what is
>> supposed to happen in the case of 'cancel' is that it spins in 'cancelling' 
>> for
>> a while in migrate_fd_cancel and then at the bottom of migrate_fd_cancel
>> it does the recovery, but because it's going to failed instead, then
>> it's jumping over that recovery.
> 
> Going back and actually looking at the patch again;
> can I ask for 1 small change;
>Can you set s->block_inactive = false   in the case where you
> don't get the local_err (Like we do at the bottom of migrate_fd_cancel)
> 
> 
> Does that make sense?
> 
> Thanks,
> 
> Dave
> 

Vladimir, one more question for you because I'm not as familiar with
this code:

In the normal case we need to invalidate the qcow2 cache as a way to
re-engage the disk (yes?) when we have failed during the late-migration
steps.

In this case, we seem to be observing a failure during the bulk transfer
loop. Why is it important to invalidate the cache at this step -- would
the disk have been inactivated yet? It shouldn't, because it's in the
bulk transfer phase -- or am I missing something?

I feel like this code is behaving in a way that's fairly surprising for
a casual reader so I was hoping you could elaborate for me.

--js

Re: [Qemu-devel] [virtio-dev] Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net

[Samudrala, Sridhar]

On 6/22/2018 5:17 PM, Siwei Liu wrote:

On Fri, Jun 22, 2018 at 4:40 PM, Siwei Liu  wrote:

On Fri, Jun 22, 2018 at 3:25 PM, Michael S. Tsirkin  wrote:

On Fri, Jun 22, 2018 at 02:51:11PM -0700, Siwei Liu wrote:

On Fri, Jun 22, 2018 at 2:29 PM, Michael S. Tsirkin  wrote:

On Fri, Jun 22, 2018 at 01:03:04PM -0700, Siwei Liu wrote:

On Fri, Jun 22, 2018 at 1:00 PM, Siwei Liu  wrote:

On Thu, Jun 21, 2018 at 7:32 PM, Michael S. Tsirkin  wrote:

On Thu, Jun 21, 2018 at 06:21:55PM -0700, Siwei Liu wrote:

On Thu, Jun 21, 2018 at 7:59 AM, Cornelia Huck  wrote:

On Wed, 20 Jun 2018 22:48:58 +0300
"Michael S. Tsirkin"  wrote:


On Wed, Jun 20, 2018 at 06:06:19PM +0200, Cornelia Huck wrote:

In any case, I'm not sure anymore why we'd want the extra uuid.

It's mostly so we can have e.g. multiple devices with same MAC
(which some people seem to want in order to then use
then with different containers).

But it is also handy for when you assign a PF, since then you
can't set the MAC.


OK, so what about the following:

- introduce a new feature bit, VIRTIO_NET_F_STANDBY_UUID that indicates
   that we have a new uuid field in the virtio-net config space
- in QEMU, add a property for virtio-net that allows to specify a uuid,
   offer VIRTIO_NET_F_STANDBY_UUID if set
- when configuring, set the property to the group UUID of the vfio-pci
   device

If feature negotiation fails on VIRTIO_NET_F_STANDBY_UUID, is it safe
to still expose UUID in the config space on virtio-pci?


Yes but guest is not supposed to read it.


I'm not even sure if it's sane to expose group UUID on the PCI bridge
where the corresponding vfio-pci device attached to for a guest which
doesn't support the feature (legacy).

-Siwei

Yes but you won't add the primary behind such a bridge.

I assume the UUID feature is a new one besides the exiting
VIRTIO_NET_F_STANDBY feature, where I think the current proposal is,
if UUID feature is present and supported by guest, we'll do pairing
based on UUID; if UUID feature present

  ^^^  is NOT present

Let's go back for a bit.

What is wrong with matching by mac?

1. Does not allow multiple NICs with same mac
2. Requires MAC to be programmed by host in the PT device
(which is often possible with VFs but isn't possible with all PT
devices)

Might not neccessarily be something wrong, but it's very limited to
prohibit the MAC of VF from changing when enslaved by failover.

You mean guest changing MAC? I'm not sure why we prohibit that.

I think Sridhar and Jiri might be better person to answer it. My
impression was that sync'ing the MAC address change between all 3
devices is challenging, as the failover driver uses MAC address to
match net_device internally.


Yes. The MAC address is assigned by the hypervisor and it needs to manage the 
movement
of the MAC between the PF and VF.  Allowing the guest to change the MAC will 
require
synchronization between the hypervisor and the PF/VF drivers. Most of the VF 
drivers
don't allow changing guest MAC unless it is a trusted VF.

Re: [Qemu-devel] [PATCH] migration: invalidate cache before source start

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> > 15.06.2018 15:06, Dr. David Alan Gilbert wrote:
> > > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> > > > Invalidate cache before source start in case of failed migration.
> > > > 
> > > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > > Why doesn't the code at the bottom of migration_completion,
> > > fail_invalidate:   and the code in migrate_fd_cancel   handle this?
> > > 
> > > What case did you see it in that those didn't handle?
> > > (Also I guess it probably should set s->block_inactive = false)
> > 
> > on source I see:
> > 
> > 81392@1529065750.766289:migrate_set_state new state 7
> > 81392@1529065750.766330:migration_thread_file_err
> > 81392@1529065750.766332:migration_thread_after_loop
> > 
> > so, we are leaving loop on
> >     if (qemu_file_get_error(s->to_dst_file)) {
> >     migrate_set_state(>state, current_active_state,
> > MIGRATION_STATUS_FAILED);
> > trace_migration_thread_file_err();
> > break;
> >     }
> > 
> > and skip migration_completion()
> 
> Yeh, OK; I'd seen soemthing else a few days ago, where a cancellation
> test that had previously ended with a 'cancelled' state has now ended up
> in 'failed' (which is the state 7 you have above).
> I suspect there's something else going on as well; I think what is
> supposed to happen in the case of 'cancel' is that it spins in 'cancelling' 
> for
> a while in migrate_fd_cancel and then at the bottom of migrate_fd_cancel
> it does the recovery, but because it's going to failed instead, then
> it's jumping over that recovery.

Going back and actually looking at the patch again;
can I ask for 1 small change;
   Can you set s->block_inactive = false   in the case where you
don't get the local_err (Like we do at the bottom of migrate_fd_cancel)


Does that make sense?

Thanks,

Dave

> Dave
> 
> > 
> > > 
> > > Dave
> > > 
> > > > ---
> > > > 
> > > >   migration/migration.c | 9 -
> > > >   1 file changed, 8 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index 1e99ec9b7e..8f39e0dc02 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -2806,7 +2806,14 @@ static void 
> > > > migration_iteration_finish(MigrationState *s)
> > > >   case MIGRATION_STATUS_FAILED:
> > > >   case MIGRATION_STATUS_CANCELLED:
> > > >   if (s->vm_was_running) {
> > > > -vm_start();
> > > > +Error *local_err = NULL;
> > > > +bdrv_invalidate_cache_all(_err);
> > > > +if (local_err) {
> > > > +error_reportf_err(local_err, "Can't invalidate disks 
> > > > before "
> > > > +  "source vm start");
> > > > +} else {
> > > > +vm_start();
> > > > +}
> > > >   } else {
> > > >   if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
> > > >   runstate_set(RUN_STATE_POSTMIGRATE);
> > > > -- 
> > > > 2.11.1
> > > > 
> > > --
> > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> > 
> > 
> > -- 
> > Best regards,
> > Vladimir
> > 
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v5 18/35] target/arm: Implement SVE Floating Point Multiply Indexed Group

[Peter Maydell]

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper.h| 14 +++
>  target/arm/translate-sve.c | 50 ++
>  target/arm/vec_helper.c| 48 
>  target/arm/sve.decode  | 19 +++
>  4 files changed, 131 insertions(+)

> +static bool trans_FMLA_zzxz(DisasContext *s, arg_FMLA_zzxz *a, uint32_t insn)
> +{
> +static gen_helper_gvec_4_ptr * const fns[3] = {
> +gen_helper_gvec_fmla_idx_h,
> +gen_helper_gvec_fmla_idx_s,
> +gen_helper_gvec_fmla_idx_d,
> +};
> +
> +if (sve_access_check(s)) {
> +unsigned vsz = vec_full_reg_size(s);
> +TCGv_ptr status = get_fpstatus_ptr(a->esz == MO_16);
> +tcg_gen_gvec_4_ptr(vec_full_reg_offset(s, a->rd),
> +   vec_full_reg_offset(s, a->rn),
> +   vec_full_reg_offset(s, a->rm),
> +   vec_full_reg_offset(s, a->ra),
> +   status, vsz, vsz, a->index * 2 + a->sub,

Why are we multiplying the index by 2 here? Are we just encoding
(index, sub) into a constant to pull out again the other side?
If so, comment might help. (Also I find shifts and ors make it
clearer that we're just shifting values around; multiplication
and addition to me implies that we're really doing arithmetic.)

> +   fns[a->esz - 1]);
> +tcg_temp_free_ptr(status);
> +}
> +return true;
> +}

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [Bug 1778350] Re: Android-x86 4.4-r5 won't boot on QEMU since v2.11.0-rc2

[John Snow]

CC Michael Tsirkin

On 06/25/2018 06:53 AM, navicrej wrote:
> After 'git bisect' -ing it I found the commit that is responsible for
> this:
> 
> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=9fa99d2519cbf71f871e46871df12cb446dc1c3e
> 

[Qemu-devel] [PATCH v5] ssh: switch from libssh2 to libssh

[Pino Toscano]

Rewrite the implementation of the ssh block driver to use libssh instead
of libssh2.  The libssh library has various advantages over libssh2:
- easier API for authentication (for example for using ssh-agent)
- easier API for known_hosts handling
- supports newer types of keys in known_hosts

Kerberos authentication can be enabled once the libssh bug for it [1] is
fixed.

The development version of libssh (i.e. the future 0.8.x) supports
fsync, so reuse the build time check for this.

[1] https://red.libssh.org/issues/242

Signed-off-by: Pino Toscano 
---

Changes from v4:
- fix wrong usages of error_setg/session_error_setg/sftp_error_setg
- fix few return code checks
- remove now-unused parameters in few internal functions
- allow authentication with "none" method
- switch to unsigned int for the port number
- enable TCP_NODELAY on the socket
- fix one reference error message in iotest 207

Changes from v3:
- fix socket cleanup in connect_to_ssh()
- add comments about the socket cleanup
- improve the error reporting (closer to what was with libssh2)
- improve EOF detection on sftp_read()

Changes from v2:
- used again an own fd
- fixed co_yield() implementation

Changes from v1:
- fixed jumbo packets writing
- fixed missing 'err' assignment
- fixed commit message

 block/Makefile.objs|   6 +-
 block/ssh.c| 566 ++---
 configure  |  65 +++--
 tests/qemu-iotests/207.out |   2 +-
 4 files changed, 307 insertions(+), 332 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 899bfb5e2c..9c3b3bfb99 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -21,7 +21,7 @@ block-obj-$(CONFIG_CURL) += curl.o
 block-obj-$(CONFIG_RBD) += rbd.o
 block-obj-$(CONFIG_GLUSTERFS) += gluster.o
 block-obj-$(CONFIG_VXHS) += vxhs.o
-block-obj-$(CONFIG_LIBSSH2) += ssh.o
+block-obj-$(CONFIG_LIBSSH) += ssh.o
 block-obj-y += accounting.o dirty-bitmap.o
 block-obj-y += write-threshold.o
 block-obj-y += backup.o
@@ -42,8 +42,8 @@ rbd.o-libs := $(RBD_LIBS)
 gluster.o-cflags   := $(GLUSTERFS_CFLAGS)
 gluster.o-libs := $(GLUSTERFS_LIBS)
 vxhs.o-libs:= $(VXHS_LIBS)
-ssh.o-cflags   := $(LIBSSH2_CFLAGS)
-ssh.o-libs := $(LIBSSH2_LIBS)
+ssh.o-cflags   := $(LIBSSH_CFLAGS)
+ssh.o-libs := $(LIBSSH_LIBS)
 block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
 dmg-bz2.o-libs := $(BZIP2_LIBS)
 qcow.o-libs:= -lz
diff --git a/block/ssh.c b/block/ssh.c
index da7bbf73e2..787245230a 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -24,8 +24,8 @@
 
 #include "qemu/osdep.h"
 
-#include 
-#include 
+#include 
+#include 
 
 #include "block/block_int.h"
 #include "block/qdict.h"
@@ -45,14 +45,12 @@
 /* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in
  * this block driver code.
  *
- * TRACE_LIBSSH2= enables tracing in libssh2 itself.  Note
- * that this requires that libssh2 was specially compiled with the
- * `./configure --enable-debug' option, so most likely you will have
- * to compile it yourself.  The meaning of  is described
- * here: http://www.libssh2.org/libssh2_trace.html
+ * TRACE_LIBSSH= enables tracing in libssh itself.
+ * The meaning of  is described here:
+ * http://api.libssh.org/master/group__libssh__log.html
  */
 #define DEBUG_SSH 0
-#define TRACE_LIBSSH2 0 /* or try: LIBSSH2_TRACE_SFTP */
+#define TRACE_LIBSSH  0 /* see: SSH_LOG_* */
 
 #define DPRINTF(fmt, ...)   \
 do {\
@@ -68,18 +66,14 @@ typedef struct BDRVSSHState {
 
 /* SSH connection. */
 int sock; /* socket */
-LIBSSH2_SESSION *session; /* ssh session */
-LIBSSH2_SFTP *sftp;   /* sftp session */
-LIBSSH2_SFTP_HANDLE *sftp_handle; /* sftp remote file handle */
+ssh_session session;  /* ssh session */
+sftp_session sftp;/* sftp session */
+sftp_file sftp_handle;/* sftp remote file handle */
 
-/* See ssh_seek() function below. */
-int64_t offset;
-bool offset_op_read;
-
-/* File attributes at open.  We try to keep the .filesize field
+/* File attributes at open.  We try to keep the .size field
  * updated if it changes (eg by writing at the end of the file).
  */
-LIBSSH2_SFTP_ATTRIBUTES attrs;
+sftp_attributes attrs;
 
 InetSocketAddress *inet;
 
@@ -91,27 +85,25 @@ static void ssh_state_init(BDRVSSHState *s)
 {
 memset(s, 0, sizeof *s);
 s->sock = -1;
-s->offset = -1;
 qemu_co_mutex_init(>lock);
 }
 
 static void ssh_state_free(BDRVSSHState *s)
 {
+if (s->attrs) {
+sftp_attributes_free(s->attrs);
+}
 if (s->sftp_handle) {
-libssh2_sftp_close(s->sftp_handle);
+sftp_close(s->sftp_handle);
 }
 if (s->sftp) {
-libssh2_sftp_shutdown(s->sftp);
+sftp_free(s->sftp);
 }
 if (s->session) {
-

Re: [Qemu-devel] [PATCH v4] ssh: switch from libssh2 to libssh

[Pino Toscano]

On Tuesday, 13 February 2018 19:49:12 CET Max Reitz wrote:
> On 2018-01-18 17:44, Pino Toscano wrote:
> > Rewrite the implementation of the ssh block driver to use libssh instead
> > of libssh2.  The libssh library has various advantages over libssh2:
> > - easier API for authentication (for example for using ssh-agent)
> > - easier API for known_hosts handling
> > - supports newer types of keys in known_hosts
> > 
> > Kerberos authentication can be enabled once the libssh buxg for it [1] is
> > fixed.
> > 
> > The development version of libssh (i.e. the future 0.8.x) supports
> > fsync, so reuse the build time check for this.
> > 
> > [1] https://red.libssh.org/issues/242
> > 
> > Signed-off-by: Pino Toscano 
> > ---

Sorry for the (very late) reply.

I fixed basically all the code issues noted with this review; follow
few replies/notes that are not just "fixed".

> > Changes from v3:
> > - fix socket cleanup in connect_to_ssh()
> > - add comments about the socket cleanup
> > - improve the error reporting (closer to what was with libssh2)
> > - improve EOF detection on sftp_read()
> > 
> > Changes from v2:
> > - used again an own fd
> > - fixed co_yield() implementation
> > 
> > Changes from v1:
> > - fixed jumbo packets writing
> > - fixed missing 'err' assignment
> > - fixed commit message
> 
> One thing: The performance seems to have dropped hugely, from what I can
> tell.
> 
> Before this patch, running the iotests 1-10 over ssh (raw/ssh) took
> 12.6 s.  With this patch, they take 59.3 s.  Perhaps the starkest
> contrast can be seen in test 1, which took 4 s before and 27 s after --
> this test simply reads and writes 128 MB of continuous data.
> 
> I like having elliptic curves, but I think this patch needs optimization
> work before we can replace libssh2.

One thing that I discovered helping a lot is setting the TCP_NODELAY
option on the socket, to disable the Nagle algorithm; this drastically
reduced the overhead, which now does not seem to be more than 200% on
the very intensive tests (at least with my benchmarks).
Also using libssh from master shows more improvements too (and a bit
more of instability though, but that's a different story), and the
resulting overhead seems more acceptable to me now.

> 
> >  block/Makefile.objs |   6 +-
> >  block/ssh.c | 522 
> > 
> >  configure   |  65 ---
> >  3 files changed, 278 insertions(+), 315 deletions(-)
> 
> [...]
> 
> > diff --git a/block/ssh.c b/block/ssh.c
> > index b049a16eb9..2975fc27d8 100644
> > --- a/block/ssh.c
> > +++ b/block/ssh.c
> 
> [...]
> 
> > @@ -87,27 +81,25 @@ static void ssh_state_init(BDRVSSHState *s)
> >  {
> >  memset(s, 0, sizeof *s);
> >  s->sock = -1;
> > -s->offset = -1;
> >  qemu_co_mutex_init(>lock);
> >  }
> >  
> >  static void ssh_state_free(BDRVSSHState *s)
> >  {
> > +if (s->attrs) {
> > +sftp_attributes_free(s->attrs);
> > +}
> >  if (s->sftp_handle) {
> > -libssh2_sftp_close(s->sftp_handle);
> > +sftp_close(s->sftp_handle);
> >  }
> >  if (s->sftp) {
> > -libssh2_sftp_shutdown(s->sftp);
> > +sftp_free(s->sftp);
> >  }
> >  if (s->session) {
> > -libssh2_session_disconnect(s->session,
> > -   "from qemu ssh client: "
> > -   "user closed the connection");
> > -libssh2_session_free(s->session);
> > -}
> > -if (s->sock >= 0) {
> > -close(s->sock);
> > +ssh_disconnect(s->session);
> > +ssh_free(s->session);
> >  }
> > +/* s->sock is owned by the ssh_session, which free's it. */
> 
> s/free's/frees/
> 
> >  }
> >  
> >  static void GCC_FMT_ATTR(3, 4)
> > @@ -121,13 +113,13 @@ session_error_setg(Error **errp, BDRVSSHState *s, 
> > const char *fs, ...)
> >  va_end(args);
> >  
> >  if (s->session) {
> > -char *ssh_err;
> > +const char *ssh_err;
> >  int ssh_err_code;
> >  
> > -/* This is not an errno.  See . */
> > -ssh_err_code = libssh2_session_last_error(s->session,
> > -  _err, NULL, 0);
> > -error_setg(errp, "%s: %s (libssh2 error code: %d)",
> > +/* This is not an errno.  See . */
> > +ssh_err = ssh_get_error(s->session);
> > +ssh_err_code = ssh_get_error_code(s->session);
> > +error_setg(errp, "%s: %s (libssh error code: %d)",
> > msg, ssh_err, ssh_err_code);
> 
> Maybe we should not append the error info if there is no error.
> 
> (Example:
> 
> $ ./qemu-img info ssh://localhost/tmp/foo
> qemu-img: Could not open 'ssh://localhost/tmp/foo': no host key was
> found in known_hosts:  (libssh error code: 0)

The current libssh2 code has the same issue -- the solution is what you
mention later on, i.e. use error_setg() directly.

There were few more "mismatches" on the usage of the error 

Re: [Qemu-devel] block/dirty-bitmap: Useless bitmaps in image can be removed

[John Snow]

On 06/24/2018 11:36 PM, 13466399...@163.com wrote:
> If qemu-kvm quit without saving bitmaps to image(coredump, host kernel panic,
> or host pooweroff), bitmaps in image can not be safely used anymore, and also
> can not be removed. Useless bitmaps should be removed.
> 
> Signed-off-by: yaoxu <13466399...@163.com>

I count at least ten copies of this same patch in my inbox. Please send
just ONE copy, and make sure it has [PATCH] in the title, like other
patches do.

> ---
> diff --git a/blockdev.c b/blockdev.c
> index 58d7570932..c85056a74b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2837,31 +2837,35 @@ void qmp_block_dirty_bitmap_remove(const char *node, 
> const char *name,
>  Error *local_err = NULL;
>  
>  bitmap = block_dirty_bitmap_lookup(node, name, , errp);
> -if (!bitmap || !bs) {
> +if (!bs) {

In what cases are we going to fail to find a bitmap but manage to return
a BlockDriverState?

>  return;
>  }
>  
> -if (bdrv_dirty_bitmap_frozen(bitmap)) {
> -error_setg(errp,
> -   "Bitmap '%s' is currently frozen and cannot be removed",
> -   name);
> -return;
> -} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
> -error_setg(errp,
> -   "Bitmap '%s' is currently locked and cannot be removed",
> -   name);
> +if (bitmap) {
> +if (bdrv_dirty_bitmap_frozen(bitmap)) {
> +error_setg(errp,
> +   "Bitmap '%s' is currently frozen and cannot be 
> removed",
> +   name);
> +return;
> +} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
> +error_setg(errp,
> +   "Bitmap '%s' is currently locked and cannot be 
> removed",
> +   name);
> +return;
> +}
> +}
> +
> +bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
> +if (local_err != NULL) {
> +error_propagate(errp, local_err);
>  return;
>  }
>  
> -if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
> -bdrv_remove_persistent_dirty_bitmap(bs, name, _err);
> -if (local_err != NULL) {
> -error_propagate(errp, local_err);
> -return;
> -}
> +if (bitmap) {
> +bdrv_release_dirty_bitmap(bs, bitmap);
>  }
>  
> -bdrv_release_dirty_bitmap(bs, bitmap);
> +if (*errp) {
> +error_free(*errp);
> +*errp = NULL;
> +}
>  }
>  
>  /**
> 

Even if bitmap was null and bs was set, this is going to forward
requests for any bitmap, whether it exists or not, straight along to the
persistence layer, which does not give you an error if the bitmap didn't
exist.

We also can't open images with corrupted bitmaps in them for editing, so
this won't work there, either.

We are otherwise aware of the problem and intend to address it via
`qemu-img check -r`.

--js

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

[Eduardo Habkost]

On Mon, Jun 25, 2018 at 12:28:34PM +0200, Paolo Bonzini wrote:
> On 25/06/2018 08:50, Markus Armbruster wrote:
> > Paolo Bonzini  writes:
> > 
> >> On 22/06/2018 21:35, Eduardo Habkost wrote:
> > Why is this better than using KVM by default if it's available?
>  The answer is (as almost always): Compatibility with migration. Nobody
>  dares to sacrifice that chicken :-(
> >>> We can now kill it if we announce the feature as deprecated a
> >>> couple of releases in advance.
> >>>
> >>> If we declare that compatibility when the accelerator is omitted
> >>> is deprecated in 3.0, in QEMU 3.3 we will be free to choose a
> >>> different default accelerator.
> >>
> >> We can, we don't necessarily want it.
> >>
> >> The status quo is that people using KVM are invoking qemu as qemu-kvm,
> >> people using TCG are invoking qemu as qemu-system-*.  All distros are
> >> shipping a qemu-kvm or more rarely kvm binary, which is invariably a
> >> wrapper script except for RHEL because RHEL doesn't have a qemu-system-*
> >> binary at all.
> >>
> >> By the way, changing qemu-system-*'s default to e.g. RHEL's "kvm or tcg"
> >> would not help distros that have "-accel kvm" in their /usr/bin/qemu-kvm
> >> script.
> > 
> > It wouldn't hurt them, either.
> 
> Right; to sum up, it does make things a little less consistent for their
> users in two ways:
> 
> - qemu-system- behaves differently from qemu-system-.
> For example, for ARM the default CPU model might not work for KVM, so
> you would have to add a "-cpu xxx" option.
> 
> - qemu-system- would still need an accelerator option on OS X or
> Windows, where there is not quite parity between TCG and the native
> accelerator, in terms of either features or stability.  Because of this
> we wouldn't be able to change the default to "whatever virtualizing
> accelerators are available followed by TCG".
> 
> > Attentive distros could even replace the wrapper script by a link.
> 
> If they are okay with replacing the "KVM only" semantics with "KVM or
> TCG", which I think is generally worse.

If we can't get agreement on what's the right default for each
QEMU binary, I think that's yet another reason to document that
upstream QEMU won't guarantee ABI compatibility if -accel is
omitted.

If downstream distributions want to keep promising ABI
compatibility, it will be up to them.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v5 17/35] target/arm: Implement SVE floating-point arithmetic with immediate

[Peter Maydell]

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 56 
>  target/arm/sve_helper.c| 69 +++
>  target/arm/translate-sve.c | 75 ++
>  target/arm/sve.decode  | 14 +++
>  4 files changed, 214 insertions(+)

> +
> +#define float16_two  make_float16(0x4000)
> +#define float32_two  make_float32(0x4000)
> +#define float64_two  make_float64(0x4000ULL)

We seem to have these already in include/fpu/softfloat.h.

> +DO_FP_IMM(FADD, fadds, half, one)
> +DO_FP_IMM(FSUB, fsubs, half, one)
> +DO_FP_IMM(FMUL, fmuls, half, two)
> +DO_FP_IMM(FSUBR, fsubrs, half, one)
> +DO_FP_IMM(FMAXNM, fmaxnms, zero, one)
> +DO_FP_IMM(FMINNM, fminnms, zero, one)
> +DO_FP_IMM(FMAX, fmaxs, zero, one)
> +DO_FP_IMM(FMIN, fmins, zero, one)
> +
> +#undef DO_FP_IMM

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 16/35] target/arm: Implement SVE floating-point compare vectors

[Peter Maydell]

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 49 ++
>  target/arm/sve_helper.c| 62 ++
>  target/arm/translate-sve.c | 40 
>  target/arm/sve.decode  | 11 +++
>  4 files changed, 162 insertions(+)


> +#define DO_FCMGE(TYPE, X, Y, ST)  TYPE##_compare(Y, X, ST) <= 0

I was expecting the RHS of this to be TYPE##_le(Y, X, ST).
This prompted me to notice that softfloat has as well as
the generic 'compare' routines also specialized _le/lt/unordered
etc functions for float64 and float32 but not float16, which is
a bit inconsistent...

> +#define DO_FCMGT(TYPE, X, Y, ST)  TYPE##_compare(Y, X, ST) < 0
> +#define DO_FCMEQ(TYPE, X, Y, ST)  TYPE##_compare_quiet(X, Y, ST) == 0
> +#define DO_FCMNE(TYPE, X, Y, ST)  TYPE##_compare_quiet(X, Y, ST) != 0
> +#define DO_FCMUO(TYPE, X, Y, ST)  \
> +TYPE##_compare_quiet(X, Y, ST) == float_relation_unordered
> +#define DO_FACGE(TYPE, X, Y, ST)  \
> +TYPE##_compare(TYPE##_abs(Y), TYPE##_abs(X), ST) <= 0
> +#define DO_FACGT(TYPE, X, Y, ST)  \
> +TYPE##_compare(TYPE##_abs(Y), TYPE##_abs(X), ST) < 0

Anyway,
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH] docker: Add debian-xtensa-cross image

[Philippe Mathieu-Daudé]

On 06/25/2018 02:18 PM, Max Filippov wrote:
> On Mon, Jun 25, 2018 at 10:10 AM, Philippe Mathieu-Daudé
>  wrote:
>> On 06/25/2018 01:59 PM, Emilio G. Cota wrote:
>>> On Sat, Jun 23, 2018 at 20:24:09 -0700, Max Filippov wrote:
 # first bad commit: [0b5c91f74f3c83a36f37740969df8c775c997e69]
 translate-all: use per-page locking in !user-mode

 Emilio, could you please take a look? The following test locks up QEMU:

 qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting -icount
 6  -kernel ./test_mmu.tst

 test_mmu.tst binary may be found here:
 http://jcmvbkbc.spb.ru/~dumb/tmp/201806232022/test_mmu.tst
>>>
>>> Thanks for the test case.
>>>
>>> I submitted a patch that fixes this for me--see this thread:
>>>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07236.html
>>
>> Thanks, this works :)
>>
>> Alex nicer test output using:
>>
>> tests/tcg/xtensa/Makefile.system
>>  run-%.tst: %.tst
>> -   $(SIM) $(SIMFLAGS) ./$<
>> +   $(SIM) -monitor null $(SIMFLAGS) ./$<
> 
> This makefile is also usable with another simulator (xtensa ISS) which would
> not understand this option. Could it be added to the proper SIMFLAGS instead?

OK, good to know.

Re: [Qemu-devel] [PATCH] crypto: Implement TLS Pre-Shared Keys (PSK).

[Daniel P . Berrangé]

On Mon, Jun 25, 2018 at 06:07:15PM +0100, Richard W.M. Jones wrote:
> Pre-Shared Keys (PSK) is a simpler mechanism for enabling TLS
> connections than using certificates.  It requires only a simple secret
> key:
> 
>   $ mkdir -m 0700 /tmp/keys
>   $ psktool -u rjones -p /tmp/keys/keys.psk
>   $ cat /tmp/keys/keys.psk
>   rjones:d543770c15ad93d76443fb56f501a31969235f47e999720ae8d2336f6a13fcbc
> 
> The key can be secretly shared between clients and servers.  Clients
> must specify the directory containing the "keys.psk" file and a
> username (defaults to "qemu").  Servers must specify only the
> directory.
> 
> Example NBD client:
> 
>   $ qemu-img info \
> --object 
> tls-creds-psk,id=tls0,dir=/tmp/keys,username=rjones,endpoint=client \
> --image-opts \
> 
> file.driver=nbd,file.host=localhost,file.port=10809,file.tls-creds=tls0,file.export=/
> 
> Example NBD server using qemu-nbd:
> 
>   $ qemu-nbd -t -x / \
> --object tls-creds-psk,id=tls0,endpoint=server,dir=/tmp/keys \
> --tls-creds tls0 \
> image.qcow2
> 
> Example NBD server using nbdkit:
> 
>   $ nbdkit -n -e / -fv \
> --tls=on --tls-psk=/tmp/keys/keys.psk \
> file file=disk.img
> 
> Signed-off-by: Richard W.M. Jones 
> ---
>  crypto/Makefile.objs |   1 +
>  crypto/tlscredspsk.c | 300 
> +++
>  crypto/tlssession.c  |  38 ++
>  crypto/trace-events  |   3 +
>  include/crypto/tlscredspsk.h | 106 +++
>  5 files changed, 448 insertions(+)

Not done a proper code review yet but just a docs point...

Can you describe the new object type in qemu-options.hx - look for the
existing entry related to tls-creds-x509

Also in qemu-doc.texi at approx

  "@section TLS setup for network services"

we'll need something added related to PSK to describe how to set it
up. I guess we'll want to put what's there already under an "x509"
section, and add a PSK section


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: [Qemu-devel] [PATCH] nbd/client: add x-block-status hack for testing server

[John Snow]

On 06/25/2018 09:50 AM, Max Reitz wrote:
> On 2018-06-21 05:25, Eric Blake wrote:
>> In order to test that the NBD server is properly advertising
>> dirty bitmaps, we need a bare minimum client that can request
>> and read the context.  This patch is a hack (hence the use of
>> the x- prefix) that serves two purposes: first, it lets the
>> client pass a request of more than one context at a time to
>> the server, to test the reaction of the server to various
>> contexts (via the list command).  Second, whatever the first
>> context in the user's list becomes the context wired up to the
>> results visible in bdrv_block_status(); this has the result
>> that if you pass in 'qemu:dirty-bitmap:b' instead of the usual
>> 'base:allocation', and the server is currently serving a named
>> bitmap 'b', then commands like 'qemu-img map' now output status
>> corresponding to the dirty bitmap (dirty sections look like
>> holes, while clean sections look like data, based on how the
>> status bits are mapped over the NBD protocol).
>>
>> Since the hack corrupts the meaning of bdrv_block_status(), I
>> would NOT try to run 'qemu-img convert' or any other program
>> that might misbehave based on thinking clusters have a different
>> status than what the normal 'base:allocation' would provide.
>>
>> The hack uses a semicolon-separated list embedded in a single
>> string, as that was easier to wire into the nbd block driver than
>> figuring out the right incantation of flattened QDict to represent
>> an array via the command line.  Oh well, just one more reason that
>> this hack deserves the 'x-' prefix.
> 
> Without having looked at the patch, would an "x-debug-" prefix work
> better?  We have that for x-debug-block-dirty-bitmap-sha256.  The reason
> is that just "x-" means "experimental", which at least to me implies
> "once we have done all of our experiments, it will no longer be
> experimental and the prefix is dropped".  "x-debug-" means that it is
> actually not experimental, but just a debugging interface that will
> never be stable.
> 
> (Yes, I know we haven't documented the meaning of x-debug- anywhere...)
> 
> Max
> 

I'm not sure we actually want to check this in even temporarily, but we
do need to make use of it as a debugging facility in the meantime.

As such, x-debug- works perfectly well, but it may not matter.

--js

>> As a demo, I was able to prove things work with the following sequence:
>>
>> $ qemu-img info file
>> image: file
>> file format: qcow2
>> virtual size: 2.0M (2097152 bytes)
>> disk size: 2.0M
>> cluster_size: 65536
>> Format specific information:
>> compat: 1.1
>> lazy refcounts: false
>> refcount bits: 16
>> corrupt: false
>>
>> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio
>> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, 
>> "package": "v2.12.0-1531-g3ab98aa673d"}, "capabilities": []}}
>> {'execute':'qmp_capabilities'}
>> {"return": {}}
>> {'execute':'blockdev-add','arguments':{'driver':'qcow2','node-name':'n','file':{'driver':'file','filename':'file'}}}
>> {"return": {}}
>> {'execute':'block-dirty-bitmap-add','arguments':{'node':'n','name':'b','persistent':true}}
>> {"return": {}}
>> {'execute':'quit'}
>> {"return": {}}
>> {"timestamp": {"seconds": 1529548814, "microseconds": 472828}, "event": 
>> "SHUTDOWN", "data": {"guest": false}}
>>
>> $ ./qemu-io -f qcow2 file
>> qemu-io> r -v 0 1
>> :  01  .
>> read 1/1 bytes at offset 0
>> 1 bytes, 1 ops; 0.0001 sec (4.957 KiB/sec and 5076.1421 ops/sec)
>> qemu-io> w -P 1 0 1
>> wrote 1/1 bytes at offset 0
>> 1 bytes, 1 ops; 0.0078 sec (127.502231 bytes/sec and 127.5022 ops/sec)
>> qemu-io> q
>>
>> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio
>> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2}, 
>> "package": "v2.12.0-1531-g3ab98aa673d"}, "capabilities": []}}
>> {'execute':'qmp_capabilities'}
>> {"return": {}}
>> {'execute':'nbd-server-start','arguments':{'addr':{'type':'inet','data':{'host':'localhost','port':'10809'
>> {"return": {}}
>> {'execute':'blockdev-add','arguments':{'driver':'qcow2','node-name':'n','file':{'driver':'file','filename':'file'}}}
>> {"return": {}}
>> {'execute':'nbd-server-add','arguments':{'device':'n'}}
>> {"return": {}}
>> {'execute':'x-nbd-server-add-bitmap','arguments':{'name':'n','bitmap':'b'}}
>> {"error": {"class": "GenericError", "desc": "Bitmap 'b' is enabled"}}
>> {'execute':'x-block-dirty-bitmap-disable','arguments':{'node':'n','name':'b'}}
>> {"return": {}}
>> {'execute':'x-nbd-server-add-bitmap','arguments':{'name':'n','bitmap':'b'}}
>> {"return": {}}
>> ... leave running
>>
>> $ ./qemu-img map --output=json --image-opts 
>> driver=nbd,export=n,server.type=inet,server.host=localhost,server.port=10809
>> [{ "start": 0, "length": 1114112, "depth": 0, "zero": false, "data": true},
>> { "start": 1114112, "length": 458752, "depth": 0, "zero": true, "data": 
>> false},
>> { "start": 1572864, 

Re: [Qemu-devel] [PATCH] translate-all: fix locking of TBs whose two pages share the same physical page

[Philippe Mathieu-Daudé]

On 06/25/2018 01:31 PM, Emilio G. Cota wrote:
> Commit 0b5c91f ("translate-all: use per-page locking in !user-mode",
> 2018-06-15) introduced per-page locking. It assumed that the physical
> pages corresponding to a TB (at most two pages) are always distinct,
> which is wrong. For instance, an xtensa test provided by Max Filippov
> is broken by the commit, since the test maps two virtual pages
> to the same physical page:
> 
>   virt1: 7fff, virt2: 8000
>   phys1 6000fff, phys2 600
> 
> Fix it by removing the assumption from page_lock_pair.
> If the two physical page addresses are equal, we only lock
> the PageDesc once. Note that the two callers of page_lock_pair,
> namely page_unlock_tb and tb_link_page, are also updated so that
> we do not try to unlock the same PageDesc twice.
> 
> Fixes: 0b5c91f74f3c83a36f37740969df8c775c997e69
> Reported-by: Max Filippov 
> Signed-off-by: Emilio G. Cota 

$ make check-tcg (qemu:debian-xtensa-cross)

Tested-by: Philippe Mathieu-Daudé 

> ---
>  accel/tcg/translate-all.c | 32 +---
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index f0c3fd4..8e0203e 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -669,9 +669,15 @@ static inline void page_lock_tb(const TranslationBlock 
> *tb)
>  
>  static inline void page_unlock_tb(const TranslationBlock *tb)
>  {
> -page_unlock(page_find(tb->page_addr[0] >> TARGET_PAGE_BITS));
> +PageDesc *p1 = page_find(tb->page_addr[0] >> TARGET_PAGE_BITS);
> +
> +page_unlock(p1);
>  if (unlikely(tb->page_addr[1] != -1)) {
> -page_unlock(page_find(tb->page_addr[1] >> TARGET_PAGE_BITS));
> +PageDesc *p2 = page_find(tb->page_addr[1] >> TARGET_PAGE_BITS);
> +
> +if (p2 != p1) {
> +page_unlock(p2);
> +}
>  }
>  }
>  
> @@ -850,22 +856,34 @@ static void page_lock_pair(PageDesc **ret_p1, 
> tb_page_addr_t phys1,
> PageDesc **ret_p2, tb_page_addr_t phys2, int 
> alloc)
>  {
>  PageDesc *p1, *p2;
> +tb_page_addr_t page1;
> +tb_page_addr_t page2;
>  
>  assert_memory_lock();
> -g_assert(phys1 != -1 && phys1 != phys2);
> -p1 = page_find_alloc(phys1 >> TARGET_PAGE_BITS, alloc);
> +g_assert(phys1 != -1);
> +
> +page1 = phys1 >> TARGET_PAGE_BITS;
> +page2 = phys2 >> TARGET_PAGE_BITS;
> +
> +p1 = page_find_alloc(page1, alloc);
>  if (ret_p1) {
>  *ret_p1 = p1;
>  }
>  if (likely(phys2 == -1)) {
>  page_lock(p1);
>  return;
> +} else if (page1 == page2) {
> +page_lock(p1);
> +if (ret_p2) {
> +*ret_p2 = p1;
> +}
> +return;
>  }
> -p2 = page_find_alloc(phys2 >> TARGET_PAGE_BITS, alloc);
> +p2 = page_find_alloc(page2, alloc);
>  if (ret_p2) {
>  *ret_p2 = p2;
>  }
> -if (phys1 < phys2) {
> +if (page1 < page2) {
>  page_lock(p1);
>  page_lock(p2);
>  } else {
> @@ -1623,7 +1641,7 @@ tb_link_page(TranslationBlock *tb, tb_page_addr_t 
> phys_pc,
>  tb = existing_tb;
>  }
>  
> -if (p2) {
> +if (p2 && p2 != p) {
>  page_unlock(p2);
>  }
>  page_unlock(p);
> 

Re: [Qemu-devel] [RFC PATCH] docker: Add debian-xtensa-cross image

[Max Filippov]

On Mon, Jun 25, 2018 at 10:10 AM, Philippe Mathieu-Daudé
 wrote:
> On 06/25/2018 01:59 PM, Emilio G. Cota wrote:
>> On Sat, Jun 23, 2018 at 20:24:09 -0700, Max Filippov wrote:
>>> # first bad commit: [0b5c91f74f3c83a36f37740969df8c775c997e69]
>>> translate-all: use per-page locking in !user-mode
>>>
>>> Emilio, could you please take a look? The following test locks up QEMU:
>>>
>>> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting -icount
>>> 6  -kernel ./test_mmu.tst
>>>
>>> test_mmu.tst binary may be found here:
>>> http://jcmvbkbc.spb.ru/~dumb/tmp/201806232022/test_mmu.tst
>>
>> Thanks for the test case.
>>
>> I submitted a patch that fixes this for me--see this thread:
>>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07236.html
>
> Thanks, this works :)
>
> Alex nicer test output using:
>
> tests/tcg/xtensa/Makefile.system
>  run-%.tst: %.tst
> -   $(SIM) $(SIMFLAGS) ./$<
> +   $(SIM) -monitor null $(SIMFLAGS) ./$<

This makefile is also usable with another simulator (xtensa ISS) which would
not understand this option. Could it be added to the proper SIMFLAGS instead?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH] migration: invalidate cache before source start

[Dr. David Alan Gilbert]

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> 15.06.2018 15:06, Dr. David Alan Gilbert wrote:
> > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> > > Invalidate cache before source start in case of failed migration.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > Why doesn't the code at the bottom of migration_completion,
> > fail_invalidate:   and the code in migrate_fd_cancel   handle this?
> > 
> > What case did you see it in that those didn't handle?
> > (Also I guess it probably should set s->block_inactive = false)
> 
> on source I see:
> 
> 81392@1529065750.766289:migrate_set_state new state 7
> 81392@1529065750.766330:migration_thread_file_err
> 81392@1529065750.766332:migration_thread_after_loop
> 
> so, we are leaving loop on
>     if (qemu_file_get_error(s->to_dst_file)) {
>     migrate_set_state(>state, current_active_state,
> MIGRATION_STATUS_FAILED);
> trace_migration_thread_file_err();
> break;
>     }
> 
> and skip migration_completion()

Yeh, OK; I'd seen soemthing else a few days ago, where a cancellation
test that had previously ended with a 'cancelled' state has now ended up
in 'failed' (which is the state 7 you have above).
I suspect there's something else going on as well; I think what is
supposed to happen in the case of 'cancel' is that it spins in 'cancelling' for
a while in migrate_fd_cancel and then at the bottom of migrate_fd_cancel
it does the recovery, but because it's going to failed instead, then
it's jumping over that recovery.

Dave

> 
> > 
> > Dave
> > 
> > > ---
> > > 
> > >   migration/migration.c | 9 -
> > >   1 file changed, 8 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 1e99ec9b7e..8f39e0dc02 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -2806,7 +2806,14 @@ static void 
> > > migration_iteration_finish(MigrationState *s)
> > >   case MIGRATION_STATUS_FAILED:
> > >   case MIGRATION_STATUS_CANCELLED:
> > >   if (s->vm_was_running) {
> > > -vm_start();
> > > +Error *local_err = NULL;
> > > +bdrv_invalidate_cache_all(_err);
> > > +if (local_err) {
> > > +error_reportf_err(local_err, "Can't invalidate disks 
> > > before "
> > > +  "source vm start");
> > > +} else {
> > > +vm_start();
> > > +}
> > >   } else {
> > >   if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
> > >   runstate_set(RUN_STATE_POSTMIGRATE);
> > > -- 
> > > 2.11.1
> > > 
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> 
> 
> -- 
> Best regards,
> Vladimir
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 1/3] hw/block/fdc: Replace error_setg(_abort) by assert()

[John Snow]

On 06/25/2018 12:57 PM, Philippe Mathieu-Daudé wrote:
> Use assert() instead of error_setg(_abort),
> as suggested by the "qapi/error.h" documentation:
> 
> Please don't error_setg(_fatal, ...), use error_report() and
> exit(), because that's more obvious.
> Likewise, don't error_setg(_abort, ...), use assert().
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Acked-by: John Snow 

AIUI this series is to be merged by someone else currently, right?
Nothing needed on my part?

Re: [Qemu-devel] [PATCH v5 14/35] target/arm: Implement SVE first-fault gather loads

[Peter Maydell]

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  67 
>  target/arm/sve_helper.c|  88 ++
>  target/arm/translate-sve.c | 126 -
>  3 files changed, 236 insertions(+), 45 deletions(-)

...ah, I see the answer about FF loads is "in the next patch"...


Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH] docker: Add debian-xtensa-cross image

[Philippe Mathieu-Daudé]

On 06/25/2018 01:59 PM, Emilio G. Cota wrote:
> On Sat, Jun 23, 2018 at 20:24:09 -0700, Max Filippov wrote:
>> # first bad commit: [0b5c91f74f3c83a36f37740969df8c775c997e69]
>> translate-all: use per-page locking in !user-mode
>>
>> Emilio, could you please take a look? The following test locks up QEMU:
>>
>> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting -icount
>> 6  -kernel ./test_mmu.tst
>>
>> test_mmu.tst binary may be found here:
>> http://jcmvbkbc.spb.ru/~dumb/tmp/201806232022/test_mmu.tst
> 
> Thanks for the test case.
> 
> I submitted a patch that fixes this for me--see this thread:
>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07236.html

Thanks, this works :)

Alex nicer test output using:

tests/tcg/xtensa/Makefile.system
 run-%.tst: %.tst
-   $(SIM) $(SIMFLAGS) ./$<
+   $(SIM) -monitor null $(SIMFLAGS) ./$<

[Qemu-devel] [PATCH v4 0/3] qapi/error: converts error_setg(_fatal) to error_report() + exit()

[Philippe Mathieu-Daudé]

Hi,

This series converts error_setg(_fatal) to error_report() + exit() as
suggested by the "qapi/error.h" documentation.

This reduce Coverity and Clang static analyzer positive falses.

See http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg07585.html:

On 07/24/2017 04:52 PM, Eric Blake wrote:
That's a shame.  Rather, we should patch this file (and others) to avoid
all the inconsistent uses of error_setg(_*), to comply with the
error.h documentation.

Since v3:
- corrected error display in copy_properties_from_host (Eric)
  display error if property required and returned -FDT_ERR_NOTFOUND
- resetted Markus R-b tag

Since v2:
- added R-b, A-b (Markus, John, David)
- fixed incorrect update of sysbus-fdt patch between v1->v2 (Markus)

Since v1:
- patch #1: use assert() directly (Markus explanation)
- patch #2: use abort() without error_report() (Markus 'no lipstick')
- patch #3: replaced exit() by assert() (Markus)
- patch #4: no change, added R-b

Regards,

Phil.

[] : patches are identical
[] : number of functional differences between upstream/downstream patch
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/3:[] [--] 'hw/block/fdc: Replace error_setg(_abort) by assert()'
002/3:[0010] [FC] 'hw/arm/sysbus-fdt: Replace error_setg(_fatal) by 
error_report() + exit()'
003/3:[] [--] 'device_tree: Replace error_setg(_fatal) by 
error_report() + exit()'

Philippe Mathieu-Daudé (3):
  hw/block/fdc: Replace error_setg(_abort) by assert()
  hw/arm/sysbus-fdt: Replace error_setg(_fatal) by error_report() + exit()
  device_tree: Replace error_setg(_fatal) by error_report() + exit()

 device_tree.c   | 23 +++-
 hw/arm/sysbus-fdt.c | 53 +
 hw/block/fdc.c  |  9 +---
 3 files changed, 44 insertions(+), 41 deletions(-)

-- 
2.18.0

[Qemu-devel] [PATCH] crypto: Implement TLS Pre-Shared Keys (PSK).

[Richard W.M. Jones]

Pre-Shared Keys (PSK) is a simpler mechanism for enabling TLS
connections than using certificates.  It requires only a simple secret
key:

  $ mkdir -m 0700 /tmp/keys
  $ psktool -u rjones -p /tmp/keys/keys.psk
  $ cat /tmp/keys/keys.psk
  rjones:d543770c15ad93d76443fb56f501a31969235f47e999720ae8d2336f6a13fcbc

The key can be secretly shared between clients and servers.  Clients
must specify the directory containing the "keys.psk" file and a
username (defaults to "qemu").  Servers must specify only the
directory.

Example NBD client:

  $ qemu-img info \
--object 
tls-creds-psk,id=tls0,dir=/tmp/keys,username=rjones,endpoint=client \
--image-opts \

file.driver=nbd,file.host=localhost,file.port=10809,file.tls-creds=tls0,file.export=/

Example NBD server using qemu-nbd:

  $ qemu-nbd -t -x / \
--object tls-creds-psk,id=tls0,endpoint=server,dir=/tmp/keys \
--tls-creds tls0 \
image.qcow2

Example NBD server using nbdkit:

  $ nbdkit -n -e / -fv \
--tls=on --tls-psk=/tmp/keys/keys.psk \
file file=disk.img

Signed-off-by: Richard W.M. Jones 
---
 crypto/Makefile.objs |   1 +
 crypto/tlscredspsk.c | 300 +++
 crypto/tlssession.c  |  38 ++
 crypto/trace-events  |   3 +
 include/crypto/tlscredspsk.h | 106 +++
 5 files changed, 448 insertions(+)

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 2b99e08062..756bab111b 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -15,6 +15,7 @@ crypto-obj-$(CONFIG_AF_ALG) += cipher-afalg.o
 crypto-obj-$(CONFIG_AF_ALG) += hash-afalg.o
 crypto-obj-y += tlscreds.o
 crypto-obj-y += tlscredsanon.o
+crypto-obj-y += tlscredspsk.o
 crypto-obj-y += tlscredsx509.o
 crypto-obj-y += tlssession.o
 crypto-obj-y += secret.o
diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
new file mode 100644
index 00..0893af4053
--- /dev/null
+++ b/crypto/tlscredspsk.c
@@ -0,0 +1,300 @@
+/*
+ * QEMU crypto TLS Pre-Shared Keys (PSK) support
+ *
+ * Copyright (c) 2018 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/tlscredspsk.h"
+#include "tlscredspriv.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "trace.h"
+
+
+#ifdef CONFIG_GNUTLS
+
+static int
+lookup_key(const char *pskfile, const char *username, gnutls_datum_t *key,
+   Error **errp)
+{
+FILE *fp;
+char line[1024]; /* Maximum key length in psktool is 512 bytes. */
+size_t ulen = strlen(username);
+size_t len;
+
+fp = fopen(pskfile, "r");
+if (fp == NULL) {
+error_setg_errno(errp, errno, "Cannot open PSK file %s", pskfile);
+return -1;
+}
+while (fgets(line, sizeof line, fp) != NULL) {
+if (strncmp(line, username, ulen) == 0 && line[ulen] == ':') {
+len = strlen(line);
+if (len > 0 && line[len - 1] == '\n') {
+len--;
+line[len] = '\0';
+}
+key->data = (unsigned char *) g_strdup([ulen + 1]);
+key->size = len - ulen - 1;
+fclose(fp);
+return 0;
+}
+}
+fclose(fp);
+error_setg(errp, "Username %s not found in PSK file %s",
+   username, pskfile);
+return -1;
+}
+
+static int
+qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
+   Error **errp)
+{
+char *pskfile = NULL, *dhparams = NULL;
+const char *username;
+int ret;
+int rv = -1;
+gnutls_datum_t key = { .data = NULL };
+
+trace_qcrypto_tls_creds_psk_load(creds,
+creds->parent_obj.dir ? creds->parent_obj.dir : "");
+
+if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
+if (qcrypto_tls_creds_get_path(>parent_obj,
+   QCRYPTO_TLS_CREDS_DH_PARAMS,
+   false, , errp) < 0 ||
+qcrypto_tls_creds_get_path(>parent_obj,
+   QCRYPTO_TLS_CREDS_PSKFILE,
+   true, , errp) < 0) {
+goto cleanup;
+}
+
+ret = gnutls_psk_allocate_server_credentials(>data.server);
+if (ret < 0) {
+error_setg(errp, "Cannot allocate credentials: %s",
+

[Qemu-devel] [PATCH] crypto: Implement TLS Pre-Shared Keys (PSK).

[Richard W.M. Jones]

TLS-PSK (Pre-Shared Keys) lets us set up TLS connections much more
easily, especially for NBD.

This is a "version 0" of the patch for now, mainly to solicit
comments.  It needs documentation at least.

Rich.

[Qemu-devel] [PATCH 1/2] block/dirty-bitmap: add bdrv_enable_dirty_bitmap_locked

[Vladimir Sementsov-Ogievskiy]

Add _locked version of bdrv_enable_dirty_bitmap, to fix dirty bitmap
migration in the following patch.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/dirty-bitmap.h | 1 +
 block/dirty-bitmap.c | 9 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 288dc6adb6..259bd27c40 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -32,6 +32,7 @@ void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
  Error **errp);
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
 uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index db1782ec1f..93744b3565 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -241,6 +241,12 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState 
*bs,
 return 0;
 }
 
+void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)
+{
+assert(!bdrv_dirty_bitmap_frozen(bitmap));
+bitmap->disabled = false;
+}
+
 /* Called with BQL taken. */
 void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
 {
@@ -424,8 +430,7 @@ void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
 bdrv_dirty_bitmap_lock(bitmap);
-assert(!bdrv_dirty_bitmap_frozen(bitmap));
-bitmap->disabled = false;
+bdrv_enable_dirty_bitmap_locked(bitmap);
 bdrv_dirty_bitmap_unlock(bitmap);
 }
 
-- 
2.11.1

Re: [Qemu-devel] [PATCH v5 15/35] target/arm: Implement SVE scatter store vector immediate

[Peter Maydell]

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-sve.c | 83 ++
>  target/arm/sve.decode  | 11 +
>  2 files changed, 69 insertions(+), 25 deletions(-)
>
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH] docker: Add debian-xtensa-cross image

[Emilio G. Cota]

On Sat, Jun 23, 2018 at 20:24:09 -0700, Max Filippov wrote:
> # first bad commit: [0b5c91f74f3c83a36f37740969df8c775c997e69]
> translate-all: use per-page locking in !user-mode
> 
> Emilio, could you please take a look? The following test locks up QEMU:
> 
> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting -icount
> 6  -kernel ./test_mmu.tst
> 
> test_mmu.tst binary may be found here:
> http://jcmvbkbc.spb.ru/~dumb/tmp/201806232022/test_mmu.tst

Thanks for the test case.

I submitted a patch that fixes this for me--see this thread:
  https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07236.html

Emilio

[Qemu-devel] [PATCH 2/2] dirty-bitmap: fix double lock on bitmap enabling

[Vladimir Sementsov-Ogievskiy]

Bitmap lock/unlock were added to bdrv_enable_dirty_bitmap in
8b1402ce80d, but some places were not updated correspondingly, which
leads to trying to take this lock twice, which is dead-lock. Fix this.

Actually, iotest 199 (about dirty bitmap postcopy migration) is broken
now, and this fixes it.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/dirty-bitmap.c   | 3 ++-
 migration/block-dirty-bitmap.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 93744b3565..c9b8a6fd52 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -250,8 +250,9 @@ void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap 
*bitmap)
 /* Called with BQL taken. */
 void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
 {
+assert(bitmap->mutex == bitmap->successor->mutex);
 qemu_mutex_lock(bitmap->mutex);
-bdrv_enable_dirty_bitmap(bitmap->successor);
+bdrv_enable_dirty_bitmap_locked(bitmap->successor);
 qemu_mutex_unlock(bitmap->mutex);
 }
 
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 3bafbbdc4c..477826330c 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -511,7 +511,7 @@ void dirty_bitmap_mig_before_vm_start(void)
 DirtyBitmapLoadBitmapState *b = item->data;
 
 if (b->migrated) {
-bdrv_enable_dirty_bitmap(b->bitmap);
+bdrv_enable_dirty_bitmap_locked(b->bitmap);
 } else {
 bdrv_dirty_bitmap_enable_successor(b->bitmap);
 }
@@ -547,7 +547,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, 
DirtyBitmapLoadState *s)
 if (enabled_bitmaps == NULL) {
 /* in postcopy */
 bdrv_reclaim_dirty_bitmap_locked(s->bs, s->bitmap, _abort);
-bdrv_enable_dirty_bitmap(s->bitmap);
+bdrv_enable_dirty_bitmap_locked(s->bitmap);
 } else {
 /* target not started, successor must be empty */
 int64_t count = bdrv_get_dirty_count(s->bitmap);
-- 
2.11.1

[Qemu-devel] [PATCH v4 3/3] device_tree: Replace error_setg(_fatal) by error_report() + exit()

[Philippe Mathieu-Daudé]

Use error_report() + exit() instead of error_setg(_fatal),
as suggested by the "qapi/error.h" documentation:

   Please don't error_setg(_fatal, ...), use error_report() and
   exit(), because that's more obvious.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Auger 
Reviewed-by: Markus Armbruster 
Reviewed-by: David Gibson 
---
 device_tree.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 52c3358a55..3553819257 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -140,15 +140,16 @@ static void read_fstree(void *fdt, const char *dirname)
 const char *parent_node;
 
 if (strstr(dirname, root_dir) != dirname) {
-error_setg(_fatal, "%s: %s must be searched within %s",
-   __func__, dirname, root_dir);
+error_report("%s: %s must be searched within %s",
+ __func__, dirname, root_dir);
+exit(1);
 }
 parent_node = [strlen(SYSFS_DT_BASEDIR)];
 
 d = opendir(dirname);
 if (!d) {
-error_setg(_fatal, "%s cannot open %s", __func__, dirname);
-return;
+error_report("%s cannot open %s", __func__, dirname);
+exit(1);
 }
 
 while ((de = readdir(d)) != NULL) {
@@ -162,7 +163,8 @@ static void read_fstree(void *fdt, const char *dirname)
 tmpnam = g_strdup_printf("%s/%s", dirname, de->d_name);
 
 if (lstat(tmpnam, ) < 0) {
-error_setg(_fatal, "%s cannot lstat %s", __func__, tmpnam);
+error_report("%s cannot lstat %s", __func__, tmpnam);
+exit(1);
 }
 
 if (S_ISREG(st.st_mode)) {
@@ -170,8 +172,9 @@ static void read_fstree(void *fdt, const char *dirname)
 gsize len;
 
 if (!g_file_get_contents(tmpnam, , , NULL)) {
-error_setg(_fatal, "%s not able to extract info from %s",
-   __func__, tmpnam);
+error_report("%s not able to extract info from %s",
+ __func__, tmpnam);
+exit(1);
 }
 
 if (strlen(parent_node) > 0) {
@@ -206,9 +209,9 @@ void *load_device_tree_from_sysfs(void)
 host_fdt = create_device_tree(_fdt_size);
 read_fstree(host_fdt, SYSFS_DT_BASEDIR);
 if (fdt_check_header(host_fdt)) {
-error_setg(_fatal,
-   "%s host device tree extracted into memory is invalid",
-   __func__);
+error_report("%s host device tree extracted into memory is invalid",
+ __func__);
+exit(1);
 }
 return host_fdt;
 }
-- 
2.18.0