date:20210422




在 2021/4/23 下午1:42, Stefan Weil 写道:

Am 23.04.21 um 05:18 schrieb Jason Wang:


There could be case that peer is NULL. This can happen when during
network device hot-add where net device needs to be added first. So
the patch check the existence of peer before trying to do the pad.

Fixes: 969e50b61a285 ("net: Pad short frames to minimum size before 
sending from SLiRP/TAP")

Signed-off-by: Jason Wang 
---
  include/net/net.h | 5 +
  net/slirp.c   | 2 +-
  net/tap-win32.c   | 2 +-
  net/tap.c | 2 +-
  4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index eff24519d2..1ef536d771 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -241,4 +241,9 @@ uint32_t net_crc32_le(const uint8_t *p, int len);
  .offset = vmstate_offset_macaddr(_state, _field), 

\

  }
  +static inline bool net_peer_needs_padding(NetClientState *nc)
+{
+  return nc->peer && !nc->peer->do_not_pad;
+}
+
  #endif
diff --git a/net/slirp.c b/net/slirp.c
index a01a0fccd3..7a4e96db5c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -119,7 +119,7 @@ static ssize_t net_slirp_send_packet(const void 
*pkt,size_t pkt_len,

  uint8_t min_pkt[ETH_ZLEN];
  size_t min_pktsz = sizeof(min_pkt);
  -    if (!s->nc.peer->do_not_pad) {
+    if (net_peer_needs_padding(&s->nc)) {
  if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pkt_len)) {
  pkt = min_pkt;
  pkt_len = min_pktsz;
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 897bd18e32..6096972f5d 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -696,7 +696,7 @@ static void tap_win32_send(void *opaque)
  if (size > 0) {
  orig_buf = buf;
  -    if (!s->nc.peer->do_not_pad) {
+    if (net_peer_needs_padding(&s->nc)) {
  if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
  buf = min_pkt;
  size = min_pktsz;
diff --git a/net/tap.c b/net/tap.c
index 7d53cedaec..820872fde8 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -203,7 +203,7 @@ static void tap_send(void *opaque)
  size -= s->host_vnet_hdr_len;
  }
  -    if (!s->nc.peer->do_not_pad) {
+    if (net_peer_needs_padding(&s->nc)) {
  if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
  buf = min_pkt;
  size = min_pktsz;



I assume that you had a test case which triggered that null pointer 
access?



Yes, it's simple to trigger by just adding a tap device and assign an IP 
to that.


Thanks



If yes, than this should indeed be applied before releasing 6.0.

The modification is simple enough for a last minute change.

Reviewed-by: Stefan Weil

Re: [PATCH RFC 0/1] To add HMP interface to dump PCI MSI-X table/PBA




在 2021/4/23 下午12:47, Dongli Zhang 写道:

This is inspired by the discussion with Jason on below patchset.

https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg09020.html

The new HMP command is introduced to dump the MSI-X table and PBA.

Initially, I was going to add new option to "info pci". However, as the
number of entries is not determined and the output of MSI-X table is much
more similar to the output of hmp_info_tlb()/hmp_info_mem(), this patch
adds interface for only HMP.

The patch is tagged with RFC because I am looking for suggestions on:

1. Is it fine to add new "info msix " command?



I wonder the reason for not simply reusing "info pci"?




2. Is there any issue with output format?



If it's not for QMP, I guess it's not a part of ABI so it should be fine.




3. Is it fine to add only for HMP, but not QMP?



I think so.

Thanks




Thank you very much!

Dongli Zhang

constant_tsc support for SVM guest

2021-04-22 Thread Wei Huang

There was a customer request for const_tsc support on AMD guests. Right 
now this feature is turned off by default for QEMU x86 CPU types (in 
CPUID_Fn8007_EDX[8]). However we are seeing a discrepancy in guest 
VM behavior between Intel and AMD.


In Linux kernel, Intel x86 code enables X86_FEATURE_CONSTANT_TSC based 
on vCPU's family & model. So it ignores CPUID_Fn8007_EDX[8] and 
guest VMs have const_tsc enabled. On AMD, however, the kernel checks 
CPUID_Fn8007_EDX[8]. So const_tsc is disabled on AMD by default.


I am thinking turning on invtsc for EPYC CPU types (see example below). 
Most AMD server CPUs have supported invariant TSC for a long time. So 
this change is compatible with the hardware behavior. The only problem 
is live migration support, which will be blocked because of invtsc. 
However this problem should be considered very minor because most server 
CPUs support TscRateMsr (see CPUID_Fn800A_EDX[4]), allowing VMs to 
migrate among CPUs with different TSC rates. This live migration 
restriction can be lifted as long as the destination supports TscRateMsr 
or has the same frequency as the source (QEMU/libvirt do it).


[BTW I believe this migration limitation might be unnecessary because it 
is apparently OK for Intel guests to ignore invtsc while claiming 
const_tsc. Have anyone reported issues?]


Do I miss anything here? Any comments about the proposal?

Thanks,
-Wei

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ad99cad0e7..3c48266884 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4077,6 +4076,21 @@ static X86CPUDefinition builtin_x86_defs[] = {
 { /* end of list */ }
 }
 },
+{
+.version = 4,
+.alias = "EPYC-IBPB",
+.props = (PropValue[]) {
+{ "ibpb", "on" },
+{ "perfctr-core", "on" },
+{ "clzero", "on" },
+{ "xsaveerptr", "on" },
+{ "xsaves", "on" },
+{ "invtsc", "on" },
+{ "model-id",
+  "AMD EPYC Processor" },
+{ /* end of list */ }
+}
+},
 { /* end of list */ }
 }
 },
@@ -4189,6 +4203,15 @@ static X86CPUDefinition builtin_x86_defs[] = {
 { /* end of list */ }
 }
 },
+{
+.version = 3,
+.props = (PropValue[]) {
+{ "ibrs", "on" },
+{ "amd-ssbd", "on" },
+{ "invtsc", "on" },
+{ /* end of list */ }
+}
+},
 { /* end of list */ }
 }
 },
@@ -4246,6 +4269,17 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .xlevel = 0x801E,
 .model_id = "AMD EPYC-Milan Processor",
 .cache_info = &epyc_milan_cache_info,
+.versions = (X86CPUVersionDefinition[]) {
+{ .version = 1 },
+{
+.version = 2,
+.props = (PropValue[]) {
+{ "invtsc", "on" },
+{ /* end of list */ }
+}
+},
+{ /* end of list */ }
+}

Re: [PATCH for 6.0] net: check the existence of peer before trying to pad

2021-04-22 Thread Stefan Weil


Am 23.04.21 um 05:18 schrieb Jason Wang:


There could be case that peer is NULL. This can happen when during
network device hot-add where net device needs to be added first. So
the patch check the existence of peer before trying to do the pad.

Fixes: 969e50b61a285 ("net: Pad short frames to minimum size before sending from 
SLiRP/TAP")
Signed-off-by: Jason Wang 
---
  include/net/net.h | 5 +
  net/slirp.c   | 2 +-
  net/tap-win32.c   | 2 +-
  net/tap.c | 2 +-
  4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index eff24519d2..1ef536d771 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -241,4 +241,9 @@ uint32_t net_crc32_le(const uint8_t *p, int len);
  .offset = vmstate_offset_macaddr(_state, _field),   

\

  }
  
+static inline bool net_peer_needs_padding(NetClientState *nc)

+{
+  return nc->peer && !nc->peer->do_not_pad;
+}
+
  #endif
diff --git a/net/slirp.c b/net/slirp.c
index a01a0fccd3..7a4e96db5c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -119,7 +119,7 @@ static ssize_t net_slirp_send_packet(const void *pkt, 
size_t pkt_len,
  uint8_t min_pkt[ETH_ZLEN];
  size_t min_pktsz = sizeof(min_pkt);
  
-if (!s->nc.peer->do_not_pad) {

+if (net_peer_needs_padding(&s->nc)) {
  if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pkt_len)) {
  pkt = min_pkt;
  pkt_len = min_pktsz;
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 897bd18e32..6096972f5d 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -696,7 +696,7 @@ static void tap_win32_send(void *opaque)
  if (size > 0) {
  orig_buf = buf;
  
-if (!s->nc.peer->do_not_pad) {

+if (net_peer_needs_padding(&s->nc)) {
  if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
  buf = min_pkt;
  size = min_pktsz;
diff --git a/net/tap.c b/net/tap.c
index 7d53cedaec..820872fde8 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -203,7 +203,7 @@ static void tap_send(void *opaque)
  size -= s->host_vnet_hdr_len;
  }
  
-if (!s->nc.peer->do_not_pad) {

+if (net_peer_needs_padding(&s->nc)) {
  if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
  buf = min_pkt;
  size = min_pktsz;



I assume that you had a test case which triggered that null pointer 
access? If yes, than this should indeed be applied before releasing 6.0.


The modification is simple enough for a last minute change.

Reviewed-by: Stefan Weil

[PATCH for-6.0 v2 1/2] hw/block/nvme: fix invalid msix exclusive uninit

2021-04-22 Thread Klaus Jensen

From: Klaus Jensen 

Commit 1901b4967c3f changed the nvme device from using a bar exclusive
for MSI-x to sharing it on bar0.

Unfortunately, the msix_uninit_exclusive_bar() call remains in
nvme_exit() which causes havoc when the device is removed with, say,
device_del. Fix this.

Additionally, a subregion is added but it is not removed on exit which
causes a reference to linger and the drive to never be unlocked.

Fixes: 1901b4967c3f ("hw/block/nvme: move msix table and pba to BAR 0")
Signed-off-by: Klaus Jensen 
---
 hw/block/nvme.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 624a1431d072..5fe082ec34c5 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -6235,7 +6235,8 @@ static void nvme_exit(PCIDevice *pci_dev)
 if (n->pmr.dev) {
 host_memory_backend_set_mapped(n->pmr.dev, false);
 }
-msix_uninit_exclusive_bar(pci_dev);
+msix_uninit(pci_dev, &n->bar0, &n->bar0);
+memory_region_del_subregion(&n->bar0, &n->iomem);
 }
 
 static Property nvme_props[] = {
-- 
2.31.1

[PATCH for-6.0 v2 2/2] hw/block/nvme: disable hotplugging for subsystem-linked controllers

2021-04-22 Thread Klaus Jensen

From: Klaus Jensen 

If a controller is linked to a subsystem, do not allow it to be
hotplugged since this will mess up the (possibly shared) namespaces.

Signed-off-by: Klaus Jensen 
---
 hw/block/nvme.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5fe082ec34c5..7606b58a39b9 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -6140,12 +6140,16 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 
 static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
 {
+DeviceClass *dc;
 int cntlid;
 
 if (!n->subsys) {
 return 0;
 }
 
+dc = DEVICE_GET_CLASS(n);
+dc->hotpluggable = false;
+
 cntlid = nvme_subsys_register_ctrl(n, errp);
 if (cntlid < 0) {
 return -1;
-- 
2.31.1

[PATCH for-6.0 v2 0/2] hw/block/nvme: fix msix uninit

2021-04-22 Thread Klaus Jensen

From: Klaus Jensen 

First patch fixes a regression where msix is not correctly uninit'ed
when an nvme device is hotplugged with device_del. When viewed in
conjunction with the commit that introduced the bug (commit
1901b4967c3f), I think the fix looks relatively obvious.

Second patch disables hotplugging for nvme controllers that are
connected to subsystems since the way namespaces are connected to the
nvme controller bus is messed up by removing the device. This bug causes
a segfault but is *not* a regression and is related to an experimental
feature.

v2:
  - remove memory subregion as well
  - add (possible) patch to disable hotplugging on subsystem connected
controllers

Klaus Jensen (2):
  hw/block/nvme: fix invalid msix exclusive uninit
  hw/block/nvme: disable hotplugging for subsystem-linked controllers

 hw/block/nvme.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.31.1

[PATCH RFC 1/1] msix: add hmp interface to dump MSI-X info

2021-04-22 Thread Dongli Zhang

This patch is to add the HMP interface to dump MSI-X table and PBA, in
order to help diagnose the loss of IRQ issue in VM (e.g., if an MSI-X
vector is erroneously masked permanently). Here is the example with
vhost-scsi:

(qemu) info msix /machine/peripheral/vscsi0
MSI-X Table
0xfee01004 0x 0x0022 0x
0xfee02004 0x 0x0023 0x
0xfee01004 0x 0x0023 0x
0xfee01004 0x 0x0021 0x
0xfee02004 0x 0x0022 0x
0x 0x 0x 0x0001
0x 0x 0x 0x0001
MSI-X PBA
0 0 0 0 0 0 0

Since the number of MSI-X entries is not determined and might be very
large, it is sometimes inappropriate to dump via QMP.

Therefore, this patch dumps MSI-X information only via HMP, which is
similar to the implementation of hmp_info_mem().

Cc: Jason Wang 
Cc: Joe Jin 
Signed-off-by: Dongli Zhang 
---
 hmp-commands-info.hx   | 13 +++
 hw/pci/msix.c  | 49 ++
 include/hw/pci/msix.h  |  2 ++
 include/monitor/hmp.h  |  1 +
 softmmu/qdev-monitor.c | 25 +
 5 files changed, 90 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ab0c7aa5ee..cbd056442b 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -221,6 +221,19 @@ SRST
 Show PCI information.
 ERST
 
+{
+.name   = "msix",
+.args_type  = "dev:s",
+.params = "dev",
+.help   = "dump MSI-X information",
+.cmd= hmp_info_msix,
+},
+
+SRST
+  ``info msix`` *dev*
+Dump MSI-X information for device *dev*.
+ERST
+
 #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \
 defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
 {
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index ae9331cd0b..a93d31da9f 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -22,6 +22,7 @@
 #include "sysemu/xen.h"
 #include "migration/qemu-file-types.h"
 #include "migration/vmstate.h"
+#include "monitor/monitor.h"
 #include "qemu/range.h"
 #include "qapi/error.h"
 #include "trace.h"
@@ -669,3 +670,51 @@ const VMStateDescription vmstate_msix = {
 VMSTATE_END_OF_LIST()
 }
 };
+
+static void msix_dump_table(Monitor *mon, PCIDevice *dev)
+{
+int vector, i, offset;
+uint32_t val;
+
+monitor_printf(mon, "MSI-X Table\n");
+
+for (vector = 0; vector < dev->msix_entries_nr; vector++) {
+for (i = 0; i < 4; i++) {
+offset = vector * PCI_MSIX_ENTRY_SIZE + i * 4;
+val = pci_get_long(dev->msix_table + offset);
+
+monitor_printf(mon, "0x%08x ", val);
+}
+monitor_printf(mon, "\n");
+}
+}
+
+static void msix_dump_pba(Monitor *mon, PCIDevice *dev)
+{
+int vector;
+
+monitor_printf(mon, "MSI-X PBA\n");
+
+for (vector = 0; vector < dev->msix_entries_nr; vector++) {
+monitor_printf(mon, "%d ", !!msix_is_pending(dev, vector));
+
+if (vector % 16 == 15) {
+monitor_printf(mon, "\n");
+}
+}
+
+if (vector % 16 != 15) {
+monitor_printf(mon, "\n");
+}
+}
+
+void msix_dump_info(Monitor *mon, PCIDevice *dev, Error **errp)
+{
+if (!msix_present(dev)) {
+error_setg(errp, "MSI-X not available");
+return;
+}
+
+msix_dump_table(mon, dev);
+msix_dump_pba(mon, dev);
+}
diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
index 4c4a60c739..10a4500295 100644
--- a/include/hw/pci/msix.h
+++ b/include/hw/pci/msix.h
@@ -47,6 +47,8 @@ int msix_set_vector_notifiers(PCIDevice *dev,
   MSIVectorPollNotifier poll_notifier);
 void msix_unset_vector_notifiers(PCIDevice *dev);
 
+void msix_dump_info(Monitor *mon, PCIDevice *dev, Error **errp);
+
 extern const VMStateDescription vmstate_msix;
 
 #define VMSTATE_MSIX_TEST(_field, _state, _test) {   \
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 605d57287a..46e0efc213 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -36,6 +36,7 @@ void hmp_info_irq(Monitor *mon, const QDict *qdict);
 void hmp_info_pic(Monitor *mon, const QDict *qdict);
 void hmp_info_rdma(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
+void hmp_info_msix(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
 void hmp_info_iothreads(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index a9955b97a0..2a37d03fb7 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/sysbus.h"
+#include "hw/pci/msix.h"
 #include "monitor/hmp.h"
 #include "monitor/monitor.h"
 #include "monitor/qdev.h"
@@ -1006,3 +1007,27 @@ bool qmp_command_available(const QmpCommand *cmd, Error 
**errp)
 }
 return tru

[PATCH RFC 0/1] To add HMP interface to dump PCI MSI-X table/PBA

2021-04-22 Thread Dongli Zhang

This is inspired by the discussion with Jason on below patchset.

https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg09020.html

The new HMP command is introduced to dump the MSI-X table and PBA.

Initially, I was going to add new option to "info pci". However, as the
number of entries is not determined and the output of MSI-X table is much
more similar to the output of hmp_info_tlb()/hmp_info_mem(), this patch
adds interface for only HMP.

The patch is tagged with RFC because I am looking for suggestions on:

1. Is it fine to add new "info msix " command?

2. Is there any issue with output format?

3. Is it fine to add only for HMP, but not QMP?

Thank you very much!

Dongli Zhang

[Bug 1580459] Re: Windows (10?) guest freezes entire host on shutdown if using PCI passthrough

2021-04-22 Thread Thomas Huth

Ok, thanks for answering! So I'm closing this issue now. In case anybody
still has similar issues, please open a new bug ticket instead.

** Changed in: qemu
   Status: Incomplete => Fix Released

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

Title:
  Windows (10?) guest freezes entire host on shutdown if using PCI
  passthrough

Status in libvirt:
  New
Status in QEMU:
  Fix Released
Status in Arch Linux:
  New
Status in Debian:
  New
Status in Fedora:
  New

Bug description:
  Problem: after leaving a Windows VM that uses PCI passthrough (as we
  do for gaming graphics cards, sound cards, and in my case, a USB card)
  running for some amount of time between 1 and 2 hours (it's not
  consistent with exactly how long), and for any amount of time longer
  than that, shutting down that guest will, right as it finishes
  shutting down, freeze the host computer, making it require a hard
  reboot. Unbinding (or in the other user's case, unbinding and THEN
  binding) any PCI device in sysfs, even one that has nothing to do with
  the VM, also has the same effect as shutting down the VM (if the VM
  has been running long enough). So, it's probably an issue related to
  unbinding and binding PCI devices.

  There's a lot of info on this problem over at 
https://bbs.archlinux.org/viewtopic.php?id=206050
  Here's a better-organized list of main details:
  -at least 2 confirmed victims of this bug; 2 (including me) have provided 
lots of info in the link
  -I'm on Arch Linux and the other one is on Gentoo (distro-nonspecific)
  -issue affects my Windows 10 guest and others' Windows guests, but not my 
Arch Linux guest (the others don't have non-Windows guests to test)
  -I'm using libvirt but the other user is not, so it's not an issue with 
libvirt
  -It seems to be version non-specific, too. I first noticed it at, or when 
testing versions still had the issue at (whichever version is lower), Linux 4.1 
and qemu 2.4.0. It still persists in all releases of both since, including the 
newest ones.
  -I can't track down exactly what package downgrade can fix it, as downgrading 
further than Linux 4.1 and qemu 2.4.0 requires Herculean and system-destroying 
changes such as downgrading ncurses, meaning I don't know whether it's a bug in 
QEMU, the Linux kernel, or some weird seemingly unrelated thing.
  -According to the other user, "graphics intensive gameplay (GTA V) can cause 
the crash to happen sooner," as soon as "15 minutes"
  -Also, "bringing up a second passthrough VM with separate hardware will cause 
the same crash," and "bringing up another VM before the two-hour mark will not 
result in a crash," further cementing that it's triggered by the un/binding of 
PCI devices.
  -This is NOT related to the very similar bug that can be worked around by not 
passing through the HDMI device or sound card. Even when we removed all traces 
of any sort of sound card from the VM, it still had the same behavior.

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

[Bug 1395217] Re: Networking in qemu 2.0.0 and beyond is not compatible with Open Solaris (Illumos) 5.11

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

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

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

Title:
  Networking in qemu 2.0.0 and beyond is not compatible with Open
  Solaris (Illumos) 5.11

Status in QEMU:
  Expired

Bug description:
  The networking code in qemu in versions 2.0.0 and beyond is non-
  functional with Solaris/Illumos 5.11 images.

  Building 1.7.1, 2.0.0, 2.0.2, 2.1.2,and 2.2.0rc1with the following
  standard Slackware config:

  # From Slackware build tree . . . 
  ./configure \
--prefix=/usr \
--libdir=/usr/lib64 \
--sysconfdir=/etc \
--localstatedir=/var \
--enable-gtk \
--enable-system \
--enable-kvm \
--disable-debug-info \
--enable-virtfs \
--enable-sdl \
--audio-drv-list=alsa,oss,sdl,esd \
--enable-libusb \
--disable-vnc \
--target-list=x86_64-linux-user,i386-linux-user,x86_64-softmmu,i386-softmmu 
\
--enable-spice \
--enable-usb-redir 

  
  And attempting to run the same VM image with the following command (or via 
virt-manager):

  macaddress="DE:AD:BE:EF:3F:A4"

  qemu-system-x86_64 nex4x -cdrom /dev/cdrom -name "Nex41" -cpu Westmere
  -machine accel=kvm -smp 2 -m 4000 -net nic,macaddr=$macaddress  -net 
bridge,br=b
  r0 -net dump,file=/usr1/tmp/ -drive file=nex4x_d1 -drive 
file=nex4x_d2
   -enable-kvm

  Gives success on 1.7.1, and a deaf VM on all subsequent versions.

  Notable in validating my config, is that a Windows 7 image runs
  cleanly with networking on *all* builds, so my configuration appears
  to be good - qemu just hates Solaris at this point.

  Watching with wireshark (as well as pulling network traces from qemu
  as noted above) it appears that the notable difference in the two
  configs is that for some reason, Solaris gets stuck arping for it's
  own interface on startup, and never really comes on line on the
  network.  If other hosts attempt to ping the Solaris instance, they
  can successfully arp the bad VM, but not the other way around.

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

[Bug 1778966] Re: Windows 1803 and later crashes on KVM

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

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

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

Title:
  Windows 1803 and later crashes on KVM

Status in QEMU:
  Expired

Bug description:
  For a bionic host, using the current public kvm modules, KVM is not
  capable of booting a WindowsInsider or msdn Windows 1803 Windows
  Server iso. In snstalling from an ISO from a started windows 2016
  guest results in an unbootable and unrepairable guest.

  The hardware is a threadripper 1920x with 32GB of main memory, disk
  mydigital BPX SSD and WD based 4 column RAID 5 via mdadm.

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

[Bug 1615212] Re: SDL UI switching to monitor half-broken and scrolling broken

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

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

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

Title:
  SDL UI switching to monitor half-broken and scrolling broken

Status in QEMU:
  Expired

Bug description:
  ctrl+alt+2 must be pressed 2 or more times for the monitor console
  window to appear with -sdl, the window flashes and disappears also
  before finally staying open

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

[Bug 1725707] Re: QEMU sends excess VNC data to websockify even when network is poor

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

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

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

Title:
  QEMU sends excess VNC data to websockify even when network is poor

Status in QEMU:
  Expired

Bug description:
  Description of problem
  -
  In my latest topic, I reported a bug relate to QEMU's websocket:
  https://bugs.launchpad.net/qemu/+bug/1718964

  It has been fixed but someone mentioned that he met the same problem when 
using QEMU with a standalone websocket proxy.
  That makes me confused because in that scenario QEMU will get a "RAW" VNC 
connection.
  So I did a test and found that there indeed existed some problems. The 
problem is:

  When the client's network is poor (on a low speed WAN), QEMU still
  sends a lot of data to the websocket proxy, then the client get stuck.
  It seems that only QEMU has this problem, other VNC servers works
  fine.

  Environment
  -
  All of the following versions have been tested:

  QEMU: 2.8.1.1 / 2.9.1 / 2.10.1 / master (Up to date)
  Host OS: Ubuntu 16.04 Server LTS / CentOS 7 x86_64_1611
  Websocket Proxy: websockify 0.6.0 / 0.7.0 / 0.8.0 / master
  VNC Web Client: noVNC 0.5.1 / 0.61 / 0.62 / master
  Other VNC Servers: TigerVNC 1.8 / x11vnc 0.9.13 / TightVNC 2.8.8

  Steps to reproduce:
  -
  100% reproducible.

  1. Launch a QEMU instance (No need websocket option):
  qemu-system-x86_64 -enable-kvm -m 6G ./win_x64.qcow2 -vnc :0

  2. Launch websockify on a separate host and connect to QEMU's VNC port

  3. Open VNC Web Client (noVNC/vnc.html) in browser and connect to
  websockify

  4. Play a video (e.g. Watch YouTube) on VM (To produce a lot of frame
  buffer update)

  5. Limit (e.g. Use NetLimiter) the client inbound bandwidth to 300KB/S
  (To simulate a low speed WAN)

  6. Then client's output gets stuck(less than 1 fps), the cursor is
  almost impossible to move

  7. Monitor network traffic on the proxy server

  Current result:
  -
  Monitor Downlink/Uplink network traffic on the proxy server
  (Refer to the attachments for more details).

  1. Used with QEMU
  - D: 5.9 MB/s U: 5.7 MB/s (Client on LAN)
  - D: 4.3 MB/s U: 334 KB/s (Client on WAN)

  2. Used with other VNC servers
  - D: 5.9 MB/s U: 5.6 MB/s (Client on LAN)
  - D: 369 KB/s U: 328 KB/s (Client on WAN)

  It is found that when the client's network is poor, all the VNC
  servers (tigervnc/x11vnc/tightvnc) will reduce the VNC data send to
  websocket proxy (uplink and downlink symmetry), but QEMU never drop
  any frames and still sends a lot of data to websockify, the client has
  no capacity to accept so much data, more and more data are accumulated
  in the websockify, then it crashes.

  Expected results:
  -
  When the client's network is poor (WAN), QEMU will reduce the VNC data send 
to websocket proxy.

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

[Bug 1820247] Re: QEMU random crash caused by libspice-server

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

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

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

Title:
  QEMU random crash caused by libspice-server

Status in QEMU:
  Expired

Bug description:
  Hi,

  One of our OpenStack instances crashed. It seems there was some
  problem related to SPICE. Attaching what we had in qemu log. Also
  sending our versions:

  Linux pre-node1 4.18.0-13-generic #14~18.04.1-Ubuntu SMP Thu Dec 6
  14:09:52 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

  QEMU emulator version 2.11.1(Debian 1:2.11+dfsg-1ubuntu7.9)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  
  root@pre-node1:~# cat /var/log/libvirt/qemu/instance-0038.log 
  2019-03-10 20:39:36.510+: starting up libvirt version: 4.0.0, package: 
1ubuntu8.6 (Christian Ehrhardt  Fri, 09 Nov 
2018 07:42:01 +0100), qemu version: 2.11.1(Debian 1:2.11+dfsg-1ubuntu7.9), 
hostname: pre-node1
  LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin 
QEMU_AUDIO_DRV=spice /usr/bin/kvm-spice -name 
guest=instance-0038,debug-threads=on -S -object 
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-5-instance-0038/master-key.aes
 -machine pc-i440fx-bionic,accel=kvm,usb=off,dump-guest-core=off,mem-merge=off 
-cpu 
Skylake-Server-IBRS,ss=on,hypervisor=on,tsc_adjust=on,clflushopt=on,pku=on,ssbd=on,xsaves=on
 -m 2048 -realtime mlock=on -smp 2,sockets=1,cores=1,threads=2 -object 
memory-backend-file,id=ram-node0,prealloc=yes,mem-path=/dev/hugepages/libvirt/qemu/5-instance-0038,share=yes,size=2147483648,host-nodes=0,policy=bind
 -numa node,nodeid=0,cpus=0-1,memdev=ram-node0 -uuid 
3c3d04f3-4b25-4ea5-8836-0e06eef9dcb7 -smbios 'type=1,manufacturer=OpenStack 
Foundation,product=OpenStack 
Nova,version=18.1.1,serial=93fa1a55-ba3a-4a99-80b3-3a7bb4e964af,uuid=3c3d04f3-4b25-4ea5-8836-0e06eef9dcb7,family=Virtual
 Machine' -no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-5-instance-0038/monitor.sock,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew 
-global kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -boot strict=on 
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device 
virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -drive 
file=/var/lib/nova/instances/3c3d04f3-4b25-4ea5-8836-0e06eef9dcb7/disk,format=qcow2,if=none,id=drive-virtio-disk0,cache=none,discard=ignore,throttling.iops-read=5000,throttling.iops-write=5000
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -add-fd set=0,fd=29 -chardev 
pty,id=charserial0,logfile=/dev/fdset/0,logappend=on -device 
isa-serial,chardev=charserial0,id=serial0 -chardev 
spicevmc,id=charchannel0,name=vdagent -device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
 -spice port=5900,addr=10.252.0.101,disable-ticketing,seamless-migration=on 
-device 
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2
 -device vfio-pci,host=25:04.1,id=hostdev0,bus=pci.0,addr=0x5 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 -msg timestamp=on
  2019-03-10T20:39:36.568276Z qemu-system-x86_64: -chardev 
pty,id=charserial0,logfile=/dev/fdset/0,logappend=on: char device redirected to 
/dev/pts/2 (label charserial0)
  inputs_channel_detach_tablet: 
  main_channel_link: add main channel client
  main_channel_client_handle_pong: net test: latency 32.76 ms, bitrate 
33384953 bps (31.838372 Mbps)
  red_qxl_set_cursor_peer: 
  inputs_connect: inputs channel client create

  (process:65324): Spice-WARNING **: 16:35:23.769: Failed to create channel 
client: Client 0x55e7c157e970: duplicate channel type 2 id 0
  red_qxl_set_cursor_peer: 

  (process:65324): Spice-WARNING **: 16:35:24.142: Failed to create
  channel client: Client 0x55e7c157e970: duplicate channel type 4 id 0

  (process:65324): Spice-CRITICAL **: 16:35:24.142: 
cursor-channel.c:353:cursor_channel_connect: condition `ccc != NULL' failed
  2019-03-13 15:35:31.785+: shutting down, reason=crashed


  
  I am also attaching some gdb information extracted from qemu crash dump file. 
These are backtraces of particular threads within the crashed QEMU process.

  
  Thread 9 (Thread 0x7f69649ea5c0 (LWP 65324)):
  #0  0x7f695f02d2b7 in __libc_write (fd=26, buf=0x7ffc33f5b330, nbytes=56) 
at ../sysdeps/unix/sysv/linux/write.c:27
  #1  0x7f695ff30ed3 in  () at 
/usr/lib/x86_64-linux-gnu/libspice-server.so.1
  #2  0x7f695ff316ce in  () at 
/usr/lib/x86_64-linux-gnu/libspice-server.so.1
  #3  0x7f695ff52db6 in  () at 
/usr/lib/x86_64-linux-gnu/libspice-server.so.1
  #4  0x7f695ff58e38 in  () at 
/usr/lib/x86_64-linux-gnu

[Bug 1879425] Re: The thread of "CPU 0 /KVM" keeping 99.9%CPU

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

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

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

Title:
  The thread of "CPU 0 /KVM" keeping 99.9%CPU

Status in QEMU:
  Expired

Bug description:
  Hi Expert:

  The VM is hung here after (2, or 3, or 5 and the longest time is 10 hours) by 
qemu-kvm.
  Notes:
  for VM:
    OS: RHEL8.1
    CPU: 1
    MEM:4G
  For qemu-kvm(host kernel RHEL7):
    1) version:
   /usr/libexec/qemu-kvm -version
   QEMU emulator version 2.10.0(qemu-kvm-ev-2.10.0-21.el7_5.4.1)
    2) once the issue is occurred, the CPU of "CPU0 /KVM" is more than 99% by 
com "top -p VM_pro_ID"
  PID  UDER   PR NI RES   S  % CPU %MEM  TIME+COMMAND
  872067   qemu   20 0  1.6g  R   99.9  0.6  37:08.87 CPU 0/KVM
    3) use "pstack 493307" and below is function trace
  Thread 1 (Thread 0x7f2572e73040 (LWP 872067)):
  #0  0x7f256cad8fcf in ppoll () from /lib64/libc.so.6
  #1  0x55ff34bdf4a9 in qemu_poll_ns ()
  #2  0x55ff34be02a8 in main_loop_wait ()
  #3  0x55ff348bfb1a in main ()
    4) use strace "strace -tt -ff -p 872067 -o cfx" and below log keep printing
  21:24:02.977833 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8, 
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82, 
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121, 
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
  21:24:02.977918 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8, 
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82, 
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121, 
events=POLLIN}], 9, {0, 911447}, NULL, 8) = 0 (Timeout)
  21:24:02.978945 ppoll([{fd=4, events=POLLIN}, {fd=6, events=POLLIN}, {fd=8, 
events=POLLIN}, {fd=9, events=POLLIN}, {fd=80, events=POLLIN}, {fd=82, 
events=POLLIN}, {fd=84, events=POLLIN}, {fd=115, events=POLLIN}, {fd=121, 
events=POLLIN}], 9, {0, 0}, NULL, 8) = 0 (Timeout)
  Therefore, I think the thread "CPU 0/KVM" is in tight loop.
    5) use reset can recover this issue. however, it will reoccurred again.
  Current work around is increase one CPU for this VM, then issue is gone.

  thanks
  Cliff

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

[Bug 1642421] Re: qemu-system-x86_64: ipv6 and dns is broken with netdev user

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

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

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

Title:
  qemu-system-x86_64: ipv6 and dns is broken with netdev user

Status in QEMU:
  Expired

Bug description:
  Hi,

  dhcp inside qemu returns an ipv6 address as dns-server. However this is not
  working. If i replace it with the ipv4 address '10.0.0.2' dns is working
  again. I would expect that the qemu emulated dhcp server responds either an
  ipv4 configuration that is working or its dns server/forwarder listens on the
  ipv6 address returned by the emulated dhcp server.

  I used latest qemu from git (
  b0bcc86d2a87456f5a276f941dc775b265b309cf) and used the following
  commands:

  $ ./qemu-system-x86_64 -enable-kvm -M pc -device virtio-rng-pci -device
  virtio-net-pci,netdev=user.0 -drive file=buildenv.img,if=virtio,bus=1,unit=0
  -no-reboot -netdev 
user,id=user.0,hostfwd=tcp::5022-:22,hostfwd=tcp::7587-:7588
  -m 1024 -usb -nographic -smp 4

  buildenv.img is a debian jessie amd64 installation.

  Inside qemu the network is configured to use dhcp:

  $ cat /etc/network/interfaces
  allow-hotplug eth0
  iface eth0 inet dhcp

  $ ifconfig eth0
  eth0  Link encap:Ethernet  HWaddr 52:54:00:12:34:56
inet addr:10.0.2.15  Bcast:10.0.2.255  Mask:255.255.255.0
inet6 addr: fe80::5054:ff:fe12:3456/64 Scope:Link
inet6 addr: fec0::5054:ff:fe12:3456/64 Scope:Site
UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
RX packets:10 errors:0 dropped:0 overruns:0 frame:0
TX packets:28 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:3215 (3.1 KiB)  TX bytes:3638 (3.5 KiB)

  $ cat /etc/resolv.conf
  nameserver fec0::3

  $ arp google.de
  google.de: Host name lookup failure

  $ strace -f arp google.de
  ...
  socket(PF_INET6, SOCK_DGRAM|SOCK_NONBLOCK, IPPROTO_IP) = 4
  connect(4, {sa_family=AF_INET6, sin6_port=htons(53), inet_pton(AF_INET6, 
"fec0::3", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = 0
  poll([{fd=4, events=POLLOUT}], 1, 0)= 1 ([{fd=4, revents=POLLOUT}])
  sendto(4, "\17\320\1\0\0\1\0\0\0\0\0\0\6google\2de\0\0\1\0\1", 27, 
MSG_NOSIGNAL, NULL, 0) = 27
  poll([{fd=4, events=POLLIN}], 1, 5000)  = 0 (Timeout)
  poll([{fd=4, events=POLLOUT}], 1, 0)= 1 ([{fd=4, revents=POLLOUT}])
  sendto(4, "\17\320\1\0\0\1\0\0\0\0\0\0\6google\2de\0\0\1\0\1", 27, 
MSG_NOSIGNAL, NULL, 0) = 27
  poll([{fd=4, events=POLLIN}], 1, 5000)  = 0 (Timeout)
  close(4)= 0
  ...

  $ echo nameserver 10.0.0.2 > /etc/resolv.conf

  $ arp google.de
  google.de (216.58.208.35) -- no entry

  Note: I reported this bug also to debian: https://bugs.debian.org/cgi-
  bin/bugreport.cgi?bug=844566

  Regards,

Manuel

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

Re: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg

2021-04-22 Thread David Gibson

On Thu, Apr 22, 2021 at 04:35:34PM -0300, Fabiano Rosas wrote:
> Bruno Piazera Larsen  writes:
> 
> >> > You are correct! I've just tweaked the code that defines spr_register and
> >> > it should be working now. I'm still working in splitting the SPR 
> >> > functions
> >> > from translate_init, since I think it would make it easier to prepare the
> >> > !TCG case and for adding new architectures in the future, and I found a
> >> > few more problems:
> >>
> >> Actually looking at the stuff below, I suspect that separating our
> >> "spr" logic specifically might be a bad idea.  At least some of the
> >> SPRs control pretty fundamental things about how the processor
> >> operates, and I suspect separating it from the main translation logic
> >> may be more trouble than it's worth.
> 
> I disagree with the code proximity argument. Having TCG code clearly
> separate from common code seems more important to me than having the SPR
> callbacks close to the init_proc functions.

Hmm.. I may be misinterpreting what you're intending here.  I
certainly agree that separating TCG only code from common code is a
good idea.  My point, though, is that the vast majority of the SPR
code *is* TCG specific - there are just a relatively few cases where
SPRs have a common path.  That basically only happens when a) the SPR
can be affected by means other than the guest executing instructions
specifically to do that (i.e. usually by hypercalls) and b) accessing
the SPR has some side effects that need to be handled in both TCG and
KVM cases

From the descriptions it sounded like you were trying to separate
*all* SPR code, not just these specific cases from the translation
core, and that's what I'm saying is a bad idea.

> But maybe we should take a look at this RFC before we start discussing
> personal preference too much.
> 
> > Well, all the errors that I got were related to to read/write functions, 
> > which
> > I was already separating into a spr_tcg file. The solutions I can see are to
> > include this file in translate.c, and either have the read/write functions 
> > not be
> > static, or include the spr_common.c in translate as well, but only for TCG
> > builds. Both solutions sound pretty bad imo, but the first sounds less bad,
> > because it's a bit less complexity in the build process.
> 
> It would be helpful if we could apply these patches and do some
> experimentation before recommending a solution. So I would pick the less
> bad for now. Mention it in the cover letter and then we can discuss
> looking at something more concrete.
> 

-- 
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 for 6.0] net: check the existence of peer before trying to pad

2021-04-22 Thread Bin Meng

On Fri, Apr 23, 2021 at 11:18 AM Jason Wang  wrote:
>
> There could be case that peer is NULL. This can happen when during
> network device hot-add where net device needs to be added first. So
> the patch check the existence of peer before trying to do the pad.
>
> Fixes: 969e50b61a285 ("net: Pad short frames to minimum size before sending 
> from SLiRP/TAP")
> Signed-off-by: Jason Wang 
> ---
>  include/net/net.h | 5 +
>  net/slirp.c   | 2 +-
>  net/tap-win32.c   | 2 +-
>  net/tap.c | 2 +-
>  4 files changed, 8 insertions(+), 3 deletions(-)
>

Reviewed-by: Bin Meng

[PATCH for 6.0] net: check the existence of peer before trying to pad

There could be case that peer is NULL. This can happen when during
network device hot-add where net device needs to be added first. So
the patch check the existence of peer before trying to do the pad.

Fixes: 969e50b61a285 ("net: Pad short frames to minimum size before sending 
from SLiRP/TAP")
Signed-off-by: Jason Wang 
---
 include/net/net.h | 5 +
 net/slirp.c   | 2 +-
 net/tap-win32.c   | 2 +-
 net/tap.c | 2 +-
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index eff24519d2..1ef536d771 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -241,4 +241,9 @@ uint32_t net_crc32_le(const uint8_t *p, int len);
 .offset = vmstate_offset_macaddr(_state, _field),\
 }
 
+static inline bool net_peer_needs_padding(NetClientState *nc)
+{
+  return nc->peer && !nc->peer->do_not_pad;
+}
+
 #endif
diff --git a/net/slirp.c b/net/slirp.c
index a01a0fccd3..7a4e96db5c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -119,7 +119,7 @@ static ssize_t net_slirp_send_packet(const void *pkt, 
size_t pkt_len,
 uint8_t min_pkt[ETH_ZLEN];
 size_t min_pktsz = sizeof(min_pkt);
 
-if (!s->nc.peer->do_not_pad) {
+if (net_peer_needs_padding(&s->nc)) {
 if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pkt_len)) {
 pkt = min_pkt;
 pkt_len = min_pktsz;
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 897bd18e32..6096972f5d 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -696,7 +696,7 @@ static void tap_win32_send(void *opaque)
 if (size > 0) {
 orig_buf = buf;
 
-if (!s->nc.peer->do_not_pad) {
+if (net_peer_needs_padding(&s->nc)) {
 if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
 buf = min_pkt;
 size = min_pktsz;
diff --git a/net/tap.c b/net/tap.c
index 7d53cedaec..820872fde8 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -203,7 +203,7 @@ static void tap_send(void *opaque)
 size -= s->host_vnet_hdr_len;
 }
 
-if (!s->nc.peer->do_not_pad) {
+if (net_peer_needs_padding(&s->nc)) {
 if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
 buf = min_pkt;
 size = min_pktsz;
-- 
2.25.1

[PATCH RESEND 1/2] target/i386: add "-cpu, lbr-fmt=*" support to enable guest LBR

2021-04-22 Thread Like Xu

The last branch recording (LBR) is a performance monitor unit (PMU)
feature on Intel processors that records a running trace of the most
recent branches taken by the processor in the LBR stack. The QEMU
could configure whether it's enabled or not for each guest via CLI.

The LBR feature would be enabled on the guest if:
- the KVM is enabled and the PMU is enabled and,
- the msr-based-feature IA32_PERF_CAPABILITIES is supporterd on KVM and,
- the supported returned value for lbr_fmt from this msr is not zero and,
- the requested guest vcpu model does support FEAT_1_ECX.CPUID_EXT_PDCM,
- the configured lbr-fmt value is the same as the host lbr_fmt value
  or use the QEMU option "-cpu host,migratable=no".

Cc: Eduardo Habkost 
Cc: Paolo Bonzini 
Signed-off-by: Like Xu 
---
 target/i386/cpu.c | 16 
 target/i386/cpu.h | 10 ++
 target/i386/kvm/kvm.c |  5 +++--
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ad99cad0e7..eee6da3ad8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6627,6 +6627,13 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool 
verbose)
 x86_cpu_get_supported_feature_word(w, false);
 uint64_t requested_features = env->features[w];
 uint64_t unavailable_features = requested_features & ~host_feat;
+if (kvm_enabled() && w == FEAT_PERF_CAPABILITIES &&
+(requested_features & PERF_CAP_LBR_FMT)) {
+if ((host_feat & PERF_CAP_LBR_FMT) !=
+(requested_features & PERF_CAP_LBR_FMT)) {
+unavailable_features |= PERF_CAP_LBR_FMT;
+}
+}
 mark_unavailable_features(cpu, w, unavailable_features, prefix);
 }
 
@@ -6734,6 +6741,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 }
 
+if (cpu->lbr_fmt) {
+if (!cpu->enable_pmu) {
+error_setg(errp, "LBR is unsupported since guest PMU is 
disabled.");
+return;
+}
+env->features[FEAT_PERF_CAPABILITIES] |= cpu->lbr_fmt;
+}
+
 /* mwait extended info: needed for Core compatibility */
 /* We always wake on interrupt even if host does not have the capability */
 cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
@@ -7300,6 +7315,7 @@ static Property x86_cpu_properties[] = {
 #endif
 DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
 DEFINE_PROP_BOOL("pmu", X86CPU, enable_pmu, false),
+DEFINE_PROP_UINT8("lbr-fmt", X86CPU, lbr_fmt, 0),
 
 DEFINE_PROP_UINT32("hv-spinlocks", X86CPU, hyperv_spinlock_attempts,
HYPERV_SPINLOCK_NEVER_NOTIFY),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 570f916878..b12c879fc4 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -354,6 +354,7 @@ typedef enum X86Seg {
 #define ARCH_CAP_TSX_CTRL_MSR  (1<<7)
 
 #define MSR_IA32_PERF_CAPABILITIES  0x345
+#define PERF_CAP_LBR_FMT  0x3f
 
 #define MSR_IA32_TSX_CTRL  0x122
 #define MSR_IA32_TSCDEADLINE0x6e0
@@ -1726,6 +1727,15 @@ struct X86CPU {
  */
 bool enable_pmu;
 
+/*
+ * Configure LBR_FMT bits on IA32_PERF_CAPABILITIES MSR.
+ * This can't be enabled by default yet because it doesn't have
+ * ABI stability guarantees, as it is only allowed to pass all
+ * LBR_FMT bits returned by kvm_arch_get_supported_msr_feature()
+ * (that depends on host CPU and kernel capabilities) to the guest.
+ */
+uint8_t lbr_fmt;
+
 /* LMCE support can be enabled/disabled via cpu option 'lmce=on/off'. It is
  * disabled by default to avoid breaking migration between QEMU with
  * different LMCE configurations.
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 7fe9f52710..4d842d32a6 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2732,8 +2732,9 @@ static void kvm_msr_entry_add_perf(X86CPU *cpu, 
FeatureWordArray f)
MSR_IA32_PERF_CAPABILITIES);
 
 if (kvm_perf_cap) {
-kvm_msr_entry_add(cpu, MSR_IA32_PERF_CAPABILITIES,
-kvm_perf_cap & f[FEAT_PERF_CAPABILITIES]);
+kvm_perf_cap = cpu->migratable ?
+(kvm_perf_cap & f[FEAT_PERF_CAPABILITIES]) : kvm_perf_cap;
+kvm_msr_entry_add(cpu, MSR_IA32_PERF_CAPABILITIES, kvm_perf_cap);
 }
 }
 
-- 
2.30.2

[PATCH RESEND 2/2] target/i386: add kvm_exact_match_flags to FeatureWordInfo

2021-04-22 Thread Like Xu

Instead of hardcoding the PERF_CAPABILITIES rules in this loop,
this could become a FeatureWordInfo field. It would be very useful
for other features like intel-pt, where we need some bits to match
the host bits too.

Suggested-by: Eduardo Habkost 
Signed-off-by: Like Xu 
---
 target/i386/cpu.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index eee6da3ad8..56a486b498 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -708,6 +708,8 @@ typedef struct FeatureWordInfo {
 uint64_t migratable_flags; /* Feature flags known to be migratable */
 /* Features that shouldn't be auto-enabled by "-cpu host" */
 uint64_t no_autoenable_flags;
+/* Bits that must match host exactly when using KVM */
+uint64_t kvm_exact_match_flags;
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
@@ -1147,6 +1149,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
= {
 .msr = {
 .index = MSR_IA32_PERF_CAPABILITIES,
 },
+/*
+ * KVM is not able to emulate a VCPU with LBR_FMT different
+ * from the host, so LBR_FMT must match the host exactly.
+ */
+.kvm_exact_match_flags = PERF_CAP_LBR_FMT,
 },
 
 [FEAT_VMX_PROCBASED_CTLS] = {
@@ -6623,16 +6630,18 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool 
verbose)
 }
 
 for (w = 0; w < FEATURE_WORDS; w++) {
+FeatureWordInfo *fi = &feature_word_info[w];
+uint64_t match_flags = fi->kvm_exact_match_flags;
 uint64_t host_feat =
 x86_cpu_get_supported_feature_word(w, false);
 uint64_t requested_features = env->features[w];
 uint64_t unavailable_features = requested_features & ~host_feat;
-if (kvm_enabled() && w == FEAT_PERF_CAPABILITIES &&
-(requested_features & PERF_CAP_LBR_FMT)) {
-if ((host_feat & PERF_CAP_LBR_FMT) !=
-(requested_features & PERF_CAP_LBR_FMT)) {
-unavailable_features |= PERF_CAP_LBR_FMT;
-}
+if (kvm_enabled() && match_flags) {
+uint64_t mismatches = (requested_features & match_flags) &&
+(requested_features ^ host_feat) & match_flags;
+mark_unavailable_features(cpu, w,
+mismatches, "feature doesn't match host");
+unavailable_features &= ~match_flags;
 }
 mark_unavailable_features(cpu, w, unavailable_features, prefix);
 }
-- 
2.30.2

Re: [PATCH v2] i386: Add ratelimit for bus locks acquired in guest

2021-04-22 Thread Chenyi Qiang

On 4/21/2021 11:18 PM, Eduardo Habkost wrote:

On Wed, Apr 21, 2021 at 10:50:10PM +0800, Xiaoyao Li wrote:

On 4/21/2021 10:12 PM, Eduardo Habkost wrote:

On Wed, Apr 21, 2021 at 02:26:42PM +0800, Chenyi Qiang wrote:

Hi, Eduardo, thanks for your comments!

On 4/21/2021 12:34 AM, Eduardo Habkost wrote:

Hello,

Thanks for the patch. Comments below:

On Tue, Apr 20, 2021 at 05:37:36PM +0800, Chenyi Qiang wrote:

Virtual Machines can exploit bus locks to degrade the performance of
system. To address this kind of performance DOS attack, bus lock VM exit
is introduced in KVM and it will report the bus locks detected in guest,
which can help userspace to enforce throttling policies.

Is there anything today that would protect the system from
similar attacks from userspace with access to /dev/kvm?

I can't fully understand your meaning for "similar attack with access to
/dev/kvm". But there are some similar associated detection features on bare
metal.

What I mean is: you say guests can make a performance DoS attack
on the host, and your patch mitigates that.

What would be the available methods to prevent untrusted
userspace running on the host with access to /dev/kvm from making
a similar DoS attack on the host?

Thanks for all the clarifications below. Considering them,
what's the answer to the question above?

Hi Eduardo,

Just make it more clear.

Bus lock detection contains two sub-features. One is bus lock debug
exception, and the other is bus lock VM exit.

Bus lock #DB exception can help detect the bus locks acquired in user
space and bus lock VM exit detects the bus locks insides VMs. To address
the attacks from non-VM userspace attackers against VM, Bus lock #DB
exception can help.

The Bus lock #DB exception support
(https://lore.kernel.org/lkml/20210322135325.682257-3-fenghua...@intel.com/)
extends the existing kernel command line parameter "split_lock_detect="
also applying to non-wb bus lock.
For example, split_lock_detect=fatal will send SIGBUS to the attackers
once this kind of #DB is detected.

1. Split lock
detection:https://lore.kernel.org/lkml/158031147976.396.8941798847364718785.tip-bot2@tip-bot2/
Some CPUs can raise an #AC trap when a split lock is attempted.

Would split_lock_detect=fatal be enough to prevent the above attacks?

NO.

There are two types bus lock:
1. split lock - lock on cacheable memory while the memory across two cache
lines.
2. non-wb lock - lock on non-writableback memory (you can find it on Intel
ISE chapter 8,
https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html)

split lock detection can only prevent 1)

Is split_lock_detect=fatal the only available way to prevent them?

as above, 2) non-wb lock can be prevented by "non-wb lock disable" feature

Bus lock VM exit applies to both 1 and 2, correct?

2. Bus lock Debug Exception:
https://lore.kernel.org/lkml/20210322135325.682257-1-fenghua...@intel.com/
The kernel can be notified by an #DB trap after a user instruction acquires
a bus lock and is executed.

I see a rate limit option mentioned at the above URL. Would a
host kernel bus lock rate limit option make this QEMU patch
redundant?

No. Bus lock Debug exception cannot be used to detect the bus lock happens
in guest (vmx non-root mode).

We have patch to virtualize this feature for guest
https://lore.kernel.org/kvm/20210202090433.13441-1-chenyi.qi...@intel.com/

that guest will have its own setting of bus lock debug exception on or off.

What's more important is that, even we force set the
MSR_DEBUGCTL.BUS_LOCK_DETECT for guest, guest still can escape from it.
Because bus lock #DB is a trap which is delivered after the instruction
completes. If the instruction acquires bus lock subsequently faults e.g.,
#PF, then no bus lock #DB generated. But the bus lock does happen.

But with bus lock VM exit, even the instruction faults, it will cause a BUS
LOCK VM exit.

Re: [PATCH-for-6.0] net: tap: fix crash on hotplug




在 2021/4/23 上午5:34, Cole Robinson 写道:

On 4/22/21 5:42 AM, Bin Meng wrote:

On Thu, Apr 22, 2021 at 5:36 PM Peter Maydell  wrote:

On Thu, 22 Apr 2021 at 05:29, Bin Meng  wrote:

On Thu, Apr 22, 2021 at 12:36 AM Philippe Mathieu-Daudé
 wrote:

Cc'ing Bin.

On 4/21/21 5:22 PM, Cole Robinson wrote:

Attempting to hotplug a tap nic with libvirt will crash qemu:

$ sudo virsh attach-interface f32 network default
error: Failed to attach interface
error: Unable to read from monitor: Connection reset by peer

0x55875b7f3a99 in tap_send (opaque=0x55875e39eae0) at ../net/tap.c:206
206   if (!s->nc.peer->do_not_pad) {
gdb$ bt

s->nc.peer may not be set at this point. This seems to be an
expected case, as qemu_send_packet_* explicitly checks for NULL
s->nc.peer later.

Fix it by checking for s->nc.peer here too. Padding is applied if
s->nc.peer is not set.

https://bugzilla.redhat.com/show_bug.cgi?id=1949786
Fixes: 969e50b61a2

Signed-off-by: Cole Robinson 
---
* Or should we skip padding if nc.peer is unset? I didn't dig into it
* tap-win3.c and slirp.c may need a similar fix, but the slirp case
   didn't crash in a simple test.

  net/tap.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tap.c b/net/tap.c
index dd42ac6134..937559dbb8 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -203,7 +203,7 @@ static void tap_send(void *opaque)
  size -= s->host_vnet_hdr_len;
  }

-if (!s->nc.peer->do_not_pad) {
+if (!s->nc.peer || !s->nc.peer->do_not_pad) {

I think we should do:

if (s->nc.peer && !s->nc.peer->do_not_pad)

Yes. If there is no peer then the qemu_send_packet() that we're about
to do is going to discard the packet anyway, so there's no point in
padding it.

Maybe consider

static inline bool net_peer_needs_padding(NetClientState *nc)
{
 return nc->peer && !nc->peer->do_not_pad;
}

since we want the same check in three places ?

Sounds good to me.


I did not get to this today. Bin/Jason/anyone want to write the patch,



I will send a patch soon.

Thanks



I
will test it tomorrow (US EDT time). If not I'll write the patch tomorrow.

Thanks,
Cole

Re: [PATCH v6 10/18] cpu: Move CPUClass::vmsd to SysemuCPUOps


On 4/22/21 12:38 PM, Philippe Mathieu-Daudé wrote:

Migration is specific to system emulation.

- Move the CPUClass::vmsd field to SysemuCPUOps,
- restrict VMSTATE_CPU() macro to sysemu,
- vmstate_dummy is now unused, remove it.

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


Reviewed-by: Richard Henderson 


  #ifndef CONFIG_USER_ONLY
  static const struct SysemuCPUOps riscv_sysemu_ops = {
+/* For now, mark unmigratable: */
+.legacy_vmsd = &vmstate_riscv_cpu,
  };
  #endif
  
@@ -628,8 +630,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)

  cc->disas_set_info = riscv_cpu_disas_set_info;
  #ifndef CONFIG_USER_ONLY
  cc->get_phys_page_debug = riscv_cpu_get_phys_page_debug;
-/* For now, mark unmigratable: */
-cc->legacy_vmsd = &vmstate_riscv_cpu;


I'll note that the comment has been incorrect since f7697f0e629.


r~


  cc->sysemu_ops = &riscv_sysemu_ops;
  cc->write_elf64_note = riscv_cpu_write_elf64_note;
  cc->write_elf32_note = riscv_cpu_write_elf32_note;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 7503b9e0c8b..131e7dfdf82 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -479,6 +479,7 @@ static void s390_cpu_reset_full(DeviceState *dev)
  
  #ifndef CONFIG_USER_ONLY

  static const struct SysemuCPUOps s390_sysemu_ops = {
+.legacy_vmsd = &vmstate_s390_cpu,
  };
  #endif
  
@@ -522,7 +523,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)

  cc->gdb_write_register = s390_cpu_gdb_write_register;
  #ifndef CONFIG_USER_ONLY
  cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
-cc->legacy_vmsd = &vmstate_s390_cpu;
  cc->get_crash_info = s390_cpu_get_crash_info;
  cc->write_elf64_note = s390_cpu_write_elf64_note;
  cc->sysemu_ops = &s390_sysemu_ops;
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 743a7287a4f..543853c24dc 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -850,6 +850,7 @@ static Property sparc_cpu_properties[] = {
  
  #ifndef CONFIG_USER_ONLY

  static const struct SysemuCPUOps sparc_sysemu_ops = {
+.legacy_vmsd = &vmstate_sparc_cpu,
  };
  #endif
  
@@ -894,7 +895,6 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)

  cc->gdb_write_register = sparc_cpu_gdb_write_register;
  #ifndef CONFIG_USER_ONLY
  cc->get_phys_page_debug = sparc_cpu_get_phys_page_debug;
-cc->legacy_vmsd = &vmstate_sparc_cpu;
  cc->sysemu_ops = &sparc_sysemu_ops;
  #endif
  cc->disas_set_info = cpu_sparc_disas_set_info;
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index b15abc36851..e3f2f2fefa3 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -10880,6 +10880,7 @@ static Property ppc_cpu_properties[] = {
  
  #ifndef CONFIG_USER_ONLY

  static const struct SysemuCPUOps ppc_sysemu_ops = {
+.legacy_vmsd = &vmstate_ppc_cpu,
  };
  #endif
  
@@ -10925,7 +10926,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)

  cc->gdb_write_register = ppc_cpu_gdb_write_register;
  #ifndef CONFIG_USER_ONLY
  cc->get_phys_page_debug = ppc_cpu_get_phys_page_debug;
-cc->legacy_vmsd = &vmstate_ppc_cpu;
  cc->sysemu_ops = &ppc_sysemu_ops;
  #endif
  #if defined(CONFIG_SOFTMMU)

Re: [PATCH v6 08/18] cpu/{avr, lm32, moxie}: Set DeviceClass vmsd field (not CPUClass one)


On 4/22/21 12:38 PM, Philippe Mathieu-Daudé wrote:

See rationale in previous commit. Targets should use the vmsd field
of DeviceClass, not CPUClass. As migration is not important on the
avr/lm32/moxie targets, break the migration compatibility and set
the DeviceClass vmsd field.

Signed-off-by: Philippe Mathieu-Daudé
---
  target/avr/cpu.c   | 2 +-
  target/lm32/cpu.c  | 2 +-
  target/moxie/cpu.c | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)


Do you not have to increment the vmstate versions?

Though I can't see how the version numbers would even get compared, since we're 
looking for them in different locations?



r~

Re: [PATCH v6 07/18] cpu: Rename CPUClass vmsd -> legacy_vmsd


On 4/22/21 12:38 PM, Philippe Mathieu-Daudé wrote:

Quoting Peter Maydell [*]:

   There are two ways to handle migration for
   a CPU object:

   (1) like any other device, so it has a dc->vmsd that covers
   migration for the whole object. As usual for objects that are a
   subclass of a parent that has state, the first entry in the
   VMStateDescription field list is VMSTATE_CPU(), which migrates
   the cpu_common fields, followed by whatever the CPU's own migration
   fields are.

   (2) a backwards-compatible mechanism for CPUs that were
   originally migrated using manual "write fields to the migration
   stream structures". The on-the-wire migration format
   for those is based on the 'env' pointer (which isn't a QOM object),
   and the cpu_common part of the migration data is elsewhere.

   cpu_exec_realizefn() handles both possibilities:

   * for type 1, dc->vmsd is set and cc->vmsd is not,
 so cpu_exec_realizefn() does nothing, and the standard
 "register dc->vmsd for a device" code does everything needed

   * for type 2, dc->vmsd is NULL and so we register the
 vmstate_cpu_common directly to handle the cpu-common fields,
 and the cc->vmsd to handle the per-CPU stuff

   You can't change a CPU from one type to the other without breaking
   migration compatibility, which is why some guest architectures
   are stuck on the cc->vmsd form. New targets should use dc->vmsd.

To avoid new targets to start using type (2), rename cc->vmsd as
cc->legacy_vmsd. The correct field to implement is dc->vmsd (the
DeviceClass one).

See also commit b170fce3dd0 ("cpu: Register VMStateDescription
through CPUState") for historic background.

[*]https://www.mail-archive.com/qemu-devel@nongnu.org/msg800849.html

Cc: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
---
  include/hw/core/cpu.h   |  5 +++--
  cpu.c   | 12 ++--
  target/arm/cpu.c|  2 +-
  target/avr/cpu.c|  2 +-
  target/i386/cpu.c   |  2 +-
  target/lm32/cpu.c   |  2 +-
  target/mips/cpu.c   |  2 +-
  target/moxie/cpu.c  |  2 +-
  target/riscv/cpu.c  |  2 +-
  target/s390x/cpu.c  |  2 +-
  target/sparc/cpu.c  |  2 +-
  target/ppc/translate_init.c.inc |  2 +-
  12 files changed, 19 insertions(+), 18 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v6 06/18] cpu: Assert DeviceClass::vmsd is NULL on user emulation


On 4/22/21 12:38 PM, Philippe Mathieu-Daudé wrote:

Migration is specific to system emulation.

Restrict current DeviceClass::vmsd to sysemu using #ifdef'ry,
and assert in cpu_exec_realizefn() that dc->vmsd not set under
user emulation.

Signed-off-by: Philippe Mathieu-Daudé
---
  cpu.c  | 1 +
  target/sh4/cpu.c   | 5 +++--
  target/unicore32/cpu.c | 4 
  target/xtensa/cpu.c| 4 +++-
  4 files changed, 11 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 0/7] linux-user: sigaction fixes/cleanups

2021-04-22 Thread no-reply

Patchew URL: 
https://patchew.org/QEMU/20210422230227.314751-1-richard.hender...@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210422230227.314751-1-richard.hender...@linaro.org
Subject: [PATCH v2 0/7] linux-user: sigaction fixes/cleanups

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/20210422230227.314751-1-richard.hender...@linaro.org -> 
patchew/20210422230227.314751-1-richard.hender...@linaro.org
Switched to a new branch 'test'
9eee746 linux-user: Tidy TARGET_NR_rt_sigaction
46c2541 linux-user/alpha: Share code for TARGET_NR_sigaction
c69776b linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER
de9e5c2 linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
57bd960 linux-user: Pass ka_restorer to do_sigaction
ef4054e linux-user/alpha: Rename the sigaction restorer field
df4fac9 linux-user/alpha: Fix rt sigframe return

=== OUTPUT BEGIN ===
1/7 Checking commit df4fac977c4c (linux-user/alpha: Fix rt sigframe return)
2/7 Checking commit ef4054e42574 (linux-user/alpha: Rename the sigaction 
restorer field)
3/7 Checking commit 57bd9604ef18 (linux-user: Pass ka_restorer to do_sigaction)
ERROR: code indent should never use tabs
#64: FILE: linux-user/syscall.c:9019:
+^Iret = get_errno(do_sigaction(arg1, pact, &oact, 0));$

total: 1 errors, 0 warnings, 97 lines checked

Patch 3/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/7 Checking commit de9e5c267f85 (linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER 
in do_syscall)
5/7 Checking commit c69776bf2b07 (linux-user/alpha: Define 
TARGET_ARCH_HAS_KA_RESTORER)
6/7 Checking commit 46c2541b617d (linux-user/alpha: Share code for 
TARGET_NR_sigaction)
7/7 Checking commit 9eee7464d318 (linux-user: Tidy TARGET_NR_rt_sigaction)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210422230227.314751-1-richard.hender...@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction

Initialize variables instead of elses.
Use an else instead of a goto.
Add braces.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall.c | 32 +---
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9bcd485423..c7c3257f40 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9060,32 +9060,26 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 target_ulong sigsetsize = arg4;
 target_ulong restorer = 0;
 #endif
-struct target_sigaction *act;
-struct target_sigaction *oact;
+struct target_sigaction *act = NULL;
+struct target_sigaction *oact = NULL;
 
 if (sigsetsize != sizeof(target_sigset_t)) {
 return -TARGET_EINVAL;
 }
-if (arg2) {
-if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
-return -TARGET_EFAULT;
-}
-} else {
-act = NULL;
+if (arg2 && !lock_user_struct(VERIFY_READ, act, arg2, 1)) {
+return -TARGET_EFAULT;
 }
-if (arg3) {
-if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
-ret = -TARGET_EFAULT;
-goto rt_sigaction_fail;
+if (arg3 && !lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
+ret = -TARGET_EFAULT;
+} else {
+ret = get_errno(do_sigaction(arg1, act, oact, restorer));
+if (oact) {
+unlock_user_struct(oact, arg3, 1);
 }
-} else
-oact = NULL;
-ret = get_errno(do_sigaction(arg1, act, oact, restorer));
-   rt_sigaction_fail:
-if (act)
+}
+if (act) {
 unlock_user_struct(act, arg2, 0);
-if (oact)
-unlock_user_struct(oact, arg3, 1);
+}
 }
 return ret;
 #ifdef TARGET_NR_sgetmask /* not on alpha */
-- 
2.25.1

[PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall

Do not access a field that may not be present.  This will
become an issue when sharing more code in the next patch.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 36169a0ded..89d641856c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9038,7 +9038,9 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 act._sa_handler = old_act->_sa_handler;
 target_siginitset(&act.sa_mask, old_act->sa_mask);
 act.sa_flags = old_act->sa_flags;
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
 act.sa_restorer = old_act->sa_restorer;
+#endif
 unlock_user_struct(old_act, arg2, 0);
 pact = &act;
 } else {
@@ -9051,7 +9053,9 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 old_act->_sa_handler = oact._sa_handler;
 old_act->sa_mask = oact.sa_mask.sig[0];
 old_act->sa_flags = oact.sa_flags;
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
 old_act->sa_restorer = oact.sa_restorer;
+#endif
 unlock_user_struct(old_act, arg3, 1);
 }
 #endif
-- 
2.25.1

[PATCH v2 3/7] linux-user: Pass ka_restorer to do_sigaction

The value of ka_restorer needs to be saved in sigact_table.
At the moment, the attempt to save it in do_syscall is
improperly clobbering user memory.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall_defs.h |  2 +-
 linux-user/signal.c   |  5 -
 linux-user/syscall.c  | 19 ++-
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 693d4f3788..e4aaf8412f 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -492,7 +492,7 @@ void target_to_host_old_sigset(sigset_t *sigset,
const abi_ulong *old_sigset);
 struct target_sigaction;
 int do_sigaction(int sig, const struct target_sigaction *act,
- struct target_sigaction *oact);
+ struct target_sigaction *oact, abi_ulong ka_restorer);
 
 #include "target_signal.h"
 
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7eecec46c4..44a5012930 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -830,7 +830,7 @@ out:
 
 /* do_sigaction() return target values and host errnos */
 int do_sigaction(int sig, const struct target_sigaction *act,
- struct target_sigaction *oact)
+ struct target_sigaction *oact, abi_ulong ka_restorer)
 {
 struct target_sigaction *k;
 struct sigaction act1;
@@ -863,6 +863,9 @@ int do_sigaction(int sig, const struct target_sigaction 
*act,
 __get_user(k->sa_flags, &act->sa_flags);
 #ifdef TARGET_ARCH_HAS_SA_RESTORER
 __get_user(k->sa_restorer, &act->sa_restorer);
+#endif
+#ifdef TARGET_ARCH_HAS_KA_RESTORER
+k->ka_restorer = ka_restorer;
 #endif
 /* To be swapped in target_to_host_sigset.  */
 k->sa_mask = act->sa_mask;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ee21eb5e6f..36169a0ded 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8989,11 +8989,10 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 act._sa_handler = old_act->_sa_handler;
 target_siginitset(&act.sa_mask, old_act->sa_mask);
 act.sa_flags = old_act->sa_flags;
-act.ka_restorer = 0;
 unlock_user_struct(old_act, arg2, 0);
 pact = &act;
 }
-ret = get_errno(do_sigaction(arg1, pact, &oact));
+ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
 if (!is_error(ret) && arg3) {
 if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
 return -TARGET_EFAULT;
@@ -9017,7 +9016,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
pact = NULL;
}
 
-   ret = get_errno(do_sigaction(arg1, pact, &oact));
+   ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
 
if (!is_error(ret) && arg3) {
 if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
@@ -9040,15 +9039,12 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 target_siginitset(&act.sa_mask, old_act->sa_mask);
 act.sa_flags = old_act->sa_flags;
 act.sa_restorer = old_act->sa_restorer;
-#ifdef TARGET_ARCH_HAS_KA_RESTORER
-act.ka_restorer = 0;
-#endif
 unlock_user_struct(old_act, arg2, 0);
 pact = &act;
 } else {
 pact = NULL;
 }
-ret = get_errno(do_sigaction(arg1, pact, &oact));
+ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
 if (!is_error(ret) && arg3) {
 if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
 return -TARGET_EFAULT;
@@ -9085,11 +9081,10 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 act._sa_handler = rt_act->_sa_handler;
 act.sa_mask = rt_act->sa_mask;
 act.sa_flags = rt_act->sa_flags;
-act.ka_restorer = arg5;
 unlock_user_struct(rt_act, arg2, 0);
 pact = &act;
 }
-ret = get_errno(do_sigaction(arg1, pact, &oact));
+ret = get_errno(do_sigaction(arg1, pact, &oact, arg5));
 if (!is_error(ret) && arg3) {
 if (!lock_user_struct(VERIFY_WRITE, rt_act, arg3, 0))
 return -TARGET_EFAULT;
@@ -9104,6 +9099,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 target_ulong sigsetsize = arg5;
 #else
 target_ulong sigsetsize = arg4;
+target_ulong restorer = 0;
 #endif
 struct target_sigaction *act;
 struct target_sigaction *oact;
@@ -9115,9 +9111,6 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
 return -TARGET_EFA

[PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field

Use ka_restorer, in line with TARGET_ARCH_HAS_KA_RESTORER
vs TARGET_ARCH_HAS_SA_RESTORER, since Alpha passes this
field as a syscall argument.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall_defs.h | 2 +-
 linux-user/alpha/signal.c | 8 
 linux-user/syscall.c  | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 25be414727..693d4f3788 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -519,7 +519,7 @@ struct target_sigaction {
 abi_ulong _sa_handler;
 abi_ulong sa_flags;
 target_sigset_t sa_mask;
-abi_ulong sa_restorer;
+abi_ulong ka_restorer;
 };
 #elif defined(TARGET_MIPS)
 struct target_sigaction {
diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
index 86f5d2276d..3aa4b339a4 100644
--- a/linux-user/alpha/signal.c
+++ b/linux-user/alpha/signal.c
@@ -138,8 +138,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
 
 setup_sigcontext(&frame->sc, env, frame_addr, set);
 
-if (ka->sa_restorer) {
-r26 = ka->sa_restorer;
+if (ka->ka_restorer) {
+r26 = ka->ka_restorer;
 } else {
 __put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
 __put_user(INSN_LDI_R0 + TARGET_NR_sigreturn,
@@ -192,8 +192,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
 }
 
-if (ka->sa_restorer) {
-r26 = ka->sa_restorer;
+if (ka->ka_restorer) {
+r26 = ka->ka_restorer;
 } else {
 __put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
 __put_user(INSN_LDI_R0 + TARGET_NR_rt_sigreturn,
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 95d79ddc43..ee21eb5e6f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8989,7 +8989,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 act._sa_handler = old_act->_sa_handler;
 target_siginitset(&act.sa_mask, old_act->sa_mask);
 act.sa_flags = old_act->sa_flags;
-act.sa_restorer = 0;
+act.ka_restorer = 0;
 unlock_user_struct(old_act, arg2, 0);
 pact = &act;
 }
@@ -9085,7 +9085,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 act._sa_handler = rt_act->_sa_handler;
 act.sa_mask = rt_act->sa_mask;
 act.sa_flags = rt_act->sa_flags;
-act.sa_restorer = arg5;
+act.ka_restorer = arg5;
 unlock_user_struct(rt_act, arg2, 0);
 pact = &act;
 }
-- 
2.25.1

[PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction

There's no longer a difference between the alpha code and
the generic code.

There is a type difference in target_old_sigaction.sa_flags,
which can be resolved with a very much smaller ifdef, which
allows us to finish sharing the target_sigaction definition.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall_defs.h | 21 ++---
 linux-user/syscall.c  | 23 +--
 2 files changed, 7 insertions(+), 37 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7a1d3b239c..18b031a2f6 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -501,21 +501,12 @@ int do_sigaction(int sig, const struct target_sigaction 
*act,
 #endif
 
 #if defined(TARGET_ALPHA)
-struct target_old_sigaction {
-abi_ulong _sa_handler;
-abi_ulong sa_mask;
-int32_t sa_flags;
-};
+typedef int32_t target_old_sa_flags;
+#else
+typedef abi_ulong target_old_sa_flags;
+#endif
 
-/* This is the struct used inside the kernel.  The ka_restorer
-   field comes from the 5th argument to sys_rt_sigaction.  */
-struct target_sigaction {
-abi_ulong _sa_handler;
-abi_ulong sa_flags;
-target_sigset_t sa_mask;
-abi_ulong ka_restorer;
-};
-#elif defined(TARGET_MIPS)
+#if defined(TARGET_MIPS)
 struct target_sigaction {
uint32_tsa_flags;
 #if defined(TARGET_ABI_MIPSN32)
@@ -533,7 +524,7 @@ struct target_sigaction {
 struct target_old_sigaction {
 abi_ulong _sa_handler;
 abi_ulong sa_mask;
-abi_ulong sa_flags;
+target_old_sa_flags sa_flags;
 #ifdef TARGET_ARCH_HAS_SA_RESTORER
 abi_ulong sa_restorer;
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 216ee4ca47..9bcd485423 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8980,28 +8980,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 #ifdef TARGET_NR_sigaction
 case TARGET_NR_sigaction:
 {
-#if defined(TARGET_ALPHA)
-struct target_sigaction act, oact, *pact = 0;
-struct target_old_sigaction *old_act;
-if (arg2) {
-if (!lock_user_struct(VERIFY_READ, old_act, arg2, 1))
-return -TARGET_EFAULT;
-act._sa_handler = old_act->_sa_handler;
-target_siginitset(&act.sa_mask, old_act->sa_mask);
-act.sa_flags = old_act->sa_flags;
-unlock_user_struct(old_act, arg2, 0);
-pact = &act;
-}
-ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
-if (!is_error(ret) && arg3) {
-if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
-return -TARGET_EFAULT;
-old_act->_sa_handler = oact._sa_handler;
-old_act->sa_mask = oact.sa_mask.sig[0];
-old_act->sa_flags = oact.sa_flags;
-unlock_user_struct(old_act, arg3, 1);
-}
-#elif defined(TARGET_MIPS)
+#if defined(TARGET_MIPS)
struct target_sigaction act, oact, *pact, *old_act;
 
if (arg2) {
-- 
2.25.1

[PATCH v2 5/7] linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER

This means that we can share the TARGET_NR_rt_sigaction code,
and the target_rt_sigaction structure is unused.  Untangling
the ifdefs so that target_sigaction can be shared will wait
until the next patch.

Signed-off-by: Richard Henderson 
---
 linux-user/alpha/target_signal.h |  1 +
 linux-user/syscall_defs.h|  6 --
 linux-user/syscall.c | 37 ++--
 3 files changed, 7 insertions(+), 37 deletions(-)

diff --git a/linux-user/alpha/target_signal.h b/linux-user/alpha/target_signal.h
index 0b90d3a897..250642913e 100644
--- a/linux-user/alpha/target_signal.h
+++ b/linux-user/alpha/target_signal.h
@@ -92,6 +92,7 @@ typedef struct target_sigaltstack {
 #define TARGET_GEN_SUBRNG7 -25
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_KA_RESTORER
 
 /* bit-flags */
 #define TARGET_SS_AUTODISARM (1U << 31) /* disable sas during sighandling */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index e4aaf8412f..7a1d3b239c 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -507,12 +507,6 @@ struct target_old_sigaction {
 int32_t sa_flags;
 };
 
-struct target_rt_sigaction {
-abi_ulong _sa_handler;
-abi_ulong sa_flags;
-target_sigset_t sa_mask;
-};
-
 /* This is the struct used inside the kernel.  The ka_restorer
field comes from the 5th argument to sys_rt_sigaction.  */
 struct target_sigaction {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 89d641856c..216ee4ca47 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9064,41 +9064,17 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 #endif
 case TARGET_NR_rt_sigaction:
 {
-#if defined(TARGET_ALPHA)
-/* For Alpha and SPARC this is a 5 argument syscall, with
+/*
+ * For Alpha and SPARC this is a 5 argument syscall, with
  * a 'restorer' parameter which must be copied into the
  * sa_restorer field of the sigaction struct.
  * For Alpha that 'restorer' is arg5; for SPARC it is arg4,
  * and arg5 is the sigsetsize.
- * Alpha also has a separate rt_sigaction struct that it uses
- * here; SPARC uses the usual sigaction struct.
  */
-struct target_rt_sigaction *rt_act;
-struct target_sigaction act, oact, *pact = 0;
-
-if (arg4 != sizeof(target_sigset_t)) {
-return -TARGET_EINVAL;
-}
-if (arg2) {
-if (!lock_user_struct(VERIFY_READ, rt_act, arg2, 1))
-return -TARGET_EFAULT;
-act._sa_handler = rt_act->_sa_handler;
-act.sa_mask = rt_act->sa_mask;
-act.sa_flags = rt_act->sa_flags;
-unlock_user_struct(rt_act, arg2, 0);
-pact = &act;
-}
-ret = get_errno(do_sigaction(arg1, pact, &oact, arg5));
-if (!is_error(ret) && arg3) {
-if (!lock_user_struct(VERIFY_WRITE, rt_act, arg3, 0))
-return -TARGET_EFAULT;
-rt_act->_sa_handler = oact._sa_handler;
-rt_act->sa_mask = oact.sa_mask;
-rt_act->sa_flags = oact.sa_flags;
-unlock_user_struct(rt_act, arg3, 1);
-}
-#else
-#ifdef TARGET_SPARC
+#if defined(TARGET_ALPHA)
+target_ulong sigsetsize = arg4;
+target_ulong restorer = arg5;
+#elif defined(TARGET_SPARC)
 target_ulong restorer = arg4;
 target_ulong sigsetsize = arg5;
 #else
@@ -9131,7 +9107,6 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 unlock_user_struct(act, arg2, 0);
 if (oact)
 unlock_user_struct(oact, arg3, 1);
-#endif
 }
 return ret;
 #ifdef TARGET_NR_sgetmask /* not on alpha */
-- 
2.25.1

[PATCH v2 0/7] linux-user: sigaction fixes/cleanups

Alpha had two bugs, one with the non-ka_restorer fallback
using the wrong offset, and the other with the ka_restorer
value getting lost in do_sigaction.

Sparc had another bug, where the ka_restorer field was
written to user memory.

Version 2 splits patch 2 into 6.


r~


Richard Henderson (7):
  linux-user/alpha: Fix rt sigframe return
  linux-user/alpha: Rename the sigaction restorer field
  linux-user: Pass ka_restorer to do_sigaction
  linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
  linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER
  linux-user/alpha: Share code for TARGET_NR_sigaction
  linux-user: Tidy TARGET_NR_rt_sigaction

 linux-user/alpha/target_signal.h |   1 +
 linux-user/syscall_defs.h|  29 ++---
 linux-user/alpha/signal.c|  10 +--
 linux-user/signal.c  |   5 +-
 linux-user/syscall.c | 107 ---
 5 files changed, 43 insertions(+), 109 deletions(-)

-- 
2.25.1

[PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return

We incorrectly used the offset of the non-rt sigframe.

Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 linux-user/alpha/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
index c5c27ce084..86f5d2276d 100644
--- a/linux-user/alpha/signal.c
+++ b/linux-user/alpha/signal.c
@@ -200,7 +200,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
&frame->retcode[1]);
 __put_user(INSN_CALLSYS, &frame->retcode[2]);
 /* imb(); */
-r26 = frame_addr + offsetof(struct target_sigframe, retcode);
+r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
 }
 
 if (err) {
-- 
2.25.1

Re: [Bug 1743191] Re: Interacting with NetBSD serial console boot blocks no longer works

2021-04-22 Thread Ottavio Caruso

On Thu, 22 Apr 2021 at 18:23, Andreas Gustafsson
<1743...@bugs.launchpad.net> wrote:
>
> Ottavio Caruso wrote:
> > I am currently using:
> >
> > $ qemu-system-x86_64 --version
> > QEMU emulator version 5.2.0
> >
> > And I have no problem selecting from menu in serial console, so I
> > assume this is fixed for me. This is my command line:
> >
> > $ cat opt/bin/boot-netbsd-virtio
> > #!/bin/sh
> > qemu-system-x86_64 \
> > -drive if=virtio,file=/home/oc/VM/img/netbsd.image,index=0,media=disk \
> > -drive if=virtio,file=/home/oc/VM/img/netbsd.image.old,index=1,media=disk \
> > -M q35,accel=kvm -m 250M -cpu host -smp $(nproc) \
> > -nic user,hostfwd=tcp:127.0.0.1:-:22,model=virtio-net-pci,ipv6=off  \
> > -daemonize -display none  -vga none \
> > -serial mon:telnet:127.0.0.1:6665,server,nowait \
> > -pidfile /home/oc/VM/pid/netbsd-pid -nodefaults
> >
> > telnet 127.0.0.1 6665
>
> Have you tried the test case in the original bug report?
> --
> Andreas Gustafsson, g...@gson.org

You're right. Using the boot-com install image, the problem persists.

-- 
Ottavio Caruso

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

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

Title:
  Interacting with NetBSD serial console boot blocks no longer works

Status in QEMU:
  New

Bug description:
  The NetBSD boot blocks display a menu allowing the user to make a
  selection using the keyboard.  For example, when booting a NetBSD
  installation CD-ROM, the menu looks like this:

   1. Install NetBSD
   2. Install NetBSD (no ACPI)
   3. Install NetBSD (no ACPI, no SMP)
   4. Drop to boot prompt

  Choose an option; RETURN for default; SPACE to stop countdown.
  Option 1 will be chosen in 30 seconds.

  When booting NetBSD in a recent qemu using an emulated serial console,
  making this menu selection no longer works: when you type the selected
  number, the keyboard input is ignored, and the 30-second countdown
  continues.  In older versions of qemu, it works.

  To reproduce the problem, run:

 wget 
http://ftp.netbsd.org/pub/NetBSD/NetBSD-7.1.1/amd64/installation/cdrom/boot-com.iso
 qemu-system-x86_64 -nographic -cdrom boot-com.iso

  During the 30-second countdown, press 4

  Expected behavior: The countdown stops and you get a ">" prompt

  Incorrect behavior: The countdown continues

  There may also be some corruption of the terminal output; for example,
  "Option 1 will be chosen in 30 seconds" may be displayed as "Option 1
  will be chosen in p0 seconds".

  Using bisection, I have determined that the problem appeared with qemu
  commit 083fab0290f2c40d3d04f7f22eed9c8f2d5b6787, in which seabios was
  updated to 1.11 prerelease, and the problem is still there as of
  commit 7398166ddf7c6dbbc9cae6ac69bb2feda14b40ac.  The host operating
  system used for the tests was Debian 9 x86_64.

  Credit for discovering this bug goes to Paul Goyette.

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

[PULL 2/2] x86: acpi: use offset instead of pointer when using build_header()

2021-04-22 Thread Michael S. Tsirkin

From: Igor Mammedov 

Do the same as in commit
 (4d027afeb3a97 Virt: ACPI: fix qemu assert due to re-assigned table data 
address)
for remaining tables that happen to use saved at
the beginning pointer to build header to avoid assert
when table_data is relocated due to implicit re-size.

In this case user is trying to start Windows 10 and getting assert at
 hw/acpi/bios-linker-loader.c:239:
  bios_linker_loader_add_checksum: Assertion `start_offset < file->blob->len' 
failed.

Fixes: https://bugs.launchpad.net/bugs/1923497
Signed-off-by: Igor Mammedov 
Message-Id: <20210414084356.3792113-1-imamm...@redhat.com>
Cc: m...@redhat.com, qemu-sta...@nongnu.org
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/acpi/aml-build.c  | 15 +--
 hw/i386/acpi-build.c |  8 ++--
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index d33ce8954a..f0035d2b4a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1830,6 +1830,7 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, GArray 
*table_offsets,
 int i;
 unsigned rsdt_entries_offset;
 AcpiRsdtDescriptorRev1 *rsdt;
+int rsdt_start = table_data->len;
 const unsigned table_data_len = (sizeof(uint32_t) * table_offsets->len);
 const unsigned rsdt_entry_size = sizeof(rsdt->table_offset_entry[0]);
 const size_t rsdt_len = sizeof(*rsdt) + table_data_len;
@@ -1846,7 +1847,8 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, GArray 
*table_offsets,
 ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
 }
 build_header(linker, table_data,
- (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
+ (void *)(table_data->data + rsdt_start),
+ "RSDT", rsdt_len, 1, oem_id, oem_table_id);
 }
 
 /* Build xsdt table */
@@ -1857,6 +1859,7 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray 
*table_offsets,
 int i;
 unsigned xsdt_entries_offset;
 AcpiXsdtDescriptorRev2 *xsdt;
+int xsdt_start = table_data->len;
 const unsigned table_data_len = (sizeof(uint64_t) * table_offsets->len);
 const unsigned xsdt_entry_size = sizeof(xsdt->table_offset_entry[0]);
 const size_t xsdt_len = sizeof(*xsdt) + table_data_len;
@@ -1873,7 +1876,8 @@ build_xsdt(GArray *table_data, BIOSLinker *linker, GArray 
*table_offsets,
 ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
 }
 build_header(linker, table_data,
- (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id);
+ (void *)(table_data->data + xsdt_start),
+ "XSDT", xsdt_len, 1, oem_id, oem_table_id);
 }
 
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
@@ -2053,10 +2057,9 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, 
GArray *tcpalog,
 uint64_t control_area_start_address;
 TPMIf *tpmif = tpm_find();
 uint32_t start_method;
-void *tpm2_ptr;
 
 tpm2_start = table_data->len;
-tpm2_ptr = acpi_data_push(table_data, sizeof(AcpiTableHeader));
+acpi_data_push(table_data, sizeof(AcpiTableHeader));
 
 /* Platform Class */
 build_append_int_noprefix(table_data, TPM2_ACPI_CLASS_CLIENT, 2);
@@ -2095,8 +2098,8 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, 
GArray *tcpalog,
log_addr_offset, 8,
ACPI_BUILD_TPMLOG_FILE, 0);
 build_header(linker, table_data,
- tpm2_ptr, "TPM2", table_data->len - tpm2_start, 4, oem_id,
- oem_table_id);
+ (void *)(table_data->data + tpm2_start),
+ "TPM2", table_data->len - tpm2_start, 4, oem_id, 
oem_table_id);
 }
 
 Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set, uint32_t io_offset,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index de98750aef..daaf8f473e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1816,6 +1816,7 @@ build_hpet(GArray *table_data, BIOSLinker *linker, const 
char *oem_id,
const char *oem_table_id)
 {
 Acpi20Hpet *hpet;
+int hpet_start = table_data->len;
 
 hpet = acpi_data_push(table_data, sizeof(*hpet));
 /* Note timer_block_id value must be kept in sync with value advertised by
@@ -1824,13 +1825,15 @@ build_hpet(GArray *table_data, BIOSLinker *linker, 
const char *oem_id,
 hpet->timer_block_id = cpu_to_le32(0x8086a201);
 hpet->addr.address = cpu_to_le64(HPET_BASE);
 build_header(linker, table_data,
- (void *)hpet, "HPET", sizeof(*hpet), 1, oem_id, oem_table_id);
+ (void *)(table_data->data + hpet_start),
+ "HPET", sizeof(*hpet), 1, oem_id, oem_table_id);
 }
 
 static void
 build_tpm_tcpa(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
const char *oem_id, const char *oem_table_id)
 {
+int tcpa_start = table_data->len;
 Acpi20Tcpa *tcpa = acpi_data_push(tabl

[PULL 0/2] pc: last minute bugfixes

2021-04-22 Thread Michael S. Tsirkin

The following changes since commit d83f46d189a26fa32434139954d264326f199a45:

  virtio-pci: compat page aligned ATS (2021-04-06 07:11:36 -0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 9106db1038bf3db5e4f8007038b3a1962018fa07:

  x86: acpi: use offset instead of pointer when using build_header() 
(2021-04-22 18:22:01 -0400)


pc: last minute bugfixes

Two bugfixes - both seem pretty obvious and safe ...

Signed-off-by: Michael S. Tsirkin 


Igor Mammedov (1):
  x86: acpi: use offset instead of pointer when using build_header()

Jean-Philippe Brucker (1):
  amd_iommu: Fix pte_override_page_mask()

 hw/acpi/aml-build.c  | 15 +--
 hw/i386/acpi-build.c |  8 ++--
 hw/i386/amd_iommu.c  |  4 ++--
 3 files changed, 17 insertions(+), 10 deletions(-)

[PULL 1/2] amd_iommu: Fix pte_override_page_mask()

2021-04-22 Thread Michael S. Tsirkin

From: Jean-Philippe Brucker 

AMD IOMMU PTEs have a special mode allowing to specify an arbitrary page
size. Quoting the AMD IOMMU specification: "When the Next Level bits [of
a pte] are 7h, the size of the page is determined by the first zero bit
in the page address, starting from bit 12."

So if the lowest bits of the page address is 0, the page is 8kB. If the
lowest bits are 011, the page is 32kB. Currently pte_override_page_mask()
doesn't compute the right value for this page size and amdvi_translate()
can return the wrong guest-physical address. With a Linux guest, DMA
from SATA devices accesses the wrong memory and causes probe failure:

qemu-system-x86_64 ... -device amd-iommu -drive id=hd1,file=foo.bin,if=none \
-device ahci,id=ahci -device ide-hd,drive=hd1,bus=ahci.0
[6.613093] ata1.00: qc timeout (cmd 0xec)
[6.615062] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4)

Fix the page mask.

Signed-off-by: Jean-Philippe Brucker 
Message-Id: <20210421084007.1190546-1-jean-phili...@linaro.org>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/amd_iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 74a93a5d93..43b6e9bf51 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -860,8 +860,8 @@ static inline uint8_t get_pte_translation_mode(uint64_t pte)
 
 static inline uint64_t pte_override_page_mask(uint64_t pte)
 {
-uint8_t page_mask = 12;
-uint64_t addr = (pte & AMDVI_DEV_PT_ROOT_MASK) ^ AMDVI_DEV_PT_ROOT_MASK;
+uint8_t page_mask = 13;
+uint64_t addr = (pte & AMDVI_DEV_PT_ROOT_MASK) >> 12;
 /* find the first zero bit */
 while (addr & 1) {
 page_mask++;
-- 
MST

[Bug 1925512] Re: UNDEFINED case for instruction BLX

The complete imm32 is computed by

%imm24   26:s1 13:1 11:1 16:10 0:11 !function=t32_branch24

so that H appears at bit 1 in a->imm in trans_BLX_i.

Returning false from any trans_* function means that the trans
function did not match.  In some cases, this means that the next
possible matching pattern is tested.  But in most cases, such as
this one, we return all the way to disas_thumb2_insn, where we
do in fact call unallocated_encoding.

If you have a test case that fails, please provide it.

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

Title:
  UNDEFINED case for instruction BLX

Status in QEMU:
  Invalid

Bug description:
  Hi

  I refer to the instruction BLX imm (T2 encoding) in ARMv7 (Thumb
  mode).

  0 S imm10H  11 J1 0 J2 imm10L H

  
  if H == '1' then UNDEFINED;
  I1 = NOT(J1 EOR S);  I2 = NOT(J2 EOR S);  imm32 = 
SignExtend(S:I1:I2:imm10H:imm10L:'00', 32);
  targetInstrSet = InstrSet_A32;
  if InITBlock() && !LastInITBlock() then UNPREDICTABLE;

  According to the manual, if H equals to 1, this instruction should be
  an UNDEFINED instruction. However, it seems QEMU does not check this
  constraint in function trans_BLX_i. Thanks

  Regards
  Muhui

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

Re: [PATCH-for-6.0] net: tap: fix crash on hotplug

2021-04-22 Thread Cole Robinson

On 4/22/21 5:42 AM, Bin Meng wrote:
> On Thu, Apr 22, 2021 at 5:36 PM Peter Maydell  
> wrote:
>>
>> On Thu, 22 Apr 2021 at 05:29, Bin Meng  wrote:
>>>
>>> On Thu, Apr 22, 2021 at 12:36 AM Philippe Mathieu-Daudé
>>>  wrote:

 Cc'ing Bin.

 On 4/21/21 5:22 PM, Cole Robinson wrote:
> Attempting to hotplug a tap nic with libvirt will crash qemu:
>
> $ sudo virsh attach-interface f32 network default
> error: Failed to attach interface
> error: Unable to read from monitor: Connection reset by peer
>
> 0x55875b7f3a99 in tap_send (opaque=0x55875e39eae0) at ../net/tap.c:206
> 206   if (!s->nc.peer->do_not_pad) {
> gdb$ bt
>
> s->nc.peer may not be set at this point. This seems to be an
> expected case, as qemu_send_packet_* explicitly checks for NULL
> s->nc.peer later.
>
> Fix it by checking for s->nc.peer here too. Padding is applied if
> s->nc.peer is not set.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1949786
> Fixes: 969e50b61a2
>
> Signed-off-by: Cole Robinson 
> ---
> * Or should we skip padding if nc.peer is unset? I didn't dig into it
> * tap-win3.c and slirp.c may need a similar fix, but the slirp case
>   didn't crash in a simple test.
>
>  net/tap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/tap.c b/net/tap.c
> index dd42ac6134..937559dbb8 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -203,7 +203,7 @@ static void tap_send(void *opaque)
>  size -= s->host_vnet_hdr_len;
>  }
>
> -if (!s->nc.peer->do_not_pad) {
> +if (!s->nc.peer || !s->nc.peer->do_not_pad) {
>>>
>>> I think we should do:
>>>
>>> if (s->nc.peer && !s->nc.peer->do_not_pad)
>>
>> Yes. If there is no peer then the qemu_send_packet() that we're about
>> to do is going to discard the packet anyway, so there's no point in
>> padding it.
>>
>> Maybe consider
>>
>> static inline bool net_peer_needs_padding(NetClientState *nc)
>> {
>> return nc->peer && !nc->peer->do_not_pad;
>> }
>>
>> since we want the same check in three places ?
> 
> Sounds good to me.
> 

I did not get to this today. Bin/Jason/anyone want to write the patch, I
will test it tomorrow (US EDT time). If not I'll write the patch tomorrow.

Thanks,
Cole

qemu/kvm tianocore restart stuck

2021-04-22 Thread VoidCC

Hello,

I'm hitting a hard wall with qemu and efi.
I'm running multiple windows server 2019 vms which usually reboot on
updates.

The issue is, efi breaks on reboot.
It randomly(race condition?, does not occur consistently) ends up in a
blackscreen: no bootloader, no efi screen and the only way to get out of
that state is to destroy the vm.
moving the host mouse cursor above the console in virt-manager results in a
flashing mouse cursor.
there are no physical devices attached and there is currently no virtio
attachment in use (os has virtio drivers installed)


Machine is Q35 with tianocore/ovmf efi.
I managed to reproduce the same behaviour on rhel 8.3 as well as
voidlinux (kernel 5.11)

gdb output of qemu is showing nothing worrisome, else ive compiled ovmf
manually for debug output.
libvirt logs don't show any issues.

root · Slexy.org Pastebin

Re: [PATCH 2/2] linux-user: Clean up sigaction ka_restorer