date:20200727

From: Laurent Vivier 

In legacy mode, virtio_pci_queue_enabled() falls back to
virtio_queue_enabled() to know if the queue is enabled.

But virtio_queue_enabled() calls again virtio_pci_queue_enabled()
if k->queue_enabled is set. This ends in a crash after a stack
overflow.

The problem can be reproduced with
"-device virtio-net-pci,disable-legacy=off,disable-modern=true
 -net tap,vhost=on"

And a look to the backtrace is very explicit:

...
#4  0x00010029a438 in virtio_queue_enabled ()
#5  0x000100497a9c in virtio_pci_queue_enabled ()
...
#130902 0x00010029a460 in virtio_queue_enabled ()
#130903 0x000100497a9c in virtio_pci_queue_enabled ()
#130904 0x00010029a460 in virtio_queue_enabled ()
#130905 0x000100454a20 in vhost_net_start ()
...

This patch fixes the problem by introducing a new function
for the legacy case and calls it from virtio_pci_queue_enabled().
It also calls it from virtio_queue_enabled() to avoid code duplication.

Fixes: f19bcdfedd53 ("virtio-pci: implement queue_enabled method")
Cc: Jason Wang 
Cc: Cindy Lu 
CC: Michael S. Tsirkin 
Reviewed-by: Richard Henderson 
Signed-off-by: Laurent Vivier 
Signed-off-by: Jason Wang 
---
 hw/virtio/virtio-pci.c | 2 +-
 hw/virtio/virtio.c | 7 ++-
 include/hw/virtio/virtio.h | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 2b1f9cc..ccdf54e 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1116,7 +1116,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int 
n)
 return proxy->vqs[n].enabled;
 }
 
-return virtio_queue_enabled(vdev, n);
+return virtio_queue_enabled_legacy(vdev, n);
 }
 
 static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 546a198..e983025 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3309,6 +3309,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, 
int n)
 return vdev->vq[n].vring.desc;
 }
 
+bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n)
+{
+return virtio_queue_get_desc_addr(vdev, n) != 0;
+}
+
 bool virtio_queue_enabled(VirtIODevice *vdev, int n)
 {
 BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
@@ -3317,7 +3322,7 @@ bool virtio_queue_enabled(VirtIODevice *vdev, int n)
 if (k->queue_enabled) {
 return k->queue_enabled(qbus->parent, n);
 }
-return virtio_queue_get_desc_addr(vdev, n) != 0;
+return virtio_queue_enabled_legacy(vdev, n);
 }
 
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 198ffc7..e424df1 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -295,6 +295,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
   VIRTIO_F_RING_PACKED, false)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
+bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
 bool virtio_queue_enabled(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
-- 
2.7.4

[PULL 4/4] net: forbid the reentrant RX

The memory API allows DMA into NIC's MMIO area. This means the NIC's
RX routine must be reentrant. Instead of auditing all the NIC, we can
simply detect the reentrancy and return early. The queue->delivering
is set and cleared by qemu_net_queue_deliver() for other queue helpers
to know whether the delivering in on going (NIC's receive is being
called). We can check it and return early in qemu_net_queue_flush() to
forbid reentrant RX.

Signed-off-by: Jason Wang 
---
 net/queue.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/queue.c b/net/queue.c
index 0164727..19e32c8 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -250,6 +250,9 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState 
*from)
 
 bool qemu_net_queue_flush(NetQueue *queue)
 {
+if (queue->delivering)
+return false;
+
 while (!QTAILQ_EMPTY(>packets)) {
 NetPacket *packet;
 int ret;
-- 
2.7.4

[PULL 0/4] Net patches

The following changes since commit 9303ecb658a0194560d1eecde165a1511223c2d8:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200727' into 
staging (2020-07-27 17:25:06 +0100)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 7142cad78d6bf4a1cbcb09d06b39935a7998c24e:

  net: forbid the reentrant RX (2020-07-28 13:50:41 +0800)


Want to send earlier but most patches just come.

- fix vhost-vdpa issues when no peer
- fix virtio-pci queue enabling check
- forbid reentrant RX


Jason Wang (2):
  virtio-net: check the existence of peer before accessing vDPA config
  net: forbid the reentrant RX

Laurent Vivier (1):
  virtio-pci: fix virtio_pci_queue_enabled()

Yuri Benditovich (1):
  virtio-pci: fix wrong index in virtio_pci_queue_enabled

 hw/net/virtio-net.c| 30 +++---
 hw/virtio/virtio-pci.c |  4 ++--
 hw/virtio/virtio.c |  7 ++-
 include/hw/virtio/virtio.h |  1 +
 net/queue.c|  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

Re: [BUG] vhost-vdpa: qemu-system-s390x crashes with second virtio-net-ccw device

On 2020/7/27 下午9:16, Michael S. Tsirkin wrote:

On Mon, Jul 27, 2020 at 08:44:09PM +0800, Jason Wang wrote:

On 2020/7/27 下午7:43, Michael S. Tsirkin wrote:

On Mon, Jul 27, 2020 at 04:51:23PM +0800, Jason Wang wrote:

On 2020/7/27 下午4:41, Cornelia Huck wrote:

On Mon, 27 Jul 2020 15:38:12 +0800
Jason Wang wrote:

On 2020/7/27 下午2:43, Cornelia Huck wrote:

On Sat, 25 Jul 2020 08:40:07 +0800
Jason Wang wrote:

On 2020/7/24 下午11:34, Cornelia Huck wrote:

On Fri, 24 Jul 2020 11:17:57 -0400
"Michael S. Tsirkin" wrote:

On Fri, Jul 24, 2020 at 04:56:27PM +0200, Cornelia Huck wrote:

On Fri, 24 Jul 2020 09:30:58 -0400
"Michael S. Tsirkin" wrote:

On Fri, Jul 24, 2020 at 03:27:18PM +0200, Cornelia Huck wrote:

When I start qemu with a second virtio-net-ccw device (i.e. adding
-device virtio-net-ccw in addition to the autogenerated device), I get
a segfault. gdb points to

#0 0x55d6ab52681d in virtio_net_get_config (vdev=,
config=0x55d6ad9e3f80 "RT") at
/home/cohuck/git/qemu/hw/net/virtio-net.c:146
146 if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {

(backtrace doesn't go further)

The core was incomplete, but running under gdb directly shows that it
is just a bog-standard config space access (first for that device).

The cause of the crash is that nc->peer is not set... no idea how that
can happen, not that familiar with that part of QEMU. (Should the code
check, or is that really something that should not happen?)

What I don't understand is why it is set correctly for the first,
autogenerated virtio-net-ccw device, but not for the second one, and
why virtio-net-pci doesn't show these problems. The only difference
between -ccw and -pci that comes to my mind here is that config space
accesses for ccw are done via an asynchronous operation, so timing
might be different.

Hopefully Jason has an idea. Could you post a full command line
please? Do you need a working guest to trigger this? Does this trigger
on an x86 host?

Yes, it does trigger with tcg-on-x86 as well. I've been using

s390x-softmmu/qemu-system-s390x -M s390-ccw-virtio,accel=tcg -cpu qemu,zpci=on
-m 1024 -nographic -device virtio-scsi-ccw,id=scsi0,devno=fe.0.0001
-drive file=/path/to/image,format=qcow2,if=none,id=drive-scsi0-0-0-0
-device
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1
-device virtio-net-ccw

It seems it needs the guest actually doing something with the nics; I
cannot reproduce the crash if I use the old advent calendar moon buggy
image and just add a virtio-net-ccw device.

(I don't think it's a problem with my local build, as I see the problem
both on my laptop and on an LPAR.)

It looks to me we forget the check the existence of peer.

Please try the attached patch to see if it works.

Thanks, that patch gets my guest up and running again. So, FWIW,

Tested-by: Cornelia Huck

Any idea why this did not hit with virtio-net-pci (or the autogenerated
virtio-net-ccw device)?

It can be hit with virtio-net-pci as well (just start without peer).

Hm, I had not been able to reproduce the crash with a 'naked' -device
virtio-net-pci. But checking seems to be the right idea anyway.

Sorry for being unclear, I meant for networking part, you just need start
without peer, and you need a real guest (any Linux) that is trying to access
the config space of virtio-net.

Thanks

A pxe guest will do it, but that doesn't support ccw, right?

Yes, it depends on the cli actually.

I'm still unclear why this triggers with ccw but not pci -
any idea?

I don't test pxe but I can reproduce this with pci (just start a linux guest
without a peer).

Thanks

Might be a good addition to a unit test. Not sure what would the
test do exactly: just make sure guest runs? Looks like a lot of work
for an empty test ... maybe we can poke at the guest config with
qtest commands at least.

That should work or we can simply extend the exist virtio-net qtest to
do that.

Thanks

Re: [PATCH 1/2] hw/net/net_tx_pkt: add function to check pkt->max_raw_frags




On 2020/7/28 上午1:08, Mauro Matteo Cascella wrote:

This patch introduces a new function in hw/net/net_tx_pkt.{c,h} to check the
current data fragment against the maximum number of data fragments.



I wonder whether it's better to do the check in 
net_tx_pkt_add_raw_fragment() and fail there.


Btw, I find net_tx_pkt_add_raw_fragment() does not unmap dma when 
returning to true, is this a bug?


Thanks




Reported-by: Ziming Zhang 
Signed-off-by: Mauro Matteo Cascella 
---
  hw/net/net_tx_pkt.c | 5 +
  hw/net/net_tx_pkt.h | 8 
  2 files changed, 13 insertions(+)

diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index 9560e4a49e..d035618f2c 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -400,6 +400,11 @@ bool net_tx_pkt_add_raw_fragment(struct NetTxPkt *pkt, 
hwaddr pa,
  }
  }
  
+bool net_tx_pkt_exceed_max_fragments(struct NetTxPkt *pkt)

+{
+return pkt->raw_frags >= pkt->max_raw_frags;
+}
+
  bool net_tx_pkt_has_fragments(struct NetTxPkt *pkt)
  {
  return pkt->raw_frags > 0;
diff --git a/hw/net/net_tx_pkt.h b/hw/net/net_tx_pkt.h
index 4ec8bbe9bd..e2ee46ae03 100644
--- a/hw/net/net_tx_pkt.h
+++ b/hw/net/net_tx_pkt.h
@@ -179,6 +179,14 @@ bool net_tx_pkt_send_loopback(struct NetTxPkt *pkt, 
NetClientState *nc);
   */
  bool net_tx_pkt_parse(struct NetTxPkt *pkt);
  
+/**

+* indicates if the current data fragment exceeds max_raw_frags
+*
+* @pkt:packet
+*
+*/
+bool net_tx_pkt_exceed_max_fragments(struct NetTxPkt *pkt);
+
  /**
  * indicates if there are data fragments held by this packet object.
  *

Re: [PATCH 1/2] net: forbid the reentrant RX




On 2020/7/22 下午4:57, Jason Wang wrote:

The memory API allows DMA into NIC's MMIO area. This means the NIC's
RX routine must be reentrant. Instead of auditing all the NIC, we can
simply detect the reentrancy and return early. The queue->delivering
is set and cleared by qemu_net_queue_deliver() for other queue helpers
to know whether the delivering in on going (NIC's receive is being
called). We can check it and return early in qemu_net_queue_flush() to
forbid reentrant RX.

Signed-off-by: Jason Wang 
---
  net/queue.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/net/queue.c b/net/queue.c
index 0164727e39..19e32c80fd 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -250,6 +250,9 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState 
*from)
  
  bool qemu_net_queue_flush(NetQueue *queue)

  {
+if (queue->delivering)
+return false;
+
  while (!QTAILQ_EMPTY(>packets)) {
  NetPacket *packet;
  int ret;



Queued for rc2.

Thanks

Re: [PATCH] target/ppc: Fix TCG leak with the evmwsmiaa instruction

2020-07-27 Thread David Gibson

On Mon, Jul 27, 2020 at 10:21:14AM -0700, Matthieu Bucchianeri wrote:
> Fix double-call to tcg_temp_new_i64(), where a temp is allocated both at
> declaration time and further down the implementation of gen_evmwsmiaa().
> 
> Note that gen_evmwsmia() and gen_evmwsmiaa() are still not implemented
> correctly, as they invoke gen_evmwsmi() which may return early, but the
> return is not propagated. This will be fixed in my patch for bug #1888918.
> 
> Signed-off-by: Matthieu Bucchianeri
> 

Applied to ppc-for-5.1.  Note that since this isn't a regression, it's
not entirely clear it's a good candidate for 5.1 this late in the
freeze.  There's a possibility it will get punted to 5.2, therefore,
but for now I'm staging it for 5.1.

> ---
>  target/ppc/translate/spe-impl.inc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/translate/spe-impl.inc.c 
> b/target/ppc/translate/spe-impl.inc.c
> index 36b4d5654d..42a0d1cffb 100644
> --- a/target/ppc/translate/spe-impl.inc.c
> +++ b/target/ppc/translate/spe-impl.inc.c
> @@ -528,14 +528,14 @@ static inline void gen_evmwsmia(DisasContext *ctx)
> 
>  tcg_temp_free_i64(tmp);
>  }
> 
>  static inline void gen_evmwsmiaa(DisasContext *ctx)
>  {
> -TCGv_i64 acc = tcg_temp_new_i64();
> -TCGv_i64 tmp = tcg_temp_new_i64();
> +TCGv_i64 acc;
> +TCGv_i64 tmp;
> 
>  gen_evmwsmi(ctx);   /* rD := rA * rB */
> 
>  acc = tcg_temp_new_i64();
>  tmp = tcg_temp_new_i64();
> 

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


signature.asc
Description: PGP signature

Re: [PATCH] virtio-pci: fix wrong index in virtio_pci_queue_enabled




On 2020/7/27 下午10:38, Yuri Benditovich wrote:

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

Signed-off-by: Yuri Benditovich 



Queued for rc2.

Thanks



---
  hw/virtio/virtio-pci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ada1101d07..2b1f9cc67b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1113,7 +1113,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int 
n)
  VirtIODevice *vdev = virtio_bus_get_device(>bus);
  
  if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {

-return proxy->vqs[vdev->queue_sel].enabled;
+return proxy->vqs[n].enabled;
  }
  
  return virtio_queue_enabled(vdev, n);

Re: [PATCH] virtio-pci: fix virtio_pci_queue_enabled()




On 2020/7/27 下午11:33, Laurent Vivier wrote:

In legacy mode, virtio_pci_queue_enabled() falls back to
virtio_queue_enabled() to know if the queue is enabled.

But virtio_queue_enabled() calls again virtio_pci_queue_enabled()
if k->queue_enabled is set. This ends in a crash after a stack
overflow.

The problem can be reproduced with
"-device virtio-net-pci,disable-legacy=off,disable-modern=true
  -net tap,vhost=on"

And a look to the backtrace is very explicit:

 ...
 #4  0x00010029a438 in virtio_queue_enabled ()
 #5  0x000100497a9c in virtio_pci_queue_enabled ()
 ...
 #130902 0x00010029a460 in virtio_queue_enabled ()
 #130903 0x000100497a9c in virtio_pci_queue_enabled ()
 #130904 0x00010029a460 in virtio_queue_enabled ()
 #130905 0x000100454a20 in vhost_net_start ()
 ...

This patch fixes the problem by introducing a new function
for the legacy case and calls it from virtio_pci_queue_enabled().
It also calls it from virtio_queue_enabled() to avoid code duplication.

Fixes: f19bcdfedd53 ("virtio-pci: implement queue_enabled method")
Cc: Jason Wang 
Cc: Cindy Lu 
CC: Michael S. Tsirkin 
Signed-off-by: Laurent Vivier 



Queued for rc2.

Thanks



---
  hw/virtio/virtio-pci.c | 2 +-
  hw/virtio/virtio.c | 7 ++-
  include/hw/virtio/virtio.h | 1 +
  3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ada1101d07bf..4ad3ad81a2cf 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1116,7 +1116,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int 
n)
  return proxy->vqs[vdev->queue_sel].enabled;
  }
  
-return virtio_queue_enabled(vdev, n);

+return virtio_queue_enabled_legacy(vdev, n);
  }
  
  static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 546a198e79b0..e98302521769 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3309,6 +3309,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, 
int n)
  return vdev->vq[n].vring.desc;
  }
  
+bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n)

+{
+return virtio_queue_get_desc_addr(vdev, n) != 0;
+}
+
  bool virtio_queue_enabled(VirtIODevice *vdev, int n)
  {
  BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
@@ -3317,7 +3322,7 @@ bool virtio_queue_enabled(VirtIODevice *vdev, int n)
  if (k->queue_enabled) {
  return k->queue_enabled(qbus->parent, n);
  }
-return virtio_queue_get_desc_addr(vdev, n) != 0;
+return virtio_queue_enabled_legacy(vdev, n);
  }
  
  hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 198ffc762678..e424df12cf6d 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -295,6 +295,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
VIRTIO_F_RING_PACKED, false)
  
  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);

+bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
  bool virtio_queue_enabled(VirtIODevice *vdev, int n);
  hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
  hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);

[PATCH v5] hw/pci-host: save/restore pci host config register for old ones

2020-07-27 Thread Hogan Wang

The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
Refer to i440FX and ICH9-LPC spcifications, there are some reserved
configuration registers can used to save/restore PCIHostState.config_reg.
It's nasty but friendly to old ones.

Reproducer steps:
step 1. Make modifications to seabios and qemu for increase reproduction
efficiency, write 0xf0 to 0x402 port notify qemu to stop vcpu after
0x0cf8 port wrote i440 configure register. qemu stop vcpu when catch
0x402 port wrote 0xf0.

seabios:/src/hw/pci.c
@@ -52,6 +52,11 @@ void pci_config_writeb(u16 bdf, u32 addr, u8 val)
 writeb(mmconfig_addr(bdf, addr), val);
 } else {
 outl(ioconfig_cmd(bdf, addr), PORT_PCI_CMD);
+   if (bdf == 0 && addr == 0x72 && val == 0xa) {
+dprintf(1, "stop vcpu\n");
+outb(0xf0, 0x402); // notify qemu to stop vcpu
+dprintf(1, "resume vcpu\n");
+}
 outb(val, PORT_PCI_DATA + (addr & 3));
 }
 }

qemu:hw/char/debugcon.c
@@ -60,6 +61,9 @@ static void debugcon_ioport_write(void *opaque, hwaddr addr, 
uint64_t val,
 printf(" [debugcon: write addr=0x%04" HWADDR_PRIx " val=0x%02" PRIx64 
"]\n", addr, val);
 #endif

+if (ch == 0xf0) {
+vm_stop(RUN_STATE_PAUSED);
+}
 /* XXX this blocks entire thread. Rewrite to use
  * qemu_chr_fe_write and background I/O callbacks */
 qemu_chr_fe_write_all(>chr, , 1);

step 2. start vm1 by the following command line, and then vm stopped.
$ qemu-system-x86_64 -machine pc-i440fx-5.0,accel=kvm\
 -netdev tap,ifname=tap-test,id=hostnet0,vhost=on,downscript=no,script=no\
 -device virtio-net-pci,netdev=hostnet0,id=net0,bus=pci.0,addr=0x13,bootindex=3\
 -device cirrus-vga,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2\
 -chardev file,id=seabios,path=/var/log/test.seabios,append=on\
 -device isa-debugcon,iobase=0x402,chardev=seabios\
 -monitor stdio

step 3. start vm2 to accept vm1 state.
$ qemu-system-x86_64 -machine pc-i440fx-5.0,accel=kvm\
 -netdev tap,ifname=tap-test1,id=hostnet0,vhost=on,downscript=no,script=no\
 -device virtio-net-pci,netdev=hostnet0,id=net0,bus=pci.0,addr=0x13,bootindex=3\
 -device cirrus-vga,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2\
 -chardev file,id=seabios,path=/var/log/test.seabios,append=on\
 -device isa-debugcon,iobase=0x402,chardev=seabios\
 -monitor stdio \
 -incoming tcp:127.0.0.1:8000

step 4. execute the following qmp command in vm1 to migrate.
(qemu) migrate tcp:127.0.0.1:8000

step 5. execute the following qmp command in vm2 to resume vcpu.
(qemu) cont

Before this patch, we get KVM "emulation failure" error on vm2.
This patch fixes it.

Signed-off-by: Hogan Wang 
---
 hw/pci-host/i440fx.c | 46 
 hw/pci-host/q35.c| 44 ++
 2 files changed, 90 insertions(+)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 8ed2417f0c..419e27c21a 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -64,6 +64,14 @@ typedef struct I440FXState {
  */
 #define I440FX_COREBOOT_RAM_SIZE 0x57
 
+/* Older I440FX machines (5.0 and older) do not support i440FX-pcihost state
+ * migration, use some reserved INTEL 82441 configuration registers to
+ * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
+ * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
+ * (DBX) Table 1. PMC Configuration Space]
+ */
+#define I440FX_PCI_HOST_CONFIG_REG 0x94
+
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
 int i;
@@ -98,15 +106,53 @@ static void i440fx_write_config(PCIDevice *dev,
 static int i440fx_post_load(void *opaque, int version_id)
 {
 PCII440FXState *d = opaque;
+PCIDevice *dev;
+PCIHostState *s = OBJECT_CHECK(PCIHostState,
+   object_resolve_path("/machine/i440fx", 
NULL),
+   TYPE_PCI_HOST_BRIDGE);
 
 i440fx_update_memory_mappings(d);
+
+if (!s->mig_enabled) {
+dev = PCI_DEVICE(d);
+s->config_reg = pci_get_long(>config[I440FX_PCI_HOST_CONFIG_REG]);
+pci_set_long(>config[I440FX_PCI_HOST_CONFIG_REG], 0);
+}
+return 0;
+}
+
+static int i440fx_pre_save(void *opaque)
+{
+PCIDevice *dev = opaque;
+PCIHostState *s = OBJECT_CHECK(PCIHostState,
+   object_resolve_path("/machine/i440fx", 
NULL),
+   TYPE_PCI_HOST_BRIDGE);
+if (!s->mig_enabled) {
+pci_set_long(>config[I440FX_PCI_HOST_CONFIG_REG],
+ s->config_reg);
+}
+return 0;
+}
+
+static int i440fx_post_save(void *opaque)
+{
+PCIDevice *dev = opaque;
+PCIHostState *s = OBJECT_CHECK(PCIHostState,
+   object_resolve_path("/machine/i440fx", 
NULL),
+   TYPE_PCI_HOST_BRIDGE);
+if (!s->mig_enabled) {
+pci_set_long(>config[I440FX_PCI_HOST_CONFIG_REG], 0);
+}

Re: [PATCH v5 3/4] target/riscv: Fix the translation of physical address

2020-07-27 Thread Zong Li

On Tue, Jul 28, 2020 at 6:49 AM Alistair Francis  wrote:
>
> On Sat, Jul 25, 2020 at 8:05 AM Zong Li  wrote:
> >
> > The real physical address should add the 12 bits page offset. It also
> > causes the PMP wrong checking due to the minimum granularity of PMP is
> > 4 byte, but we always get the physical address which is 4KB alignment,
> > that means, we always use the start address of the page to check PMP for
> > all addresses which in the same page.
>
> So riscv_cpu_tlb_fill() will clear these bits when calling
> tlb_set_page(), so this won't have an impact on actual translation
> (although it will change in input address for 2-stage translation, but
> that seems fine).
>
> Your point about PMP seems correct as we allow a smaller then page
> granularity this seems like the right approach.
>
> Can you edit riscv_cpu_get_phys_page_debug() to mask these bits out at
> the end? Otherwise we will break what callers to
> cpu_get_phys_page_attrs_debug() expect.
>

OK, I checked that already, the callers would add these bits again,
because they expect to get the address for the page. Thanks for your
reviewing, modify it in the next version.

> Alistair
>
> >
> > Signed-off-by: Zong Li 
> > ---
> >  target/riscv/cpu_helper.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 75d2ae3434..08b069f0c9 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -543,7 +543,8 @@ restart:
> >  /* for superpage mappings, make a fake leaf PTE for the TLB's
> > benefit. */
> >  target_ulong vpn = addr >> PGSHIFT;
> > -*physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
> > +*physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) 
> > |
> > +(addr & ~TARGET_PAGE_MASK);
> >
> >  /* set permissions on the TLB entry */
> >  if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
> > --
> > 2.27.0
> >
> >

Re: [RFC PATCH 1/2] hw/riscv: sifive_u: Add file-backed OTP. softmmu/vl: add otp-file to boot option

2020-07-27 Thread Green Wan

Hi Bin,

Thanks for the reply.

I think we can add property to sifive_u_otp_properties[] (something like
below) and remove generic code dependency. What do you think of it?

@@ -243,6 +245,7 @@ static const MemoryRegionOps sifive_u_otp_ops = {

 static Property sifive_u_otp_properties[] = {
 DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0),
+DEFINE_PROP_STRING("otp_file", SiFiveUOTPState, otp_file),
 DEFINE_PROP_END_OF_LIST(),
 };

 typedef struct SiFiveUOTPState {
 /*< private >*/
 SysBusDevice parent_obj;
@@ -77,6 +75,7 @@ typedef struct SiFiveUOTPState {
 uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
 /* config */
 uint32_t serial;
+char *otp_file;
 uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
 } SiFiveUOTPState;

Regards,
Green


On Fri, Jul 24, 2020 at 10:20 PM Bin Meng  wrote:

> Hi Green,
>
> On Fri, Jul 24, 2020 at 5:51 PM Green Wan  wrote:
> >
> > Add a file-backed implementation for OTP of sifive_u machine. Use
> > '-boot otp-file=xxx' to enable it. Do file open, mmap and close
> > for every OTP read/write in case keep the update-to-date snapshot
> > of OTP.
> >
> > Signed-off-by: Green Wan 
> > ---
> >  hw/riscv/sifive_u_otp.c | 88 -
> >  include/hw/riscv/sifive_u_otp.h |  2 +
> >  qemu-options.hx |  3 +-
> >  softmmu/vl.c|  6 ++-
> >  4 files changed, 96 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
> > index f6ecbaa2ca..26e1965821 100644
> > --- a/hw/riscv/sifive_u_otp.c
> > +++ b/hw/riscv/sifive_u_otp.c
> > @@ -24,6 +24,72 @@
> >  #include "qemu/log.h"
> >  #include "qemu/module.h"
> >  #include "hw/riscv/sifive_u_otp.h"
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define TRACE_PREFIX"FU540_OTP: "
> > +#define SIFIVE_FU540_OTP_SIZE   (SIFIVE_U_OTP_NUM_FUSES * 4)
> > +
> > +static int otp_backed_fd;
> > +static unsigned int *otp_mmap;
> > +
> > +static void sifive_u_otp_backed_load(const char *filename);
> > +static uint64_t sifive_u_otp_backed_read(uint32_t fuseidx);
> > +static void sifive_u_otp_backed_write(uint32_t fuseidx,
> > +  uint32_t paio,
> > +  uint32_t pdin);
> > +static void sifive_u_otp_backed_unload(void);
> > +
> > +void sifive_u_otp_backed_load(const char *filename)
> > +{
> > +if (otp_backed_fd < 0) {
> > +
> > +otp_backed_fd = open(filename, O_RDWR);
> > +
> > +if (otp_backed_fd < 0)
> > +qemu_log_mask(LOG_TRACE,
> > +  TRACE_PREFIX "Warning: can't open otp
> file\n");
> > +else {
> > +
> > +otp_mmap = (unsigned int *)mmap(0,
> > +SIFIVE_FU540_OTP_SIZE,
> > +PROT_READ | PROT_WRITE |
> PROT_EXEC,
> > +MAP_FILE | MAP_SHARED,
> > +otp_backed_fd,
> > +0);
> > +
> > +if (otp_mmap == MAP_FAILED)
> > +qemu_log_mask(LOG_TRACE,
> > +  TRACE_PREFIX "Warning: can't mmap otp
> file\n");
> > +}
> > +}
> > +
> > +}
> > +
> > +uint64_t sifive_u_otp_backed_read(uint32_t fuseidx)
> > +{
> > +return (uint64_t)(otp_mmap[fuseidx]);
> > +}
> > +
> > +void sifive_u_otp_backed_write(uint32_t fuseidx, uint32_t paio,
> uint32_t pdin)
> > +{
> > +otp_mmap[fuseidx] &= ~(pdin << paio);
> > +otp_mmap[fuseidx] |= (pdin << paio);
> > +}
> > +
> > +
> > +void sifive_u_otp_backed_unload(void)
> > +{
> > +munmap(otp_mmap, SIFIVE_FU540_OTP_SIZE);
> > +close(otp_backed_fd);
> > +otp_backed_fd = -1;
> > +}
> >
> >  static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned
> int size)
> >  {
> > @@ -46,7 +112,17 @@ static uint64_t sifive_u_otp_read(void *opaque,
> hwaddr addr, unsigned int size)
> >  if ((s->pce & SIFIVE_U_OTP_PCE_EN) &&
> >  (s->pdstb & SIFIVE_U_OTP_PDSTB_EN) &&
> >  (s->ptrim & SIFIVE_U_OTP_PTRIM_EN)) {
> > -return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
> > +
> > +if (otp_file) {
> > +uint64_t val;
> > +
> > +sifive_u_otp_backed_load(otp_file);
> > +val = sifive_u_otp_backed_read(s->pa);
> > +sifive_u_otp_backed_unload();
> > +
> > +return val;
> > +} else
> > +return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
> >  } else {
> >  return 0xff;
> >  }
> > @@ -123,6 +199,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr
> addr,
> >  s->ptrim = val32;
> >  break;
> >  case SIFIVE_U_OTP_PWE:
> > +if (otp_file) {
> > +

[Bug 1390520] Re: virtual machine fails to start with connected audio cd

Dropping from my queue due to capacity.

** Changed in: qemu
 Assignee: John Snow (jnsnow) => (unassigned)

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

Title:
  virtual machine fails to start with connected audio cd

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in libvirt source package in Trusty:
  Won't Fix

Bug description:
  when connecting a data cd with a virtual machine (IDE CDROM 1), the virtual 
machine starts up and the data cd is accessable (for example to install 
software package or drivers),
  but connecting an audio cd the following error appears:

  
---
  cannot read header '/dev/sr0': Input/output error

  Traceback (most recent call last):
File "/usr/share/virt-manager/virtManager/details.py", line 2530, in 
_change_config_helper
  func(*args)
File "/usr/share/virt-manager/virtManager/domain.py", line 850, in 
hotplug_storage_media
  self.attach_device(devobj)
File "/usr/share/virt-manager/virtManager/domain.py", line 798, in 
attach_device
  self._backend.attachDevice(devxml)
File "/usr/lib/python2.7/dist-packages/libvirt.py", line 493, in 
attachDevice
  if ret == -1: raise libvirtError ('virDomainAttachDevice() failed', 
dom=self)
  libvirtError: cannot read header '/dev/sr0': Input/output error
  


  Description:Ubuntu 14.04.1 LTS
  Release:14.04

  qemu:
Installiert:   2.0.0+dfsg-2ubuntu1.6
Installationskandidat: 2.0.0+dfsg-2ubuntu1.6

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

[Bug 1070762] Re: savevm fails with inserted CD, "Device '%s' is writable but does not support snapshots."

Very old bug. If anyone sees this behavior, please re-file against a
supported release (5.0 at time of writing, soon to be 5.1) and please
paste a full command-line and steps to reproduce.

(To my knowledge, this bug is not present in modern QEMU builds, but do
not know when it would have changed.)

--js

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

** Changed in: qemu
 Assignee: John Snow (jnsnow) => (unassigned)

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

Title:
  savevm fails with inserted CD, "Device '%s' is writable but does not
  support  snapshots."

Status in QEMU:
  Incomplete

Bug description:
  Hi,

  yesterday unfortunately a customer reported a failed snapshot of his
  VM. Going through the logfile I discovered:

  "Device 'ide1-cd0' is writable but does not support snapshots"

  this is with qemu-1.2.0 and 1.0.1 at least...

  Why writeable?
  Even if I specify "-drive ...,readonly=on,snapshot=off" to qemu the 
monitor-command sees the CD-ROM-device as being writeable?!

  Somewhere I saw a "hint" for blockdev.c:
  === snip ===

  --- /tmp/blockdev.c   2012-10-24 11:37:10.0 +0200
  +++ blockdev.c2012-10-24 11:37:17.0 +0200
  @@ -551,6 +551,7 @@
   case IF_XEN:
   case IF_NONE:
   dinfo->media_cd = media == MEDIA_CDROM;
  + dinfo->bdrv->read_only = 1;
   break;
   case IF_SD:
   case IF_FLOPPY:

  === snap ===

  after installing with this small patch applied it works, so insert CD, savevm 
 succeeds.
  This should be fixed at all correct places, and the tags 
"readonly=on,snapshot=off" should do it, too. Or even just work after 
specifying a drive being a CD-rom should do the trick ;-)

  Another "bad habit" is, that the ISO/DVD-file has to be writeable to
  be changed?

  Thnx for attention and regards,

  Oliver.

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

[Bug 1777315] Re: IDE short PRDT abort

** Summary changed:

- Denial of service
+ IDE short PRDT abort

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

Title:
  IDE short PRDT abort

Status in QEMU:
  In Progress

Bug description:
  Hi,
  QEMU 'hw/ide/core.c:871' Denial of Service Vulnerability in version 
qemu-2.12.0

  run the program in qemu-2.12.0:
  #define _GNU_SOURCE 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 

  static uintptr_t syz_open_dev(uintptr_t a0, uintptr_t a1, uintptr_t a2)
  {
  if (a0 == 0xc || a0 == 0xb) {
  char buf[128];
  sprintf(buf, "/dev/%s/%d:%d", a0 == 0xc ? "char" : "block", 
(uint8_t)a1, (uint8_t)a2);
  return open(buf, O_RDWR, 0);
  } else {
  char buf[1024];
  char* hash;
  strncpy(buf, (char*)a0, sizeof(buf) - 1);
  buf[sizeof(buf) - 1] = 0;
  while ((hash = strchr(buf, '#'))) {
  *hash = '0' + (char)(a1 % 10);
  a1 /= 10;
  }
  return open(buf, a2, 0);
  }
  }

  uint64_t r[2] = {0x, 0x};
  void loop()
  {
  long res = 0;
  memcpy((void*)0x2000, "/dev/sg#", 9);
  res = syz_open_dev(0x2000, 0, 2);
  if (res != -1)
  r[0] = res;
  res = syscall(__NR_dup2, r[0], r[0]);
  if (res != -1)
  r[1] = res;
  *(uint8_t*)0x2ec0 = 0;
  *(uint8_t*)0x2ec1 = 0;
  *(uint8_t*)0x2ec2 = 0;
  *(uint8_t*)0x2ec3 = 0;
  *(uint32_t*)0x2ec8 = 0;
  *(uint8_t*)0x2ed8 = 0;
  *(uint8_t*)0x2ed9 = 0;
  *(uint8_t*)0x2eda = 0;
  *(uint8_t*)0x2edb = 0;
  memcpy((void*)0x2ee0, "\x9c\x4d\xe7\xd5\x0a\x62\x43\xa7\x77\x53\x67\xb3", 
12);
  syscall(__NR_write, r[1], 0x2ec0, 0x323);
  }

  int main()
  {
  syscall(__NR_mmap, 0x2000, 0x100, 3, 0x32, -1, 0);
  loop();
  return 0;
  }
  this will crash qemu, output information:
   qemu-system-x86_64: hw/ide/core.c:843: ide_dma_cb: Assertion `n * 512 == 
s->sg.size' failed.

  
  Thanks 
  owl337

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

Re: [PATCH] bugfix: irq: Avoid covering object refcount of qemu_irq

2020-07-27 Thread zhukeqian

Hi Peter,

On 2020/7/27 22:41, Peter Maydell wrote:
> On Mon, 27 Jul 2020 at 14:03, Keqian Zhu  wrote:
>>
>> Avoid covering object refcount of qemu_irq, otherwise it may causes
>> memory leak.
>>
>> Signed-off-by: Keqian Zhu 
>> ---
>>  hw/core/irq.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/irq.c b/hw/core/irq.c
>> index fb3045b912..59af4dfc74 100644
>> --- a/hw/core/irq.c
>> +++ b/hw/core/irq.c
>> @@ -125,7 +125,9 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, 
>> qemu_irq_handler handler, int n)
>>  int i;
>>  qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n);
>>  for (i = 0; i < n; i++) {
>> -*old_irqs[i] = *gpio_in[i];
>> +old_irqs[i]->handler = gpio_in[i]->handler;
>> +old_irqs[i]->opaque = gpio_in[i]->opaque;
>> +
>>  gpio_in[i]->handler = handler;
>>  gpio_in[i]->opaque = _irqs[i];
>>  }
> 
> This function is leaky by design, because it doesn't do anything
> with the old_irqs array and there's no function for un-intercepting
> the IRQs (which would need to free that memory). This is not ideal
> but OK because it's only used in the test suite.
One of our internal self-developed module also use this function, and we
implemented a function to remove intercepting, so there is no memory leak
after this bugfix.

I suggest to merge this bugfix to prepare for future code which may invoke
this function.

> 
> Is there a specific bug you're trying to fix here?
The memory leak is reported by ASAN.
> 

Thanks,
Keqian
> thanks
> -- PMM
> .
>

[Bug 1883739] Re: ide_dma_cb: Assertion `prep_size >= 0 && prep_size <= n * 512' failed.

*** This bug is a duplicate of bug 1777315 ***
https://bugs.launchpad.net/bugs/1777315

** This bug has been marked a duplicate of bug 1777315
   Denial of service

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

Title:
  ide_dma_cb: Assertion `prep_size >= 0 && prep_size <= n * 512' failed.

Status in QEMU:
  Confirmed

Bug description:
  To reproduce run the QEMU with the following command line:
  ```
  qemu-system-x86_64 -cdrom hypertrash.iso -nographic -m 100 -enable-kvm -net 
none -drive id=disk,file=hda.img,if=none -device ahci,id=ahci -device 
ide-hd,drive=disk,bus=ahci.0
  ```

  QEMU Version:
  ```
  # qemu-5.0.0
  $ ./configure --target-list=x86_64-softmmu --enable-sanitizers; make
  $ x86_64-softmmu/qemu-system-x86_64 --version
  QEMU emulator version 5.0.0
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers
  ```

  To create disk image run:
  ```
  dd if=/dev/zero of=hda.img bs=1024 count=1024
  ```

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

[Bug 1681439] Re: qemu-system-x86_64: hw/ide/core.c:685: ide_cancel_dma_sync: Assertion `s->bus->dma->aiocb == NULL' failed.

** Changed in: qemu
   Status: Confirmed => In Progress

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

Title:
  qemu-system-x86_64: hw/ide/core.c:685: ide_cancel_dma_sync: Assertion
  `s->bus->dma->aiocb == NULL' failed.

Status in QEMU:
  In Progress

Bug description:
  Since upgrading to QEMU 2.8.0, my Windows 7 64-bit virtual machines
  started crashing due to the assertion quoted in the summary failing.
  The assertion in question was added by commit 9972354856 ("block: add
  BDS field to count in-flight requests").  My tests show that setting
  discard=unmap is needed to reproduce the issue.  Speaking of
  reproduction, it is a bit flaky, because I have been unable to come up
  with specific instructions that would allow the issue to be triggered
  outside of my environment, but I do have a semi-sane way of testing that
  appears to depend on a specific initial state of data on the underlying
  storage volume, actions taken within the VM and waiting for about 20
  minutes.

  Here is the shortest QEMU command line that I managed to reproduce the
  bug with:

  qemu-system-x86_64 \
  -machine pc-i440fx-2.7,accel=kvm \
  -m 3072 \
  -drive file=/dev/lvm/qemu,format=raw,if=ide,discard=unmap \
-netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no,vhost=on \
  -device virtio-net-pci,netdev=hostnet0 \
-vnc :0

  The underlying storage (/dev/lvm/qemu) is a thin LVM snapshot.

  QEMU was compiled using:

  ./configure --python=/usr/bin/python2.7 --target-list=x86_64-softmmu
  make -j3

  My virtualization environment is not really a critical one and
  reproduction is not that much of a hassle, so if you need me to gather
  further diagnostic information or test patches, I will be happy to help.

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

Re: [PATCH] bugfix: irq: Avoid covering object refcount of qemu_irq

2020-07-27 Thread zhukeqian

Hi Qiang,

On 2020/7/27 22:37, Li Qiang wrote:
> Keqian Zhu  于2020年7月27日周一 下午9:03写道：
>>
>> Avoid covering object refcount of qemu_irq, otherwise it may causes
>> memory leak.
> 
> Any reproducer?
> 
In mainline Qemu. this function is only used in qtest. One of our internal
self-developed module also use this function. The memory leak is reported
by ASAN.

Thanks,
Keqian

> Thanks,
> Li Qiang
> 
>>
>> Signed-off-by: Keqian Zhu 
>> ---
>>  hw/core/irq.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/irq.c b/hw/core/irq.c
>> index fb3045b912..59af4dfc74 100644
>> --- a/hw/core/irq.c
>> +++ b/hw/core/irq.c
>> @@ -125,7 +125,9 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, 
>> qemu_irq_handler handler, int n)
>>  int i;
>>  qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n);
>>  for (i = 0; i < n; i++) {
>> -*old_irqs[i] = *gpio_in[i];
>> +old_irqs[i]->handler = gpio_in[i]->handler;
>> +old_irqs[i]->opaque = gpio_in[i]->opaque;
>> +
>>  gpio_in[i]->handler = handler;
>>  gpio_in[i]->opaque = _irqs[i];
>>  }
>> --
>> 2.19.1
>>
> .
>

[Bug 1681439] Re: qemu-system-x86_64: hw/ide/core.c:685: ide_cancel_dma_sync: Assertion `s->bus->dma->aiocb == NULL' failed.

The qtest reproducers are so nice.

writel 0x0 0x

outw 0x171 0x32a
  features := 0x2ab8cb
  count := 0x03;  b8cb
outw 0x176 0x3570
  device := 0x70 (select device1)   b8cb
  command := 0x35(DMA WRITE EXT)8f98

outl 0xcf8 0x8903
outl 0xcfc 0x4e002700
outl 0xcf8 0x8920
outb 0xcfc 0x5e

outb 0x58 0xe1
  bmdma_cmd_writeb val = 0xe1 [1110 0001]
   DMA READ ^  ^ DMA Start
outw 0x57 0x0
  bmdma_cmd_writeb val = 0x00 [ ]
   ^ DMA Cancel
EOF


This should be a straightforward DMA cancel. I added some more traces;

# After the 0x35 command write:
ide_exec_cmd IDE exec cmd: bus 0x561808b0ecc0; state 0x561808b0f118; cmd 0x35
ide_sector_start_dma IDEState 0x561808b0f118;
ide_start_dma IDEState 0x561808b0f118;

# After the 0xe1 bmdma kick:
ide_dma_cb_entry IDEState 0x561808b0f118; ret 0;
ide_dma_cb IDEState 0x561808b0f118; sector_num=1 n=259 cmd=DMA WRITE
ide_dma_cb_next IDEState 0x561808b0f118;

So far, pretty normal. IDE calls the HBA's DMA start, but the HBA
doesn't have DMA enabled, so it stalls. Later, when we turn on DMA, the
HBA engages the DMA callback and sets up the first transfer. This sets
s->bus->dma->aiocb.

Then, we try to cancel DMA:

ide_cancel_dma_sync IDEState 0x561808b0f118;
ide_cancel_dma_sync_remaining draining all remaining requests
1343877@1595891049.469050:dma_blk_cb dbs=0x55baededdc50 ret=0
1343877@1595891049.469054:dma_map_wait dbs=0x55baededdc50
qemu-system-i386: /home/jsnow/src/qemu/hw/ide/core.c:732: void 
ide_cancel_dma_sync(IDEState *): Assertion `s->bus->dma->aiocb == NULL' failed.

We still have a DMA callback out, so we try to synchronously cancel it;
but the blk_drain doesn't appear to be effective!

We apparently wind up here:

if (dbs->iov.size == 0) {
trace_dma_map_wait(dbs);
dbs->bh = aio_bh_new(dbs->ctx, reschedule_dma, dbs);
cpu_register_map_client(dbs->bh);
return;
}


... The DMA simply re-schedules itself (?) when iov.size is zero. unfortunately 
for us, that means that the original point of scheduling the drain doesn't 
work, because the DMA never returns all the way to the IDE device emulation 
code.

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

Title:
  qemu-system-x86_64: hw/ide/core.c:685: ide_cancel_dma_sync: Assertion
  `s->bus->dma->aiocb == NULL' failed.

Status in QEMU:
  Confirmed

Bug description:
  Since upgrading to QEMU 2.8.0, my Windows 7 64-bit virtual machines
  started crashing due to the assertion quoted in the summary failing.
  The assertion in question was added by commit 9972354856 ("block: add
  BDS field to count in-flight requests").  My tests show that setting
  discard=unmap is needed to reproduce the issue.  Speaking of
  reproduction, it is a bit flaky, because I have been unable to come up
  with specific instructions that would allow the issue to be triggered
  outside of my environment, but I do have a semi-sane way of testing that
  appears to depend on a specific initial state of data on the underlying
  storage volume, actions taken within the VM and waiting for about 20
  minutes.

  Here is the shortest QEMU command line that I managed to reproduce the
  bug with:

  qemu-system-x86_64 \
  -machine pc-i440fx-2.7,accel=kvm \
  -m 3072 \
  -drive file=/dev/lvm/qemu,format=raw,if=ide,discard=unmap \
-netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no,vhost=on \
  -device virtio-net-pci,netdev=hostnet0 \
-vnc :0

  The underlying storage (/dev/lvm/qemu) is a thin LVM snapshot.

  QEMU was compiled using:

  ./configure --python=/usr/bin/python2.7 --target-list=x86_64-softmmu
  make -j3

  My virtualization environment is not really a critical one and
  reproduction is not that much of a hassle, so if you need me to gather
  further diagnostic information or test patches, I will be happy to help.

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

[PATCH 2/6 v3] KVM: SVM: Fill in conforming svm_x86_ops via macro

The names of some of the svm_x86_ops functions do not have a corresponding
'svm_' prefix. Generate the names using a macro so that the names are
conformant. Fixing the naming will help in better readability and
maintenance of the code.

Suggested-by: Vitaly Kuznetsov 
Suggested-by: Paolo Bonzini 
Signed-off-by: Sean Christopherson 
Signed-off-by: Krish Sadhukhan 
---
 arch/x86/kvm/svm/avic.c   |   4 +-
 arch/x86/kvm/svm/nested.c |   2 +-
 arch/x86/kvm/svm/sev.c|   6 +-
 arch/x86/kvm/svm/svm.c| 218 +++---
 arch/x86/kvm/svm/svm.h|   8 +-
 5 files changed, 120 insertions(+), 118 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index e80daa9..619391e 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -579,7 +579,7 @@ int avic_init_vcpu(struct vcpu_svm *svm)
return ret;
 }
 
-void avic_post_state_restore(struct kvm_vcpu *vcpu)
+void svm_avic_post_state_restore(struct kvm_vcpu *vcpu)
 {
if (avic_handle_apic_id_update(vcpu) != 0)
return;
@@ -660,7 +660,7 @@ void svm_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
 * we need to check and update the AVIC logical APIC ID table
 * accordingly before re-activating.
 */
-   avic_post_state_restore(vcpu);
+   svm_avic_post_state_restore(vcpu);
vmcb->control.int_ctl |= AVIC_ENABLE_MASK;
} else {
vmcb->control.int_ctl &= ~AVIC_ENABLE_MASK;
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 6bceafb..3be6256 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -348,7 +348,7 @@ static void nested_prepare_vmcb_control(struct vcpu_svm 
*svm)
/* Guest paging mode is active - reset mmu */
kvm_mmu_reset_context(>vcpu);
 
-   svm_flush_tlb(>vcpu);
+   svm_tlb_flush(>vcpu);
 
svm->vmcb->control.tsc_offset = svm->vcpu.arch.tsc_offset =
svm->vcpu.arch.l1_tsc_offset + svm->nested.ctl.tsc_offset;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 5573a97..1ca9f60 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -969,7 +969,7 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
return r;
 }
 
-int svm_register_enc_region(struct kvm *kvm,
+int svm_mem_enc_register_region(struct kvm *kvm,
struct kvm_enc_region *range)
 {
struct kvm_sev_info *sev = _kvm_svm(kvm)->sev_info;
@@ -1038,8 +1038,8 @@ static void __unregister_enc_region_locked(struct kvm 
*kvm,
kfree(region);
 }
 
-int svm_unregister_enc_region(struct kvm *kvm,
- struct kvm_enc_region *range)
+int svm_mem_enc_unregister_region(struct kvm *kvm,
+ struct kvm_enc_region *range)
 {
struct enc_region *region;
int ret;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 24755eb..d63181e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -254,7 +254,7 @@ static inline void invlpga(unsigned long addr, u32 asid)
asm volatile (__ex("invlpga %1, %0") : : "c"(asid), "a"(addr));
 }
 
-static int get_npt_level(struct kvm_vcpu *vcpu)
+static int svm_get_tdp_level(struct kvm_vcpu *vcpu)
 {
 #ifdef CONFIG_X86_64
return PT64_ROOT_4LEVEL;
@@ -312,7 +312,7 @@ static void svm_set_interrupt_shadow(struct kvm_vcpu *vcpu, 
int mask)
 
 }
 
-static int skip_emulated_instruction(struct kvm_vcpu *vcpu)
+static int svm_skip_emulated_instruction(struct kvm_vcpu *vcpu)
 {
struct vcpu_svm *svm = to_svm(vcpu);
 
@@ -351,7 +351,7 @@ static void svm_queue_exception(struct kvm_vcpu *vcpu)
 * raises a fault that is not intercepted. Still better than
 * failing in all cases.
 */
-   (void)skip_emulated_instruction(>vcpu);
+   (void)svm_skip_emulated_instruction(>vcpu);
rip = kvm_rip_read(>vcpu);
svm->int3_rip = rip + svm->vmcb->save.cs.base;
svm->int3_injected = rip - old_rip;
@@ -1153,7 +1153,7 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool 
init_event)
avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
 }
 
-static int svm_create_vcpu(struct kvm_vcpu *vcpu)
+static int svm_vcpu_create(struct kvm_vcpu *vcpu)
 {
struct vcpu_svm *svm;
struct page *page;
@@ -1232,7 +1232,7 @@ static void svm_clear_current_vmcb(struct vmcb *vmcb)
cmpxchg(_cpu(svm_data, i)->current_vmcb, vmcb, NULL);
 }
 
-static void svm_free_vcpu(struct kvm_vcpu *vcpu)
+static void svm_vcpu_free(struct kvm_vcpu *vcpu)
 {
struct vcpu_svm *svm = to_svm(vcpu);
 
@@ -1585,7 +1585,7 @@ int svm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
return 1;
 
if (npt_enabled && ((old_cr4 ^ cr4) & X86_CR4_PGE))
-   svm_flush_tlb(vcpu);
+

[PATCH 4/6 v3] KVM: VMX: Fill in conforming vmx_x86_ops via macro

The names of some of the vmx_x86_ops functions do not have a corresponding
'vmx_' prefix. Generate the names using a macro so that the names are
conformant. Fixing the naming will help in better readability and
maintenance of the code.

Suggested-by: Vitaly Kuznetsov 
Suggested-by: Paolo Bonzini 
Signed-off-by: Sean Christopherson 
Signed-off-by: Krish Sadhukhan 
---
 arch/x86/kvm/vmx/nested.c |   2 +-
 arch/x86/kvm/vmx/vmx.c| 234 +++---
 arch/x86/kvm/vmx/vmx.h|   2 +-
 3 files changed, 120 insertions(+), 118 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index d1af20b..a898b53 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3016,7 +3016,7 @@ static int nested_vmx_check_vmentry_hw(struct kvm_vcpu 
*vcpu)
 
preempt_disable();
 
-   vmx_prepare_switch_to_guest(vcpu);
+   vmx_prepare_guest_switch(vcpu);
 
/*
 * Induce a consistency check VMExit by clearing bit 1 in GUEST_RFLAGS,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 90d91524..f6a6674 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1125,7 +1125,7 @@ void vmx_set_host_fs_gs(struct vmcs_host_state *host, u16 
fs_sel, u16 gs_sel,
}
 }
 
-void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
+void vmx_prepare_guest_switch(struct kvm_vcpu *vcpu)
 {
struct vcpu_vmx *vmx = to_vmx(vcpu);
struct vmcs_host_state *host_state;
@@ -2317,7 +2317,7 @@ static int kvm_cpu_vmxon(u64 vmxon_pointer)
return -EFAULT;
 }
 
-static int hardware_enable(void)
+static int vmx_hardware_enable(void)
 {
int cpu = raw_smp_processor_id();
u64 phys_addr = __pa(per_cpu(vmxarea, cpu));
@@ -2366,7 +2366,7 @@ static void kvm_cpu_vmxoff(void)
cr4_clear_bits(X86_CR4_VMXE);
 }
 
-static void hardware_disable(void)
+static void vmx_hardware_disable(void)
 {
vmclear_local_loaded_vmcss();
kvm_cpu_vmxoff();
@@ -2911,7 +2911,7 @@ static void exit_lmode(struct kvm_vcpu *vcpu)
 
 #endif
 
-static void vmx_flush_tlb_all(struct kvm_vcpu *vcpu)
+static void vmx_tlb_flush_all(struct kvm_vcpu *vcpu)
 {
struct vcpu_vmx *vmx = to_vmx(vcpu);
 
@@ -2934,7 +2934,7 @@ static void vmx_flush_tlb_all(struct kvm_vcpu *vcpu)
}
 }
 
-static void vmx_flush_tlb_current(struct kvm_vcpu *vcpu)
+static void vmx_tlb_flush_current(struct kvm_vcpu *vcpu)
 {
u64 root_hpa = vcpu->arch.mmu->root_hpa;
 
@@ -2950,16 +2950,16 @@ static void vmx_flush_tlb_current(struct kvm_vcpu *vcpu)
vpid_sync_context(nested_get_vpid02(vcpu));
 }
 
-static void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr)
+static void vmx_tlb_flush_gva(struct kvm_vcpu *vcpu, gva_t addr)
 {
/*
 * vpid_sync_vcpu_addr() is a nop if vmx->vpid==0, see the comment in
-* vmx_flush_tlb_guest() for an explanation of why this is ok.
+* vmx_tlb_flush_guest() for an explanation of why this is ok.
 */
vpid_sync_vcpu_addr(to_vmx(vcpu)->vpid, addr);
 }
 
-static void vmx_flush_tlb_guest(struct kvm_vcpu *vcpu)
+static void vmx_tlb_flush_guest(struct kvm_vcpu *vcpu)
 {
/*
 * vpid_sync_context() is a nop if vmx->vpid==0, e.g. if enable_vpid==0
@@ -4455,16 +4455,16 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool 
init_event)
vmx_clear_hlt(vcpu);
 }
 
-static void enable_irq_window(struct kvm_vcpu *vcpu)
+static void vmx_enable_irq_window(struct kvm_vcpu *vcpu)
 {
exec_controls_setbit(to_vmx(vcpu), CPU_BASED_INTR_WINDOW_EXITING);
 }
 
-static void enable_nmi_window(struct kvm_vcpu *vcpu)
+static void vmx_enable_nmi_window(struct kvm_vcpu *vcpu)
 {
if (!enable_vnmi ||
vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_STI) {
-   enable_irq_window(vcpu);
+   vmx_enable_irq_window(vcpu);
return;
}
 
@@ -6173,7 +6173,7 @@ static void vmx_l1d_flush(struct kvm_vcpu *vcpu)
: "eax", "ebx", "ecx", "edx");
 }
 
-static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
+static void vmx_update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 {
struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
int tpr_threshold;
@@ -6261,7 +6261,7 @@ static void vmx_set_apic_access_page_addr(struct kvm_vcpu 
*vcpu)
return;
 
vmcs_write64(APIC_ACCESS_ADDR, page_to_phys(page));
-   vmx_flush_tlb_current(vcpu);
+   vmx_tlb_flush_current(vcpu);
 
/*
 * Do not pin apic access page in memory, the MMU notifier
@@ -6837,7 +6837,7 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
return exit_fastpath;
 }
 
-static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
+static void vmx_vcpu_free(struct kvm_vcpu *vcpu)
 {
struct vcpu_vmx *vmx = to_vmx(vcpu);
 
@@ -6848,7 +6848,7 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)

[PATCH 6/6 v3] QEMU: x86: Change KVM_MEMORY_ENCRYPT_* #defines to make them conformant to the kernel

Suggested-by: Vitaly Kuznetsov 
Suggested-by: Paolo Bonzini 
Signed-off-by: Sean Christopherson 
Signed-off-by: Krish Sadhukhan 
---
 target/i386/sev.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index c3ecf86..0913782 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -113,7 +113,7 @@ sev_ioctl(int fd, int cmd, void *data, int *error)
 input.sev_fd = fd;
 input.data = (__u64)(unsigned long)data;
 
-r = kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_OP, );
+r = kvm_vm_ioctl(kvm_state, KVM_MEM_ENC_OP, );
 
 if (error) {
 *error = input.error;
@@ -187,7 +187,7 @@ sev_ram_block_added(RAMBlockNotifier *n, void *host, size_t 
size)
 range.size = size;
 
 trace_kvm_memcrypt_register_region(host, size);
-r = kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_REG_REGION, );
+r = kvm_vm_ioctl(kvm_state, KVM_MEM_ENC_REGISTER_REGION, );
 if (r) {
 error_report("%s: failed to register region (%p+%#zx) error '%s'",
  __func__, host, size, strerror(errno));
@@ -216,7 +216,7 @@ sev_ram_block_removed(RAMBlockNotifier *n, void *host, 
size_t size)
 range.size = size;
 
 trace_kvm_memcrypt_unregister_region(host, size);
-r = kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_UNREG_REGION, );
+r = kvm_vm_ioctl(kvm_state, KVM_MEM_ENC_UNREGISTER_REGION, );
 if (r) {
 error_report("%s: failed to unregister region (%p+%#zx)",
  __func__, host, size);
@@ -454,7 +454,7 @@ sev_get_capabilities(Error **errp)
 error_setg(errp, "KVM not enabled");
 return NULL;
 }
-if (kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_OP, NULL) < 0) {
+if (kvm_vm_ioctl(kvm_state, KVM_MEM_ENC_OP, NULL) < 0) {
 error_setg(errp, "SEV is not enabled in KVM");
 return NULL;
 }
-- 
1.8.3.1

Re: [PATCH] docs/nvdimm: add 'pmem=on' for the device dax backend file

2020-07-27 Thread Liu, Jingqi


Hi Paolo,

Any comments for this patch ?

Thanks,

Jingqi

On 7/15/2020 10:54 AM, Liu, Jingqi wrote:

At the end of live migration, QEMU uses msync() to flush the data to
the backend storage. When the backend file is a character device dax,
the pages explicitly avoid the page cache. It will return failure from msync().
The following warning is output.

 "warning: qemu_ram_msync: failed to sync memory range“

So we add 'pmem=on' to avoid calling msync(), use the QEMU command line:

 -object memory-backend-file,id=mem1,pmem=on,mem-path=/dev/dax0.0,size=4G

Signed-off-by: Jingqi Liu 
---
  docs/nvdimm.txt | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
index c2c6e441b3..31048aff5e 100644
--- a/docs/nvdimm.txt
+++ b/docs/nvdimm.txt
@@ -243,6 +243,13 @@ use the QEMU command line:
  
  -object memory-backend-file,id=nv_mem,mem-path=/XXX/yyy,size=4G,pmem=on
  
+At the end of live migration, QEMU uses msync() to flush the data to the

+backend storage. When the backend file is a character device dax, the pages
+explicitly avoid the page cache. It will return failure from msync().
+So we add 'pmem=on' to avoid calling msync(), use the QEMU command line:
+
+-object memory-backend-file,id=mem1,pmem=on,mem-path=/dev/dax0.0,size=4G
+
  References
  --

[PATCH 1/6 v3] KVM: x86: Change names of some of the kvm_x86_ops functions to make them more semantical and readable

Suggested-by: Vitaly Kuznetsov 
Suggested-by: Paolo Bonzini 
Signed-off-by: Sean Christopherson 
Signed-off-by: Krish Sadhukhan 
---
 arch/arm64/include/asm/kvm_host.h   |  2 +-
 arch/mips/include/asm/kvm_host.h|  2 +-
 arch/powerpc/include/asm/kvm_host.h |  2 +-
 arch/s390/kvm/kvm-s390.c|  2 +-
 arch/x86/include/asm/kvm_host.h | 12 ++--
 arch/x86/kvm/svm/svm.c  | 12 ++--
 arch/x86/kvm/vmx/vmx.c  |  8 
 arch/x86/kvm/x86.c  | 28 ++--
 include/linux/kvm_host.h|  2 +-
 include/uapi/linux/kvm.h|  6 +++---
 tools/include/uapi/linux/kvm.h  |  6 +++---
 virt/kvm/kvm_main.c |  4 ++--
 12 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index c3e6fcc6..f5be4fa 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -545,7 +545,7 @@ static inline bool kvm_arch_requires_vhe(void)
 
 void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu);
 
-static inline void kvm_arch_hardware_unsetup(void) {}
+static inline void kvm_arch_hardware_teardown(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
 static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 363e7a89..95cea05 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -1178,7 +1178,7 @@ extern int kvm_mips_trans_mtc0(union mips_instruction 
inst, u32 *opc,
 extern int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
 struct kvm_mips_interrupt *irq);
 
-static inline void kvm_arch_hardware_unsetup(void) {}
+static inline void kvm_arch_hardware_teardown(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_free_memslot(struct kvm *kvm,
 struct kvm_memory_slot *slot) {}
diff --git a/arch/powerpc/include/asm/kvm_host.h 
b/arch/powerpc/include/asm/kvm_host.h
index 7e2d061..892b0e2 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -856,7 +856,7 @@ struct kvm_vcpu_arch {
 #define __KVM_HAVE_CREATE_DEVICE
 
 static inline void kvm_arch_hardware_disable(void) {}
-static inline void kvm_arch_hardware_unsetup(void) {}
+static inline void kvm_arch_hardware_teardown(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen) {}
 static inline void kvm_arch_flush_shadow_all(struct kvm *kvm) {}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d47c197..5c9 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -312,7 +312,7 @@ int kvm_arch_hardware_setup(void *opaque)
return 0;
 }
 
-void kvm_arch_hardware_unsetup(void)
+void kvm_arch_hardware_teardown(void)
 {
gmap_unregister_pte_notifier(_notifier);
gmap_unregister_pte_notifier(_gmap_notifier);
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index be5363b..ccad66d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1080,7 +1080,7 @@ static inline u16 kvm_lapic_irq_dest_mode(bool 
dest_mode_logical)
 struct kvm_x86_ops {
int (*hardware_enable)(void);
void (*hardware_disable)(void);
-   void (*hardware_unsetup)(void);
+   void (*hardware_teardown)(void);
bool (*cpu_has_accelerated_tpr)(void);
bool (*has_emulated_msr)(u32 index);
void (*cpuid_update)(struct kvm_vcpu *vcpu);
@@ -1141,7 +1141,7 @@ struct kvm_x86_ops {
 */
void (*tlb_flush_guest)(struct kvm_vcpu *vcpu);
 
-   enum exit_fastpath_completion (*run)(struct kvm_vcpu *vcpu);
+   enum exit_fastpath_completion (*vcpu_run)(struct kvm_vcpu *vcpu);
int (*handle_exit)(struct kvm_vcpu *vcpu,
enum exit_fastpath_completion exit_fastpath);
int (*skip_emulated_instruction)(struct kvm_vcpu *vcpu);
@@ -1150,8 +1150,8 @@ struct kvm_x86_ops {
u32 (*get_interrupt_shadow)(struct kvm_vcpu *vcpu);
void (*patch_hypercall)(struct kvm_vcpu *vcpu,
unsigned char *hypercall_addr);
-   void (*set_irq)(struct kvm_vcpu *vcpu);
-   void (*set_nmi)(struct kvm_vcpu *vcpu);
+   void (*inject_irq)(struct kvm_vcpu *vcpu);
+   void (*inject_nmi)(struct kvm_vcpu *vcpu);
void (*queue_exception)(struct kvm_vcpu *vcpu);
void (*cancel_injection)(struct kvm_vcpu *vcpu);
int (*interrupt_allowed)(struct kvm_vcpu *vcpu, bool for_injection);
@@ -1258,8 +1258,8 @@ struct kvm_x86_ops {
void (*enable_smi_window)(struct kvm_vcpu *vcpu);
 
int (*mem_enc_op)(struct kvm

[PATCH 3/6 v3] KVM: nSVM: Fill in conforming svm_nested_ops via macro

The names of the nested_svm_ops functions do not have a corresponding
'nested_svm_' prefix. Generate the names using a macro so that the names are
conformant. Fixing the naming will help in better readability and
maintenance of the code.

Suggested-by: Vitaly Kuznetsov 
Suggested-by: Paolo Bonzini 
Signed-off-by: Sean Christopherson 
Signed-off-by: Krish Sadhukhan 
---
 arch/x86/kvm/svm/nested.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 3be6256..7cb834a 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -718,7 +718,7 @@ static int nested_svm_intercept(struct vcpu_svm *svm)
/*
 * Host-intercepted exceptions have been checked already in
 * nested_svm_exit_special.  There is nothing to do here,
-* the vmexit is injected by svm_check_nested_events.
+* the vmexit is injected by nested_svm_check_events().
 */
vmexit = NESTED_EXIT_DONE;
break;
@@ -850,7 +850,7 @@ static void nested_svm_init(struct vcpu_svm *svm)
 }
 
 
-static int svm_check_nested_events(struct kvm_vcpu *vcpu)
+static int nested_svm_check_events(struct kvm_vcpu *vcpu)
 {
struct vcpu_svm *svm = to_svm(vcpu);
bool block_nested_events =
@@ -933,7 +933,7 @@ int nested_svm_exit_special(struct vcpu_svm *svm)
return NESTED_EXIT_CONTINUE;
 }
 
-static int svm_get_nested_state(struct kvm_vcpu *vcpu,
+static int nested_svm_get_state(struct kvm_vcpu *vcpu,
struct kvm_nested_state __user 
*user_kvm_nested_state,
u32 user_data_size)
 {
@@ -990,7 +990,7 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
return kvm_state.size;
 }
 
-static int svm_set_nested_state(struct kvm_vcpu *vcpu,
+static int nested_svm_set_state(struct kvm_vcpu *vcpu,
struct kvm_nested_state __user 
*user_kvm_nested_state,
struct kvm_nested_state *kvm_state)
 {
@@ -1075,8 +1075,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
return 0;
 }
 
+#define KVM_X86_NESTED_OP(name) .name = nested_svm_##name
+
 struct kvm_x86_nested_ops svm_nested_ops = {
-   .check_events = svm_check_nested_events,
-   .get_state = svm_get_nested_state,
-   .set_state = svm_set_nested_state,
+   KVM_X86_NESTED_OP(check_events),
+   KVM_X86_NESTED_OP(get_state),
+   KVM_X86_NESTED_OP(set_state),
 };
-- 
1.8.3.1

[PATCH 0/6 v3] KVM: x86: Fill in conforming {vmx|svm}_x86_ops and {vmx|svm}_nested_ops via macros

v2 -> v3:
1. kvm_arch_hardware_unsetup() is changed to
   kvm_arch_hardware_teardown() on non-x86 arches as well.

2. The following #defines

KVM_MEMORY_ENCRYPT_OP
KVM_MEMORY_ENCRYPT_REG_REGION
KVM_MEMORY_ENCRYPT_UNREG_REGION

   have been changed to:

KVM_MEM_ENC_OP
KVM_MEM_ENC_REGISTER_REGION
KVM_MEM_ENC_UNREGISTER_REGION

3. Patch# 6 is new. It changes the KVM_MEMORY_ENCRYPT_* #defines in
   QEMU to make them conformant to those in the kernel.


[PATCH 1/6 v3] KVM: x86: Change names of some of the kvm_x86_ops
[PATCH 2/6 v3] KVM: SVM: Fill in conforming svm_x86_ops via macro
[PATCH 3/6 v3] KVM: nSVM: Fill in conforming svm_nested_ops via macro
[PATCH 4/6 v3] KVM: VMX: Fill in conforming vmx_x86_ops via macro
[PATCH 5/6 v3] KVM: nVMX: Fill in conforming vmx_nested_ops via macro
[PATCH 6/6 v3] QEMU: x86: Change KVM_MEMORY_ENCRYPT_*  #defines to make them

 arch/arm64/include/asm/kvm_host.h   |   2 +-
 arch/mips/include/asm/kvm_host.h|   2 +-
 arch/powerpc/include/asm/kvm_host.h |   2 +-
 arch/s390/kvm/kvm-s390.c|   2 +-
 arch/x86/include/asm/kvm_host.h |  12 +-
 arch/x86/kvm/svm/avic.c |   4 +-
 arch/x86/kvm/svm/nested.c   |  18 +--
 arch/x86/kvm/svm/sev.c  |   6 +-
 arch/x86/kvm/svm/svm.c  | 218 +
 arch/x86/kvm/svm/svm.h  |   8 +-
 arch/x86/kvm/vmx/nested.c   |  26 ++--
 arch/x86/kvm/vmx/nested.h   |   2 +-
 arch/x86/kvm/vmx/vmx.c  | 238 ++--
 arch/x86/kvm/vmx/vmx.h  |   2 +-
 arch/x86/kvm/x86.c  |  28 ++---
 include/linux/kvm_host.h|   2 +-
 include/uapi/linux/kvm.h|   6 +-
 tools/include/uapi/linux/kvm.h  |   6 +-
 virt/kvm/kvm_main.c |   4 +-
 19 files changed, 298 insertions(+), 290 deletions(-)

Krish Sadhukhan (5):
  KVM: x86: Change names of some of the kvm_x86_ops functions to make them m
  KVM: SVM: Fill in conforming svm_x86_ops via macro
  KVM: nSVM: Fill in conforming svm_nested_ops via macro
  KVM: VMX: Fill in conforming vmx_x86_ops via macro
  KVM: nVMX: Fill in conforming vmx_nested_ops via macro

 target/i386/sev.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

Krish Sadhukhan (1):
  QEMU: x86: Change KVM_MEMORY_ENCRYPT_*  #defines to make them conformant t

[PATCH 5/6 v3] KVM: nVMX: Fill in conforming vmx_nested_ops via macro

The names of some of the vmx_nested_ops functions do not have a corresponding
'nested_vmx_' prefix. Generate the names using a macro so that the names are
conformant. Fixing the naming will help in better readability and
maintenance of the code.

Suggested-by: Vitaly Kuznetsov 
Suggested-by: Paolo Bonzini 
Signed-off-by: Sean Christopherson 
Signed-off-by: Krish Sadhukhan 
---
 arch/x86/kvm/vmx/nested.c | 24 +---
 arch/x86/kvm/vmx/nested.h |  2 +-
 arch/x86/kvm/vmx/vmx.c|  4 ++--
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index a898b53..fc09bb0 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3105,7 +3105,7 @@ static int nested_vmx_check_vmentry_hw(struct kvm_vcpu 
*vcpu)
return 0;
 }
 
-static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
+static bool nested_vmx_get_vmcs12_pages(struct kvm_vcpu *vcpu)
 {
struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -3295,7 +3295,7 @@ enum nvmx_vmentry_status 
nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
prepare_vmcs02_early(vmx, vmcs12);
 
if (from_vmentry) {
-   if (unlikely(!nested_get_vmcs12_pages(vcpu)))
+   if (unlikely(!nested_vmx_get_vmcs12_pages(vcpu)))
return NVMX_VMENTRY_KVM_INTERNAL_ERROR;
 
if (nested_vmx_check_vmentry_hw(vcpu)) {
@@ -3711,7 +3711,7 @@ static bool nested_vmx_preemption_timer_pending(struct 
kvm_vcpu *vcpu)
   to_vmx(vcpu)->nested.preemption_timer_expired;
 }
 
-static int vmx_check_nested_events(struct kvm_vcpu *vcpu)
+static int nested_vmx_check_events(struct kvm_vcpu *vcpu)
 {
struct vcpu_vmx *vmx = to_vmx(vcpu);
unsigned long exit_qual;
@@ -5907,7 +5907,7 @@ bool nested_vmx_reflect_vmexit(struct kvm_vcpu *vcpu)
return true;
 }
 
-static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
+static int nested_vmx_get_state(struct kvm_vcpu *vcpu,
struct kvm_nested_state __user 
*user_kvm_nested_state,
u32 user_data_size)
 {
@@ -6031,7 +6031,7 @@ void vmx_leave_nested(struct kvm_vcpu *vcpu)
free_nested(vcpu);
 }
 
-static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
+static int nested_vmx_set_state(struct kvm_vcpu *vcpu,
struct kvm_nested_state __user 
*user_kvm_nested_state,
struct kvm_nested_state *kvm_state)
 {
@@ -6448,7 +6448,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs 
*msrs, u32 ept_caps)
msrs->vmcs_enum = VMCS12_MAX_FIELD_INDEX << 1;
 }
 
-void nested_vmx_hardware_unsetup(void)
+void nested_vmx_hardware_teardown(void)
 {
int i;
 
@@ -6473,7 +6473,7 @@ __init int nested_vmx_hardware_setup(int 
(*exit_handlers[])(struct kvm_vcpu *))
vmx_bitmap[i] = (unsigned long *)
__get_free_page(GFP_KERNEL);
if (!vmx_bitmap[i]) {
-   nested_vmx_hardware_unsetup();
+   nested_vmx_hardware_teardown();
return -ENOMEM;
}
}
@@ -6497,12 +6497,14 @@ __init int nested_vmx_hardware_setup(int 
(*exit_handlers[])(struct kvm_vcpu *))
return 0;
 }
 
+#define KVM_X86_NESTED_OP(name) .name = nested_vmx_##name
+
 struct kvm_x86_nested_ops vmx_nested_ops = {
-   .check_events = vmx_check_nested_events,
+   KVM_X86_NESTED_OP(check_events),
.hv_timer_pending = nested_vmx_preemption_timer_pending,
-   .get_state = vmx_get_nested_state,
-   .set_state = vmx_set_nested_state,
-   .get_vmcs12_pages = nested_get_vmcs12_pages,
+   KVM_X86_NESTED_OP(get_state),
+   KVM_X86_NESTED_OP(set_state),
+   KVM_X86_NESTED_OP(get_vmcs12_pages),
.enable_evmcs = nested_enable_evmcs,
.get_evmcs_version = nested_get_evmcs_version,
 };
diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index 758bccc..ac6b561 100644
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -18,7 +18,7 @@ enum nvmx_vmentry_status {
 
 void vmx_leave_nested(struct kvm_vcpu *vcpu);
 void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps);
-void nested_vmx_hardware_unsetup(void);
+void nested_vmx_hardware_teardown(void);
 __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu 
*));
 void nested_vmx_set_vmcs_shadowing_bitmap(void);
 void nested_vmx_free_vcpu(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f6a6674..6512e6e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7830,7 +7830,7 @@ static void vmx_migrate_timers(struct kvm_vcpu *vcpu)
 static void vmx_hardware_teardown(void)
 {
if (nested)
-

RE: [PATCH v2 3/3] virtiofsd: probe unshare(CLONE_FS) and print an error

2020-07-27 Thread misono.tomoh...@fujitsu.com

> Subject: [PATCH v2 3/3] virtiofsd: probe unshare(CLONE_FS) and print an error
> 
> An assertion failure is raised during request processing if
> unshare(CLONE_FS) fails. Implement a probe at startup so the problem can
> be detected right away.
> 
> Unfortunately Docker/Moby does not include unshare in the seccomp.json
> list unless CAP_SYS_ADMIN is given. Other seccomp.json lists always
> include unshare (e.g. podman is unaffected):
> https://raw.githubusercontent.com/seccomp/containers-golang/master/seccomp.json
> 
> Use "docker run --security-opt seccomp=path/to/seccomp.json ..." if the
> default seccomp.json is missing unshare.

Hi, sorry for a bit late.

unshare() was added to fix xattr problem: 
  https://github.com/qemu/qemu/commit/bdfd66788349acc43cd3f1298718ad491663cfcc#
In theory we don't need to call unshare if xattr is disabled, but it is hard to 
get to know
if xattr is enabled or disabled in fv_queue_worker(), right?

So, it looks good to me.
Reviewed-by: Misono Tomohiro 

Regards,
Misono

> 
> Cc: Misono Tomohiro 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tools/virtiofsd/fuse_virtio.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> index 3b6d16a041..9e5537506c 100644
> --- a/tools/virtiofsd/fuse_virtio.c
> +++ b/tools/virtiofsd/fuse_virtio.c
> @@ -949,6 +949,22 @@ int virtio_session_mount(struct fuse_session *se)
>  {
>  int ret;
> 
> +/*
> + * Test that unshare(CLONE_FS) works. fv_queue_worker() will need it. 
> It's
> + * an unprivileged system call but some Docker/Moby versions are known to
> + * reject it via seccomp when CAP_SYS_ADMIN is not given.
> + *
> + * Note that the program is single-threaded here so this syscall has no
> + * visible effect and is safe to make.
> + */
> +ret = unshare(CLONE_FS);
> +if (ret == -1 && errno == EPERM) {
> +fuse_log(FUSE_LOG_ERR, "unshare(CLONE_FS) failed with EPERM. If "
> +"running in a container please check that the container "
> +"runtime seccomp policy allows unshare.\n");
> +return -1;
> +}
> +
>  ret = fv_create_listen_socket(se);
>  if (ret < 0) {
>  return ret;
> --
> 2.26.2

RE: [PATCH v2 1/3] hw/i386: Initialize topo_ids from CpuInstanceProperties

2020-07-27 Thread Babu Moger




> -Original Message-
> From: Igor Mammedov 
> Sent: Monday, July 27, 2020 12:14 PM
> To: Moger, Babu 
> Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; ehabk...@redhat.com;
> r...@twiddle.net
> Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> CpuInstanceProperties
> 
> On Mon, 27 Jul 2020 10:49:08 -0500
> Babu Moger  wrote:
> 
> > > -Original Message-
> > > From: Igor Mammedov 
> > > Sent: Friday, July 24, 2020 12:05 PM
> > > To: Moger, Babu 
> > > Cc: qemu-devel@nongnu.org; pbonz...@redhat.com;
> ehabk...@redhat.com;
> > > r...@twiddle.net
> > > Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > CpuInstanceProperties
> > >
> > > On Mon, 13 Jul 2020 14:30:29 -0500
> > > Babu Moger  wrote:
> > >
> > > > > -Original Message-
> > > > > From: Igor Mammedov 
> > > > > Sent: Monday, July 13, 2020 12:32 PM
> > > > > To: Moger, Babu 
> > > > > Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com;
> > > > > qemu- de...@nongnu.org
> > > > > Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > > > CpuInstanceProperties
> > > > >
> > > > > On Mon, 13 Jul 2020 11:43:33 -0500 Babu Moger
> > > > >  wrote:
> > > > >
> > > > > > On 7/13/20 11:17 AM, Igor Mammedov wrote:
> > > > > > > On Mon, 13 Jul 2020 10:02:22 -0500 Babu Moger
> > > > > > >  wrote:
> > > > > > >
> > > > > > >>> -Original Message-
> > > > > > >>> From: Igor Mammedov 
> > > > > > >>> Sent: Monday, July 13, 2020 4:08 AM
> > > > > > >>> To: Moger, Babu 
> > > > > > >>> Cc: pbonz...@redhat.com; r...@twiddle.net;
> > > > > > >>> ehabk...@redhat.com;
> > > > > > >>> qemu- de...@nongnu.org
> > > > > > >>> Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids
> > > > > > >>> from CpuInstanceProperties
> > > > > > > [...]
> > > > > >  +
> > > > > >  +/*
> > > > > >  + * Initialize topo_ids from CpuInstanceProperties
> > > > > >  + * node_id in CpuInstanceProperties(or in CPU device) is
> > > > > >  +a sequential
> > > > > >  + * number, but while building the topology
> > > > > > >>>
> > > > > >  we need to separate it for
> > > > > >  + * each socket(mod nodes_per_pkg).
> > > > > > >>> could you clarify a bit more on why this is necessary?
> > > > > > >>
> > > > > > >> If you have two sockets and 4 numa nodes, node_id in
> > > > > > >> CpuInstanceProperties will be number sequentially as 0, 1, 2, 3.
> > > > > > >> But in EPYC topology, it will be  0, 1, 0, 1( Basically mod
> > > > > > >> % number of nodes
> > > > > per socket).
> > > > > > >
> > > > > > > I'm confused, let's suppose we have 2 EPYC sockets with 2
> > > > > > > nodes per socket so APIC id woulbe be composed like:
> > > > > > >
> > > > > > >  1st socket
> > > > > > >pkg_id(0) | node_id(0)
> > > > > > >pkg_id(0) | node_id(1)
> > > > > > >
> > > > > > >  2nd socket
> > > > > > >pkg_id(1) | node_id(0)
> > > > > > >pkg_id(1) | node_id(1)
> > > > > > >
> > > > > > > if that's the case, then EPYC's node_id here doesn't look
> > > > > > > like a NUMA node in the sense it's usually used (above
> > > > > > > config would have 4 different memory controllers => 4 conventional
> NUMA nodes).
> > > > > >
> > > > > > EPIC model uses combination of socket id and node id to
> > > > > > identify the numa nodes. So, it internally uses all the information.
> > > > >
> > > > > well with above values, EPYC's node_id doesn't look like it's
> > > > > specifying a machine numa node, but rather a node index within
> > > > > single socket. In which case, it doesn't make much sense calling
> > > > > it NUMA node_id, it's rather some index within a socket. (it
> > > > > starts looking like terminology is all mixed up)
> > > > >
> > > > > If you have access to a milti-socket EPYC machine, can you dump
> > > > > and post here its apic ids, pls?
> > > >
> > > > Here is the output from my EPYC machine with 2 sockets and totally
> > > > 8 nodes(SMT disabled). The cpus 0-31 are in socket 0 and  cpus
> > > > 32-63 in socket 1.
> > > >
> > > > # lscpu
> > > > Architecture:x86_64
> > > > CPU op-mode(s):  32-bit, 64-bit
> > > > Byte Order:  Little Endian
> > > > CPU(s):  64
> > > > On-line CPU(s) list: 0-63
> > > > Thread(s) per core:  1
> > > > Core(s) per socket:  32
> > > > Socket(s):   2
> > > > NUMA node(s):8
> > > > Vendor ID:   AuthenticAMD
> > > > CPU family:  23
> > > > Model:   1
> > > > Model name:  AMD Eng Sample: 1S1901A4VIHF5_30/19_N
> > > > Stepping:2
> > > > CPU MHz: 2379.233
> > > > CPU max MHz: 1900.
> > > > CPU min MHz: 1200.
> > > > BogoMIPS:3792.81
> > > > Virtualization:  AMD-V
> > > > L1d cache:   32K
> > > > L1i cache:   64K
> > > > L2 cache:512K
> > > > L3 cache:8192K
> > > > NUMA node0 CPU(s):   0-7
> > > > NUMA node1 CPU(s):   8-15
> > > > NUMA node2 CPU(s):   16-23
> > > > NUMA node3

Re: [PATCH v2 0/7] target/riscv: NaN-boxing for multiple precison

On Thu, Jul 23, 2020 at 5:28 PM Richard Henderson
 wrote:
>
> This is my take on Liu Zhiwei's patch set:
> https://patchew.org/QEMU/20200626205917.4545-1-zhiwei_...@c-sky.com
>
> This differs from Zhiwei's v1 in:
>
>  * If a helper is involved, the helper does the boxing and unboxing.
>
>  * Which leaves only LDW and FSGN*.S as the only instructions that
>are expanded inline which need to handle nanboxing.
>
>  * All mention of RVD is dropped vs boxing.  This means that an
>RVF-only cpu will still generate and check nanboxes into the
>64-bit cpu_fpu slots.  There should be no way an RVF-only cpu
>can generate an unboxed cpu_fpu value.
>
>This choice is made to speed up the common case: RVF+RVD, so
>that we do not have to check whether RVD is enabled.
>
>  * The translate.c primitives take TCGv values rather than fpu
>regno, which will make it possible to use them with RVV,
>since v0.9 does proper nanboxing.
>
>  * I have adjusted the current naming to be float32 specific ("*_s"),
>to avoid confusion with the float16 data type supported by RVV.

Thanks Richard. As Zhiwei has reviewed all of these I have applied
them to the riscv-to-apply.next tree for 5.2.

Alistair

>
>
> r~
>
>
> LIU Zhiwei (2):
>   target/riscv: Clean up fmv.w.x
>   target/riscv: check before allocating TCG temps
>
> Richard Henderson (5):
>   target/riscv: Generate nanboxed results from fp helpers
>   target/riscv: Generalize gen_nanbox_fpr to gen_nanbox_s
>   target/riscv: Generate nanboxed results from trans_rvf.inc.c
>   target/riscv: Check nanboxed inputs to fp helpers
>   target/riscv: Check nanboxed inputs in trans_rvf.inc.c
>
>  target/riscv/internals.h|  16 
>  target/riscv/fpu_helper.c   | 102 
>  target/riscv/insn_trans/trans_rvd.inc.c |   8 +-
>  target/riscv/insn_trans/trans_rvf.inc.c |  99 ++-
>  target/riscv/translate.c|  29 +++
>  5 files changed, 178 insertions(+), 76 deletions(-)
>
> --
> 2.25.1
>

Re: [PATCH v10] qga: add command guest-get-devices for reporting VirtIO devices

Quoting Tomáš Golembiovský (2020-07-21 10:40:41)
> Add command for reporting devices on Windows guest. The intent is not so
> much to report the devices but more importantly the driver (and its
> version) that is assigned to the device. This gives caller the
> information whether VirtIO drivers are installed and/or whether
> inadequate driver is used on a device (e.g. QXL device with base VGA
> driver).
> 
> Example:
> [
> {
>   "driver-date": "2019-08-12",
>   "driver-name": "Red Hat VirtIO SCSI controller",
>   "driver-version": "100.80.104.17300",
>   "address": {
> "type": "pci",
> "data": {
>   "device-id": 4162,
>   "vendor-id": 6900
> }
>   }
> },
> ...
> ]
> 
> Signed-off-by: Tomáš Golembiovský 
> Reviewed-by: Marc-André Lureau 
> Reviewed-by: Philippe Mathieu-Daudé 

Thanks, applied to qga-staging tree for 5.2:
  https://github.com/mdroth/qemu/commits/qga-staging

Sorry for the delays in processing this.

> ---
> 
> Changes in v10:
> - rebased to current master
> - changed `since` tag in schema to 5.2
> 
>  qga/commands-posix.c |   9 ++
>  qga/commands-win32.c | 212 ++-
>  qga/qapi-schema.json |  51 +++
>  3 files changed, 271 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 1a62a3a70d..f509a1f525 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2761,6 +2761,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
>  blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
>  #endif
> 
> +blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
> +
>  return blacklist;
>  }
> 
> @@ -2981,3 +2983,10 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> 
>  return info;
>  }
> +
> +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> +{
> +error_setg(errp, QERR_UNSUPPORTED);
> +
> +return NULL;
> +}
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index aaa71f147b..1302bae9eb 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -21,10 +21,11 @@
>  #ifdef CONFIG_QGA_NTDDSCSI
>  #include 
>  #include 
> +#endif
>  #include 
>  #include 
>  #include 
> -#endif
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -39,6 +40,36 @@
>  #include "qemu/base64.h"
>  #include "commands-common.h"
> 
> +/*
> + * The following should be in devpkey.h, but it isn't. The key names were
> + * prefixed to avoid (future) name clashes. Once the definitions get into
> + * mingw the following lines can be removed.
> + */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_NAME, 0xb725f130, 0x47ef, 0x101a, 0xa5,
> +0xf1, 0x02, 0x60, 0x8c, 0x9e, 0xeb, 0xac, 10);
> +/* DEVPROP_TYPE_STRING */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_HardwareIds, 0xa45c254e, 0xdf1c,
> +0x4efd, 0x80, 0x20, 0x67, 0xd1, 0x46, 0xa8, 0x50, 0xe0, 3);
> +/* DEVPROP_TYPE_STRING_LIST */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverDate, 0xa8b865dd, 0x2e3d,
> +0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 2);
> +/* DEVPROP_TYPE_FILETIME */
> +DEFINE_DEVPROPKEY(qga_DEVPKEY_Device_DriverVersion, 0xa8b865dd, 0x2e3d,
> +0x4094, 0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 3);
> +/* DEVPROP_TYPE_STRING */
> +/* The following shoud be in cfgmgr32.h, but it isn't */
> +#ifndef CM_Get_DevNode_Property
> +CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
> +DEVINST  dnDevInst,
> +CONST DEVPROPKEY * PropertyKey,
> +DEVPROPTYPE  * PropertyType,
> +PBYTEPropertyBuffer,
> +PULONG   PropertyBufferSize,
> +ULONGulFlags
> +);
> +#define CM_Get_DevNode_Property CM_Get_DevNode_PropertyW
> +#endif
> +
>  #ifndef SHTDN_REASON_FLAG_PLANNED
>  #define SHTDN_REASON_FLAG_PLANNED 0x8000
>  #endif
> @@ -93,6 +124,8 @@ static OpenFlags guest_file_open_modes[] = {
>  g_free(suffix); \
>  } while (0)
> 
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(GuestDeviceInfo, qapi_free_GuestDeviceInfo)
> +
>  static OpenFlags *find_open_flag(const char *mode_str)
>  {
>  int mode;
> @@ -2229,3 +2262,180 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> 
>  return info;
>  }
> +
> +/*
> + * Safely get device property. Returned strings are using wide characters.
> + * Caller is responsible for freeing the buffer.
> + */
> +static LPBYTE cm_get_property(DEVINST devInst, const DEVPROPKEY *propName,
> +PDEVPROPTYPE propType)
> +{
> +CONFIGRET cr;
> +g_autofree LPBYTE buffer = NULL;
> +ULONG buffer_len = 0;
> +
> +/* First query for needed space */
> +cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> +buffer, _len, 0);
> +if (cr != CR_SUCCESS && cr != CR_BUFFER_SMALL) {
> +
> +slog("failed to get property size, error=0x%lx", cr);
> +return NULL;
> +}
> +buffer = g_new0(BYTE, buffer_len + 1);
> +cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
> +

Re: [PATCH v2 0/4] Allow guest-get-fsinfo also for non-PCI devices

Quoting Thomas Huth (2020-07-21 23:40:24)
> The information that can be retrieved via UDEV is also usable for non-PCI
> devices. So let's allow build_guest_fsinfo_for_real_device() on non-PCI
> devices, too. This is required to fix the bug that CCW devices show up
> without "Target" when running libvirt's "virsh domfsinfo" command (see
> https://bugzilla.redhat.com/show_bug.cgi?id=1755075 for details).
> 
> v2:
>  - Use g_new0 instead of g_malloc0 (as suggested by Daniel)
>  - Init fields to -1 explicitely, not via memset (Daniel)
>  - Add the fourth patch to also fill in virtio information on s390x

Thanks, patches 2-4 applied to qga-staging tree for 5.2:
  https://github.com/mdroth/qemu/commits/qga-staging

I've sent a pull request for 5.1 with patch 1/4

> 
> Thomas Huth (4):
>   qga/qapi-schema: Document -1 for invalid PCI address fields
>   qga/commands-posix: Rework build_guest_fsinfo_for_real_device()
> function
>   qga/commands-posix: Move the udev code from the pci to the generic
> function
>   qga/commands-posix: Support fsinfo for non-PCI virtio devices, too
> 
>  qga/commands-posix.c | 157 ++-
>  qga/qapi-schema.json |   2 +-
>  2 files changed, 110 insertions(+), 49 deletions(-)
> 
> -- 
> 2.18.1
>

[PULL for-5.1 0/2] qemu-ga patch queue for hard-freeze

The following changes since commit 9303ecb658a0194560d1eecde165a1511223c2d8:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200727' into 
staging (2020-07-27 17:25:06 +0100)

are available in the Git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2020-07-27-tag

for you to fetch changes up to ba620541d0db7e3433babbd97c0413a371e6fb4a:

  qga/qapi-schema: Document -1 for invalid PCI address fields (2020-07-27 
18:03:55 -0500)


qemu-ga patch queue for hard-freeze

* document use of -1 when pci_controller field can't be retrieved for
  guest-get-fsinfo
* fix incorrect filesystem type reporting on w32 for guest-get-fsinfo
  when a volume is not mounted


Basil Salman (1):
  qga-win: fix "guest-get-fsinfo" wrong filesystem type

Thomas Huth (1):
  qga/qapi-schema: Document -1 for invalid PCI address fields

 qga/commands-win32.c | 29 +++--
 qga/qapi-schema.json |  2 +-
 2 files changed, 24 insertions(+), 7 deletions(-)

[PULL for-5.1 1/2] qga-win: fix "guest-get-fsinfo" wrong filesystem type

From: Basil Salman 

This patch handles the case where unmounted volumes exist,
where in that case GetVolumePathNamesForVolumeName returns
empty path, GetVolumeInformation will use the current working
directory instead.
This patch fixes the issue by opening a handle to the volumes,
and using GetVolumeInformationByHandleW instead.

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

Signed-off-by: Basil Salman 
Signed-off-by: Basil Salman 
*fix crash when guest_build_fsinfo() sets errp multiple times
*make new error message more distinct from existing ones
Signed-off-by: Michael Roth 
---
 qga/commands-win32.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index aaa71f147b..15c9d7944b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -958,11 +958,13 @@ static GuestFilesystemInfo *build_guest_fsinfo(char 
*guid, Error **errp)
 {
 DWORD info_size;
 char mnt, *mnt_point;
+wchar_t wfs_name[32];
 char fs_name[32];
-char vol_info[MAX_PATH+1];
+wchar_t vol_info[MAX_PATH + 1];
 size_t len;
 uint64_t i64FreeBytesToCaller, i64TotalBytes, i64FreeBytes;
 GuestFilesystemInfo *fs = NULL;
+HANDLE hLocalDiskHandle = NULL;
 
 GetVolumePathNamesForVolumeName(guid, (LPCH), 0, _size);
 if (GetLastError() != ERROR_MORE_DATA) {
@@ -977,18 +979,27 @@ static GuestFilesystemInfo *build_guest_fsinfo(char 
*guid, Error **errp)
 goto free;
 }
 
+hLocalDiskHandle = CreateFile(guid, 0 , 0, NULL, OPEN_EXISTING,
+  FILE_ATTRIBUTE_NORMAL |
+  FILE_FLAG_BACKUP_SEMANTICS, NULL);
+if (INVALID_HANDLE_VALUE == hLocalDiskHandle) {
+error_setg_win32(errp, GetLastError(), "failed to get handle for 
volume");
+goto free;
+}
+
 len = strlen(mnt_point);
 mnt_point[len] = '\\';
 mnt_point[len+1] = 0;
-if (!GetVolumeInformation(mnt_point, vol_info, sizeof(vol_info), NULL, 
NULL,
-  NULL, (LPSTR)_name, sizeof(fs_name))) {
+
+if (!GetVolumeInformationByHandleW(hLocalDiskHandle, vol_info,
+   sizeof(vol_info), NULL, NULL, NULL,
+   (LPWSTR) & wfs_name, sizeof(wfs_name))) 
{
 if (GetLastError() != ERROR_NOT_READY) {
 error_setg_win32(errp, GetLastError(), "failed to get volume 
info");
 }
 goto free;
 }
 
-fs_name[sizeof(fs_name) - 1] = 0;
 fs = g_malloc(sizeof(*fs));
 fs->name = g_strdup(guid);
 fs->has_total_bytes = false;
@@ -1007,9 +1018,11 @@ static GuestFilesystemInfo *build_guest_fsinfo(char 
*guid, Error **errp)
 fs->has_used_bytes = true;
 }
 }
+wcstombs(fs_name, wfs_name, sizeof(wfs_name));
 fs->type = g_strdup(fs_name);
 fs->disk = build_guest_disk_info(guid, errp);
 free:
+CloseHandle(hLocalDiskHandle);
 g_free(mnt_point);
 return fs;
 }
@@ -1027,8 +1040,12 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error 
**errp)
 }
 
 do {
-GuestFilesystemInfo *info = build_guest_fsinfo(guid, errp);
-if (info == NULL) {
+Error *local_err = NULL;
+GuestFilesystemInfo *info = build_guest_fsinfo(guid, _err);
+if (local_err) {
+g_debug("failed to get filesystem info, ignoring error: %s",
+error_get_pretty(local_err));
+error_free(local_err);
 continue;
 }
 new = g_malloc(sizeof(*ret));
-- 
2.17.1

[PULL for-5.1 2/2] qga/qapi-schema: Document -1 for invalid PCI address fields

From: Thomas Huth 

The "guest-get-fsinfo" could also be used for non-PCI devices in the
future. And the code in GuestPCIAddress() in qga/commands-win32.c seems
to be using "-1" for fields that it can not determine already. Thus
let's properly document "-1" as value for invalid PCI address fields.

Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Thomas Huth 
Signed-off-by: Michael Roth 
---
 qga/qapi-schema.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 4be9aad48e..408a662ea5 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,7 +846,7 @@
 ##
 # @GuestDiskAddress:
 #
-# @pci-controller: controller's PCI address
+# @pci-controller: controller's PCI address (fields are set to -1 if invalid)
 # @bus-type: bus type
 # @bus: bus id
 # @target: target id
-- 
2.17.1

Re: migration: broken snapshot saves appear on s390 when small fields in migration stream removed

2020-07-27 Thread Bruce Rogers

On Tue, 2020-07-21 at 10:22 +0200, Claudio Fontana wrote:
> On 7/20/20 8:24 PM, Claudio Fontana wrote:
> > I have now been able to reproduce this on X86 as well.
> > 
> > It happens much more rarely, about once every 10 times.
> > 
> > I will sort out the data and try to make it even more reproducible,
> > then post my findings in detail.
> > 
> > Overall I proceeded as follows:
> > 
> > 1) hooked the savevm code to skip all fields with the exception of
> > "s390-skeys". So only s390-skeys are actually saved.
> > 
> > 2) reimplemented "s390-skeys" in a common implementation in cpus.c,
> > used on both x86 and s390, modeling the behaviour of save/load from
> > hw/s390
> > 
> > 3) ran ./check -qcow2 267 on both x86 and s390.
> > 
> > In the case of s390, failure seems to be reproducible 100% of the
> > times.
> > On X86, it is as mentioned failing about 10% of the times.
> > 
> > Ciao,
> > 
> > Claudio
> 
> And here is a small series of two patches that can be used to
> reproduce the problem.
> 
> Clearly, this is not directly related to s390 or to skeys or to
> icount in particular, it is just an issue that happened to be more
> visible there.
> 
> If you could help with this, please apply the attached patches.
> 
> Patch 1 just adds a new "300" iotest. It is way easier to extract the
> relevant part out of test 267, which does a bit too much in the same
> file.
> Also this allows easier use of valgrind, since it does not "require"
> anything.
> 
> Patch 2 hooks the savevm code to skip all fields during the snapshot
> with the exception of "s390-skeys", a new artificial field
> implemented to
> model what the real s390-skeys is doing.
> 
> After applying patch 1 and patch 2, you can test (also on X86), with:
> 
> ./check -qcow2 300
> 
> On X86 many runs will be successful, but a certain % of them will
> instead fail like this:
> 
> 
> claudio@linux-ch70:~/git/qemu-pristine/qemu-build/tests/qemu-iotests> 
> ./check -qcow2 300
> QEMU  -- "/home/claudio/git/qemu-pristine/qemu-
> build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64"
> -nodefaults -display none -accel qtest
> QEMU_IMG  -- "/home/claudio/git/qemu-pristine/qemu-
> build/tests/qemu-iotests/../../qemu-img" 
> QEMU_IO   -- "/home/claudio/git/qemu-pristine/qemu-
> build/tests/qemu-iotests/../../qemu-io"  --cache writeback --aio
> threads -f qcow2
> QEMU_NBD  -- "/home/claudio/git/qemu-pristine/qemu-
> build/tests/qemu-iotests/../../qemu-nbd" 
> IMGFMT-- qcow2 (compat=1.1)
> IMGPROTO  -- file
> PLATFORM  -- Linux/x86_64 linux-ch70 4.12.14-lp151.28.36-default
> TEST_DIR  -- /home/claudio/git/qemu-pristine/qemu-
> build/tests/qemu-iotests/scratch
> SOCK_DIR  -- /tmp/tmp.gdcUu3l0SM
> SOCKET_SCM_HELPER -- /home/claudio/git/qemu-pristine/qemu-
> build/tests/qemu-iotests/socket_scm_helper
> 
> 300  fail   [10:14:05] [10:14:06]  (last: 0s)output
> mismatch (see 300.out.bad)
> --- /home/claudio/git/qemu-pristine/qemu/tests/qemu-
> iotests/300.out 2020-07-21 10:03:54.468104764 +0200
> +++ /home/claudio/git/qemu-pristine/qemu-build/tests/qemu-
> iotests/300.out.bad   2020-07-21 10:14:06.098090543 +0200
> @@ -12,6 +12,9 @@
>  IDTAG VM SIZEDATE   VM
> CLOCK
>  --snap0  SIZE -mm-dd
> hh:mm:ss   00:00:00.000
>  (qemu) loadvm snap0
> +Unexpected storage key data: 0
> +error while loading state for instance 0x0 of device 's390-skeys'
> +Error: Error -22 while loading VM state
>  (qemu) quit
>  
>  *** done
> Failures: 300
> Failed 1 of 1 iotests
> 
> 
> At this point somebody more knowledgeable about QCOW2, coroutines and
> backing files could chime in?
> 


I used the reproducer you provide here to do a git bisect as I assume
whatever is now broken wasn't always broken, and it pointed to the
following commit:

commit df893d25ceea3c0dcbe6d6b425309317fab6b22e (refs/bisect/bad)
Author: Vladimir Sementsov-Ogievskiy 
Date:   Tue Jun 4 19:15:13 2019 +0300

block/qcow2: implement .bdrv_co_preadv_part

Indeed, I am currently able to reliable reproduce the issue with this
commit applied, and not reproduce it without it.

That said, I've not been able to identify exactly what is going wrong.
I'm fairly confident the savevm data is correctly written out, but on
the loadvm side, somehow the last part of the s390 data is not
correctly read in the data (it's in the second pass through the while
loop in qcow2_co_preadv_part() where that happens.)

If anyone familiar with this code can have a look or provide some
pointers, it would be much appreciated.

Adding commit author to CC.

Thanks,

Bruce

Re: [PATCH v5 3/4] target/riscv: Fix the translation of physical address

On Sat, Jul 25, 2020 at 8:05 AM Zong Li  wrote:
>
> The real physical address should add the 12 bits page offset. It also
> causes the PMP wrong checking due to the minimum granularity of PMP is
> 4 byte, but we always get the physical address which is 4KB alignment,
> that means, we always use the start address of the page to check PMP for
> all addresses which in the same page.

So riscv_cpu_tlb_fill() will clear these bits when calling
tlb_set_page(), so this won't have an impact on actual translation
(although it will change in input address for 2-stage translation, but
that seems fine).

Your point about PMP seems correct as we allow a smaller then page
granularity this seems like the right approach.

Can you edit riscv_cpu_get_phys_page_debug() to mask these bits out at
the end? Otherwise we will break what callers to
cpu_get_phys_page_attrs_debug() expect.

Alistair

>
> Signed-off-by: Zong Li 
> ---
>  target/riscv/cpu_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 75d2ae3434..08b069f0c9 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -543,7 +543,8 @@ restart:
>  /* for superpage mappings, make a fake leaf PTE for the TLB's
> benefit. */
>  target_ulong vpn = addr >> PGSHIFT;
> -*physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
> +*physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) |
> +(addr & ~TARGET_PAGE_MASK);
>
>  /* set permissions on the TLB entry */
>  if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
> --
> 2.27.0
>
>

[Bug 1681439] Re: qemu-system-x86_64: hw/ide/core.c:685: ide_cancel_dma_sync: Assertion `s->bus->dma->aiocb == NULL' failed.

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

** Changed in: qemu
 Assignee: (unassigned) => John Snow (jnsnow)

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

Title:
  qemu-system-x86_64: hw/ide/core.c:685: ide_cancel_dma_sync: Assertion
  `s->bus->dma->aiocb == NULL' failed.

Status in QEMU:
  Confirmed

Bug description:
  Since upgrading to QEMU 2.8.0, my Windows 7 64-bit virtual machines
  started crashing due to the assertion quoted in the summary failing.
  The assertion in question was added by commit 9972354856 ("block: add
  BDS field to count in-flight requests").  My tests show that setting
  discard=unmap is needed to reproduce the issue.  Speaking of
  reproduction, it is a bit flaky, because I have been unable to come up
  with specific instructions that would allow the issue to be triggered
  outside of my environment, but I do have a semi-sane way of testing that
  appears to depend on a specific initial state of data on the underlying
  storage volume, actions taken within the VM and waiting for about 20
  minutes.

  Here is the shortest QEMU command line that I managed to reproduce the
  bug with:

  qemu-system-x86_64 \
  -machine pc-i440fx-2.7,accel=kvm \
  -m 3072 \
  -drive file=/dev/lvm/qemu,format=raw,if=ide,discard=unmap \
-netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no,vhost=on \
  -device virtio-net-pci,netdev=hostnet0 \
-vnc :0

  The underlying storage (/dev/lvm/qemu) is a thin LVM snapshot.

  QEMU was compiled using:

  ./configure --python=/usr/bin/python2.7 --target-list=x86_64-softmmu
  make -j3

  My virtualization environment is not really a critical one and
  reproduction is not that much of a hassle, so if you need me to gather
  further diagnostic information or test patches, I will be happy to help.

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

Re: [PATCH v5 2/4] target/riscv/pmp.c: Fix the index offset on RV64

On Sat, Jul 25, 2020 at 8:04 AM Zong Li  wrote:
>
> On RV64, the reg_index is 2 (pmpcfg2 CSR) after the seventh pmp
> entry, it is not 1 (pmpcfg1 CSR) like RV32. In the original
> implementation, the second parameter of pmp_write_cfg is
> "reg_index * sizeof(target_ulong)", and we get the the result
> which is started from 16 if reg_index is 2, but we expect that
> it should be started from 8. Separate the implementation for
> RV32 and RV64 respectively.
>
> Signed-off-by: Zong Li 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/pmp.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a2b9f5363..aeba796484 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -318,6 +318,10 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t 
> reg_index,
>  return;
>  }
>
> +#if defined(TARGET_RISCV64)
> +reg_index >>= 1;
> +#endif
> +
>  for (i = 0; i < sizeof(target_ulong); i++) {
>  cfg_val = (val >> 8 * i)  & 0xff;
>  pmp_write_cfg(env, (reg_index * sizeof(target_ulong)) + i,
> @@ -335,11 +339,16 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, 
> uint32_t reg_index)
>  target_ulong cfg_val = 0;
>  target_ulong val = 0;
>
> +trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val);
> +
> +#if defined(TARGET_RISCV64)
> +reg_index >>= 1;
> +#endif
> +
>  for (i = 0; i < sizeof(target_ulong); i++) {
>  val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i);
>  cfg_val |= (val << (i * 8));
>  }
> -trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val);
>
>  return cfg_val;
>  }
> --
> 2.27.0
>
>

[Bug 1884693] Re: Assertion failure in address_space_unmap through ahci_map_clb_address

** Changed in: qemu
   Status: Confirmed => In Progress

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

Title:
  Assertion failure in address_space_unmap through ahci_map_clb_address

Status in QEMU:
  In Progress

Bug description:
  Hello,
  Reproducer:
  cat << EOF | ./i386-softmmu/qemu-system-i386 -qtest stdio -monitor none 
-serial none -M pc-q35-5.0 -nographic
  outl 0xcf8 0x8000fa24
  outl 0xcfc 0xe1068000
  outl 0xcf8 0x8000fa04
  outw 0xcfc 0x7
  outl 0xcf8 0x8000fb20
  write 0xe1068304 0x1 0x21
  write 0xe1068318 0x1 0x21
  write 0xe1068384 0x1 0x21
  write 0xe1068398 0x2 0x21
  EOF

  Stack trace:
  #0 0x55bfabfe9ea0 in __libc_start_main 
/build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16
  #1 0x55bfabfc8ef9 in __sanitizer_print_stack_trace 
(build/i386-softmmu/qemu-fuzz-i386+0x7b7ef9)
  #2 0x55bfabfaf933 in fuzzer::PrintStackTrace() FuzzerUtil.cpp:210:5
  #3 0x7f88df76110f  (/lib/x86_64-linux-gnu/libpthread.so.0+0x1410f)
  #4 0x7f88df5a4760 in __libc_signal_restore_set 
/build/glibc-GwnBeO/glibc-2.30/signal/../sysdeps/unix/sysv/linux/internal-signals.h:84:10
  #5 0x7f88df5a4760 in raise 
/build/glibc-GwnBeO/glibc-2.30/signal/../sysdeps/unix/sysv/linux/raise.c:48:3
  #6 0x7f88df58e55a in abort /build/glibc-GwnBeO/glibc-2.30/stdlib/abort.c:79:7
  #7 0x7f88df58e42e in __assert_fail_base 
/build/glibc-GwnBeO/glibc-2.30/assert/assert.c:92:3
  #8 0x7f88df59d091 in __assert_fail 
/build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
  #9 0x55bfabff7182 in address_space_unmap exec.c:3602:9
  #10 0x55bfac4a452f in dma_memory_unmap include/sysemu/dma.h:148:5
  #11 0x55bfac4a452f in map_page hw/ide/ahci.c:254:9
  #12 0x55bfac4a1f98 in ahci_map_clb_address hw/ide/ahci.c:748:5
  #13 0x55bfac4a1f98 in ahci_cond_start_engines hw/ide/ahci.c:276:14
  #14 0x55bfac4a074e in ahci_port_write hw/ide/ahci.c:339:9
  #15 0x55bfac4a074e in ahci_mem_write hw/ide/ahci.c:513:9
  #16 0x55bfac0e0dc2 in memory_region_write_accessor memory.c:483:5
  #17 0x55bfac0e0bde in access_with_adjusted_size memory.c:544:18
  #18 0x55bfac0e0917 in memory_region_dispatch_write memory.c
  #19 0x55bfabffa4fd in flatview_write_continue exec.c:3146:23
  #20 0x55bfabff569b in flatview_write exec.c:3186:14
  #21 0x55bfabff569b in address_space_write exec.c:3280:18
  #22 0x55bfac8982a9 in op_write_pattern tests/qtest/fuzz/general_fuzz.c:407:5
  #23 0x55bfac897749 in general_fuzz tests/qtest/fuzz/general_fuzz.c:481:17
  #24 0x55bfac8930a2 in LLVMFuzzerTestOneInput tests/qtest/fuzz/fuzz.c:136:5
  #25 0x55bfabfb0e68 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, 
unsigned long) FuzzerLoop.cpp:558:15
  #26 0x55bfabfb0485 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned 
long, bool, fuzzer::InputInfo*, bool*) FuzzerLoop.cpp:470:3
  #27 0x55bfabfb18a1 in fuzzer::Fuzzer::MutateAndTestOne() FuzzerLoop.cpp:701:19
  #28 0x55bfabfb2305 in fuzzer::Fuzzer::Loop(std::vector >&) FuzzerLoop.cpp:837:5
  #29 0x55bfabfa2018 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned 
char const*, unsigned long)) FuzzerDriver.cpp:846:6
  #30 0x55bfabfb8722 in main FuzzerMain.cpp:19:10
  #31 0x7f88df58fe0a in __libc_start_main 
/build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16
  #32 0x55bfabf97869 in _start (build/i386-softmmu/qemu-fuzz-i386+0x786869)

  The same error can be triggered through  ahci_map_fis_address @ 
hw/ide/ahci.c:721:5
  Found with generic device fuzzer: 
https://patchew.org/QEMU/20200611055651.13784-1-alx...@bu.edu/

  Please let me know if I can provide any further info.

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

Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale

On Mon, Jul 27, 2020 at 9:26 AM Peter Maydell  wrote:
>
> The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
> global, which meant that if guest code used the systick timer in "use
> the processor clock" mode it would hang because time never advances.
>
> Set the global to match the documented CPU clock speed of these boards.
> Judging by the data sheet this is slightly simplistic because the
> SoC allows configuration of the SYSCLK source and frequency via the
> RCC (reset and clock control) module, but we don't model that.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
> Signed-off-by: Peter Maydell 

Reviewed-by: Alistair Francis 

Alistair

> ---
> NB: tested with "make check" only...
>
>  hw/arm/netduino2.c | 10 ++
>  hw/arm/netduinoplus2.c | 10 ++
>  2 files changed, 20 insertions(+)
>
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index 79e19392b56..8f103341443 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f205_soc.h"
>  #include "hw/arm/boot.h"
>
> +/* Main SYSCLK frequency in Hz (120MHz) */
> +#define SYSCLK_FRQ 12000ULL
> +
>  static void netduino2_init(MachineState *machine)
>  {
>  DeviceState *dev;
>
> +/*
> + * TODO: ideally we would model the SoC RCC and let it handle
> + * system_clock_scale, including its ability to define different
> + * possible SYSCLK sources.
> + */
> +system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>  dev = qdev_new(TYPE_STM32F205_SOC);
>  qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
>  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
> diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
> index 958d21dd9f9..68abd3ec69d 100644
> --- a/hw/arm/netduinoplus2.c
> +++ b/hw/arm/netduinoplus2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f405_soc.h"
>  #include "hw/arm/boot.h"
>
> +/* Main SYSCLK frequency in Hz (168MHz) */
> +#define SYSCLK_FRQ 16800ULL
> +
>  static void netduinoplus2_init(MachineState *machine)
>  {
>  DeviceState *dev;
>
> +/*
> + * TODO: ideally we would model the SoC RCC and let it handle
> + * system_clock_scale, including its ability to define different
> + * possible SYSCLK sources.
> + */
> +system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>  dev = qdev_new(TYPE_STM32F405_SOC);
>  qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
> --
> 2.20.1
>
>

[Bug 1883739] Re: ide_dma_cb: Assertion `prep_size >= 0 && prep_size <= n * 512' failed.

** Changed in: qemu
 Assignee: (unassigned) => John Snow (jnsnow)

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

Title:
  ide_dma_cb: Assertion `prep_size >= 0 && prep_size <= n * 512' failed.

Status in QEMU:
  Confirmed

Bug description:
  To reproduce run the QEMU with the following command line:
  ```
  qemu-system-x86_64 -cdrom hypertrash.iso -nographic -m 100 -enable-kvm -net 
none -drive id=disk,file=hda.img,if=none -device ahci,id=ahci -device 
ide-hd,drive=disk,bus=ahci.0
  ```

  QEMU Version:
  ```
  # qemu-5.0.0
  $ ./configure --target-list=x86_64-softmmu --enable-sanitizers; make
  $ x86_64-softmmu/qemu-system-x86_64 --version
  QEMU emulator version 5.0.0
  Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers
  ```

  To create disk image run:
  ```
  dd if=/dev/zero of=hda.img bs=1024 count=1024
  ```

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

[Bug 1887303] Re: Assertion failure in *bmdma_active_if `bmdma->bus->retry_unit != (uint8_t)-1' failed.

This is another manifestation of the SRST bug.

New proposal: https://lists.gnu.org/archive/html/qemu-
devel/2020-07/msg06974.html

More analysis of the problem in response to Philippe's proposed fix:
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg06237.html

** Changed in: qemu
   Status: New => In Progress

** Changed in: qemu
 Assignee: (unassigned) => John Snow (jnsnow)

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

Title:
  Assertion failure in *bmdma_active_if `bmdma->bus->retry_unit !=
  (uint8_t)-1' failed.

Status in QEMU:
  In Progress

Bug description:
  Hello,
  Here is a QTest Reproducer:

  cat << EOF | ./i386-softmmu/qemu-system-i386 -M pc,accel=qtest\
   -qtest null -nographic -vga qxl -qtest stdio -nodefaults\
   -drive if=none,id=drive0,file=null-co://,file.read-zeroes=on,format=raw\
   -drive if=none,id=drive1,file=null-co://,file.read-zeroes=on,format=raw\
   -device ide-cd,drive=drive0 -device ide-hd,drive=drive1
  outw 0x176 0x3538
  outw 0x376 0x6007
  outw 0x376 0x6b6b
  outw 0x176 0x985c
  outl 0xcf8 0x8903
  outl 0xcfc 0x2f2931
  outl 0xcf8 0x8920
  outb 0xcfc 0x6b
  outb 0x68 0x7
  outw 0x176 0x2530
  EOF

  Here is the call-stack:

  #8 0x7f00e0443091 in __assert_fail 
/build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
  #9 0x55e163f5a1af in bmdma_active_if 
/home/alxndr/Development/qemu/include/hw/ide/pci.h:59:5
  #10 0x55e163f5a1af in bmdma_prepare_buf 
/home/alxndr/Development/qemu/hw/ide/pci.c:132:19
  #11 0x55e163f4f00d in ide_dma_cb 
/home/alxndr/Development/qemu/hw/ide/core.c:898:17
  #12 0x55e163de86ad in dma_complete 
/home/alxndr/Development/qemu/dma-helpers.c:120:9
  #13 0x55e163de86ad in dma_blk_cb 
/home/alxndr/Development/qemu/dma-helpers.c:138:9
  #14 0x55e1642ade85 in blk_aio_complete 
/home/alxndr/Development/qemu/block/block-backend.c:1402:9
  #15 0x55e1642ade85 in blk_aio_complete_bh 
/home/alxndr/Development/qemu/block/block-backend.c:1412:5
  #16 0x55e16443556f in aio_bh_call 
/home/alxndr/Development/qemu/util/async.c:136:5
  #17 0x55e16443556f in aio_bh_poll 
/home/alxndr/Development/qemu/util/async.c:164:13
  #18 0x55e16440fac3 in aio_dispatch 
/home/alxndr/Development/qemu/util/aio-posix.c:380:5
  #19 0x55e164436dac in aio_ctx_dispatch 
/home/alxndr/Development/qemu/util/async.c:306:5
  #20 0x7f00e16e29ed in g_main_context_dispatch 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed)
  #21 0x55e164442f2b in glib_pollfds_poll 
/home/alxndr/Development/qemu/util/main-loop.c:219:9
  #22 0x55e164442f2b in os_host_main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:242:5
  #23 0x55e164442f2b in main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:518:11
  #24 0x55e164376953 in flush_events 
/home/alxndr/Development/qemu/tests/qtest/fuzz/fuzz.c:47:9
  #25 0x55e16437b8fa in general_fuzz 
/home/alxndr/Development/qemu/tests/qtest/fuzz/general_fuzz.c:544:17

  =

  Here is the same assertion failure but triggered through a different
  call-stack:

  cat << EOF | ./i386-softmmu/qemu-system-i386 -M pc,accel=qtest\
   -qtest null -nographic -vga qxl -qtest stdio -nodefaults\
   -drive if=none,id=drive0,file=null-co://,file.read-zeroes=on,format=raw\
   -drive if=none,id=drive1,file=null-co://,file.read-zeroes=on,format=raw\
   -device ide-cd,drive=drive0 -device ide-hd,drive=drive1
  outw 0x171 0x2fe9
  outb 0x177 0xa0
  outl 0x170 0x928
  outl 0x170 0x2b923b31
  outl 0x170 0x800a24d7
  outl 0xcf8 0x8903
  outl 0xcfc 0x842700
  outl 0xcf8 0x8920
  outb 0xcfc 0x5e
  outb 0x58 0x7
  outb 0x376 0x5
  outw 0x376 0x11
  outw 0x176 0x3538
  EOF

  Call-stack:
  #8 0x7f00e0443091 in __assert_fail 
/build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
  #9 0x55e163f5a622 in bmdma_active_if 
/home/alxndr/Development/qemu/include/hw/ide/pci.h:59:5
  #10 0x55e163f5a622 in bmdma_rw_buf 
/home/alxndr/Development/qemu/hw/ide/pci.c:187:19
  #11 0x55e163f57577 in ide_atapi_cmd_read_dma_cb 
/home/alxndr/Development/qemu/hw/ide/atapi.c:375:13
  #12 0x55e163f44c55 in ide_buffered_readv_cb 
/home/alxndr/Development/qemu/hw/ide/core.c:650:9
  #13 0x55e1642ade85 in blk_aio_complete 
/home/alxndr/Development/qemu/block/block-backend.c:1402:9
  #14 0x55e1642ade85 in blk_aio_complete_bh 
/home/alxndr/Development/qemu/block/block-backend.c:1412:5
  #15 0x55e16443556f in aio_bh_call 
/home/alxndr/Development/qemu/util/async.c:136:5
  #16 0x55e16443556f in aio_bh_poll 
/home/alxndr/Development/qemu/util/async.c:164:13
  #17 0x55e16440fac3 in aio_dispatch 
/home/alxndr/Development/qemu/util/aio-posix.c:380:5
  #18 0x55e164436dac in aio_ctx_dispatch 
/home/alxndr/Development/qemu/util/async.c:306:5
  #19 0x7f00e16e29ed in g_main_context_dispatch

Re: device compatibility interface for live migration with assigned devices

2020-07-27 Thread Alex Williamson

On Mon, 27 Jul 2020 15:24:40 +0800
Yan Zhao  wrote:

> > > As you indicate, the vendor driver is responsible for checking version
> > > information embedded within the migration stream.  Therefore a
> > > migration should fail early if the devices are incompatible.  Is it  
> > but as I know, currently in VFIO migration protocol, we have no way to
> > get vendor specific compatibility checking string in migration setup stage
> > (i.e. .save_setup stage) before the device is set to _SAVING state.
> > In this way, for devices who does not save device data in precopy stage,
> > the migration compatibility checking is as late as in stop-and-copy
> > stage, which is too late.
> > do you think we need to add the getting/checking of vendor specific
> > compatibility string early in save_setup stage?
> >  
> hi Alex,
> after an offline discussion with Kevin, I realized that it may not be a
> problem if migration compatibility check in vendor driver occurs late in
> stop-and-copy phase for some devices, because if we report device
> compatibility attributes clearly in an interface, the chances for
> libvirt/openstack to make a wrong decision is little.

I think it would be wise for a vendor driver to implement a pre-copy
phase, even if only to send version information and verify it at the
target.  Deciding you have no device state to send during pre-copy does
not mean your vendor driver needs to opt-out of the pre-copy phase
entirely.  Please also note that pre-copy is at the user's discretion,
we've defined that we can enter stop-and-copy at any point, including
without a pre-copy phase, so I would recommend that vendor drivers
validate compatibility at the start of both the pre-copy and the
stop-and-copy phases.

> so, do you think we are now arriving at an agreement that we'll give up
> the read-and-test scheme and start to defining one interface (perhaps in
> json format), from which libvirt/openstack is able to parse and find out
> compatibility list of a source mdev/physical device?

Based on the feedback we've received, the previously proposed interface
is not viable.  I think there's agreement that the user needs to be
able to parse and interpret the version information.  Using json seems
viable, but I don't know if it's the best option.  Is there any
precedent of markup strings returned via sysfs we could follow?

Your idea of having both a "self" object and an array of "compatible"
objects is perhaps something we can build on, but we must not assume
PCI devices at the root level of the object.  Providing both the
mdev-type and the driver is a bit redundant, since the former includes
the latter.  We can't have vendor specific versioning schemes though,
ie. gvt-version. We need to agree on a common scheme and decide which
fields the version is relative to, ex. just the mdev type?

I had also proposed fields that provide information to create a
compatible type, for example to create a type_x2 device from a type_x1
mdev type, they need to know to apply an aggregation attribute.  If we
need to explicitly list every aggregation value and the resulting type,
I think we run aground of what aggregation was trying to avoid anyway,
so we might need to pick a language that defines variable substitution
or some kind of tagging.  For example if we could define ${aggr} as an
integer within a specified range, then we might be able to define a type
relative to that value (type_x${aggr}) which requires an aggregation
attribute using the same value.  I dunno, just spit balling.  Thanks,

Alex

[Bug 1887309] Re: Floating-point exception in ide_set_sector

New proposal: https://lists.gnu.org/archive/html/qemu-
devel/2020-07/msg06974.html

(The root cause is that SRST is not handled correctly.)

More analysis in the replies to Philippe's patch:
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg05949.html

** Changed in: qemu
 Assignee: (unassigned) => John Snow (jnsnow)

** Changed in: qemu
   Status: Confirmed => In Progress

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

Title:
  Floating-point exception in ide_set_sector

Status in QEMU:
  In Progress

Bug description:
  Hello,
  Here is a reproducer:
  cat << EOF | ./i386-softmmu/qemu-system-i386 -M pc,accel=qtest\
   -qtest null -nographic -vga qxl -qtest stdio -nodefaults\
   -drive if=none,id=drive0,file=null-co://,file.read-zeroes=on,format=raw\
   -drive if=none,id=drive1,file=null-co://,file.read-zeroes=on,format=raw\
   -device ide-cd,drive=drive0 -device ide-hd,drive=drive1
  outw 0x176 0x3538
  outl 0xcf8 0x8903
  outl 0xcfc 0x184275c
  outb 0x376 0x2f
  outb 0x376 0x0
  outw 0x176 0xa1a4
  outl 0xcf8 0x8920
  outb 0xcfc 0xff
  outb 0xf8 0x9
  EOF

  The stack-trace:
  ==16513==ERROR: UndefinedBehaviorSanitizer: FPE on unknown address 
0x556783603fdc (pc 0x556783603fdc bp 0x7fff82143b10 sp 0x7fff82143ab0 T16513)
  #0 0x556783603fdc in ide_set_sector 
/home/alxndr/Development/qemu/hw/ide/core.c:626:26
  #1 0x556783603fdc in ide_dma_cb 
/home/alxndr/Development/qemu/hw/ide/core.c:883:9
  #2 0x55678349d74d in dma_complete 
/home/alxndr/Development/qemu/dma-helpers.c:120:9
  #3 0x55678349d74d in dma_blk_cb 
/home/alxndr/Development/qemu/dma-helpers.c:138:9
  #4 0x556783962f25 in blk_aio_complete 
/home/alxndr/Development/qemu/block/block-backend.c:1402:9
  #5 0x556783962f25 in blk_aio_complete_bh 
/home/alxndr/Development/qemu/block/block-backend.c:1412:5
  #6 0x556783ac030f in aio_bh_call 
/home/alxndr/Development/qemu/util/async.c:136:5
  #7 0x556783ac030f in aio_bh_poll 
/home/alxndr/Development/qemu/util/async.c:164:13
  #8 0x556783a9a863 in aio_dispatch 
/home/alxndr/Development/qemu/util/aio-posix.c:380:5
  #9 0x556783ac1b4c in aio_ctx_dispatch 
/home/alxndr/Development/qemu/util/async.c:306:5
  #10 0x7f4f1c0fe9ed in g_main_context_dispatch 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed)
  #11 0x556783acdccb in glib_pollfds_poll 
/home/alxndr/Development/qemu/util/main-loop.c:219:9
  #12 0x556783acdccb in os_host_main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:242:5
  #13 0x556783acdccb in main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:518:11
  #14 0x5567833613e5 in qemu_main_loop 
/home/alxndr/Development/qemu/softmmu/vl.c:1664:9
  #15 0x556783a07a4d in main 
/home/alxndr/Development/qemu/softmmu/main.c:49:5
  #16 0x7f4f1ac84e0a in __libc_start_main 
/build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16
  #17 0x5567830a9089 in _start 
(/home/alxndr/Development/qemu/build/i386-softmmu/qemu-system-i386+0x7d2089)

  With -trace ide*

  12163@1594585516.671265:ide_reset IDEstate 0x56162a269058
  [R +0.024963] outw 0x176 0x3538
  12163@1594585516.673676:ide_ioport_write IDE PIO wr @ 0x176 (Device/Head); 
val 0x38; bus 0x56162a268c00 IDEState 0x56162a268c88
  12163@1594585516.673683:ide_ioport_write IDE PIO wr @ 0x177 (Command); val 
0x35; bus 0x56162a268c00 IDEState 0x56162a269058
  12163@1594585516.673686:ide_exec_cmd IDE exec cmd: bus 0x56162a268c00; state 
0x56162a269058; cmd 0x35
  OK
  [S +0.025002] OK
  [R +0.025012] outl 0xcf8 0x8903
  OK
  [S +0.025018] OK
  [R +0.025026] outl 0xcfc 0x184275c
  OK
  [S +0.025210] OK
  [R +0.025219] outb 0x376 0x2f
  12163@1594585516.673916:ide_cmd_write IDE PIO wr @ 0x376 (Device Control); 
val 0x2f; bus 0x56162a268c00
  OK
  [S +0.025229] OK
  [R +0.025234] outb 0x376 0x0
  12163@1594585516.673928:ide_cmd_write IDE PIO wr @ 0x376 (Device Control); 
val 0x00; bus 0x56162a268c00
  OK
  [S +0.025240] OK
  [R +0.025246] outw 0x176 0xa1a4
  12163@1594585516.673940:ide_ioport_write IDE PIO wr @ 0x176 (Device/Head); 
val 0xa4; bus 0x56162a268c00 IDEState 0x56162a269058
  12163@1594585516.673943:ide_ioport_write IDE PIO wr @ 0x177 (Command); val 
0xa1; bus 0x56162a268c00 IDEState 0x56162a268c88
  12163@1594585516.673946:ide_exec_cmd IDE exec cmd: bus 0x56162a268c00; state 
0x56162a268c88; cmd 0xa1
  OK
  [S +0.025265] OK
  [R +0.025270] outl 0xcf8 0x8920
  OK
  [S +0.025274] OK
  [R +0.025279] outb 0xcfc 0xff
  OK
  [S +0.025443] OK
  [R +0.025451] outb 0xf8 0x9
  12163@1594585516.674221:ide_dma_cb IDEState 0x56162a268c88; sector_num=0 n=1 
cmd=DMA READ
  OK
  [S +0.025724] OK
  UndefinedBehaviorSanitizer:DEADLYSIGNAL
  ==12163==ERROR: UndefinedBehaviorSanitizer: FPE on unknown address 
0x5616279cffdc (pc 0x5616279cffdc bp 0x7ffcdaabae90 sp 0x7ffcdaabae30 T12163)

  -Alex

To manage notifications about this bug go

[Bug 1878253] Re: null-ptr dereference in address_space_to_flatview through ide

Proposed fix: https://lists.gnu.org/archive/html/qemu-
devel/2020-07/msg06974.html

** Changed in: qemu
 Assignee: (unassigned) => John Snow (jnsnow)

** Changed in: qemu
   Status: New => In Progress

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

Title:
  null-ptr dereference in address_space_to_flatview through ide

Status in QEMU:
  In Progress

Bug description:
  Hello,
  While fuzzing, I found an input that triggers a null-ptr dereference in
  address_space_to_flatview through ide:

  ==31699==ERROR: AddressSanitizer: SEGV on unknown address 0x0020 (pc 
0x55e0f562bafd bp 0x7ffee92355b0 sp 0x7ffee92354e0 T0)
  ==31699==The signal is caused by a READ memory access.
  ==31699==Hint: address points to the zero page.
  #0 0x55e0f562bafd in address_space_to_flatview 
/home/alxndr/Development/qemu/include/exec/memory.h:693:12
  #1 0x55e0f562bafd in address_space_write 
/home/alxndr/Development/qemu/exec.c:3267:14
  #2 0x55e0f562dd9c in address_space_unmap 
/home/alxndr/Development/qemu/exec.c:3592:9
  #3 0x55e0f5ab8277 in dma_memory_unmap 
/home/alxndr/Development/qemu/include/sysemu/dma.h:145:5
  #4 0x55e0f5ab8277 in dma_blk_unmap 
/home/alxndr/Development/qemu/dma-helpers.c:104:9
  #5 0x55e0f5ab8277 in dma_blk_cb 
/home/alxndr/Development/qemu/dma-helpers.c:139:5
  #6 0x55e0f617a6b8 in blk_aio_complete 
/home/alxndr/Development/qemu/block/block-backend.c:1398:9
  #7 0x55e0f617a6b8 in blk_aio_complete_bh 
/home/alxndr/Development/qemu/block/block-backend.c:1408:5
  #8 0x55e0f6355efb in aio_bh_call 
/home/alxndr/Development/qemu/util/async.c:136:5
  #9 0x55e0f6355efb in aio_bh_poll 
/home/alxndr/Development/qemu/util/async.c:164:13
  #10 0x55e0f63608ce in aio_dispatch 
/home/alxndr/Development/qemu/util/aio-posix.c:380:5
  #11 0x55e0f635799a in aio_ctx_dispatch 
/home/alxndr/Development/qemu/util/async.c:306:5
  #12 0x7f16e85d69ed in g_main_context_dispatch 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed)
  #13 0x55e0f635e384 in glib_pollfds_poll 
/home/alxndr/Development/qemu/util/main-loop.c:219:9
  #14 0x55e0f635e384 in os_host_main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:242:5
  #15 0x55e0f635e384 in main_loop_wait 
/home/alxndr/Development/qemu/util/main-loop.c:518:11
  #16 0x55e0f593d676 in qemu_main_loop 
/home/alxndr/Development/qemu/softmmu/vl.c:1664:9
  #17 0x55e0f6267c6a in main 
/home/alxndr/Development/qemu/softmmu/main.c:49:5
  #18 0x7f16e7186e0a in __libc_start_main 
/build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16
  #19 0x55e0f55727b9 in _start 
(/home/alxndr/Development/qemu/build/i386-softmmu/qemu-system-i386+0x9027b9)

  AddressSanitizer can not provide additional info.
  SUMMARY: AddressSanitizer: SEGV 
/home/alxndr/Development/qemu/include/exec/memory.h:693:12 in 
address_space_to_flatview

  I can reproduce it in qemu 5.0 using:

  cat << EOF | ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc 
-nographic -drive file=null-co://,if=ide,cache=writeback,format=raw -nodefaults 
-display none -nographic -qtest stdio -monitor none -serial none
  outl 0xcf8 0x8920
  outl 0xcfc 0xc001
  outl 0xcf8 0x8924
  outl 0xcf8 0x8904
  outw 0xcfc 0x7
  outb 0x1f7 0xc8
  outw 0x3f6 0xe784
  outw 0x3f6 0xeb01
  outb 0xc005 0x21
  write 0x2103 0x1 0x4e
  outb 0xc000 0x1b
  outw 0x1f7 0xff35
  EOF

  I also attached the traces to this launchpad report, in case the
  formatting is broken:

  qemu-system-i386 -M pc -nographic -drive file=null-
  co://,if=ide,cache=writeback,format=raw -nodefaults -display none
  -nographic -qtest stdio -monitor none -serial none < attachment

  Please let me know if I can provide any further info.
  -Alex

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

[Bug 1878255] Re: Assertion failure in bdrv_aio_cancel, through ide