Re: [PATCH 1/3] qga-win: Increase VSS freeze timeout to 60 secs instead of 10

[Konstantin Kostiuk]

ping

On Mon, Apr 5, 2021 at 4:14 PM Basil Salman  wrote:

> Currently Requester freeze times out after 10 seconds, while
> the default timeout for Writer Freeze is 60 seconds. according to
> VSS Documentation [1].
> [1]:
> https://docs.microsoft.com/en-us/windows/win32/vss/overview-of-processing-a-backup-under-vss
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1909073
>
> Signed-off-by: Basil Salman 
> Signed-off-by: Basil Salman 
> ---
>  qga/vss-win32/requester.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 5378c55d23..940a2c8f55 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -18,7 +18,7 @@
>  #include 
>
>  /* Max wait time for frozen event (VSS can only hold writes for 10
> seconds) */
> -#define VSS_TIMEOUT_FREEZE_MSEC 1
> +#define VSS_TIMEOUT_FREEZE_MSEC 6
>
>  /* Call QueryStatus every 10 ms while waiting for frozen event */
>  #define VSS_TIMEOUT_EVENT_MSEC 10
> --
> 2.17.2
>
>

Re: [PATCH 0/4] AIA local interrupt CSR support

[Anup Patel]

Hi Alistair,

On Fri, May 14, 2021 at 8:03 PM Anup Patel  wrote:
>
> The advanced interrupt architecture (AIA) extends the per-HART local
> interrupt support. Along with this, it also adds IMSIC (MSI contrllor)
> and Advanced PLIC (wired interrupt controller).
>
> The latest AIA draft specification can be found here:
> http://jhauser.us/private/RISCV-AIA/riscv-interrupts-021.pdf
>
> This series adds initial AIA support in QEMU which includes emulating all
> AIA local CSR. To enable AIA in QEMU, we just need to pass "x-aia=true"
> paramenter in "-cpu" QEMU command-line.
>
> To test series, we require OpenSBI and Linux with AIA support which
> can be found in riscv_aia_v1 branch at:
> https://github.com/avpatel/opensbi.git
> https://github.com/avpatel/linux.git
>
> Anup Patel (4):
>   target/riscv: Add defines for AIA local interrupt CSRs
>   target/riscv: Add CPU feature for AIA CSRs
>   target/riscv: Implement AIA local interrupt CSRs
>   hw/riscv: virt: Use AIA INTC compatible string when available

The ACLINT specification will be frozen soon (probably early next
month). The ACLINT QEMU support patches are also ready and don't
depend on the AIA QEMU support patches.

Is it okay to target ACLINT support in QEMU first ?

I can rebase this series on ACLINT support patches and also include
more AIA emulation patches (APLIC and IMSIC) in the AIA series.

Regards,
Anup

>
>  hw/riscv/virt.c   |   11 +-
>  target/riscv/cpu.c|   32 +-
>  target/riscv/cpu.h|   56 +-
>  target/riscv/cpu_bits.h   |  128 +
>  target/riscv/cpu_helper.c |  245 -
>  target/riscv/csr.c| 1059 +++--
>  target/riscv/machine.c|   26 +-
>  7 files changed, 1454 insertions(+), 103 deletions(-)
>
> --
> 2.25.1
>

[Bug 391880] Re: migrate exec hangs for several minutes if the pipe is closed before all its data is written

[Thomas Huth]

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

** No longer affects: qemu

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

Title:
  migrate exec hangs for several minutes if the pipe is closed before
  all its data is written

Status in qemu-kvm package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: kvm

  Using

migrate "exec:true"

  in the monitor hangs the VM for several minutes. What I expect is that
  the VM stops attempting to migrate after the pipe has been closed.

  Indicating a background migrate with -d doesn't help. Presumably the
  migration is not backgrounded until a certain amount of data is
  written to the pipe, or the migration times out. What I expect is that
  the migration is backgrounded immediately.

  == Version information

  $ lsb_release -rd
  Description: Ubuntu 9.04
  Release: 9.04

  $ apt-cache policy kvm
  kvm:
Installed: 1:84+dfsg-0ubuntu11
Candidate: 1:84+dfsg-0ubuntu11
Version table:
   *** 1:84+dfsg-0ubuntu11 0
  500 http://gb.archive.ubuntu.com jaunty/main Packages
  100 /var/lib/dpkg/status

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/391880/+subscriptions

[PULL 11/12] Add the function of colo_compare_cleanup

[Jason Wang]

From: "Rao, Lei" 

This patch fixes the following:
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x7f6ae4559859 in __GI_abort () at abort.c:79
#2  0x559aaa386720 in error_exit (err=16, msg=0x559aaa5973d0 
<__func__.16227> "qemu_mutex_destroy") at util/qemu-thread-posix.c:36
#3  0x559aaa3868c5 in qemu_mutex_destroy (mutex=0x559aabffe828) at 
util/qemu-thread-posix.c:69
#4  0x559aaa2f93a8 in char_finalize (obj=0x559aabffe800) at 
chardev/char.c:285
#5  0x559aaa23318a in object_deinit (obj=0x559aabffe800, 
type=0x559aabfd7d20) at qom/object.c:606
#6  0x559aaa2331b8 in object_deinit (obj=0x559aabffe800, 
type=0x559aabfd9060) at qom/object.c:610
#7  0x559aaa233200 in object_finalize (data=0x559aabffe800) at 
qom/object.c:620
#8  0x559aaa234202 in object_unref (obj=0x559aabffe800) at 
qom/object.c:1074
#9  0x559aaa2356b6 in object_finalize_child_property 
(obj=0x559aac0dac10, name=0x559aac778760 "compare0-0", opaque=0x559aabffe800) 
at qom/object.c:1584
#10 0x559aaa232f70 in object_property_del_all (obj=0x559aac0dac10) at 
qom/object.c:557
#11 0x559aaa2331ed in object_finalize (data=0x559aac0dac10) at 
qom/object.c:619
#12 0x559aaa234202 in object_unref (obj=0x559aac0dac10) at 
qom/object.c:1074
#13 0x559aaa2356b6 in object_finalize_child_property 
(obj=0x559aac0c75c0, name=0x559aac0dadc0 "chardevs", opaque=0x559aac0dac10) at 
qom/object.c:1584
#14 0x559aaa233071 in object_property_del_child (obj=0x559aac0c75c0, 
child=0x559aac0dac10, errp=0x0) at qom/object.c:580
#15 0x559aaa233155 in object_unparent (obj=0x559aac0dac10) at 
qom/object.c:599
#16 0x559aaa2fb721 in qemu_chr_cleanup () at chardev/char.c:1159
#17 0x559aa9f9b110 in main (argc=54, argv=0x7ffeb62fa998, 
envp=0x7ffeb62fab50) at vl.c:4539

When chardev is cleaned up, chr_write_lock needs to be destroyed. But
the colo-compare module is not cleaned up normally before it when the
guest poweroff. It is holding chr_write_lock at this time. This will
cause qemu crash.So we add the function of colo_compare_cleanup() before
qemu_chr_cleanup() to fix the bug.

Signed-off-by: Lei Rao 
Reviewed-by: Zhang Chen 
Reviewed-by: Lukas Straub 
Tested-by: Lukas Straub 
Signed-off-by: Zhang Chen 
Signed-off-by: Jason Wang 
---
 net/colo-compare.c | 10 ++
 net/colo-compare.h |  1 +
 net/net.c  |  4 
 3 files changed, 15 insertions(+)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index c142c08..5b538f4 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1402,6 +1402,16 @@ static void colo_compare_init(Object *obj)
  compare_set_vnet_hdr);
 }
 
+void colo_compare_cleanup(void)
+{
+CompareState *tmp = NULL;
+CompareState *n = NULL;
+
+QTAILQ_FOREACH_SAFE(tmp, &net_compares, next, n) {
+object_unparent(OBJECT(tmp));
+}
+}
+
 static void colo_compare_finalize(Object *obj)
 {
 CompareState *s = COLO_COMPARE(obj);
diff --git a/net/colo-compare.h b/net/colo-compare.h
index 22ddd51..b055270 100644
--- a/net/colo-compare.h
+++ b/net/colo-compare.h
@@ -20,5 +20,6 @@
 void colo_notify_compares_event(void *opaque, int event, Error **errp);
 void colo_compare_register_notifier(Notifier *notify);
 void colo_compare_unregister_notifier(Notifier *notify);
+void colo_compare_cleanup(void);
 
 #endif /* QEMU_COLO_COMPARE_H */
diff --git a/net/net.c b/net/net.c
index 2a47260..76bbb7c 100644
--- a/net/net.c
+++ b/net/net.c
@@ -52,6 +52,7 @@
 #include "qapi/error.h"
 #include "qapi/opts-visitor.h"
 #include "sysemu/runstate.h"
+#include "net/colo-compare.h"
 #include "net/filter.h"
 #include "qapi/string-output-visitor.h"
 
@@ -1402,6 +1403,9 @@ void net_cleanup(void)
 {
 NetClientState *nc;
 
+/*cleanup colo compare module for COLO*/
+colo_compare_cleanup();
+
 /* We may del multiple entries during qemu_del_net_client(),
  * so QTAILQ_FOREACH_SAFE() is also not safe here.
  */
-- 
2.7.4

[PULL 12/12] Fixed calculation error of pkt->header_size in fill_pkt_tcp_info()

[Jason Wang]

From: "Rao, Lei" 

The data pointer has skipped vnet_hdr_len in the function of
parse_packet_early().So, we can not subtract vnet_hdr_len again
when calculating pkt->header_size in fill_pkt_tcp_info(). Otherwise,
it will cause network packet comparsion errors and greatly increase
the frequency of checkpoints.

Signed-off-by: Lei Rao 
Signed-off-by: Zhang Chen 
Reviewed-by: Li Zhijian 
Reviewed-by: Zhang Chen 
Reviewed-by: Lukas Straub 
Tested-by: Lukas Straub 
Signed-off-by: Jason Wang 
---
 net/colo-compare.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 5b538f4..b100e7b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -211,7 +211,7 @@ static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
 pkt->tcp_ack = ntohl(tcphd->th_ack);
 *max_ack = *max_ack > pkt->tcp_ack ? *max_ack : pkt->tcp_ack;
 pkt->header_size = pkt->transport_header - (uint8_t *)pkt->data
-   + (tcphd->th_off << 2) - pkt->vnet_hdr_len;
+   + (tcphd->th_off << 2);
 pkt->payload_size = pkt->size - pkt->header_size;
 pkt->seq_end = pkt->tcp_seq + pkt->payload_size;
 pkt->flags = tcphd->th_flags;
-- 
2.7.4

[PULL 08/12] Optimize the function of filter_send

[Jason Wang]

From: "Rao, Lei" 

The iov_size has been calculated in filter_send(). we can directly
return the size.In this way, this is no need to repeat calculations
in filter_redirector_receive_iov();

Signed-off-by: Lei Rao 
Reviewed-by: Li Zhijian 
Reviewed-by: Zhang Chen 
Reviewed-by: Lukas Straub 
Tested-by: Lukas Straub 
Signed-off-by: Zhang Chen 
Signed-off-by: Jason Wang 
---
 net/filter-mirror.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index f8e6500..f20240c 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -88,7 +88,7 @@ static int filter_send(MirrorState *s,
 goto err;
 }
 
-return 0;
+return size;
 
 err:
 return ret < 0 ? ret : -EIO;
@@ -159,7 +159,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
 int ret;
 
 ret = filter_send(s, iov, iovcnt);
-if (ret) {
+if (ret < 0) {
 error_report("filter mirror send failed(%s)", strerror(-ret));
 }
 
@@ -182,10 +182,10 @@ static ssize_t 
filter_redirector_receive_iov(NetFilterState *nf,
 
 if (qemu_chr_fe_backend_connected(&s->chr_out)) {
 ret = filter_send(s, iov, iovcnt);
-if (ret) {
+if (ret < 0) {
 error_report("filter redirector send failed(%s)", strerror(-ret));
 }
-return iov_size(iov, iovcnt);
+return ret;
 } else {
 return 0;
 }
-- 
2.7.4

[PULL 06/12] Remove some duplicate trace code.

[Jason Wang]

From: "Rao, Lei" 

There is the same trace code in the colo_compare_packet_payload.

Signed-off-by: Lei Rao 
Reviewed-by: Li Zhijian 
Reviewed-by: Zhang Chen 
Reviewed-by: Lukas Straub 
Tested-by: Lukas Straub 
Signed-off-by: Zhang Chen 
Signed-off-by: Jason Wang 
---
 net/colo-compare.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 9d1ad99..c142c08 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -590,19 +590,6 @@ static int colo_packet_compare_other(Packet *spkt, Packet 
*ppkt)
 uint16_t offset = ppkt->vnet_hdr_len;
 
 trace_colo_compare_main("compare other");
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
-char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
-
-strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
-strcpy(pri_ip_dst, inet_ntoa(ppkt->ip->ip_dst));
-strcpy(sec_ip_src, inet_ntoa(spkt->ip->ip_src));
-strcpy(sec_ip_dst, inet_ntoa(spkt->ip->ip_dst));
-
-trace_colo_compare_ip_info(ppkt->size, pri_ip_src,
-   pri_ip_dst, spkt->size,
-   sec_ip_src, sec_ip_dst);
-}
-
 if (ppkt->size != spkt->size) {
 trace_colo_compare_main("Other: payload size of packets are 
different");
 return -1;
-- 
2.7.4

[PULL 10/12] Add a function named packet_new_nocopy for COLO.

[Jason Wang]

From: "Rao, Lei" 

Use the packet_new_nocopy instead of packet_new in the
filter-rewriter module. There will be one less memory
copy in the processing of each network packet.

Signed-off-by: Lei Rao 
Signed-off-by: Zhang Chen 
Reviewed-by: Zhang Chen 
Signed-off-by: Jason Wang 
---
 net/colo.c| 25 +
 net/colo.h|  1 +
 net/filter-rewriter.c |  3 +--
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/net/colo.c b/net/colo.c
index ef00609..3a3e6e8 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -157,19 +157,28 @@ void connection_destroy(void *opaque)
 
 Packet *packet_new(const void *data, int size, int vnet_hdr_len)
 {
-Packet *pkt = g_slice_new(Packet);
+Packet *pkt = g_slice_new0(Packet);
 
 pkt->data = g_memdup(data, size);
 pkt->size = size;
 pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST);
 pkt->vnet_hdr_len = vnet_hdr_len;
-pkt->tcp_seq = 0;
-pkt->tcp_ack = 0;
-pkt->seq_end = 0;
-pkt->header_size = 0;
-pkt->payload_size = 0;
-pkt->offset = 0;
-pkt->flags = 0;
+
+return pkt;
+}
+
+/*
+ * packet_new_nocopy will not copy data, so the caller can't release
+ * the data. And it will be released in packet_destroy.
+ */
+Packet *packet_new_nocopy(void *data, int size, int vnet_hdr_len)
+{
+Packet *pkt = g_slice_new0(Packet);
+
+pkt->data = data;
+pkt->size = size;
+pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST);
+pkt->vnet_hdr_len = vnet_hdr_len;
 
 return pkt;
 }
diff --git a/net/colo.h b/net/colo.h
index 573ab91..d91cd24 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -101,6 +101,7 @@ bool connection_has_tracked(GHashTable 
*connection_track_table,
 ConnectionKey *key);
 void connection_hashtable_reset(GHashTable *connection_track_table);
 Packet *packet_new(const void *data, int size, int vnet_hdr_len);
+Packet *packet_new_nocopy(void *data, int size, int vnet_hdr_len);
 void packet_destroy(void *opaque, void *user_data);
 void packet_destroy_partial(void *opaque, void *user_data);
 
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 10fe393..cb3a96c 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -270,8 +270,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
 vnet_hdr_len = nf->netdev->vnet_hdr_len;
 }
 
-pkt = packet_new(buf, size, vnet_hdr_len);
-g_free(buf);
+pkt = packet_new_nocopy(buf, size, vnet_hdr_len);
 
 /*
  * if we get tcp packet
-- 
2.7.4

[PULL 05/12] netdev: add more commands to preconfig mode

[Jason Wang]

From: Paolo Bonzini 

Creating and destroying network backend does not require a fully
constructed machine.  Allow the related monitor commands to run before
machine initialization has concluded.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Jason Wang 
---
 hmp-commands.hx | 2 ++
 qapi/net.json   | 6 --
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 84dcc3a..8e45bce 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1269,6 +1269,7 @@ ERST
 .help   = "add host network device",
 .cmd= hmp_netdev_add,
 .command_completion = netdev_add_completion,
+.flags  = "p",
 },
 
 SRST
@@ -1283,6 +1284,7 @@ ERST
 .help   = "remove host network device",
 .cmd= hmp_netdev_del,
 .command_completion = netdev_del_completion,
+.flags  = "p",
 },
 
 SRST
diff --git a/qapi/net.json b/qapi/net.json
index af3f5b0..7fab2e7 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -55,7 +55,8 @@
 # <- { "return": {} }
 #
 ##
-{ 'command': 'netdev_add', 'data': 'Netdev', 'boxed': true }
+{ 'command': 'netdev_add', 'data': 'Netdev', 'boxed': true,
+  'allow-preconfig': true }
 
 ##
 # @netdev_del:
@@ -75,7 +76,8 @@
 # <- { "return": {} }
 #
 ##
-{ 'command': 'netdev_del', 'data': {'id': 'str'} }
+{ 'command': 'netdev_del', 'data': {'id': 'str'},
+  'allow-preconfig': true }
 
 ##
 # @NetLegacyNicOptions:
-- 
2.7.4

[PULL 07/12] Fix the qemu crash when guest shutdown during checkpoint

[Jason Wang]

From: "Rao, Lei" 

This patch fixes the following:
qemu-system-x86_64: invalid runstate transition: 'colo' ->'shutdown'
Aborted (core dumped)

Signed-off-by: Lei Rao 
Reviewed-by: Li Zhijian 
Reviewed-by: Zhang Chen 
Reviewed-by: Lukas Straub 
Tested-by: Lukas Straub 
Signed-off-by: Zhang Chen 
Signed-off-by: Jason Wang 
---
 softmmu/runstate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index ce8977c..1564057 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -126,6 +126,7 @@ static const RunStateTransition runstate_transitions_def[] 
= {
 { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
 
 { RUN_STATE_COLO, RUN_STATE_RUNNING },
+{ RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
 
 { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
 { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
-- 
2.7.4

[PULL 09/12] Remove migrate_set_block_enabled in checkpoint

[Jason Wang]

From: "Rao, Lei" 

We can detect disk migration in migrate_prepare, if disk migration
is enabled in COLO mode, we can directly report an error.and there
is no need to disable block migration at every checkpoint.

Signed-off-by: Lei Rao 
Signed-off-by: Zhang Chen 
Reviewed-by: Li Zhijian 
Reviewed-by: Zhang Chen 
Reviewed-by: Lukas Straub 
Tested-by: Lukas Straub 
Signed-off-by: Jason Wang 
---
 migration/colo.c  | 6 --
 migration/migration.c | 4 
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index e498fdb..79fa1f6 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -435,12 +435,6 @@ static int colo_do_checkpoint_transaction(MigrationState 
*s,
 if (failover_get_state() != FAILOVER_STATUS_NONE) {
 goto out;
 }
-
-/* Disable block migration */
-migrate_set_block_enabled(false, &local_err);
-if (local_err) {
-goto out;
-}
 qemu_mutex_lock_iothread();
 
 #ifdef CONFIG_REPLICATION
diff --git a/migration/migration.c b/migration/migration.c
index 393299e..4828997 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2217,6 +2217,10 @@ static bool migrate_prepare(MigrationState *s, bool blk, 
bool blk_inc,
 }
 
 if (blk || blk_inc) {
+if (migrate_colo_enabled()) {
+error_setg(errp, "No disk migration is required in COLO mode");
+return false;
+}
 if (migrate_use_block() || migrate_use_block_incremental()) {
 error_setg(errp, "Command options are incompatible with "
"current migration capabilities");
-- 
2.7.4

[PULL 03/12] vhost-vdpa: don't initialize backend_features

[Jason Wang]

We used to initialize backend_features during vhost_vdpa_init()
regardless whether or not it was supported by vhost. This will lead
the unsupported features like VIRTIO_F_IN_ORDER to be included and set
to the vhost-vdpa during vhost_dev_start. Because the
VIRTIO_F_IN_ORDER is not supported by vhost-vdpa so it won't be
advertised to guest which will break the datapath.

Fix this by not initializing the backend_features, so the
acked_features could be built only from guest features via
vhost_net_ack_features().

Fixes: 108a64818e69b ("vhost-vdpa: introduce vhost-vdpa backend")
Cc: qemu-sta...@nongnu.org
Cc: Gautam Dawar 
Signed-off-by: Jason Wang 
---
 hw/virtio/vhost-vdpa.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 7bcccf1..61ba313 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -268,15 +268,12 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, 
uint8_t status)
 static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque)
 {
 struct vhost_vdpa *v;
-uint64_t features;
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
 trace_vhost_vdpa_init(dev, opaque);
 
 v = opaque;
 v->dev = dev;
 dev->opaque =  opaque ;
-vhost_vdpa_call(dev, VHOST_GET_FEATURES, &features);
-dev->backend_features = features;
 v->listener = vhost_vdpa_memory_listener;
 v->msg_type = VHOST_IOTLB_MSG_V2;
 
-- 
2.7.4

[PULL 04/12] vhost-vdpa: remove the unused vhost_vdpa_get_acked_features()

[Jason Wang]

No user for this helper, let's remove it.

Signed-off-by: Jason Wang 
---
 include/net/vhost-vdpa.h | 1 -
 net/vhost-vdpa.c | 9 -
 2 files changed, 10 deletions(-)

diff --git a/include/net/vhost-vdpa.h b/include/net/vhost-vdpa.h
index 45e34b7..b81f9a6 100644
--- a/include/net/vhost-vdpa.h
+++ b/include/net/vhost-vdpa.h
@@ -15,7 +15,6 @@
 #define TYPE_VHOST_VDPA "vhost-vdpa"
 
 struct vhost_net *vhost_vdpa_get_vhost_net(NetClientState *nc);
-uint64_t vhost_vdpa_get_acked_features(NetClientState *nc);
 
 extern const int vdpa_feature_bits[];
 
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 8b14215..19187dc 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -68,15 +68,6 @@ VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
 return s->vhost_net;
 }
 
-uint64_t vhost_vdpa_get_acked_features(NetClientState *nc)
-{
-VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
-assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
-s->acked_features = vhost_net_get_acked_features(s->vhost_net);
-
-return s->acked_features;
-}
-
 static int vhost_vdpa_net_check_device_id(struct vhost_net *net)
 {
 uint32_t device_id;
-- 
2.7.4

[PULL 02/12] vhost-vdpa: map virtqueue notification area if possible

[Jason Wang]

This patch implements the vq notification mapping support for
vhost-vDPA. This is simply done by using mmap()/munmap() for the
vhost-vDPA fd during device start/stop. For the device without
notification mapping support, we fall back to eventfd based
notification gracefully.

Reviewed-by: Si-Wei Liu 
Signed-off-by: Jason Wang 
---
 hw/virtio/vhost-vdpa.c | 85 ++
 include/hw/virtio/vhost-vdpa.h |  6 +++
 2 files changed, 91 insertions(+)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index aef5055..7bcccf1 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -286,12 +286,95 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void 
*opaque)
 return 0;
 }
 
+static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
+int queue_index)
+{
+size_t page_size = qemu_real_host_page_size;
+struct vhost_vdpa *v = dev->opaque;
+VirtIODevice *vdev = dev->vdev;
+VhostVDPAHostNotifier *n;
+
+n = &v->notifier[queue_index];
+
+if (n->addr) {
+virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, false);
+object_unparent(OBJECT(&n->mr));
+munmap(n->addr, page_size);
+n->addr = NULL;
+}
+}
+
+static void vhost_vdpa_host_notifiers_uninit(struct vhost_dev *dev, int n)
+{
+int i;
+
+for (i = 0; i < n; i++) {
+vhost_vdpa_host_notifier_uninit(dev, i);
+}
+}
+
+static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int 
queue_index)
+{
+size_t page_size = qemu_real_host_page_size;
+struct vhost_vdpa *v = dev->opaque;
+VirtIODevice *vdev = dev->vdev;
+VhostVDPAHostNotifier *n;
+int fd = v->device_fd;
+void *addr;
+char *name;
+
+vhost_vdpa_host_notifier_uninit(dev, queue_index);
+
+n = &v->notifier[queue_index];
+
+addr = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED, fd,
+queue_index * page_size);
+if (addr == MAP_FAILED) {
+goto err;
+}
+
+name = g_strdup_printf("vhost-vdpa/host-notifier@%p mmaps[%d]",
+   v, queue_index);
+memory_region_init_ram_device_ptr(&n->mr, OBJECT(vdev), name,
+  page_size, addr);
+g_free(name);
+
+if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
+munmap(addr, page_size);
+goto err;
+}
+n->addr = addr;
+
+return 0;
+
+err:
+return -1;
+}
+
+static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
+{
+int i;
+
+for (i = dev->vq_index; i < dev->vq_index + dev->nvqs; i++) {
+if (vhost_vdpa_host_notifier_init(dev, i)) {
+goto err;
+}
+}
+
+return;
+
+err:
+vhost_vdpa_host_notifiers_uninit(dev, i);
+return;
+}
+
 static int vhost_vdpa_cleanup(struct vhost_dev *dev)
 {
 struct vhost_vdpa *v;
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
 v = dev->opaque;
 trace_vhost_vdpa_cleanup(dev, v);
+vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
 memory_listener_unregister(&v->listener);
 
 dev->opaque = NULL;
@@ -468,6 +551,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool 
started)
 if (started) {
 uint8_t status = 0;
 memory_listener_register(&v->listener, &address_space_memory);
+vhost_vdpa_host_notifiers_init(dev);
 vhost_vdpa_set_vring_ready(dev);
 vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
 vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
@@ -477,6 +561,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool 
started)
 vhost_vdpa_reset_device(dev);
 vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
VIRTIO_CONFIG_S_DRIVER);
+vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
 memory_listener_unregister(&v->listener);
 
 return 0;
diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index ae9ee7a..9188226 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -14,11 +14,17 @@
 
 #include "hw/virtio/virtio.h"
 
+typedef struct VhostVDPAHostNotifier {
+MemoryRegion mr;
+void *addr;
+} VhostVDPAHostNotifier;
+
 typedef struct vhost_vdpa {
 int device_fd;
 uint32_t msg_type;
 MemoryListener listener;
 struct vhost_dev *dev;
+VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
 } VhostVDPA;
 
 #endif
-- 
2.7.4

[PULL 01/12] vhost-vdpa: skip ram device from the IOTLB mapping

[Jason Wang]

vDPA is not tie to any specific hardware, for safety and simplicity,
vhost-vDPA doesn't allow MMIO area to be mapped via IOTLB. Only the
doorbell could be mapped via mmap(). So this patch exclude skip the
ram device from the IOTLB mapping.

Reviewed-by: Si-Wei Liu 
Signed-off-by: Jason Wang 
---
 hw/virtio/vhost-vdpa.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index ee51863..aef5055 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -28,6 +28,8 @@ static bool 
vhost_vdpa_listener_skipped_section(MemoryRegionSection *section)
 {
 return (!memory_region_is_ram(section->mr) &&
 !memory_region_is_iommu(section->mr)) ||
+   /* vhost-vDPA doesn't allow MMIO to be mapped  */
+memory_region_is_ram_device(section->mr) ||
/*
 * Sizing an enabled 64-bit BAR can cause spurious mappings to
 * addresses in the upper part of the 64-bit address space.  These
@@ -172,22 +174,12 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
  vaddr, section->readonly);
 if (ret) {
 error_report("vhost vdpa map fail!");
-if (memory_region_is_ram_device(section->mr)) {
-/* Allow unexpected mappings not to be fatal for RAM devices */
-error_report("map ram fail!");
-  return ;
-}
 goto fail;
 }
 
 return;
 
 fail:
-if (memory_region_is_ram_device(section->mr)) {
-error_report("failed to vdpa_dma_map. pci p2p may not work");
-return;
-
-}
 /*
  * On the initfn path, store the first error in the container so we
  * can gracefully fail.  Runtime, there's not much we can do other
-- 
2.7.4

[PULL 00/12] Net patches

[Jason Wang]

The following changes since commit 7fe7fae8b48e3f9c647fd685e5155ebc8e6fb84d:

  Merge remote-tracking branch 
'remotes/dgilbert-gitlab/tags/pull-migration-20210609a' into staging 
(2021-06-09 16:40:21 +0100)

are available in the git repository at:

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

for you to fetch changes up to 5a2d9929ac1f01a1e8ef2a3f56f69e6069863dad:

  Fixed calculation error of pkt->header_size in fill_pkt_tcp_info() 
(2021-06-11 10:30:13 +0800)




Jason Wang (4):
  vhost-vdpa: skip ram device from the IOTLB mapping
  vhost-vdpa: map virtqueue notification area if possible
  vhost-vdpa: don't initialize backend_features
  vhost-vdpa: remove the unused vhost_vdpa_get_acked_features()

Paolo Bonzini (1):
  netdev: add more commands to preconfig mode

Rao, Lei (7):
  Remove some duplicate trace code.
  Fix the qemu crash when guest shutdown during checkpoint
  Optimize the function of filter_send
  Remove migrate_set_block_enabled in checkpoint
  Add a function named packet_new_nocopy for COLO.
  Add the function of colo_compare_cleanup
  Fixed calculation error of pkt->header_size in fill_pkt_tcp_info()

 hmp-commands.hx|   2 +
 hw/virtio/vhost-vdpa.c | 100 +++--
 include/hw/virtio/vhost-vdpa.h |   6 +++
 include/net/vhost-vdpa.h   |   1 -
 migration/colo.c   |   6 ---
 migration/migration.c  |   4 ++
 net/colo-compare.c |  25 +--
 net/colo-compare.h |   1 +
 net/colo.c |  25 +++
 net/colo.h |   1 +
 net/filter-mirror.c|   8 ++--
 net/filter-rewriter.c  |   3 +-
 net/net.c  |   4 ++
 net/vhost-vdpa.c   |   9 
 qapi/net.json  |   6 ++-
 softmmu/runstate.c |   1 +
 16 files changed, 143 insertions(+), 59 deletions(-)

Re: [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd.

[Jason Wang]

在 2021/6/10 下午2:55, Yuri Benditovich 写道:

On Thu, Jun 10, 2021 at 9:41 AM Jason Wang  wrote:

在 2021/6/9 下午6:04, Andrew Melnychenko 写道:

Libvirt usually launches qemu with strict permissions.
To enable eBPF RSS steering, qemu-ebpf-rss-helper was added.

A silly question:

Kernel had the following permission checks in bpf syscall:

 if (sysctl_unprivileged_bpf_disabled && !bpf_capable())
  return -EPERM;
...

  err = security_bpf(cmd, &attr, size);
  if (err < 0)
  return err;

So if I understand the code correctly, bpf syscall can only be done if:

1) unprivileged_bpf is enabled or
2) has the capability  and pass the LSM checks

So I think the series is for unprivileged_bpf disabled. If I'm not
wrong, I guess the policy is to grant CAP_BPF but do fine grain checks
via LSM.

If this is correct, need to describe it in the commit log.



Added property "ebpf_rss_fds" for "virtio-net" that allows to
initialize eBPF RSS context with passed program & maps fds.

Added qemu-ebpf-rss-helper - simple helper that loads eBPF
context and passes fds through unix socket.
Libvirt should call the helper and pass fds to qemu through
"ebpf_rss_fds" property.

Added explicit target OS check for libbpf dependency in meson.
eBPF RSS works only with Linux TAP, so there is no reason to
build eBPF loader/helper for non-Linux.

Overall, libvirt process should not be aware of the "interface"
of eBPF RSS, it will not be aware of eBPF maps/program "type" and
their quantity.

I'm not sure this is the best. We have several examples that let libvirt
to involve. Examples:

1) create TAP device (and the TUN_SETIFF)

2) open vhost devices



   That's why qemu and the helper should be from
the same build and be "synchronized". Technically each qemu may
have its own helper. That's why "query-helper-paths" qmp command
was added. Qemu should return the path to the helper that suits
and libvirt should use "that" helper for "that" emulator.

qmp sample:
C: { "execute": "query-helper-paths" }
S: { "return": [
   {
 "name": "qemu-ebpf-rss-helper",
 "path": "/usr/local/libexec/qemu-ebpf-rss-helper"
   }
  ]
 }

I think we need an example on the detail steps for how libvirt is
expected to use this.

The preliminary patches for libvirt are at
https://github.com/daynix/libvirt/tree/RSSv1



Will have a look but it would be better if the assumption of the 
management is detailed here to ease the reviewers.


Thanks

Re: [PATCH 3/4] target/riscv: Implement AIA local interrupt CSRs

[Anup Patel]

On Fri, Jun 11, 2021 at 4:49 AM Alistair Francis  wrote:
>
> On Sat, May 15, 2021 at 12:34 AM Anup Patel  wrote:
> >
> > We implement various AIA local interrupt CSRs for M-mode, HS-mode,
> > and VS-mode.
> >
> > Signed-off-by: Anup Patel 
> > ---
> >  target/riscv/cpu.c|   27 +-
> >  target/riscv/cpu.h|   52 +-
> >  target/riscv/cpu_helper.c |  245 -
> >  target/riscv/csr.c| 1059 +++--
> >  target/riscv/machine.c|   26 +-
> >  5 files changed, 1309 insertions(+), 100 deletions(-)
>
> I feel this patch could be split up more :)

This is patch is large because I did not want to break functionality.

I try again to break this patch. At the moment, the best I can do is
to break in to two parts.
1) AIA local interrupt CSRs without IMSIC
2) Extend AIA local interrupt CSRs to support IMSIC register access

>
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index f3702111ae..795162834b 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -256,11 +256,11 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE 
> > *f, int flags)
> >  qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vsstatus ",
> >   (target_ulong)env->vsstatus);
> >  }
> > -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mip ", env->mip);
> > -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mie ", env->mie);
> > -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mideleg ", env->mideleg);
> > +qemu_fprintf(f, " %s %016" PRIx64 "\n", "mip ", env->mip);
> > +qemu_fprintf(f, " %s %016" PRIx64 "\n", "mie ", env->mie);
> > +qemu_fprintf(f, " %s %016" PRIx64 "\n", "mideleg ", env->mideleg);
> >  if (riscv_has_ext(env, RVH)) {
> > -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hideleg ", 
> > env->hideleg);
> > +qemu_fprintf(f, " %s %016" PRIx64 "\n", "hideleg ", env->hideleg);
> >  }
> >  qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "medeleg ", env->medeleg);
> >  if (riscv_has_ext(env, RVH)) {
> > @@ -345,6 +345,8 @@ void restore_state_to_opc(CPURISCVState *env, 
> > TranslationBlock *tb,
> >
> >  static void riscv_cpu_reset(DeviceState *dev)
> >  {
> > +uint8_t iprio;
> > +int i, irq, rdzero;
> >  CPUState *cs = CPU(dev);
> >  RISCVCPU *cpu = RISCV_CPU(cs);
> >  RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> > @@ -357,6 +359,23 @@ static void riscv_cpu_reset(DeviceState *dev)
> >  env->mcause = 0;
> >  env->pc = env->resetvec;
> >  env->two_stage_lookup = false;
> > +
> > +/* Initialized default priorities of local interrupts. */
> > +for (i = 0; i < ARRAY_SIZE(env->miprio); i++) {
> > +iprio = riscv_cpu_default_priority(i);
> > +env->miprio[i] = iprio;
> > +env->siprio[i] = iprio;
> > +env->hviprio[i] = IPRIO_DEFAULT_MMAXIPRIO;
> > +}
> > +i = 0;
> > +while (!riscv_cpu_hviprio_index2irq(i, &irq, &rdzero)) {
> > +if (rdzero) {
> > +env->hviprio[irq] = 0;
> > +} else {
> > +env->hviprio[irq] = env->miprio[irq];
> > +}
> > +i++;
> > +}
> >  #endif
> >  cs->exception_index = EXCP_NONE;
> >  env->load_res = -1;
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index f00c60c840..780d3f9058 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -157,12 +157,12 @@ struct CPURISCVState {
> >   */
> >  uint64_t mstatus;
> >
> > -target_ulong mip;
> > +uint64_t mip;
>
> This isn't right. MIP is a MXLEN-1 CSR. I didn't check but I assume
> all the other existing target_ulong CSRs are the same.

When AIA is available the number of local interrupts are 64 for
both RV32 and RV64.

The width of CSRs remain same as target_ulong but we have
new CSRs for RV32 (such as mipH) for the high-half.

Also, this patch changes does not break the case when AIA
is not available (or disabled).

Regards,
Anup

>
> Alistair
>
> >
> > -uint32_t miclaim;
> > +uint64_t miclaim;
> >
> > -target_ulong mie;
> > -target_ulong mideleg;
> > +uint64_t mie;
> > +uint64_t mideleg;
> >
> >  target_ulong sptbr;  /* until: priv-1.9.1 */
> >  target_ulong satp;   /* since: priv-1.10.0 */
> > @@ -179,16 +179,27 @@ struct CPURISCVState {
> >  target_ulong mcause;
> >  target_ulong mtval;  /* since: priv-1.10.0 */
> >
> > +/* AIA CSRs */
> > +target_ulong miselect;
> > +target_ulong siselect;
> > +
> > +uint8_t miprio[64];
> > +uint8_t siprio[64];
> > +
> >  /* Hypervisor CSRs */
> >  target_ulong hstatus;
> >  target_ulong hedeleg;
> > -target_ulong hideleg;
> > +uint64_t hideleg;
> >  target_ulong hcounteren;
> >  target_ulong htval;
> >  target_ulong htinst;
> >  target_ulong hgatp;
> >  uint64_t htimedelta;
> >
> > +/* AIA HS-mode CSRs */
> > +uint8_t hviprio[64];
> > +target_ulong hvicontrol;
> > +
> >  /* Vir

Re: [PATCH 2/4] target/riscv: Add CPU feature for AIA CSRs

[Anup Patel]

On Fri, Jun 11, 2021 at 4:46 AM Alistair Francis  wrote:
>
> On Sat, May 15, 2021 at 12:35 AM Anup Patel  wrote:
> >
> > We add experimental CPU feature to enable AIA CSRs. This experimental
> > feature can be enabled by setting "x-aia=true" for CPU in the QEMU
> > command-line parameters.
> >
> > Signed-off-by: Anup Patel 
> > ---
> >  target/riscv/cpu.c | 5 +
> >  target/riscv/cpu.h | 4 +++-
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 7d6ed80f6b..f3702111ae 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -414,6 +414,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> > **errp)
> >  set_feature(env, RISCV_FEATURE_PMP);
> >  }
> >
> > +if (cpu->cfg.aia) {
> > +set_feature(env, RISCV_FEATURE_AIA);
> > +}
> > +
> >  set_resetvec(env, cpu->cfg.resetvec);
> >
> >  /* If only XLEN is set for misa, then set misa from properties */
> > @@ -554,6 +558,7 @@ static Property riscv_cpu_properties[] = {
> >  DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> >  DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
> >  DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
> > +DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
>
> This line should be a seperate patch at the end of the series.
>
> The idea is that we don't allow users to enable the feature until it
> has been fully implemented.

Okay, I will move this into a separate patch towards the end of the series.

Regards,
Anup

>
> Alistair
>
> >  DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
> >  DEFINE_PROP_END_OF_LIST(),
> >  };
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 0a33d387ba..f00c60c840 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -80,7 +80,8 @@
> >  enum {
> >  RISCV_FEATURE_MMU,
> >  RISCV_FEATURE_PMP,
> > -RISCV_FEATURE_MISA
> > +RISCV_FEATURE_MISA,
> > +RISCV_FEATURE_AIA
> >  };
> >
> >  #define PRIV_VERSION_1_10_0 0x00011000
> > @@ -303,6 +304,7 @@ struct RISCVCPU {
> >  uint16_t elen;
> >  bool mmu;
> >  bool pmp;
> > +bool aia;
> >  uint64_t resetvec;
> >  } cfg;
> >  };
> > --
> > 2.25.1
> >
> >

Re: [PATCH v2 04/37] target/riscv: 8-bit Addition & Subtraction Instruction

[LIU Zhiwei]

On 6/11/21 3:39 AM, Richard Henderson wrote:


On 6/10/21 12:58 AM, LIU Zhiwei wrote:

  include/tcg/tcg-op-gvec.h |  6 ++
  tcg/tcg-op-gvec.c   | 47 


Likewise, should be split from the larger patch.


OK
+static void gen_addv_mask_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b, 
TCGv_i32 m)

+{
+    TCGv_i32 t1 = tcg_temp_new_i32();
+    TCGv_i32 t2 = tcg_temp_new_i32();
+    TCGv_i32 t3 = tcg_temp_new_i32();
+
+    tcg_gen_andc_i32(t1, a, m);
+    tcg_gen_andc_i32(t2, b, m);
+    tcg_gen_xor_i32(t3, a, b);
+    tcg_gen_add_i32(d, t1, t2);
+    tcg_gen_and_i32(t3, t3, m);
+    tcg_gen_xor_i32(d, d, t3);
+
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t2);
+    tcg_temp_free_i32(t3);
+}
+
+void tcg_gen_vec_add8_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b)
+{
+    TCGv_i32 m = tcg_constant_i32((int32_t)dup_const(MO_8, 0x80));
+    gen_addv_mask_i32(d, a, b, m);
+}


There will only ever be one use; we might as well merge them.
The cast is unnecessary.


A little puzzling. Should I still split it?


Zhiwei




r~

[PATCH] docs/nvdimm: update doc

[Li Zhijian]

The prompt was updated since def835f0da ('hostmem: Don't report pmem attribute 
if unsupported')

Signed-off-by: Li Zhijian 
---
 docs/nvdimm.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
index 0aae682be3e..71cdbdf554b 100644
--- a/docs/nvdimm.txt
+++ b/docs/nvdimm.txt
@@ -247,7 +247,8 @@ is built with libpmem [2] support (configured with 
--enable-libpmem), QEMU
 will take necessary operations to guarantee the persistence of its own writes
 to the vNVDIMM backend(e.g., in vNVDIMM label emulation and live migration).
 If 'pmem' is 'on' while there is no libpmem support, qemu will exit and report
-a "lack of libpmem support" message to ensure the persistence is available.
+a "lack of libpmem support" (or "Invalid parameter 'pmem'" since v6.0.0)
+message to ensure the persistence is available.
 For example, if we want to ensure the persistence for some backend file,
 use the QEMU command line:
 
-- 
2.30.2

Re: TCG op for 32 bit only cpu on qemu-riscv64

[LIU Zhiwei]

On 6/10/21 9:29 PM, Richard Henderson wrote:

On 6/9/21 6:43 PM, LIU Zhiwei wrote:
1)First a multiply instruction, if the source value big enough, it 
will return a result with some bits not zero in MSW 32-bit.


Multiply is fine.  Input bits outside the low 32 cannot appear in the 
low 32 of the output.  Multiply-high-part on the other hand will 
require sign- or zero-extension of inputs.


2)If next instruction is a divide instruction,  the MSW 32-bit will 
influence the divide instruction result.


Yes, division requires extension too.

So I think use *_tl can't satisfy the need to run 32-bit program on 
qemu-riscv64.


I said some operations will require extra work -- I gave right-shift 
as an example.


You just have to be careful about deciding what extra work to do. I am 
suggesting that truncation to *_i32 is almost always not the correct 
answer.


Perhaps make it easier by changing gen_get_gpr and gen_set_gpr:

/* Return sign-extended version of gpr. */
static void get_gpr_s(DisasContext *ctx, TCGv t, int reg_num)
{
    if (reg_num == 0) {
    tcg_gen_movi_tl(t, 0);
    } else if (is_32bit(ctx)) {
    tcg_gen_ext32s_tl(t, cpu_gpr[reg_num]);
    } else {
    tcg_gen_mov_tl(t, cpu_gpr[reg_num]);
    }
}

/* Return zero-extended version of gpr. */
static void get_gpr_u(DisasContext *ctx, TCGv t, int reg_num);
{
    if (reg_num == 0) {
    tcg_gen_movi_tl(t, 0);
    } else if (is_32bit(ctx)) {
    tcg_gen_ext32u_tl(t, cpu_gpr[reg_num]);
    } else {
    tcg_gen_mov_tl(t, cpu_gpr[reg_num]);
    }
}

/* Return gpr with undefined high bits (which will be ignored). */
static void get_gpr_x(TCGv t, int reg_num);
{
    if (reg_num == 0) {
    tcg_gen_movi_tl(t, 0);
    } else {
    tcg_gen_mov_tl(t, cpu_gpr[reg_num]);
    }
}

And similarly for set.


It's very instructive. Thanks.

I once thought we have to split the registers in CPURISCVSTATE into two 
parts, gpr32 or gpr64.
Now I think we can still use most currently work with  a small 
cost(tcg_gen_ext32s_i64 has more cost than tcg_gen_mov_i32)


I will have a try later. Thanks again.

Best Regards,
Zhiwei



r~

Re: [PATCH v4 6/8] hw/intc: GICv3 redistributor ITS processing

[Shashi Mallela]

Have addressed all comments except the ones with responses(inline) below:-

On Jun 8 2021, at 9:57 am, Peter Maydell  wrote:
> On Wed, 2 Jun 2021 at 19:00, Shashi Mallela  wrote:
> >
> > Implemented lpi processing at redistributor to get lpi config info
> > from lpi configuration table,determine priority,set pending state in
> > lpi pending table and forward the lpi to cpuif.Added logic to invoke
> > redistributor lpi processing with translated LPI which set/clear LPI
> > from ITS device as part of ITS INT,CLEAR,DISCARD command and
> > GITS_TRANSLATER processing.
> >
> > Signed-off-by: Shashi Mallela 
> > ---
> > hw/intc/arm_gicv3.c | 9 ++
> > hw/intc/arm_gicv3_common.c | 1 +
> > hw/intc/arm_gicv3_cpuif.c | 7 +-
> > hw/intc/arm_gicv3_its.c | 14 ++-
> > hw/intc/arm_gicv3_redist.c | 145 +
> > hw/intc/gicv3_internal.h | 10 ++
> > include/hw/intc/arm_gicv3_common.h | 10 ++
> > 7 files changed, 190 insertions(+), 6 deletions(-)
>
> The code for finding/updating the best pending LPI looks a lot
> better in this version -- thanks for working through that.
>
> An important thing which I hadn't realized previously:
> the hpplpi information counts as information cached from the
> LPI configuration tables (because it is based on the priority
> and enable-bit information from those tables). That means that when
> the guest sends the ITS INV or INVALL command we need to throw it
> away and recalculate by calling gicv3_redist_update_lpi().
> (The idea here is that the guest can validly raise the priority
> of an interrupt by the sequence "write to table; INVALL; SYNC",
> and we need to correctly figure out that that might mean that
> that LPI is now the interrupt we should be taking.)
>

> Agreed,will be implementing the INV/INVALL command processing in addition to 
> existing ITS commands

> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
> index d63f8af604..4d19190b9c 100644
> --- a/hw/intc/arm_gicv3.c
> +++ b/hw/intc/arm_gicv3.c
> @@ -165,6 +165,15 @@ static void gicv3_redist_update_noirqset(GICv3CPUState 
> *cs)
> cs->hppi.grp = gicv3_irq_group(cs->gic, cs, cs->hppi.irq);
> }
>
> + if (cs->gic->lpi_enable && cs->lpivalid) {

You don't need a separate lpivalid flag -- you can use
hpplpi.prio == 0xff as your "no pending LPI" indication.
This is how the existing cs->hppi works.
(irqbetter() will always return false if passed an 0xff priority,
so you don't need to special case check anything here.)

> + if (irqbetter(cs, cs->hpplpi.irq, cs->hpplpi.prio)) {
> + cs->hppi.irq = cs->hpplpi.irq;
> + cs->hppi.prio = cs->hpplpi.prio;
> + cs->hppi.grp = cs->hpplpi.grp;
> + seenbetter = true;
> + }
> + }
> +
> /* If the best interrupt we just found would preempt whatever
> * was the previous best interrupt before this update, then
> * we know it's definitely the best one now.
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 53dea2a775..223db16fec 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -435,6 +435,7 @@ static void arm_gicv3_common_reset(DeviceState *dev)
> memset(cs->gicr_ipriorityr, 0, sizeof(cs->gicr_ipriorityr));
>
> cs->hppi.prio = 0xff;
> + cs->hpplpi.prio = 0xff;
>
> /* State in the CPU interface must *not* be reset here, because it
> * is part of the CPU's reset domain, not the GIC device's.
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index 81f94c7f4a..5be3efaa3f 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -898,10 +898,12 @@ static void icc_activate_irq(GICv3CPUState *cs, int irq)
> cs->gicr_iactiver0 = deposit32(cs->gicr_iactiver0, irq, 1, 1);
> cs->gicr_ipendr0 = deposit32(cs->gicr_ipendr0, irq, 1, 0);
> gicv3_redist_update(cs);
> - } else {
> + } else if (irq < GICV3_LPI_INTID_START) {
> gicv3_gicd_active_set(cs->gic, irq);
> gicv3_gicd_pending_clear(cs->gic, irq);
> gicv3_update(cs->gic, irq, 1);
> + } else {
> + gicv3_redist_lpi_pending(cs, irq, 0);
> }
> }
>
> @@ -1317,7 +1319,8 @@ static void icc_eoir_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
> trace_gicv3_icc_eoir_write(is_eoir0 ? 0 : 1,
> gicv3_redist_affid(cs), value);
>
> - if (irq >= cs->gic->num_irq) {
> + if ((irq >= cs->gic->num_irq) && (!(cs->gic->lpi_enable &&
> + (irq >= GICV3_LPI_INTID_START {

Please put the line break after the first &&, not the second. That means
that you avoid linebreaking in the middle of a () expression.
Also you don't need the () on the outside of the !.

> /* This handles two cases:
> * 1. If software writes the ID of a spurious interrupt [ie 1020-1023]
> * to the GICC_EOIR, the GIC ignores that write.
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 0a978cf55b..e0fbd4041f 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -211,6 +211,7 @@ static MemTxResult process_int(GICv3ITSState *s, uint64_t 
> value,
> bool ite_valid = false;
> uint64_t cte = 0;
> bool cte_valid = false;
> + uint64_t rdbase;
> u

Re: [RFC PATCH v3 1/2] Adding Andes AX25 CPU model

[Bin Meng]

Hi Ruinland,

On Thu, Jun 10, 2021 at 10:45 PM Ruinland Chuan-Tzu Tsai
 wrote:
>
> From: Ruinaldn ChuanTzu Tsai 
>
> Adding the skeleton of Andes Technology AX25 CPU model for the future commits,
> which will utilize custom/vendor CSR handling mechaism.

typo: mechanism

> ---
>  target/riscv/cpu.c | 8 
>  target/riscv/cpu.h | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddea8fbeeb..4ae21cbf9b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -159,6 +159,13 @@ static void rv64_base_cpu_init(Object *obj)
>  set_misa(env, RV64);
>  }
>
> +static void ax25_cpu_init(Object *obj)
> +{
> +CPURISCVState *env = &RISCV_CPU(obj)->env;
> +set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> +set_priv_version(env, PRIV_VERSION_1_10_0);
> +}
> +
>  static void rv64_sifive_u_cpu_init(Object *obj)
>  {
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -705,6 +712,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,   rv32_sifive_u_cpu_init),
>  #elif defined(TARGET_RISCV64)
>  DEFINE_CPU(TYPE_RISCV_CPU_BASE64,   rv64_base_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),

What about the 32-bit variant of A25, and the SMP variant of A25MP/AX25MP?

Also how about the latest A45 (RV32) and AX45 (RV64)?

How should we name these? I think we may need to name this using the
SMP variant name, no?

>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,   rv64_sifive_e_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   rv64_sifive_u_cpu_init),
>  #endif

Regards,
Bin

Re: [PATCH 4/4] hw/riscv: virt: Use AIA INTC compatible string when available

[Alistair Francis]

On Sat, May 15, 2021 at 12:36 AM Anup Patel  wrote:
>
> We should use the AIA INTC compatible string in the CPU INTC
> DT nodes when the CPUs support AIA feature. This will allow
> Linux INTC driver to use AIA local interrupt CSRs.
>
> Signed-off-by: Anup Patel 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/riscv/virt.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index c0dc69ff33..981a3a06d5 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -262,8 +262,15 @@ static void create_fdt(RISCVVirtState *s, const 
> MemMapEntry *memmap,
>  qemu_fdt_add_subnode(fdt, intc_name);
>  intc_phandle = phandle++;
>  qemu_fdt_setprop_cell(fdt, intc_name, "phandle", intc_phandle);
> -qemu_fdt_setprop_string(fdt, intc_name, "compatible",
> -"riscv,cpu-intc");
> +if (riscv_feature(&s->soc[socket].harts[cpu].env,
> +  RISCV_FEATURE_AIA)) {
> +const char intcomp[] = "riscv,cpu-intc-aia\0riscv,cpu-intc";
> +qemu_fdt_setprop(fdt, name, "compatible",
> +intcomp, sizeof(intcomp));
> +} else {
> +qemu_fdt_setprop_string(fdt, intc_name, "compatible",
> +"riscv,cpu-intc");
> +}
>  qemu_fdt_setprop(fdt, intc_name, "interrupt-controller", NULL, 
> 0);
>  qemu_fdt_setprop_cell(fdt, intc_name, "#interrupt-cells", 1);
>
> --
> 2.25.1
>
>

Re: [PATCH 3/4] target/riscv: Implement AIA local interrupt CSRs

[Alistair Francis]

On Sat, May 15, 2021 at 12:34 AM Anup Patel  wrote:
>
> We implement various AIA local interrupt CSRs for M-mode, HS-mode,
> and VS-mode.
>
> Signed-off-by: Anup Patel 
> ---
>  target/riscv/cpu.c|   27 +-
>  target/riscv/cpu.h|   52 +-
>  target/riscv/cpu_helper.c |  245 -
>  target/riscv/csr.c| 1059 +++--
>  target/riscv/machine.c|   26 +-
>  5 files changed, 1309 insertions(+), 100 deletions(-)

I feel this patch could be split up more :)

>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f3702111ae..795162834b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -256,11 +256,11 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, 
> int flags)
>  qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vsstatus ",
>   (target_ulong)env->vsstatus);
>  }
> -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mip ", env->mip);
> -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mie ", env->mie);
> -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mideleg ", env->mideleg);
> +qemu_fprintf(f, " %s %016" PRIx64 "\n", "mip ", env->mip);
> +qemu_fprintf(f, " %s %016" PRIx64 "\n", "mie ", env->mie);
> +qemu_fprintf(f, " %s %016" PRIx64 "\n", "mideleg ", env->mideleg);
>  if (riscv_has_ext(env, RVH)) {
> -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hideleg ", env->hideleg);
> +qemu_fprintf(f, " %s %016" PRIx64 "\n", "hideleg ", env->hideleg);
>  }
>  qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "medeleg ", env->medeleg);
>  if (riscv_has_ext(env, RVH)) {
> @@ -345,6 +345,8 @@ void restore_state_to_opc(CPURISCVState *env, 
> TranslationBlock *tb,
>
>  static void riscv_cpu_reset(DeviceState *dev)
>  {
> +uint8_t iprio;
> +int i, irq, rdzero;
>  CPUState *cs = CPU(dev);
>  RISCVCPU *cpu = RISCV_CPU(cs);
>  RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> @@ -357,6 +359,23 @@ static void riscv_cpu_reset(DeviceState *dev)
>  env->mcause = 0;
>  env->pc = env->resetvec;
>  env->two_stage_lookup = false;
> +
> +/* Initialized default priorities of local interrupts. */
> +for (i = 0; i < ARRAY_SIZE(env->miprio); i++) {
> +iprio = riscv_cpu_default_priority(i);
> +env->miprio[i] = iprio;
> +env->siprio[i] = iprio;
> +env->hviprio[i] = IPRIO_DEFAULT_MMAXIPRIO;
> +}
> +i = 0;
> +while (!riscv_cpu_hviprio_index2irq(i, &irq, &rdzero)) {
> +if (rdzero) {
> +env->hviprio[irq] = 0;
> +} else {
> +env->hviprio[irq] = env->miprio[irq];
> +}
> +i++;
> +}
>  #endif
>  cs->exception_index = EXCP_NONE;
>  env->load_res = -1;
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index f00c60c840..780d3f9058 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -157,12 +157,12 @@ struct CPURISCVState {
>   */
>  uint64_t mstatus;
>
> -target_ulong mip;
> +uint64_t mip;

This isn't right. MIP is a MXLEN-1 CSR. I didn't check but I assume
all the other existing target_ulong CSRs are the same.

Alistair

>
> -uint32_t miclaim;
> +uint64_t miclaim;
>
> -target_ulong mie;
> -target_ulong mideleg;
> +uint64_t mie;
> +uint64_t mideleg;
>
>  target_ulong sptbr;  /* until: priv-1.9.1 */
>  target_ulong satp;   /* since: priv-1.10.0 */
> @@ -179,16 +179,27 @@ struct CPURISCVState {
>  target_ulong mcause;
>  target_ulong mtval;  /* since: priv-1.10.0 */
>
> +/* AIA CSRs */
> +target_ulong miselect;
> +target_ulong siselect;
> +
> +uint8_t miprio[64];
> +uint8_t siprio[64];
> +
>  /* Hypervisor CSRs */
>  target_ulong hstatus;
>  target_ulong hedeleg;
> -target_ulong hideleg;
> +uint64_t hideleg;
>  target_ulong hcounteren;
>  target_ulong htval;
>  target_ulong htinst;
>  target_ulong hgatp;
>  uint64_t htimedelta;
>
> +/* AIA HS-mode CSRs */
> +uint8_t hviprio[64];
> +target_ulong hvicontrol;
> +
>  /* Virtual CSRs */
>  /*
>   * For RV32 this is 32-bit vsstatus and 32-bit vsstatush.
> @@ -202,6 +213,9 @@ struct CPURISCVState {
>  target_ulong vstval;
>  target_ulong vsatp;
>
> +/* AIA VS-mode CSRs */
> +target_ulong vsiselect;
> +
>  target_ulong mtval2;
>  target_ulong mtinst;
>
> @@ -236,6 +250,18 @@ struct CPURISCVState {
>  uint64_t (*rdtime_fn)(uint32_t);
>  uint32_t rdtime_fn_arg;
>
> +/* machine specific AIA IMSIC read-modify-write callback */
> +#define IMSIC_MAKE_REG(__isel, __priv, __virt, __vgein) \
> +__vgein) & 0x3f) << 24) | (((__virt) & 0x1) << 20) | \
> + (((__priv) & 0x3) << 16) | (__isel & 0x))
> +#define IMSIC_REG_ISEL(__reg)  ((__reg) & 0x)
> +#define IMSIC_REG_PRIV(__reg)  (((__reg) >> 16) & 0x3)
> +#define IMSIC_REG_VIRT(__reg)  (((__reg) >> 20) & 

Re: [PATCH 2/4] target/riscv: Add CPU feature for AIA CSRs

[Alistair Francis]

On Sat, May 15, 2021 at 12:35 AM Anup Patel  wrote:
>
> We add experimental CPU feature to enable AIA CSRs. This experimental
> feature can be enabled by setting "x-aia=true" for CPU in the QEMU
> command-line parameters.
>
> Signed-off-by: Anup Patel 
> ---
>  target/riscv/cpu.c | 5 +
>  target/riscv/cpu.h | 4 +++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7d6ed80f6b..f3702111ae 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -414,6 +414,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>  set_feature(env, RISCV_FEATURE_PMP);
>  }
>
> +if (cpu->cfg.aia) {
> +set_feature(env, RISCV_FEATURE_AIA);
> +}
> +
>  set_resetvec(env, cpu->cfg.resetvec);
>
>  /* If only XLEN is set for misa, then set misa from properties */
> @@ -554,6 +558,7 @@ static Property riscv_cpu_properties[] = {
>  DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>  DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
>  DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
> +DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),

This line should be a seperate patch at the end of the series.

The idea is that we don't allow users to enable the feature until it
has been fully implemented.

Alistair

>  DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
>  DEFINE_PROP_END_OF_LIST(),
>  };
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0a33d387ba..f00c60c840 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -80,7 +80,8 @@
>  enum {
>  RISCV_FEATURE_MMU,
>  RISCV_FEATURE_PMP,
> -RISCV_FEATURE_MISA
> +RISCV_FEATURE_MISA,
> +RISCV_FEATURE_AIA
>  };
>
>  #define PRIV_VERSION_1_10_0 0x00011000
> @@ -303,6 +304,7 @@ struct RISCVCPU {
>  uint16_t elen;
>  bool mmu;
>  bool pmp;
> +bool aia;
>  uint64_t resetvec;
>  } cfg;
>  };
> --
> 2.25.1
>
>

Re: [PATCH 2/2] target/riscv: remove force HS exception

[Alistair Francis]

On Thu, Jun 3, 2021 at 5:12 AM Jose Martins  wrote:
>
> There is no need to "force an hs exception" as the current privilege
> level, the state of the global ie and of the delegation registers should
> be enough to route the interrupt to the appropriate privilege level in
> riscv_cpu_do_interrupt. The is true for both asynchronous and
> synchronous exceptions, specifically, guest page faults which must be
> hardwired to zero hedeleg. As such the hs_force_except mechanism can be
> removed.
>
> Signed-off-by: Jose Martins 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.h|  2 --
>  target/riscv/cpu_bits.h   |  6 --
>  target/riscv/cpu_helper.c | 26 +-
>  3 files changed, 1 insertion(+), 33 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0a33d387ba..a30a64241a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -337,8 +337,6 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int 
> interrupt_request);
>  bool riscv_cpu_fp_enabled(CPURISCVState *env);
>  bool riscv_cpu_virt_enabled(CPURISCVState *env);
>  void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
> -bool riscv_cpu_force_hs_excep_enabled(CPURISCVState *env);
> -void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable);
>  bool riscv_cpu_two_stage_lookup(int mmu_idx);
>  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
>  hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index caf4599207..7322f54157 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -462,12 +462,6 @@
>
>  /* Virtulisation Register Fields */
>  #define VIRT_ONOFF  1
> -/* This is used to save state for when we take an exception. If this is set
> - * that means that we want to force a HS level exception (no matter what the
> - * delegation is set to). This will occur for things such as a second level
> - * page table fault.
> - */
> -#define FORCE_HS_EXCEP  2
>
>  /* RV32 satp CSR field masks */
>  #define SATP32_MODE 0x8000
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 592d4642be..babe3d844b 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -178,24 +178,6 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool 
> enable)
>  env->virt = set_field(env->virt, VIRT_ONOFF, enable);
>  }
>
> -bool riscv_cpu_force_hs_excep_enabled(CPURISCVState *env)
> -{
> -if (!riscv_has_ext(env, RVH)) {
> -return false;
> -}
> -
> -return get_field(env->virt, FORCE_HS_EXCEP);
> -}
> -
> -void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable)
> -{
> -if (!riscv_has_ext(env, RVH)) {
> -return;
> -}
> -
> -env->virt = set_field(env->virt, FORCE_HS_EXCEP, enable);
> -}
> -
>  bool riscv_cpu_two_stage_lookup(int mmu_idx)
>  {
>  return mmu_idx & TB_FLAGS_PRIV_HYP_ACCESS_MASK;
> @@ -884,7 +866,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>
>  RISCVCPU *cpu = RISCV_CPU(cs);
>  CPURISCVState *env = &cpu->env;
> -bool force_hs_execp = riscv_cpu_force_hs_excep_enabled(env);
>  uint64_t s;
>
>  /* cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide
> @@ -913,8 +894,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>  case RISCV_EXCP_INST_GUEST_PAGE_FAULT:
>  case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
>  case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
> -force_hs_execp = true;
> -/* fallthrough */
>  case RISCV_EXCP_INST_ADDR_MIS:
>  case RISCV_EXCP_INST_ACCESS_FAULT:
>  case RISCV_EXCP_LOAD_ADDR_MIS:
> @@ -973,8 +952,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>  env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0);
>  }
>
> -if (riscv_cpu_virt_enabled(env) && ((hdeleg >> cause) & 1) &&
> -!force_hs_execp) {
> +if (riscv_cpu_virt_enabled(env) && ((hdeleg >> cause) & 1)) {
>  /* Trap to VS mode */
>  /*
>   * See if we need to adjust cause. Yes if its VS mode 
> interrupt
> @@ -996,7 +974,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>  htval = env->guest_phys_fault_addr;
>
>  riscv_cpu_set_virt_enabled(env, 0);
> -riscv_cpu_set_force_hs_excep(env, 0);
>  } else {
>  /* Trap into HS mode */
>  env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false);
> @@ -1032,7 +1009,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>
>  /* Trapping to M mode, virt is disabled */
>  riscv_cpu_set_virt_enabled(env, 0);
> -riscv_cpu_set_force_hs_excep(env, 0);
>  }
>
>  s = env->mstatus;
> --
> 2.30.2
>
>

Re: [PATCH 1/2] target/riscv: fix VS interrupts forwarding to HS

[Alistair Francis]

On Thu, Jun 3, 2021 at 5:13 AM Jose Martins  wrote:
>
> VS interrupts (2, 6, 10) were not correctly forwarded to hs-mode when
> not delegated in hideleg (which was not being taken into account). This
> was mainly because hs level sie was not always considered enabled when
> it should. The spec states that "Interrupts for higher-privilege modes,
> y>x, are always globally enabled regardless of the setting of the global
> yIE bit for the higher-privilege mode." and also "For purposes of
> interrupt global enables, HS-mode is considered more privileged than
> VS-mode, and VS-mode is considered more privileged than VU-mode". Also,
> vs-level interrupts were not being taken into account unless V=1, but
> should be unless delegated.
>
> Finally, there is no need for a special case for to handle vs interrupts
> as the current privilege level, the state of the global ie and of the
> delegation registers should be enough to route all interrupts to the
> appropriate privilege level in riscv_cpu_do_interrupt.
>
> Signed-off-by: Jose Martins 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu_helper.c | 28 
>  1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 21c54ef561..592d4642be 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -38,36 +38,24 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>  #ifndef CONFIG_USER_ONLY
>  static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>  {
> -target_ulong irqs;
> +target_ulong virt_enabled = riscv_cpu_virt_enabled(env);
>
>  target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
>  target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
> -target_ulong hs_mstatus_sie = get_field(env->mstatus_hs, MSTATUS_SIE);
>
> -target_ulong pending = env->mip & env->mie &
> -   ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
> -target_ulong vspending = (env->mip & env->mie &
> -  (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP));
> +target_ulong pending = env->mip & env->mie;
>
>  target_ulong mie= env->priv < PRV_M ||
>(env->priv == PRV_M && mstatus_mie);
>  target_ulong sie= env->priv < PRV_S ||
>(env->priv == PRV_S && mstatus_sie);
> -target_ulong hs_sie = env->priv < PRV_S ||
> -  (env->priv == PRV_S && hs_mstatus_sie);
> +target_ulong hsie   = virt_enabled || sie;
> +target_ulong vsie   = virt_enabled && sie;
>
> -if (riscv_cpu_virt_enabled(env)) {
> -target_ulong pending_hs_irq = pending & -hs_sie;
> -
> -if (pending_hs_irq) {
> -riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> -return ctz64(pending_hs_irq);
> -}
> -
> -pending = vspending;
> -}
> -
> -irqs = (pending & ~env->mideleg & -mie) | (pending &  env->mideleg & 
> -sie);
> +target_ulong irqs =
> +(pending & ~env->mideleg & -mie) |
> +(pending &  env->mideleg & ~env->hideleg & -hsie) |
> +(pending &  env->mideleg &  env->hideleg & -vsie);
>
>  if (irqs) {
>  return ctz64(irqs); /* since non-zero */
> --
> 2.30.2
>
>

Re: [PATCH] target/riscv: hardwire bits in hideleg and hedeleg

[Alistair Francis]

On Sun, May 23, 2021 at 1:59 AM Jose Martins  wrote:
>
> The specification mandates for certain bits to be hardwired in the
> hypervisor delegation registers. This was not being enforced.
>
> Signed-off-by: Jose Martins 
> ---
>  target/riscv/csr.c | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d2585395bf..9b74a00cc9 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -394,6 +394,7 @@ static int read_timeh(CPURISCVState *env, int csrno, 
> target_ulong *val)
>
>  static const target_ulong delegable_ints = S_MODE_INTERRUPTS |
> VS_MODE_INTERRUPTS;
> +static const target_ulong vs_delegable_ints = VS_MODE_INTERRUPTS;
>  static const target_ulong all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
>   VS_MODE_INTERRUPTS;
>  static const target_ulong delegable_excps =
> @@ -416,6 +417,14 @@ static const target_ulong delegable_excps =
>  (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
>  (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
>  (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT));
> +static const target_ulong vs_delegable_excps = delegable_excps &
> +~((1ULL << (RISCV_EXCP_S_ECALL)) |

> +(1ULL << (RISCV_EXCP_VS_ECALL)) |
> +(1ULL << (RISCV_EXCP_M_ECALL)) |

These two are both read only 0, shouldn't they not be included in this list?

> +(1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) |
> +(1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
> +(1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
> +(1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)));
>  static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
>  SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
>  SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD;
> @@ -963,7 +972,7 @@ static int read_hedeleg(CPURISCVState *env, int csrno, 
> target_ulong *val)
>
>  static int write_hedeleg(CPURISCVState *env, int csrno, target_ulong val)
>  {
> -env->hedeleg = val;
> +env->hedeleg = val & vs_delegable_excps;

Because we then allow a write to occur here.

Alistair

>  return 0;
>  }
>
> @@ -975,7 +984,7 @@ static int read_hideleg(CPURISCVState *env, int csrno, 
> target_ulong *val)
>
>  static int write_hideleg(CPURISCVState *env, int csrno, target_ulong val)
>  {
> -env->hideleg = val;
> +env->hideleg = val & vs_delegable_ints;
>  return 0;
>  }
>
> --
> 2.30.2
>
>

[PATCH v2 4/8] ui/gtk: Implement wait_dmabuf function

[Vivek Kasireddy]

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 ui/gtk.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 6132bab52f..cd884ca26c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -583,6 +583,19 @@ static void gd_gl_release_dmabuf(DisplayChangeListener 
*dcl,
 #endif
 }
 
+static void gd_gl_wait_dmabuf(DisplayChangeListener *dcl,
+  QemuDmaBuf *dmabuf)
+{
+#ifdef CONFIG_GBM
+egl_dmabuf_create_fence(dmabuf);
+if (dmabuf->fence_fd <= 0) {
+return;
+}
+
+egl_dmabuf_wait_sync(dmabuf);
+#endif
+}
+
 /** DisplayState Callbacks (opengl version) **/
 
 static const DisplayChangeListenerOps dcl_gl_area_ops = {
@@ -602,6 +615,7 @@ static const DisplayChangeListenerOps dcl_gl_area_ops = {
 .dpy_gl_update   = gd_gl_area_scanout_flush,
 .dpy_gl_scanout_dmabuf   = gd_gl_area_scanout_dmabuf,
 .dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
+.dpy_gl_wait_dmabuf  = gd_gl_wait_dmabuf,
 .dpy_has_dmabuf  = gd_has_dmabuf,
 };
 
@@ -626,6 +640,7 @@ static const DisplayChangeListenerOps dcl_egl_ops = {
 .dpy_gl_cursor_position  = gd_egl_cursor_position,
 .dpy_gl_update   = gd_egl_scanout_flush,
 .dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
+.dpy_gl_wait_dmabuf  = gd_gl_wait_dmabuf,
 .dpy_has_dmabuf  = gd_has_dmabuf,
 };
 
-- 
2.30.2

Re: [PATCH v2 2/3] hw/timer: Initial commit of Ibex Timer

[Alistair Francis]

On Wed, Jun 9, 2021 at 5:57 PM Paolo Bonzini  wrote:
>
> On 09/06/21 01:48, Alistair Francis wrote:
> > Add support for the Ibex timer. This is used with the RISC-V
> > mtime/mtimecmp similar to the SiFive CLINT.
> >
> > We currently don't support changing the prescale or the timervalue.
> >
> > Signed-off-by: Alistair Francis 
>
> Any chance this could have a qtest?  It would also be nice if the CPU

Yep, do you have a good example of what the qtest should look like?

> had qemu_irqs for MEIP/MTIP/SEIP (possibly MSIP too but that one is
> bidirectional), so that instead of riscv_cpu_update_mip you can just
> connect to a GPIO pin of the CPU and do qemu_set_irq.  It could also be
> used by the qtests via irq_intercept_in.

Yeah the riscv_cpu_update_mip() is not ideal. As it is what we
currently also use for the CLINT I don't want to fix it up here. In
the future I'll work on changing riscv_cpu_update_mip in all the
RISC-V timers to use GPIO lines instead.

Alistair

>
> Paolo
>
> > ---
> >   include/hw/timer/ibex_timer.h |  52 ++
> >   hw/timer/ibex_timer.c | 305 ++
> >   MAINTAINERS   |   6 +-
> >   hw/timer/meson.build  |   1 +
> >   4 files changed, 360 insertions(+), 4 deletions(-)
> >   create mode 100644 include/hw/timer/ibex_timer.h
> >   create mode 100644 hw/timer/ibex_timer.c
> >
> > diff --git a/include/hw/timer/ibex_timer.h b/include/hw/timer/ibex_timer.h
> > new file mode 100644
> > index 00..6a43537003
> > --- /dev/null
> > +++ b/include/hw/timer/ibex_timer.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * QEMU lowRISC Ibex Timer device
> > + *
> > + * Copyright (c) 2021 Western Digital
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#ifndef HW_IBEX_TIMER_H
> > +#define HW_IBEX_TIMER_H
> > +
> > +#include "hw/sysbus.h"
> > +
> > +#define TYPE_IBEX_TIMER "ibex-timer"
> > +OBJECT_DECLARE_SIMPLE_TYPE(IbexTimerState, IBEX_TIMER)
> > +
> > +struct IbexTimerState {
> > +/*  */
> > +SysBusDevice parent_obj;
> > +
> > +/*  */
> > +MemoryRegion mmio;
> > +
> > +uint32_t timer_ctrl;
> > +uint32_t timer_cfg0;
> > +uint32_t timer_compare_lower0;
> > +uint32_t timer_compare_upper0;
> > +uint32_t timer_intr_enable;
> > +uint32_t timer_intr_state;
> > +uint32_t timer_intr_test;
> > +
> > +uint32_t timebase_freq;
> > +
> > +qemu_irq irq;
> > +};
> > +#endif /* HW_IBEX_TIMER_H */
> > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c
> > new file mode 100644
> > index 00..4d55eb5088
> > --- /dev/null
> > +++ b/hw/timer/ibex_timer.c
> > @@ -0,0 +1,305 @@
> > +/*
> > + * QEMU lowRISC Ibex Timer device
> > + *
> > + * Copyright (c) 2021 Western Digital
> > + *
> > + * For details check the documentation here:
> > + *https://docs.opentitan.org/hw/ip/rv_timer/doc/
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FO

[PATCH v2 2/8] ui/egl: Add egl helpers to help with synchronization

[Vivek Kasireddy]

These egl helpers would be used for creating and waiting on
a sync object.

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 include/ui/console.h |  2 ++
 include/ui/egl-helpers.h |  3 +++
 ui/egl-helpers.c | 44 
 3 files changed, 49 insertions(+)

diff --git a/include/ui/console.h b/include/ui/console.h
index b30b63976a..49978fdae3 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -168,6 +168,8 @@ typedef struct QemuDmaBuf {
 uint64_t  modifier;
 uint32_t  texture;
 bool  y0_top;
+void  *sync;
+int   fence_fd;
 } QemuDmaBuf;
 
 typedef struct DisplayState DisplayState;
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index f1bf8f97fc..5a7575dc13 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -45,6 +45,9 @@ int egl_get_fd_for_texture(uint32_t tex_id, EGLint *stride, 
EGLint *fourcc,
 
 void egl_dmabuf_import_texture(QemuDmaBuf *dmabuf);
 void egl_dmabuf_release_texture(QemuDmaBuf *dmabuf);
+void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf);
+void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf);
+void egl_dmabuf_wait_sync(QemuDmaBuf *dmabuf);
 
 #endif
 
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 6d0cb2b5cb..47220b66e0 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -76,6 +76,50 @@ void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
   GL_TEXTURE_2D, fb->texture, 0);
 }
 
+void egl_dmabuf_create_sync(QemuDmaBuf *dmabuf)
+{
+EGLSyncKHR sync;
+
+if (epoxy_has_egl_extension(qemu_egl_display,
+"EGL_KHR_fence_sync") &&
+epoxy_has_egl_extension(qemu_egl_display,
+"EGL_ANDROID_native_fence_sync")) {
+sync = eglCreateSyncKHR(qemu_egl_display,
+   EGL_SYNC_NATIVE_FENCE_ANDROID, NULL);
+if (sync != EGL_NO_SYNC_KHR) {
+dmabuf->sync = sync;
+}
+}
+}
+
+void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
+{
+if (dmabuf->sync) {
+dmabuf->fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
+  dmabuf->sync);
+eglDestroySyncKHR(qemu_egl_display, dmabuf->sync);
+dmabuf->sync = NULL;
+}
+}
+
+void egl_dmabuf_wait_sync(QemuDmaBuf *dmabuf)
+{
+EGLSyncKHR sync;
+EGLint attrib_list[] = {
+EGL_SYNC_NATIVE_FENCE_FD_ANDROID, dmabuf->fence_fd,
+EGL_NONE,
+};
+
+sync = eglCreateSyncKHR(qemu_egl_display,
+EGL_SYNC_NATIVE_FENCE_ANDROID, attrib_list);
+if (sync != EGL_NO_SYNC_KHR) {
+eglClientWaitSyncKHR(qemu_egl_display, sync,
+ 0, EGL_FOREVER_KHR);
+eglDestroySyncKHR(qemu_egl_display, sync);
+dmabuf->fence_fd = -1;
+}
+}
+
 void egl_fb_setup_new_tex(egl_fb *fb, int width, int height)
 {
 GLuint texture;
-- 
2.30.2

[PATCH v2 5/8] ui: Create sync objects only for blobs

[Vivek Kasireddy]

For now, create sync objects only for dmabufs that are blobs.

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 hw/display/virtio-gpu-udmabuf.c |  2 ++
 include/ui/console.h|  1 +
 include/ui/egl-helpers.h|  1 +
 ui/gtk-egl.c| 10 ++
 ui/gtk-gl-area.c|  8 
 5 files changed, 22 insertions(+)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 3c01a415e7..33e329e8aa 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -185,6 +185,8 @@ static VGPUDMABuf
 dmabuf->buf.stride = fb->stride;
 dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
 dmabuf->buf.fd = res->dmabuf_fd;
+dmabuf->buf.blob = true;
+dmabuf->buf.sync = NULL;
 
 dmabuf->scanout_id = scanout_id;
 QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next);
diff --git a/include/ui/console.h b/include/ui/console.h
index a89f739f10..310d34c67a 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -170,6 +170,7 @@ typedef struct QemuDmaBuf {
 bool  y0_top;
 void  *sync;
 int   fence_fd;
+bool  blob;
 } QemuDmaBuf;
 
 typedef struct DisplayState DisplayState;
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 5a7575dc13..1bc0e31b03 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -19,6 +19,7 @@ typedef struct egl_fb {
 GLuint texture;
 GLuint framebuffer;
 bool delete_texture;
+QemuDmaBuf *dmabuf;
 } egl_fb;
 
 void egl_fb_destroy(egl_fb *fb);
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index b671181272..b748f51b0b 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -209,6 +209,8 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
QemuDmaBuf *dmabuf)
 {
 #ifdef CONFIG_GBM
+VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+
 egl_dmabuf_import_texture(dmabuf);
 if (!dmabuf->texture) {
 return;
@@ -217,6 +219,10 @@ void gd_egl_scanout_dmabuf(DisplayChangeListener *dcl,
 gd_egl_scanout_texture(dcl, dmabuf->texture,
false, dmabuf->width, dmabuf->height,
0, 0, dmabuf->width, dmabuf->height);
+
+if (dmabuf->blob) {
+vc->gfx.guest_fb.dmabuf = dmabuf;
+}
 #endif
 }
 
@@ -281,6 +287,10 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
 egl_fb_blit(&vc->gfx.win_fb, &vc->gfx.guest_fb, !vc->gfx.y0_top);
 }
 
+if (vc->gfx.guest_fb.dmabuf) {
+egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf);
+}
+
 eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
 }
 
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index dd5783fec7..94f3b87c42 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -71,6 +71,10 @@ void gd_gl_area_draw(VirtualConsole *vc)
 surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
 }
 
+if (vc->gfx.guest_fb.dmabuf) {
+egl_dmabuf_create_sync(vc->gfx.guest_fb.dmabuf);
+}
+
 glFlush();
 graphic_hw_gl_flushed(vc->gfx.dcl.con);
 }
@@ -231,6 +235,10 @@ void gd_gl_area_scanout_dmabuf(DisplayChangeListener *dcl,
 gd_gl_area_scanout_texture(dcl, dmabuf->texture,
false, dmabuf->width, dmabuf->height,
0, 0, dmabuf->width, dmabuf->height);
+
+if (dmabuf->blob) {
+vc->gfx.guest_fb.dmabuf = dmabuf;
+}
 #endif
 }
 
-- 
2.30.2

Re: [PATCH v1 1/1] target/riscv: Use target_ulong for the DisasContext misa

[Alistair Francis]

On Mon, May 31, 2021 at 2:27 PM Alistair Francis
 wrote:
>
> The is_32bit() check in translate.c expects a 64-bit guest to have a
> 64-bit misa value otherwise the macro check won't work. This patches
> fixes that and fixes a Coverity issue at the same time.
>
> Fixes: CID 1453107
> Signed-off-by: Alistair Francis 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index e945352bca..a35a58df92 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -47,7 +47,7 @@ typedef struct DisasContext {
>  bool virt_enabled;
>  uint32_t opcode;
>  uint32_t mstatus_fs;
> -uint32_t misa;
> +target_ulong misa;
>  uint32_t mem_idx;
>  /* Remember the rounding mode encoded in the previous fp instruction,
> which we have already installed into env->fp_status.  Or -1 for
> --
> 2.31.1
>

[PATCH v2 8/8] virtio-gpu: Add gl_flushed callback

[Vivek Kasireddy]

Adding this callback provides a way to determine when the UI
has submitted the buffer to the Host windowing system. Making
the guest wait for this event will ensure that the dmabuf/buffer
updates are synchronized.

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 hw/display/virtio-gpu.c | 44 -
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4d549377cb..bd96332973 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -982,7 +982,7 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
 cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
 break;
 }
-if (!cmd->finished) {
+if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
 virtio_gpu_ctrl_response_nodata(g, cmd, cmd->error ? cmd->error :
 VIRTIO_GPU_RESP_OK_NODATA);
 }
@@ -1040,6 +1040,46 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
 g->processing_cmdq = false;
 }
 
+static void virtio_gpu_signal_fence(VirtIOGPU *g,
+struct virtio_gpu_ctrl_command *cmd,
+enum virtio_gpu_ctrl_type type)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_flush rf;
+
+VIRTIO_GPU_FILL_CMD(rf);
+virtio_gpu_bswap_32(&rf, sizeof(rf));
+res = virtio_gpu_find_check_resource(g, rf.resource_id, true,
+ __func__, &cmd->error);
+if (res) {
+virtio_gpu_resource_wait_sync(g, res);
+}
+virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
+}
+
+static void virtio_gpu_process_fenceq(VirtIOGPU *g)
+{
+struct virtio_gpu_ctrl_command *cmd, *tmp;
+
+QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
+trace_virtio_gpu_fence_resp(cmd->cmd_hdr.fence_id);
+virtio_gpu_signal_fence(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
+QTAILQ_REMOVE(&g->fenceq, cmd, next);
+g_free(cmd);
+g->inflight--;
+if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
+fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
+}
+}
+}
+
+static void virtio_gpu_handle_gl_flushed(VirtIOGPUBase *b)
+{
+VirtIOGPU *g = container_of(b, VirtIOGPU, parent_obj);
+
+virtio_gpu_process_fenceq(g);
+}
+
 static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1398,10 +1438,12 @@ static void virtio_gpu_class_init(ObjectClass *klass, 
void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
 VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
+VirtIOGPUBaseClass *vgbc = &vgc->parent;
 
 vgc->handle_ctrl = virtio_gpu_handle_ctrl;
 vgc->process_cmd = virtio_gpu_simple_process_cmd;
 vgc->update_cursor_data = virtio_gpu_update_cursor_data;
+vgbc->gl_flushed = virtio_gpu_handle_gl_flushed;
 
 vdc->realize = virtio_gpu_device_realize;
 vdc->reset = virtio_gpu_reset;
-- 
2.30.2

[PATCH v2 1/8] ui/gtk: Create a common release_dmabuf helper

[Vivek Kasireddy]

Since the texture release mechanism is same for both gtk-egl
and gtk-glarea, move the helper from gtk-egl to common gtk
code so that it can be shared by both gtk backends.

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 include/ui/gtk.h |  2 --
 ui/gtk-egl.c |  8 
 ui/gtk.c | 11 ++-
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 9516670ebc..e6cbf0507c 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -178,8 +178,6 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
   uint32_t hot_x, uint32_t hot_y);
 void gd_egl_cursor_position(DisplayChangeListener *dcl,
 uint32_t pos_x, uint32_t pos_y);
-void gd_egl_release_dmabuf(DisplayChangeListener *dcl,
-   QemuDmaBuf *dmabuf);
 void gd_egl_scanout_flush(DisplayChangeListener *dcl,
   uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 void gtk_egl_init(DisplayGLMode mode);
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 2a2e6d3a17..b671181272 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -249,14 +249,6 @@ void gd_egl_cursor_position(DisplayChangeListener *dcl,
 vc->gfx.cursor_y = pos_y * vc->gfx.scale_y;
 }
 
-void gd_egl_release_dmabuf(DisplayChangeListener *dcl,
-   QemuDmaBuf *dmabuf)
-{
-#ifdef CONFIG_GBM
-egl_dmabuf_release_texture(dmabuf);
-#endif
-}
-
 void gd_egl_scanout_flush(DisplayChangeListener *dcl,
   uint32_t x, uint32_t y, uint32_t w, uint32_t h)
 {
diff --git a/ui/gtk.c b/ui/gtk.c
index 98046f577b..6132bab52f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -575,6 +575,14 @@ static bool gd_has_dmabuf(DisplayChangeListener *dcl)
 return vc->gfx.has_dmabuf;
 }
 
+static void gd_gl_release_dmabuf(DisplayChangeListener *dcl,
+ QemuDmaBuf *dmabuf)
+{
+#ifdef CONFIG_GBM
+egl_dmabuf_release_texture(dmabuf);
+#endif
+}
+
 /** DisplayState Callbacks (opengl version) **/
 
 static const DisplayChangeListenerOps dcl_gl_area_ops = {
@@ -593,6 +601,7 @@ static const DisplayChangeListenerOps dcl_gl_area_ops = {
 .dpy_gl_scanout_disable  = gd_gl_area_scanout_disable,
 .dpy_gl_update   = gd_gl_area_scanout_flush,
 .dpy_gl_scanout_dmabuf   = gd_gl_area_scanout_dmabuf,
+.dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
 .dpy_has_dmabuf  = gd_has_dmabuf,
 };
 
@@ -615,8 +624,8 @@ static const DisplayChangeListenerOps dcl_egl_ops = {
 .dpy_gl_scanout_dmabuf   = gd_egl_scanout_dmabuf,
 .dpy_gl_cursor_dmabuf= gd_egl_cursor_dmabuf,
 .dpy_gl_cursor_position  = gd_egl_cursor_position,
-.dpy_gl_release_dmabuf   = gd_egl_release_dmabuf,
 .dpy_gl_update   = gd_egl_scanout_flush,
+.dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
 .dpy_has_dmabuf  = gd_has_dmabuf,
 };
 
-- 
2.30.2

[PATCH v2 6/8] ui/gtk-egl: Wait for the draw signal for dmabuf blobs

[Vivek Kasireddy]

Instead of immediately drawing and submitting, queue and wait
for the draw signal if the dmabuf submitted is a blob.

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 include/ui/gtk.h |  2 ++
 ui/gtk-egl.c | 14 ++
 ui/gtk.c |  2 +-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index e6cbf0507c..34e767a1da 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -178,6 +178,8 @@ void gd_egl_cursor_dmabuf(DisplayChangeListener *dcl,
   uint32_t hot_x, uint32_t hot_y);
 void gd_egl_cursor_position(DisplayChangeListener *dcl,
 uint32_t pos_x, uint32_t pos_y);
+void gd_egl_flush(DisplayChangeListener *dcl,
+  uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 void gd_egl_scanout_flush(DisplayChangeListener *dcl,
   uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 void gtk_egl_init(DisplayGLMode mode);
diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index b748f51b0b..a5655b6bbc 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -294,6 +294,20 @@ void gd_egl_scanout_flush(DisplayChangeListener *dcl,
 eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
 }
 
+void gd_egl_flush(DisplayChangeListener *dcl,
+  uint32_t x, uint32_t y, uint32_t w, uint32_t h)
+{
+VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
+GtkWidget *area = vc->gfx.drawing_area;
+
+if (vc->gfx.guest_fb.dmabuf) {
+gtk_widget_queue_draw_area(area, x, y, w, h);
+return;
+}
+
+gd_egl_scanout_flush(&vc->gfx.dcl, x, y, w, h);
+}
+
 void gtk_egl_init(DisplayGLMode mode)
 {
 GdkDisplay *gdk_display = gdk_display_get_default();
diff --git a/ui/gtk.c b/ui/gtk.c
index cd884ca26c..af94f12a98 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -638,7 +638,7 @@ static const DisplayChangeListenerOps dcl_egl_ops = {
 .dpy_gl_scanout_dmabuf   = gd_egl_scanout_dmabuf,
 .dpy_gl_cursor_dmabuf= gd_egl_cursor_dmabuf,
 .dpy_gl_cursor_position  = gd_egl_cursor_position,
-.dpy_gl_update   = gd_egl_scanout_flush,
+.dpy_gl_update   = gd_egl_flush,
 .dpy_gl_release_dmabuf   = gd_gl_release_dmabuf,
 .dpy_gl_wait_dmabuf  = gd_gl_wait_dmabuf,
 .dpy_has_dmabuf  = gd_has_dmabuf,
-- 
2.30.2

[PATCH v2 3/8] ui: Add a helper to wait on a dmabuf sync object

[Vivek Kasireddy]

This will be called by virtio-gpu in the subsequent patches.

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 include/ui/console.h |  5 +
 ui/console.c | 10 ++
 2 files changed, 15 insertions(+)

diff --git a/include/ui/console.h b/include/ui/console.h
index 49978fdae3..a89f739f10 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -242,6 +242,9 @@ typedef struct DisplayChangeListenerOps {
 /* optional */
 void (*dpy_gl_release_dmabuf)(DisplayChangeListener *dcl,
   QemuDmaBuf *dmabuf);
+/* optional */
+void (*dpy_gl_wait_dmabuf)(DisplayChangeListener *dcl,
+   QemuDmaBuf *dmabuf);
 /* required if GL */
 void (*dpy_gl_update)(DisplayChangeListener *dcl,
   uint32_t x, uint32_t y, uint32_t w, uint32_t h);
@@ -314,6 +317,8 @@ void dpy_gl_cursor_position(QemuConsole *con,
 uint32_t pos_x, uint32_t pos_y);
 void dpy_gl_release_dmabuf(QemuConsole *con,
QemuDmaBuf *dmabuf);
+void dpy_gl_wait_dmabuf(QemuConsole *con,
+QemuDmaBuf *dmabuf);
 void dpy_gl_update(QemuConsole *con,
uint32_t x, uint32_t y, uint32_t w, uint32_t h);
 
diff --git a/ui/console.c b/ui/console.c
index 2de5f4105b..b0abfd2246 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1917,6 +1917,16 @@ void dpy_gl_release_dmabuf(QemuConsole *con,
 }
 }
 
+void dpy_gl_wait_dmabuf(QemuConsole *con,
+QemuDmaBuf *dmabuf)
+{
+assert(con->gl);
+
+if (con->gl->ops->dpy_gl_wait_dmabuf) {
+con->gl->ops->dpy_gl_wait_dmabuf(con->gl, dmabuf);
+}
+}
+
 void dpy_gl_update(QemuConsole *con,
uint32_t x, uint32_t y, uint32_t w, uint32_t h)
 {
-- 
2.30.2

[PATCH v2 0/8] virtio-gpu: Add a default synchronization mechanism for blobs

[Vivek Kasireddy]

When the Guest and Host are using Blob resources, there is a chance
that they may use the underlying storage associated with a Blob at
the same time leading to glitches such as flickering or tearing.
To prevent these from happening, the Host needs to ensure that it
waits until its Blit is completed by the Host GPU before letting
the Guest reuse the Blob.

This should be the default behavior regardless of the type of Guest
that is using Blob resources but would be particularly useful for 
Guests that are using frontbuffer rendering such as Linux with X
or Windows 10, etc.

The way it works is the Guest includes a fence as part of 
resource_flush and waits for it to be signalled. The Host will
queue a repaint request and signal the fence after it completes
waiting on the sync object associated with the Blit.

v2:
- Added more description in the cover letter
- Removed the wait from resource_flush and included it in
  a gl_flushed() callback

Cc: Gerd Hoffmann 
Cc: Dongwon Kim 
Cc: Tina Zhang 

Vivek Kasireddy (8):
  ui/gtk: Create a common release_dmabuf helper
  ui/egl: Add egl helpers to help with synchronization
  ui: Add a helper to wait on a dmabuf sync object
  ui/gtk: Implement wait_dmabuf function
  ui: Create sync objects only for blobs
  ui/gtk-egl: Wait for the draw signal for dmabuf blobs
  virtio-gpu: Add dmabuf helpers for synchronization
  virtio-gpu: Add gl_flushed callback

 hw/display/virtio-gpu-udmabuf.c | 30 ++
 hw/display/virtio-gpu.c | 44 -
 include/hw/virtio/virtio-gpu.h  |  2 ++
 include/ui/console.h|  8 ++
 include/ui/egl-helpers.h|  4 +++
 include/ui/gtk.h|  4 +--
 stubs/virtio-gpu-udmabuf.c  |  6 +
 ui/console.c| 10 
 ui/egl-helpers.c| 44 +
 ui/gtk-egl.c| 32 ++--
 ui/gtk-gl-area.c|  8 ++
 ui/gtk.c| 28 +++--
 12 files changed, 207 insertions(+), 13 deletions(-)

-- 
2.30.2

[PATCH v2 7/8] virtio-gpu: Add dmabuf helpers for synchronization

[Vivek Kasireddy]

These helpers will be used in the next subsequent patches to
wait until a dmabuf object (via a texture) has been used
by the UI to render and submit its buffer.

Cc: Gerd Hoffmann 
Signed-off-by: Vivek Kasireddy 
---
 hw/display/virtio-gpu-udmabuf.c | 28 
 include/hw/virtio/virtio-gpu.h  |  2 ++
 stubs/virtio-gpu-udmabuf.c  |  6 ++
 3 files changed, 36 insertions(+)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 33e329e8aa..8c1b6f8763 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -167,6 +167,34 @@ static void virtio_gpu_free_dmabuf(VirtIOGPU *g, 
VGPUDMABuf *dmabuf)
 g_free(dmabuf);
 }
 
+static VGPUDMABuf
+*virtio_gpu_find_dmabuf(VirtIOGPU *g,
+struct virtio_gpu_simple_resource *res)
+{
+VGPUDMABuf *dmabuf, *tmp;
+
+QTAILQ_FOREACH_SAFE(dmabuf, &g->dmabuf.bufs, next, tmp) {
+if (dmabuf->buf.fd == res->dmabuf_fd) {
+return dmabuf;
+}
+}
+
+return NULL;
+}
+
+void virtio_gpu_resource_wait_sync(VirtIOGPU *g,
+   struct virtio_gpu_simple_resource *res)
+{
+struct virtio_gpu_scanout *scanout;
+VGPUDMABuf *dmabuf;
+
+dmabuf = virtio_gpu_find_dmabuf(g, res);
+if (dmabuf && dmabuf->buf.sync) {
+scanout = &g->parent_obj.scanout[dmabuf->scanout_id];
+dpy_gl_wait_dmabuf(scanout->con, &dmabuf->buf);
+}
+}
+
 static VGPUDMABuf
 *virtio_gpu_create_dmabuf(VirtIOGPU *g,
   uint32_t scanout_id,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index bcf54d970f..9b9b499d06 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -274,6 +274,8 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
  uint32_t scanout_id,
  struct virtio_gpu_simple_resource *res,
  struct virtio_gpu_framebuffer *fb);
+void virtio_gpu_resource_wait_sync(VirtIOGPU *g,
+   struct virtio_gpu_simple_resource *res);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
diff --git a/stubs/virtio-gpu-udmabuf.c b/stubs/virtio-gpu-udmabuf.c
index 81f661441a..59dab1a66c 100644
--- a/stubs/virtio-gpu-udmabuf.c
+++ b/stubs/virtio-gpu-udmabuf.c
@@ -25,3 +25,9 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
 /* nothing (stub) */
 return 0;
 }
+
+void virtio_gpu_resource_wait_sync(VirtIOGPU *g,
+   struct virtio_gpu_simple_resource *res)
+{
+/* nothing (stub) */
+}
-- 
2.30.2

Re: [PATCH v2 2/3] hw/timer: Initial commit of Ibex Timer

[Alistair Francis]

On Wed, Jun 9, 2021 at 11:44 AM Bin Meng  wrote:
>
> On Wed, Jun 9, 2021 at 7:49 AM Alistair Francis
>  wrote:
> >
> > Add support for the Ibex timer. This is used with the RISC-V
> > mtime/mtimecmp similar to the SiFive CLINT.
> >
> > We currently don't support changing the prescale or the timervalue.
> >
> > Signed-off-by: Alistair Francis 
> > ---
> >  include/hw/timer/ibex_timer.h |  52 ++
> >  hw/timer/ibex_timer.c | 305 ++
> >  MAINTAINERS   |   6 +-
> >  hw/timer/meson.build  |   1 +
> >  4 files changed, 360 insertions(+), 4 deletions(-)
> >  create mode 100644 include/hw/timer/ibex_timer.h
> >  create mode 100644 hw/timer/ibex_timer.c
> >
> > diff --git a/include/hw/timer/ibex_timer.h b/include/hw/timer/ibex_timer.h
> > new file mode 100644
> > index 00..6a43537003
> > --- /dev/null
> > +++ b/include/hw/timer/ibex_timer.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * QEMU lowRISC Ibex Timer device
> > + *
> > + * Copyright (c) 2021 Western Digital
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#ifndef HW_IBEX_TIMER_H
> > +#define HW_IBEX_TIMER_H
> > +
> > +#include "hw/sysbus.h"
> > +
> > +#define TYPE_IBEX_TIMER "ibex-timer"
> > +OBJECT_DECLARE_SIMPLE_TYPE(IbexTimerState, IBEX_TIMER)
> > +
> > +struct IbexTimerState {
> > +/*  */
> > +SysBusDevice parent_obj;
> > +
> > +/*  */
> > +MemoryRegion mmio;
> > +
> > +uint32_t timer_ctrl;
> > +uint32_t timer_cfg0;
> > +uint32_t timer_compare_lower0;
> > +uint32_t timer_compare_upper0;
> > +uint32_t timer_intr_enable;
> > +uint32_t timer_intr_state;
> > +uint32_t timer_intr_test;
> > +
> > +uint32_t timebase_freq;
> > +
> > +qemu_irq irq;
> > +};
> > +#endif /* HW_IBEX_TIMER_H */
> > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c
> > new file mode 100644
> > index 00..4d55eb5088
> > --- /dev/null
> > +++ b/hw/timer/ibex_timer.c
> > @@ -0,0 +1,305 @@
> > +/*
> > + * QEMU lowRISC Ibex Timer device
> > + *
> > + * Copyright (c) 2021 Western Digital
> > + *
> > + * For details check the documentation here:
> > + *https://docs.opentitan.org/hw/ip/rv_timer/doc/
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qemu/timer.h"
> > +#include "hw/timer/ibex_timer.h"
> > +#include "hw/irq.h"
> > +#include "hw/qdev-properties.h"
> > +#include "target/riscv/cpu.h"
> > +#include "migration/vmstate.h"
> > +
> > +REG32(CTRL, 0x00)
> > +FIELD(CTRL, ACTIVE, 0, 1)
> > +REG32(CFG0, 0x100)
> > +FIELD(CFG0, PRESCALE, 0, 12)
> > +FIELD(CFG0, ST

Re: [RFC PATCH v3 2/2] Adding preliminary custom/vendor CSR handling mechanism

[Alistair Francis]

On Fri, Jun 11, 2021 at 12:44 AM Ruinland Chuan-Tzu Tsai
 wrote:
>
> From: Ruinaldn ChuanTzu Tsai 
>
> For now we add a custom CSR handling mechanism to handle non-standard CSR read
> or write.
>
> The write_stub() and read_zero() are provided for quick placeholder usage if
> such CSRs' behavior are expected to fail-over in its user code.
>
> For demonstration, we modify Andes Technology AX25 to showcase how the such
> mechanism works and how the UITB and MMSC_CFG CSR could be emulated.

Thanks for the patch!

This patch probably needs to be split into at least 2 different
patches. One adding the implementation and a second one adding the
Andes custom CSRs.

You also should run checkpatch on all patches:
https://wiki.qemu.org/Contribute/SubmitAPatch#Use_the_QEMU_coding_style

You are missing a SoB line here.

> ---
>  target/riscv/andes_cpu_bits.h  | 113 
>  target/riscv/cpu.c |  29 +++
>  target/riscv/cpu.h |  20 -
>  target/riscv/cpu_bits.h|   3 +
>  target/riscv/csr.c |  46 --
>  target/riscv/csr_andes.inc.c   | 153 +
>  target/riscv/custom_cpu_bits.h |   3 +
>  7 files changed, 358 insertions(+), 9 deletions(-)
>  create mode 100644 target/riscv/andes_cpu_bits.h
>  create mode 100644 target/riscv/csr_andes.inc.c
>  create mode 100644 target/riscv/custom_cpu_bits.h
>
> diff --git a/target/riscv/andes_cpu_bits.h b/target/riscv/andes_cpu_bits.h
> new file mode 100644
> index 00..8dee58a2c2
> --- /dev/null
> +++ b/target/riscv/andes_cpu_bits.h
> @@ -0,0 +1,113 @@
> +/* = AndeStar V5 machine mode CSRs = */
> +/* Configuration Registers */
> +#define CSR_MICM_CFG0xfc0
> +#define CSR_MDCM_CFG0xfc1
> +#define CSR_MMSC_CFG0xfc2
> +#define CSR_MMSC_CFG2   0xfc3
> +#define CSR_MVEC_CFG0xfc7
> +
> +/* Crash Debug CSRs */
> +#define CSR_MCRASH_STATESAVE0xfc8
> +#define CSR_MSTATUS_CRASHSAVE   0xfc9
> +
> +/* Memory CSRs */
> +#define CSR_MILMB   0x7c0
> +#define CSR_MDLMB   0x7c1
> +#define CSR_MECC_CODE   0x7C2
> +#define CSR_MNVEC   0x7c3
> +#define CSR_MCACHE_CTL  0x7ca
> +#define CSR_MCCTLBEGINADDR  0x7cb
> +#define CSR_MCCTLCOMMAND0x7cc
> +#define CSR_MCCTLDATA   0x7cd
> +#define CSR_MPPIB   0x7f0
> +#define CSR_MFIOB   0x7f1
> +
> +/* Hardware Stack Protection & Recording */
> +#define CSR_MHSP_CTL0x7c6
> +#define CSR_MSP_BOUND   0x7c7
> +#define CSR_MSP_BASE0x7c8
> +#define CSR_MXSTATUS0x7c4
> +#define CSR_MDCAUSE 0x7c9
> +#define CSR_MSLIDELEG   0x7d5
> +#define CSR_MSAVESTATUS 0x7d6
> +#define CSR_MSAVEEPC1   0x7d7
> +#define CSR_MSAVECAUSE1 0x7d8
> +#define CSR_MSAVEEPC2   0x7d9
> +#define CSR_MSAVECAUSE2 0x7da
> +#define CSR_MSAVEDCAUSE10x7db
> +#define CSR_MSAVEDCAUSE20x7dc
> +
> +/* Control CSRs */
> +#define CSR_MPFT_CTL0x7c5
> +#define CSR_MMISC_CTL   0x7d0
> +#define CSR_MCLK_CTL0x7df
> +
> +/* Counter related CSRs */
> +#define CSR_MCOUNTERWEN 0x7ce
> +#define CSR_MCOUNTERINTEN   0x7cf
> +#define CSR_MCOUNTERMASK_M  0x7d1
> +#define CSR_MCOUNTERMASK_S  0x7d2
> +#define CSR_MCOUNTERMASK_U  0x7d3
> +#define CSR_MCOUNTEROVF 0x7d4
> +
> +/* Enhanced CLIC CSRs */
> +#define CSR_MIRQ_ENTRY  0x7ec
> +#define CSR_MINTSEL_JAL 0x7ed
> +#define CSR_PUSHMCAUSE  0x7ee
> +#define CSR_PUSHMEPC0x7ef
> +#define CSR_PUSHMXSTATUS0x7eb
> +
> +/* Andes Physical Memory Attribute(PMA) CSRs */
> +#define CSR_PMACFG0 0xbc0
> +#define CSR_PMACFG1 0xbc1
> +#define CSR_PMACFG2 0xbc2
> +#define CSR_PMACFG3 0xbc3
> +#define CSR_PMAADDR00xbd0
> +#define CSR_PMAADDR10xbd1
> +#define CSR_PMAADDR20xbd2
> +#define CSR_PMAADDR30xbd2
> +#define CSR_PMAADDR40xbd4
> +#define CSR_PMAADDR50xbd5
> +#define CSR_PMAADDR60xbd6
> +#define CSR_PMAADDR70xbd7
> +#define CSR_PMAADDR80xbd8
> +#define CSR_PMAADDR90xbd9
> +#define CSR_PMAADDR10   0xbda
> +#define CSR_PMAADDR11   0xbdb
> +#define CSR_PMAADDR12   0xbdc
> +#define CSR_PMAADDR13   0xbdd
> +#define CSR_PMAADDR14   0xbde
> +#define CSR_PMAADDR15   0xbdf
> +
> +/* = AndeStar V5 supervisor mode CSRs = */
> +/* Supervisor trap registers */
> +#define CSR_SLIE0x9c4
> +#define CSR_SLIP0x9c5
> +#define CSR_SDCAUSE 0x9c9
> +
> +/* Supervisor counter registers */
> +#define CSR_SCOUNTERINTEN   0x9cf
> +#define CSR_SCOUNTERMASK_M  0x9d1
> +#define CSR_SCOUNTERMASK_S  0x9d2
> +#define CSR_SCOUNTERMASK_U

Re: [RFC PATCH v3 1/2] Adding Andes AX25 CPU model

[Alistair Francis]

On Fri, Jun 11, 2021 at 12:44 AM Ruinland Chuan-Tzu Tsai
 wrote:
>
> From: Ruinaldn ChuanTzu Tsai 
>
> Adding the skeleton of Andes Technology AX25 CPU model for the future commits,
> which will utilize custom/vendor CSR handling mechaism.

You are missing a SoB line.

See https://wiki.qemu.org/Contribute/SubmitAPatch for more details.

Alistair

> ---
>  target/riscv/cpu.c | 8 
>  target/riscv/cpu.h | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddea8fbeeb..4ae21cbf9b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -159,6 +159,13 @@ static void rv64_base_cpu_init(Object *obj)
>  set_misa(env, RV64);
>  }
>
> +static void ax25_cpu_init(Object *obj)
> +{
> +CPURISCVState *env = &RISCV_CPU(obj)->env;
> +set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> +set_priv_version(env, PRIV_VERSION_1_10_0);
> +}
> +
>  static void rv64_sifive_u_cpu_init(Object *obj)
>  {
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -705,6 +712,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,   rv32_sifive_u_cpu_init),
>  #elif defined(TARGET_RISCV64)
>  DEFINE_CPU(TYPE_RISCV_CPU_BASE64,   rv64_base_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,   rv64_sifive_e_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   rv64_sifive_u_cpu_init),
>  #endif
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0edb2826a2..bff9af7f3f 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -37,6 +37,7 @@
>  #define TYPE_RISCV_CPU_ANY  RISCV_CPU_TYPE_NAME("any")
>  #define TYPE_RISCV_CPU_BASE32   RISCV_CPU_TYPE_NAME("rv32")
>  #define TYPE_RISCV_CPU_BASE64   RISCV_CPU_TYPE_NAME("rv64")
> +#define TYPE_RISCV_CPU_AX25 RISCV_CPU_TYPE_NAME("andes-ax25")
>  #define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
>  #define TYPE_RISCV_CPU_SIFIVE_E31   RISCV_CPU_TYPE_NAME("sifive-e31")
>  #define TYPE_RISCV_CPU_SIFIVE_E34   RISCV_CPU_TYPE_NAME("sifive-e34")
> --
> 2.31.1
>

Re: [PATCH v9 2/6] [RISCV_PM] Support CSRs required for RISC-V PM extension except for the h-mode

[Alistair Francis]

On Thu, May 27, 2021 at 4:23 AM Alexey Baturo  wrote:
>
> Signed-off-by: Alexey Baturo 

Hey,

Thanks for the patch. Sorry it takes me so long to get to. It would
help if you could split this patch up just a little bit more.

Overall it looks good. Thanks for updating it to use the new CSR mechanism.

> ---
>  target/riscv/cpu.c  |   6 +
>  target/riscv/cpu.h  |  12 ++
>  target/riscv/cpu_bits.h |  97 +
>  target/riscv/csr.c  | 306 
>  4 files changed, 421 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 3191fd0082..9841711e71 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -486,6 +486,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>  if (cpu->cfg.ext_h) {
>  target_misa |= RVH;
>  }
> +if (cpu->cfg.ext_j) {
> +#ifndef CONFIG_USER_ONLY
> +/* mmte is supposed to have pm.current hardwired to 1 */
> +env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT);
> +#endif
> +}
>  if (cpu->cfg.ext_v) {
>  target_misa |= RVV;
>  if (!is_power_of_2(cpu->cfg.vlen)) {
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 1673872223..a68e523d87 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -233,6 +233,18 @@ struct CPURISCVState {
>
>  /* True if in debugger mode.  */
>  bool debugger;
> +
> +/*
> + * CSRs for PointerMasking extension
> + * TODO: move these csr to appropriate groups

I don't think we need this TODO here.

> + */
> +target_ulong mmte;
> +target_ulong mpmmask;
> +target_ulong mpmbase;
> +target_ulong spmmask;
> +target_ulong spmbase;
> +target_ulong upmmask;
> +target_ulong upmbase;
>  #endif
>
>  float_status fp_status;
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 52640e6856..97e2aa8a97 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -334,6 +334,39 @@
>  #define CSR_MHPMCOUNTER30H  0xb9e
>  #define CSR_MHPMCOUNTER31H  0xb9f
>
> +
> +/*
> + * User PointerMasking registers
> + * NB: actual CSR numbers might be changed in future
> + */
> +#define CSR_UMTE0x4c0
> +#define CSR_UPMMASK 0x4c1
> +#define CSR_UPMBASE 0x4c2
> +
> +/*
> + * Machine PointerMasking registers
> + * NB: actual CSR numbers might be changed in future
> + */
> +#define CSR_MMTE0x3c0
> +#define CSR_MPMMASK 0x3c1
> +#define CSR_MPMBASE 0x3c2
> +
> +/*
> + * Supervisor PointerMaster registers
> + * NB: actual CSR numbers might be changed in future
> + */
> +#define CSR_SMTE0x1c0
> +#define CSR_SPMMASK 0x1c1
> +#define CSR_SPMBASE 0x1c2
> +
> +/*
> + * Hypervisor PointerMaster registers
> + * NB: actual CSR numbers might be changed in future
> + */
> +#define CSR_VSMTE   0x2c0
> +#define CSR_VSPMMASK0x2c1
> +#define CSR_VSPMBASE0x2c2
> +

I do expect these to change. Have they been submitted yet to determine
the final addresses?

>  /* mstatus CSR bits */
>  #define MSTATUS_UIE 0x0001
>  #define MSTATUS_SIE 0x0002
> @@ -530,4 +563,68 @@ typedef enum RISCVException {
>  #define MIE_UTIE   (1 << IRQ_U_TIMER)
>  #define MIE_SSIE   (1 << IRQ_S_SOFT)
>  #define MIE_USIE   (1 << IRQ_U_SOFT)
> +
> +/* General PointerMasking CSR bits*/
> +#define PM_ENABLE   0x0001ULL
> +#define PM_CURRENT  0x0002ULL
> +#define PM_INSN 0x0004ULL
> +#define PM_XS_MASK  0x0003ULL
> +
> +/* PointerMasking XS bits values */
> +#define PM_EXT_DISABLE  0xULL
> +#define PM_EXT_INITIAL  0x0001ULL
> +#define PM_EXT_CLEAN0x0002ULL
> +#define PM_EXT_DIRTY0x0003ULL
> +
> +/* Offsets for every pair of control bits per each priv level */
> +#define XS_OFFSET0ULL
> +#define U_OFFSET 2ULL
> +#define S_OFFSET 5ULL
> +#define M_OFFSET 8ULL
> +
> +#define PM_XS_BITS   (PM_XS_MASK << XS_OFFSET)
> +#define U_PM_ENABLE  (PM_ENABLE  << U_OFFSET)
> +#define U_PM_CURRENT (PM_CURRENT << U_OFFSET)
> +#define U_PM_INSN(PM_INSN<< U_OFFSET)
> +#define S_PM_ENABLE  (PM_ENABLE  << S_OFFSET)
> +#define S_PM_CURRENT (PM_CURRENT << S_OFFSET)
> +#define S_PM_INSN(PM_INSN<< S_OFFSET)
> +#define M_PM_ENABLE  (PM_ENABLE  << M_OFFSET)
> +#define M_PM_CURRENT (PM_CURRENT << M_OFFSET)
> +#define M_PM_INSN(PM_INSN<< M_OFFSET)
> +
> +/* mmte CSR bits */
> +#define MMTE_PM_XS_BITS PM_XS_BITS
> +#define MMTE_U_PM_ENABLEU_PM_ENABLE
> +#define MMTE_U_PM_CURRENT   U_PM_CURRENT
> +#define MMTE_U_PM_INSN  U_PM_INSN
> +#define MMTE_S_PM_ENABLES_PM_ENABLE
> +#define MMTE_S_PM_CURRENT   S_PM_CURRENT
> +#define MMTE_S_PM_INSN  S_PM_INSN
> +#define MMTE_M_PM_ENABLEM_PM_ENABLE
> +#define MMTE_M_PM_CURRENT   M_PM_CURRENT
> +#define MMTE

[PATCH] tcg/arm: Fix tcg_out_op function signature

[Jose R. Ziviani]

Commit 5e8892db93 fixed several function signatures but tcg_out_op for
arm is missing. This patch fixes it as well.

Signed-off-by: Jose R. Ziviani 
---
 tcg/arm/tcg-target.c.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tcg/arm/tcg-target.c.inc b/tcg/arm/tcg-target.c.inc
index f4c9cb8f9f..5157143246 100644
--- a/tcg/arm/tcg-target.c.inc
+++ b/tcg/arm/tcg-target.c.inc
@@ -1984,7 +1984,8 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
*args, bool is64)
 static void tcg_out_epilogue(TCGContext *s);
 
 static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
-const TCGArg *args, const int *const_args)
+const TCGArg args[TCG_MAX_OP_ARGS],
+const int const_args[TCG_MAX_OP_ARGS])
 {
 TCGArg a0, a1, a2, a3, a4, a5;
 int c;
-- 
2.31.1

Re: [PATCH 1/4] target/riscv: Add defines for AIA local interrupt CSRs

[Alistair Francis]

On Sat, May 15, 2021 at 12:33 AM Anup Patel  wrote:
>
> The RISC-V AIA specification extends RISC-V local interrupts and
> introduces new CSRs. This patch adds defines for the new AIA
> local interrupt CSRs.
>
> Signed-off-by: Anup Patel 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu_bits.h | 128 
>  1 file changed, 128 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index caf4599207..d23242655e 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -169,6 +169,31 @@
>  /* Legacy Machine Trap Handling (priv v1.9.1) */
>  #define CSR_MBADADDR0x343
>
> +/* Machine-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_MISELECT0x350
> +#define CSR_MIREG   0x351
> +
> +/* Machine-Level Interrupts (AIA) */
> +#define CSR_MTOPI   0xfb0
> +
> +/* Machine-Level IMSIC Interface (AIA) */
> +#define CSR_MSETEIPNUM  0x358
> +#define CSR_MCLREIPNUM  0x359
> +#define CSR_MSETEIENUM  0x35a
> +#define CSR_MCLREIENUM  0x35b
> +#define CSR_MCLAIMEI0xfa8
> +
> +/* Virtual Interrupts for Supervisor Level (AIA) */
> +#define CSR_MVIEN   0x308
> +#define CSR_MVIP0x309
> +
> +/* Machine-Level High-Half CSRs (AIA) */
> +#define CSR_MIDELEGH0x313
> +#define CSR_MIEH0x314
> +#define CSR_MVIENH  0x318
> +#define CSR_MVIPH   0x319
> +#define CSR_MIPH0x354
> +
>  /* Supervisor Trap Setup */
>  #define CSR_SSTATUS 0x100
>  #define CSR_SEDELEG 0x102
> @@ -191,6 +216,24 @@
>  #define CSR_SPTBR   0x180
>  #define CSR_SATP0x180
>
> +/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_SISELECT0x150
> +#define CSR_SIREG   0x151
> +
> +/* Supervisor-Level Interrupts (AIA) */
> +#define CSR_STOPI   0xdb0
> +
> +/* Supervisor-Level IMSIC Interface (AIA) */
> +#define CSR_SSETEIPNUM  0x158
> +#define CSR_SCLREIPNUM  0x159
> +#define CSR_SSETEIENUM  0x15a
> +#define CSR_SCLREIENUM  0x15b
> +#define CSR_SCLAIMEI0xda8
> +
> +/* Supervisor-Level High-Half CSRs (AIA) */
> +#define CSR_SIEH0x114
> +#define CSR_SIPH0x154
> +
>  /* Hpervisor CSRs */
>  #define CSR_HSTATUS 0x600
>  #define CSR_HEDELEG 0x602
> @@ -232,6 +275,34 @@
>  #define CSR_MTINST  0x34a
>  #define CSR_MTVAL2  0x34b
>
> +/* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */
> +#define CSR_HVIEN   0x608
> +#define CSR_HVICONTROL  0x609
> +#define CSR_HVIPRIO10x646
> +#define CSR_HVIPRIO20x647
> +
> +/* VS-Level Window to Indirectly Accessed Registers (H-extension with AIA) */
> +#define CSR_VSISELECT   0x250
> +#define CSR_VSIREG  0x251
> +
> +/* VS-Level Interrupts (H-extension with AIA) */
> +#define CSR_VSTOPI  0xeb0
> +
> +/* VS-Level IMSIC Interface (H-extension with AIA) */
> +#define CSR_VSSETEIPNUM 0x258
> +#define CSR_VSCLREIPNUM 0x259
> +#define CSR_VSSETEIENUM 0x25a
> +#define CSR_VSCLREIENUM 0x25b
> +
> +/* Hypervisor and VS-Level High-Half CSRs (H-extension with AIA) */
> +#define CSR_HIDELEGH0x613
> +#define CSR_HVIENH  0x618
> +#define CSR_HVIPH   0x655
> +#define CSR_HVIPRIO1H   0x656
> +#define CSR_HVIPRIO2H   0x657
> +#define CSR_VSIEH   0x214
> +#define CSR_VSIPH   0x254
> +
>  /* Physical Memory Protection */
>  #define CSR_PMPCFG0 0x3a0
>  #define CSR_PMPCFG1 0x3a1
> @@ -436,6 +507,7 @@
>  #define HSTATUS_SPVP 0x0100
>  #define HSTATUS_HU   0x0200
>  #define HSTATUS_VGEIN0x0003F000
> +#define HSTATUS_VGEIN_SHIFT  12
>  #define HSTATUS_VTVM 0x0010
>  #define HSTATUS_VTSR 0x0040
>  #define HSTATUS_VSXL 0x3
> @@ -565,6 +637,7 @@
>  #define IRQ_S_EXT  9
>  #define IRQ_VS_EXT 10
>  #define IRQ_M_EXT  11
> +#define IRQ_S_GEXT 12
>
>  /* mip masks */
>  #define MIP_USIP   (1 << IRQ_U_SOFT)
> @@ -592,4 +665,59 @@
>  #define MIE_UTIE   (1 << IRQ_U_TIMER)
>  #define MIE_SSIE   (1 << IRQ_S_SOFT)
>  #define MIE_USIE   (1 << IRQ_U_SOFT)
> +
> +/* MISELECT, SISELECT, and VSISELECT bits (AIA) */
> +#define ISELECT_IPRIO0 0x30
> +#define ISELECT_IPRIO150x3f
> +#define ISELECT_IMSIC_EIDELIVERY   0x70
> +#define ISELECT_IMSIC_EITHRESHOLD  0x72
> +#define ISELECT_IMSIC_TOPEI0x76
> +#define ISELECT_IMSIC_EIP0 0x80
> +#define ISELECT_IMSIC_EIP630xbf
> +#define ISELECT_IMSIC_EIE0 0xc0
> +#define ISELECT_IMSIC_EIE630xff
> +#defi

Re: [RFC PATCH v2 2/2] cputlb: implement load_helper_unaligned() for unaligned loads

[Mark Cave-Ayland]

On 10/06/2021 22:41, Richard Henderson wrote:


On 6/9/21 7:10 AM, Philippe Mathieu-Daudé wrote:

+    oi = make_memop_idx(MO_UB, mmu_idx);
+    if (memop_big_endian(op)) {
+    for (i = 0; i < size; ++i) {
+    /* Big-endian load.  */
+    uint8_t val8 = helper_ret_ldub_mmu(env, addr + i, oi, retaddr);
+    val |= val8 << (((size - 1) * 8) - (i * 8));
+    }
+    } else {
+    for (i = 0; i < size; ++i) {
+    /* Little-endian load.  */
+    uint8_t val8 = helper_ret_ldub_mmu(env, addr + i, oi, retaddr);
+    val |= val8 << (i * 8);
+    }
+    }


This doesn't quite work.  You can't just call helper_ret_ldub_mmu, as the other 
option is full_ldub_code.  So, at present you've broken unaligned code loads.


We also need noinline markup for clang, like we do for helper_ret_stb_mmu. I've no 
proof of that, but it certainly makes sense to record how we expect the inline loop 
to be resolved.


Finally, you have to use uint64_t for val8, otherwise the shift fails for size 
== 8.

I'll fix these up and see how things go.


Ah that makes sense - I wasn't quite sure whether the previous full_load() would 
already be handled correctly by the code_read logic in load_helper_unaligned().


Sleeping on the problem, I do wonder if the role of load_helper() and store_helper() 
should be just to ensure that if an access crosses a page then both entries are in 
the softtlb before passing MMIO accesses down through the memory API with Andrew 
Jeffrey's proposed unaligned access patch (see 
https://gitlab.com/qemu-project/qemu/-/issues/360#note_597130838 for a summary of the 
IRC discussion).


However as I suspect this may cause some breakage then I would still be fine with 
load_helper_unaligned() in the meantime.



ATB,

Mark.

Re: [RFC PATCH v2 2/2] cputlb: implement load_helper_unaligned() for unaligned loads

[Richard Henderson]

On 6/9/21 7:10 AM, Philippe Mathieu-Daudé wrote:

+oi = make_memop_idx(MO_UB, mmu_idx);
+if (memop_big_endian(op)) {
+for (i = 0; i < size; ++i) {
+/* Big-endian load.  */
+uint8_t val8 = helper_ret_ldub_mmu(env, addr + i, oi, retaddr);
+val |= val8 << (((size - 1) * 8) - (i * 8));
+}
+} else {
+for (i = 0; i < size; ++i) {
+/* Little-endian load.  */
+uint8_t val8 = helper_ret_ldub_mmu(env, addr + i, oi, retaddr);
+val |= val8 << (i * 8);
+}
+}


This doesn't quite work.  You can't just call helper_ret_ldub_mmu, as the other 
option is full_ldub_code.  So, at present you've broken unaligned code loads.


We also need noinline markup for clang, like we do for helper_ret_stb_mmu. 
I've no proof of that, but it certainly makes sense to record how we expect the 
inline loop to be resolved.


Finally, you have to use uint64_t for val8, otherwise the shift fails for size 
== 8.


I'll fix these up and see how things go.


r~

[PATCH] qemu-img: Use "depth":-1 to make backing probes obvious

[Eric Blake]

The recently-added NBD context qemu:allocation-depth makes an obvious
case for why it is important to distinguish between locally-present
data (even with that data is sparse) [shown as depth 1 over NBD], and
data that could not be found anywhere in the backing chain [shown as
depth 0].  But qemu-img map --output=json predates that addition, and
has the unfortunate behavior that all portions of the backing chain
that resolve without finding a hit in any backing layer report the
same depth as the final backing layer.  This makes it harder to
reconstruct a qcow2 backing chain using just 'qemu-img map' output
when using "backing":null to artificially limit a backing chain,
because it is impossible to distinguish between a
QCOW2_CLUSTER_UNALLOCATED (which defers to a [missing] backing file)
and a QCOW2_CLUSTER_ZERO_PLAIN cluster (which would override any
backing file), since both types of clusters otherwise show as
"data":false,"zero":true" (but note that we can distinguish a
QCOW2_CLUSTER_ZERO_ALLOCATED, which would also have an "offset":
listing).

The task of reconstructing a qcow2 chain was made harder in commit
0da9856851 (nbd: server: Report holes for raw images), because prior
to that point, it was possible to abuse NBD's block status command to
see which portions of a qcow2 file resulted in BDRV_BLOCK_ALLOCATED
(showing up as NBD_STATE_ZERO in isolation) vs. missing from the chain
(showing up as NBD_STATE_ZERO|NBD_STATE_HOLE); but now qemu reports
more accurate sparseness information over NBD.

An obvious solution is to make 'qemu-img map --output=json' visually
distinguish between clusters that have a local allocation from those
that are found nowhere in the chain, by adding "depth":-1 as the new
witness of data that could not be tied to a specific backing image.
Several iotests are impacted, but glancing through the changes shows
that it is an improvement in that it shows more accurate details.

Note that the documentation is specifically worded to allow qemu-img
to report "depth":-1 both in the scenario where the last file in the
backing chain still defers the cluster (corresponding to
BDRV_BLOCK_ALLOCATED not being set anywhere in the chain), and in the
scenario where qemu is unable to determine which backing chain element
(if any) provides the data.  The latter case does not exist now, but
I'm considering an upcoming patch to add a BDRV_BLOCK_BACKING that
would let a specific driver (such as NBD) inform the block layer that
it is known that a cluster comes from a backing layer, but where there
is insufficient data to determine which layer.

As a quick demonstration:

# Create a qcow2 image with a raw backing file:
$ qemu-img create base.raw $((4*64*1024))
$ qemu-img create -f qcow2 -b base.raw -F raw top.qcow2

# Write to first 3 clusters of base:
$ qemu-io -f raw -c "w -P 65 0 64k" -c "w -P 66 64k 64k" \
  -c "w -P 67 128k 64k" base.raw

# Write to second and third clusters of top, hiding base:
$ qemu-io -f qcow2 -c "w -P 69 64k 64k" -c "w -z 128k 64k" top.qcow2

# Examine the full backing chain
$ qemu-img map --output=json -f qcow2 top.qcow2
[{ "start": 0, "length": 65536, "depth": 1, "zero": false, "data": true, 
"offset": 0},
{ "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
{ "start": 131072, "length": 65536, "depth": 0, "zero": true, "data": 
false},
{ "start": 196608, "length": 65536, "depth": 1, "zero": true, "data": 
false, "offset": 196608}]

# Repeat, but with the backing chain clamped. Pre-patch:
$ qemu-img map --output=json -f qcow2 json:'{"driver":"qcow2", \
  "backing":null, "file":{"driver":"file", "filename":"top.qcow2"}}'
[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false},
{ "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
{ "start": 131072, "length": 131072, "depth": 0, "zero": true, "data": 
false}]

# Repeat, but post-patch:
$ qemu-img map --output=json -f qcow2 json:'{"driver":"qcow2", \
  "backing":null, "file":{"driver":"file", "filename":"top.qcow2"}}'
[{ "start": 0, "length": 65536, "depth": -1, "zero": true, "data": false},
{ "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
{ "start": 131072, "length": 65536, "depth": 0, "zero": true, "data": 
false},
{ "start": 196608, "length": 65536, "depth": -1, "zero": true, "data": 
false}]

Note that pre-patch, it was impossible to determine which portions of
the qcow2 file override the backing file because the "depth":0 regions
were combined, so even though qemu internally can tell the difference
between sclusters 2 and 3, the command line user could not.  But
post-patch, the "depth":-1 markings match the "depth":1 markings when
the backing chain is intact, and it becomes obvious which clusters are
important.

Signed-off-by: Eric Blake 
---

If desired, I can send a fol

[PATCH RFC 1/1] linux-user/s390x: save/restore condition code state during signal handling

[Jonathan Albrecht]

When handling a signal, the signal handler may have clobbered the
condition code set by the interrupted thread.

Signed-off-by: Jonathan Albrecht 
Buglink: https://bugs.launchpad.net/qemu/+bug/1886793
Buglink: https://bugs.launchpad.net/qemu/+bug/1893040
---
 linux-user/s390x/signal.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index ef136dae33..03cacb2829 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -65,6 +65,10 @@ typedef struct {
 uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
 target_sigcontext sc;
 target_sigregs sregs;
+uint32_t scc_op;
+uint64_t scc_src;
+uint64_t scc_dst;
+uint64_t scc_vr;
 int signo;
 target_sigregs_ext sregs_ext;
 uint16_t retcode;
@@ -86,6 +90,10 @@ typedef struct {
 uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
 uint16_t retcode;
 struct target_siginfo info;
+uint32_t scc_op;
+uint64_t scc_src;
+uint64_t scc_dst;
+uint64_t scc_vr;
 struct target_ucontext uc;
 } rt_sigframe;
 
@@ -224,6 +232,12 @@ void setup_frame(int sig, struct target_sigaction *ka,
 env->regs[5] = 0; /* FIXME: regs->int_parm_long */
 env->regs[6] = 0; /* FIXME: current->thread.last_break */
 
+/* Some programs could clobber the condition code, so save it here */
+__put_user(env->cc_op, &frame->scc_op);
+__put_user(env->cc_src, &frame->scc_src);
+__put_user(env->cc_dst, &frame->scc_dst);
+__put_user(env->cc_vr, &frame->scc_vr);
+
 unlock_user_struct(frame, frame_addr, 1);
 }
 
@@ -273,6 +287,12 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 save_sigregs_ext(env, &frame->uc.tuc_mcontext_ext);
 tswap_sigset(&frame->uc.tuc_sigmask, set);
 
+/* Some programs could clobber the condition code, so save it here */
+__put_user(env->cc_op, &frame->scc_op);
+__put_user(env->cc_src, &frame->scc_src);
+__put_user(env->cc_dst, &frame->scc_dst);
+__put_user(env->cc_vr, &frame->scc_vr);
+
 /* Set up registers for signal handler */
 env->regs[14] = restorer;
 env->regs[15] = frame_addr;
@@ -285,6 +305,10 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 env->regs[3] = frame_addr + offsetof(typeof(*frame), info);
 env->regs[4] = frame_addr + offsetof(typeof(*frame), uc);
 env->regs[5] = 0; /* FIXME: current->thread.last_break */
+
+/* QUESTION: Was there a missing unlock user struct here? */
+unlock_user_struct(frame, frame_addr, 1);
+return;
 }
 
 static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
@@ -350,6 +374,12 @@ long do_sigreturn(CPUS390XState *env)
 restore_sigregs(env, &frame->sregs);
 restore_sigregs_ext(env, &frame->sregs_ext);
 
+/* restore the condition code */
+__get_user(env->cc_op, &frame->scc_op);
+__get_user(env->cc_src, &frame->scc_src);
+__get_user(env->cc_dst, &frame->scc_dst);
+__get_user(env->cc_vr, &frame->scc_vr);
+
 unlock_user_struct(frame, frame_addr, 0);
 return -TARGET_QEMU_ESIGRETURN;
 }
@@ -372,6 +402,11 @@ long do_rt_sigreturn(CPUS390XState *env)
 restore_sigregs(env, &frame->uc.tuc_mcontext);
 restore_sigregs_ext(env, &frame->uc.tuc_mcontext_ext);
 
+/* restore the condition code */
+__get_user(env->cc_op, &frame->scc_op);
+__get_user(env->cc_src, &frame->scc_src);
+__get_user(env->cc_dst, &frame->scc_dst);
+__get_user(env->cc_vr, &frame->scc_vr);
 target_restore_altstack(&frame->uc.tuc_stack, env);
 
 unlock_user_struct(frame, frame_addr, 0);
-- 
2.31.1

[PATCH RFC 0/1] linux-user/s390x: save/restore condition code state during signal handling

[Jonathan Albrecht]

Peter Bao  and I have been looking at some issues with
qemu user mode x86_64 host/s390x guest when running go1.14+ executables.
>From the qemu cpu traces, it looks like the condition code is not restored
after a signal handler is run. This affects go1.14+ because it uses signals
heavily to implement the async preempt feature in the goroutine scheduler
that was added in go1.14.

This patch tries save and restore the condition code related fields when
handling a signal. We're submitting it as an RFC since we're new to qemu and
not sure if this is s390x specific. We have some C code (s390x specific)
that reproduces the issue and can try to add it as a unit test.

Jonathan Albrecht (1):
  linux-user/s390x: save/restore condition code state during signal
handling

 linux-user/s390x/signal.c | 35 +++
 1 file changed, 35 insertions(+)

-- 
2.31.1

Re: iotest 233 failing

[Daniel P . Berrangé]

On Thu, Jun 10, 2021 at 10:31:14PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 10, 2021 at 03:34:46PM -0500, Eric Blake wrote:
> > I'm now getting failures on iotest 233:
> > 
> > 233   fail   [15:26:01] [15:26:03]   2.1s   (last: 1.3s)  output 
> > mismatch (see 233.out.bad)
> > --- /home/eblake/qemu/tests/qemu-iotests/233.out
> > +++ 233.out.bad
> > @@ -65,6 +65,6 @@
> >  == final server log ==
> >  qemu-nbd: option negotiation failed: Verify failed: No certificate was 
> > found.
> >  qemu-nbd: option negotiation failed: Verify failed: No certificate was 
> > found.
> > -qemu-nbd: option negotiation failed: TLS x509 authz check for 
> > CN=localhost,O=Cthulhu Dark Lord Enterprises client1,L=R'lyeh,C=South 
> > Pacific is denied
> > -qemu-nbd: option negotiation failed: TLS x509 authz check for 
> > CN=localhost,O=Cthulhu Dark Lord Enterprises client3,L=R'lyeh,C=South 
> > Pacific is denied
> > +qemu-nbd: option negotiation failed: TLS x509 authz check for C=South 
> > Pacific,L=R'lyeh,O=Cthulhu Dark Lord Enterprises client1,CN=localhost is 
> > denied
> > +qemu-nbd: option negotiation failed: TLS x509 authz check for C=South 
> > Pacific,L=R'lyeh,O=Cthulhu Dark Lord Enterprises client3,CN=localhost is 
> > denied
> >  *** done
> > Failures: 233
> > Failed 1 of 1 iotests
> > 
> > Looks like I recently updated to gnutls-3.7.2-1.fc34 on June 1, could
> > that be the culprit for the error message being reordered?
> 
> It is possible I guess. They have indeed made such a change in the past
> and reverted it when I pointed out that this is effectively an ABI for
> apps, because access control lists are based on matching the distinguish
> name string, as an opaque string. The cause certainly needs investigating
> as a matter of urgency because this is ABI for QEMU's authz access control
> lists.

There is an ominous sounding NEWS item in 3.7.2

** certtool: When producing certificates and certificate requests, subject DN
   components that are provided individually will now be ordered by
   assumed scale (e.g. Country before State, Organization before
   OrganizationalUnit).  This change also affects the order in which
   certtool prompts interactively.  Please rely on the template
   mechanism for automated use of certtool! (#1243)

This ordering change in certtool seems to correspond with the new order
you see above in the distinguished name, so I wonder if the certtool
change had accidental side effects.


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

Re: iotest 233 failing

[Daniel P . Berrangé]

On Thu, Jun 10, 2021 at 03:34:46PM -0500, Eric Blake wrote:
> I'm now getting failures on iotest 233:
> 
> 233   fail   [15:26:01] [15:26:03]   2.1s   (last: 1.3s)  output mismatch 
> (see 233.out.bad)
> --- /home/eblake/qemu/tests/qemu-iotests/233.out
> +++ 233.out.bad
> @@ -65,6 +65,6 @@
>  == final server log ==
>  qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
>  qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
> -qemu-nbd: option negotiation failed: TLS x509 authz check for 
> CN=localhost,O=Cthulhu Dark Lord Enterprises client1,L=R'lyeh,C=South Pacific 
> is denied
> -qemu-nbd: option negotiation failed: TLS x509 authz check for 
> CN=localhost,O=Cthulhu Dark Lord Enterprises client3,L=R'lyeh,C=South Pacific 
> is denied
> +qemu-nbd: option negotiation failed: TLS x509 authz check for C=South 
> Pacific,L=R'lyeh,O=Cthulhu Dark Lord Enterprises client1,CN=localhost is 
> denied
> +qemu-nbd: option negotiation failed: TLS x509 authz check for C=South 
> Pacific,L=R'lyeh,O=Cthulhu Dark Lord Enterprises client3,CN=localhost is 
> denied
>  *** done
> Failures: 233
> Failed 1 of 1 iotests
> 
> Looks like I recently updated to gnutls-3.7.2-1.fc34 on June 1, could
> that be the culprit for the error message being reordered?

It is possible I guess. They have indeed made such a change in the past
and reverted it when I pointed out that this is effectively an ABI for
apps, because access control lists are based on matching the distinguish
name string, as an opaque string. The cause certainly needs investigating
as a matter of urgency because this is ABI for QEMU's authz access control
lists.


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

Re: [PATCH V2 1/2] vhost-vdpa: skip ram device from the IOTLB mapping

[Si-Wei Liu]

On 6/2/2021 1:41 AM, Jason Wang wrote:

vDPA is not tie to any specific hardware, for safety and simplicity,
vhost-vDPA doesn't allow MMIO area to be mapped via IOTLB. Only the
doorbell could be mapped via mmap(). So this patch exclude skip the
ram device from the IOTLB mapping.

Signed-off-by: Jason Wang 

Reviewed-by: Si-Wei Liu 

---
  hw/virtio/vhost-vdpa.c | 12 ++--
  1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 01d2101d09..dd4321bac2 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -27,6 +27,8 @@ static bool 
vhost_vdpa_listener_skipped_section(MemoryRegionSection *section)
  {
  return (!memory_region_is_ram(section->mr) &&
  !memory_region_is_iommu(section->mr)) ||
+   /* vhost-vDPA doesn't allow MMIO to be mapped  */
+memory_region_is_ram_device(section->mr) ||
 /*
  * Sizing an enabled 64-bit BAR can cause spurious mappings to
  * addresses in the upper part of the 64-bit address space.  These
@@ -171,22 +173,12 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
   vaddr, section->readonly);
  if (ret) {
  error_report("vhost vdpa map fail!");
-if (memory_region_is_ram_device(section->mr)) {
-/* Allow unexpected mappings not to be fatal for RAM devices */
-error_report("map ram fail!");
-  return ;
-}
  goto fail;
  }
  
  return;
  
  fail:

-if (memory_region_is_ram_device(section->mr)) {
-error_report("failed to vdpa_dma_map. pci p2p may not work");
-return;
-
-}
  /*
   * On the initfn path, store the first error in the container so we
   * can gracefully fail.  Runtime, there's not much we can do other

Re: [PATCH V2 2/2] vhost-vdpa: map virtqueue notification area if possible

[Si-Wei Liu]

Looks good.

On 6/2/2021 1:41 AM, Jason Wang wrote:

This patch implements the vq notification mapping support for
vhost-vDPA. This is simply done by using mmap()/munmap() for the
vhost-vDPA fd during device start/stop. For the device without
notification mapping support, we fall back to eventfd based
notification gracefully.

Signed-off-by: Jason Wang 

Reviewed-by: Si-Wei Liu 


---
Changes since v1:
- use dev->vq_index to calculate the virtqueue index
- remove the unused host_notifier_set
---
  hw/virtio/vhost-vdpa.c | 85 ++
  include/hw/virtio/vhost-vdpa.h |  6 +++
  2 files changed, 91 insertions(+)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index dd4321bac2..f9a86afe64 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -285,12 +285,95 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void 
*opaque)
  return 0;
  }
  
+static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,

+int queue_index)
+{
+size_t page_size = qemu_real_host_page_size;
+struct vhost_vdpa *v = dev->opaque;
+VirtIODevice *vdev = dev->vdev;
+VhostVDPAHostNotifier *n;
+
+n = &v->notifier[queue_index];
+
+if (n->addr) {
+virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, false);
+object_unparent(OBJECT(&n->mr));
+munmap(n->addr, page_size);
+n->addr = NULL;
+}
+}
+
+static void vhost_vdpa_host_notifiers_uninit(struct vhost_dev *dev, int n)
+{
+int i;
+
+for (i = 0; i < n; i++) {
+vhost_vdpa_host_notifier_uninit(dev, i);
+}
+}
+
+static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int 
queue_index)
+{
+size_t page_size = qemu_real_host_page_size;
+struct vhost_vdpa *v = dev->opaque;
+VirtIODevice *vdev = dev->vdev;
+VhostVDPAHostNotifier *n;
+int fd = v->device_fd;
+void *addr;
+char *name;
+
+vhost_vdpa_host_notifier_uninit(dev, queue_index);
+
+n = &v->notifier[queue_index];
+
+addr = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED, fd,
+queue_index * page_size);
+if (addr == MAP_FAILED) {
+goto err;
+}
+
+name = g_strdup_printf("vhost-vdpa/host-notifier@%p mmaps[%d]",
+   v, queue_index);
+memory_region_init_ram_device_ptr(&n->mr, OBJECT(vdev), name,
+  page_size, addr);
+g_free(name);
+
+if (virtio_queue_set_host_notifier_mr(vdev, queue_index, &n->mr, true)) {
+munmap(addr, page_size);
+goto err;
+}
+n->addr = addr;
+
+return 0;
+
+err:
+return -1;
+}
+
+static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
+{
+int i;
+
+for (i = dev->vq_index; i < dev->vq_index + dev->nvqs; i++) {
+if (vhost_vdpa_host_notifier_init(dev, i)) {
+goto err;
+}
+}
+
+return;
+
+err:
+vhost_vdpa_host_notifiers_uninit(dev, i);
+return;
+}
+
  static int vhost_vdpa_cleanup(struct vhost_dev *dev)
  {
  struct vhost_vdpa *v;
  assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
  v = dev->opaque;
  trace_vhost_vdpa_cleanup(dev, v);
+vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
  memory_listener_unregister(&v->listener);
  
  dev->opaque = NULL;

@@ -467,6 +550,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool 
started)
  if (started) {
  uint8_t status = 0;
  memory_listener_register(&v->listener, &address_space_memory);
+vhost_vdpa_host_notifiers_init(dev);
  vhost_vdpa_set_vring_ready(dev);
  vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
  vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
@@ -476,6 +560,7 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool 
started)
  vhost_vdpa_reset_device(dev);
  vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
 VIRTIO_CONFIG_S_DRIVER);
+vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
  memory_listener_unregister(&v->listener);
  
  return 0;

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 9b81a409da..56bef30ec2 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -14,11 +14,17 @@
  
  #include "hw/virtio/virtio.h"
  
+typedef struct VhostVDPAHostNotifier {

+MemoryRegion mr;
+void *addr;
+} VhostVDPAHostNotifier;
+
  typedef struct vhost_vdpa {
  int device_fd;
  uint32_t msg_type;
  MemoryListener listener;
  struct vhost_dev *dev;
+VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
  } VhostVDPA;
  
  extern AddressSpace address_space_memory;

Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole

[Eric Blake]

On Thu, Jun 10, 2021 at 11:09:05PM +0300, Nir Soffer wrote:
> > But:
> >
> > $ qemu-img map --output=json -f qcow2 
> > json:'{"driver":"qcow2","backing":null, \
> >   "file":{"driver":"file","filename":"top.qcow2"}}'
> > [{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false},
> > { "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
> > "offset": 327680},
> > { "start": 131072, "length": 131072, "depth": 0, "zero": true, "data": 
> > false}]
> >
> > also reports the entire file at "depth":0, which is misleading, since
> > we have just been arguing from the qemu:allocation-depth perspective
> > (and also from bdrv_block_status) that the qcow2 image is NOT 100%
> > allocated (in the sense where allocation == data comes locally).
> > Perhaps it might be better if we tweaked the above qemu-img map to
> > produce:
> >
> > [{ "start": 0, "length": 65536, "depth": -1, "zero": true, "data": false},
> > { "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
> > "offset": 327680},
> > { "start": 131072, "length": 65536, "depth": 0, "zero": true, "data": 
> > false},
> > { "start": 196608, "length": 65536, "depth": -1, "zero": true, "data": 
> > false}]
> 
> It will be more consistent with "offset" to drop "depth" from output
> if we don't have it:
> 
> [{ "start": 0, "length": 65536, "zero": true, "data": false},
>  { "start": 65536, "length": 65536, "depth": 0, "zero": false,
> "data": true, "offset": 327680},
>  { "start": 131072, "length": 65536, "depth": 0, "zero": true,
> "data": false},
>  { "start": 196608, "length": 65536, "zero": true, "data": false}]

Yes, that might work as well.  But we didn't previously document
depth to be optional.  Removing something from output risks breaking
more downstream tools that expect it to be non-optional, compared to
providing a new value.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/1] vfio-ccw: Fix garbage sense data on I/O error

[Eric Farman]

On Thu, 2021-06-10 at 16:25 -0400, Matthew Rosato wrote:
> On 6/10/21 4:20 PM, Eric Farman wrote:
> > Hi Conny,
> > 
> > Per our offline discussion, here's a fix for the error when a guest
> > issues "dasdfmt -M quick". It basically reverts commit 334e76850bbb
> > ("vfio/ccw: update sense data if a unit check is pending")
> > and modifies the check that builds sense data in the TSCH handler.
> 
> Should it includes a Fixes: tag then?

Yeah, probably. I'll fix it locally so it's prepped for a v2.

> 
> > I opted to NOT disable PMCW.CSENSE, because doing so prevents
> > vfio-ccw devices from coming online at all (didn't pursue deep
> > enough to explain why). Turning it off in reaction to a unit
> > check (in this now-reverted codepath) works, but only because of
> > the corresponding PMCW.CSENSE check in the TSCH code.
> > 
> > I don't know if anything is needed for the (unaltered) ECW data
> > that commit b498484ed49a ("s390x/css: sense data endianness")
> > addressed for the copied sense_data bytes, but figure we can
> > use this as a starting point. Thoughts?
> > 
> > Eric Farman (1):
> >vfio-ccw: Keep passthrough sense data intact
> > 
> >   hw/s390x/css.c | 3 ++-
> >   hw/vfio/ccw.c  | 6 --
> >   2 files changed, 2 insertions(+), 7 deletions(-)
> > 

iotest 233 failing

[Eric Blake]

I'm now getting failures on iotest 233:

233   fail   [15:26:01] [15:26:03]   2.1s   (last: 1.3s)  output mismatch 
(see 233.out.bad)
--- /home/eblake/qemu/tests/qemu-iotests/233.out
+++ 233.out.bad
@@ -65,6 +65,6 @@
 == final server log ==
 qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
 qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
-qemu-nbd: option negotiation failed: TLS x509 authz check for 
CN=localhost,O=Cthulhu Dark Lord Enterprises client1,L=R'lyeh,C=South Pacific 
is denied
-qemu-nbd: option negotiation failed: TLS x509 authz check for 
CN=localhost,O=Cthulhu Dark Lord Enterprises client3,L=R'lyeh,C=South Pacific 
is denied
+qemu-nbd: option negotiation failed: TLS x509 authz check for C=South 
Pacific,L=R'lyeh,O=Cthulhu Dark Lord Enterprises client1,CN=localhost is denied
+qemu-nbd: option negotiation failed: TLS x509 authz check for C=South 
Pacific,L=R'lyeh,O=Cthulhu Dark Lord Enterprises client3,CN=localhost is denied
 *** done
Failures: 233
Failed 1 of 1 iotests

Looks like I recently updated to gnutls-3.7.2-1.fc34 on June 1, could
that be the culprit for the error message being reordered?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Wainer dos Santos Moschetta]

Hi,

On 6/8/21 8:55 PM, Cleber Rosa Junior wrote:



On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta 
mailto:waine...@redhat.com>> wrote:


Hi,

On 6/8/21 11:09 AM, Cleber Rosa wrote:
> Which can be used to check for any "feature" that is available as a
> QEMU command line option, and that will return its list of available
> options.
>
> This is a generalization of the list_accel() utility function, which
> is itself re-implemented in terms of the more generic feature.
>
> Signed-off-by: Cleber Rosa mailto:cr...@redhat.com>>
> ---
>   python/qemu/utils/__init__.py |  2 ++
>   python/qemu/utils/accel.py    | 15 ++--
>   python/qemu/utils/feature.py  | 44
+++
>   3 files changed, 48 insertions(+), 13 deletions(-)
>   create mode 100644 python/qemu/utils/feature.py
>
> diff --git a/python/qemu/utils/__init__.py
b/python/qemu/utils/__init__.py
> index 7f1a5138c4..1d0789eaa2 100644
> --- a/python/qemu/utils/__init__.py
> +++ b/python/qemu/utils/__init__.py
> @@ -20,12 +20,14 @@
>
>   # pylint: disable=import-error
>   from .accel import kvm_available, list_accel, tcg_available
> +from .feature import list_feature
>
>
>   __all__ = (
>       'get_info_usernet_hostfwd_port',
>       'kvm_available',
>       'list_accel',
> +    'list_feature',
>       'tcg_available',
>   )
>
> diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
> index 297933df2a..b5bb80c6d3 100644
> --- a/python/qemu/utils/accel.py
> +++ b/python/qemu/utils/accel.py
> @@ -14,13 +14,11 @@
>   # the COPYING file in the top-level directory.
>   #
>
> -import logging
>   import os
> -import subprocess
>   from typing import List, Optional
>
> +from qemu.utils.feature import list_feature
>
> -LOG = logging.getLogger(__name__)
>
>   # Mapping host architecture to any additional architectures it can
>   # support which often includes its 32 bit cousin.
> @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
>       @raise Exception: if failed to run `qemu -accel help`
>       @return a list of accelerator names.
>       """
> -    if not qemu_bin:
> -        return []
> -    try:
> -        out = subprocess.check_output([qemu_bin, '-accel', 'help'],
> - universal_newlines=True)
> -    except:
> -        LOG.debug("Failed to get the list of accelerators in
%s", qemu_bin)
> -        raise
> -    # Skip the first line which is the header.
> -    return [acc.strip() for acc in out.splitlines()[1:]]
> +    return list_feature(qemu_bin, 'accel')
>
>
>   def kvm_available(target_arch: Optional[str] = None,
> diff --git a/python/qemu/utils/feature.py
b/python/qemu/utils/feature.py
> new file mode 100644
> index 00..b4a5f929ab
> --- /dev/null
> +++ b/python/qemu/utils/feature.py
> @@ -0,0 +1,44 @@
> +"""
> +QEMU feature module:
> +
> +This module provides a utility for discovering the availability of
> +generic features.
> +"""
> +# Copyright (C) 2022 Red Hat Inc.
Cleber, please, tell me how is the future like! :)


I grabbed a sports almanac.  That's all I can say. :)

Now seriously, thanks for spotting the typo.

> +#
> +# Authors:
> +#  Cleber Rosa mailto:cr...@redhat.com>>
> +#
> +# This work is licensed under the terms of the GNU GPL, version
2.  See
> +# the COPYING file in the top-level directory.
> +#
> +
> +import logging
> +import subprocess
> +from typing import List
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +def list_feature(qemu_bin: str, feature: str) -> List[str]:
> +    """
> +    List available options the QEMU binary for a given feature
type.
> +
> +    By calling a "qemu $feature -help" and parsing its output.

I understand we need a mean to easily cancel the test if given
feature
is not present. However, I'm unsure this generic list_feature() is
what
we need.

The `-accel help` returns a simple list of strings (besides the
header,
which is dismissed). Whereas `-machine help` returns what could be
parsed as a tuple of (name, description).

Another example is `-cpu help` which will print a similar list as
`-machine`, plus a section with CPUID flags.


I made sure it worked with both "accel" and "machine", but you're 
right about many other "-$feature help" that won't conform to the 
mapping ("-chardev help" is probably the only other one that should 
work).  What I thought about was to keep the same list_feature(), but 
make its parsing of items flexible.  Then it could be reused for other 
list_$feature() like methods.  At the same time, it could be an 

Re: [PATCH 0/1] vfio-ccw: Fix garbage sense data on I/O error

[Matthew Rosato]

On 6/10/21 4:20 PM, Eric Farman wrote:

Hi Conny,

Per our offline discussion, here's a fix for the error when a guest
issues "dasdfmt -M quick". It basically reverts commit 334e76850bbb
("vfio/ccw: update sense data if a unit check is pending")
and modifies the check that builds sense data in the TSCH handler.


Should it includes a Fixes: tag then?



I opted to NOT disable PMCW.CSENSE, because doing so prevents
vfio-ccw devices from coming online at all (didn't pursue deep
enough to explain why). Turning it off in reaction to a unit
check (in this now-reverted codepath) works, but only because of
the corresponding PMCW.CSENSE check in the TSCH code.

I don't know if anything is needed for the (unaltered) ECW data
that commit b498484ed49a ("s390x/css: sense data endianness")
addressed for the copied sense_data bytes, but figure we can
use this as a starting point. Thoughts?

Eric Farman (1):
   vfio-ccw: Keep passthrough sense data intact

  hw/s390x/css.c | 3 ++-
  hw/vfio/ccw.c  | 6 --
  2 files changed, 2 insertions(+), 7 deletions(-)

[PATCH 0/1] vfio-ccw: Fix garbage sense data on I/O error

[Eric Farman]

Hi Conny,

Per our offline discussion, here's a fix for the error when a guest
issues "dasdfmt -M quick". It basically reverts commit 334e76850bbb
("vfio/ccw: update sense data if a unit check is pending")
and modifies the check that builds sense data in the TSCH handler.

I opted to NOT disable PMCW.CSENSE, because doing so prevents
vfio-ccw devices from coming online at all (didn't pursue deep
enough to explain why). Turning it off in reaction to a unit
check (in this now-reverted codepath) works, but only because of
the corresponding PMCW.CSENSE check in the TSCH code.

I don't know if anything is needed for the (unaltered) ECW data
that commit b498484ed49a ("s390x/css: sense data endianness")
addressed for the copied sense_data bytes, but figure we can
use this as a starting point. Thoughts?

Eric Farman (1):
  vfio-ccw: Keep passthrough sense data intact

 hw/s390x/css.c | 3 ++-
 hw/vfio/ccw.c  | 6 --
 2 files changed, 2 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/1] vfio-ccw: Keep passthrough sense data intact

[Eric Farman]

For virtual devices, there is space for sense data to be built
and later copied into the IRB's ECW space once a TSCH is handled.

For passthrough devices, the IRB is passed up from hardware.
There might already be sense data in the ECW, in which case it
would be unusual to overwrite the IRB ESW/ECW data with itself.

In either case, if there isn't sense data, then an explicit SENSE
operation might be needed from the guest driver. Regardless,
fabricating sense data out of zeros seems like it would result
in unexpected behavior.

Let's remove the copying of the ECW/sense data in the vfio-ccw
driver, and adapt the check in the TSCH handler to look for
non-zero data in the local sense data before building a sense
data response in the IRB.

This fixes a "dasdfmt -M quick" command issued within a guest.

Signed-off-by: Eric Farman 
---
 hw/s390x/css.c | 3 ++-
 hw/vfio/ccw.c  | 6 --
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index bed46f5ec3..29234daa27 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1661,7 +1661,8 @@ int css_do_tsch_get_irb(SubchDev *sch, IRB *target_irb, 
int *irb_len)
 }
 /* If a unit check is pending, copy sense data. */
 if ((schib->scsw.dstat & SCSW_DSTAT_UNIT_CHECK) &&
-(schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE)) {
+(schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE) &&
+(sch->sense_data[0] != 0)) {
 int i;
 
 irb.scsw.flags |= SCSW_FLAGS_MASK_ESWF | SCSW_FLAGS_MASK_ECTL;
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 139a3d9d1b..a4dc4acb34 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -371,12 +371,6 @@ static void vfio_ccw_io_notifier_handler(void *opaque)
 copy_scsw_to_guest(&s, &irb.scsw);
 schib->scsw = s;
 
-/* If a uint check is pending, copy sense data. */
-if ((schib->scsw.dstat & SCSW_DSTAT_UNIT_CHECK) &&
-(schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE)) {
-memcpy(sch->sense_data, irb.ecw, sizeof(irb.ecw));
-}
-
 read_err:
 css_inject_io_interrupt(sch);
 }
-- 
2.25.1

[Bug 1914870] Re: libvixl compilation failure on Debian unstable

[Thomas Huth]

Fix has been committed here:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=2fed21d25b3a9562869

** Changed in: qemu
   Status: In Progress => Fix Committed

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

Title:
  libvixl compilation failure on Debian unstable

Status in QEMU:
  Fix Committed

Bug description:
  As of commit 0e324626306:

  $ lsb_release -d
  Description:Debian GNU/Linux bullseye/sid

  Project version: 5.2.50
  C compiler for the host machine: cc (gcc 10.2.1 "cc (Debian 10.2.1-6) 10.2.1 
20210110")
  C linker for the host machine: cc ld.bfd 2.35.1
  C++ compiler for the host machine: c++ (gcc 10.2.1 "c++ (Debian 10.2.1-6) 
10.2.1 20210110")
  C++ linker for the host machine: c++ ld.bfd 2.35.1

  [6/79] Compiling C++ object libcommon.fa.p/disas_libvixl_vixl_utils.cc.o
  FAILED: libcommon.fa.p/disas_libvixl_vixl_utils.cc.o 
  c++ -Ilibcommon.fa.p -I. -I.. -Iqapi -Itrace -Iui/shader 
-I/usr/include/capstone -I/usr/include/glib-2.0 
-I/usr/lib/hppa-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -pipe -Wall 
-Winvalid-pch -Wnon-virtual-dtor -Werror -std=gnu++11 -O2 -g -isystem 
/home/philmd/qemu/linux-headers -isystem linux-headers -iquote . -iquote 
/home/philmd/qemu -iquote /home/philmd/qemu/include -iquote 
/home/philmd/qemu/disas/libvixl -iquote /home/philmd/qemu/tcg/hppa -iquote 
/home/philmd/qemu/accel/tcg -pthread -D__STDC_LIMIT_MACROS 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -U_FORTIFY_SOURCE 
-D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-Wundef -Wwrite-strings -fno-strict-aliasing -fno-common -fwrapv -Wtype-limits 
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body 
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fPIE -MD -MQ 
libcommon.fa.p/disas_libvixl_vixl_utils.cc.o -MF 
libcommon.fa.p/disas_libvixl_vixl_utils.cc.o.d -o 
libcommon.fa.p/disas_libvixl_vixl_utils.cc.o -c ../disas/libvixl/vixl/utils.cc
  In file included from /home/philmd/qemu/disas/libvixl/vixl/utils.h:30,
   from ../disas/libvixl/vixl/utils.cc:27:
  /usr/include/string.h:36:43: error: missing binary operator before token "("
 36 | #if defined __cplusplus && (__GNUC_PREREQ (4, 4) \
|   ^
  /usr/include/string.h:53:62: error: missing binary operator before token "("
 53 | #if defined __USE_MISC || defined __USE_XOPEN || __GLIBC_USE (ISOC2X)
|  ^
  /usr/include/string.h:165:21: error: missing binary operator before token "("
165 |  || __GLIBC_USE (LIB_EXT2) || __GLIBC_USE (ISOC2X))
| ^
  /usr/include/string.h:174:43: error: missing binary operator before token "("
174 | #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2) || __GLIBC_USE 
(ISOC2X)
|   ^
  /usr/include/string.h:492:19: error: missing binary operator before token "("
492 | #if __GNUC_PREREQ (3,4)
|   ^
  In file included from /home/philmd/qemu/disas/libvixl/vixl/utils.h:30,
   from ../disas/libvixl/vixl/utils.cc:27:
  /usr/include/string.h:28:1: error: ‘__BEGIN_DECLS’ does not name a type
 28 | __BEGIN_DECLS
| ^
  In file included from /home/philmd/qemu/disas/libvixl/vixl/utils.h:30,
   from ../disas/libvixl/vixl/utils.cc:27:
  /usr/include/string.h:44:8: error: ‘size_t’ has not been declared
 44 |size_t __n) __THROW __nonnull ((1, 2));
|^~
  /usr/include/string.h:44:20: error: expected initializer before ‘__THROW’
 44 |size_t __n) __THROW __nonnull ((1, 2));
|^~~
  /usr/include/string.h:47:56: error: ‘size_t’ has not been declared
 47 | extern void *memmove (void *__dest, const void *__src, size_t __n)
|^~
  /usr/include/string.h:48:6: error: expected initializer before ‘__THROW’
 48 |  __THROW __nonnull ((1, 2));
|  ^~~
  /usr/include/string.h:61:42: error: ‘size_t’ has not been declared
 61 | extern void *memset (void *__s, int __c, size_t __n) __THROW 
__nonnull ((1));
|  ^~

  Is there a package dependency missing?

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

Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole

[Nir Soffer]

On Thu, Jun 10, 2021 at 9:35 PM Eric Blake  wrote:
>
> On Tue, Jun 08, 2021 at 07:38:10PM +0300, Nir Soffer wrote:
> > The example I provided was not detailed enough, what we actually do is:
> >
> > qemu-nbd .. 'json:{"driver": "qcow2", "backing": null, "file":
> > {"driver": "file", "filename": "top.qcow2"}}'
> >
> > So there is no backing chain and allocation depth is not relevant.
> > - Allocated areas should be reported with flags 0
> > - Zero areas which are not holes should be reported as NBD_STATE_ZERO
> > - Zero areas which are holes (not allocated in this image) should be
> > reported as NBD_STATE_HOLE
>
> Thinking about this a bit more, here's something I noticed:
>
> $ qemu-img map --output=json -f raw base.raw
> [{ "start": 0, "length": 196608, "depth": 0, "zero": false, "data": true, 
> "offset": 0},
> { "start": 196608, "length": 65536, "depth": 0, "zero": true, "data": false, 
> "offset": 196608}]
>
> which matches what I've said elsewhere in this thread: the entire
> image is reported as "depth":0 because the raw file is responsible for
> 100% of the content.
>
> But:
>
> $ qemu-img map --output=json -f qcow2 json:'{"driver":"qcow2","backing":null, 
> \
>   "file":{"driver":"file","filename":"top.qcow2"}}'
> [{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false},
> { "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
> "offset": 327680},
> { "start": 131072, "length": 131072, "depth": 0, "zero": true, "data": false}]
>
> also reports the entire file at "depth":0, which is misleading, since
> we have just been arguing from the qemu:allocation-depth perspective
> (and also from bdrv_block_status) that the qcow2 image is NOT 100%
> allocated (in the sense where allocation == data comes locally).
> Perhaps it might be better if we tweaked the above qemu-img map to
> produce:
>
> [{ "start": 0, "length": 65536, "depth": -1, "zero": true, "data": false},
> { "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
> "offset": 327680},
> { "start": 131072, "length": 65536, "depth": 0, "zero": true, "data": false},
> { "start": 196608, "length": 65536, "depth": -1, "zero": true, "data": false}]

It will be more consistent with "offset" to drop "depth" from output
if we don't have it:

[{ "start": 0, "length": 65536, "zero": true, "data": false},
 { "start": 65536, "length": 65536, "depth": 0, "zero": false,
"data": true, "offset": 327680},
 { "start": 131072, "length": 65536, "depth": 0, "zero": true,
"data": false},
 { "start": 196608, "length": 65536, "zero": true, "data": false}]

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:09 AM Cleber Rosa  wrote:
>
> Which can be used to check for any "feature" that is available as a
> QEMU command line option, and that will return its list of available
> options.
>
> This is a generalization of the list_accel() utility function, which
> is itself re-implemented in terms of the more generic feature.
>
> Signed-off-by: Cleber Rosa 
> ---
>  python/qemu/utils/__init__.py |  2 ++
>  python/qemu/utils/accel.py| 15 ++--
>  python/qemu/utils/feature.py  | 44 +++
>  3 files changed, 48 insertions(+), 13 deletions(-)
>  create mode 100644 python/qemu/utils/feature.py
>

Based on my comments from the next patch of this series:

Reviewed-by: Willian Rampazzo 

Re: [PATCH 3/4] Acceptance Tests: introduce method to require a feature and option

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:09 AM Cleber Rosa  wrote:
>
> In this context, and according to the qemu.utils.list_feature() utility
> function, a feature is something is available as a QEMU command line
> option that can take the "help" value.
>
> This builds on top of that utility function, and allows test writers
> to require, for instance, the "x-remote" (option) machine type
> (feature).
>
> This example is actually applied here to the reespective test, given
> that the option is conditionally built, and the test will ERROR
> without it.
>
> Signed-off-by: Cleber Rosa 
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 29 ++-
>  tests/acceptance/multiprocess.py  |  1 +
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> b/tests/acceptance/avocado_qemu/__init__.py
> index 93c4b9851f..432caff4e6 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -11,6 +11,7 @@
>  import logging
>  import os
>  import shutil
> +import subprocess
>  import sys
>  import uuid
>  import tempfile
> @@ -45,7 +46,8 @@
>  from qemu.utils import (
>  get_info_usernet_hostfwd_port,
>  kvm_available,
> -tcg_available,
> +list_feature,
> +tcg_available
>  )
>
>  def is_readable_executable_file(path):
> @@ -182,6 +184,31 @@ def _get_unique_tag_val(self, tag_name):
>  return vals.pop()
>  return None
>
> +def require_feature(self, feature, option=None):
> +"""
> +Requires a feature to be available for the test to continue
> +
> +It takes into account the currently set qemu binary, and only checks
> +for by running a "qemu -$feature help" command.  If the specific 
> option
> +is given, it checks if it's listed for the given feature.
> +
> +If the check fails, the test is canceled.
> +
> +:param feature: name of a QEMU feature, such as "accel" or "machine"
> +:type feature: str
> +:param option: the specific value for the feature.  For instance,
> +   if feature is "machine", option can be "q35".
> +type option: str
> +"""
> +try:
> +options_available = list_feature(self.qemu_bin, feature)

Looking at this code, the previous patch makes more sense the way it is now :)

Maybe, splitting it into multiple `list_` functions will make the code
here more complex.

Anyway, I'll let you decide.

Reviewed-by: Willian Rampazzo 

Re: [PATCH v2 05/37] target/riscv: SIMD 16-bit Shift Instructions

[Richard Henderson]

On 6/10/21 12:58 AM, LIU Zhiwei wrote:

  include/tcg/tcg-op-gvec.h   |   9 ++
  tcg/tcg-op-gvec.c   |  28 +++


Again, should be split out, with a

Reviewed-by: Richard Henderson 


r~

Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 8:55 PM Cleber Rosa Junior  wrote:
>
>
>
> On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta 
>  wrote:
>>
>> Hi,
>>
>> On 6/8/21 11:09 AM, Cleber Rosa wrote:
>> > Which can be used to check for any "feature" that is available as a
>> > QEMU command line option, and that will return its list of available
>> > options.
>> >
>> > This is a generalization of the list_accel() utility function, which
>> > is itself re-implemented in terms of the more generic feature.
>> >
>> > Signed-off-by: Cleber Rosa 
>> > ---
>> >   python/qemu/utils/__init__.py |  2 ++
>> >   python/qemu/utils/accel.py| 15 ++--
>> >   python/qemu/utils/feature.py  | 44 +++
>> >   3 files changed, 48 insertions(+), 13 deletions(-)
>> >   create mode 100644 python/qemu/utils/feature.py
>> >
>> > diff --git a/python/qemu/utils/__init__.py b/python/qemu/utils/__init__.py
>> > index 7f1a5138c4..1d0789eaa2 100644
>> > --- a/python/qemu/utils/__init__.py
>> > +++ b/python/qemu/utils/__init__.py
>> > @@ -20,12 +20,14 @@
>> >
>> >   # pylint: disable=import-error
>> >   from .accel import kvm_available, list_accel, tcg_available
>> > +from .feature import list_feature
>> >
>> >
>> >   __all__ = (
>> >   'get_info_usernet_hostfwd_port',
>> >   'kvm_available',
>> >   'list_accel',
>> > +'list_feature',
>> >   'tcg_available',
>> >   )
>> >
>> > diff --git a/python/qemu/utils/accel.py b/python/qemu/utils/accel.py
>> > index 297933df2a..b5bb80c6d3 100644
>> > --- a/python/qemu/utils/accel.py
>> > +++ b/python/qemu/utils/accel.py
>> > @@ -14,13 +14,11 @@
>> >   # the COPYING file in the top-level directory.
>> >   #
>> >
>> > -import logging
>> >   import os
>> > -import subprocess
>> >   from typing import List, Optional
>> >
>> > +from qemu.utils.feature import list_feature
>> >
>> > -LOG = logging.getLogger(__name__)
>> >
>> >   # Mapping host architecture to any additional architectures it can
>> >   # support which often includes its 32 bit cousin.
>> > @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) -> List[str]:
>> >   @raise Exception: if failed to run `qemu -accel help`
>> >   @return a list of accelerator names.
>> >   """
>> > -if not qemu_bin:
>> > -return []
>> > -try:
>> > -out = subprocess.check_output([qemu_bin, '-accel', 'help'],
>> > -  universal_newlines=True)
>> > -except:
>> > -LOG.debug("Failed to get the list of accelerators in %s", 
>> > qemu_bin)
>> > -raise
>> > -# Skip the first line which is the header.
>> > -return [acc.strip() for acc in out.splitlines()[1:]]
>> > +return list_feature(qemu_bin, 'accel')
>> >
>> >
>> >   def kvm_available(target_arch: Optional[str] = None,
>> > diff --git a/python/qemu/utils/feature.py b/python/qemu/utils/feature.py
>> > new file mode 100644
>> > index 00..b4a5f929ab
>> > --- /dev/null
>> > +++ b/python/qemu/utils/feature.py
>> > @@ -0,0 +1,44 @@
>> > +"""
>> > +QEMU feature module:
>> > +
>> > +This module provides a utility for discovering the availability of
>> > +generic features.
>> > +"""
>> > +# Copyright (C) 2022 Red Hat Inc.
>> Cleber, please, tell me how is the future like! :)
>
>
> I grabbed a sports almanac.  That's all I can say. :)
>
> Now seriously, thanks for spotting the typo.
>
>>
>> > +#
>> > +# Authors:
>> > +#  Cleber Rosa 
>> > +#
>> > +# This work is licensed under the terms of the GNU GPL, version 2.  See
>> > +# the COPYING file in the top-level directory.
>> > +#
>> > +
>> > +import logging
>> > +import subprocess
>> > +from typing import List
>> > +
>> > +
>> > +LOG = logging.getLogger(__name__)
>> > +
>> > +
>> > +def list_feature(qemu_bin: str, feature: str) -> List[str]:
>> > +"""
>> > +List available options the QEMU binary for a given feature type.
>> > +
>> > +By calling a "qemu $feature -help" and parsing its output.
>>
>> I understand we need a mean to easily cancel the test if given feature
>> is not present. However, I'm unsure this generic list_feature() is what
>> we need.
>>
>> The `-accel help` returns a simple list of strings (besides the header,
>> which is dismissed). Whereas `-machine help` returns what could be
>> parsed as a tuple of (name, description).
>>
>> Another example is `-cpu help` which will print a similar list as
>> `-machine`, plus a section with CPUID flags.
>>
>
> I made sure it worked with both "accel" and "machine", but you're right about 
> many other "-$feature help" that won't conform to the mapping ("-chardev 
> help" is probably the only other one that should work).  What I thought about 
> was to keep the same list_feature(), but make its parsing of items flexible.  
> Then it could be reused for other list_$feature() like methods.  At the same 
> time, it could be an opportunity to standardize a bit more of the "help" 
> outputs.
>
> For instance, I think it would make sense for "cpu" to keep showing the

Re: [PATCH v2 04/37] target/riscv: 8-bit Addition & Subtraction Instruction

[Richard Henderson]

On 6/10/21 12:58 AM, LIU Zhiwei wrote:

  include/tcg/tcg-op-gvec.h   |  6 ++
  tcg/tcg-op-gvec.c   | 47 


Likewise, should be split from the larger patch.


+static void gen_addv_mask_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b, TCGv_i32 m)
+{
+TCGv_i32 t1 = tcg_temp_new_i32();
+TCGv_i32 t2 = tcg_temp_new_i32();
+TCGv_i32 t3 = tcg_temp_new_i32();
+
+tcg_gen_andc_i32(t1, a, m);
+tcg_gen_andc_i32(t2, b, m);
+tcg_gen_xor_i32(t3, a, b);
+tcg_gen_add_i32(d, t1, t2);
+tcg_gen_and_i32(t3, t3, m);
+tcg_gen_xor_i32(d, d, t3);
+
+tcg_temp_free_i32(t1);
+tcg_temp_free_i32(t2);
+tcg_temp_free_i32(t3);
+}
+
+void tcg_gen_vec_add8_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b)
+{
+TCGv_i32 m = tcg_constant_i32((int32_t)dup_const(MO_8, 0x80));
+gen_addv_mask_i32(d, a, b, m);
+}


There will only ever be one use; we might as well merge them.
The cast is unnecessary.


r~

Re: [PATCH 49/55] target/arm: Implement MVE VQDMULL (vector)

[Richard Henderson]

On 6/10/21 12:08 PM, Peter Maydell wrote:

On Wed, 9 Jun 2021 at 21:20, Richard Henderson
 wrote:


On 6/7/21 9:58 AM, Peter Maydell wrote:

+++ b/target/arm/mve.decode
@@ -39,6 +39,8 @@
   @1op_nosz         &1op qd=%qd qm=%qm size=0
   @2op   .. size:2      &2op qd=%qd qm=%qm qn=%qn
   @2op_nosz         &2op qd=%qd qm=%qm qn=%qn 
size=0
+@2op_sz28         &2op qd=%qd qm=%qm qn=%qn \
+ size=%size_28


Move this back to VQDMULL[BT]_scalar, I think.


Why? VQDMULL[BT]_scalar uses an entirely different format
(as a scalar it uses the &2scalar arg struct with an rm field
for the gp register).


Oops, mis-read the @2op vs @2scalar.  Sorry.

r~

Re: [PATCH 2/2] tests: migration-test: Add dirty ring test

[Peter Xu]

On Thu, Jun 10, 2021 at 08:01:44PM +0100, Dr. David Alan Gilbert wrote:
> > +#include 
> 
> Does that get you the system headers, which may or may not have
> KVM_CAP_DIRTY_LOG_RING if you're on an old host, or does it get you
> qemu's linux-headers which definitely does?

I tested it and it's using the linux-headers/ file even if I also got the other
/usr/include one.  So I think the qemu one just has higher priority in the "-I"
paths.

Btw, IIUC quotting with <> or "" should be the same here for headers, so I'm
thinking maybe I should switch to "" like the rest headers.

> 
> What happens on a BSD or the like?

Ah, good point..

How about I squash this into the patch?

---8<---
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index cc6e396d1a2..9ef6b471353 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -12,7 +12,6 @@
 
 #include "qemu/osdep.h"
 
-#include 
 #include "libqos/libqtest.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
@@ -28,6 +27,10 @@
 #include "migration-helpers.h"
 #include "tests/migration/migration-test.h"
 
+#if defined(__linux__)
+#include "linux/kvm.h"
+#endif
+
 /* TODO actually test the results and get rid of this */
 #define qtest_qmp_discard_response(...) qobject_unref(qtest_qmp(__VA_ARGS__))
 
@@ -1392,6 +1395,7 @@ static void test_multifd_tcp_cancel(void)
 
 static bool kvm_dirty_ring_supported(void)
 {
+#if defined(__linux__)
 int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
 
 if (kvm_fd < 0) {
@@ -1407,6 +1411,9 @@ static bool kvm_dirty_ring_supported(void)
 }
 
 return true;
+#else
+return false;
+#endif
 }
 
 int main(int argc, char **argv)
---8<---

Thanks!

-- 
Peter Xu

Re: [PATCH 4/4] Jobs based on custom runners: add CentOS Stream 8

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:10 AM Cleber Rosa  wrote:
>
> This introduces three different parts of a job designed to run
> on a custom runner managed by Red Hat.  The goals include:
>
>  a) serve as a model for other organizations that want to onboard
> their own runners, with their specific platforms, build
> configuration and tests.
>
>  b) bring awareness to the differences between upstream QEMU and the
> version available under CentOS Stream, which is "A preview of
> upcoming Red Hat Enterprise Linux minor and major releases.".
>
>  c) becase of b), it should be easier to identify and reduce the gap
> between Red Hat's downstream and upstream QEMU.
>
> The components themselves to achieve this custom job are:
>
>  1) build environment configuration: documentation and a playbook for
> a base Enterprise Linux 8 system (also applicable to CentOS
> Stream), which other users can run on their system to get the
> environment suitable for building QEMU.
>
>  2) QEMU build configuration: how QEMU will be built to match, as
> closely as possible, the binaries built and packaged on CentOS
> stream 8.
>
>  3) job definition: GitLab CI jobs that will dispatch the build/test
> job to the machine specifically configured according to #1.
>
> Signed-off-by: Cleber Rosa 
> ---
>  .gitlab-ci.d/custom-runners.yml|  29 
>  scripts/ci/org.centos/stream/README|   2 +
>  scripts/ci/org.centos/stream/configure | 190 +
>  scripts/ci/setup/build-environment.yml |  38 +
>  4 files changed, 259 insertions(+)
>  create mode 100644 scripts/ci/org.centos/stream/README
>  create mode 100755 scripts/ci/org.centos/stream/configure
>
> diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
> index 061d3cdfed..ee5143995e 100644
> --- a/.gitlab-ci.d/custom-runners.yml
> +++ b/.gitlab-ci.d/custom-runners.yml
> @@ -220,3 +220,32 @@ ubuntu-20.04-aarch64-notcg:
>   - ../configure --disable-libssh --disable-tcg
>   - make --output-sync -j`nproc`
>   - make --output-sync -j`nproc` check V=1
> +
> +centos-stream-8-x86_64:
> + allow_failure: true
> + needs: []
> + stage: build
> + tags:
> + - centos_stream_8
> + - x86_64

What happens if another organization wants to add its own custom
runner with its own set of tests based on centos stream 8? My
suggestion is to add an organization tag to the custom runners. If
this job runs tests important to Red Hat, we should name it and tag
the runner with it.

Unless Red Hat is willing to add other tests that are interesting to
other organizations and run it on its custom runner. If that is the
case, who should check those tests in case of failure?

> + rules:
> + - if: '$CI_COMMIT_BRANCH =~ /^staging/'
> + artifacts:
> +   name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
> +   when: on_failure
> +   expire_in: 7 days
> +   paths:
> + - build/tests/results/latest/results.xml
> + - build/tests/results/latest/test-results
> +   reports:
> + junit: build/tests/results/latest/results.xml
> + script:
> + - mkdir build
> + - cd build
> + - ../scripts/ci/org.centos/stream/configure
> + - make --output-sync -j`nproc`
> + - make --output-sync -j`nproc` check V=1
> + - make get-vm-images
> + # Only run tests that are either marked explicitly for KVM and x86_64
> + # or tests that are supposed to be valid for all targets
> + - ./tests/venv/bin/avocado run --job-results-dir=tests/results/ 
> --filter-by-tags-include-empty --filter-by-tags-include-empty-key -t 
> accel:kvm,arch:x86_64 -- tests/acceptance/
> diff --git a/scripts/ci/org.centos/stream/README 
> b/scripts/ci/org.centos/stream/README
> new file mode 100644
> index 00..f99bda99b8
> --- /dev/null
> +++ b/scripts/ci/org.centos/stream/README
> @@ -0,0 +1,2 @@
> +This directory contains scripts for generating a build of QEMU that
> +closely matches the CentOS Stream builds of the qemu-kvm package.
> diff --git a/scripts/ci/org.centos/stream/configure 
> b/scripts/ci/org.centos/stream/configure
> new file mode 100755
> index 00..1e7207faec
> --- /dev/null
> +++ b/scripts/ci/org.centos/stream/configure
> @@ -0,0 +1,190 @@
> +#!/bin/sh -e
> +../configure \
> +--prefix="/usr" \
> +--libdir="/usr/lib64" \
> +--datadir="/usr/share" \
> +--sysconfdir="/etc" \
> +--interp-prefix=/usr/qemu-%M \
> +--localstatedir="/var" \
> +--docdir="/usr/share/doc" \
> +--libexecdir="/usr/libexec" \
> +--extra-ldflags="-Wl,--build-id -Wl,-z,relro -Wl,-z,now" \
> +--extra-cflags="-O2 -g -pipe -Wall -Werror=format-security 
> -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions 
> -fstack-protector-strong -grecord-gcc-switches 
> -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 
> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic 
> -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" \
> +--with-suffix="qemu-kvm" \
> +--firmwarepath=/usr/share/qemu-firmware \
> +--meson="/usr/bin/meson" \
> +--target-list="x86_64-softm

Re: [PATCH 52/55] target/arm: Implement MVE VCADD

[Peter Maydell]

On Wed, 9 Jun 2021 at 22:16, Richard Henderson
 wrote:
>
> On 6/7/21 9:58 AM, Peter Maydell wrote:
> > +/*
> > + * VCADD Qd == Qm at size MO_32 is UNPREDICTABLE; we choose not to diagnose
> > + * so we can reuse the DO_2OP macro. (Our implementation calculates the
> > + * "expected" results in this case.)
> > + */
> You've done this elsewhere, though.

Yeah, because in those cases the op had to have its own hand-written
trans_ function for other reasons so the check was easy to add. Hence
the comment about why this particular case doesn't do that.

> Either way,
> Reviewed-by: Richard Henderson 

thanks
-- PMM

Re: [PATCH 49/55] target/arm: Implement MVE VQDMULL (vector)

[Peter Maydell]

On Wed, 9 Jun 2021 at 21:20, Richard Henderson
 wrote:
>
> On 6/7/21 9:58 AM, Peter Maydell wrote:
> > +++ b/target/arm/mve.decode
> > @@ -39,6 +39,8 @@
> >   @1op_nosz         &1op qd=%qd qm=%qm 
> > size=0
> >   @2op   .. size:2      &2op qd=%qd qm=%qm 
> > qn=%qn
> >   @2op_nosz         &2op qd=%qd qm=%qm 
> > qn=%qn size=0
> > +@2op_sz28         &2op qd=%qd qm=%qm 
> > qn=%qn \
> > + size=%size_28
>
> Move this back to VQDMULL[BT]_scalar, I think.

Why? VQDMULL[BT]_scalar uses an entirely different format
(as a scalar it uses the &2scalar arg struct with an rm field
for the gp register).

-- PMM

Re: [PATCH 2/2] tests: migration-test: Add dirty ring test

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> Add dirty ring test if kernel supports it.  Add the dirty ring parameter on
> source should be mostly enough, but let's change the dest too to make them
> match always.
> 
> Signed-off-by: Peter Xu 
> ---
>  tests/qtest/migration-test.c | 51 +---
>  1 file changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d9225f58d4d..cc6e396d1a2 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -12,6 +12,7 @@
>  
>  #include "qemu/osdep.h"
>  
> +#include 

Does that get you the system headers, which may or may not have
KVM_CAP_DIRTY_LOG_RING if you're on an old host, or does it get you
qemu's linux-headers which definitely does?

What happens on a BSD or the like?

Dave

>  #include "libqos/libqtest.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qdict.h"
> @@ -467,6 +468,8 @@ typedef struct {
>  bool use_shmem;
>  /* only launch the target process */
>  bool only_target;
> +/* Use dirty ring if true; dirty logging otherwise */
> +bool use_dirty_ring;
>  char *opts_source;
>  char *opts_target;
>  } MigrateStart;
> @@ -573,11 +576,13 @@ static int test_migrate_start(QTestState **from, 
> QTestState **to,
>  shmem_opts = g_strdup("");
>  }
>  
> -cmd_source = g_strdup_printf("-accel kvm -accel tcg%s%s "
> +cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
>   "-name source,debug-threads=on "
>   "-m %s "
>   "-serial file:%s/src_serial "
>   "%s %s %s %s",
> + args->use_dirty_ring ?
> + ",dirty-ring-size=4096" : "",
>   machine_opts ? " -machine " : "",
>   machine_opts ? machine_opts : "",
>   memory_size, tmpfs,
> @@ -587,12 +592,14 @@ static int test_migrate_start(QTestState **from, 
> QTestState **to,
>  *from = qtest_init(cmd_source);
>  }
>  
> -cmd_target = g_strdup_printf("-accel kvm -accel tcg%s%s "
> +cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
>   "-name target,debug-threads=on "
>   "-m %s "
>   "-serial file:%s/dest_serial "
>   "-incoming %s "
>   "%s %s %s %s",
> + args->use_dirty_ring ?
> + ",dirty-ring-size=4096" : "",
>   machine_opts ? " -machine " : "",
>   machine_opts ? machine_opts : "",
>   memory_size, tmpfs, uri,
> @@ -785,12 +792,14 @@ static void test_baddest(void)
>  test_migrate_end(from, to, false);
>  }
>  
> -static void test_precopy_unix(void)
> +static void test_precopy_unix_common(bool dirty_ring)
>  {
>  g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
>  MigrateStart *args = migrate_start_new();
>  QTestState *from, *to;
>  
> +args->use_dirty_ring = dirty_ring;
> +
>  if (test_migrate_start(&from, &to, uri, args)) {
>  return;
>  }
> @@ -825,6 +834,18 @@ static void test_precopy_unix(void)
>  test_migrate_end(from, to, true);
>  }
>  
> +static void test_precopy_unix(void)
> +{
> +/* Using default dirty logging */
> +test_precopy_unix_common(false);
> +}
> +
> +static void test_precopy_unix_dirty_ring(void)
> +{
> +/* Using dirty ring tracking */
> +test_precopy_unix_common(true);
> +}
> +
>  #if 0
>  /* Currently upset on aarch64 TCG */
>  static void test_ignore_shared(void)
> @@ -1369,6 +1390,25 @@ static void test_multifd_tcp_cancel(void)
>  test_migrate_end(from, to2, true);
>  }
>  
> +static bool kvm_dirty_ring_supported(void)
> +{
> +int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
> +
> +if (kvm_fd < 0) {
> +return false;
> +}
> +
> +ret = ioctl(kvm_fd, KVM_CHECK_EXTENSION, KVM_CAP_DIRTY_LOG_RING);
> +close(kvm_fd);
> +
> +/* We test with 4096 slots */
> +if (ret < 4096) {
> +return false;
> +}
> +
> +return true;
> +}
> +
>  int main(int argc, char **argv)
>  {
>  char template[] = "/tmp/migration-test-XX";
> @@ -1438,6 +1478,11 @@ int main(int argc, char **argv)
>  qtest_add_func("/migration/multifd/tcp/zstd", test_multifd_tcp_zstd);
>  #endif
>  
> +if (kvm_dirty_ring_supported()) {
> +qtest_add_func("/migration/dirty_ring",
> +   test_precopy_unix_dirty_ring);
> +}
> +
>  ret = g_test_run();
>  
>  g_assert_cmpint(ret, ==, 0);
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 1/2] tests: migration-test: Still run the rest even if uffd missing

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> Currently we'll skip the whole migration-test if uffd missing.
> 
> It's a bit harsh - we can still run the rest besides postcopy!  Enable them
> when we still can.
> 
> It'll happen more frequently now after kernel UFFD_USER_MODE_ONLY introduced 
> in
> commit 37cd0575b8510159, as qemu test normally requires kernel faults.  One
> alternative is we disable kvm and create the uffd with UFFD_USER_MODE_ONLY for
> all postcopy tests, however to be simple for now just skip postcopy tests only
> by default.  If we wanna run them use "sudo" or root, they'll still work.  In
> all cases, it's still better than running nothing for migration-test.
> 
> Signed-off-by: Peter Xu 

Ouch! Yes; that check was originally in a standalone test file for
postcopy that's then morphed into the full test over a few years.



Reviewed-by: Dr. David Alan Gilbert 

> ---
>  tests/qtest/migration-test.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 2b028df6875..d9225f58d4d 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1376,10 +1376,6 @@ int main(int argc, char **argv)
>  
>  g_test_init(&argc, &argv, NULL);
>  
> -if (!ufd_version_check()) {
> -return g_test_run();
> -}
> -
>  /*
>   * On ppc64, the test only works with kvm-hv, but not with kvm-pr and TCG
>   * is touchy due to race conditions on dirty bits (especially on PPC for
> @@ -1416,8 +1412,11 @@ int main(int argc, char **argv)
>  
>  module_call_init(MODULE_INIT_QOM);
>  
> -qtest_add_func("/migration/postcopy/unix", test_postcopy);
> -qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery);
> +if (ufd_version_check()) {
> +qtest_add_func("/migration/postcopy/unix", test_postcopy);
> +qtest_add_func("/migration/postcopy/recovery", 
> test_postcopy_recovery);
> +}
> +
>  qtest_add_func("/migration/bad_dest", test_baddest);
>  qtest_add_func("/migration/precopy/unix", test_precopy_unix);
>  qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 0/4] Jobs based on custom runners: add CentOS Stream 8

[Willian Rampazzo]

On Tue, Jun 8, 2021 at 11:09 AM Cleber Rosa  wrote:
>
> This builds on top the "GitLab Custom Runners and Jobs (was: QEMU
> Gating CI)" series, showing an example of how other entities can
> add their own custom jobs to the GitLab CI pipeline.
>
> First of all, it may be useful to see an actual pipeline (and the
> reespective job introduced here) combined with the jobs introduced
> on "GitLab Custom Runners and Jobs (was: QEMU Gating CI)":
>
>  * https://gitlab.com/cleber.gnu/qemu/-/pipelines/316527166
>  * https://gitlab.com/cleber.gnu/qemu/-/jobs/1325976765
>
> The runner (the machine and job) is to be managed by Red Hat, and
> adds, at the very least, bare metal x86_64 KVM testing capabilities to
> the QEMU pipeline.  This brings extra coverage for some unittests, and
> the ability to run the acceptance tests dependent on KVM.
>
> The runner is already completely set up and registered to the
> https://gitlab.com/qemu-project/qemu project instance, and jobs will
> be triggered according to the same rules for the jobs introduced on
> "GitLab Custom Runners and Jobs (was: QEMU Gating CI)", that is,
> but pushes to the staging branch.  Still, the job is set with mode
> "allow failures", so it should not disrupt the existing pipeline.
> Once its reliability is proved (rules and service levels are to be
> determined), that can be reverted.
>
> Even though the formal method of tracking machine/job maintainers have
> not been formalized, it should be known that the contacts/admins for
> this machine and job are:
>
>  - Cleber Rosa
>
>clebergnu on #qemu
>
>  - Willian Rampazzo
>
>willianr on #qemu

Acked-by: Willian Rampazzo 

Re: [PATCH v4 04/32] block/nbd: connect_thread_func(): do qio_channel_set_delay(false)

[Eric Blake]

On Thu, Jun 10, 2021 at 01:07:34PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> nbd_open() does it (through nbd_establish_connection()).
> Actually we lost that call on reconnect path in 1dc4718d849e1a1fe
> "block/nbd: use non-blocking connect: fix vm hang on connect()"
> when we have introduced reconnect thread.
> 
> Fixes: 1dc4718d849e1a1fe665ce5241ed79048cfa2cfc
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/nbd.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake 

> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 01d2c2efad..f3a036354d 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -408,6 +408,8 @@ static void *connect_thread_func(void *opaque)
>  thr->sioc = NULL;
>  }
>  
> +qio_channel_set_delay(QIO_CHANNEL(thr->sioc), false);
> +
>  qemu_mutex_lock(&thr->mutex);
>  
>  switch (thr->state) {
> -- 
> 2.29.2
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[PATCH 1/1] hw: virt: consider hw_compat_6_0

[Heinrich Schuchardt]

virt-6.0 must consider hw_compat_6_0.

Signed-off-by: Heinrich Schuchardt 
---
 hw/arm/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 840758666d..8bc3b408fe 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2764,6 +2764,8 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 1)

 static void virt_machine_6_0_options(MachineClass *mc)
 {
+virt_machine_6_1_options(mc);
+compat_props_add(mc->compat_props, hw_compat_6_0, hw_compat_6_0_len);
 }
 DEFINE_VIRT_MACHINE(6, 0)

--
2.30.2

Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole

[Eric Blake]

On Tue, Jun 08, 2021 at 07:38:10PM +0300, Nir Soffer wrote:
> The example I provided was not detailed enough, what we actually do is:
> 
> qemu-nbd .. 'json:{"driver": "qcow2", "backing": null, "file":
> {"driver": "file", "filename": "top.qcow2"}}'
> 
> So there is no backing chain and allocation depth is not relevant.
> - Allocated areas should be reported with flags 0
> - Zero areas which are not holes should be reported as NBD_STATE_ZERO
> - Zero areas which are holes (not allocated in this image) should be
> reported as NBD_STATE_HOLE

Thinking about this a bit more, here's something I noticed:

$ qemu-img map --output=json -f raw base.raw
[{ "start": 0, "length": 196608, "depth": 0, "zero": false, "data": true, 
"offset": 0},
{ "start": 196608, "length": 65536, "depth": 0, "zero": true, "data": false, 
"offset": 196608}]

which matches what I've said elsewhere in this thread: the entire
image is reported as "depth":0 because the raw file is responsible for
100% of the content.

But:

$ qemu-img map --output=json -f qcow2 json:'{"driver":"qcow2","backing":null, \
  "file":{"driver":"file","filename":"top.qcow2"}}'
[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false},
{ "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
{ "start": 131072, "length": 131072, "depth": 0, "zero": true, "data": false}]

also reports the entire file at "depth":0, which is misleading, since
we have just been arguing from the qemu:allocation-depth perspective
(and also from bdrv_block_status) that the qcow2 image is NOT 100%
allocated (in the sense where allocation == data comes locally).
Perhaps it might be better if we tweaked the above qemu-img map to
produce:

[{ "start": 0, "length": 65536, "depth": -1, "zero": true, "data": false},
{ "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
{ "start": 131072, "length": 65536, "depth": 0, "zero": true, "data": false},
{ "start": 196608, "length": 65536, "depth": -1, "zero": true, "data": false}]

that is, use "depth":-1 to explicitly denote portions of a qcow2 file
which are NOT provided locally, and which are not found anywhere in
the backing chain.  In other words, make it explicit in qemu-img map
output what is possible with qemu:allocation-depth==0.

Or tweak it slightly to mean that "depth":-1 corresponds to "cluster
is not provided by the current layer, but we could not determine if it
is provided by a particular backing layer or if it was unallocated
overall".  Then positive depth means we know which point in the
backing chain we deferred to, 0 is local, and negative depth means
that we defer to a backing layer (but could not report WHICH layer, if
any).  This tweak would make it easier for my thoughts of having qemu
NBD clients automatically request qemu:allocation-depth without having
to resort to x-dirty-bitmap hacks, and still be able to expose the
information via qemu-img map.

I'm off to another round of code hacking to see how it looks...

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] hw/nvme: be more careful when deasserting IRQs

[Klaus Jensen]

On Jun 10 13:46, Jakub Jermář wrote:

An IRQ vector used by a completion queue cannot be deasserted without
first checking if the same vector does not need to stay asserted for
some other completion queue.

Signed-off-by: Jakub Jermar 
---
hw/nvme/ctrl.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 0bcaf7192f..c0980929eb 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -473,6 +473,21 @@ static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
}
}

+/*
+ * Check if the vector used by the cq can be deasserted, i.e. it needn't be
+ * asserted for some other cq.
+ */
+static bool nvme_irq_can_deassert(NvmeCtrl *n, NvmeCQueue *cq)
+{
+for (unsigned qid = 0; qid < n->params.max_ioqpairs + 1; qid++) {
+NvmeCQueue *q = n->cq[qid];
+
+if (q && q->vector == cq->vector && q->head != q->tail)
+return false;  /* some queue needs this to stay asserted */
+}
+return true;
+}
+
static void nvme_req_clear(NvmeRequest *req)
{
req->ns = NULL;
@@ -4089,7 +4104,9 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeRequest *req)
trace_pci_nvme_err_invalid_del_cq_notempty(qid);
return NVME_INVALID_QUEUE_DEL;
}
-nvme_irq_deassert(n, cq);
+if (nvme_irq_can_deassert(n, cq)) {
+nvme_irq_deassert(n, cq);
+}
trace_pci_nvme_del_cq(qid);
nvme_free_cq(cq, n);
return NVME_SUCCESS;
@@ -5757,7 +5774,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int 
val)
timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
}

-if (cq->tail == cq->head) {
+if (nvme_irq_can_deassert(n, cq)) {
nvme_irq_deassert(n, cq);
}
} else {
--
2.31.1



This is actually an artifact of commit ca247d35098d3 ("hw/block/nvme: 
fix pin-based interrupt behavior") that I did a year ago. Prior to that 
fix, the completion queue id was used to index the internal IS register 
(irq_status), which, while wrong spec-wise, had the effect of... 
actually working.


Anyway, I agree that the logic is flawed right now, since we should only 
deassert when all outstanding cqe's have been acknowledged by the host.


nvme_irq_can_deassert should be guarded with a check on msix_enabled(), 
but in any case I am not happy about looping over all completion queues 
on each cq doorbell write. I think this can be ref counted? I.e. 
decrement when cq->tail == cq->head on the cq doorbell write and 
increment only when going from empty to non-empty in nvme_post_cqes().


signature.asc
Description: PGP signature

Re: [PATCH v2 03/37] target/riscv: 16-bit Addition & Subtraction Instructions

[Richard Henderson]

On 6/10/21 12:58 AM, LIU Zhiwei wrote:

Include 5 groups: Wrap-around (dropping overflow), Signed Halving,
Unsigned Halving, Signed Saturation, and Unsigned Saturation.

Signed-off-by: LIU Zhiwei
---
  include/tcg/tcg-op-gvec.h   |  10 +
  target/riscv/helper.h   |  30 ++
  target/riscv/insn32.decode  |  32 +++
  target/riscv/insn_trans/trans_rvp.c.inc | 117 
  target/riscv/meson.build|   1 +
  target/riscv/packed_helper.c| 354 
  target/riscv/translate.c|   1 +
  tcg/tcg-op-gvec.c   |  28 ++


The tcg part needs to be split out, and I'm happy to give a

Reviewed-by: Richard Henderson 

on that.


r~

Re: [PATCH] accel/tcg: Use MiB in tcg_init_machine

[Philippe Mathieu-Daudé]

On 6/10/21 6:40 PM, Richard Henderson wrote:
> Suggested-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
> 
> This sits in the middle of my "Clean up code_gen_buffer allocation"
> patch set.  Alex mentioned it during review, and I had already made
> the change.
> 
> This is the only patch in the set that has not been posted and
> reviewed.  Rather than re-posting the entire set, I'm just
> sending this one and will queue the whole thing to tcg-next.
> 
> 
> r~
> 
> ---
>  accel/tcg/tcg-all.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé 

RE: [PATCH] tcg: Fix documentation for tcg_constant_* vs tcg_temp_free_*

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> At some point during the development of tcg_constant_*, I changed my mind
> about whether such temps should be able to be passed to tcg_temp_free_*.
> The final version committed allows this, but the commentary was not updated
> to match.
> 
> Fixes: c0522136adf
> Reported-by: Peter Maydell 
> Signed-off-by: Richard Henderson 
> ---
>  include/tcg/tcg.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

Re: [PATCH v4 01/32] co-queue: drop extra coroutine_fn marks

[Vladimir Sementsov-Ogievskiy]

10.06.2021 20:22, Eric Blake wrote:

On Thu, Jun 10, 2021 at 01:07:31PM +0300, Vladimir Sementsov-Ogievskiy wrote:

qemu_co_queue_next() and qemu_co_queue_restart_all() just call
aio_co_wake() which works well in non-coroutine context. So these
functions can be called from non-coroutine context as well. And
actually qemu_co_queue_restart_all() is called from
nbd_cancel_in_flight(), which is called from non-coroutine context.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/qemu/coroutine.h | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)


In the back of my mind, I have to repeatedly question my faulty memory
on whether:

absence of coroutine_fn means this function is unsafe to call from a
coroutine context (that is, coroutines can only call functions tagged
with coroutine_fn)

vs.

presence of coroutine_fn means this function must only use
coroutine-safe actions, but not all coroutine-safe functions have this
tag (in particular, functions which are designed to work both in or
out of coroutine context do not have the tag, but coroutines can call
functions without the tag)

But thankfully, rereading include/qemu/coroutine.h clears it up and
both of my initial thoughts are wrong although the second is closer;
it is yet another definition:

presence of coroutine_fn means this function must NOT be called except
from a coroutine context.  coroutines can call functions with or
without the tag, but if they lack the tag, the coroutine must ensure
the function won't block.

Thus, adding the tag is something we do when writing a function that
obeys coroutine rules and requires a coroutine context to already be
present, and once a function is then relaxed enough to work from both
regular and coroutine functions, we drop the marker.  But we keep the
_co_ in the function name to remind ourselves that it is
coroutine-safe, in addition to normal-safe.

And your patch is doing just that - we have functions that work from
both normal and coroutine context, so we can drop the marker but keep
_co_ in the name.

Reviewed-by: Eric Blake 



Actually, usually _co_ == coroutine_fn. I don't drop it here because it's the 
name of the object: CoQueue, so qemu_co_queue_ refers to it..

We also have for example aio_co_wake, which is safe to call from non-coroutine 
context. And _co_ refers to what function do: wake a coroutine.

However it would be strange to have a function with _co_ in the name (in any 
meaning) which is unsafe to call from coroutine context.

--
Best regards,
Vladimir

Re: [PATCH] hw/nvme: be more careful when deasserting IRQs

[Klaus Jensen]

+cc qemu-block, maintainers

On Jun 10 13:46, Jakub Jermář wrote:

An IRQ vector used by a completion queue cannot be deasserted without
first checking if the same vector does not need to stay asserted for
some other completion queue.

Signed-off-by: Jakub Jermar 
---
hw/nvme/ctrl.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 0bcaf7192f..c0980929eb 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -473,6 +473,21 @@ static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
}
}

+/*
+ * Check if the vector used by the cq can be deasserted, i.e. it needn't be
+ * asserted for some other cq.
+ */
+static bool nvme_irq_can_deassert(NvmeCtrl *n, NvmeCQueue *cq)
+{
+for (unsigned qid = 0; qid < n->params.max_ioqpairs + 1; qid++) {
+NvmeCQueue *q = n->cq[qid];
+
+if (q && q->vector == cq->vector && q->head != q->tail)
+return false;  /* some queue needs this to stay asserted */
+}
+return true;
+}
+
static void nvme_req_clear(NvmeRequest *req)
{
req->ns = NULL;
@@ -4089,7 +4104,9 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeRequest *req)
trace_pci_nvme_err_invalid_del_cq_notempty(qid);
return NVME_INVALID_QUEUE_DEL;
}
-nvme_irq_deassert(n, cq);
+if (nvme_irq_can_deassert(n, cq)) {
+nvme_irq_deassert(n, cq);
+}
trace_pci_nvme_del_cq(qid);
nvme_free_cq(cq, n);
return NVME_SUCCESS;
@@ -5757,7 +5774,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int 
val)
timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
}

-if (cq->tail == cq->head) {
+if (nvme_irq_can_deassert(n, cq)) {
nvme_irq_deassert(n, cq);
}
} else {
--
2.31.1




--
One of us - No more doubt, silence or taboo about mental illness.


signature.asc
Description: PGP signature

Re: Re: [RFC v1] virtio/vsock: add two more queues for datagram types

[Jiang Wang .]

On Thu, Jun 10, 2021 at 2:40 AM Stefano Garzarella  wrote:
>
> On Thu, Jun 10, 2021 at 12:14:24AM +, Jiang Wang wrote:
> >Datagram sockets are connectionless and unreliable.
> >The sender does not know the capacity of the receiver
> >and may send more packets than the receiver can handle.
> >
> >Add two more dedicate virtqueues for datagram sockets,
> >so that it will not unfairly steal resources from
> >stream and future connection-oriented sockets.
> >
> >The virtio spec patch is here:
> >https://www.spinics.net/lists/linux-virtualization/msg50027.html
> >
> >Here is the link for the linux kernel git repo with patches
> >to support dgram sockets:
> >https://github.com/Jiang1155/linux/tree/vsock-dgram-v1
> >
> >Signed-off-by: Jiang Wang 
> >---
> > configure | 13 +
> > hw/virtio/vhost-vsock-common.c| 11 ++-
> > hw/virtio/vhost-vsock.c   |  8 +---
> > include/hw/virtio/vhost-vsock-common.h| 10 +-
> > include/standard-headers/linux/virtio_vsock.h |  3 +++
> > meson.build   |  1 +
> > 6 files changed, 41 insertions(+), 5 deletions(-)
> >
> >diff --git a/configure b/configure
> >index 9f016b06b5..6455b283a5 100755
> >--- a/configure
> >+++ b/configure
> >@@ -343,6 +343,7 @@ vhost_net="$default_feature"
> > vhost_crypto="$default_feature"
> > vhost_scsi="$default_feature"
> > vhost_vsock="$default_feature"
> >+vhost_vsock_dgram="no"
> > vhost_user="no"
> > vhost_user_blk_server="auto"
> > vhost_user_fs="$default_feature"
> >@@ -1272,6 +1273,10 @@ for opt do
> >   ;;
> >   --enable-vhost-vsock) vhost_vsock="yes"
> >   ;;
> >+  --disable-vhost-vsock-dgram) vhost_vsock_dgram="no"
> >+  ;;
> >+  --enable-vhost-vsock-dgram) vhost_vsock_dgram="yes"
> >+  ;;
>
> I don't think we should add a configuration option to enable/disable the
> dgram support at build time.
>
> I think we should do it at runtime looking at the features negiotated.
>
> Take a look at virtio_net_set_multiqueue().

Got it. Will check. Thanks.

> Thanks,
> Stefano
>

RE: [PATCH] accel/tcg: Use MiB in tcg_init_machine

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Suggested-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
> ---
> 
> This sits in the middle of my "Clean up code_gen_buffer allocation"
> patch set.  Alex mentioned it during review, and I had already made the 
> change.
> 
> This is the only patch in the set that has not been posted and reviewed.  
> Rather
> than re-posting the entire set, I'm just sending this one and will queue the 
> whole
> thing to tcg-next.
> 
> 
> r~
> 
> ---
>  accel/tcg/tcg-all.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

Re: [PATCH v4 01/32] co-queue: drop extra coroutine_fn marks

[Eric Blake]

On Thu, Jun 10, 2021 at 01:07:31PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> qemu_co_queue_next() and qemu_co_queue_restart_all() just call
> aio_co_wake() which works well in non-coroutine context. So these
> functions can be called from non-coroutine context as well. And
> actually qemu_co_queue_restart_all() is called from
> nbd_cancel_in_flight(), which is called from non-coroutine context.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  include/qemu/coroutine.h | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

In the back of my mind, I have to repeatedly question my faulty memory
on whether:

absence of coroutine_fn means this function is unsafe to call from a
coroutine context (that is, coroutines can only call functions tagged
with coroutine_fn)

vs.

presence of coroutine_fn means this function must only use
coroutine-safe actions, but not all coroutine-safe functions have this
tag (in particular, functions which are designed to work both in or
out of coroutine context do not have the tag, but coroutines can call
functions without the tag)

But thankfully, rereading include/qemu/coroutine.h clears it up and
both of my initial thoughts are wrong although the second is closer;
it is yet another definition:

presence of coroutine_fn means this function must NOT be called except
from a coroutine context.  coroutines can call functions with or
without the tag, but if they lack the tag, the coroutine must ensure
the function won't block.

Thus, adding the tag is something we do when writing a function that
obeys coroutine rules and requires a coroutine context to already be
present, and once a function is then relaxed enough to work from both
regular and coroutine functions, we drop the marker.  But we keep the
_co_ in the function name to remind ourselves that it is
coroutine-safe, in addition to normal-safe.

And your patch is doing just that - we have functions that work from
both normal and coroutine context, so we can drop the marker but keep
_co_ in the name.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 1/3] target/ppc: Turn ppc_tlb_invalid_all in a noop

[Bruno Piazera Larsen]

On 10/06/2021 13:46, Lucas Mateus Castro (alqotel) wrote:

The function ppc_tlb_invalid_all is now a no op when compiling without TCG.

Signed-off-by: Lucas Mateus Castro (alqotel) 
---
  target/ppc/mmu_helper.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 1ecb36e85a..e7ba39c9e1 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -825,6 +825,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, 
mmu_ctx_t *ctx,
  return ret;
  }
  
+#ifdef CONFIG_TCG

  static void booke206_flush_tlb(CPUPPCState *env, int flags,
 const int check_iprot)
  {
@@ -846,6 +847,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
  
  tlb_flush(env_cpu(env));

  }
+#endif
I think you could use ATTRIBUTE_UNUSED instead of ifdefs here. Not sure 
which would be preferable in this case, but IMHO unused is a bit more 
informative as to what is happening...
  
  static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,

  ppcmas_tlb_t *tlb)
@@ -1956,6 +1958,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t nr, 
target_ulong value)
  /* TLB management */
  void ppc_tlb_invalidate_all(CPUPPCState *env)
  {
+#ifdef CONFIG_TCG
  #if defined(TARGET_PPC64)
  if (mmu_is_64bit(env->mmu_model)) {
  env->tlb_need_flush = 0;
@@ -1994,6 +1997,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
  cpu_abort(env_cpu(env), "Unknown MMU model %x\n", env->mmu_model);
  break;
  }
+#endif
  }
  
  #ifdef CONFIG_TCG

--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO 


Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer 

[Bug 1913919] Re: Heap-buffer-overflow in sdhci_write_dataport

[Thomas Huth]

Ok, thanks for confirmation, then let's close this ticket here. If you
encounter such a problem again, please open a new ticket in the new
gitlab issue tracker.

** 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/1913919

Title:
  Heap-buffer-overflow in sdhci_write_dataport

Status in QEMU:
  Fix Released

Bug description:
  Reproducer:
  cat << EOF | ./qemu-system-aarch64 -qtest stdio \
  -machine raspi3b,accel=qtest -m 1G 
  write 0x3f30002c 0x1 0x25
  write 0x3f34 0x1 0x01
  write 0x3f36 0x1 0xc0
  write 0x3f3c 0x1 0x22
  write 0x3f3e 0x1 0x20
  write 0x3f3f 0x1 0x0
  write 0x3f30 0x1 0x48
  write 0x3f33 0x1 0x0
  write 0x3f35 0x1 0x14
  write 0x3f37 0x1 0x10
  write 0x3f3c 0x1 0x32
  write 0x3f3f 0x1 0x0
  write 0x3f31 0x1 0x00
  write 0x3f32 0x1 0x30
  write 0x3f33 0x1 0x3f
  EOF

  Stacktrace:
  ==654080==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x61917b80 at pc 0x562988348719 bp 0x7fffd26552d0 sp 0x7fffd26552c8
  WRITE of size 1 at 0x61917b80 thread T0
  #0 0x562988348718 in sdhci_write_dataport 
/home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:560:39
  #1 0x562988348718 in sdhci_write 
/home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1172:13
  #2 0x5629890591fe in memory_region_write_accessor 
/home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5
  #3 0x562989058bfb in access_with_adjusted_size 
/home/alxndr/Development/qemu/build/../softmmu/memory.c:552:18
  #4 0x562989058467 in memory_region_dispatch_write 
/home/alxndr/Development/qemu/build/../softmmu/memory.c
  #5 0x5629893e8ffb in flatview_write_continue 
/home/alxndr/Development/qemu/build/../softmmu/physmem.c:2759:23
  #6 0x5629893de71b in flatview_write 
/home/alxndr/Development/qemu/build/../softmmu/physmem.c:2799:14
  #7 0x5629893de71b in address_space_write 
/home/alxndr/Development/qemu/build/../softmmu/physmem.c:2891:18
  #8 0x562988334d9c in dma_memory_rw_relaxed 
/home/alxndr/Development/qemu/include/sysemu/dma.h:88:12
  #9 0x562988334d9c in dma_memory_rw 
/home/alxndr/Development/qemu/include/sysemu/dma.h:127:12
  #10 0x562988334d9c in dma_memory_write 
/home/alxndr/Development/qemu/include/sysemu/dma.h:163:12
  #11 0x562988334d9c in sdhci_sdma_transfer_multi_blocks 
/home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:617:13
  #12 0x56298834427f in sdhci_write 
/home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1129:17
  #13 0x5629890591fe in memory_region_write_accessor 
/home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5
  #14 0x562989058bfb in access_with_adjusted_size 
/home/alxndr/Development/qemu/build/../softmmu/memory.c:552:18
  #15 0x562989058467 in memory_region_dispatch_write 
/home/alxndr/Development/qemu/build/../softmmu/memory.c
  #16 0x5629893e8ffb in flatview_write_continue 
/home/alxndr/Development/qemu/build/../softmmu/physmem.c:2759:23
  #17 0x5629893de71b in flatview_write 
/home/alxndr/Development/qemu/build/../softmmu/physmem.c:2799:14
  #18 0x5629893de71b in address_space_write 
/home/alxndr/Development/qemu/build/../softmmu/physmem.c:2891:18
  #19 0x56298904ad35 in qtest_process_command 
/home/alxndr/Development/qemu/build/../softmmu/qtest.c:654:9
  #20 0x562989043b97 in qtest_process_inbuf 
/home/alxndr/Development/qemu/build/../softmmu/qtest.c:797:9
  #21 0x562989894286 in fd_chr_read 
/home/alxndr/Development/qemu/build/../chardev/char-fd.c:68:9
  #22 0x7f535645baae in g_main_context_dispatch 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51aae)
  #23 0x562989eef363 in glib_pollfds_poll 
/home/alxndr/Development/qemu/build/../util/main-loop.c:232:9
  #24 0x562989eef363 in os_host_main_loop_wait 
/home/alxndr/Development/qemu/build/../util/main-loop.c:255:5
  #25 0x562989eef363 in main_loop_wait 
/home/alxndr/Development/qemu/build/../util/main-loop.c:531:11
  #26 0x562988faa599 in qemu_main_loop 
/home/alxndr/Development/qemu/build/../softmmu/runstate.c:721:9
  #27 0x5629872371fd in main 
/home/alxndr/Development/qemu/build/../softmmu/main.c:50:5
  #28 0x7f5355f00cc9 in __libc_start_main csu/../csu/libc-start.c:308:16
  #29 0x56298718abc9 in _start 
(/home/alxndr/Development/qemu/build/qemu-system-aarch64+0x3350bc9)

  0x61917b80 is located 0 bytes to the right of 1024-byte region 
[0x61917780,0x61917b80)
  allocated by thread T0 here:
  #0 0x562987204db2 in calloc 
(/home/alxndr/Development/qemu/build/qemu-system-aarch64+0x33cadb2)
  #1 0x7f5356461ae0 in g_malloc0 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57ae0)
  #2 0x56298834a187 in sdhci_sysbus_realize 
/home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1469:5
  #3 0x56298987fe77 in device_set_realized 
/home/alxndr/Development/qemu/buil

Re: [PATCH v2 3/3] target/ppc: moved ppc_store_sdr1 to mmu_common.c

[Bruno Piazera Larsen]

On 10/06/2021 13:46, Lucas Mateus Castro (alqotel) wrote:

Moved ppc_store_sdr1 to mmu_common.c as it was originally in
mmu_helper.c.

Signed-off-by: Lucas Mateus Castro (alqotel) 
---
  target/ppc/cpu.c| 28 
  target/ppc/mmu_common.c | 28 
  2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 19d67b5b07..7ad9bd6044 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -67,34 +67,6 @@ uint32_t ppc_get_vscr(CPUPPCState *env)
  return env->vscr | (sat << VSCR_SAT);
  }
  
-#ifdef CONFIG_SOFTMMU

-void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
-{
-PowerPCCPU *cpu = env_archcpu(env);
-qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__, value);
-assert(!cpu->vhyp);
-#if defined(TARGET_PPC64)
-if (mmu_is_64bit(env->mmu_model)) {
-target_ulong sdr_mask = SDR_64_HTABORG | SDR_64_HTABSIZE;
-target_ulong htabsize = value & SDR_64_HTABSIZE;
-
-if (value & ~sdr_mask) {
-qemu_log_mask(LOG_GUEST_ERROR, "Invalid bits 0x"TARGET_FMT_lx
- " set in SDR1", value & ~sdr_mask);
-value &= sdr_mask;
-}
-if (htabsize > 28) {
-qemu_log_mask(LOG_GUEST_ERROR, "Invalid HTABSIZE 0x" TARGET_FMT_lx
- " stored in SDR1", htabsize);
-return;
-}
-}
-#endif /* defined(TARGET_PPC64) */
-/* FIXME: Should check for valid HTABMASK values in 32-bit case */
-env->spr[SPR_SDR1] = value;
-}
-#endif /* CONFIG_SOFTMMU */
-
  /* GDBstub can read and write MSR... */
  void ppc_store_msr(CPUPPCState *env, target_ulong value)
  {
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 93e7b8f955..54c6a7ac6f 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -58,6 +58,34 @@
  #  define LOG_BATS(...) do { } while (0)
  #endif
  
+#ifdef CONFIG_SOFTMMU

+void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
+{
+PowerPCCPU *cpu = env_archcpu(env);
+qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__, value);
+assert(!cpu->vhyp);
+#if defined(TARGET_PPC64)
+if (mmu_is_64bit(env->mmu_model)) {
+target_ulong sdr_mask = SDR_64_HTABORG | SDR_64_HTABSIZE;
+target_ulong htabsize = value & SDR_64_HTABSIZE;
+
+if (value & ~sdr_mask) {
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid bits 0x"TARGET_FMT_lx
+ " set in SDR1", value & ~sdr_mask);
+value &= sdr_mask;
+}
+if (htabsize > 28) {
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid HTABSIZE 0x" TARGET_FMT_lx
+ " stored in SDR1", htabsize);
+return;
+}
+}
+#endif /* defined(TARGET_PPC64) */
+/* FIXME: Should check for valid HTABMASK values in 32-bit case */
+env->spr[SPR_SDR1] = value;
+}
+#endif /* CONFIG_SOFTMMU */
+
  
/*/
  /* PowerPC MMU emulation */
  


Maybe just a nit, but if this file is being compiled, CONFIG_SOFTMMU 
will be defined. This ifdef here is redundant. I only added because it 
was needed for cpu.c


Other than that, LGTM

--
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO 


Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer 

[PATCH v2 2/3] target/ppc: divided mmu_helper.c in 2 files

[Lucas Mateus Castro (alqotel)]

Moved functions in mmu_helper.c that should be compiled in build to
mmu_common.c, moved declaration of functions that both files use to
cpu.h and moved struct declarations and inline functions needed by
both to target/ppc/internal.h. Updated meson.build to compile the
new file. ppc6xx_tlb_getnum is not an inline function anymore.

Signed-off-by: Lucas Mateus Castro (alqotel) 
---
 target/ppc/cpu.h|   21 +
 target/ppc/internal.h   |   26 +
 target/ppc/meson.build  |6 +-
 target/ppc/mmu_common.c | 1623 ++
 target/ppc/mmu_helper.c | 1814 +++
 5 files changed, 1777 insertions(+), 1713 deletions(-)
 create mode 100644 target/ppc/mmu_common.c

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b4de0db7ff..0804acc6f2 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1330,6 +1330,27 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
 void ppc_tlb_invalidate_all(CPUPPCState *env);
 void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
 void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
+
+typedef struct mmu_ctx_t mmu_ctx_t;
+int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
+hwaddr *raddrp,
+target_ulong address, uint32_t pid, int ext,
+int i);
+int get_physical_address_wtlb(CPUPPCState *env, mmu_ctx_t *ctx,
+ target_ulong eaddr,
+ MMUAccessType access_type, int type,
+ int mmu_idx);
+hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
+ppcmas_tlb_t *tlb);
+int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
+hwaddr *raddrp, target_ulong address,
+uint32_t pid);
+int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
+target_ulong eaddr, MMUAccessType access_type,
+int type);
+int ppc6xx_tlb_getnum(CPUPPCState *env, target_ulong eaddr,
+int way, int is_code);
+
 #endif
 #endif
 
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index f1fd3c8d04..ec21a1fbfd 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -245,4 +245,30 @@ static inline int prot_for_access_type(MMUAccessType 
access_type)
 g_assert_not_reached();
 }
 
+/* Context used internally during MMU translations */
+
+struct mmu_ctx_t {
+hwaddr raddr;  /* Real address  */
+hwaddr eaddr;  /* Effective address */
+int prot;  /* Protection bits   */
+hwaddr hash[2];/* Pagetable hash values */
+target_ulong ptem; /* Virtual segment ID | API  */
+int key;   /* Access key*/
+int nx;/* Non-execute area  */
+};
+
+/* Common routines used by software and hardware TLBs emulation */
+static inline int pte_is_valid(target_ulong pte0)
+{
+return pte0 & 0x8000 ? 1 : 0;
+}
+
+static inline void pte_invalidate(target_ulong *pte0)
+{
+*pte0 &= ~0x8000;
+}
+
+#define PTE_PTEM_MASK 0x7FBF
+#define PTE_CHECK_MASK (TARGET_PAGE_MASK | 0x7B)
+
 #endif /* PPC_INTERNAL_H */
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index a4f18ff414..03933b666f 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -37,10 +37,12 @@ ppc_softmmu_ss.add(files(
   'arch_dump.c',
   'machine.c',
   'mmu-hash32.c',
-  'mmu_helper.c',
+  'mmu_common.c',
   'monitor.c',
 ))
-ppc_softmmu_ss.add(when: 'CONFIG_TCG', if_false: files(
+ppc_softmmu_ss.add(when: 'CONFIG_TCG', if_true: files(
+  'mmu_helper.c',
+), if_false: files(
   'tcg-stub.c'
 ))
 
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
new file mode 100644
index 00..93e7b8f955
--- /dev/null
+++ b/target/ppc/mmu_common.c
@@ -0,0 +1,1623 @@
+/*
+ *  PowerPC CPU initialization for qemu.
+ *
+ *  Copyright (C) 2021 Instituto de Pesquisas Eldorado (eldorado.org.br)
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "cpu.h"
+#include "sysemu/kvm.h"
+#include "kvm_ppc.h"
+#inc

[PATCH v2 3/3] target/ppc: moved ppc_store_sdr1 to mmu_common.c

[Lucas Mateus Castro (alqotel)]

Moved ppc_store_sdr1 to mmu_common.c as it was originally in
mmu_helper.c.

Signed-off-by: Lucas Mateus Castro (alqotel) 
---
 target/ppc/cpu.c| 28 
 target/ppc/mmu_common.c | 28 
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 19d67b5b07..7ad9bd6044 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -67,34 +67,6 @@ uint32_t ppc_get_vscr(CPUPPCState *env)
 return env->vscr | (sat << VSCR_SAT);
 }
 
-#ifdef CONFIG_SOFTMMU
-void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
-{
-PowerPCCPU *cpu = env_archcpu(env);
-qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__, value);
-assert(!cpu->vhyp);
-#if defined(TARGET_PPC64)
-if (mmu_is_64bit(env->mmu_model)) {
-target_ulong sdr_mask = SDR_64_HTABORG | SDR_64_HTABSIZE;
-target_ulong htabsize = value & SDR_64_HTABSIZE;
-
-if (value & ~sdr_mask) {
-qemu_log_mask(LOG_GUEST_ERROR, "Invalid bits 0x"TARGET_FMT_lx
- " set in SDR1", value & ~sdr_mask);
-value &= sdr_mask;
-}
-if (htabsize > 28) {
-qemu_log_mask(LOG_GUEST_ERROR, "Invalid HTABSIZE 0x" TARGET_FMT_lx
- " stored in SDR1", htabsize);
-return;
-}
-}
-#endif /* defined(TARGET_PPC64) */
-/* FIXME: Should check for valid HTABMASK values in 32-bit case */
-env->spr[SPR_SDR1] = value;
-}
-#endif /* CONFIG_SOFTMMU */
-
 /* GDBstub can read and write MSR... */
 void ppc_store_msr(CPUPPCState *env, target_ulong value)
 {
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 93e7b8f955..54c6a7ac6f 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -58,6 +58,34 @@
 #  define LOG_BATS(...) do { } while (0)
 #endif
 
+#ifdef CONFIG_SOFTMMU
+void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
+{
+PowerPCCPU *cpu = env_archcpu(env);
+qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__, value);
+assert(!cpu->vhyp);
+#if defined(TARGET_PPC64)
+if (mmu_is_64bit(env->mmu_model)) {
+target_ulong sdr_mask = SDR_64_HTABORG | SDR_64_HTABSIZE;
+target_ulong htabsize = value & SDR_64_HTABSIZE;
+
+if (value & ~sdr_mask) {
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid bits 0x"TARGET_FMT_lx
+ " set in SDR1", value & ~sdr_mask);
+value &= sdr_mask;
+}
+if (htabsize > 28) {
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid HTABSIZE 0x" TARGET_FMT_lx
+ " stored in SDR1", htabsize);
+return;
+}
+}
+#endif /* defined(TARGET_PPC64) */
+/* FIXME: Should check for valid HTABMASK values in 32-bit case */
+env->spr[SPR_SDR1] = value;
+}
+#endif /* CONFIG_SOFTMMU */
+
 /*/
 /* PowerPC MMU emulation */
 
-- 
2.17.1

[PATCH v2 0/3] target/ppc: mmu cleanup

[Lucas Mateus Castro (alqotel)]

This patch series aims to clean up some of the code  mmu_helper.c,
including removing the #includes inside ifdef.

Helpers are in mmu_helper.c now and code that is needed in a !TCG build
is in mmu_common.c.

Comments are welcome, thanks,
Lucas Mateus.
Based-on: 6f398e533f5e259b4f937f4aa9de970f7201d166 

Lucas Mateus Castro (alqotel) (3):
  target/ppc: Turn ppc_tlb_invalid_all in a noop
  target/ppc: divided mmu_helper.c in 2 files
  target/ppc: moved ppc_store_sdr1 to mmu_common.c

 target/ppc/cpu.c|   28 -
 target/ppc/cpu.h|   21 +
 target/ppc/internal.h   |   26 +
 target/ppc/meson.build  |6 +-
 target/ppc/mmu_common.c | 1651 +++
 target/ppc/mmu_helper.c | 1606 +
 6 files changed, 1703 insertions(+), 1635 deletions(-)
 create mode 100644 target/ppc/mmu_common.c

-- 
2.17.1

[PATCH v2 1/3] target/ppc: Turn ppc_tlb_invalid_all in a noop

[Lucas Mateus Castro (alqotel)]

The function ppc_tlb_invalid_all is now a no op when compiling without TCG.

Signed-off-by: Lucas Mateus Castro (alqotel) 
---
 target/ppc/mmu_helper.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 1ecb36e85a..e7ba39c9e1 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -825,6 +825,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, 
mmu_ctx_t *ctx,
 return ret;
 }
 
+#ifdef CONFIG_TCG
 static void booke206_flush_tlb(CPUPPCState *env, int flags,
const int check_iprot)
 {
@@ -846,6 +847,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
 
 tlb_flush(env_cpu(env));
 }
+#endif
 
 static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
 ppcmas_tlb_t *tlb)
@@ -1956,6 +1958,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t nr, 
target_ulong value)
 /* TLB management */
 void ppc_tlb_invalidate_all(CPUPPCState *env)
 {
+#ifdef CONFIG_TCG
 #if defined(TARGET_PPC64)
 if (mmu_is_64bit(env->mmu_model)) {
 env->tlb_need_flush = 0;
@@ -1994,6 +1997,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
 cpu_abort(env_cpu(env), "Unknown MMU model %x\n", env->mmu_model);
 break;
 }
+#endif
 }
 
 #ifdef CONFIG_TCG
-- 
2.17.1

[PATCH] tcg: Fix documentation for tcg_constant_* vs tcg_temp_free_*

[Richard Henderson]

At some point during the development of tcg_constant_*, I changed
my mind about whether such temps should be able to be passed to
tcg_temp_free_*.  The final version committed allows this, but the
commentary was not updated to match.

Fixes: c0522136adf
Reported-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 include/tcg/tcg.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 1d056ed0ed..064dab383b 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -1095,7 +1095,8 @@ TCGv_vec tcg_const_ones_vec_matching(TCGv_vec);
 
 /*
  * Locate or create a read-only temporary that is a constant.
- * This kind of temporary need not and should not be freed.
+ * This kind of temporary need not be freed, but for convenience
+ * will be silently ignored by tcg_temp_free_*.
  */
 TCGTemp *tcg_constant_internal(TCGType type, int64_t val);
 
-- 
2.25.1

[PATCH] accel/tcg: Use MiB in tcg_init_machine

[Richard Henderson]

Suggested-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---

This sits in the middle of my "Clean up code_gen_buffer allocation"
patch set.  Alex mentioned it during review, and I had already made
the change.

This is the only patch in the set that has not been posted and
reviewed.  Rather than re-posting the entire set, I'm just
sending this one and will queue the whole thing to tcg-next.


r~

---
 accel/tcg/tcg-all.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index e990180c4b..1ee89902c3 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -32,6 +32,7 @@
 #include "qemu/error-report.h"
 #include "qemu/accel.h"
 #include "qapi/qapi-builtin-visit.h"
+#include "qemu/units.h"
 #include "internal.h"
 
 struct TCGState {
@@ -115,7 +116,7 @@ static int tcg_init_machine(MachineState *ms)
 
 page_init();
 tb_htable_init();
-tcg_init(s->tb_size * 1024 * 1024, s->splitwx_enabled);
+tcg_init(s->tb_size * MiB, s->splitwx_enabled);
 
 #if defined(CONFIG_SOFTMMU)
 /*
-- 
2.25.1

Re: [Bug 1913919] Re: Heap-buffer-overflow in sdhci_write_dataport

[Alexander Bulekov]

There were a few patches some months ago that fixed the sdhci issues,
and OSS-Fuzz said that all of the heap-overflows that it has seen in
sdhci have been fixed.
-Alex

On 210610 1544, Thomas Huth wrote:
> Can you still reproduce this issue with the latest git version of QEMU?
> ... for me, it does not crash anymore.
> 
> ** Changed in: qemu
>Status: New => Incomplete
> 
> -- 
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1913919
> 
> Title:
>   Heap-buffer-overflow in sdhci_write_dataport
> 
> Status in QEMU:
>   Incomplete
> 
> Bug description:
>   Reproducer:
>   cat << EOF | ./qemu-system-aarch64 -qtest stdio \
>   -machine raspi3b,accel=qtest -m 1G 
>   write 0x3f30002c 0x1 0x25
>   write 0x3f34 0x1 0x01
>   write 0x3f36 0x1 0xc0
>   write 0x3f3c 0x1 0x22
>   write 0x3f3e 0x1 0x20
>   write 0x3f3f 0x1 0x0
>   write 0x3f30 0x1 0x48
>   write 0x3f33 0x1 0x0
>   write 0x3f35 0x1 0x14
>   write 0x3f37 0x1 0x10
>   write 0x3f3c 0x1 0x32
>   write 0x3f3f 0x1 0x0
>   write 0x3f31 0x1 0x00
>   write 0x3f32 0x1 0x30
>   write 0x3f33 0x1 0x3f
>   EOF
> 
>   Stacktrace:
>   ==654080==ERROR: AddressSanitizer: heap-buffer-overflow on address 
> 0x61917b80 at pc 0x562988348719 bp 0x7fffd26552d0 sp 0x7fffd26552c8
>   WRITE of size 1 at 0x61917b80 thread T0
>   #0 0x562988348718 in sdhci_write_dataport 
> /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:560:39
>   #1 0x562988348718 in sdhci_write 
> /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1172:13
>   #2 0x5629890591fe in memory_region_write_accessor 
> /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5
>   #3 0x562989058bfb in access_with_adjusted_size 
> /home/alxndr/Development/qemu/build/../softmmu/memory.c:552:18
>   #4 0x562989058467 in memory_region_dispatch_write 
> /home/alxndr/Development/qemu/build/../softmmu/memory.c
>   #5 0x5629893e8ffb in flatview_write_continue 
> /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2759:23
>   #6 0x5629893de71b in flatview_write 
> /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2799:14
>   #7 0x5629893de71b in address_space_write 
> /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2891:18
>   #8 0x562988334d9c in dma_memory_rw_relaxed 
> /home/alxndr/Development/qemu/include/sysemu/dma.h:88:12
>   #9 0x562988334d9c in dma_memory_rw 
> /home/alxndr/Development/qemu/include/sysemu/dma.h:127:12
>   #10 0x562988334d9c in dma_memory_write 
> /home/alxndr/Development/qemu/include/sysemu/dma.h:163:12
>   #11 0x562988334d9c in sdhci_sdma_transfer_multi_blocks 
> /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:617:13
>   #12 0x56298834427f in sdhci_write 
> /home/alxndr/Development/qemu/build/../hw/sd/sdhci.c:1129:17
>   #13 0x5629890591fe in memory_region_write_accessor 
> /home/alxndr/Development/qemu/build/../softmmu/memory.c:491:5
>   #14 0x562989058bfb in access_with_adjusted_size 
> /home/alxndr/Development/qemu/build/../softmmu/memory.c:552:18
>   #15 0x562989058467 in memory_region_dispatch_write 
> /home/alxndr/Development/qemu/build/../softmmu/memory.c
>   #16 0x5629893e8ffb in flatview_write_continue 
> /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2759:23
>   #17 0x5629893de71b in flatview_write 
> /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2799:14
>   #18 0x5629893de71b in address_space_write 
> /home/alxndr/Development/qemu/build/../softmmu/physmem.c:2891:18
>   #19 0x56298904ad35 in qtest_process_command 
> /home/alxndr/Development/qemu/build/../softmmu/qtest.c:654:9
>   #20 0x562989043b97 in qtest_process_inbuf 
> /home/alxndr/Development/qemu/build/../softmmu/qtest.c:797:9
>   #21 0x562989894286 in fd_chr_read 
> /home/alxndr/Development/qemu/build/../chardev/char-fd.c:68:9
>   #22 0x7f535645baae in g_main_context_dispatch 
> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51aae)
>   #23 0x562989eef363 in glib_pollfds_poll 
> /home/alxndr/Development/qemu/build/../util/main-loop.c:232:9
>   #24 0x562989eef363 in os_host_main_loop_wait 
> /home/alxndr/Development/qemu/build/../util/main-loop.c:255:5
>   #25 0x562989eef363 in main_loop_wait 
> /home/alxndr/Development/qemu/build/../util/main-loop.c:531:11
>   #26 0x562988faa599 in qemu_main_loop 
> /home/alxndr/Development/qemu/build/../softmmu/runstate.c:721:9
>   #27 0x5629872371fd in main 
> /home/alxndr/Development/qemu/build/../softmmu/main.c:50:5
>   #28 0x7f5355f00cc9 in __libc_start_main csu/../csu/libc-start.c:308:16
>   #29 0x56298718abc9 in _start 
> (/home/alxndr/Development/qemu/build/qemu-system-aarch64+0x3350bc9)
> 
>   0x61917b80 is located 0 bytes to the right of 1024-byte region 
> [0x61917780,0x61917b80)
>   allocated by thread T0 here:
>   #0 0x562987204db2 in calloc 
> (/home/alxndr/Develo