Re: [PATCH] hw/net/net_tx_pkt: Fix virtio header without checksum offloading

2024-03-26 Thread Akihiko Odaki

On 2024/03/26 15:51, Jason Wang wrote:

On Sun, Mar 24, 2024 at 4:32 PM Akihiko Odaki  wrote:


It is incorrect to have the VIRTIO_NET_HDR_F_NEEDS_CSUM set when
checksum offloading is disabled so clear the bit. Set the
VIRTIO_NET_HDR_F_DATA_VALID bit instead to tell the checksum is valid.

TCP/UDP checksum is usually offloaded when the peer requires virtio
headers because they can instruct the peer to compute checksum. However,
igb disables TX checksum offloading when a VF is enabled whether the
peer requires virtio headers because a transmitted packet can be routed
to it and it expects the packet has a proper checksum. Therefore, it
is necessary to have a correct virtio header even when checksum
offloading is disabled.

A real TCP/UDP checksum will be computed and saved in the buffer when
checksum offloading is disabled. The virtio specification requires to
set the packet checksum stored in the buffer to the TCP/UDP pseudo
header when the VIRTIO_NET_HDR_F_NEEDS_CSUM bit is set so the bit must
be cleared in that case.

The VIRTIO_NET_HDR_F_NEEDS_CSUM bit also tells to skip checksum
validation. Even if checksum offloading is disabled, it is desirable to
skip checksum validation because the checksum is always correct. Use the
VIRTIO_NET_HDR_F_DATA_VALID bit to claim the validity of the checksum.

Fixes: ffbd2dbd8e64 ("e1000e: Perform software segmentation for loopback")
Buglink: https://issues.redhat.com/browse/RHEL-23067
Signed-off-by: Akihiko Odaki 
---
  hw/net/net_tx_pkt.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index 2e5f58b3c9cc..c225cf706513 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -833,6 +833,9 @@ bool net_tx_pkt_send_custom(struct NetTxPkt *pkt, bool 
offload,

  if (offload || gso_type == VIRTIO_NET_HDR_GSO_NONE) {
  if (!offload && pkt->virt_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+pkt->virt_hdr.flags =
+(pkt->virt_hdr.flags & ~VIRTIO_NET_HDR_F_NEEDS_CSUM) |
+VIRTIO_NET_HDR_F_DATA_VALID;


Why VIRTIO_NET_HDR_F_DATA_VALID is used in TX path?


On igb, a packet sent from a PCI function may be routed to another 
function. The virtio header updated here will be directly provided to 
the RX path in such a case.


Regards,
Akihiko Odaki



Thanks


  net_tx_pkt_do_sw_csum(pkt, &pkt->vec[NET_TX_PKT_L2HDR_FRAG],
pkt->payload_frags + 
NET_TX_PKT_PL_START_FRAG - 1,
pkt->payload_len);

---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240324-tx-c57d3c22ad73

Best regards,
--
Akihiko Odaki 







[PULL 09/20] qapi: Fix typo in request-ebpf documentation

2024-03-26 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-7-arm...@redhat.com>
---
 qapi/ebpf.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/ebpf.json b/qapi/ebpf.json
index f413d00154..61359e1c0f 100644
--- a/qapi/ebpf.json
+++ b/qapi/ebpf.json
@@ -51,7 +51,7 @@
 # @request-ebpf:
 #
 # Retrieve an eBPF object that can be loaded with libbpf.  Management
-# applications (g.e. libvirt) may load it and pass file descriptors to
+# applications (e.g. libvirt) may load it and pass file descriptors to
 # QEMU, so they can run running QEMU without BPF capabilities.
 #
 # @id: The ID of the program to return.
-- 
2.44.0




[PULL 12/20] qapi: Don't repeat member type in its documentation text

2024-03-26 Thread Markus Armbruster
Documentation generated for the arguments of MEMORY_FAILURE looks like

"recipient": "MemoryFailureRecipient"
   recipient is defined as "MemoryFailureRecipient".

"action": "MemoryFailureAction"
   action that has been taken.  action is defined as
   "MemoryFailureAction".

"flags": "MemoryFailureFlags"
   flags for MemoryFailureAction.  action is defined as
   "MemoryFailureFlags".

The "action is defined as ..." are redundant.  Drop.

Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-10-arm...@redhat.com>
---
 qapi/run-state.json | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/qapi/run-state.json b/qapi/run-state.json
index 789fc34559..ae084e13a0 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -568,11 +568,9 @@
 #
 # @recipient: recipient is defined as @MemoryFailureRecipient.
 #
-# @action: action that has been taken.  action is defined as
-# @MemoryFailureAction.
+# @action: action that has been taken.
 #
-# @flags: flags for MemoryFailureAction.  action is defined as
-# @MemoryFailureFlags.
+# @flags: flags for MemoryFailureAction.
 #
 # Since: 5.2
 #
-- 
2.44.0




[PULL 15/20] qga/qapi-schema: Refill doc comments to conform to current conventions

2024-03-26 Thread Markus Armbruster
For legibility, wrap text paragraphs so every line is at most 70
characters long.

To check the generated documentation does not change, I compared the
generated HTML before and after this commit with "wdiff -3".  Finds no
differences.  Comparing with diff is not useful, as the refilled
paragraphs are visible there.

Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-13-arm...@redhat.com>
---
 qga/qapi-schema.json | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 9554b566a7..d5af155007 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1220,13 +1220,13 @@
 # @signal: signal number (linux) or unhandled exception code (windows)
 # if the process was abnormally terminated.
 #
-# @out-data: base64-encoded stdout of the process. This field will only
-# be populated after the process exits.
+# @out-data: base64-encoded stdout of the process.  This field will
+# only be populated after the process exits.
 #
-# @err-data: base64-encoded stderr of the process. Note: @out-data and
-# @err-data are present only if 'capture-output' was specified for
-# 'guest-exec'. This field will only be populated after the process
-# exits.
+# @err-data: base64-encoded stderr of the process.  Note: @out-data
+# and @err-data are present only if 'capture-output' was specified
+# for 'guest-exec'.  This field will only be populated after the
+# process exits.
 #
 # @out-truncated: true if stdout was not fully captured due to size
 # limitation.
@@ -1273,12 +1273,16 @@
 # An enumeration of guest-exec capture modes.
 #
 # @none: do not capture any output
+#
 # @stdout: only capture stdout
+#
 # @stderr: only capture stderr
+#
 # @separated: capture both stdout and stderr, but separated into
-# GuestExecStatus out-data and err-data, respectively
-# @merged: capture both stdout and stderr, but merge together
-#  into out-data. not effective on windows guests.
+# GuestExecStatus out-data and err-data, respectively
+#
+# @merged: capture both stdout and stderr, but merge together into
+# out-data.  Not effective on windows guests.
 #
 # Since: 8.0
 ##
@@ -1291,8 +1295,9 @@
 #
 # Controls what guest-exec output gets captures.
 #
-# @flag: captures both stdout and stderr if true. Equivalent
-#to GuestExecCaptureOutputMode::all. (since 2.5)
+# @flag: captures both stdout and stderr if true.  Equivalent to
+# GuestExecCaptureOutputMode::all.  (since 2.5)
+#
 # @mode: capture mode; preferred interface
 #
 # Since: 8.0
@@ -1315,7 +1320,7 @@
 # @input-data: data to be passed to process stdin (base64 encoded)
 #
 # @capture-output: bool flag to enable capture of stdout/stderr of
-# running process.  defaults to false.
+# running process.  Defaults to false.
 #
 # Returns: PID
 #
-- 
2.44.0




[PULL 03/20] qapi: Fix bogus documentation of query-migrationthreads

2024-03-26 Thread Markus Armbruster
The doc comment documents an argument that doesn't exist.  Would
fail compilation if it was marked up correctly.  Delete.

The Returns: section fails to refer to the data type, leaving the user
to guess.  Fix that.

The command name violates QAPI naming rules: it should be
query-migration-threads.  Too late to fix.

Reported-by: John Snow 
Fixes: 671326201dac (migration: Introduce interface query-migrationthreads)
Signed-off-by: Markus Armbruster 
Message-ID: <20240322135117.195489-4-arm...@redhat.com>
Reviewed-by: Fabiano Rosas 
Reviewed-by: Peter Xu 
Reviewed-by: John Snow 
---
 qapi/migration.json | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 744d05f364..c865ab00c8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -2419,9 +2419,7 @@
 #
 # Returns information of migration threads
 #
-# data: migration thread name
-#
-# Returns: information about migration threads
+# Returns: @MigrationThreadInfo
 #
 # Since: 7.2
 ##
-- 
2.44.0




[PULL 11/20] qapi: Start sentences with a capital letter, end them with a period

2024-03-26 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-9-arm...@redhat.com>
---
 qapi/migration.json | 16 
 qapi/ui.json|  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index a4319f87bf..8fa1b7f8ed 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -829,8 +829,8 @@
 # and recreated on the fly while the migration server is active.
 # If missing, it will default to denying access (Since 4.0)
 #
-# @max-bandwidth: to set maximum speed for migration.  maximum speed
-# in bytes per second.  (Since 2.8)
+# @max-bandwidth: maximum speed for migration, in bytes per second.
+# (Since 2.8)
 #
 # @avail-switchover-bandwidth: to set the available bandwidth that
 # migration can use during switchover phase.  NOTE!  This does not
@@ -1036,8 +1036,8 @@
 # and recreated on the fly while the migration server is active.
 # If missing, it will default to denying access (Since 4.0)
 #
-# @max-bandwidth: to set maximum speed for migration.  maximum speed
-# in bytes per second.  (Since 2.8)
+# @max-bandwidth: maximum speed for migration, in bytes per second.
+# (Since 2.8)
 #
 # @avail-switchover-bandwidth: to set the available bandwidth that
 # migration can use during switchover phase.  NOTE!  This does not
@@ -1267,8 +1267,8 @@
 # control checking of the TLS x509 certificate distinguished name.
 # (Since 4.0)
 #
-# @max-bandwidth: to set maximum speed for migration.  maximum speed
-# in bytes per second.  (Since 2.8)
+# @max-bandwidth: maximum speed for migration, in bytes per second.
+# (Since 2.8)
 #
 # @avail-switchover-bandwidth: to set the available bandwidth that
 # migration can use during switchover phase.  NOTE!  This does not
@@ -1960,8 +1960,8 @@
 #
 # @primary: true for primary or false for secondary.
 #
-# @failover: true to do failover, false to stop.  but cannot be
-# specified if 'enable' is true.  default value is false.
+# @failover: true to do failover, false to stop.  Cannot be specified
+# if 'enable' is true.  Default value is false.
 #
 # Example:
 #
diff --git a/qapi/ui.json b/qapi/ui.json
index 5744c24e3c..e71cd2f50b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -290,7 +290,7 @@
 # @enabled: true if the SPICE server is enabled, false otherwise
 #
 # @migrated: true if the last guest migration completed and spice
-# migration had completed as well.  false otherwise.  (since 1.4)
+# migration had completed as well, false otherwise (since 1.4)
 #
 # @host: The hostname the SPICE server is bound to.  This depends on
 # the name resolution on the host and may be an IP address.
-- 
2.44.0




[PULL 06/20] qapi: Tidy up block-latency-histogram-set documentation some more

2024-03-26 Thread Markus Armbruster
Commit a937b6aa739 (qapi: Reformat doc comments to conform to current
conventions) reflowed some text that should have been left alone.
Revert that.

Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-4-arm...@redhat.com>
---
 qapi/block.json | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/qapi/block.json b/qapi/block.json
index 65d9804bdf..2410145cd3 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -545,8 +545,8 @@
 #
 # Example:
 #
-# Set new histograms for all io types with intervals [0, 10), [10,
-# 50), [50, 100), [100, +inf):
+# Set new histograms for all io types with intervals
+# [0, 10), [10, 50), [50, 100), [100, +inf):
 #
 # -> { "execute": "block-latency-histogram-set",
 #  "arguments": { "id": "drive0",
@@ -565,9 +565,9 @@
 #
 # Example:
 #
-# Set new histograms with the following intervals:   read, flush: [0,
-# 10), [10, 50), [50, 100), [100, +inf)   write: [0, 1000), [1000,
-# 5000), [5000, +inf)
+# Set new histograms with the following intervals:
+#   read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
+#   write: [0, 1000), [1000, 5000), [5000, +inf)
 #
 # -> { "execute": "block-latency-histogram-set",
 #  "arguments": { "id": "drive0",
-- 
2.44.0




[PULL 04/20] qapi: Drop stray Arguments: line from qmp_capabilities docs

2024-03-26 Thread Markus Armbruster
Reported-by: John Snow 
Fixes: 119ebac1feb2 (qapi-schema: use generated marshaller for 
'qmp_capabilities')
Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-2-arm...@redhat.com>
Reviewed-by: John Snow 
---
 qapi/control.json | 2 --
 1 file changed, 2 deletions(-)

diff --git a/qapi/control.json b/qapi/control.json
index f404daef60..6bdbf077c2 100644
--- a/qapi/control.json
+++ b/qapi/control.json
@@ -11,8 +11,6 @@
 #
 # Enable QMP capabilities.
 #
-# Arguments:
-#
 # @enable: An optional list of QMPCapability values to enable.  The
 # client must not enable any capability that is not mentioned in
 # the QMP greeting message.  If the field is not provided, it
-- 
2.44.0




[PULL 08/20] qapi: Fix argument markup in drive-mirror documentation

2024-03-26 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-6-arm...@redhat.com>
---
 qapi/block-core.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1874f880a8..64668b080d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2117,7 +2117,7 @@
 # Start mirroring a block device's writes to a new destination.
 # target specifies the target of the new image.  If the file exists,
 # or if it is a device, it will be used as the new destination for
-# writes.  If it does not exist, a new file will be created.  format
+# writes.  If it does not exist, a new file will be created.  @format
 # specifies the format of the mirror image, default is to probe if
 # mode='existing', else the format of the source.
 #
-- 
2.44.0




[PULL 18/20] qapi: document leftover members in qapi/stats.json

2024-03-26 Thread Markus Armbruster
From: Paolo Bonzini 

Suggested-by: Markus Armbruster 
Signed-off-by: Paolo Bonzini 
Message-ID: <20240325104504.1358734-1-pbonz...@redhat.com>
Reviewed-by: Markus Armbruster 
[Update qapi/pragma.json]
Signed-off-by: Markus Armbruster 
---
 qapi/pragma.json |  5 +
 qapi/stats.json  | 14 +-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/qapi/pragma.json b/qapi/pragma.json
index 1a302981c1..99e4052ab3 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -75,8 +75,6 @@
 'Qcow2OverlapCheckFlags',
 'RbdAuthMode',
 'RbdImageEncryptionFormat',
-'StatsFilter',
-'StatsValue',
 'String',
 'StringWrapper',
 'SysEmuTarget',
@@ -91,8 +89,7 @@
 'query-cpu-model-comparison',
 'query-cpu-model-expansion',
 'query-rocker',
-'query-rocker-ports',
-'query-stats-schemas' ],
+'query-rocker-ports' ],
 # Externally visible types whose member names may use uppercase
 'member-name-exceptions': [ # visible in:
 'ACPISlotType', # query-acpi-ospm-status
diff --git a/qapi/stats.json b/qapi/stats.json
index ce9d8161ec..578b52c7ef 100644
--- a/qapi/stats.json
+++ b/qapi/stats.json
@@ -114,13 +114,13 @@
 #
 # The arguments to the query-stats command; specifies a target for
 # which to request statistics and optionally the required subset of
-# information for that target:
+# information for that target.
 #
-# - which vCPUs to request statistics for
-# - which providers to request statistics from
-# - which named values to return within each provider
+# @target: the kind of objects to query.  Note that each possible
+#  target may enable additional filtering options
 #
-# @target: the kind of objects to query
+# @providers: which providers to request statistics from, and optionally
+# which named values to return within each provider
 #
 # Since: 7.1
 ##
@@ -136,6 +136,8 @@
 #
 # @scalar: single unsigned 64-bit integers.
 #
+# @boolean: single boolean value.
+#
 # @list: list of unsigned 64-bit integers (used for histograms).
 #
 # Since: 7.1
@@ -254,6 +256,8 @@
 #
 # Return the schema for all available runtime-collected statistics.
 #
+# @provider: a provider to restrict the query to.
+#
 # Note: runtime-collected statistics and their names fall outside
 # QEMU's usual deprecation policies.  QEMU will try to keep the
 # set of available data stable, together with their names, but
-- 
2.44.0




[PULL 07/20] qapi: Tidy up indentation of add_client's example

2024-03-26 Thread Markus Armbruster
Commit d23055b8db8 (qapi: Require descriptions and tagged sections to
be indented) indented add_client's example too much.  Revert that.

Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-5-arm...@redhat.com>
[Move a stray hunk to the later patch it belongs to]
---
 qapi/misc.json | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qapi/misc.json b/qapi/misc.json
index 1b0c5dad88..83def5edc4 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -32,9 +32,9 @@
 #
 # Example:
 #
-# -> { "execute": "add_client", "arguments": { "protocol": "vnc",
-#  "fdname": "myclient" } }
-# <- { "return": {} }
+# -> { "execute": "add_client", "arguments": { "protocol": "vnc",
+#  "fdname": "myclient" } }
+# <- { "return": {} }
 ##
 { 'command': 'add_client',
   'data': { 'protocol': 'str', 'fdname': 'str', '*skipauth': 'bool',
-- 
2.44.0




[PULL 05/20] qapi: Expand a few awkward abbreviations in documentation

2024-03-26 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-3-arm...@redhat.com>
---
 qapi/replay.json | 4 ++--
 qapi/virtio.json | 8 +---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/qapi/replay.json b/qapi/replay.json
index 8626fb58f4..d3559f9c8f 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -105,8 +105,8 @@
 # replaying the execution.  The command automatically loads nearest
 # snapshot and replays the execution to find the desired instruction.
 # When there is no preceding snapshot or the execution is not
-# replayed, then the command fails.  icount for the reference may be
-# obtained with @query-replay command.
+# replayed, then the command fails.  Instruction count can be obtained
+# with the @query-replay command.
 #
 # @icount: target instruction count
 #
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 95745fdfd7..b0cd41be72 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -642,15 +642,17 @@
 #
 # @num: vhost_virtqueue num
 #
-# @desc-phys: vhost_virtqueue desc_phys (descriptor area phys. addr.)
+# @desc-phys: vhost_virtqueue desc_phys (descriptor area physical
+# address)
 #
 # @desc-size: vhost_virtqueue desc_size
 #
-# @avail-phys: vhost_virtqueue avail_phys (driver area phys. addr.)
+# @avail-phys: vhost_virtqueue avail_phys (driver area physical
+# address)
 #
 # @avail-size: vhost_virtqueue avail_size
 #
-# @used-phys: vhost_virtqueue used_phys (device area phys. addr.)
+# @used-phys: vhost_virtqueue used_phys (device area physical address)
 #
 # @used-size: vhost_virtqueue used_size
 #
-- 
2.44.0




[PULL 13/20] qapi: Refill doc comments to conform to current conventions

2024-03-26 Thread Markus Armbruster
For legibility, wrap text paragraphs so every line is at most 70
characters long.

To check the generated documentation does not change, I compared the
generated HTML before and after this commit with "wdiff -3".  Finds no
differences.  Comparing with diff is not useful, as the refilled
paragraphs are visible there.

Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-11-arm...@redhat.com>
---
 qapi/block-core.json |  24 -
 qapi/block.json  |   4 +-
 qapi/cxl.json|   4 +-
 qapi/dump.json   |  16 +++---
 qapi/ebpf.json   |  12 ++---
 qapi/machine-target.json |  22 +
 qapi/machine.json|  15 +++---
 qapi/migration.json  | 104 ---
 qapi/net.json|  27 +-
 qapi/qom.json|  34 ++---
 qapi/run-state.json  |   4 +-
 qapi/virtio.json |  12 +++--
 12 files changed, 142 insertions(+), 136 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 64668b080d..e6b392ffe7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1819,10 +1819,10 @@
 # Care should be taken when specifying the string, to specify a
 # valid filename or protocol.  (Since 2.1)
 #
-# @backing-mask-protocol: If true, replace any protocol mentioned in the
-# 'backing file format' with 'raw', rather than storing the protocol
-# name as the backing format.  Can be used even when no image header
-# will be updated (default false; since 9.0).
+# @backing-mask-protocol: If true, replace any protocol mentioned in
+# the 'backing file format' with 'raw', rather than storing the
+# protocol name as the backing format.  Can be used even when no
+# image header will be updated (default false; since 9.0).
 #
 # @speed: the maximum speed, in bytes per second
 #
@@ -2825,10 +2825,10 @@
 # Care should be taken when specifying the string, to specify a
 # valid filename or protocol.  (Since 2.1)
 #
-# @backing-mask-protocol: If true, replace any protocol mentioned in the
-# 'backing file format' with 'raw', rather than storing the protocol
-# name as the backing format.  Can be used even when no image header
-# will be updated (default false; since 9.0).
+# @backing-mask-protocol: If true, replace any protocol mentioned in
+# the 'backing file format' with 'raw', rather than storing the
+# protocol name as the backing format.  Can be used even when no
+# image header will be updated (default false; since 9.0).
 #
 # @speed: the maximum speed, in bytes per second
 #
@@ -3547,10 +3547,10 @@
 # re-allocating them later.  Besides potential performance
 # degradation, such fragmentation can lead to increased allocation
 # of clusters past the end of the image file, resulting in image
-# files whose file length can grow much larger than their guest disk
-# size would suggest.  If image file length is of concern (e.g. when
-# storing qcow2 images directly on block devices), you should
-# consider enabling this option.  (since 8.1)
+# files whose file length can grow much larger than their guest
+# disk size would suggest.  If image file length is of concern
+# (e.g. when storing qcow2 images directly on block devices), you
+# should consider enabling this option.  (since 8.1)
 #
 # @overlap-check: which overlap checks to perform for writes to the
 # image, defaults to 'cached' (since 2.2)
diff --git a/qapi/block.json b/qapi/block.json
index 2410145cd3..5de99fe09d 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -555,8 +555,8 @@
 #
 # Example:
 #
-# Set new histogram only for write, other histograms will remain not
-# changed (or not created):
+# Set new histogram only for write, other histograms will remain
+# not changed (or not created):
 #
 # -> { "execute": "block-latency-histogram-set",
 #  "arguments": { "id": "drive0",
diff --git a/qapi/cxl.json b/qapi/cxl.json
index 8cc4c72fa9..4281726dec 100644
--- a/qapi/cxl.json
+++ b/qapi/cxl.json
@@ -144,8 +144,8 @@
 # @cxl-inject-memory-module-event:
 #
 # Inject an event record for a Memory Module Event (CXL r3.0
-# 8.2.9.2.1.3).  This event includes a copy of the Device Health
-# info at the time of the event.
+# 8.2.9.2.1.3).  This event includes a copy of the Device Health info
+# at the time of the event.
 #
 # @path: CXL type 3 device canonical QOM path
 #
diff --git a/qapi/dump.json b/qapi/dump.json
index 4c021dd53c..ef1f3b62fc 100644
--- a/qapi/dump.json
+++ b/qapi/dump.json
@@ -15,20 +15,20 @@
 #
 # @elf: elf format
 #
-# @kdump-zlib: makedumpfile flattened, kdump-compressed format with zlib
-# compression
+# @kdump-zlib: makedumpfile flattened, kdump-compressed format with
+# zlib compression
 #
 # @kdump-lzo: makedumpfile flattened, kdump-compressed format with lzo
 # compression
 #
-# @kdump-snappy: makedumpfile flattened, kdump-compressed format with snappy
-# 

[PULL 02/20] qapi: Resync MigrationParameter and MigrateSetParameters

2024-03-26 Thread Markus Armbruster
Enum MigrationParameter mirrors the members of struct
MigrateSetParameters.  Differences to MigrateSetParameters's member
documentation are pointless.  Clean them up:

* @compress-level, @compress-threads, @decompress-threads, and
  x-checkpoint-delay are more thoroughly documented for
  MigrationParameter, so use that version for both.

* @max-cpu-throttle is almost the same.  Use MigrationParameter's
  version for both.

Signed-off-by: Markus Armbruster 
Message-ID: <20240322135117.195489-3-arm...@redhat.com>
Reviewed-by: Fabiano Rosas 
Reviewed-by: Peter Xu 
---
 qapi/migration.json | 24 +---
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index bebe9f71ba..744d05f364 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -966,16 +966,26 @@
 # @announce-step: Increase in delay (in milliseconds) between
 # subsequent packets in the announcement (Since 4.0)
 #
-# @compress-level: compression level
+# @compress-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0 and 9,
+# where 0 means no compression, 1 means the best compression
+# speed, and 9 means best compression ratio which will consume
+# more CPU.
 #
-# @compress-threads: compression thread count
+# @compress-threads: Set compression thread count to be used in live
+# migration, the compression thread count is an integer between 1
+# and 255.
 #
 # @compress-wait-thread: Controls behavior when all compression
 # threads are currently busy.  If true (default), wait for a free
 # compression thread to become available; otherwise, send the page
 # uncompressed.  (Since 3.1)
 #
-# @decompress-threads: decompression thread count
+# @decompress-threads: Set decompression thread count to be used in
+# live migration, the decompression thread count is an integer
+# between 1 and 255. Usually, decompression is at least 4 times as
+# fast as compression, so set the decompress-threads to the number
+# about 1/4 of compress-threads is adequate.
 #
 # @throttle-trigger-threshold: The ratio of bytes_dirty_period and
 # bytes_xfer_period to trigger throttling.  It is expressed as
@@ -1042,8 +1052,8 @@
 # @downtime-limit: set maximum tolerated downtime for migration.
 # maximum downtime in milliseconds (Since 2.8)
 #
-# @x-checkpoint-delay: the delay time between two COLO checkpoints.
-# (Since 2.8)
+# @x-checkpoint-delay: The delay time (in ms) between two COLO
+# checkpoints in periodic mode.  (Since 2.8)
 #
 # @block-incremental: Affects how much storage is migrated when the
 # block migration capability is enabled.  When false, the entire
@@ -1064,8 +1074,8 @@
 # postcopy.  Defaults to 0 (unlimited).  In bytes per second.
 # (Since 3.0)
 #
-# @max-cpu-throttle: maximum cpu throttle percentage.  The default
-# value is 99. (Since 3.1)
+# @max-cpu-throttle: maximum cpu throttle percentage.  Defaults to 99.
+# (Since 3.1)
 #
 # @multifd-compression: Which compression method to use.  Defaults to
 # none.  (Since 5.0)
-- 
2.44.0




[PULL 14/20] qapi: Correct documentation indentation and whitespace

2024-03-26 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-12-arm...@redhat.com>
[Add a previous patch's stray hunk]
---
 qapi/block-core.json | 20 ++--
 qapi/crypto.json | 12 ++--
 qapi/dump.json   |  2 +-
 qapi/machine.json|  3 +--
 qapi/migration.json  | 38 ++
 qapi/misc.json   |  2 +-
 qapi/qom.json|  4 ++--
 qapi/run-state.json  |  9 -
 qapi/sockets.json|  3 +--
 qapi/ui.json | 14 +++---
 10 files changed, 51 insertions(+), 56 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index e6b392ffe7..7d3fe59f6c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2593,27 +2593,27 @@
 #
 # @bps_max_length: maximum length of the @bps_max burst period, in
 # seconds.  It must only be set if @bps_max is set as well.
-# Defaults to 1. (Since 2.6)
+# Defaults to 1.  (Since 2.6)
 #
 # @bps_rd_max_length: maximum length of the @bps_rd_max burst period,
 # in seconds.  It must only be set if @bps_rd_max is set as well.
-# Defaults to 1. (Since 2.6)
+# Defaults to 1.  (Since 2.6)
 #
 # @bps_wr_max_length: maximum length of the @bps_wr_max burst period,
 # in seconds.  It must only be set if @bps_wr_max is set as well.
-# Defaults to 1. (Since 2.6)
+# Defaults to 1.  (Since 2.6)
 #
 # @iops_max_length: maximum length of the @iops burst period, in
 # seconds.  It must only be set if @iops_max is set as well.
-# Defaults to 1. (Since 2.6)
+# Defaults to 1.  (Since 2.6)
 #
 # @iops_rd_max_length: maximum length of the @iops_rd_max burst
 # period, in seconds.  It must only be set if @iops_rd_max is set
-# as well.  Defaults to 1. (Since 2.6)
+# as well.  Defaults to 1.  (Since 2.6)
 #
 # @iops_wr_max_length: maximum length of the @iops_wr_max burst
 # period, in seconds.  It must only be set if @iops_wr_max is set
-# as well.  Defaults to 1. (Since 2.6)
+# as well.  Defaults to 1.  (Since 2.6)
 #
 # @iops_size: an I/O size in bytes (Since 1.7)
 #
@@ -3354,7 +3354,7 @@
 # decryption key (since 2.6). Mandatory except when doing a
 # metadata-only probe of the image.
 #
-# @header: block device holding a detached LUKS header. (since 9.0)
+# @header: block device holding a detached LUKS header.  (since 9.0)
 #
 # Since: 2.9
 ##
@@ -4619,7 +4619,7 @@
 # seconds for copy-before-write operation.  When a timeout occurs,
 # the respective copy-before-write operation will fail, and the
 # @on-cbw-error parameter will decide how this failure is handled.
-# Default 0. (Since 7.1)
+# Default 0.  (Since 7.1)
 #
 # Since: 6.2
 ##
@@ -4953,9 +4953,9 @@
 # Driver specific image creation options for LUKS.
 #
 # @file: Node to create the image format on, mandatory except when
-#'preallocation' is not requested
+# 'preallocation' is not requested
 #
-# @header: Block device holding a detached LUKS header. (since 9.0)
+# @header: Block device holding a detached LUKS header.  (since 9.0)
 #
 # @size: Size of the virtual disk in bytes
 #
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 931c88e688..e102be337b 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -48,15 +48,15 @@
 #
 # @sha1: SHA-1. Should not be used in any new code, legacy compat only
 #
-# @sha224: SHA-224. (since 2.7)
+# @sha224: SHA-224.  (since 2.7)
 #
 # @sha256: SHA-256. Current recommended strong hash.
 #
-# @sha384: SHA-384. (since 2.7)
+# @sha384: SHA-384.  (since 2.7)
 #
-# @sha512: SHA-512. (since 2.7)
+# @sha512: SHA-512.  (since 2.7)
 #
-# @ripemd160: RIPEMD-160. (since 2.7)
+# @ripemd160: RIPEMD-160.  (since 2.7)
 #
 # Since: 2.6
 ##
@@ -224,9 +224,9 @@
 # 'sha256'
 #
 # @iter-time: number of milliseconds to spend in PBKDF passphrase
-# processing.  Currently defaults to 2000. (since 2.8)
+# processing.  Currently defaults to 2000.  (since 2.8)
 #
-# @detached-header: create a detached LUKS header. (since 9.0)
+# @detached-header: create a detached LUKS header.  (since 9.0)
 #
 # Since: 2.6
 ##
diff --git a/qapi/dump.json b/qapi/dump.json
index ef1f3b62fc..2fa9504d86 100644
--- a/qapi/dump.json
+++ b/qapi/dump.json
@@ -77,7 +77,7 @@
 #
 # @detach: if true, QMP will return immediately rather than waiting
 # for the dump to finish.  The user can track progress using
-# "query-dump". (since 2.6).
+# "query-dump".  (since 2.6).
 #
 # @begin: if specified, the starting physical address.
 #
diff --git a/qapi/machine.json b/qapi/machine.json
index 01be411fa7..e8b60641f2 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -920,7 +920,7 @@
 # @socket-id: socket number within parent container the CPU belongs to
 #
 # @die-id: die number within the parent container the CPU belongs to
-#(since 4.1)
+# (since 4.1)
 #
 # @cluster-id: cluster number within the parent container the CPU
 # belongs to (since 7.1)
@@ -1190,7 +1190,6 @@
 # <- { "event": "HV_BALLOON_STA

[PULL 17/20] qapi: document leftover members in qapi/run-state.json

2024-03-26 Thread Markus Armbruster
From: Paolo Bonzini 

Suggested-by: Markus Armbruster 
Signed-off-by: Paolo Bonzini 
Message-ID: <20240325104502.1358693-1-pbonz...@redhat.com>
Reviewed-by: Markus Armbruster 
[Capitalize "ID", update qapi/pragma.json]
Signed-off-by: Markus Armbruster 
---
 qapi/pragma.json|  4 +---
 qapi/run-state.json | 26 +-
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/qapi/pragma.json b/qapi/pragma.json
index 92715d22b3..1a302981c1 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -57,7 +57,6 @@
 'DummyForceArrays',
 'DummyVirtioForceArrays',
 'GrabToggleKeys',
-'GuestPanicInformationHyperV',
 'HotKeyMod',
 'ImageInfoSpecificKind',
 'InputAxis',
@@ -93,8 +92,7 @@
 'query-cpu-model-expansion',
 'query-rocker',
 'query-rocker-ports',
-'query-stats-schemas',
-'watchdog-set-action' ],
+'query-stats-schemas' ],
 # Externally visible types whose member names may use uppercase
 'member-name-exceptions': [ # visible in:
 'ACPISlotType', # query-acpi-ospm-status
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 5f07444b84..f8773f23b2 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -376,9 +376,17 @@
 ##
 # @watchdog-set-action:
 #
-# Set watchdog action
+# Set watchdog action.
+#
+# @action: @WatchdogAction action taken when watchdog timer expires.
 #
 # Since: 2.11
+#
+# Example:
+#
+# -> { "execute": "watchdog-set-action",
+#  "arguments": { "action": "inject-nmi" } }
+# <- { "return": {} }
 ##
 { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} }
 
@@ -504,6 +512,22 @@
 #
 # Hyper-V specific guest panic information (HV crash MSRs)
 #
+# @arg1: for Windows, STOP code for the guest crash.  For Linux,
+#an error code.
+#
+# @arg2: for Windows, first argument of the STOP.  For Linux, the
+#guest OS ID, which has the kernel version in bits 16-47
+#and 0x8100 in bits 48-63.
+#
+# @arg3: for Windows, second argument of the STOP.  For Linux, the
+#program counter of the guest.
+#
+# @arg4: for Windows, third argument of the STOP.  For Linux, the
+#RAX register (x86) or the stack pointer (aarch64) of the guest.
+#
+# @arg5: for Windows, fourth argument of the STOP.  For x86 Linux, the
+#stack pointer of the guest.
+#
 # Since: 2.9
 ##
 {'struct': 'GuestPanicInformationHyperV',
-- 
2.44.0




[PULL 10/20] qapi: Fix abbreviation punctuation in doc comments

2024-03-26 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-ID: <20240322140910.328840-8-arm...@redhat.com>
---
 qapi/migration.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index c865ab00c8..a4319f87bf 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1773,7 +1773,7 @@
 #default network.
 #
 # 5. For now, number of migration streams is restricted to one,
-#i.e number of items in 'channels' list is just 1.
+#i.e. number of items in 'channels' list is just 1.
 #
 # 6. The 'uri' and 'channels' arguments are mutually exclusive;
 #exactly one of the two should be present.
@@ -1850,7 +1850,7 @@
 # 3. The uri format is the same as for -incoming
 #
 # 4. For now, number of migration streams is restricted to one,
-#i.e number of items in 'channels' list is just 1.
+#i.e. number of items in 'channels' list is just 1.
 #
 # 5. The 'uri' and 'channels' arguments are mutually exclusive;
 #exactly one of the two should be present.
-- 
2.44.0




[PULL 00/20] QAPI patches patches for 2024-03-26

2024-03-26 Thread Markus Armbruster
This pull request does not touch code, only QAPI schema documentation
comments and error-suppressing QAPI schema pragma
documentation-exceptions.

The following changes since commit 6a4180af9686830d88c387baab6d79563ce42a15:

  Merge tag 'pull-request-2024-03-25' of https://gitlab.com/thuth/qemu into 
staging (2024-03-25 14:19:42 +)

are available in the Git repository at:

  https://repo.or.cz/qemu/armbru.git tags/pull-qapi-2024-03-26

for you to fetch changes up to 1a533ce986f52c35f324f5f4fff22cdc2619a47c:

  qapi: document parameters of query-cpu-model-* QAPI commands (2024-03-26 
06:36:08 +0100)


QAPI patches patches for 2024-03-26


David Hildenbrand (1):
  qapi: document parameters of query-cpu-model-* QAPI commands

Marc-André Lureau (1):
  qapi: document InputMultiTouchType

Markus Armbruster (15):
  qapi: Improve migration TLS documentation
  qapi: Resync MigrationParameter and MigrateSetParameters
  qapi: Fix bogus documentation of query-migrationthreads
  qapi: Drop stray Arguments: line from qmp_capabilities docs
  qapi: Expand a few awkward abbreviations in documentation
  qapi: Tidy up block-latency-histogram-set documentation some more
  qapi: Tidy up indentation of add_client's example
  qapi: Fix argument markup in drive-mirror documentation
  qapi: Fix typo in request-ebpf documentation
  qapi: Fix abbreviation punctuation in doc comments
  qapi: Start sentences with a capital letter, end them with a period
  qapi: Don't repeat member type in its documentation text
  qapi: Refill doc comments to conform to current conventions
  qapi: Correct documentation indentation and whitespace
  qga/qapi-schema: Refill doc comments to conform to current conventions

Paolo Bonzini (2):
  qapi: document leftover members in qapi/run-state.json
  qapi: document leftover members in qapi/stats.json

Vladimir Sementsov-Ogievskiy (1):
  qapi/block-core: improve Qcow2OverlapCheckFlags documentation

 qapi/block-core.json |  71 -
 qapi/block.json  |  14 +--
 qapi/control.json|   2 -
 qapi/crypto.json |  12 +--
 qapi/cxl.json|   4 +-
 qapi/dump.json   |  18 ++--
 qapi/ebpf.json   |  14 ++-
 qapi/machine-target.json |  68 -
 qapi/machine.json|  18 ++--
 qapi/migration.json  | 253 ---
 qapi/misc.json   |   8 +-
 qapi/net.json|  27 ++---
 qapi/pragma.json |  13 +--
 qapi/qom.json|  38 +++
 qapi/replay.json |   4 +-
 qapi/run-state.json  |  45 ++---
 qapi/sockets.json|   3 +-
 qapi/stats.json  |  14 ++-
 qapi/ui.json |  28 --
 qapi/virtio.json |  20 ++--
 qga/qapi-schema.json |  29 +++---
 21 files changed, 389 insertions(+), 314 deletions(-)

-- 
2.44.0




[PULL 01/20] qapi: Improve migration TLS documentation

2024-03-26 Thread Markus Armbruster
MigrateSetParameters is about setting parameters, and
MigrationParameters is about querying them.  Their documentation of
@tls-creds and @tls-hostname has residual damage from a failed attempt
at de-duplicating them (see commit de63ab61241 "migrate: Share common
MigrationParameters struct" and commit 1bda8b3c695 "migration: Unshare
MigrationParameters struct for now").

MigrateSetParameters documentation issues:

* It claims plain text mode "was reported by omitting tls-creds"
  before 2.9.  MigrateSetParameters is not used for reporting, so this
  is misleading.  Delete.

* It similarly claims hostname defaulting to migration URI "was
  reported by omitting tls-hostname" before 2.9.  Delete as well.

Rephrase the remaining @tls-hostname contents for clarity.

Enum MigrationParameter mirrors the members of struct
MigrateSetParameters.  Differences to MigrateSetParameters's member
documentation are pointless.  Copy the new text to MigrationParameter.

MigrationParameters documentation issues:

* @tls-creds runs the two last sentences together without punctuation.
  Fix that.

* Much of the contents on @tls-hostname only applies to setting
  parameters, resulting in confusion.  Replace by a suitable abridged
  version of the new MigrateSetParameters text, and a note on
  @tls-hostname omission in 2.8.

Additional damage is due to flawed doc fix commit
66fcb9d651d (qapi/migration: Add missing tls-authz documentation):
since it copied the missing MigrateSetParameters text from
MigrationParameters instead of MigrationParameter, the part on
recreating @tls-authz on the fly is missing.  Copy that, too.

Signed-off-by: Markus Armbruster 
Message-ID: <20240322135117.195489-2-arm...@redhat.com>
Reviewed-by: Peter Xu 
[Some typos corrected]
---
 qapi/migration.json | 63 +++--
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index aa1b39bce1..bebe9f71ba 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -809,16 +809,19 @@
 # for establishing a TLS connection over the migration data
 # channel.  On the outgoing side of the migration, the credentials
 # must be for a 'client' endpoint, while for the incoming side the
-# credentials must be for a 'server' endpoint.  Setting this will
-# enable TLS for all migrations.  The default is unset, resulting
-# in unsecured migration at the QEMU level.  (Since 2.7)
+# credentials must be for a 'server' endpoint.  Setting this to a
+# non-empty string enables TLS for all migrations.  An empty
+# string means that QEMU will use plain text mode for migration,
+# rather than TLS.  (Since 2.7)
 #
-# @tls-hostname: hostname of the target host for the migration.  This
-# is required when using x509 based TLS credentials and the
-# migration URI does not already include a hostname.  For example
-# if using fd: or exec: based migration, the hostname must be
-# provided so that the server's x509 certificate identity can be
-# validated.  (Since 2.7)
+# @tls-hostname: migration target's hostname for validating the
+# server's x509 certificate identity.  If empty, QEMU will use the
+# hostname from the migration URI, if any.  A non-empty value is
+# required when using x509 based TLS credentials and the migration
+# URI does not include a hostname, such as fd: or exec: based
+# migration.  (Since 2.7)
+#
+# Note: empty value works only since 2.9.
 #
 # @tls-authz: ID of the 'authz' object subclass that provides access
 # control checking of the TLS x509 certificate distinguished name.
@@ -1006,22 +1009,22 @@
 # credentials must be for a 'server' endpoint.  Setting this to a
 # non-empty string enables TLS for all migrations.  An empty
 # string means that QEMU will use plain text mode for migration,
-# rather than TLS (Since 2.9) Previously (since 2.7), this was
-# reported by omitting tls-creds instead.
+# rather than TLS.  This is the default.  (Since 2.7)
 #
-# @tls-hostname: hostname of the target host for the migration.  This
-# is required when using x509 based TLS credentials and the
-# migration URI does not already include a hostname.  For example
-# if using fd: or exec: based migration, the hostname must be
-# provided so that the server's x509 certificate identity can be
-# validated.  (Since 2.7) An empty string means that QEMU will use
-# the hostname associated with the migration URI, if any.  (Since
-# 2.9) Previously (since 2.7), this was reported by omitting
-# tls-hostname instead.
+# @tls-hostname: migration target's hostname for validating the
+# server's x509 certificate identity.  If empty, QEMU will use the
+# hostname from the migration URI, if any.  A non-empty value is
+# required when using x509 based TLS credentials and the migration
+# URI does not include a hostname, such as fd: or exec: based
+# 

[PULL 16/20] qapi: document InputMultiTouchType

2024-03-26 Thread Markus Armbruster
From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Message-ID: <20240325095648.2835381-1-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
[Update qapi/pragma.json]
Signed-off-by: Markus Armbruster 
---
 qapi/pragma.json |  2 --
 qapi/ui.json | 12 
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/qapi/pragma.json b/qapi/pragma.json
index 6929ab776e..92715d22b3 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -62,8 +62,6 @@
 'ImageInfoSpecificKind',
 'InputAxis',
 'InputButton',
-'InputMultiTouchEvent',
-'InputMultiTouchType',
 'IscsiHeaderDigest',
 'IscsiTransport',
 'JSONType',
diff --git a/qapi/ui.json b/qapi/ui.json
index 9721c1e5af..f610bce118 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1080,6 +1080,16 @@
 #
 # Type of a multi-touch event.
 #
+# @begin: A new touch event sequence has just started.
+#
+# @update: A touch event sequence has been updated.
+#
+# @end: A touch event sequence has finished.
+#
+# @cancel: A touch event sequence has been canceled.
+#
+# @data: Absolute position data.
+#
 # Since: 8.1
 ##
 { 'enum'  : 'InputMultiTouchType',
@@ -1137,6 +1147,8 @@
 #
 # MultiTouch input event.
 #
+# @type: The type of multi-touch event.
+#
 # @slot: Which slot has generated the event.
 #
 # @tracking-id: ID to correlate this event with previously generated
-- 
2.44.0




[PULL 19/20] qapi/block-core: improve Qcow2OverlapCheckFlags documentation

2024-03-26 Thread Markus Armbruster
From: Vladimir Sementsov-Ogievskiy 

Most of fields have no description at all. Let's fix that. Still, no
reason to place here more detailed descriptions of what these
structures are, as we have public Qcow2 format specification.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-ID: <20240325120054.2693236-1-vsement...@yandex-team.ru>
Acked-by: Markus Armbruster 
[Capitalize "QEMU", update qapi/pragma.json]
Signed-off-by: Markus Armbruster 
---
 qapi/block-core.json | 25 +
 qapi/pragma.json |  1 -
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7d3fe59f6c..746d1694c2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3403,14 +3403,31 @@
 # @Qcow2OverlapCheckFlags:
 #
 # Structure of flags for each metadata structure.  Setting a field to
-# 'true' makes qemu guard that structure against unintended
-# overwriting.  The default value is chosen according to the template
-# given.
+# 'true' makes QEMU guard that Qcow2 format structure against
+# unintended overwriting.  See Qcow2 format specification for detailed
+# information on these structures.  The default value is chosen
+# according to the template given.
 #
 # @template: Specifies a template mode which can be adjusted using the
 # other flags, defaults to 'cached'
 #
-# @bitmap-directory: since 3.0
+# @main-header: Qcow2 format header
+#
+# @active-l1: Qcow2 active L1 table
+#
+# @active-l2: Qcow2 active L2 table
+#
+# @refcount-table: Qcow2 refcount table
+#
+# @refcount-block: Qcow2 refcount blocks
+#
+# @snapshot-table: Qcow2 snapshot table
+#
+# @inactive-l1: Qcow2 inactive L1 tables
+#
+# @inactive-l2: Qcow2 inactive L2 tables
+#
+# @bitmap-directory: Qcow2 bitmap directory (since 3.0)
 #
 # Since: 2.9
 ##
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 99e4052ab3..9e28de1721 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -72,7 +72,6 @@
 'QCryptoAkCipherKeyType',
 'QCryptodevBackendServiceType',
 'QKeyCode',
-'Qcow2OverlapCheckFlags',
 'RbdAuthMode',
 'RbdImageEncryptionFormat',
 'String',
-- 
2.44.0




[PULL 20/20] qapi: document parameters of query-cpu-model-* QAPI commands

2024-03-26 Thread Markus Armbruster
From: David Hildenbrand 

Let's document the parameters of these commands, so we can remove them
from the "documentation-exceptions" list.

While at it, extend the "Returns:" documentation as well, fixing a wrong
use of CpuModelBaselineInfo vs. CpuModelCompareInfo for
query-cpu-model-comparison.

Cc: Markus Armbruster 
Cc: Eric Blake 
Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: "Philippe Mathieu-Daudé" 
Cc: Yanan Wang 
Signed-off-by: David Hildenbrand 
Message-ID: <20240325150141.342720-1-da...@redhat.com>
Reviewed-by: Markus Armbruster 
[Punctuation tweaked]
Signed-off-by: Markus Armbruster 
---
 qapi/machine-target.json | 46 +++-
 qapi/pragma.json |  3 ---
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 03d7a185b9..29e695aa06 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -124,11 +124,12 @@
 ##
 # @query-cpu-model-comparison:
 #
-# Compares two CPU models, returning how they compare in a specific
-# configuration.  The results indicates how both models compare
-# regarding runnability.  This result can be used by tooling to make
-# decisions if a certain CPU model will run in a certain configuration
-# or if a compatible CPU model has to be created by baselining.
+# Compares two CPU models, @modela and @modelb, returning how they
+# compare in a specific configuration.  The results indicates how
+# both models compare regarding runnability.  This result can be
+# used by tooling to make decisions if a certain CPU model will
+# run in a certain configuration or if a compatible CPU model has
+# to be created by baselining.
 #
 # Usually, a CPU model is compared against the maximum possible CPU
 # model of a certain configuration (e.g. the "host" model for KVM).
@@ -154,7 +155,14 @@
 # Some architectures may not support comparing CPU models.  s390x
 # supports comparing CPU models.
 #
-# Returns: a CpuModelBaselineInfo
+# @modela: description of the first CPU model to compare, referred to as
+# "model A" in CpuModelCompareResult
+#
+# @modelb: description of the second CPU model to compare, referred to as
+# "model B" in CpuModelCompareResult
+#
+# Returns: a CpuModelCompareInfo describing how both CPU models
+# compare
 #
 # Errors:
 # - if comparing CPU models is not supported
@@ -175,9 +183,9 @@
 ##
 # @query-cpu-model-baseline:
 #
-# Baseline two CPU models, creating a compatible third model.  The
-# created model will always be a static, migration-safe CPU model (see
-# "static" CPU model expansion for details).
+# Baseline two CPU models, @modela and @modelb, creating a compatible
+# third model.  The created model will always be a static,
+# migration-safe CPU model (see "static" CPU model expansion for details).
 #
 # This interface can be used by tooling to create a compatible CPU
 # model out two CPU models.  The created CPU model will be identical
@@ -204,7 +212,11 @@
 # Some architectures may not support baselining CPU models.  s390x
 # supports baselining CPU models.
 #
-# Returns: a CpuModelBaselineInfo
+# @modela: description of the first CPU model to baseline
+#
+# @modelb: description of the second CPU model to baseline
+#
+# Returns: a CpuModelBaselineInfo describing the baselined CPU model
 #
 # Errors:
 # - if baselining CPU models is not supported
@@ -243,10 +255,10 @@
 ##
 # @query-cpu-model-expansion:
 #
-# Expands a given CPU model (or a combination of CPU model +
-# additional options) to different granularities, allowing tooling to
-# get an understanding what a specific CPU model looks like in QEMU
-# under a certain configuration.
+# Expands a given CPU model, @model, (or a combination of CPU model +
+# additional options) to different granularities, specified by
+# @type, allowing tooling to get an understanding what a specific
+# CPU model looks like in QEMU under a certain configuration.
 #
 # This interface can be used to query the "host" CPU model.
 #
@@ -269,7 +281,11 @@
 # Some architectures may not support all expansion types.  s390x
 # supports "full" and "static". Arm only supports "full".
 #
-# Returns: a CpuModelExpansionInfo
+# @model: description of the CPU model to expand
+#
+# @type: expansion type, specifying how to expand the CPU model
+#
+# Returns: a CpuModelExpansionInfo describing the expanded CPU model
 #
 # Errors:
 # - if expanding CPU models is not supported
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 9e28de1721..59fbe74b8c 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -84,9 +84,6 @@
 'XDbgBlockGraph',
 'YankInstanceType',
 'blockdev-reopen',
-'query-cpu-model-baseline',
-'query-cpu-model-comparison',
-'query-cpu-model-expansion',
 'query-rocker',
 'query-rocker-ports' ],
 # Externally visible types whose member names may use uppercase
-- 
2.44.0




Re: Let's close member documentation gaps

2024-03-26 Thread Markus Armbruster
Markus Armbruster  writes:

> If you're cc'ed, I have a bit of doc work for you.  Search for your
> name to find it.
>
> The QAPI generator forces you to document your stuff.  Except for
> commands, events, enum and object types listed in pragma
> documentation-exceptions, the generator silently defaults missing
> documentation to "Not documented".  Right now, we're using this loophole
> some 500 times.
>
> Most of the offenders are enumeration values.  Their meaning is perhaps
> easier to guess than the meaning of command arguments, member data, and
> object type members.  Ignoring enumerations leaves 62 offenders.  Let's
> examine them.

[...]

> = qapi/net.json
>
> * String
>
>   Lack of the @str: section produces an embarrassing "Not documented" in
>   the generated documentation.  I can post a patch to make it less
>   embarrassing.  I doubt we can make it actually good, as generic
>   wrapper types like this one have meaning only in the context they are
>   used.  Therefore, their meaning can be usefully explained only at
>   their uses, not their definition.

I decided not to.

String is used for NetdevUserOptions members @dnssearch, @hostfwd, and
@guestfwd.  NetdevUserOptions are the type-specific arguments of
netdev-add with type "user".

@dnssearch is documented tolerably well, although there's less
information than for the CLI equivalent in qemu-options.hx.

@hostfwd is not: the string format is not documented at all.  It's its
own mini-language.  The CLI equivalent in qemu-options.hx documents it.

Of course, encoding information in strings like that is bad practice in
QMP.  Probably not worth fixing now.

Same for @guestfwd.

These documentation deficiencies are more serious than the "Not
documented" for String.  Only fixing the latter seems not worthwhile.

Jason, thoughts?

[...]




Re: [PATCH 25/26] kvm: handle KVM_EXIT_MEMORY_FAULT

2024-03-26 Thread Xiaoyao Li

On 3/23/2024 2:11 AM, Paolo Bonzini wrote:

From: Chao Peng 

When geeting KVM_EXIT_MEMORY_FAULT exit, it indicates userspace needs to


typo: /s/geeting/getting


do the memory conversion on the RAMBlock to turn the memory into desired
attribute, i.e., private/shared.

Currently only KVM_MEMORY_EXIT_FLAG_PRIVATE in flags is valid when
KVM_EXIT_MEMORY_FAULT happens.

Note, KVM_EXIT_MEMORY_FAULT makes sense only when the RAMBlock has
guest_memfd memory backend.

Note, KVM_EXIT_MEMORY_FAULT returns with -EFAULT, so special handling is
added.

When page is converted from shared to private, the original shared
memory can be discarded via ram_block_discard_range(). Note, shared
memory can be discarded only when it's not back'ed by hugetlb because
hugetlb is supposed to be pre-allocated and no need for discarding.

Signed-off-by: Chao Peng 
Co-developed-by: Xiaoyao Li 
Signed-off-by: Xiaoyao Li 

Message-ID: <20240320083945.991426-13-michael.r...@amd.com>
Signed-off-by: Paolo Bonzini 
---
  include/sysemu/kvm.h   |  2 +
  accel/kvm/kvm-all.c| 99 +-
  accel/kvm/trace-events |  2 +
  3 files changed, 93 insertions(+), 10 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 2cb31925091..698f1640fe2 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -541,4 +541,6 @@ int kvm_create_guest_memfd(uint64_t size, uint64_t flags, 
Error **errp);
  
  int kvm_set_memory_attributes_private(hwaddr start, hwaddr size);

  int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size);
+
+int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private);
  #endif
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 56b17cbd8aa..afd7f992e39 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2893,6 +2893,70 @@ static void kvm_eat_signals(CPUState *cpu)
  } while (sigismember(&chkset, SIG_IPI));
  }
  
+int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)

+{
+MemoryRegionSection section;
+ram_addr_t offset;
+MemoryRegion *mr;
+RAMBlock *rb;
+void *addr;
+int ret = -1;
+
+trace_kvm_convert_memory(start, size, to_private ? "shared_to_private" : 
"private_to_shared");
+
+if (!QEMU_PTR_IS_ALIGNED(start, qemu_real_host_page_size()) ||
+!QEMU_PTR_IS_ALIGNED(size, qemu_real_host_page_size())) {
+return -1;
+}
+
+if (!size) {
+return -1;
+}
+
+section = memory_region_find(get_system_memory(), start, size);
+mr = section.mr;
+if (!mr) {
+return -1;
+}
+
+if (!memory_region_has_guest_memfd(mr)) {
+error_report("Converting non guest_memfd backed memory region "
+ "(0x%"HWADDR_PRIx" ,+ 0x%"HWADDR_PRIx") to %s",
+ start, size, to_private ? "private" : "shared");
+ret = -1;


No need for it. ret is initialized as -1 at the function start.


+goto out_unref;
+}
+
+if (to_private) {
+ret = kvm_set_memory_attributes_private(start, size);
+} else {
+ret = kvm_set_memory_attributes_shared(start, size);
+}
+if (ret) {
+goto out_unref;
+}
+
+addr = memory_region_get_ram_ptr(mr) + section.offset_within_region;
+rb = qemu_ram_block_from_host(addr, false, &offset);
+
+if (to_private) {
+if (rb->page_size == qemu_real_host_page_size()) {
+/*
+* shared memory is back'ed by  hugetlb, which is supposed to be


Please fix the bad comment indentation for me, as well as the extra 
space before 'hugetlb'



+* pre-allocated and doesn't need to be discarded
+*/
+goto out_unref;
+}
+ret = ram_block_discard_range(rb, offset, size);
+} else {
+ret = ram_block_discard_guest_memfd_range(rb, offset, size);
+}
+
+out_unref:
+memory_region_unref(section.mr);
+return ret;
+}
+
  int kvm_cpu_exec(CPUState *cpu)
  {
  struct kvm_run *run = cpu->kvm_run;
@@ -2960,18 +3024,20 @@ int kvm_cpu_exec(CPUState *cpu)
  ret = EXCP_INTERRUPT;
  break;
  }
-fprintf(stderr, "error: kvm run failed %s\n",
-strerror(-run_ret));
+if (!(run_ret == -EFAULT && run->exit_reason == 
KVM_EXIT_MEMORY_FAULT)) {
+fprintf(stderr, "error: kvm run failed %s\n",
+strerror(-run_ret));
  #ifdef TARGET_PPC
-if (run_ret == -EBUSY) {
-fprintf(stderr,
-"This is probably because your SMT is enabled.\n"
-"VCPU can only run on primary threads with all "
-"secondary threads offline.\n");
-}
+if (run_ret == -EBUSY) {
+fprintf(stderr,
+"This is probably because your SMT is enabled.\n"
+"VCPU can only run on primary threads with all "
+

Re: [PATCH v6 10/11] virtio-gpu: Initialize Venus

2024-03-26 Thread Pierre-Eric Pelloux-Prayer




Le 23/02/2024 à 10:15, Huang Rui a écrit :

On Tue, Jan 02, 2024 at 09:33:11PM +0800, Marc-André Lureau wrote:

Hi

On Tue, Dec 19, 2023 at 11:55 AM Huang Rui  wrote:


From: Antonio Caggiano 

Request Venus when initializing VirGL.

Signed-off-by: Antonio Caggiano 
Signed-off-by: Huang Rui 
---

Changes in v6:
- Remove the unstable API flags check because virglrenderer is already 1.0.
- Squash the render server flag support into "Initialize Venus".

  hw/display/virtio-gpu-virgl.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index f35a751824..c523a6717a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -964,6 +964,10 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
  }
  #endif

+#ifdef VIRGL_RENDERER_VENUS
+flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
+#endif
+


I wonder if it's a good idea to initialize venus by default. It
doesn't seem to require vulkan during initialization, but this may
evolve. Make it optional?



I am fine. In fact, vulkan is widely used for graphic area such as gaming,
compute, VR/AR, etc.


Actually, making it optional is useful because Venus support is optional in
virglrenderer (= having VIRGL_RENDERER_VENUS defined doesn't mean that
Venus is supported).

Thanks,
Pierre-Eric




Thanks,
Ray


  ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
  if (ret != 0) {
  error_report("virgl could not be initialized: %d", ret);
--
2.25.1




--
Marc-André Lureau






Re: [PATCH 21/26] kvm/memory: Make memory type private by default if it has guest memfd backend

2024-03-26 Thread Xiaoyao Li

On 3/23/2024 2:11 AM, Paolo Bonzini wrote:

From: Xiaoyao Li 

KVM side leaves the memory to shared by default, while may incur the


/s/while/which/

fix typo from myself.


overhead of paging conversion on the first visit of each page. Because
the expectation is that page is likely to private for the VMs that
require private memory (has guest memfd).

Explicitly set the memory to private when memory region has valid
guest memfd backend.

Signed-off-by: Xiaoyao Li 
Signed-off-by: Michael Roth 
Message-ID: <20240320083945.991426-16-michael.r...@amd.com>
Signed-off-by: Paolo Bonzini 
---
  accel/kvm/kvm-all.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 7fbaf31cbaf..56b17cbd8aa 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1430,6 +1430,16 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
  strerror(-err));
  abort();
  }
+
+if (memory_region_has_guest_memfd(mr)) {
+err = kvm_set_memory_attributes_private(start_addr, slot_size);
+if (err) {
+error_report("%s: failed to set memory attribute private: 
%s\n",
+ __func__, strerror(-err));
+exit(1);
+}
+}
+
  start_addr += slot_size;
  ram_start_offset += slot_size;
  ram += slot_size;





Re: [PATCH 1/2] copy-before-write: allow specifying minimum cluster size

2024-03-26 Thread Markus Armbruster
Fiona Ebner  writes:

> Useful to make discard-source work in the context of backup fleecing
> when the fleecing image has a larger granularity than the backup
> target.
>
> Copy-before-write operations will use at least this granularity and in
> particular, discard requests to the source node will too. If the
> granularity is too small, they will just be aligned down in
> cbw_co_pdiscard_snapshot() and thus effectively ignored.
>
> The QAPI uses uint32 so the value will be non-negative, but still fit
> into a uint64_t.
>
> Suggested-by: Vladimir Sementsov-Ogievskiy 
> Signed-off-by: Fiona Ebner 
> ---
>  block/block-copy.c | 17 +
>  block/copy-before-write.c  |  3 ++-
>  include/block/block-copy.h |  1 +
>  qapi/block-core.json   |  8 +++-
>  4 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/block/block-copy.c b/block/block-copy.c
> index 7e3b378528..adb1cbb440 100644
> --- a/block/block-copy.c
> +++ b/block/block-copy.c
> @@ -310,6 +310,7 @@ void block_copy_set_copy_opts(BlockCopyState *s, bool 
> use_copy_range,
>  }
>  
>  static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
> + int64_t min_cluster_size,
>   Error **errp)
>  {
>  int ret;
> @@ -335,7 +336,7 @@ static int64_t 
> block_copy_calculate_cluster_size(BlockDriverState *target,
>  "used. If the actual block size of the target exceeds "
>  "this default, the backup may be unusable",
>  BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
> -return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
> +return MAX(min_cluster_size, BLOCK_COPY_CLUSTER_SIZE_DEFAULT);

min_cluster_size is int64_t, BLOCK_COPY_CLUSTER_SIZE_DEFAULT is int, and
gets converted to int64_t.  The return type is int64_t.  Okay.

>  } else if (ret < 0 && !target_does_cow) {
>  error_setg_errno(errp, -ret,
>  "Couldn't determine the cluster size of the target image, "
> @@ -345,16 +346,18 @@ static int64_t 
> block_copy_calculate_cluster_size(BlockDriverState *target,
>  return ret;
>  } else if (ret < 0 && target_does_cow) {
>  /* Not fatal; just trudge on ahead. */
> -return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
> +return MAX(min_cluster_size, BLOCK_COPY_CLUSTER_SIZE_DEFAULT);

Same.

>  }
>  
> -return MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
> +return MAX(min_cluster_size,
> +   MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size));

Similar: bdi.cluster_size is int.

>  }
>  
>  BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
>   BlockDriverState *copy_bitmap_bs,
>   const BdrvDirtyBitmap *bitmap,
>   bool discard_source,
> + int64_t min_cluster_size,
>   Error **errp)
>  {
>  ERRP_GUARD();
> @@ -365,7 +368,13 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, 
> BdrvChild *target,
>  
>  GLOBAL_STATE_CODE();
>  
> -cluster_size = block_copy_calculate_cluster_size(target->bs, errp);
> +if (min_cluster_size && !is_power_of_2(min_cluster_size)) {

min_cluster_size is int64_t, is_power_of_2() takes uint64_t.  Bad if
min_cluster_size is negative.  Could this happen?


> +error_setg(errp, "min-cluster-size needs to be a power of 2");
> +return NULL;
> +}
> +
> +cluster_size = block_copy_calculate_cluster_size(target->bs,
> + min_cluster_size, errp);

min_cluster_size is int64_t, block_copy_calculate_cluster_size() takes
int64_t, returns int64_t, and cluster_size is int64_t.  Good.

>  if (cluster_size < 0) {
>  return NULL;
>  }
> diff --git a/block/copy-before-write.c b/block/copy-before-write.c
> index dac57481c5..f9896c6c1e 100644
> --- a/block/copy-before-write.c
> +++ b/block/copy-before-write.c
> @@ -476,7 +476,8 @@ static int cbw_open(BlockDriverState *bs, QDict *options, 
> int flags,
>  
>  s->discard_source = flags & BDRV_O_CBW_DISCARD_SOURCE;
>  s->bcs = block_copy_state_new(bs->file, s->target, bs, bitmap,
> -  flags & BDRV_O_CBW_DISCARD_SOURCE, errp);
> +  flags & BDRV_O_CBW_DISCARD_SOURCE,
> +  opts->min_cluster_size, errp);

@opts is BlockdevOptionsCbw *, opts->min_cluster_size is uint32_t (see
last hunk), block_copy_state_new() takes int64_t: opts->min_cluster_size
gets zero-extended.  Okay.

>  if (!s->bcs) {
>  error_prepend(errp, "Cannot create block-copy-state: ");
>  return -EINVAL;
> diff --git a/include/block/block-copy.h b/include/block/block-copy.h
> index bdc703bacd..77857c6c68 100644
> --- a/include/block/block-c

Re: [PATCH 1/3] target/hppa: Squash d for pa1.x during decode

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

The cond_need_ext predicate was created while we still had a
32-bit compilation mode.  It now makes more sense to treat D
as an absolute indicator of a 64-bit operation.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Helge



---
  target/hppa/insns.decode | 20 +---
  target/hppa/translate.c  | 38 --
  2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index f58455dfdb..6a74cf23cd 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -57,6 +57,9 @@
  %neg_to_m   0:1  !function=neg_to_m
  %a_to_m 2:1  !function=neg_to_m
  %cmpbid_c   13:2 !function=cmpbid_c
+%d_55:1  !function=pa20_d
+%d_11   11:1 !function=pa20_d
+%d_13   13:1 !function=pa20_d

  
  # Argument set definitions
@@ -84,15 +87,16 @@
  # Format definitions
  

-@rr_cf_d.. r:5 . cf:4 .. d:1 t:5&rr_cf_d
+@rr_cf_d.. r:5 . cf:4 .. . t:5  &rr_cf_d d=%d_5
  @rrr.. r2:5 r1:5  ... t:5   &rrr
  @rrr_cf .. r2:5 r1:5 cf:4 ... t:5   &rrr_cf
-@rrr_cf_d   .. r2:5 r1:5 cf:4 .. d:1 t:5&rrr_cf_d
+@rrr_cf_d   .. r2:5 r1:5 cf:4 .. . t:5  &rrr_cf_d d=%d_5
  @rrr_sh .. r2:5 r1:5  sh:2 . t:5&rrr_sh
-@rrr_cf_d_sh.. r2:5 r1:5 cf:4  sh:2 d:1 t:5 &rrr_cf_d_sh
-@rrr_cf_d_sh0   .. r2:5 r1:5 cf:4 .. d:1 t:5&rrr_cf_d_sh sh=0
+@rrr_cf_d_sh.. r2:5 r1:5 cf:4  sh:2 . t:5   &rrr_cf_d_sh d=%d_5
+@rrr_cf_d_sh0   .. r2:5 r1:5 cf:4 .. . t:5  &rrr_cf_d_sh d=%d_5 
sh=0
  @rri_cf .. r:5  t:5  cf:4 . ... &rri_cf i=%lowsign_11
-@rri_cf_d   .. r:5  t:5  cf:4 d:1 ...   &rri_cf_d i=%lowsign_11
+@rri_cf_d   .. r:5  t:5  cf:4 . ... \
+&rri_cf_d d=%d_11 i=%lowsign_11

  @rrb_cf .. r2:5 r1:5 c:3 ... n:1 .  \
  &rrb_c_f disp=%assemble_12
@@ -368,8 +372,10 @@ fmpysub_d   100110 . . . . 1 .  
@mpyadd
  # Conditional Branches
  

-bb_sar  11 0 r:5 c:1 1 d:1 ... n:1 . disp=%assemble_12
-bb_imm  110001 p:5   r:5 c:1 1 d:1 ... n:1 . disp=%assemble_12
+bb_sar  11 0 r:5 c:1 1 . ... n:1 . \
+disp=%assemble_12 d=%d_13
+bb_imm  110001 p:5   r:5 c:1 1 . ... n:1 . \
+disp=%assemble_12 d=%d_13

  movb110010 . . ... ... . .  @rrb_cf f=0
  movbi   110011 . . ... ... . .  @rib_cf f=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 99c5c4cbca..a70d644c0b 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -200,6 +200,14 @@ static int cmpbid_c(DisasContext *ctx, int val)
  return val ? val : 4; /* 0 == "*<<" */
  }

+/*
+ * In many places pa1.x did not decode the bit that later became
+ * the pa2.0 D bit.  Suppress D unless the cpu is pa2.0.
+ */
+static int pa20_d(DisasContext *ctx, int val)
+{
+return ctx->is_pa20 & val;
+}

  /* Include the auto-generated decoder.  */
  #include "decode-insns.c.inc"
@@ -693,12 +701,6 @@ static bool cond_need_cb(int c)
  return c == 4 || c == 5;
  }

-/* Need extensions from TCGv_i32 to TCGv_i64. */
-static bool cond_need_ext(DisasContext *ctx, bool d)
-{
-return !(ctx->is_pa20 && d);
-}
-
  /*
   * Compute conditional for arithmetic.  See Page 5-3, Table 5-1, of
   * the Parisc 1.1 Architecture Reference Manual for details.
@@ -715,7 +717,7 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  cond = cond_make_f();
  break;
  case 1: /* = / <>(Z / !Z) */
-if (cond_need_ext(ctx, d)) {
+if (!d) {
  tmp = tcg_temp_new_i64();
  tcg_gen_ext32u_i64(tmp, res);
  res = tmp;
@@ -725,7 +727,7 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  case 2: /* < / >=(N ^ V / !(N ^ V) */
  tmp = tcg_temp_new_i64();
  tcg_gen_xor_i64(tmp, res, sv);
-if (cond_need_ext(ctx, d)) {
+if (!d) {
  tcg_gen_ext32s_i64(tmp, tmp);
  }
  cond = cond_make_0_tmp(TCG_COND_LT, tmp);
@@ -742,7 +744,7 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
   */
  tmp = tcg_temp_new_i64();
  tcg_gen_eqv_i64(tmp, res, sv);
-if (cond_need_ext(ctx, d)) {
+if (!d) {
  tcg_gen_sextract_i64(tmp, tmp, 31, 1);
  tcg_gen_and_i64(tmp, tmp, res);
  tcg_gen_ext32u_i64(tmp, tmp);
@@ -760,13 +762,13 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  tmp = tcg_temp_new_i64();
  

Re: [PATCH 2/3] target/hppa: Replace c with uv in do_cond

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

Prepare for proper indication of shladd unsigned overflow.
The UV indicator will be zero/not-zero instead of a single bit.

Signed-off-by: Richard Henderson 



Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Helge


---
  target/hppa/translate.c | 12 +---
  1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index a70d644c0b..9d31ef5764 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -707,7 +707,7 @@ static bool cond_need_cb(int c)
   */

  static DisasCond do_cond(DisasContext *ctx, unsigned cf, bool d,
- TCGv_i64 res, TCGv_i64 cb_msb, TCGv_i64 sv)
+ TCGv_i64 res, TCGv_i64 uv, TCGv_i64 sv)
  {
  DisasCond cond;
  TCGv_i64 tmp;
@@ -754,14 +754,12 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  }
  cond = cond_make_0_tmp(TCG_COND_EQ, tmp);
  break;
-case 4: /* NUV / UV  (!C / C) */
-/* Only bit 0 of cb_msb is ever set. */
-cond = cond_make_0(TCG_COND_EQ, cb_msb);
+case 4: /* NUV / UV  (!UV / UV) */
+cond = cond_make_0(TCG_COND_EQ, uv);
  break;
-case 5: /* ZNV / VNZ (!C | Z / C & !Z) */
+case 5: /* ZNV / VNZ (!UV | Z / UV & !Z) */
  tmp = tcg_temp_new_i64();
-tcg_gen_neg_i64(tmp, cb_msb);
-tcg_gen_and_i64(tmp, tmp, res);
+tcg_gen_movcond_i64(TCG_COND_EQ, tmp, uv, ctx->zero, ctx->zero, res);
  if (!d) {
  tcg_gen_ext32u_i64(tmp, tmp);
  }





Re: [PATCH v2] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI

2024-03-26 Thread Andrew Jones
On Mon, Mar 25, 2024 at 04:01:16PM +0300, Alexei Filippov wrote:
> kvm_riscv_handle_sbi() may return not supported return code to not trigger
> qemu abort with vendor-specific sbi.
> 
> Added SBI related return code's defines.
> 
> Signed-off-by: Alexei Filippov 
> Fixes: 4eb47125 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")
> Reviewed-by: Daniel Henrique Barboza 
> ---
> 
> Changes since v1:
> -Add Fixes and Revied-by lines.
>  target/riscv/kvm/kvm-cpu.c |  5 +++--
>  target/riscv/sbi_ecall_interface.h | 11 +++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 6a6c6cae80..a4f84ad950 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1404,7 +1404,7 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct 
> kvm_run *run)
>  if (ret == sizeof(ch)) {
>  run->riscv_sbi.ret[0] = ch;
>  } else {
> -run->riscv_sbi.ret[0] = -1;
> +run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
>  }
>  ret = 0;
>  break;
> @@ -1412,7 +1412,8 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct 
> kvm_run *run)
>  qemu_log_mask(LOG_UNIMP,
>"%s: un-handled SBI EXIT, specific reasons is %lu\n",
>__func__, run->riscv_sbi.extension_id);

While changing this, can we also change this log to something like

 "%s: Unhandled SBI exit with extension-id %lu\n", __func__, 
run->riscv_sbi.extension_id


> -ret = -1;
> +run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
> +ret = 0;

We don't have any paths that set ret to anything other than zero now.
Let's return zero at the bottom of the function instead. And the top
of the function can then be cleaned up to

 unsigned char ch;
 int ret;

 switch (run->riscv_sbi.extension_id) {


>  break;
>  }
>  return ret;
> diff --git a/target/riscv/sbi_ecall_interface.h 
> b/target/riscv/sbi_ecall_interface.h
> index 43899d08f6..0279e92a36 100644
> --- a/target/riscv/sbi_ecall_interface.h
> +++ b/target/riscv/sbi_ecall_interface.h
> @@ -69,4 +69,15 @@
>  #define SBI_EXT_VENDOR_END  0x09FF
>  /* clang-format on */
>  
> +/* SBI return error codes */
> +#define SBI_SUCCESS  0
> +#define SBI_ERR_FAILURE -1
> +#define SBI_ERR_NOT_SUPPORTED   -2
> +#define SBI_ERR_INVALID_PARAM   -3
> +#define SBI_ERR_DENIED  -4
> +#define SBI_ERR_INVALID_ADDRESS -5
> +#define SBI_ERR_ALREADY_AVAILABLE   -6
> +#define SBI_ERR_ALREADY_STARTED -7
> +#define SBI_ERR_ALREADY_STOPPED -8

v2 of the spec has SBI_ERR_NO_SHMEM as well.

Thanks,
drew

> +
>  #endif
> -- 
> 2.34.1
> 
> 



Re: [PATCH 3/3] target/hppa: Fix overflow computation for shladd

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

Overflow indicator should include the effect of the shift step.
We had previously left ??? comments about the issue.

Signed-off-by: Richard Henderson 



Tested-by: Helge Deller 

Helge


---
  target/hppa/translate.c | 85 -
  1 file changed, 66 insertions(+), 19 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 9d31ef5764..0976372d16 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -994,7 +994,8 @@ static TCGv_i64 get_psw_carry(DisasContext *ctx, bool d)

  /* Compute signed overflow for addition.  */
  static TCGv_i64 do_add_sv(DisasContext *ctx, TCGv_i64 res,
-  TCGv_i64 in1, TCGv_i64 in2)
+  TCGv_i64 in1, TCGv_i64 in2,
+  TCGv_i64 orig_in1, int shift, bool d)
  {
  TCGv_i64 sv = tcg_temp_new_i64();
  TCGv_i64 tmp = tcg_temp_new_i64();
@@ -1003,9 +1004,50 @@ static TCGv_i64 do_add_sv(DisasContext *ctx, TCGv_i64 
res,
  tcg_gen_xor_i64(tmp, in1, in2);
  tcg_gen_andc_i64(sv, sv, tmp);

+switch (shift) {
+case 0:
+break;
+case 1:
+/* Shift left by one and compare the sign. */
+tcg_gen_add_i64(tmp, orig_in1, orig_in1);
+tcg_gen_xor_i64(tmp, tmp, orig_in1);
+/* Incorporate into the overflow. */
+tcg_gen_or_i64(sv, sv, tmp);
+break;
+default:
+{
+int sign_bit = d ? 63 : 31;
+uint64_t mask = MAKE_64BIT_MASK(sign_bit - shift, shift);
+
+/* Compare the sign against all lower bits. */
+tcg_gen_sextract_i64(tmp, orig_in1, sign_bit, 1);
+tcg_gen_xor_i64(tmp, tmp, orig_in1);
+/*
+ * If one of the bits shifting into or through the sign
+ * differs, then we have overflow.
+ */
+tcg_gen_movcond_i64(TCG_COND_TSTNE, sv,
+tmp, tcg_constant_i64(mask),
+tcg_constant_i64(-1), sv);
+}
+}
  return sv;
  }

+/* Compute unsigned overflow for addition.  */
+static TCGv_i64 do_add_uv(DisasContext *ctx, TCGv_i64 cb, TCGv_i64 cb_msb,
+  TCGv_i64 in1, int shift, bool d)
+{
+if (shift == 0) {
+return get_carry(ctx, d, cb, cb_msb);
+} else {
+TCGv_i64 tmp = tcg_temp_new_i64();
+tcg_gen_extract_i64(tmp, in1, (d ? 63 : 31) - shift, shift);
+tcg_gen_or_i64(tmp, tmp, get_carry(ctx, d, cb, cb_msb));
+return tmp;
+}
+}
+
  /* Compute signed overflow for subtraction.  */
  static TCGv_i64 do_sub_sv(DisasContext *ctx, TCGv_i64 res,
TCGv_i64 in1, TCGv_i64 in2)
@@ -1020,19 +1062,19 @@ static TCGv_i64 do_sub_sv(DisasContext *ctx, TCGv_i64 
res,
  return sv;
  }

-static void do_add(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
+static void do_add(DisasContext *ctx, unsigned rt, TCGv_i64 orig_in1,
 TCGv_i64 in2, unsigned shift, bool is_l,
 bool is_tsv, bool is_tc, bool is_c, unsigned cf, bool d)
  {
-TCGv_i64 dest, cb, cb_msb, cb_cond, sv, tmp;
+TCGv_i64 dest, cb, cb_msb, in1, uv, sv, tmp;
  unsigned c = cf >> 1;
  DisasCond cond;

  dest = tcg_temp_new_i64();
  cb = NULL;
  cb_msb = NULL;
-cb_cond = NULL;

+in1 = orig_in1;
  if (shift) {
  tmp = tcg_temp_new_i64();
  tcg_gen_shli_i64(tmp, in1, shift);
@@ -1050,9 +1092,6 @@ static void do_add(DisasContext *ctx, unsigned rt, 
TCGv_i64 in1,
  }
  tcg_gen_xor_i64(cb, in1, in2);
  tcg_gen_xor_i64(cb, cb, dest);
-if (cond_need_cb(c)) {
-cb_cond = get_carry(ctx, d, cb, cb_msb);
-}
  } else {
  tcg_gen_add_i64(dest, in1, in2);
  if (is_c) {
@@ -1063,18 +1102,23 @@ static void do_add(DisasContext *ctx, unsigned rt, 
TCGv_i64 in1,
  /* Compute signed overflow if required.  */
  sv = NULL;
  if (is_tsv || cond_need_sv(c)) {
-sv = do_add_sv(ctx, dest, in1, in2);
+sv = do_add_sv(ctx, dest, in1, in2, orig_in1, shift, d);
  if (is_tsv) {
  if (!d) {
  tcg_gen_ext32s_i64(sv, sv);
  }
-/* ??? Need to include overflow from shift.  */
  gen_helper_tsv(tcg_env, sv);
  }
  }

+/* Compute unsigned overflow if required.  */
+uv = NULL;
+if (cond_need_cb(c)) {
+uv = do_add_uv(ctx, cb, cb_msb, orig_in1, shift, d);
+}
+
  /* Emit any conditional trap before any writeback.  */
-cond = do_cond(ctx, cf, d, dest, cb_cond, sv);
+cond = do_cond(ctx, cf, d, dest, uv, sv);
  if (is_tc) {
  tmp = tcg_temp_new_i64();
  tcg_gen_setcond_i64(cond.c, tmp, cond.a0, cond.a1);
@@ -2843,7 +2887,6 @@ static bool trans_dcor_i(DisasContext *ctx, arg_rr_cf_d 
*a)
  static bool trans_ds(DisasContext *ctx, arg_r

Re: [PULL 00/15] riscv-to-apply queue

2024-03-26 Thread Michael Tokarev

On 24.03.2024 21:12, Daniel Henrique Barboza wrote:

On 3/24/24 12:07, Michael Tokarev wrote:



Unfortunately this doesn't quite work, the following changes
fail to apply to 8.2:

929e521a47 target/riscv: always clear vstart for ldst_whole insns
b46631f122 target/riscv: remove 'over' brconds from vector trans
d57dfe4b37 trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
bac802ada8 target/riscv: enable 'vstart_eq_zero' in the end of insns
385e575cd5 target/riscv/kvm: fix timebase-frequency when using KVM acceleration



The amount of work can be non-trivial for this backport, so I'd say we should
leave it aside for now. If someone has a good argument for this work then we
can re-evaluate.


So, out of 15 patches in this series (minus the first one already
mentioned) - should I pick 9 remaining patches for stable (the ones
which applies) or none at all? :)

Thanks,

/mjt



Re: [PATCH for-9.0 0/3] target/hppa: Fix overflow computation for shladd

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

These ??? notes have been there since day one.
This fixes l2diag test 59.


Your patches fix the 64-bit wdiag test 66 (shladd) too.

I tested 32/64-bit Linux & 32-bit HP-UX.
No regressions.

Helge



Richard Henderson (3):
   target/hppa: Squash d for pa1.x during decode
   target/hppa: Replace c with uv in do_cond
   target/hppa: Fix overflow computation for shladd

  target/hppa/insns.decode |  20 --
  target/hppa/translate.c  | 135 ++-
  2 files changed, 104 insertions(+), 51 deletions(-)






Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning

2024-03-26 Thread Philippe Mathieu-Daudé

On 26/3/24 04:33, Pierrick Bouvier wrote:

On 3/26/24 05:52, Yao Xingtao wrote:

1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
    Use g_pattern_spec_match_string() instead to avoid this problem.

2. The type of second parameter in g_ptr_array_add() is
    'gpointer' {aka 'void *'}, but the type of reg->name is 'const 
char*'.

    Cast the type of reg->name to 'gpointer' to avoid this problem.

compiler warning message:
/root/qemu/contrib/plugins/execlog.c:330:17: warning: 
‘g_pattern_match_string’

is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
   330 | if (g_pattern_match_string(pat, rd->name) ||
   | ^~
In file included from /usr/include/glib-2.0/glib.h:67,
  from /root/qemu/contrib/plugins/execlog.c:9:
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
    57 | gboolean  g_pattern_match_string   (GPatternSpec *pspec,
   |   ^~
/root/qemu/contrib/plugins/execlog.c:331:21: warning: 
‘g_pattern_match_string’

is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
   331 | g_pattern_match_string(pat, rd_lower)) {
   | ^~
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
    57 | gboolean  g_pattern_match_string   (GPatternSpec *pspec,
   |   ^~
/root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument 
2 of

‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
   339 | g_ptr_array_add(all_reg_names, 
reg->name);
   |
~~~^~

In file included from /usr/include/glib-2.0/glib.h:33:
/usr/include/glib-2.0/glib/garray.h:198:62: note: expected
‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’
   198 |    gpointer  
data);
   |
~~^~~~


Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
Signed-off-by: Yao Xingtao 
---
  contrib/plugins/execlog.c | 24 +---
  1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..fab18113d4 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -311,6 +311,24 @@ static Register 
*init_vcpu_register(qemu_plugin_reg_descriptor *desc)

  return reg;
  }
+/*
+ * g_pattern_match_string has been deprecated in Glib since 2.70 and
+ * will complain about it if you try to use it. Fortunately the
+ * signature of both functions is the same making it easy to work
+ * around.
+ */
+static inline
+gboolean g_pattern_spec_match_string_qemu(GPatternSpec *pspec,
+  const gchar *string)
+{
+#if GLIB_CHECK_VERSION(2, 70, 0)
+    return g_pattern_spec_match_string(pspec, string);
+#else
+    return g_pattern_match_string(pspec, string);
+#endif
+};
+#define g_pattern_spec_match_string(p, s) 
g_pattern_spec_match_string_qemu(p, s)

+
  static GPtrArray *registers_init(int vcpu_index)
  {
  g_autoptr(GPtrArray) registers = g_ptr_array_new();
@@ -327,8 +345,8 @@ static GPtrArray *registers_init(int vcpu_index)
  for (int p = 0; p < rmatches->len; p++) {
  g_autoptr(GPatternSpec) pat = 
g_pattern_spec_new(rmatches->pdata[p]);
  g_autofree gchar *rd_lower = 
g_utf8_strdown(rd->name, -1);

-    if (g_pattern_match_string(pat, rd->name) ||
-    g_pattern_match_string(pat, rd_lower)) {
+    if (g_pattern_spec_match_string(pat, rd->name) ||
+    g_pattern_spec_match_string(pat, rd_lower)) {
  Register *reg = init_vcpu_register(rd);
  g_ptr_array_add(registers, reg);
@@ -336,7 +354,7 @@ static GPtrArray *registers_init(int vcpu_index)
  if (disas_assist) {
  g_mutex_lock(&add_reg_name_lock);
  if (!g_ptr_array_find(all_reg_names, 
reg->name, NULL)) {

-    g_ptr_array_add(all_reg_names, reg->name);
+    g_ptr_array_add(all_reg_names, 
(gpointer)reg->name);

  }
  g_mutex_unlock(&add_reg_name_lock);
  }


Would be nice if it's still possible to merge this in 9.0 Peter.


I will post a small PR later today, so until Peter has something
else planned, I can take it, since the patch LGTM now.


Reviewed-by: Pierrick Bouvier 





Re: [PATCH] misc/pca955*: Move models under hw/gpio

2024-03-26 Thread Philippe Mathieu-Daudé

On 25/3/24 14:48, Cédric Le Goater wrote:

The PCA9552 and PCA9554 devices are both I2C GPIO controllers and the
PCA9552 also can drive LEDs. Do all the necessary adjustments to move
the models under hw/gpio.

Cc: Glenn Miles 
Signed-off-by: Cédric Le Goater 
---
  MAINTAINERS  | 4 ++--
  include/hw/{misc => gpio}/pca9552.h  | 0
  include/hw/{misc => gpio}/pca9552_regs.h | 0
  include/hw/{misc => gpio}/pca9554.h  | 0
  include/hw/{misc => gpio}/pca9554_regs.h | 0
  hw/arm/aspeed.c  | 2 +-
  hw/{misc => gpio}/pca9552.c  | 4 ++--
  hw/{misc => gpio}/pca9554.c  | 4 ++--
  tests/qtest/pca9552-test.c   | 2 +-
  tests/qtest/pnv-host-i2c-test.c  | 4 ++--
  hw/gpio/meson.build  | 2 ++
  hw/gpio/trace-events | 4 
  hw/misc/meson.build  | 2 --
  hw/misc/trace-events | 4 
  14 files changed, 16 insertions(+), 16 deletions(-)
  rename include/hw/{misc => gpio}/pca9552.h (100%)
  rename include/hw/{misc => gpio}/pca9552_regs.h (100%)
  rename include/hw/{misc => gpio}/pca9554.h (100%)
  rename include/hw/{misc => gpio}/pca9554_regs.h (100%)
  rename hw/{misc => gpio}/pca9552.c (99%)
  rename hw/{misc => gpio}/pca9554.c (99%)


Thanks, patch queued.



Re: [PATCH] docs/system/ppc/amigang.rst: Fix formatting

2024-03-26 Thread Philippe Mathieu-Daudé

On 24/3/24 17:11, BALATON Zoltan wrote:

Add missing space to fix character formatting where it was missed in
two places.

Fixes: 623d9065b6 (docs/system/ppc: Document running Linux on AmigaNG machines)
Signed-off-by: BALATON Zoltan 
---
  docs/system/ppc/amigang.rst | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Thanks, patch queued.



Re: [PATCH v2 0/3] Fixes for "ui/cocoa: Let the platform toggle fullscreen"

2024-03-26 Thread Philippe Mathieu-Daudé

On 23/3/24 07:20, Akihiko Odaki wrote:

This series contains patches for regressions caused by commit 91aa508d0274
("ui/cocoa: Let the platform toggle fullscreen").




---
Akihiko Odaki (3):
   ui/cocoa: Fix aspect ratio
   ui/cocoa: Resize window after toggling zoom-to-fit
   ui/cocoa: Use NSTrackingInVisibleRect

  ui/cocoa.m | 90 ++
  1 file changed, 55 insertions(+), 35 deletions(-)


Thanks, series queued.



Re: [PULL 00/15] riscv-to-apply queue

2024-03-26 Thread Alistair Francis
On Tue, Mar 26, 2024 at 7:53 PM Michael Tokarev  wrote:
>
> On 24.03.2024 21:12, Daniel Henrique Barboza wrote:
> > On 3/24/24 12:07, Michael Tokarev wrote:
>
> >> Unfortunately this doesn't quite work, the following changes
> >> fail to apply to 8.2:
> >>
> >> 929e521a47 target/riscv: always clear vstart for ldst_whole insns
> >> b46631f122 target/riscv: remove 'over' brconds from vector trans
> >> d57dfe4b37 trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
> >> bac802ada8 target/riscv: enable 'vstart_eq_zero' in the end of insns
> >> 385e575cd5 target/riscv/kvm: fix timebase-frequency when using KVM 
> >> acceleration
>
> > The amount of work can be non-trivial for this backport, so I'd say we 
> > should
> > leave it aside for now. If someone has a good argument for this work then we
> > can re-evaluate.
>
> So, out of 15 patches in this series (minus the first one already
> mentioned) - should I pick 9 remaining patches for stable (the ones
> which applies) or none at all? :)

Sorry for the confusion.

The 9 patches that applied and

385e575cd5 target/riscv/kvm: fix timebase-frequency when using KVM acceleration

should all be picked for stable.

PS: What is the best way in future to help ease some of the stable
burden? Should I try and cherry pick them beforehand and then mention
that as a follow up to the PR?

Alistair

>
> Thanks,
>
> /mjt



Re: [PATCH] target/tricore/helper: Use correct string format in cpu_tlb_fill()

2024-03-26 Thread Philippe Mathieu-Daudé

On 19/3/24 06:14, Philippe Mathieu-Daudé wrote:

'address' got converted from target_ulong to vaddr in commit
68d6eee73c ("target/tricore: Convert to CPUClass::tlb_fill").
Use the corresponding format string to avoid casting.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/tricore/helper.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Thanks, patch queued.



Re: [PATCH] monitor/hmp-cmds-target.c: append a space in error message in gpa2hva()

2024-03-26 Thread Philippe Mathieu-Daudé

On 19/3/24 03:16, Shiyang Ruan via wrote:

From: Yao Xingtao 

In qemu monitor mode, when we use gpa2hva command to print the host
virtual address corresponding to a guest physical address, if the gpa is
not in RAM, the error message is below:

(qemu) gpa2hva 0x75000
Memory at address 0x75000is not RAM

a space is missed between '0x75000' and 'is'.

Signed-off-by: Yao Xingtao 
---
  monitor/hmp-cmds-target.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Thanks, patch queued.



[PATCH 1/1] docs: sbsa: update specs, add dt note

2024-03-26 Thread Marcin Juszkiewicz
Hardware of sbsa-ref board is nowadays defined by both BSA and SBSA
specifications. Then BBR defines firmware interface.

Added note about DeviceTree data passed from QEMU to firmware. It is
very minimal and provides only data we use in firmware.

Added NUMA information to list of things reported by DeviceTree.

Signed-off-by: Marcin Juszkiewicz 
---
 docs/system/arm/sbsa.rst | 37 -
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/docs/system/arm/sbsa.rst b/docs/system/arm/sbsa.rst
index bca61608ff..d4d1f2efe3 100644
--- a/docs/system/arm/sbsa.rst
+++ b/docs/system/arm/sbsa.rst
@@ -1,12 +1,16 @@
 Arm Server Base System Architecture Reference board (``sbsa-ref``)
 ==
 
-While the ``virt`` board is a generic board platform that doesn't match
-any real hardware the ``sbsa-ref`` board intends to look like real
-hardware. The `Server Base System Architecture
-`_ defines a
-minimum base line of hardware support and importantly how the firmware
-reports that to any operating system.
+The ``sbsa-ref`` board intends to look like real hardware (while the ``virt``
+board is a generic board platform that doesn't match any real hardware).
+
+The hardware part is defined by two specifications:
+
+  - `Base System Architecture 
`__ (BSA)
+  - `Server Base System Architecture 
`__ (SBSA)
+
+The `Arm Base Boot Requirements 
`__ (BBR)
+specification defines how the firmware reports that to any operating system.
 
 It is intended to be a machine for developing firmware and testing
 standards compliance with operating systems.
@@ -35,16 +39,31 @@ includes both internal hardware and parts affected by the 
qemu command line
 (i.e. CPUs and memory). As a result it must have a firmware specifically built
 to expect a certain hardware layout (as you would in a real machine).
 
+Note
+
+
+QEMU provides us with minimal information about hardware platform using
+minimalistic devicetree. This is not a Linux devicetree. It is not even a
+firmware devicetree.
+
+It is information passed from QEMU to describe the information a hardware
+platform would have other mechanisms to discover at runtime, that are affected
+by the QEMU command line.
+
+Ultimately this devicetree will be replaced by IPC calls to an emulated SCP.
+And when we do that, we won't then have to rewrite Normal world firmware to
+cope.
+
 DeviceTree information
 ''
 
-The devicetree provided by the board model to the firmware is not intended
-to be a complete compliant DT. It currently reports:
+The devicetree reports:
 
- CPUs
- memory
- platform version
- GIC addresses
+   - NUMA node id for CPUs and memory
 
 Platform version
 
@@ -70,4 +89,4 @@ Platform version changes:
   GIC ITS information is present in devicetree.
 
 0.3
-  The USB controller is an XHCI device, not EHCI
+  The USB controller is an XHCI device, not EHCI.
-- 
2.44.0




Re: [PATCH] scsi-generic: fix io_timeout property not applying

2024-03-26 Thread Philippe Mathieu-Daudé

On 15/3/24 15:58, Lorenz Brun wrote:

The io_timeout property, introduced in c9b6609 (part of 6.0) is
silently overwritten by the hardcoded default value of 30 seconds
(DEFAULT_IO_TIMEOUT) in scsi_generic_realize because that function is
being called after the properties have already been applied.

The property definition already has a default value which is applied
correctly when no value is explicitly set, so we can just remove the
code which overrides the io_timeout completely.

This has been tested by stracing SG_IO operations with the io_timeout
property set and unset and now sets the timeout field in the ioctl
request to the proper value.

Fixes: c9b6609b69facad ("scsi: make io_timeout configurable")
Signed-off-by: Lorenz Brun 
---
  hw/scsi/scsi-generic.c | 1 -
  1 file changed, 1 deletion(-)


Thanks, patch queued.




[PATCH 0/2] virtio-net: Fix RSS

2024-03-26 Thread Akihiko Odaki
Some recent changes made RSS unfunctional so here are fixes.

Signed-off-by: Akihiko Odaki 
---
Akihiko Odaki (2):
  virtio-net: Fix vhost virtqueue notifiers for RSS
  ebpf: Fix indirections table setting

 ebpf/ebpf_rss.h | 2 +-
 ebpf/ebpf_rss.c | 5 +++--
 hw/net/virtio-net.c | 4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)
---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240324-vhost-5e26c8a2da5a

Best regards,
-- 
Akihiko Odaki 




[PATCH 2/2] ebpf: Fix indirections table setting

2024-03-26 Thread Akihiko Odaki
The kernel documentation says:
> The value stored can be of any size, however, all array elements are
> aligned to 8 bytes.
https://www.kernel.org/doc/html/v6.8/bpf/map_array.html

Fixes: 333b3e5fab75 ("ebpf: Added eBPF map update through mmap.")
Signed-off-by: Akihiko Odaki 
---
 ebpf/ebpf_rss.h | 2 +-
 ebpf/ebpf_rss.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ebpf/ebpf_rss.h b/ebpf/ebpf_rss.h
index 239242b0d26e..7d15b600bf5b 100644
--- a/ebpf/ebpf_rss.h
+++ b/ebpf/ebpf_rss.h
@@ -26,7 +26,7 @@ struct EBPFRSSContext {
 /* mapped eBPF maps for direct access to omit bpf_map_update_elem() */
 void *mmap_configuration;
 void *mmap_toeplitz_key;
-void *mmap_indirections_table;
+uint64_t *mmap_indirections_table;
 };
 
 struct EBPFRSSConfig {
diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
index 2e506f974357..e0f300febb77 100644
--- a/ebpf/ebpf_rss.c
+++ b/ebpf/ebpf_rss.c
@@ -190,8 +190,9 @@ static bool ebpf_rss_set_indirections_table(struct 
EBPFRSSContext *ctx,
 return false;
 }
 
-memcpy(ctx->mmap_indirections_table, indirections_table,
-sizeof(*indirections_table) * len);
+for (size_t i = 0; i < len; i++) {
+ctx->mmap_indirections_table[i] = indirections_table[i];
+}
 return true;
 }
 

-- 
2.44.0




[PATCH 1/2] virtio-net: Fix vhost virtqueue notifiers for RSS

2024-03-26 Thread Akihiko Odaki
virtio_net_guest_notifier_pending() and virtio_net_guest_notifier_mask()
checked VIRTIO_NET_F_MQ to know there are multiple queues, but
VIRTIO_NET_F_RSS also enables multiple queues. Refer to n->multiqueue,
which is set to true either of VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is
enabled.

Fixes: 68b0a6395f36 ("virtio-net: align ctrl_vq index for non-mq guest for 
vhost_vdpa")
Signed-off-by: Akihiko Odaki 
---
 hw/net/virtio-net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9959f1932b1b..a6ff000cd9d3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3426,7 +3426,7 @@ static bool 
virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
 VirtIONet *n = VIRTIO_NET(vdev);
 NetClientState *nc;
 assert(n->vhost_started);
-if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) {
+if (!n->multiqueue && idx == 2) {
 /* Must guard against invalid features and bogus queue index
  * from being set by malicious guest, or penetrated through
  * buggy migration stream.
@@ -3458,7 +3458,7 @@ static void virtio_net_guest_notifier_mask(VirtIODevice 
*vdev, int idx,
 VirtIONet *n = VIRTIO_NET(vdev);
 NetClientState *nc;
 assert(n->vhost_started);
-if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ) && idx == 2) {
+if (!n->multiqueue && idx == 2) {
 /* Must guard against invalid features and bogus queue index
  * from being set by malicious guest, or penetrated through
  * buggy migration stream.

-- 
2.44.0




Re: [RFC v2 0/5] ARM Nested Virt Support

2024-03-26 Thread Eric Auger
Hi Peter,

On 3/5/24 17:57, Peter Maydell wrote:
> On Fri, 9 Feb 2024 at 16:00, Eric Auger  wrote:
>> This series adds ARM Nested Virtualization support in KVM mode.
>> This is a respin of previous contributions from Miguel [1] and Haibo [2].
>>
>> This was tested with Marc's v11 [3] on Ampere HW with fedora L1 guest and
>> L2 guests booted without EDK2. However it does not work yet with
>> EDK2 but it looks unrelated to this qemu integration (host hard lockups).
>>
>> The host needs to be booted with "kvm-arm.mode=nested" option and
>> qemu needs to be invoked with :
>>
>> -machine virt,virtualization=on
>>
>> There is a known issue with hosts supporting SVE. Kernel does not support 
>> both
>> SVE and NV2 and the current qemu integration has an issue with the
>> scratch_host_vcpu startup because both are enabled if exposed by the kernel.
>> This is independent on whether sve is disabled on the command line. 
>> Unfortunately
>> I lost access to the HW that expose that issue so I couldn't fix it in this
>> version.
>>
>> This series can be found at:
>> https://github.com/eauger/qemu/tree/v8.2-nv-rfcv2
>>
>> Previous version from Miguel:
>> [1] 
>> https://lore.kernel.org/all/20230227163718.62003-1-miguel.l...@oracle.com/
>> Previous version from Haibo:
>> [2] 
>> https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo...@linaro.org/
>> [3] Marc's kernel v11 series:
>> [PATCH v11 00/43] KVM: arm64: Nested Virtualization support (FEAT_NV2 
>> only)
>> 
>> https://lore.kernel.org/linux-arm-kernel/20231120131027.854038-1-...@kernel.org/T/
>> available at: https://github.com/eauger/linux/tree/nv-6.8-nv2-v11
>>
>> Haibo Xu (5):
>>   [Placeholder] headers: Partial headers update for NV2 enablement
>>   hw/arm: Allow setting KVM vGIC maintenance IRQ
>>   target/arm/kvm: Add helper to detect EL2 when using KVM
>>   target/arm: Enable feature ARM_FEATURE_EL2 if EL2 is supported
>>   hw/arm/virt: Allow virt extensions with KVM
>>
>>  hw/arm/virt.c  |  6 +-
>>  hw/intc/arm_gicv3_common.c |  1 +
>>  hw/intc/arm_gicv3_kvm.c| 21 +
>>  include/hw/intc/arm_gicv3_common.h |  1 +
>>  linux-headers/asm-arm64/kvm.h  |  1 +
>>  linux-headers/linux/kvm.h  |  1 +
>>  target/arm/kvm.c   | 21 +
>>  target/arm/kvm_arm.h   | 12 
>>  8 files changed, 63 insertions(+), 1 deletion(-)
> All the patches in this series seem reasonable, but the series
> as a whole is so short I wonder if we're missing something :-)
> Does migration Just Work? (I guess as long as the kernel exposes
> all the EL2 sysregs via the ONE_REG ioctl interface it ought to...)
To be honest I have not tested yet. I would also guess there is no mig
blocker but that definitively deserves some testing. I will check before
sending the next version.
>
> Anyway, I don't think there's anything that stood out as needing
> major changes, so for now I guess we just wait for whenever the
> KVM side patches eventually land.

yup

Thanks

Eric
>
> thanks
> -- PMM
>




Re: [PATCH for-9.1 v5 1/3] hw: Add compat machines for 9.1

2024-03-26 Thread Harsh Prateek Bora




On 3/25/24 19:44, Paolo Bonzini wrote:

Add 9.1 machine types for arm/i440fx/m68k/q35/s390x/spapr.

Cc: Cornelia Huck 
Cc: Thomas Huth 
Cc: Harsh Prateek Bora 
Cc: Gavin Shan 
Signed-off-by: Paolo Bonzini 
---
  include/hw/boards.h|  3 +++
  include/hw/i386/pc.h   |  3 +++
  hw/arm/virt.c  | 11 +--
  hw/core/machine.c  |  3 +++
  hw/i386/pc.c   |  3 +++
  hw/i386/pc_piix.c  | 17 ++---
  hw/i386/pc_q35.c   | 14 --
  hw/m68k/virt.c | 11 +--
  hw/ppc/spapr.c | 17 ++---


For spapr:
Reviewed-by: Harsh Prateek Bora 


  hw/s390x/s390-virtio-ccw.c | 14 +-
  10 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 8b8f6d5c00d..50e0cf4278e 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -425,6 +425,9 @@ struct MachineState {
  } \
  type_init(machine_initfn##_register_types)
  
+extern GlobalProperty hw_compat_9_0[];

+extern const size_t hw_compat_9_0_len;
+
  extern GlobalProperty hw_compat_8_2[];
  extern const size_t hw_compat_8_2_len;
  
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h

index 27a68071d77..349f79df086 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -198,6 +198,9 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t 
flash_size);
  /* sgx.c */
  void pc_machine_init_sgx_epc(PCMachineState *pcms);
  
+extern GlobalProperty pc_compat_9_0[];

+extern const size_t pc_compat_9_0_len;
+
  extern GlobalProperty pc_compat_8_2[];
  extern const size_t pc_compat_8_2_len;
  
diff --git a/hw/arm/virt.c b/hw/arm/virt.c

index a9a913aeadb..c9119ef3847 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3223,10 +3223,17 @@ static void machvirt_machine_init(void)
  }
  type_init(machvirt_machine_init);
  
-static void virt_machine_9_0_options(MachineClass *mc)

+static void virt_machine_9_1_options(MachineClass *mc)
  {
  }
-DEFINE_VIRT_MACHINE_AS_LATEST(9, 0)
+DEFINE_VIRT_MACHINE_AS_LATEST(9, 1)
+
+static void virt_machine_9_0_options(MachineClass *mc)
+{
+virt_machine_9_1_options(mc);
+compat_props_add(mc->compat_props, hw_compat_9_0, hw_compat_9_0_len);
+}
+DEFINE_VIRT_MACHINE(9, 0)
  
  static void virt_machine_8_2_options(MachineClass *mc)

  {
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 37ede0e7d4f..a92bec23147 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -33,6 +33,9 @@
  #include "hw/virtio/virtio-iommu.h"
  #include "audio/audio.h"
  
+GlobalProperty hw_compat_9_0[] = {};

+const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
+
  GlobalProperty hw_compat_8_2[] = {
  { "migration", "zero-page-detection", "legacy"},
  { TYPE_VIRTIO_IOMMU_PCI, "granule", "4k" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e80f02bef41..461fcaa1b48 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -78,6 +78,9 @@
  { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
  { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
  
+GlobalProperty pc_compat_9_0[] = {};

+const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0);
+
  GlobalProperty pc_compat_8_2[] = {};
  const size_t pc_compat_8_2_len = G_N_ELEMENTS(pc_compat_8_2);
  
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c

index 18ba0766092..8850c49c66a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -513,13 +513,26 @@ static void pc_i440fx_machine_options(MachineClass *m)
   "Use a different south bridge than 
PIIX3");
  }
  
-static void pc_i440fx_9_0_machine_options(MachineClass *m)

+static void pc_i440fx_9_1_machine_options(MachineClass *m)
  {
  pc_i440fx_machine_options(m);
  m->alias = "pc";
  m->is_default = true;
  }
  
+DEFINE_I440FX_MACHINE(v9_1, "pc-i440fx-9.1", NULL,

+  pc_i440fx_9_1_machine_options);
+
+static void pc_i440fx_9_0_machine_options(MachineClass *m)
+{
+pc_i440fx_9_1_machine_options(m);
+m->alias = NULL;
+m->is_default = false;
+
+compat_props_add(m->compat_props, hw_compat_9_0, hw_compat_9_0_len);
+compat_props_add(m->compat_props, pc_compat_9_0, pc_compat_9_0_len);
+}
+
  DEFINE_I440FX_MACHINE(v9_0, "pc-i440fx-9.0", NULL,
pc_i440fx_9_0_machine_options);
  
@@ -528,8 +541,6 @@ static void pc_i440fx_8_2_machine_options(MachineClass *m)

  PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
  
  pc_i440fx_9_0_machine_options(m);

-m->alias = NULL;
-m->is_default = false;
  
  compat_props_add(m->compat_props, hw_compat_8_2, hw_compat_8_2_len);

  compat_props_add(m->compat_props, pc_compat_8_2, pc_compat_8_2_len);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c7bc8a2041f..6e1180d4b60 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -365,12 +365,23 @@ static void pc_q35_machine_options(MachineClass *m)
   pc_q35_comp

Re: [PATCH] misc/pca955*: Move models under hw/gpio

2024-03-26 Thread Cédric Le Goater

On 3/26/24 10:55, Philippe Mathieu-Daudé wrote:

On 25/3/24 14:48, Cédric Le Goater wrote:

The PCA9552 and PCA9554 devices are both I2C GPIO controllers and the
PCA9552 also can drive LEDs. Do all the necessary adjustments to move
the models under hw/gpio.

Cc: Glenn Miles 
Signed-off-by: Cédric Le Goater 
---
  MAINTAINERS  | 4 ++--
  include/hw/{misc => gpio}/pca9552.h  | 0
  include/hw/{misc => gpio}/pca9552_regs.h | 0
  include/hw/{misc => gpio}/pca9554.h  | 0
  include/hw/{misc => gpio}/pca9554_regs.h | 0
  hw/arm/aspeed.c  | 2 +-
  hw/{misc => gpio}/pca9552.c  | 4 ++--
  hw/{misc => gpio}/pca9554.c  | 4 ++--
  tests/qtest/pca9552-test.c   | 2 +-
  tests/qtest/pnv-host-i2c-test.c  | 4 ++--
  hw/gpio/meson.build  | 2 ++
  hw/gpio/trace-events | 4 
  hw/misc/meson.build  | 2 --
  hw/misc/trace-events | 4 
  14 files changed, 16 insertions(+), 16 deletions(-)
  rename include/hw/{misc => gpio}/pca9552.h (100%)
  rename include/hw/{misc => gpio}/pca9552_regs.h (100%)
  rename include/hw/{misc => gpio}/pca9554.h (100%)
  rename include/hw/{misc => gpio}/pca9554_regs.h (100%)
  rename hw/{misc => gpio}/pca9552.c (99%)
  rename hw/{misc => gpio}/pca9554.c (99%)


Thanks, patch queued.


This one is merged,

https://gitlab.com/qemu-project/qemu/-/commit/6328d8ffa6cb9d750e4bfcfd73ac25d3a39ceb63

Thanks,

C.





Re: [RFC PATCH-for-9.1 00/21] qapi: Make @query-cpu-definitions command target-agnostic

2024-03-26 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> Hi Alex, Markus,
>
> Markus mentioned QAPI problems with the heterogeneous emulation
> binary. My understanding is, while QAPI can use host-specific
> conditional (OS, library available, configure option), it
> shouldn't use target-specific ones.

"Shouldn't" is too strong in the current state of things.  It can be
awkward, though.

Target-specific macros may be used only in target-specific code,
i.e. code that is compiled per target.  To catch uses in
target-independent code, we poison them there; see exec/poison.h.

QAPI-generated .c are not normally compiled per target.  To enable use
of target-specific macros in conditionals, we compile .c generated QAPI
submodules named *-target.json per target.  This is a bit of a hack.

Since the same conditionals will also appear in the .h generated from
them, these headers can only be used in target-specific code.

Sometimes, these headers "infect" target-independent code: we need to
compile per target just for the headers.  Awkward.  I can dig out an
example if there's interest.

But what about possible future state of things?

The QAPI schema is compile-time static by design.  QAPI conditionals
permit adjusting the schema for build configuration.  Target-specific
conditionals adjust it for the build configuration of the
target-specific part.  Each QEMU binary contains just one target's
target-specific part.

However, a single QEMU binary will contain several target-specific
parts, one per target it supports.  The targets' QAPI schema adjustments
may conflict.

For a single binary, we need to resolve these conflicts.

Special case: QAPI definitions that exist only for some targets.

Example: command query-sev exists only for TARGET_I386.  It's actually a
stub when CONFIG_SEV is off.

Obvious solution: make it exist if it needs to exist for any target
compiled into the binary.  Requires command stubs for the other targets.

Example: query-sev now exists when the single binary supports x86.  It's
a stub when CONFIG_SEV is off.  It behaves like a stub when CONFIG_SEV
is on, but the machine isn't x86.

Drawback: introspection with query-qmp-schema becomes less precise.
When the binary supports just one target, introspection can answer
target-specific questions like "does this target support SEV?"  With a
single binary, that's no longer possible.

Harmless enough for SEV, but consider query-cpu-model-expansion.  The
command may support expansion types "full" and "static".  Currently,
s390x supports both, ARM supports only "full", Loongarch only "static",
RISC-V only "full", and x86 supports both.

(I think.  The documentation is incomplete:

# Some architectures may not support all expansion types.  s390x
# supports "full" and "static". Arm only supports "full".
)

A management application may want to find out which expansion type is
supported.  Right now, it can't.  But we could improve the schema so it
can find out via introspection: define the expansion types only when
they're actually supported.

With a single binary, that's no longer possible: we have to define an
expansion type when *any* target supports it.

Such loss of introspection power is not a show-stopper, just something
we need to keep in mind.

> This series is an example on how to remove target specific
> bits from the @query-cpu-definitions command.

This is an instance of the special case with an additional twist: each
target defines its own QMP command handler.

>   Target specific
> code is registered as CPUClass handlers, then a generic method
> is used, iterating over all targets built in.
>
> The first set of patches were already posted / reviewed last
> year.
>
> The PPC and S390X targets still need work (help welcomed),
> however the code is useful enough to be tested and see if this
> is a good approach.
>
> The only drawback is a change in QAPI introspection, because
> targets not implementing @query-cpu-definitions were returning
> "CommandNotFound".

The change is introspection is actually something else.  Before the
series, query-qmp-schema returns a description of command
query-cpu-definitions exactly when the (single) target supports it.
Afterwards, it always returns one (I think).

The CommandNotFound change is a change in behavior when you try to
execute query-cpu-definitions, but the target doesn't support it.
Before, the command doesn't exist, and the QMP core duly replies
CommandNotFound.  Afterwards, it does exist, but the target doesn't
implement the call back, so the command fails.  I guess it fails with
GenericError.  We could make it fail with CommandNotFound if we care.

>My view is this was an incomplete
> implementation, rather than a feature.

Feels fair (but I'm not an expert in this area).




Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning

2024-03-26 Thread Peter Maydell
On Tue, 26 Mar 2024 at 09:54, Philippe Mathieu-Daudé  wrote:
>
> On 26/3/24 04:33, Pierrick Bouvier wrote:
> > On 3/26/24 05:52, Yao Xingtao wrote:
> >> 1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
> >> Use g_pattern_spec_match_string() instead to avoid this problem.
> >>
> >> 2. The type of second parameter in g_ptr_array_add() is
> >> 'gpointer' {aka 'void *'}, but the type of reg->name is 'const
> >> char*'.
> >> Cast the type of reg->name to 'gpointer' to avoid this problem.
> >>
> >> compiler warning message:
> >> /root/qemu/contrib/plugins/execlog.c:330:17: warning:
> >> ‘g_pattern_match_string’
> >> is deprecated: Use 'g_pattern_spec_match_string'
> >> instead [-Wdeprecated-declarations]
> >>330 | if (g_pattern_match_string(pat, rd->name) ||
> >>| ^~
> >> In file included from /usr/include/glib-2.0/glib.h:67,
> >>   from /root/qemu/contrib/plugins/execlog.c:9:
> >> /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
> >> 57 | gboolean  g_pattern_match_string   (GPatternSpec *pspec,
> >>|   ^~
> >> /root/qemu/contrib/plugins/execlog.c:331:21: warning:
> >> ‘g_pattern_match_string’
> >> is deprecated: Use 'g_pattern_spec_match_string'
> >> instead [-Wdeprecated-declarations]
> >>331 | g_pattern_match_string(pat, rd_lower)) {
> >>| ^~
> >> /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
> >> 57 | gboolean  g_pattern_match_string   (GPatternSpec *pspec,
> >>|   ^~
> >> /root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument
> >> 2 of
> >> ‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type
> >> [-Wdiscarded-qualifiers]
> >>339 | g_ptr_array_add(all_reg_names,
> >> reg->name);
> >>|
> >> ~~~^~
> >> In file included from /usr/include/glib-2.0/glib.h:33:
> >> /usr/include/glib-2.0/glib/garray.h:198:62: note: expected
> >> ‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’
> >>198 |gpointer
> >> data);
> >>|
> >> ~~^~~~
> >>
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
> >> Signed-off-by: Yao Xingtao 
> >> ---
> >>   contrib/plugins/execlog.c | 24 +---
> >>   1 file changed, 21 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> >> index a1dfd59ab7..fab18113d4 100644
> >> --- a/contrib/plugins/execlog.c
> >> +++ b/contrib/plugins/execlog.c
> >> @@ -311,6 +311,24 @@ static Register
> >> *init_vcpu_register(qemu_plugin_reg_descriptor *desc)
> >>   return reg;
> >>   }
> >> +/*
> >> + * g_pattern_match_string has been deprecated in Glib since 2.70 and
> >> + * will complain about it if you try to use it. Fortunately the
> >> + * signature of both functions is the same making it easy to work
> >> + * around.
> >> + */
> >> +static inline
> >> +gboolean g_pattern_spec_match_string_qemu(GPatternSpec *pspec,
> >> +  const gchar *string)
> >> +{
> >> +#if GLIB_CHECK_VERSION(2, 70, 0)
> >> +return g_pattern_spec_match_string(pspec, string);
> >> +#else
> >> +return g_pattern_match_string(pspec, string);
> >> +#endif
> >> +};
> >> +#define g_pattern_spec_match_string(p, s)
> >> g_pattern_spec_match_string_qemu(p, s)
> >> +
> >>   static GPtrArray *registers_init(int vcpu_index)
> >>   {
> >>   g_autoptr(GPtrArray) registers = g_ptr_array_new();
> >> @@ -327,8 +345,8 @@ static GPtrArray *registers_init(int vcpu_index)
> >>   for (int p = 0; p < rmatches->len; p++) {
> >>   g_autoptr(GPatternSpec) pat =
> >> g_pattern_spec_new(rmatches->pdata[p]);
> >>   g_autofree gchar *rd_lower =
> >> g_utf8_strdown(rd->name, -1);
> >> -if (g_pattern_match_string(pat, rd->name) ||
> >> -g_pattern_match_string(pat, rd_lower)) {
> >> +if (g_pattern_spec_match_string(pat, rd->name) ||
> >> +g_pattern_spec_match_string(pat, rd_lower)) {
> >>   Register *reg = init_vcpu_register(rd);
> >>   g_ptr_array_add(registers, reg);
> >> @@ -336,7 +354,7 @@ static GPtrArray *registers_init(int vcpu_index)
> >>   if (disas_assist) {
> >>   g_mutex_lock(&add_reg_name_lock);
> >>   if (!g_ptr_array_find(all_reg_names,
> >> reg->name, NULL)) {
> >> -g_ptr_array_add(all_reg_names, reg->name);
> >> +g_ptr_array_add(all_reg_names,
> >> (gpointer)reg->name);
> >>   }
> >>   g_mutex_unlock(&add_reg_name_lock);
> >>

Re: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

2024-03-26 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> "target/foo/cpu-qom.h" can not use any target specific definitions.
>
> Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
> on the i386/x86_64 build type. This doesn't scale in a heterogeneous
> context where we need to access both types concurrently.
>
> In order to do that, introduce the new I386_CPU / X86_64_CPU
> types, both inheriting a common TYPE_X86_CPU base type.
>
> Keep the current "base" and "max" CPU types as 32 or 64-bit,
> depending on the binary built.
>
> Adapt the cpu-plug-test, since the 'base' architecture is now
> common to both 32/64-bit x86 targets.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> Acked-by: Richard Henderson 

[...]

> diff --git a/tests/qtest/cpu-plug-test.c b/tests/qtest/cpu-plug-test.c
> index 7f5dd5f85a..97316d131f 100644
> --- a/tests/qtest/cpu-plug-test.c
> +++ b/tests/qtest/cpu-plug-test.c
> @@ -90,7 +90,7 @@ static void add_pc_test_case(const char *mname)
>  data->machine = g_strdup(mname);
>  data->cpu_model = "Haswell"; /* 1.3+ theoretically */
>  data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
> - qtest_get_arch());
> + qtest_get_base_arch());
>  data->sockets = 1;
>  data->cores = 3;
>  data->threads = 2;

Doesn't build for me:

../tests/qtest/cpu-plug-test.c: In function ‘add_pc_test_case’:
../tests/qtest/cpu-plug-test.c:93:42: error: implicit declaration of function 
‘qtest_get_base_arch’; did you mean ‘qtest_get_arch’? 
[-Werror=implicit-function-declaration]
   93 |  qtest_get_base_arch());
  |  ^~~
  |  qtest_get_arch
../tests/qtest/cpu-plug-test.c:93:42: error: nested extern declaration of 
‘qtest_get_base_arch’ [-Werror=nested-externs]
../tests/qtest/cpu-plug-test.c:92:47: error: format ‘%s’ expects argument of 
type ‘char *’, but argument 3 has type ‘int’ [-Werror=format=]
   92 | data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
  |  ~^
  |   |
  |   char *
  |  %d
   93 |  qtest_get_base_arch());
  |  ~
  |  |
  |  int




Question about block graph lock limitation with generated co-wrappers

2024-03-26 Thread Fiona Ebner
Hi,
we have a custom block driver downstream, which currently calls
bdrv_get_info() (for its file child) in the bdrv_refresh_limits()
callback. However, with graph locking, this doesn't work anymore.
AFAICT, the reason is the following:

The block driver has a backing file option.
During initialization, in bdrv_set_backing_hd(), the graph lock is
acquired exclusively.
Then the bdrv_refresh_limits() callback is invoked.
Now bdrv_get_info() is called, which is a generated co-wrapper.
The bdrv_co_get_info_entry() function tries to acquire the graph lock
for reading, sees that has_writer is true and so the coroutine will be
put to wait, leading to a deadlock.

For my specific case, I can move the bdrv_get_info() call to bdrv_open()
as a workaround. But I wanted to ask if there is a way to make generated
co-wrappers inside an exclusively locked section work? And if not, could
we introduce/extend the annotations, so the compiler can catch this kind
of issue, i.e. calling a generated co-wrapper while in an exclusively
locked section?

Best Regards,
Fiona




Re: [RFC v2 2/5] hw/arm: Allow setting KVM vGIC maintenance IRQ

2024-03-26 Thread Eric Auger
Hi Peter,

On 3/5/24 17:46, Peter Maydell wrote:
> On Fri, 9 Feb 2024 at 16:00, Eric Auger  wrote:
>> From: Haibo Xu 
>>
>> Allow virt arm machine to set the intid for the KVM GIC maintenance
>> interrupt.
>>
>> Signed-off-by: Haibo Xu 
>> Signed-off-by: Miguel Luis 
>> Signed-off-by: Eric Auger 
>>
>> ---
>> v1 -> v2:
>> - [Miguel] replaced the has_virt_extensions by the maintenance irq
>>   intid property. [Eric] restored kvm_device_check_attr and
>>   kvm_device_access standard usage and conditionally call those
>>   if the prop is set
> This seems reasonable, but it's not the same way we opted to
> handle telling the kernel the IRQ number for the PMU interrupt
> (where we use kvm_arm_pmu_set_irq()). I guess we have to do
> it this way because it's a device attr so we need to set it
> in gic realize, though?
>
> By the way, does the kernel automatically complain and fail
> if we try to enable nested-virt with a GICv2 or with a
> userspace GIC, or do we need to catch and produce error
> messages for those (invalid) combinations ourselves?
I don't think there is any check of that kind in Marc's series yet.
This may be added if GICv2 KVM device is created while kvm_mode is set
to KVM_MODE_NV.

Wrt userspace irqchip compat, KVM_CAP_ARM_USER_IRQ extension may not be
exposed in case of nested virt.

Marc, is that something you would like to integrate into the kernel series?

Thanks

Eric
>
> thanks
> -- PMM
>




Re: [PATCH v1] coroutine: avoid inserting duplicate coroutine to co_queue_wakeup

2024-03-26 Thread zhuyangyang via
On Mon, 25 Mar 2024 11:50:41 -0400, Stefan Hajnoczi wrote:
> On Mon, Mar 25, 2024 at 05:18:50PM +0800, zhuyangyang wrote:
> > If g_main_loop_run()/aio_poll() is called in the coroutine context,
> > the pending coroutine may be woken up repeatedly, and the co_queue_wakeup
> > may be disordered.
> 
> aio_poll() must not be called from coroutine context:
> 
>   bool no_coroutine_fn aio_poll(AioContext *ctx, bool blocking);
>^^^
> 
> Coroutines are not supposed to block. Instead, they should yield.
> 
> > When the poll() syscall exited in g_main_loop_run()/aio_poll(), it means
> > some listened events is completed. Therefore, the completion callback
> > function is dispatched.
> > 
> > If this callback function needs to invoke aio_co_enter(), it will only
> > wake up the coroutine (because we are already in coroutine context),
> > which may cause that the data on this listening event_fd/socket_fd
> > is not read/cleared. When the next poll () exits, it will be woken up again
> > and inserted into the wakeup queue again.
> > 
> > For example, if TLS is enabled in NBD, the server will call 
> > g_main_loop_run()
> > in the coroutine, and repeatedly wake up the io_read event on a socket.
> > The call stack is as follows:
> > 
> > aio_co_enter()
> > aio_co_wake()
> > qio_channel_restart_read()
> > aio_dispatch_handler()
> > aio_dispatch_handlers()
> > aio_dispatch()
> > aio_ctx_dispatch()
> > g_main_context_dispatch()
> > g_main_loop_run()
> > nbd_negotiate_handle_starttls()
> 
> This code does not look like it was designed to run in coroutine
> context. Two options:
> 
> 1. Don't run it in coroutine context (e.g. use a BH instead). This
>avoids blocking the coroutine but calling g_main_loop_run() is still
>ugly, in my opinion.
> 
> 2. Get rid of data.loop and use coroutine APIs instead:
> 
>while (!data.complete) {
>qemu_coroutine_yield();
>}
> 
>and update nbd_tls_handshake() to call aio_co_wake(data->co) instead
>of g_main_loop_quit(data->loop).
> 
>This requires auditing the code to check whether the event loop might
>invoke something that interferes with
>nbd_negotiate_handle_starttls(). Typically this means monitor
>commands or fd activity that could change the state of this
>connection while it is yielded. This is where the real work is but
>hopefully it will not be that hard to figure out.

Thank you for your help, I'll try to fix it using the coroutine api.

> 
> > nbd_negotiate_options()
> > nbd_negotiate()
> > nbd_co_client_start()
> > coroutine_trampoline()
> > 

-- 
Best Regards,
Zhu Yangyang



Re: [PATCH] virt: set the CPU type in BOOTINFO

2024-03-26 Thread Philippe Mathieu-Daudé

On 23/2/24 16:57, Laurent Vivier wrote:

BI_CPUTYPE/BI_MMUTYPE/BI_FPUTYPE were statically assigned to the
68040 information.
This patch changes the code to set in bootinfo the information
provided by the command line '-cpu' parameter.

Bug: https://gitlab.com/qemu-project/qemu/-/issues/2091
Reported-by: Daniel Palmer 
Signed-off-by: Laurent Vivier 
---
  hw/m68k/virt.c | 17 ++---
  1 file changed, 14 insertions(+), 3 deletions(-)


Merged as e39a0809b9.




Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning

2024-03-26 Thread Philippe Mathieu-Daudé

On 26/3/24 11:33, Peter Maydell wrote:

On Tue, 26 Mar 2024 at 09:54, Philippe Mathieu-Daudé  wrote:


On 26/3/24 04:33, Pierrick Bouvier wrote:

On 3/26/24 05:52, Yao Xingtao wrote:

1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
 Use g_pattern_spec_match_string() instead to avoid this problem.

2. The type of second parameter in g_ptr_array_add() is
 'gpointer' {aka 'void *'}, but the type of reg->name is 'const
char*'.
 Cast the type of reg->name to 'gpointer' to avoid this problem.

compiler warning message:
/root/qemu/contrib/plugins/execlog.c:330:17: warning:
‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
330 | if (g_pattern_match_string(pat, rd->name) ||
| ^~
In file included from /usr/include/glib-2.0/glib.h:67,
   from /root/qemu/contrib/plugins/execlog.c:9:
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
 57 | gboolean  g_pattern_match_string   (GPatternSpec *pspec,
|   ^~
/root/qemu/contrib/plugins/execlog.c:331:21: warning:
‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
331 | g_pattern_match_string(pat, rd_lower)) {
| ^~
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
 57 | gboolean  g_pattern_match_string   (GPatternSpec *pspec,
|   ^~
/root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument
2 of
‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
339 | g_ptr_array_add(all_reg_names,
reg->name);
|
~~~^~
In file included from /usr/include/glib-2.0/glib.h:33:
/usr/include/glib-2.0/glib/garray.h:198:62: note: expected
‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’
198 |gpointer
data);
|
~~^~~~

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
Signed-off-by: Yao Xingtao 
---
   contrib/plugins/execlog.c | 24 +---
   1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..fab18113d4 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -311,6 +311,24 @@ static Register
*init_vcpu_register(qemu_plugin_reg_descriptor *desc)
   return reg;
   }
+/*
+ * g_pattern_match_string has been deprecated in Glib since 2.70 and
+ * will complain about it if you try to use it. Fortunately the
+ * signature of both functions is the same making it easy to work
+ * around.
+ */
+static inline
+gboolean g_pattern_spec_match_string_qemu(GPatternSpec *pspec,
+  const gchar *string)
+{
+#if GLIB_CHECK_VERSION(2, 70, 0)
+return g_pattern_spec_match_string(pspec, string);
+#else
+return g_pattern_match_string(pspec, string);
+#endif
+};
+#define g_pattern_spec_match_string(p, s)
g_pattern_spec_match_string_qemu(p, s)
+
   static GPtrArray *registers_init(int vcpu_index)
   {
   g_autoptr(GPtrArray) registers = g_ptr_array_new();
@@ -327,8 +345,8 @@ static GPtrArray *registers_init(int vcpu_index)
   for (int p = 0; p < rmatches->len; p++) {
   g_autoptr(GPatternSpec) pat =
g_pattern_spec_new(rmatches->pdata[p]);
   g_autofree gchar *rd_lower =
g_utf8_strdown(rd->name, -1);
-if (g_pattern_match_string(pat, rd->name) ||
-g_pattern_match_string(pat, rd_lower)) {
+if (g_pattern_spec_match_string(pat, rd->name) ||
+g_pattern_spec_match_string(pat, rd_lower)) {
   Register *reg = init_vcpu_register(rd);
   g_ptr_array_add(registers, reg);
@@ -336,7 +354,7 @@ static GPtrArray *registers_init(int vcpu_index)
   if (disas_assist) {
   g_mutex_lock(&add_reg_name_lock);
   if (!g_ptr_array_find(all_reg_names,
reg->name, NULL)) {
-g_ptr_array_add(all_reg_names, reg->name);
+g_ptr_array_add(all_reg_names,
(gpointer)reg->name);
   }
   g_mutex_unlock(&add_reg_name_lock);
   }


Would be nice if it's still possible to merge this in 9.0 Peter.


I will post a small PR later today, so until Peter has something
else planned, I can take it, since the patch LGTM now.


That would be great (I don't have any more patches I wanted
to put in a PR).

Reviewed-by: Peter Maydell 


OK, patch queued then.

Yao, for your future contributions, please post patch iterations
as new thread rather than replying to previous v

Re: [PATCH] misc/pca955*: Move models under hw/gpio

2024-03-26 Thread Philippe Mathieu-Daudé

On 26/3/24 11:16, Cédric Le Goater wrote:

On 3/26/24 10:55, Philippe Mathieu-Daudé wrote:

On 25/3/24 14:48, Cédric Le Goater wrote:

The PCA9552 and PCA9554 devices are both I2C GPIO controllers and the
PCA9552 also can drive LEDs. Do all the necessary adjustments to move
the models under hw/gpio.

Cc: Glenn Miles 
Signed-off-by: Cédric Le Goater 
---
  MAINTAINERS  | 4 ++--
  include/hw/{misc => gpio}/pca9552.h  | 0
  include/hw/{misc => gpio}/pca9552_regs.h | 0
  include/hw/{misc => gpio}/pca9554.h  | 0
  include/hw/{misc => gpio}/pca9554_regs.h | 0
  hw/arm/aspeed.c  | 2 +-
  hw/{misc => gpio}/pca9552.c  | 4 ++--
  hw/{misc => gpio}/pca9554.c  | 4 ++--
  tests/qtest/pca9552-test.c   | 2 +-
  tests/qtest/pnv-host-i2c-test.c  | 4 ++--
  hw/gpio/meson.build  | 2 ++
  hw/gpio/trace-events | 4 
  hw/misc/meson.build  | 2 --
  hw/misc/trace-events | 4 
  14 files changed, 16 insertions(+), 16 deletions(-)
  rename include/hw/{misc => gpio}/pca9552.h (100%)
  rename include/hw/{misc => gpio}/pca9552_regs.h (100%)
  rename include/hw/{misc => gpio}/pca9554.h (100%)
  rename include/hw/{misc => gpio}/pca9554_regs.h (100%)
  rename hw/{misc => gpio}/pca9552.c (99%)
  rename hw/{misc => gpio}/pca9554.c (99%)


Thanks, patch queued.


This one is merged,

https://gitlab.com/qemu-project/qemu/-/commit/6328d8ffa6cb9d750e4bfcfd73ac25d3a39ceb63


Yes I just realized when updating my tree that Thomas sent a PR with
your patches. Sorry for the noise.



Thanks,

C.







Re: [PULL 00/15] riscv-to-apply queue

2024-03-26 Thread Daniel Henrique Barboza




On 3/26/24 06:56, Alistair Francis wrote:

On Tue, Mar 26, 2024 at 7:53 PM Michael Tokarev  wrote:


On 24.03.2024 21:12, Daniel Henrique Barboza wrote:

On 3/24/24 12:07, Michael Tokarev wrote:



Unfortunately this doesn't quite work, the following changes
fail to apply to 8.2:

929e521a47 target/riscv: always clear vstart for ldst_whole insns
b46631f122 target/riscv: remove 'over' brconds from vector trans
d57dfe4b37 trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
bac802ada8 target/riscv: enable 'vstart_eq_zero' in the end of insns
385e575cd5 target/riscv/kvm: fix timebase-frequency when using KVM acceleration



The amount of work can be non-trivial for this backport, so I'd say we should
leave it aside for now. If someone has a good argument for this work then we
can re-evaluate.


So, out of 15 patches in this series (minus the first one already
mentioned) - should I pick 9 remaining patches for stable (the ones
which applies) or none at all? :)


Sorry for the confusion.

The 9 patches that applied and

385e575cd5 target/riscv/kvm: fix timebase-frequency when using KVM acceleration

should all be picked for stable.

PS: What is the best way in future to help ease some of the stable
burden? Should I try and cherry pick them beforehand and then mention
that as a follow up to the PR?


We believe your judgement about what should or shouldn't be in stable, so IMO 
you can
be pro-active into cherry picking fixes into stable and mention it in the PR.


Thanks,

Daniel



Alistair



Thanks,

/mjt




Re: [PATCH] tests/qemu-iotests: Test 157 and 227 require virtio-blk

2024-03-26 Thread Kevin Wolf
Am 25.03.2024 um 16:47 hat Thomas Huth geschrieben:
> Tests 157 and 227 use the virtio-blk device, so we have to mark these
> tests accordingly to be skipped if this devices is not available (e.g.
> when running the tests with qemu-system-avr only).
> 
> Signed-off-by: Thomas Huth 

Thanks, applied to the block branch.

Kevin




Re: [PATCH-for-9.1 09/21] qapi: Merge machine-common.json with qapi/machine.json

2024-03-26 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> machine-common.json declares a single type, which isn't
> restricted to a particular target. Move this type in
> machine.json.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Previous discussion at
https://lore.kernel.org/qemu-devel/875y45kt8i@pond.sub.org/

CpuS390Entitlement is specific to s390x.  We need it for
set-cpu-topology and the s390x-specific part of query-cpus-fast.  The
former is conditional on TARGET_S390X, and therefore must be in
machine-target.json.  The latter is not conditional, and in
machine.json.  If I remember correctly, I briefly explored making it
conditional, but it was too messy to be worth the bother.

Anyway.  We have a target-specific type we want to use both in
machine.json and machine-target.json.  Neither of the two includes the
other.  Target-independent machine.json cannot include target-specific
machine-target.json.  Two solutions:

1. Define the type in machine.json, have machine-target.json include
machine.json.  Ugly: even more target-stuff in machine.json.
Potentially slow: including qapi/qapi-*-machine-target.h now includes
more.

2. Define the type in a submodule both include.  Since nothing they
include is a fitting home for the type, create one: machine-common.json.
Ugly & potentially slow: yet another submodule.

Nina chose to do 2.

I figure I'd leave it as is until we get to the point where we can get
rid of the *-target.json entirely.




Re: [PATCH-for-9.1 10/21] qapi: Make CpuModel* definitions target agnostic

2024-03-26 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> CpuModelInfo, CpuModelExpansionType and CpuModelCompareResult
> are not restricted to any particular target. Define them in
> machine.json to generate them once.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Well, the stuff in machine-target.json is generated once just fine.  But
that single copy is then compiled once per target.

On the one hand, less target-specific code is good.  On the other hand,
I'm reluctant to move these types away from their users.




Re: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

2024-03-26 Thread Philippe Mathieu-Daudé

On 26/3/24 11:57, Markus Armbruster wrote:

Philippe Mathieu-Daudé  writes:


"target/foo/cpu-qom.h" can not use any target specific definitions.

Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
on the i386/x86_64 build type. This doesn't scale in a heterogeneous
context where we need to access both types concurrently.

In order to do that, introduce the new I386_CPU / X86_64_CPU
types, both inheriting a common TYPE_X86_CPU base type.

Keep the current "base" and "max" CPU types as 32 or 64-bit,
depending on the binary built.

Adapt the cpu-plug-test, since the 'base' architecture is now
common to both 32/64-bit x86 targets.

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Richard Henderson 


[...]


diff --git a/tests/qtest/cpu-plug-test.c b/tests/qtest/cpu-plug-test.c
index 7f5dd5f85a..97316d131f 100644
--- a/tests/qtest/cpu-plug-test.c
+++ b/tests/qtest/cpu-plug-test.c
@@ -90,7 +90,7 @@ static void add_pc_test_case(const char *mname)
  data->machine = g_strdup(mname);
  data->cpu_model = "Haswell"; /* 1.3+ theoretically */
  data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
- qtest_get_arch());
+ qtest_get_base_arch());
  data->sockets = 1;
  data->cores = 3;
  data->threads = 2;


Doesn't build for me:

../tests/qtest/cpu-plug-test.c: In function ‘add_pc_test_case’:
../tests/qtest/cpu-plug-test.c:93:42: error: implicit declaration of function 
‘qtest_get_base_arch’; did you mean ‘qtest_get_arch’? 
[-Werror=implicit-function-declaration]
93 |  qtest_get_base_arch());
   |  ^~~
   |  qtest_get_arch
../tests/qtest/cpu-plug-test.c:93:42: error: nested extern declaration of 
‘qtest_get_base_arch’ [-Werror=nested-externs]
../tests/qtest/cpu-plug-test.c:92:47: error: format ‘%s’ expects argument of 
type ‘char *’, but argument 3 has type ‘int’ [-Werror=format=]
92 | data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
   |  ~^
   |   |
   |   char *
   |  %d
93 |  qtest_get_base_arch());
   |  ~
   |  |
   |  int


Sorry, I forgot to mention this series is based on:
https://lore.kernel.org/qemu-devel/20231010074952.79165-1-phi...@linaro.org/
"qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits()"



Re: [PATCH v1] coroutine: avoid inserting duplicate coroutine to co_queue_wakeup

2024-03-26 Thread zhuyangyang via
On Mon, 25 Mar 2024 11:00:31 -0500 Eric Blake wrote:
> >  util/async.c | 13 -
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/util/async.c b/util/async.c
> > index 0467890052..25fc1e6083 100644
> > --- a/util/async.c
> > +++ b/util/async.c
> > @@ -705,7 +705,18 @@ void aio_co_enter(AioContext *ctx, Coroutine *co)
> >  if (qemu_in_coroutine()) {
> >  Coroutine *self = qemu_coroutine_self();
> >  assert(self != co);
> > -QSIMPLEQ_INSERT_TAIL(&self->co_queue_wakeup, co, co_queue_next);
> > +/*
> > + * If the Coroutine *co is already in the co_queue_wakeup, this
> > + * repeated insertion will causes the loss of other queue element
> 
> s/causes/cause/
> 
> > + * or infinite loop.
> > + * For examplex:
> 
> s/examplex/example/
> 
> > + * Head->a->b->c->NULL, after insert_tail(head, b) => 
> > Head->a->b->NULL
> > + * Head->a-b>->NULL, after insert_tail(head, b) => Head->a->b->b...
> 
> s/b>->/b->/
> 
> > + */
> > +if (!co->co_queue_next.sqe_next &&
> > +self->co_queue_wakeup.sqh_last != &co->co_queue_next.sqe_next) 
> > {
> > +QSIMPLEQ_INSERT_TAIL(&self->co_queue_wakeup, co, 
> > co_queue_next);
> > +}
> >  } else {
> >  qemu_aio_coroutine_enter(ctx, co);
> >  }
> 
> Intuitively, attacking the symptoms (avoiding bogus list insertion
> when it is already on the list) makes sense; but I want to make sure
> we attack the root cause.

Repairing the nbd_negotiate_handle_starttls() can solve this problem, therefore,
I'm not sure if this commit is still needed.

--
Best Regards,
Zhu Yangyang




[PATCH trivial for-9.0] smbios: add stub for smbios_get_table_legacy()

2024-03-26 Thread Igor Mammedov
QEMU build fails with
  hw/i386/fw_cfg.c:74: undefined reference to `smbios_get_table_legacy'
when it's built with only 'microvm' enabled i.e. with config patch
   +++ b/configs/devices/i386-softmmu/default.mak
   @@ -26,7 +26,7 @@

   # Boards:
   #
   -CONFIG_ISAPC=y
   -CONFIG_I440FX=y
   -CONFIG_Q35=y
   +CONFIG_ISAPC=n
   +CONFIG_I440FX=n
   +CONFIG_Q35=n

it happens because I've fogotten/lost smbios_get_table_legacy() stub.

Fix it by adding missing stub as Philippe suggested.

Fixes: b42b0e4daaa5 "smbios: build legacy mode code only for 'pc' machine"
Reported-by: Michael Tokarev 
Singned-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Igor Mammedov 
---
Compile tested only.

While it's fixing bug for off-tree usecase with non-upstream config,
it's trivial enough to go into 9.0 if time frame allows.
Benefit of it going into 9.0 is that folks who play with minimal builds
won't have to carry the patch in their tree.


 hw/smbios/smbios_legacy_stub.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/smbios/smbios_legacy_stub.c b/hw/smbios/smbios_legacy_stub.c
index f29b15316c..7d593dca98 100644
--- a/hw/smbios/smbios_legacy_stub.c
+++ b/hw/smbios/smbios_legacy_stub.c
@@ -13,3 +13,8 @@
 void smbios_add_usr_blob_size(size_t size)
 {
 }
+
+uint8_t *smbios_get_table_legacy(size_t *length, Error **errp)
+{
+g_assert_not_reached();
+}
-- 
2.43.0




Re: [PATCH v3 7/8] plugins: distinct types for callbacks

2024-03-26 Thread Pierrick Bouvier

On 3/25/24 23:23, Richard Henderson wrote:

On 3/25/24 02:41, Pierrick Bouvier wrote:

To prevent errors when writing new types of callbacks or inline
operations, we split callbacks data to distinct types.

Signed-off-by: Pierrick Bouvier 
---
   include/qemu/plugin.h  | 46 ++---
   plugins/plugin.h   |  2 +-
   accel/tcg/plugin-gen.c | 58 +---
   plugins/core.c | 76 ++
   4 files changed, 98 insertions(+), 84 deletions(-)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index bb224b8e4c7..a078229942f 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -73,34 +73,40 @@ enum plugin_dyn_cb_type {
   PLUGIN_CB_INLINE_STORE_U64,
   };
   
+struct qemu_plugin_regular_cb {

+union qemu_plugin_cb_sig f;
+TCGHelperInfo *info;
+void *userp;
+enum qemu_plugin_mem_rw rw;
+};
+
+struct qemu_plugin_inline_cb {
+qemu_plugin_u64 entry;
+enum qemu_plugin_op op;
+uint64_t imm;
+enum qemu_plugin_mem_rw rw;
+};


Do you still need 'op' anymore here?
It seems redundant with 'type'.



You're right, removed it in a new commit, will post new series.


Otherwise,
Reviewed-by: Richard Henderson 


r~




Re: [PATCH trivial for-9.0] hw/i386/fw_cfg.c: fix non-legacy smbios build

2024-03-26 Thread Igor Mammedov
On Mon, 25 Mar 2024 21:01:42 +0300
Michael Tokarev  wrote:

> 25.03.2024 18:20, Igor Mammedov wrote
> > On Mon, 25 Mar 2024 16:09:20 +0300
> > Michael Tokarev  wrote:
> >   
> >> When building qemu with smbios but not legacy mode (eg minimal microvm 
> >> build),
> >> link fails with:
> >>
> >>hw/i386/fw_cfg.c:74: undefined reference to `smbios_get_table_legacy'
> >>
> >> This is because fw_cfg interface can call this function if CONFIG_SMBIOS
> >> is defined.  Made this code block to depend on CONFIG_SMBIOS_LEGACY.  
> > 
> > stub supposedly should have handled that
> > what configure options do you use to build 'minimal microvm'?  
> 
> This is a custom build, not only configure options but also custom
> devices.mak: 
> https://salsa.debian.org/qemu-team/qemu/-/blob/master/debian/microvm-devices.mak
> 
> == cut ==
> # see configs/devices/i386-softmmu/default.mak
> # for additional devices which can be disabled
> #
> CONFIG_PCI_DEVICES=n
> # we can't disable all machine types (boards) as of 6.1
> # since the resulting binary fails to link
> #CONFIG_ISAPC=y
> #CONFIG_I440FX=y
> CONFIG_Q35=y
> CONFIG_MICROVM=y
> CONFIG_VIRTIO_BLK=y
> CONFIG_VIRTIO_SERIAL=y
> CONFIG_VIRTIO_INPUT=y
> CONFIG_VIRTIO_INPUT_HOST=y
> CONFIG_VHOST_USER_INPUT=y
> CONFIG_VIRTIO_NET=y
> CONFIG_VIRTIO_SCSI=y
> CONFIG_VIRTIO_RNG=y
> CONFIG_VIRTIO_CRYPTO=y
> CONFIG_VIRTIO_BALLOON=y
> CONFIG_VIRTIO_MEM=y
> CONFIG_VIRTIO_PMEM=y
> CONFIG_VIRTIO_GPU=y
> CONFIG_VHOST_USER_GPU=y
> == cut ==
> 
> Relevant configure options:
> https://salsa.debian.org/qemu-team/qemu/-/blob/master/debian/rules#L293-308
> 
>   ../../configure ${common_configure_opts} \
>   --extra-cflags="${extra-cflags} -DCONFIG_MICROVM_DEFAULT=1" \
>   --without-default-features \
>   --target-list=x86_64-softmmu --enable-kvm --disable-tcg \
>   --enable-pixman --enable-vnc \
>   --enable-attr \
>   --enable-coroutine-pool \
>   --audio-drv-list="" \
>   --without-default-devices \
>   --with-devices-x86_64=microvm \
>   --enable-vhost-kernel --enable-vhost-net \
>   --enable-vhost-vdpa \
>   --enable-vhost-user --enable-vhost-user-blk-server \
>   --enable-vhost-crypto \
> 
> I dunno how relevant these are, - it come from ubuntu and I haven't
> looked there for a long time.  The idea was to have a build especially
> for microvm with minimal footprint, as light as possible, for fastest
> startup time etc.
> 
> Enabling (selecting) CONFIG_SMBIOS does not help since it is already
> enabled by something, but not SMBIOS_LEGACY (which should not be
> enabled in this case).

I'll look into what pulls in fwcfg and SMBIOS into microvm only config

> I still think it is better to avoid pcmc->smbios_legacy_mode variable
> (field) entirely.

Defines are usually frowned upon in QEMU and stubs
are typically preferred way to go.

I've just sent a patch adding a missing stub,
fill free to take it via your tree if time permits.

> 
> Thanks,
> 
> /mjt
> 
> >>
> >> Fixes: b42b0e4daaa5 "smbios: build legacy mode code only for 'pc' machine"
> >> Signed-off-by: Michael Tokarev 
> >> ---
> >>   hw/i386/fw_cfg.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> >> index d802d2787f..d5e78a9183 100644
> >> --- a/hw/i386/fw_cfg.c
> >> +++ b/hw/i386/fw_cfg.c
> >> @@ -70,6 +70,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, 
> >> FWCfgState *fw_cfg,
> >>   /* tell smbios about cpuid version and features */
> >>   smbios_set_cpuid(cpu->env.cpuid_version, 
> >> cpu->env.features[FEAT_1_EDX]);
> >>   
> >> +#ifdef CONFIG_SMBIOS_LEGACY
> >>   if (pcmc->smbios_legacy_mode) {
> >>   smbios_tables = smbios_get_table_legacy(&smbios_tables_len,
> >>   &error_fatal);
> >> @@ -77,6 +78,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, 
> >> FWCfgState *fw_cfg,
> >>smbios_tables, smbios_tables_len);
> >>   return;
> >>   }
> >> +#endif
> >>   
> >>   /* build the array of physical mem area from e820 table */
> >>   mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());  
> > 
> >   
> 




[PATCH for 9.1 v2 2/2] meson: Fix MESONINTROSPECT parsing

2024-03-26 Thread Akihiko Odaki
The arguments in MESONINTROSPECT are quoted with shlex.quote() so it
must be parsed with shlex.split().

Fixes: cf60ccc330 ("cutils: Introduce bundle mechanism")
Reported-by: Michael Tokarev 
Reviewed-by: Michael Tokarev 
Tested-by: Michael Tokarev 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Akihiko Odaki 
---
 scripts/symlink-install-tree.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/symlink-install-tree.py b/scripts/symlink-install-tree.py
index 8ed97e3c943d..b72563895c56 100644
--- a/scripts/symlink-install-tree.py
+++ b/scripts/symlink-install-tree.py
@@ -4,6 +4,7 @@
 import errno
 import json
 import os
+import shlex
 import subprocess
 import sys
 
@@ -14,7 +15,7 @@ def destdir_join(d1: str, d2: str) -> str:
 return str(PurePath(d1, *PurePath(d2).parts[1:]))
 
 introspect = os.environ.get('MESONINTROSPECT')
-out = subprocess.run([*introspect.split(' '), '--installed'],
+out = subprocess.run([*shlex.split(introspect), '--installed'],
  stdout=subprocess.PIPE, check=True).stdout
 for source, dest in json.loads(out).items():
 bundle_dest = destdir_join('qemu-bundle', dest)

-- 
2.44.0




[PATCH v4 0/9] TCG plugins new inline operations

2024-03-26 Thread Pierrick Bouvier
This series implement two new operations for plugins:
- Store inline allows to write a specific value to a scoreboard.
- Conditional callback executes a callback only when a given condition is true.
  The condition is evaluated inline.

It's possible to mix various inline operations (add, store) with conditional
callbacks, allowing efficient "trap" based counters.

It builds on top of new scoreboard API, introduced in the previous series.

NOTE: Two patches still need review

v2
--

- fixed issue with udata not being passed to conditional callback
- added specific test for this in tests/plugin/inline.c (udata was NULL before).

v3
--

- rebased on top of "plugins: Rewrite plugin code generation":
  20240316015720.3661236-1-richard.hender...@linaro.org
- single pass code generation
- small cleanups for code generation

v4
--

- remove op field from qemu_plugin_inline_cb
- use tcg_constant_i64 to load immediate value to store

Pierrick Bouvier (9):
  plugins: prepare introduction of new inline ops
  plugins: extract generate ptr for qemu_plugin_u64
  plugins: add new inline op STORE_U64
  tests/plugin/inline: add test for STORE_U64 inline op
  plugins: conditional callbacks
  tests/plugin/inline: add test for conditional callback
  plugins: distinct types for callbacks
  plugins: extract cpu_index generate
  plugins: remove op from qemu_plugin_inline_cb

 include/qemu/plugin.h|  42 +++
 include/qemu/qemu-plugin.h   |  80 -
 plugins/plugin.h |  12 +++-
 accel/tcg/plugin-gen.c   | 136 +++
 plugins/api.c|  39 ++
 plugins/core.c   | 109 
 tests/plugin/inline.c| 130 +++--
 plugins/qemu-plugins.symbols |   2 +
 8 files changed, 466 insertions(+), 84 deletions(-)

-- 
2.39.2




[PATCH for 9.1 v2 1/2] buildsys: Bump known good meson version to v1.4.0

2024-03-26 Thread Akihiko Odaki
We need meson v1.4.0 to fix MESONINTROSPECT quoting on Windows:
https://github.com/mesonbuild/meson/pull/12807

Signed-off-by: Akihiko Odaki 
---
 python/scripts/vendor.py   |   4 ++--
 python/wheels/meson-1.2.3-py3-none-any.whl | Bin 964928 -> 0 bytes
 python/wheels/meson-1.4.0-py3-none-any.whl | Bin 0 -> 935471 bytes
 pythondeps.toml|   2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/python/scripts/vendor.py b/python/scripts/vendor.py
index 1038b14ae0c8..da463fbde557 100755
--- a/python/scripts/vendor.py
+++ b/python/scripts/vendor.py
@@ -41,8 +41,8 @@ def main() -> int:
 parser.parse_args()
 
 packages = {
-"meson==1.2.3":
-"4533a43c34548edd1f63a276a42690fce15bde9409bcf20c4b8fa3d7e4d7cac1",
+"meson==1.4.0":
+"476a458d51fcfa322a6bdc64da5138997c542d08e6b2e49b9fa68c46fd7c4475",
 
 "tomli==2.0.1":
 "939de3e7a6161af0c887ef91b7d41a53e7c5a1ca976325f429cb46ea9bc30ecc",
diff --git a/python/wheels/meson-1.2.3-py3-none-any.whl 
b/python/wheels/meson-1.2.3-py3-none-any.whl
deleted file mode 100644
index a8b84e5f114a..
Binary files a/python/wheels/meson-1.2.3-py3-none-any.whl and /dev/null differ
diff --git a/python/wheels/meson-1.4.0-py3-none-any.whl 
b/python/wheels/meson-1.4.0-py3-none-any.whl
new file mode 100644
index ..ca9adc3f024d
Binary files /dev/null and b/python/wheels/meson-1.4.0-py3-none-any.whl differ
diff --git a/pythondeps.toml b/pythondeps.toml
index 0e8841599935..4269decf0e3e 100644
--- a/pythondeps.toml
+++ b/pythondeps.toml
@@ -19,7 +19,7 @@
 
 [meson]
 # The install key should match the version in python/wheels/
-meson = { accepted = ">=0.63.0", installed = "1.2.3", canary = "meson" }
+meson = { accepted = ">=0.63.0", installed = "1.4.0", canary = "meson" }
 
 [docs]
 sphinx = { accepted = ">=1.6", installed = "5.3.0", canary = "sphinx-build" }

-- 
2.44.0




[PATCH v4 2/9] plugins: extract generate ptr for qemu_plugin_u64

2024-03-26 Thread Pierrick Bouvier
Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
 accel/tcg/plugin-gen.c | 27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 41d4d83f547..d3667203546 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -120,24 +120,33 @@ static void gen_udata_cb(struct qemu_plugin_dyn_cb *cb)
 tcg_temp_free_i32(cpu_index);
 }
 
-static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
+static TCGv_ptr gen_plugin_u64_ptr(qemu_plugin_u64 entry)
 {
-GArray *arr = cb->inline_insn.entry.score->data;
-size_t offset = cb->inline_insn.entry.offset;
-TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
-TCGv_i64 val = tcg_temp_ebb_new_i64();
 TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
 
+GArray *arr = entry.score->data;
+char *base_ptr = arr->data + entry.offset;
+size_t entry_size = g_array_get_element_size(arr);
+
+TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
 tcg_gen_ld_i32(cpu_index, tcg_env,
-offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
-tcg_gen_muli_i32(cpu_index, cpu_index, g_array_get_element_size(arr));
+tcg_gen_muli_i32(cpu_index, cpu_index, entry_size);
 tcg_gen_ext_i32_ptr(ptr, cpu_index);
 tcg_temp_free_i32(cpu_index);
+tcg_gen_addi_ptr(ptr, ptr, (intptr_t) base_ptr);
 
-tcg_gen_addi_ptr(ptr, ptr, (intptr_t)arr->data);
-tcg_gen_ld_i64(val, ptr, offset);
+return ptr;
+}
+
+static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
+{
+TCGv_ptr ptr = gen_plugin_u64_ptr(cb->inline_insn.entry);
+TCGv_i64 val = tcg_temp_ebb_new_i64();
+
+tcg_gen_ld_i64(val, ptr, 0);
 tcg_gen_addi_i64(val, val, cb->inline_insn.imm);
-tcg_gen_st_i64(val, ptr, offset);
+tcg_gen_st_i64(val, ptr, 0);
 
 tcg_temp_free_i64(val);
 tcg_temp_free_ptr(ptr);
-- 
2.39.2




[PATCH v4 3/9] plugins: add new inline op STORE_U64

2024-03-26 Thread Pierrick Bouvier
Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
 include/qemu/plugin.h  |  1 +
 include/qemu/qemu-plugin.h |  4 ++--
 accel/tcg/plugin-gen.c | 13 +
 plugins/core.c |  6 ++
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 23271fbe36a..d1d9b4490df 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -69,6 +69,7 @@ enum plugin_dyn_cb_type {
 PLUGIN_CB_REGULAR,
 PLUGIN_CB_MEM_REGULAR,
 PLUGIN_CB_INLINE_ADD_U64,
+PLUGIN_CB_INLINE_STORE_U64,
 };
 
 /*
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 4fc6c3739b2..c5cac897a0b 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -305,12 +305,12 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct 
qemu_plugin_tb *tb,
  * enum qemu_plugin_op - describes an inline op
  *
  * @QEMU_PLUGIN_INLINE_ADD_U64: add an immediate value uint64_t
- *
- * Note: currently only a single inline op is supported.
+ * @QEMU_PLUGIN_INLINE_STORE_U64: store an immediate value uint64_t
  */
 
 enum qemu_plugin_op {
 QEMU_PLUGIN_INLINE_ADD_U64,
+QEMU_PLUGIN_INLINE_STORE_U64,
 };
 
 /**
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index d3667203546..1cfd7908df1 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -152,6 +152,16 @@ static void gen_inline_add_u64_cb(struct 
qemu_plugin_dyn_cb *cb)
 tcg_temp_free_ptr(ptr);
 }
 
+static void gen_inline_store_u64_cb(struct qemu_plugin_dyn_cb *cb)
+{
+TCGv_ptr ptr = gen_plugin_u64_ptr(cb->inline_insn.entry);
+TCGv_i64 val = tcg_constant_i64(cb->inline_insn.imm);
+
+tcg_gen_st_i64(val, ptr, 0);
+
+tcg_temp_free_ptr(ptr);
+}
+
 static void gen_mem_cb(struct qemu_plugin_dyn_cb *cb,
qemu_plugin_meminfo_t meminfo, TCGv_i64 addr)
 {
@@ -177,6 +187,9 @@ static void inject_cb(struct qemu_plugin_dyn_cb *cb)
 case PLUGIN_CB_INLINE_ADD_U64:
 gen_inline_add_u64_cb(cb);
 break;
+case PLUGIN_CB_INLINE_STORE_U64:
+gen_inline_store_u64_cb(cb);
+break;
 default:
 g_assert_not_reached();
 }
diff --git a/plugins/core.c b/plugins/core.c
index a8557b54ff7..e1bf0dc3717 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -321,6 +321,8 @@ static enum plugin_dyn_cb_type op_to_cb_type(enum 
qemu_plugin_op op)
 switch (op) {
 case QEMU_PLUGIN_INLINE_ADD_U64:
 return PLUGIN_CB_INLINE_ADD_U64;
+case QEMU_PLUGIN_INLINE_STORE_U64:
+return PLUGIN_CB_INLINE_STORE_U64;
 default:
 g_assert_not_reached();
 }
@@ -535,6 +537,9 @@ void exec_inline_op(struct qemu_plugin_dyn_cb *cb, int 
cpu_index)
 case QEMU_PLUGIN_INLINE_ADD_U64:
 *val += cb->inline_insn.imm;
 break;
+case QEMU_PLUGIN_INLINE_STORE_U64:
+*val = cb->inline_insn.imm;
+break;
 default:
 g_assert_not_reached();
 }
@@ -562,6 +567,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
vaddr, cb->userp);
 break;
 case PLUGIN_CB_INLINE_ADD_U64:
+case PLUGIN_CB_INLINE_STORE_U64:
 exec_inline_op(cb, cpu->cpu_index);
 break;
 default:
-- 
2.39.2




[PATCH v4 4/9] tests/plugin/inline: add test for STORE_U64 inline op

2024-03-26 Thread Pierrick Bouvier
Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
 tests/plugin/inline.c | 41 +
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 0163e9b51c5..103c3a22f6e 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -22,6 +22,12 @@ typedef struct {
 uint64_t count_mem_inline;
 } CPUCount;
 
+typedef struct {
+uint64_t data_insn;
+uint64_t data_tb;
+uint64_t data_mem;
+} CPUData;
+
 static struct qemu_plugin_scoreboard *counts;
 static qemu_plugin_u64 count_tb;
 static qemu_plugin_u64 count_tb_inline;
@@ -29,6 +35,10 @@ static qemu_plugin_u64 count_insn;
 static qemu_plugin_u64 count_insn_inline;
 static qemu_plugin_u64 count_mem;
 static qemu_plugin_u64 count_mem_inline;
+static struct qemu_plugin_scoreboard *data;
+static qemu_plugin_u64 data_insn;
+static qemu_plugin_u64 data_tb;
+static qemu_plugin_u64 data_mem;
 
 static uint64_t global_count_tb;
 static uint64_t global_count_insn;
@@ -109,11 +119,13 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
 stats_mem();
 
 qemu_plugin_scoreboard_free(counts);
+qemu_plugin_scoreboard_free(data);
 }
 
 static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
 {
 qemu_plugin_u64_add(count_tb, cpu_index, 1);
+g_assert(qemu_plugin_u64_get(data_tb, cpu_index) == (uintptr_t) udata);
 g_mutex_lock(&tb_lock);
 max_cpu_index = MAX(max_cpu_index, cpu_index);
 global_count_tb++;
@@ -123,6 +135,7 @@ static void vcpu_tb_exec(unsigned int cpu_index, void 
*udata)
 static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
 {
 qemu_plugin_u64_add(count_insn, cpu_index, 1);
+g_assert(qemu_plugin_u64_get(data_insn, cpu_index) == (uintptr_t) udata);
 g_mutex_lock(&insn_lock);
 global_count_insn++;
 g_mutex_unlock(&insn_lock);
@@ -131,9 +144,10 @@ static void vcpu_insn_exec(unsigned int cpu_index, void 
*udata)
 static void vcpu_mem_access(unsigned int cpu_index,
 qemu_plugin_meminfo_t info,
 uint64_t vaddr,
-void *userdata)
+void *udata)
 {
 qemu_plugin_u64_add(count_mem, cpu_index, 1);
+g_assert(qemu_plugin_u64_get(data_mem, cpu_index) == (uintptr_t) udata);
 g_mutex_lock(&mem_lock);
 global_count_mem++;
 g_mutex_unlock(&mem_lock);
@@ -141,20 +155,34 @@ static void vcpu_mem_access(unsigned int cpu_index,
 
 static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
 {
+void *tb_store = tb;
+qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+tb, QEMU_PLUGIN_INLINE_STORE_U64, data_tb, (uintptr_t) tb_store);
 qemu_plugin_register_vcpu_tb_exec_cb(
-tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS, 0);
+tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS, tb_store);
 qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
 tb, QEMU_PLUGIN_INLINE_ADD_U64, count_tb_inline, 1);
 
 for (int idx = 0; idx < qemu_plugin_tb_n_insns(tb); ++idx) {
 struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
+void *insn_store = insn;
+void *mem_store = (char *)insn_store + 0xff;
+
+qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+insn, QEMU_PLUGIN_INLINE_STORE_U64, data_insn,
+(uintptr_t) insn_store);
 qemu_plugin_register_vcpu_insn_exec_cb(
-insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, 0);
+insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, insn_store);
 qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
 insn, QEMU_PLUGIN_INLINE_ADD_U64, count_insn_inline, 1);
+
+qemu_plugin_register_vcpu_mem_inline_per_vcpu(
+insn, QEMU_PLUGIN_MEM_RW,
+QEMU_PLUGIN_INLINE_STORE_U64,
+data_mem, (uintptr_t) mem_store);
 qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
  QEMU_PLUGIN_CB_NO_REGS,
- QEMU_PLUGIN_MEM_RW, 0);
+ QEMU_PLUGIN_MEM_RW, mem_store);
 qemu_plugin_register_vcpu_mem_inline_per_vcpu(
 insn, QEMU_PLUGIN_MEM_RW,
 QEMU_PLUGIN_INLINE_ADD_U64,
@@ -179,6 +207,11 @@ int qemu_plugin_install(qemu_plugin_id_t id, const 
qemu_info_t *info,
 counts, CPUCount, count_insn_inline);
 count_mem_inline = qemu_plugin_scoreboard_u64_in_struct(
 counts, CPUCount, count_mem_inline);
+data = qemu_plugin_scoreboard_new(sizeof(CPUData));
+data_insn = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_insn);
+data_tb = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_tb);
+data_mem = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_mem);
+
 qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
 qemu_plugin_register_atexit_cb(id, plugin_exit

[PATCH v4 9/9] plugins: remove op from qemu_plugin_inline_cb

2024-03-26 Thread Pierrick Bouvier
Signed-off-by: Pierrick Bouvier 
---
 include/qemu/plugin.h |  1 -
 plugins/plugin.h  |  4 +++-
 plugins/core.c| 13 +++--
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index a078229942f..7f7b915e495 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -82,7 +82,6 @@ struct qemu_plugin_regular_cb {
 
 struct qemu_plugin_inline_cb {
 qemu_plugin_u64 entry;
-enum qemu_plugin_op op;
 uint64_t imm;
 enum qemu_plugin_mem_rw rw;
 };
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 80d5daa9171..30e2299a54d 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -108,7 +108,9 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
  enum qemu_plugin_mem_rw rw,
  void *udata);
 
-void exec_inline_op(struct qemu_plugin_inline_cb *cb, int cpu_index);
+void exec_inline_op(enum plugin_dyn_cb_type type,
+struct qemu_plugin_inline_cb *cb,
+int cpu_index);
 
 int plugin_num_vcpus(void);
 
diff --git a/plugins/core.c b/plugins/core.c
index 7ea2ee208db..a9f19e197aa 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -338,7 +338,6 @@ void plugin_register_inline_op_on_entry(GArray **arr,
 
 struct qemu_plugin_inline_cb inline_cb = { .rw = rw,
.entry = entry,
-   .op = op,
.imm = imm };
 dyn_cb = plugin_get_dyn_cb(arr);
 dyn_cb->type = op_to_cb_type(op);
@@ -557,7 +556,9 @@ void qemu_plugin_flush_cb(void)
 plugin_cb__simple(QEMU_PLUGIN_EV_FLUSH);
 }
 
-void exec_inline_op(struct qemu_plugin_inline_cb *cb, int cpu_index)
+void exec_inline_op(enum plugin_dyn_cb_type type,
+struct qemu_plugin_inline_cb *cb,
+int cpu_index)
 {
 char *ptr = cb->entry.score->data->data;
 size_t elem_size = g_array_get_element_size(
@@ -565,11 +566,11 @@ void exec_inline_op(struct qemu_plugin_inline_cb *cb, int 
cpu_index)
 size_t offset = cb->entry.offset;
 uint64_t *val = (uint64_t *)(ptr + offset + cpu_index * elem_size);
 
-switch (cb->op) {
-case QEMU_PLUGIN_INLINE_ADD_U64:
+switch (type) {
+case PLUGIN_CB_INLINE_ADD_U64:
 *val += cb->imm;
 break;
-case QEMU_PLUGIN_INLINE_STORE_U64:
+case PLUGIN_CB_INLINE_STORE_U64:
 *val = cb->imm;
 break;
 default:
@@ -601,7 +602,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
 case PLUGIN_CB_INLINE_ADD_U64:
 case PLUGIN_CB_INLINE_STORE_U64:
 if (rw && cb->inline_insn.rw) {
-exec_inline_op(&cb->inline_insn, cpu->cpu_index);
+exec_inline_op(cb->type, &cb->inline_insn, cpu->cpu_index);
 }
 break;
 default:
-- 
2.39.2




[PATCH v4 1/9] plugins: prepare introduction of new inline ops

2024-03-26 Thread Pierrick Bouvier
Until now, only add_u64 was available, and all functions assumed this or
were named uniquely.

Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
 include/qemu/plugin.h  |  2 +-
 accel/tcg/plugin-gen.c |  6 +++---
 plugins/core.c | 14 --
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 201889cbeec..23271fbe36a 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -68,7 +68,7 @@ union qemu_plugin_cb_sig {
 enum plugin_dyn_cb_type {
 PLUGIN_CB_REGULAR,
 PLUGIN_CB_MEM_REGULAR,
-PLUGIN_CB_INLINE,
+PLUGIN_CB_INLINE_ADD_U64,
 };
 
 /*
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index c3548257798..41d4d83f547 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -120,7 +120,7 @@ static void gen_udata_cb(struct qemu_plugin_dyn_cb *cb)
 tcg_temp_free_i32(cpu_index);
 }
 
-static void gen_inline_cb(struct qemu_plugin_dyn_cb *cb)
+static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
 {
 GArray *arr = cb->inline_insn.entry.score->data;
 size_t offset = cb->inline_insn.entry.offset;
@@ -165,8 +165,8 @@ static void inject_cb(struct qemu_plugin_dyn_cb *cb)
 case PLUGIN_CB_REGULAR:
 gen_udata_cb(cb);
 break;
-case PLUGIN_CB_INLINE:
-gen_inline_cb(cb);
+case PLUGIN_CB_INLINE_ADD_U64:
+gen_inline_add_u64_cb(cb);
 break;
 default:
 g_assert_not_reached();
diff --git a/plugins/core.c b/plugins/core.c
index 0213513ec65..a8557b54ff7 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -316,6 +316,16 @@ static struct qemu_plugin_dyn_cb *plugin_get_dyn_cb(GArray 
**arr)
 return &g_array_index(cbs, struct qemu_plugin_dyn_cb, cbs->len - 1);
 }
 
+static enum plugin_dyn_cb_type op_to_cb_type(enum qemu_plugin_op op)
+{
+switch (op) {
+case QEMU_PLUGIN_INLINE_ADD_U64:
+return PLUGIN_CB_INLINE_ADD_U64;
+default:
+g_assert_not_reached();
+}
+}
+
 void plugin_register_inline_op_on_entry(GArray **arr,
 enum qemu_plugin_mem_rw rw,
 enum qemu_plugin_op op,
@@ -326,7 +336,7 @@ void plugin_register_inline_op_on_entry(GArray **arr,
 
 dyn_cb = plugin_get_dyn_cb(arr);
 dyn_cb->userp = NULL;
-dyn_cb->type = PLUGIN_CB_INLINE;
+dyn_cb->type = op_to_cb_type(op);
 dyn_cb->rw = rw;
 dyn_cb->inline_insn.entry = entry;
 dyn_cb->inline_insn.op = op;
@@ -551,7 +561,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
 cb->regular.f.vcpu_mem(cpu->cpu_index, make_plugin_meminfo(oi, rw),
vaddr, cb->userp);
 break;
-case PLUGIN_CB_INLINE:
+case PLUGIN_CB_INLINE_ADD_U64:
 exec_inline_op(cb, cpu->cpu_index);
 break;
 default:
-- 
2.39.2




[PATCH for 9.1 v2 0/2] meson: Fix MESONINTROSPECT parsing

2024-03-26 Thread Akihiko Odaki
The arguments in MESONINTROSPECT are quoted with shlex.quote() so it
must be parsed with shlex.split().

Signed-off-by: Akihiko Odaki 

To: Beraldo Leal 
Cc: qemu-devel@nongnu.org
Cc: Philippe Mathieu-Daudé 
Cc: Thomas Huth 
Cc: Daniel P. Berrangé 
Cc: Marc-André Lureau 
Cc: Paolo Bonzini 
Cc: Cleber Rosa 
Cc: John Snow 
Cc: Michael Tokarev 

Changes in v2:
- Added patch "buildsys: Bump known good meson version to v1.4.0".
- Link to v1: 
https://lore.kernel.org/r/20230812061540.5398-1-akihiko.od...@daynix.com

---
Akihiko Odaki (2):
  buildsys: Bump known good meson version to v1.4.0
  meson: Fix MESONINTROSPECT parsing

 python/scripts/vendor.py   |   4 ++--
 python/wheels/meson-1.2.3-py3-none-any.whl | Bin 964928 -> 0 bytes
 python/wheels/meson-1.4.0-py3-none-any.whl | Bin 0 -> 935471 bytes
 pythondeps.toml|   2 +-
 scripts/symlink-install-tree.py|   3 ++-
 5 files changed, 5 insertions(+), 4 deletions(-)
---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240326-meson-b5e2c7a2fb2f

Best regards,
-- 
Akihiko Odaki 




[PATCH v4 8/9] plugins: extract cpu_index generate

2024-03-26 Thread Pierrick Bouvier
Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
 accel/tcg/plugin-gen.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index d00a2b3fd67..eaa0160cab1 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -108,12 +108,17 @@ static void gen_disable_mem_helper(void)
offsetof(ArchCPU, env));
 }
 
+static TCGv_i32 gen_cpu_index(void)
+{
+TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
+tcg_gen_ld_i32(cpu_index, tcg_env,
+   -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+return cpu_index;
+}
+
 static void gen_udata_cb(struct qemu_plugin_regular_cb *cb)
 {
-TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
-
-tcg_gen_ld_i32(cpu_index, tcg_env,
-   -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+TCGv_i32 cpu_index = gen_cpu_index();
 tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
   tcgv_i32_temp(cpu_index),
   tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
@@ -128,9 +133,7 @@ static TCGv_ptr gen_plugin_u64_ptr(qemu_plugin_u64 entry)
 char *base_ptr = arr->data + entry.offset;
 size_t entry_size = g_array_get_element_size(arr);
 
-TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
-tcg_gen_ld_i32(cpu_index, tcg_env,
-   -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+TCGv_i32 cpu_index = gen_cpu_index();
 tcg_gen_muli_i32(cpu_index, cpu_index, entry_size);
 tcg_gen_ext_i32_ptr(ptr, cpu_index);
 tcg_temp_free_i32(cpu_index);
@@ -163,7 +166,6 @@ static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond 
cond)
 static void gen_udata_cond_cb(struct qemu_plugin_conditional_cb *cb)
 {
 TCGv_ptr ptr = gen_plugin_u64_ptr(cb->entry);
-TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
 TCGv_i64 val = tcg_temp_ebb_new_i64();
 TCGLabel *after_cb = gen_new_label();
 
@@ -172,15 +174,14 @@ static void gen_udata_cond_cb(struct 
qemu_plugin_conditional_cb *cb)
 
 tcg_gen_ld_i64(val, ptr, 0);
 tcg_gen_brcondi_i64(cond, val, cb->imm, after_cb);
-tcg_gen_ld_i32(cpu_index, tcg_env,
-   -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+TCGv_i32 cpu_index = gen_cpu_index();
 tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
   tcgv_i32_temp(cpu_index),
   tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
+tcg_temp_free_i32(cpu_index);
 gen_set_label(after_cb);
 
 tcg_temp_free_i64(val);
-tcg_temp_free_i32(cpu_index);
 tcg_temp_free_ptr(ptr);
 }
 
@@ -210,10 +211,7 @@ static void gen_inline_store_u64_cb(struct 
qemu_plugin_inline_cb *cb)
 static void gen_mem_cb(struct qemu_plugin_regular_cb *cb,
qemu_plugin_meminfo_t meminfo, TCGv_i64 addr)
 {
-TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
-
-tcg_gen_ld_i32(cpu_index, tcg_env,
-   -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+TCGv_i32 cpu_index = gen_cpu_index();
 tcg_gen_call4(cb->f.vcpu_mem, cb->info, NULL,
   tcgv_i32_temp(cpu_index),
   tcgv_i32_temp(tcg_constant_i32(meminfo)),
-- 
2.39.2




[PATCH v4 6/9] tests/plugin/inline: add test for conditional callback

2024-03-26 Thread Pierrick Bouvier
Count number of tb and insn executed using a conditional callback. We
ensure the callback has been called expected number of time (per vcpu).

Signed-off-by: Pierrick Bouvier 
---
 tests/plugin/inline.c | 89 +--
 1 file changed, 86 insertions(+), 3 deletions(-)

diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 103c3a22f6e..cd63827b7d8 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -20,8 +20,14 @@ typedef struct {
 uint64_t count_insn_inline;
 uint64_t count_mem;
 uint64_t count_mem_inline;
+uint64_t tb_cond_num_trigger;
+uint64_t tb_cond_track_count;
+uint64_t insn_cond_num_trigger;
+uint64_t insn_cond_track_count;
 } CPUCount;
 
+static const uint64_t cond_trigger_limit = 100;
+
 typedef struct {
 uint64_t data_insn;
 uint64_t data_tb;
@@ -35,6 +41,10 @@ static qemu_plugin_u64 count_insn;
 static qemu_plugin_u64 count_insn_inline;
 static qemu_plugin_u64 count_mem;
 static qemu_plugin_u64 count_mem_inline;
+static qemu_plugin_u64 tb_cond_num_trigger;
+static qemu_plugin_u64 tb_cond_track_count;
+static qemu_plugin_u64 insn_cond_num_trigger;
+static qemu_plugin_u64 insn_cond_track_count;
 static struct qemu_plugin_scoreboard *data;
 static qemu_plugin_u64 data_insn;
 static qemu_plugin_u64 data_tb;
@@ -56,12 +66,19 @@ static void stats_insn(void)
 const uint64_t per_vcpu = qemu_plugin_u64_sum(count_insn);
 const uint64_t inl_per_vcpu =
 qemu_plugin_u64_sum(count_insn_inline);
+const uint64_t cond_num_trigger =
+qemu_plugin_u64_sum(insn_cond_num_trigger);
+const uint64_t cond_track_left = 
qemu_plugin_u64_sum(insn_cond_track_count);
+const uint64_t conditional =
+cond_num_trigger * cond_trigger_limit + cond_track_left;
 printf("insn: %" PRIu64 "\n", expected);
 printf("insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
 printf("insn: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+printf("insn: %" PRIu64 " (cond cb)\n", conditional);
 g_assert(expected > 0);
 g_assert(per_vcpu == expected);
 g_assert(inl_per_vcpu == expected);
+g_assert(conditional == expected);
 }
 
 static void stats_tb(void)
@@ -70,12 +87,18 @@ static void stats_tb(void)
 const uint64_t per_vcpu = qemu_plugin_u64_sum(count_tb);
 const uint64_t inl_per_vcpu =
 qemu_plugin_u64_sum(count_tb_inline);
+const uint64_t cond_num_trigger = qemu_plugin_u64_sum(tb_cond_num_trigger);
+const uint64_t cond_track_left = qemu_plugin_u64_sum(tb_cond_track_count);
+const uint64_t conditional =
+cond_num_trigger * cond_trigger_limit + cond_track_left;
 printf("tb: %" PRIu64 "\n", expected);
 printf("tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
 printf("tb: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+printf("tb: %" PRIu64 " (conditional cb)\n", conditional);
 g_assert(expected > 0);
 g_assert(per_vcpu == expected);
 g_assert(inl_per_vcpu == expected);
+g_assert(conditional == expected);
 }
 
 static void stats_mem(void)
@@ -104,14 +127,35 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
 const uint64_t insn_inline = qemu_plugin_u64_get(count_insn_inline, i);
 const uint64_t mem = qemu_plugin_u64_get(count_mem, i);
 const uint64_t mem_inline = qemu_plugin_u64_get(count_mem_inline, i);
-printf("cpu %d: tb (%" PRIu64 ", %" PRIu64 ") | "
-   "insn (%" PRIu64 ", %" PRIu64 ") | "
+const uint64_t tb_cond_trigger =
+qemu_plugin_u64_get(tb_cond_num_trigger, i);
+const uint64_t tb_cond_left =
+qemu_plugin_u64_get(tb_cond_track_count, i);
+const uint64_t insn_cond_trigger =
+qemu_plugin_u64_get(insn_cond_num_trigger, i);
+const uint64_t insn_cond_left =
+qemu_plugin_u64_get(insn_cond_track_count, i);
+printf("cpu %d: tb (%" PRIu64 ", %" PRIu64
+   ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+   ") | "
+   "insn (%" PRIu64 ", %" PRIu64
+   ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+   ") | "
"mem (%" PRIu64 ", %" PRIu64 ")"
"\n",
-   i, tb, tb_inline, insn, insn_inline, mem, mem_inline);
+   i,
+   tb, tb_inline,
+   tb_cond_trigger, cond_trigger_limit, tb_cond_left,
+   insn, insn_inline,
+   insn_cond_trigger, cond_trigger_limit, insn_cond_left,
+   mem, mem_inline);
 g_assert(tb == tb_inline);
 g_assert(insn == insn_inline);
 g_assert(mem == mem_inline);
+g_assert(tb_cond_trigger == tb / cond_trigger_limit);
+g_assert(tb_cond_left == tb % cond_trigger_limit);
+g_assert(insn_cond_trigger == insn / cond_trigger_limit);
+g_assert(insn_cond_left == insn % cond_trigger_limit);
 }
 
 stats_tb();
@@ -132,6 +176,24 @@ static void vcpu_tb_exec(unsigned i

Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning

2024-03-26 Thread Pierrick Bouvier



On 3/26/24 13:54, Philippe Mathieu-Daudé wrote:


I will post a small PR later today, so until Peter has something
else planned, I can take it, since the patch LGTM now.



Thanks Philippe :)


Re: [PATCH for 9.1 v2 1/2] buildsys: Bump known good meson version to v1.4.0

2024-03-26 Thread Peter Maydell
On Tue, 26 Mar 2024 at 12:35, Akihiko Odaki  wrote:
>
> We need meson v1.4.0 to fix MESONINTROSPECT quoting on Windows:
> https://github.com/mesonbuild/meson/pull/12807
>
> Signed-off-by: Akihiko Odaki 
> ---
>  python/scripts/vendor.py   |   4 ++--
>  python/wheels/meson-1.2.3-py3-none-any.whl | Bin 964928 -> 0 bytes
>  python/wheels/meson-1.4.0-py3-none-any.whl | Bin 0 -> 935471 bytes
>  pythondeps.toml|   2 +-
>  4 files changed, 3 insertions(+), 3 deletions(-)

> --- a/pythondeps.toml
> +++ b/pythondeps.toml
> @@ -19,7 +19,7 @@
>
>  [meson]
>  # The install key should match the version in python/wheels/
> -meson = { accepted = ">=0.63.0", installed = "1.2.3", canary = "meson" }
> +meson = { accepted = ">=0.63.0", installed = "1.4.0", canary = "meson" }

If we need 1.4.0 why does this still say we accept anything 0.63.0
or better ?

If we use shlex.split(), does that go wrong for pre-1.4.0
meson only on Windows, or is it broken for all platforms?
(i.e. could we if we wanted to make the requirement
"1.4.0 for windows, 0.63.0 for others"?)

thanks
-- PMM



Re: [PULL 00/20] QAPI patches patches for 2024-03-26

2024-03-26 Thread Peter Maydell
On Tue, 26 Mar 2024 at 07:34, Markus Armbruster  wrote:
>
> This pull request does not touch code, only QAPI schema documentation
> comments and error-suppressing QAPI schema pragma
> documentation-exceptions.
>
> The following changes since commit 6a4180af9686830d88c387baab6d79563ce42a15:
>
>   Merge tag 'pull-request-2024-03-25' of https://gitlab.com/thuth/qemu into 
> staging (2024-03-25 14:19:42 +)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/armbru.git tags/pull-qapi-2024-03-26
>
> for you to fetch changes up to 1a533ce986f52c35f324f5f4fff22cdc2619a47c:
>
>   qapi: document parameters of query-cpu-model-* QAPI commands (2024-03-26 
> 06:36:08 +0100)
>
> 
> QAPI patches patches for 2024-03-26
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0
for any user-visible changes.

-- PMM



Re: [PATCH v3 2/4] block-backend: fix edge case in bdrv_next() where BDS associated to BB changes

2024-03-26 Thread Kevin Wolf
Am 22.03.2024 um 10:50 hat Fiona Ebner geschrieben:
> The old_bs variable in bdrv_next() is currently determined by looking
> at the old block backend. However, if the block graph changes before
> the next bdrv_next() call, it might be that the associated BDS is not
> the same that was referenced previously. In that case, the wrong BDS
> is unreferenced, leading to an assertion failure later:
> 
> > bdrv_unref: Assertion `bs->refcnt > 0' failed.

Your change makes sense, but in theory it shouldn't make a difference.
The real bug is in the caller, you can't allow graph modifications while
iterating the list of nodes. Even if it doesn't crash (like after your
patch), you don't have any guarantee that you will have seen every node
that exists that the end - and maybe not even that you don't get the
same node twice.

> In particular, this can happen in the context of bdrv_flush_all(),
> when polling for bdrv_co_flush() in the generated co-wrapper leads to
> a graph change (for example with a stream block job [0]).

The whole locking around this case is a bit tricky and would deserve
some cleanup.

The basic rule for bdrv_next() callers is that they need to hold the
graph reader lock as long as they are iterating the graph, otherwise
it's not safe. This implies that the ref/unref pairs in it should never
make a difference either - which is important, because at least
releasing the last reference is forbidden while holding the graph lock.
I intended to remove the ref/unref for bdrv_next(), but I didn't because
I realised that the callers need to be audited first that they really
obey the rules. You found one that would be problematic.

The thing that bdrv_flush_all() gets wrong is that it promises to follow
the graph lock rules with GRAPH_RDLOCK_GUARD_MAINLOOP(), but then calls
something that polls. The compiler can't catch this because bdrv_flush()
is a co_wrapper_mixed_bdrv_rdlock. The behaviour for these functions is:

- If called outside of coroutine context, they are GRAPH_UNLOCKED
- If called in coroutine context, they are GRAPH_RDLOCK

We should probably try harder to get rid of the mixed functions, because
a synchronous co_wrapper_bdrv_rdlock could actually be marked
GRAPH_UNLOCKED in the code and then the compiler could catch this case.

The fix for bdrv_flush_all() is probably to make it bdrv_co_flush_all()
with a coroutine wrapper so that the graph lock is held for the whole
function. Then calling bdrv_co_flush() while iterating the list is safe
and doesn't allow concurrent graph modifications.

Kevin




Re: [PATCH trivial for-9.0] smbios: add stub for smbios_get_table_legacy()

2024-03-26 Thread Philippe Mathieu-Daudé

On 26/3/24 13:26, Igor Mammedov wrote:

QEMU build fails with
   hw/i386/fw_cfg.c:74: undefined reference to `smbios_get_table_legacy'
when it's built with only 'microvm' enabled i.e. with config patch
+++ b/configs/devices/i386-softmmu/default.mak
@@ -26,7 +26,7 @@

# Boards:
#
-CONFIG_ISAPC=y
-CONFIG_I440FX=y
-CONFIG_Q35=y
+CONFIG_ISAPC=n
+CONFIG_I440FX=n
+CONFIG_Q35=n

it happens because I've fogotten/lost smbios_get_table_legacy() stub.

Fix it by adding missing stub as Philippe suggested.

Fixes: b42b0e4daaa5 "smbios: build legacy mode code only for 'pc' machine"
Reported-by: Michael Tokarev 
Singned-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Igor Mammedov 
---
Compile tested only.

While it's fixing bug for off-tree usecase with non-upstream config,
it's trivial enough to go into 9.0 if time frame allows.
Benefit of it going into 9.0 is that folks who play with minimal builds
won't have to carry the patch in their tree.


  hw/smbios/smbios_legacy_stub.c | 5 +
  1 file changed, 5 insertions(+)


Thanks, patch queued.



Re: [PATCH for 9.1 v2 1/2] buildsys: Bump known good meson version to v1.4.0

2024-03-26 Thread Akihiko Odaki

On 2024/03/26 21:40, Peter Maydell wrote:

On Tue, 26 Mar 2024 at 12:35, Akihiko Odaki  wrote:


We need meson v1.4.0 to fix MESONINTROSPECT quoting on Windows:
https://github.com/mesonbuild/meson/pull/12807

Signed-off-by: Akihiko Odaki 
---
  python/scripts/vendor.py   |   4 ++--
  python/wheels/meson-1.2.3-py3-none-any.whl | Bin 964928 -> 0 bytes
  python/wheels/meson-1.4.0-py3-none-any.whl | Bin 0 -> 935471 bytes
  pythondeps.toml|   2 +-
  4 files changed, 3 insertions(+), 3 deletions(-)



--- a/pythondeps.toml
+++ b/pythondeps.toml
@@ -19,7 +19,7 @@

  [meson]
  # The install key should match the version in python/wheels/
-meson = { accepted = ">=0.63.0", installed = "1.2.3", canary = "meson" }
+meson = { accepted = ">=0.63.0", installed = "1.4.0", canary = "meson" }


If we need 1.4.0 why does this still say we accept anything 0.63.0
or better ?

If we use shlex.split(), does that go wrong for pre-1.4.0
meson only on Windows, or is it broken for all platforms?


It is only needed for Windows.


(i.e. could we if we wanted to make the requirement
"1.4.0 for windows, 0.63.0 for others"?)


I just followed what commit 1a1e889f3576 ("buildsys: Bump known good 
meson version to v1.2.3") did, which don't bump the accepted version.


But certainly we can do better and conditionally ensure meson==1.4.0 on 
Windows in the configure script as commit edc210789500 ("python: use 
vendored tomli") does.


Regards,
Akihiko Odaki



thanks
-- PMM




[PATCH for-9.0] docs/about: Mark the iaspc machine type as deprecated

2024-03-26 Thread Igor Mammedov
ISAPC machine was introduced 25 years ago and it's a lot of time since
such machine was around with real ISA only PC hardware practically defunct.
Also it's slowly bit-rots (for example: I was able to boot RHEL6 on RHEL9 host
in only TCG mode, while in KVM mode it hung in the middle of boot)

Rather than spending time on fixing 'the oldest' no longer tested machine type,
deprecate it so we can clean up QEMU code from legacy fixups and hopefully
make it easier to follow.

Folks who have to use ancient guest that requires ISAPC can still
use older QEMU to play with it.

Signed-off-by: Igor Mammedov 
---
 docs/about/deprecated.rst | 7 +++
 hw/i386/pc_piix.c | 1 +
 2 files changed, 8 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b548519b5..5708296991 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -226,6 +226,13 @@ These old machine types are quite neglected nowadays and 
thus might have
 various pitfalls with regards to live migration. Use a newer machine type
 instead.
 
+``isapc`` (since 9.0)
+'
+
+These old machine type are quite neglected nowadays and thus might have
+various pitfalls with regards to live migration. Use a newer machine type
+instead.
+
 Nios II ``10m50-ghrd`` and ``nios2-generic-nommu`` machines (since 8.2)
 '''
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 18ba076609..96f72384dd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -921,6 +921,7 @@ static void isapc_machine_options(MachineClass *m)
 m->default_nic = "ne2k_isa";
 m->default_cpu_type = X86_CPU_TYPE_NAME("486");
 m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
+m->deprecation_reason = "old and unattended - use a newer version instead";
 }
 
 DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
-- 
2.43.0




Re: [PATCH v3 0/4] fix two edge cases related to stream block jobs

2024-03-26 Thread Kevin Wolf
Am 25.03.2024 um 21:11 hat Stefan Hajnoczi geschrieben:
> On Fri, Mar 22, 2024 at 10:50:05AM +0100, Fiona Ebner wrote:
> > Changes in v3:
> > * Also deal with edge case in bdrv_next_cleanup(). Haven't run
> >   into an actual issue there, but at least the caller in
> >   migration/block.c uses bdrv_nb_sectors() which, while not a
> >   coroutine wrapper itself (it's written manually), may call
> >   bdrv_refresh_total_sectors(), which is a generated coroutine
> >   wrapper, so AFAIU, the block graph can change during that call.
> >   And even without that, it's just better to be more consistent
> >   with bdrv_next().
> > 
> > Changes in v2:
> > * Ran into another issue while writing the IO test Stefan wanted
> >   to have (good call :)), so include a fix for that and add the
> >   test. I didn't notice during manual testing, because I hadn't
> >   used a scripted QMP 'quit', so there was no race.
> > 
> > Fiona Ebner (3):
> >   block-backend: fix edge case in bdrv_next() where BDS associated to BB
> > changes
> >   block-backend: fix edge case in bdrv_next_cleanup() where BDS
> > associated to BB changes
> >   iotests: add test for stream job with an unaligned prefetch read
> > 
> > Stefan Reiter (1):
> >   block/io: accept NULL qiov in bdrv_pad_request
> > 
> >  block/block-backend.c | 18 ++--
> >  block/io.c| 31 ---
> >  .../tests/stream-unaligned-prefetch   | 86 +++
> >  .../tests/stream-unaligned-prefetch.out   |  5 ++
> >  4 files changed, 117 insertions(+), 23 deletions(-)
> >  create mode 100755 tests/qemu-iotests/tests/stream-unaligned-prefetch
> >  create mode 100644 tests/qemu-iotests/tests/stream-unaligned-prefetch.out
> 
> Looks good to me. I will wait until Thursday before merging in case
> Hanna, Vladimir, or Kevin have comments. Thanks!

Let's not delay it to -rc2. If something turns out to be wrong with it,
we can still revert it, but I think getting fixes in earlier is better
during freeze.

Thanks, applied to the block branch.

Kevin


signature.asc
Description: PGP signature


Re: [RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()

2024-03-26 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> Each target use a common template for qmp_query_cpu_definitions().
>
> Extract it as generic_query_cpu_definitions(), keeping the
> target-specific implementations as the following SysemuCPUOps
> handlers:
>  - cpu_list_compare()
>  - add_definition()
>  - add_alias_definitions()
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS   |  2 +
>  include/hw/core/sysemu-cpu-ops.h  | 14 ++
>  include/qapi/commands-target-compat.h | 14 ++
>  system/cpu-qmp-cmds.c | 71 +++
>  system/meson.build|  1 +
>  5 files changed, 102 insertions(+)
>  create mode 100644 include/qapi/commands-target-compat.h
>  create mode 100644 system/cpu-qmp-cmds.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index af27490243..39d7c14d98 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -148,6 +148,7 @@ M: Richard Henderson 
>  R: Paolo Bonzini 
>  S: Maintained
>  F: system/cpus.c
> +F: system/cpu-qmp-cmds.c
>  F: system/cpu-qom-helpers.c
>  F: system/watchpoint.c
>  F: cpu-common.c
> @@ -1894,6 +1895,7 @@ F: qapi/machine-target.json
>  F: include/hw/boards.h
>  F: include/hw/core/cpu.h
>  F: include/hw/cpu/cluster.h
> +F: include/qapi/commands-target-compat.h
>  F: include/sysemu/numa.h
>  F: tests/unit/test-smp-parse.c
>  T: git https://gitlab.com/ehabkost/qemu.git machine-next
> diff --git a/include/hw/core/sysemu-cpu-ops.h 
> b/include/hw/core/sysemu-cpu-ops.h
> index 24d003fe04..2173226e97 100644
> --- a/include/hw/core/sysemu-cpu-ops.h
> +++ b/include/hw/core/sysemu-cpu-ops.h
> @@ -11,6 +11,7 @@
>  #define SYSEMU_CPU_OPS_H
>  
>  #include "hw/core/cpu.h"
> +#include "qapi/qapi-types-machine.h"
>  
>  /*
>   * struct SysemuCPUOps: System operations specific to a CPU class
> @@ -81,6 +82,19 @@ typedef struct SysemuCPUOps {
>   */
>  bool (*virtio_is_big_endian)(CPUState *cpu);
>  
> +/**
> + * @cpu_list_compare: Sort alphabetically by type name,
> + *respecting CPUClass::ordering.
> + */
> +gint (*cpu_list_compare)(gconstpointer cpu_class_a, gconstpointer 
> cpu_class_b);

Peeking ahead...  This is used for sorting the subtypes of the CPU type
returned by cpu_typename_by_arch_bit().  Implementing the callback is
optional, and when absent, we don't sort, i.e. we get hash table order.

Worth mentioning it's optional?

> +/**
> + * @add_definition: Add the @cpu_class definition to @cpu_list.
> + */
> +void (*add_definition)(gpointer cpu_class, gpointer cpu_list);

This one appears to default to cpu_common_add_definition().  Worth
mentioning?

I despise Glib's pointless typedefs for ordinary C types.

> +/**
> + * @add_alias_definitions: Add CPU alias definitions to @cpu_list.
> + */
> +void (*add_alias_definitions)(CpuDefinitionInfoList **cpu_list);
>  /**
>   * @legacy_vmsd: Legacy state for migration.
>   *   Do not use in new targets, use #DeviceClass::vmsd 
> instead.
> diff --git a/include/qapi/commands-target-compat.h 
> b/include/qapi/commands-target-compat.h
> new file mode 100644
> index 00..86d45d8fcc
> --- /dev/null
> +++ b/include/qapi/commands-target-compat.h

Why "compat"?

> @@ -0,0 +1,14 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QAPI_COMPAT_TARGET_H
> +#define QAPI_COMPAT_TARGET_H
> +
> +#include "qapi/qapi-types-machine.h"
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
> +
> +#endif
> diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
> new file mode 100644
> index 00..daeb131159
> --- /dev/null
> +++ b/system/cpu-qmp-cmds.c
> @@ -0,0 +1,71 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +#include "qapi/commands-target-compat.h"
> +#include "sysemu/arch_init.h"
> +#include "hw/core/cpu.h"
> +#include "hw/core/sysemu-cpu-ops.h"
> +
> +static void cpu_common_add_definition(gpointer data, gpointer user_data)
> +{
> +ObjectClass *oc = data;
> +CpuDefinitionInfoList **cpu_list = user_data;
> +CpuDefinitionInfo *info;
> +const char *typename;
> +
> +typename = object_class_get_name(oc);
> +info = g_malloc0(sizeof(*info));
> +info->name = cpu_model_from_type(typename);
> +info->q_typename = g_strdup(typename);
> +
> +QAPI_LIST_PREPEND(*cpu_list, info);
> +}
> +
> +static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
> + const char *cpu_typename)
> +{
> +ObjectClass *oc;
> +GSList *list;
> +const struct SysemuCPUOps *ops;
> +
> +oc = object_class_by_name(cpu_typename);
> +if (!oc) {
> +return;
> +}
> 

Re: [PATCH for-9.0] docs/about: Mark the iaspc machine type as deprecated

2024-03-26 Thread Ani Sinha



> On 26 Mar 2024, at 18:21, Igor Mammedov  wrote:
> 
> ISAPC machine was introduced 25 years ago and it's a lot of time since
> such machine was around with real ISA only PC hardware practically defunct.
> Also it's slowly bit-rots (for example: I was able to boot RHEL6 on RHEL9 host
> in only TCG mode, while in KVM mode it hung in the middle of boot)
> 
> Rather than spending time on fixing 'the oldest' no longer tested machine 
> type,
> deprecate it so we can clean up QEMU code from legacy fixups and hopefully
> make it easier to follow.
> 
> Folks who have to use ancient guest that requires ISAPC can still
> use older QEMU to play with it.
> 
> Signed-off-by: Igor Mammedov 

Reviewed-by: Ani Sinha 

> ---
> docs/about/deprecated.rst | 7 +++
> hw/i386/pc_piix.c | 1 +
> 2 files changed, 8 insertions(+)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 7b548519b5..5708296991 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -226,6 +226,13 @@ These old machine types are quite neglected nowadays and 
> thus might have
> various pitfalls with regards to live migration. Use a newer machine type
> instead.
> 
> +``isapc`` (since 9.0)
> +'
> +
> +These old machine type are quite neglected nowadays and thus might have
> +various pitfalls with regards to live migration. Use a newer machine type
> +instead.
> +
> Nios II ``10m50-ghrd`` and ``nios2-generic-nommu`` machines (since 8.2)
> '''
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 18ba076609..96f72384dd 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -921,6 +921,7 @@ static void isapc_machine_options(MachineClass *m)
> m->default_nic = "ne2k_isa";
> m->default_cpu_type = X86_CPU_TYPE_NAME("486");
> m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
> +m->deprecation_reason = "old and unattended - use a newer version 
> instead";
> }
> 
> DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
> -- 
> 2.43.0
> ___
> Devel mailing list -- de...@lists.libvirt.org
> To unsubscribe send an email to devel-le...@lists.libvirt.org




Re: [PATCH for-9.0] docs/about: Mark the iaspc machine type as deprecated

2024-03-26 Thread Thomas Huth



s/iaspc/isapc/ in the subject

On 26/03/2024 13.51, Igor Mammedov wrote:

ISAPC machine was introduced 25 years ago and it's a lot of time since
such machine was around with real ISA only PC hardware practically defunct.
Also it's slowly bit-rots (for example: I was able to boot RHEL6 on RHEL9 host
in only TCG mode, while in KVM mode it hung in the middle of boot)

Rather than spending time on fixing 'the oldest' no longer tested machine type,
deprecate it so we can clean up QEMU code from legacy fixups and hopefully
make it easier to follow.

Folks who have to use ancient guest that requires ISAPC can still
use older QEMU to play with it.

Signed-off-by: Igor Mammedov 
---
  docs/about/deprecated.rst | 7 +++
  hw/i386/pc_piix.c | 1 +
  2 files changed, 8 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b548519b5..5708296991 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -226,6 +226,13 @@ These old machine types are quite neglected nowadays and 
thus might have
  various pitfalls with regards to live migration. Use a newer machine type
  instead.
  
+``isapc`` (since 9.0)

+'
+
+These old machine type are quite neglected nowadays and thus might have


"This old machine type is ..." ?


+various pitfalls with regards to live migration. Use a newer machine type
+instead.


I doubt that isapc could ever be used reliably for live migration, since 
it's an unversioned machine type, so I think it would be better to not 
mention live migration here.


 Thomas




Re: [PATCH trivial for-9.0] smbios: add stub for smbios_get_table_legacy()

2024-03-26 Thread Philippe Mathieu-Daudé

On 26/3/24 13:44, Philippe Mathieu-Daudé wrote:

On 26/3/24 13:26, Igor Mammedov wrote:

QEMU build fails with
   hw/i386/fw_cfg.c:74: undefined reference to `smbios_get_table_legacy'
when it's built with only 'microvm' enabled i.e. with config patch
    +++ b/configs/devices/i386-softmmu/default.mak
    @@ -26,7 +26,7 @@

    # Boards:
    #
    -CONFIG_ISAPC=y
    -CONFIG_I440FX=y
    -CONFIG_Q35=y
    +CONFIG_ISAPC=n
    +CONFIG_I440FX=n
    +CONFIG_Q35=n

it happens because I've fogotten/lost smbios_get_table_legacy() stub.

Fix it by adding missing stub as Philippe suggested.

Fixes: b42b0e4daaa5 "smbios: build legacy mode code only for 'pc' 
machine"

Reported-by: Michael Tokarev 
Singned-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Igor Mammedov 
---
Compile tested only.

While it's fixing bug for off-tree usecase with non-upstream config,
it's trivial enough to go into 9.0 if time frame allows.
Benefit of it going into 9.0 is that folks who play with minimal builds
won't have to carry the patch in their tree.


  hw/smbios/smbios_legacy_stub.c | 5 +
  1 file changed, 5 insertions(+)


Thanks, patch queued.


Tested-by: Philippe Mathieu-Daudé 

BTW I tested using the following patch:

-- >8 --
commit 8be7b26b430d3ab192a2d22215ee512072bd88fb
Author: Philippe Mathieu-Daudé 
Date:   Tue Mar 26 13:52:17 2024 +0100

hw/i386: Add a config to only build the microvm machine

Add a config file to build a binary only containing the
microvm machine.

As suggested in commit d1d5e9eefd ("configure: allow the
selection of alternate config in the build"), it can be
built using:

  $ ../configure --without-default-features \
 --target-list=x86_64-softmmu \
 --with-devices-x86_64=microvm

Signed-off-by: Philippe Mathieu-Daudé 

diff --git a/configs/devices/x86_64-softmmu/microvm.mak 
b/configs/devices/x86_64-softmmu/microvm.mak

new file mode 100644
index 00..fe48b5b4a7
--- /dev/null
+++ b/configs/devices/x86_64-softmmu/microvm.mak
@@ -0,0 +1,20 @@
+# SPDX-FileCopyrightText: 2024 Linaro Ltd.
+#
+# Config that only supports the 64-bit microvm machine.
+# This avoids bringing in any of numerous legacy features from
+# the legacy machines or the 32bit platform.
+#
+
+CONFIG_MICROVM=y
+CONFIG_PCI_DEVICES=n
+CONFIG_SMBIOS=y
+CONFIG_SMBIOS_LEGACY=n
+CONFIG_VIRTIO_BALLOON=y
+CONFIG_VIRTIO_BLK=y
+CONFIG_VIRTIO_CRYPTO=y
+CONFIG_VIRTIO_GPU=y
+CONFIG_VIRTIO_INPUT=y
+CONFIG_VIRTIO_NET=y
+CONFIG_VIRTIO_RNG=y
+CONFIG_VIRTIO_SCSI=y
+CONFIG_VIRTIO_SERIAL=y
---

Before:

Undefined symbols for architecture arm64:
  "_smbios_get_table_legacy", referenced from:
  _fw_cfg_build_smbios in hw_i386_fw_cfg.c.o
ld: symbol(s) not found for architecture arm64

After:

$ ./qemu-system-x86_64 -S -monitor stdio -M microvm
QEMU 8.2.90 monitor - type 'help' for more information
(qemu) info qom-tree
/machine (microvm-machine)
...



[PATCH-for-9.1] hw/i386: Add a config to only build the microvm machine

2024-03-26 Thread Philippe Mathieu-Daudé
Add a config file to build a binary only containing the
microvm machine, inspired by a discussion on the list:
https://lore.kernel.org/qemu-devel/604bf457-23a7-4d06-b59f-a7b46945c...@tls.msk.ru/

As suggested in commit d1d5e9eefd ("configure: allow the
selection of alternate config in the build"), it can be
built using:

  $ ../configure --without-default-features \
 --target-list=x86_64-softmmu \
 --with-devices-x86_64=microvm

Inspired-by: Michael Tokarev 
Signed-off-by: Philippe Mathieu-Daudé 
---
 configs/devices/x86_64-softmmu/microvm.mak | 20 
 1 file changed, 20 insertions(+)
 create mode 100644 configs/devices/x86_64-softmmu/microvm.mak

diff --git a/configs/devices/x86_64-softmmu/microvm.mak 
b/configs/devices/x86_64-softmmu/microvm.mak
new file mode 100644
index 00..fe48b5b4a7
--- /dev/null
+++ b/configs/devices/x86_64-softmmu/microvm.mak
@@ -0,0 +1,20 @@
+# SPDX-FileCopyrightText: 2024 Linaro Ltd.
+#
+# Config that only supports the 64-bit microvm machine.
+# This avoids bringing in any of numerous legacy features from
+# the legacy machines or the 32bit platform.
+#
+
+CONFIG_MICROVM=y
+CONFIG_PCI_DEVICES=n
+CONFIG_SMBIOS=y
+CONFIG_SMBIOS_LEGACY=n
+CONFIG_VIRTIO_BALLOON=y
+CONFIG_VIRTIO_BLK=y
+CONFIG_VIRTIO_CRYPTO=y
+CONFIG_VIRTIO_GPU=y
+CONFIG_VIRTIO_INPUT=y
+CONFIG_VIRTIO_NET=y
+CONFIG_VIRTIO_RNG=y
+CONFIG_VIRTIO_SCSI=y
+CONFIG_VIRTIO_SERIAL=y
-- 
2.41.0




Re: [PATCH-for-9.0 v3 0/3] hw/clock: Propagate clock changes when STM32L4X5 MUX is updated

2024-03-26 Thread Philippe Mathieu-Daudé

On 25/3/24 16:28, Philippe Mathieu-Daudé wrote:


Per 
https://www.qemu.org/docs/master/devel/clocks.html#clock-multiplier-and-divider-settings:

   Note that clock_set_mul_div() does not automatically call
   clock_propagate(). If you make a runtime change to the
   multiplier or divider you must call clock_propagate() yourself.

Fix what we forgot to do that in recent commit ec7d83acbd
("hw/misc/stm32l4x5_rcc: Add an internal clock multiplexer object")

Arnaud Minier (1):
   hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock

Philippe Mathieu-Daudé (2):
   hw/clock: Let clock_set_mul_div() return a boolean value
   hw/misc/stm32l4x5_rcc: Inline clock_update() in clock_mux_update()


Series queued (thanks Alistair!).



Re: [PATCH v3 0/1] target/i386: Fix page walking from MMIO memory.

2024-03-26 Thread Philippe Mathieu-Daudé

On 7/3/24 16:53, Jonathan Cameron via wrote:

Previously: tcg/i386: Page tables in MMIO memory fixes (CXL)
Richard Henderson picked up patches 1 and 3 which were architecture independent
leaving just this x86 specific patch.

No change to the patch. Resending because it's hard to spot individual
unapplied patches in a larger series.


Thanks, patch queued!




[PATCH for-9.0 0/3] qtest/virtio-9p-test.c: fix slow tests

2024-03-26 Thread Daniel Henrique Barboza
Hi,

Thomas reported in [1] a problem that happened with the RISC-V machine
where some tests from virtio-9p-test.c were failing with '-m slow', i.e.
enabling slow tests.

In the end it wasn't a RISC-V specific problem. It just so happens that
the recently added riscv machine nodes runs the tests from
virtio-9p-test two times for each qos-test run: one with the
virtio-9p-device device and another with the virtio-9p-pci. The temp dir
for these tests is being created at the start of qos-test and removed
only at the end of qos-test, and the tests are leaving dirs and files
behind. virtio-9-device tests run first, creates stuff in the temp dir,
then when virtio-9p-pci tests runs again it'll fail because the previous
run left created dirs and files in the same temp dir. Here's a run that
exemplifies the problem:

$ MALLOC_PERTURB_=21 V=2 QTEST_QEMU_BINARY=./qemu-system-riscv64 
./tests/qtest/qos-test -m slow
(...)
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-621710.sock 
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-621710.qmp,id=char0 -mon 
chardev=char0,mode=control -display none -audio none -M 
virt,aclint=on,aia=aplic-imsic -fsdev 
local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
 -device virtio-9p-device,fsdev=fsdev0,mount_tag=qtest -accel qtest
( goes ok ...)
# starting QEMU: exec ./qemu-system-riscv64 -qtest unix:/tmp/qtest-621710.sock 
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-621710.qmp,id=char0 -mon 
chardev=char0,mode=control -display none -audio none -M 
virt,aclint=on,aia=aplic-imsic -fsdev 
local,id=fsdev0,path='/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2',security_model=mapped-xattr
 -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
ok 168 
/riscv64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
Received response 7 (RLERROR) instead of 73 (RMKDIR)
Rlerror has errno 17 (File exists)
**
ERROR:../tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv: assertion 
failed (hdr.id == id): (7 == 73)

As we can see we're running both 'virtio-9p-device' tests and 'virtio-9p-pci'
tests using the same '/home/danielhb/work/qemu/build/qtest-9p-local-7E16K2'
temp dir. 

The quick fix I came up with was to make each test clean themselves up
after each run. The tests were also consolidated, i.e. fewer tests with the
same coverage, because the 'unlikat' tests were doing the same thing the
'create' tests were doing but removing stuff after. Might as well keep just
the 'unlikat' tests.

I also went ahead and reverted 558f5c42efd ("tests/9pfs: Mark "local"
tests as "slow"") after realizing that the problem I was fixing is also
the same problem that this patch was trying to working around with the
skip [2]. I validated this change in this Gitlab pipeline:

https://gitlab.com/danielhb/qemu/-/pipelines/1227953967

[1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html
[2] https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html

Daniel Henrique Barboza (3):
  qtest/virtio-9p-test.c: consolidate create dir, file and symlink tests
  qtest/virtio-9p-test.c: consolidate hardlink tests
  qtest/virtio-9p-test.c: remove g_test_slow() gate

 tests/qtest/virtio-9p-test.c | 155 +++
 1 file changed, 48 insertions(+), 107 deletions(-)

-- 
2.44.0




[PATCH for-9.0 3/3] qtest/virtio-9p-test.c: remove g_test_slow() gate

2024-03-26 Thread Daniel Henrique Barboza
Commit 558f5c42ef gated the local tests with g_test_slow() to skip them
in 'make check'. The reported issue back then was this following CI
problem:

https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html

This problem ended up being fixed when the recently added risc-v machine
nodes faced the same issue [1]. We're now able to run these tests with
'make check' in the CI.

This reverts commit 558f5c42efded3e0d0b20a90bce2a9a14580d824.

[1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html

Signed-off-by: Daniel Henrique Barboza 
---
 tests/qtest/virtio-9p-test.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 9938516fe7..ff69eb334a 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -687,15 +687,6 @@ static void register_virtio_9p_test(void)
 
 
 /* 9pfs test cases using the 'local' filesystem driver */
-
-/*
- * XXX: Until we are sure that these tests can run everywhere,
- * keep them as "slow" so that they aren't run with "make check".
- */
-if (!g_test_slow()) {
-return;
-}
-
 opts.before = assign_9p_local_driver;
 qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
 qos_add_test("local/create_unlinkat_dir", "virtio-9p",
-- 
2.44.0




[PATCH for-9.0 1/3] qtest/virtio-9p-test.c: consolidate create dir, file and symlink tests

2024-03-26 Thread Daniel Henrique Barboza
The local 9p driver in virtio-9p-test.c its temporary dir right at the
start of qos-test (via virtio_9p_create_local_test_dir()) and only
deletes it after qos-test is finished (via
virtio_9p_remove_local_test_dir()).

This means that any qos-test machine that ends up running virtio-9p-test local
tests more than once will end up re-using the same temp dir. This is
what's happening in [1] after we introduced the riscv machine nodes: if
we enable slow tests with the '-m slow' flag using qemu-system-riscv64,
this is what happens:

- a temp dir is created, e.g. qtest-9p-local-WZLDL2;

- virtio-9p-device tests will run virtio-9p-test successfully;

- virtio-9p-pci tests will run virtio-9p-test, and fail right at the
  first slow test at fs_create_dir() because the "01" file was already
  created by fs_create_dir() test when running with the virtio-9p-device.

We can fix it by making every test clean up their changes in the
filesystem after they're done. But we don't need every test either:
what fs_create_file() does is already exercised in fs_unlinkat_dir(),
i.e. a dir is created, verified to be created, and then removed. Fixing
fs_create_file() would turn it into fs_unlikat_dir(), so we don't need
both. The same theme follows every test in virtio-9p-test.c, where the
'unlikat' variant does the same thing the 'create' does but with some
cleaning in the end.

Consolide some tests as follows:

- fs_create_dir() is removed. fs_unlinkat_dir() is renamed to
  fs_create_unlinkat_dir();

- fs_create_file() is removed. fs_unlinkat_file() is renamed to
  fs_create_unlinkat_file(). The "04" dir it uses is now being removed;

- fs_symlink_file() is removed. fs_unlinkat_symlink() is renamed to
  fs_create_unlinkat_symlink(). Both "real_file" and the "06" dir it
  creates is now being removed.

We're still missing the 'hardlink' tests. We'll do it in the next patch
since it's less trivial to consolidate than these.

[1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html

Reported-by: Thomas Huth 
Signed-off-by: Daniel Henrique Barboza 
---
 tests/qtest/virtio-9p-test.c | 97 +++-
 1 file changed, 29 insertions(+), 68 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 65e69491e5..cdbe3e78ea 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -506,26 +506,8 @@ static void fs_readdir_split_512(void *obj, void *data,
 
 /* tests using the 9pfs 'local' fs driver */
 
-static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc)
-{
-QVirtio9P *v9p = obj;
-v9fs_set_allocator(t_alloc);
-struct stat st;
-g_autofree char *root_path = virtio_9p_test_path("");
-g_autofree char *new_dir = virtio_9p_test_path("01");
-
-g_assert(root_path != NULL);
-
-tattach({ .client = v9p });
-tmkdir({ .client = v9p, .atPath = "/", .name = "01" });
-
-/* check if created directory really exists now ... */
-g_assert(stat(new_dir, &st) == 0);
-/* ... and is actually a directory */
-g_assert((st.st_mode & S_IFMT) == S_IFDIR);
-}
-
-static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc)
+static void fs_create_unlinkat_dir(void *obj, void *data,
+   QGuestAllocator *t_alloc)
 {
 QVirtio9P *v9p = obj;
 v9fs_set_allocator(t_alloc);
@@ -551,28 +533,13 @@ static void fs_unlinkat_dir(void *obj, void *data, 
QGuestAllocator *t_alloc)
 g_assert(stat(new_dir, &st) != 0);
 }
 
-static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc)
-{
-QVirtio9P *v9p = obj;
-v9fs_set_allocator(t_alloc);
-struct stat st;
-g_autofree char *new_file = virtio_9p_test_path("03/1st_file");
-
-tattach({ .client = v9p });
-tmkdir({ .client = v9p, .atPath = "/", .name = "03" });
-tlcreate({ .client = v9p, .atPath = "03", .name = "1st_file" });
-
-/* check if created file exists now ... */
-g_assert(stat(new_file, &st) == 0);
-/* ... and is a regular file */
-g_assert((st.st_mode & S_IFMT) == S_IFREG);
-}
-
-static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc)
+static void fs_create_unlinkat_file(void *obj, void *data,
+QGuestAllocator *t_alloc)
 {
 QVirtio9P *v9p = obj;
 v9fs_set_allocator(t_alloc);
 struct stat st;
+g_autofree char *new_dir = virtio_9p_test_path("04");
 g_autofree char *new_file = virtio_9p_test_path("04/doa_file");
 
 tattach({ .client = v9p });
@@ -587,37 +554,22 @@ static void fs_unlinkat_file(void *obj, void *data, 
QGuestAllocator *t_alloc)
 tunlinkat({ .client = v9p, .atPath = "04", .name = "doa_file" });
 /* file should be gone now */
 g_assert(stat(new_file, &st) != 0);
-}
-
-static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
-{
-QVirtio9P *v9p = obj;
-v9fs_set_allocator(t_alloc);
-struct stat st;
-g_autofree char *real_file =

[PATCH for-9.0 2/3] qtest/virtio-9p-test.c: consolidate hardlink tests

2024-03-26 Thread Daniel Henrique Barboza
We need all virtio-9p-test.c tests to clean themselves up after being
executed to avoid an issue where multiple test runs of this same test
will find remains of the previous run. See [1] to see the side effects
of that.

Previous patch dealt with most tests in this file. We're now
consolidating the 'hardlink' tests:

- copy the asserts that checks if the created link is a hard link from
  fs_hardlink_file() to fs_unlinkat_hardlink(). This will make
  fs_unlinkat_hardlink() do exactly what fs_hardlink_file() is doing;

- remove fs_hardlink_file();

- fs_unlinkat_hardlink() is renamed to fs_create_unlinkat_hardlink().
  We'll also remove 'real_file' and the '08' dir it creates.

With this last change all local 9p tests of the file are not leaving
files or dirs behind in the temporary dir, allowing multiple runs in the
same session to succeed.

[1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html

Reported-by: Thomas Huth 
Signed-off-by: Daniel Henrique Barboza 
---
 tests/qtest/virtio-9p-test.c | 55 +++-
 1 file changed, 22 insertions(+), 33 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index cdbe3e78ea..9938516fe7 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -600,39 +600,13 @@ static void fs_create_unlinkat_symlink(void *obj, void 
*data,
 g_assert(stat(new_dir, &st) != 0);
 }
 
-static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
+static void fs_create_unlinkat_hardlink(void *obj, void *data,
+QGuestAllocator *t_alloc)
 {
 QVirtio9P *v9p = obj;
 v9fs_set_allocator(t_alloc);
-struct stat st_real, st_link;
-g_autofree char *real_file = virtio_9p_test_path("07/real_file");
-g_autofree char *hardlink_file = virtio_9p_test_path("07/hardlink_file");
-
-tattach({ .client = v9p });
-tmkdir({ .client = v9p, .atPath = "/", .name = "07" });
-tlcreate({ .client = v9p, .atPath = "07", .name = "real_file" });
-g_assert(stat(real_file, &st_real) == 0);
-g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
-
-tlink({
-.client = v9p, .atPath = "07", .name = "hardlink_file",
-.toPath = "07/real_file"
-});
-
-/* check if link exists now ... */
-g_assert(stat(hardlink_file, &st_link) == 0);
-/* ... and it's a hard link, right? */
-g_assert((st_link.st_mode & S_IFMT) == S_IFREG);
-g_assert(st_link.st_dev == st_real.st_dev);
-g_assert(st_link.st_ino == st_real.st_ino);
-}
-
-static void fs_unlinkat_hardlink(void *obj, void *data,
- QGuestAllocator *t_alloc)
-{
-QVirtio9P *v9p = obj;
-v9fs_set_allocator(t_alloc);
-struct stat st_real, st_link;
+struct stat st_real, st_link, st;
+g_autofree char *new_dir = virtio_9p_test_path("08");
 g_autofree char *real_file = virtio_9p_test_path("08/real_file");
 g_autofree char *hardlink_file = virtio_9p_test_path("08/hardlink_file");
 
@@ -646,13 +620,29 @@ static void fs_unlinkat_hardlink(void *obj, void *data,
 .client = v9p, .atPath = "08", .name = "hardlink_file",
 .toPath = "08/real_file"
 });
+
+/* check if link exists now ... */
 g_assert(stat(hardlink_file, &st_link) == 0);
+/* ... and it's a hard link, right? */
+g_assert((st_link.st_mode & S_IFMT) == S_IFREG);
+g_assert(st_link.st_dev == st_real.st_dev);
+g_assert(st_link.st_ino == st_real.st_ino);
 
 tunlinkat({ .client = v9p, .atPath = "08", .name = "hardlink_file" });
 /* symlink should be gone now */
 g_assert(stat(hardlink_file, &st_link) != 0);
 /* and old file should still exist */
 g_assert(stat(real_file, &st_real) == 0);
+
+/* cleanup: remove old file and dir */
+tunlinkat({ .client = v9p, .atPath = "08", .name = "real_file" });
+g_assert(stat(real_file, &st_real) != 0);
+
+tunlinkat({
+.client = v9p, .atPath = "/", .name = "08",
+.flags = P9_DOTL_AT_REMOVEDIR
+});
+g_assert(stat(new_dir, &st) != 0);
 }
 
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
@@ -714,9 +704,8 @@ static void register_virtio_9p_test(void)
  fs_create_unlinkat_file, &opts);
 qos_add_test("local/create_unlinkat_symlink", "virtio-9p",
  fs_create_unlinkat_symlink, &opts);
-qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts);
-qos_add_test("local/unlinkat_hardlink", "virtio-9p", fs_unlinkat_hardlink,
- &opts);
+qos_add_test("local/create_unlinkat_hardlink", "virtio-9p",
+ fs_create_unlinkat_hardlink, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.44.0




Re: [RFC PATCH-for-9.1 14/21] system: Introduce QMP generic_query_cpu_definitions()

2024-03-26 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> Each target use a common template for qmp_query_cpu_definitions().
>
> Extract it as generic_query_cpu_definitions(), keeping the
> target-specific implementations as the following SysemuCPUOps
> handlers:
>  - cpu_list_compare()
>  - add_definition()
>  - add_alias_definitions()
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS   |  2 +
>  include/hw/core/sysemu-cpu-ops.h  | 14 ++
>  include/qapi/commands-target-compat.h | 14 ++
>  system/cpu-qmp-cmds.c | 71 +++
>  system/meson.build|  1 +
>  5 files changed, 102 insertions(+)
>  create mode 100644 include/qapi/commands-target-compat.h
>  create mode 100644 system/cpu-qmp-cmds.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index af27490243..39d7c14d98 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -148,6 +148,7 @@ M: Richard Henderson 
>  R: Paolo Bonzini 
>  S: Maintained
>  F: system/cpus.c
> +F: system/cpu-qmp-cmds.c
>  F: system/cpu-qom-helpers.c
>  F: system/watchpoint.c
>  F: cpu-common.c
> @@ -1894,6 +1895,7 @@ F: qapi/machine-target.json
>  F: include/hw/boards.h
>  F: include/hw/core/cpu.h
>  F: include/hw/cpu/cluster.h
> +F: include/qapi/commands-target-compat.h
>  F: include/sysemu/numa.h
>  F: tests/unit/test-smp-parse.c
>  T: git https://gitlab.com/ehabkost/qemu.git machine-next
> diff --git a/include/hw/core/sysemu-cpu-ops.h 
> b/include/hw/core/sysemu-cpu-ops.h
> index 24d003fe04..2173226e97 100644
> --- a/include/hw/core/sysemu-cpu-ops.h
> +++ b/include/hw/core/sysemu-cpu-ops.h
> @@ -11,6 +11,7 @@
>  #define SYSEMU_CPU_OPS_H
>  
>  #include "hw/core/cpu.h"
> +#include "qapi/qapi-types-machine.h"
>  
>  /*
>   * struct SysemuCPUOps: System operations specific to a CPU class
> @@ -81,6 +82,19 @@ typedef struct SysemuCPUOps {
>   */
>  bool (*virtio_is_big_endian)(CPUState *cpu);
>  
> +/**
> + * @cpu_list_compare: Sort alphabetically by type name,
> + *respecting CPUClass::ordering.
> + */
> +gint (*cpu_list_compare)(gconstpointer cpu_class_a, gconstpointer 
> cpu_class_b);
> +/**
> + * @add_definition: Add the @cpu_class definition to @cpu_list.
> + */
> +void (*add_definition)(gpointer cpu_class, gpointer cpu_list);
> +/**
> + * @add_alias_definitions: Add CPU alias definitions to @cpu_list.
> + */
> +void (*add_alias_definitions)(CpuDefinitionInfoList **cpu_list);
>  /**
>   * @legacy_vmsd: Legacy state for migration.
>   *   Do not use in new targets, use #DeviceClass::vmsd 
> instead.
> diff --git a/include/qapi/commands-target-compat.h 
> b/include/qapi/commands-target-compat.h
> new file mode 100644
> index 00..86d45d8fcc
> --- /dev/null
> +++ b/include/qapi/commands-target-compat.h
> @@ -0,0 +1,14 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QAPI_COMPAT_TARGET_H
> +#define QAPI_COMPAT_TARGET_H
> +
> +#include "qapi/qapi-types-machine.h"
> +
> +CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
> +
> +#endif
> diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
> new file mode 100644
> index 00..daeb131159
> --- /dev/null
> +++ b/system/cpu-qmp-cmds.c
> @@ -0,0 +1,71 @@
> +/*
> + * QAPI helpers for target specific QMP commands
> + *
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +#include "qapi/commands-target-compat.h"
> +#include "sysemu/arch_init.h"
> +#include "hw/core/cpu.h"
> +#include "hw/core/sysemu-cpu-ops.h"
> +
> +static void cpu_common_add_definition(gpointer data, gpointer user_data)
> +{
> +ObjectClass *oc = data;
> +CpuDefinitionInfoList **cpu_list = user_data;
> +CpuDefinitionInfo *info;
> +const char *typename;
> +
> +typename = object_class_get_name(oc);
> +info = g_malloc0(sizeof(*info));
> +info->name = cpu_model_from_type(typename);
> +info->q_typename = g_strdup(typename);
> +
> +QAPI_LIST_PREPEND(*cpu_list, info);
> +}
> +
> +static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
> + const char *cpu_typename)
> +{
> +ObjectClass *oc;
> +GSList *list;
> +const struct SysemuCPUOps *ops;
> +
> +oc = object_class_by_name(cpu_typename);
> +if (!oc) {
> +return;
> +}
> +ops = CPU_CLASS(oc)->sysemu_ops;
> +
> +list = object_class_get_list(cpu_typename, false);
> +if (ops->cpu_list_compare) {
> +list = g_slist_sort(list, ops->cpu_list_compare);
> +}
> +g_slist_foreach(list, ops->add_definition ? : cpu_common_add_definition,
> +cpu_list);
> +g_slist_free(list);
> +
> +if (ops->add_alias_definitions) {
> +op

Re: [PATCH for-9.0] docs/about: Mark the iaspc machine type as deprecated

2024-03-26 Thread Philippe Mathieu-Daudé

Hi Igor,

On 26/3/24 14:08, Thomas Huth wrote:


s/iaspc/isapc/ in the subject

On 26/03/2024 13.51, Igor Mammedov wrote:

ISAPC machine was introduced 25 years ago and it's a lot of time since
such machine was around with real ISA only PC hardware practically 
defunct.
Also it's slowly bit-rots (for example: I was able to boot RHEL6 on 
RHEL9 host

in only TCG mode, while in KVM mode it hung in the middle of boot)


I'm quite opposed to this patch. QEMU models various very-old /
defunct hardware. I'm pretty sure Bernhard and myself are OK to
keep maintaining it, besides we are working in separating it from
the i440fx+piix machine. Also, this machine is particularly
interesting for my single-binary experiments.

Where I agree is we should stop reporting "KVM on ISA/PC machine"
as supported.

Regards,

Phil.

Rather than spending time on fixing 'the oldest' no longer tested 
machine type,
deprecate it so we can clean up QEMU code from legacy fixups and 
hopefully

make it easier to follow.

Folks who have to use ancient guest that requires ISAPC can still
use older QEMU to play with it.

Signed-off-by: Igor Mammedov 
---
  docs/about/deprecated.rst | 7 +++
  hw/i386/pc_piix.c | 1 +
  2 files changed, 8 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b548519b5..5708296991 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -226,6 +226,13 @@ These old machine types are quite neglected 
nowadays and thus might have
  various pitfalls with regards to live migration. Use a newer machine 
type

  instead.
+``isapc`` (since 9.0)
+'
+
+These old machine type are quite neglected nowadays and thus might have


"This old machine type is ..." ?

+various pitfalls with regards to live migration. Use a newer machine 
type

+instead.


I doubt that isapc could ever be used reliably for live migration, since 
it's an unversioned machine type, so I think it would be better to not 
mention live migration here.


  Thomas







Re: [PATCH for-9.0] docs/about: Mark the iaspc machine type as deprecated

2024-03-26 Thread Mark Cave-Ayland

On 26/03/2024 12:51, Igor Mammedov wrote:


ISAPC machine was introduced 25 years ago and it's a lot of time since
such machine was around with real ISA only PC hardware practically defunct.
Also it's slowly bit-rots (for example: I was able to boot RHEL6 on RHEL9 host
in only TCG mode, while in KVM mode it hung in the middle of boot)

Rather than spending time on fixing 'the oldest' no longer tested machine type,
deprecate it so we can clean up QEMU code from legacy fixups and hopefully
make it easier to follow.

Folks who have to use ancient guest that requires ISAPC can still
use older QEMU to play with it.


Heh I've actually been using isapc over the past couple of weeks to fire up some old 
programs in a Windows 3 VM :)


I'd really hate to see isapc disappear as there are a number of people from the retro 
crowd (such as myself) who fire up QEMU/KVM on various historical images, and whilst 
there are alternatives, there isn't really anything that touches QEMU performance-wise.


This leads into the question as to how QEMU should handle less recent machines: I 
appreciate that from an enterprise perspective there is little interest, but there 
are plenty of hobbyists and historians who are.


From my personal experience with SPARC/macppc machines I accept that they are not 
first-class citizens, and so my approach here is that I don't mind if patches break 
migration or command-line compatibility as long as I can still fire up the VM. 
Regressions do occur, but fortunately they don't tend to occur that often.


I can see how there is a lot of legacy cruft around handling legacy command line 
options that could be improved by removing isapc, and I think that a lot of this is 
around preserving historical behaviour.


How about splitting the isapc machine out of the generic PC init so that it can be 
used going forward with less command-line/migration compatibility guarantees, but 
also won't prevent subsequent tidy-ups/changes to the main PC machines going forward?



Signed-off-by: Igor Mammedov 
---
  docs/about/deprecated.rst | 7 +++
  hw/i386/pc_piix.c | 1 +
  2 files changed, 8 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b548519b5..5708296991 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -226,6 +226,13 @@ These old machine types are quite neglected nowadays and 
thus might have
  various pitfalls with regards to live migration. Use a newer machine type
  instead.
  
+``isapc`` (since 9.0)

+'
+
+These old machine type are quite neglected nowadays and thus might have
+various pitfalls with regards to live migration. Use a newer machine type
+instead.
+
  Nios II ``10m50-ghrd`` and ``nios2-generic-nommu`` machines (since 8.2)
  '''
  
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c

index 18ba076609..96f72384dd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -921,6 +921,7 @@ static void isapc_machine_options(MachineClass *m)
  m->default_nic = "ne2k_isa";
  m->default_cpu_type = X86_CPU_TYPE_NAME("486");
  m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
+m->deprecation_reason = "old and unattended - use a newer version instead";
  }
  
  DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,



ATB,

Mark.




Re: [RFC PATCH-for-9.1 21/21] qapi: Make @query-cpu-definitions target-agnostic

2024-03-26 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> All targets use the generic_query_cpu_definitions() method,
> which is not target-specific. Make the command target agnostic
> by moving it to machine.json. Rename generic_query_cpu_definitions
> as qmp_query_cpu_definitions.
>
> This is an introspection change for the target that were not
> implementing qmp_query_cpu_definitions(): now query-cpu-definitions
> returns an their CPUs list.

Do you mean "returns their CPUs list"?

>
> Example with SH4 before:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "error": {"class": "CommandNotFound", "desc": "The command 
> query-cpu-definitions has not been found"} }
>
> and after:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "return": [
>   {"name": "sh7751r", "typename": "sh7751r-superh-cpu", "static": 
> false, "deprecated": false},
>   {"name": "sh7750r", "typename": "sh7750r-superh-cpu", "static": 
> false, "deprecated": false},
>   {"name": "sh7785", "typename": "sh7785-superh-cpu", "static": 
> false, "deprecated": false}
>   ]
>   }

Does the result make sense?  What do the callbacks add for the targets
that define them, and why do the other targets don't need them?

Conversion steps:

0. Create a generic version of the command, with optional callbacks
   [PATCH 14]

1. Turn the target-specific versions of the command into thin wrappers
   around the generic version one by one, supplying callbacks to
   preserve behavior [PATCH 15-20]

2. Peel off the wrappers [this patch]

3. Enable the command for all the other targets [also this patch]

Correct?

Split this patch?  Question, not a demand.

I think the commit messages of the step 1 patches should explain the
target-specific behavior, i.e. the difference between their callbacks
and the default behavior.

Obvious for cpu_list_compare(): sorted vs. unsorted.

Not obvious for add_definition().

> However this allows heterogeneous emulation to return a correct list.

Cryptic.  It made me go back to PATCH 14, where I discovered the loop
over the arch bits.

> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Well, not all target got converted, I left the s390x one for later :)

Haha!




  1   2   3   >