Re: [Qemu-devel] [PATCH v6 1/4] vfio: Mediated device Core driver
> From: Kirti Wankhede [mailto:kwankh...@nvidia.com] > Sent: Thursday, August 04, 2016 3:04 AM > > > 2. Physical device driver interface > This interface provides vendor driver the set APIs to manage physical > device related work in their own driver. APIs are : > - supported_config: provide supported configuration list by the vendor > driver > - create: to allocate basic resources in vendor driver for a mediated > device. > - destroy: to free resources in vendor driver when mediated device is > destroyed. > - reset: to free and reallocate resources in vendor driver during reboot Currently I saw 'reset' callback only invoked from VFIO ioctl path. Do you think whether it makes sense to expose a sysfs 'reset' node too, similar to what people see under a PCI device node? > - start: to initiate mediated device initialization process from vendor >driver > - shutdown: to teardown mediated device resources during teardown. I think 'shutdown' should be 'stop' based on actual code. Thanks Kevin
[Qemu-devel] [PATCH v2] net: check fragment length during fragmentation
From: Prasad J Pandit Network transport abstraction layer supports packet fragmentation. While fragmenting a packet, it checks for more fragments from packet length and current fragment length. It is susceptible to an infinite loop, if the current fragment length is zero. Add check to avoid it. Reported-by: Li Qiang Signed-off-by: Prasad J Pandit --- hw/net/net_tx_pkt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Updated as per -> https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg00751.html diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c index efd43b4..53dfaa2 100644 --- a/hw/net/net_tx_pkt.c +++ b/hw/net/net_tx_pkt.c @@ -590,7 +590,7 @@ static bool net_tx_pkt_do_sw_fragmentation(struct NetTxPkt *pkt, fragment_offset += fragment_len; -} while (more_frags); +} while (fragment_len && more_frags); return true; } -- 2.5.5
Re: [Qemu-devel] [PATCH v6 4/4] docs: Add Documentation for Mediated devices
> From: Kirti Wankhede [mailto:kwankh...@nvidia.com] > Sent: Thursday, August 04, 2016 3:04 AM > > + > +* mdev_supported_types: (read only) > +List the current supported mediated device types and its details. > + > +* mdev_create: (write only) > + Create a mediated device on target physical device. > + Input syntax: > + where, > + UUID: mediated device's UUID > + idx: mediated device index inside a VM Is above description too specific to VM usage? mediated device can be used by other user components too, e.g. an user space driver. Better to make the description general (you can list above as one example). Also I think calling it idx a bit limited, which means only numbers possible. Is it more flexible to call it 'handle' and then any string can be used here? > + params: extra parameters required by driver > + Example: > + # echo "12345678-1234-1234-1234-123456789abc:0:0" > > + /sys/bus/pci/devices/\:05\:00.0/mdev_create > + > +* mdev_destroy: (write only) > + Destroy a mediated device on a target physical device. > + Input syntax: > + where, > + UUID: mediated device's UUID > + idx: mediated device index inside a VM > + Example: > + # echo "12345678-1234-1234-1234-123456789abc:0" > > +/sys/bus/pci/devices/\:05\:00.0/mdev_destroy > + > +Under mdev class sysfs /sys/class/mdev/: > + > + > +* mdev_start: (write only) > + This trigger the registration interface to notify the driver to > + commit mediated device resource for target VM. > + The mdev_start function is a synchronized call, successful return of > + this call will indicate all the requested mdev resource has been fully > + committed, the VMM should continue. > + Input syntax: > + Example: > + # echo "12345678-1234-1234-1234-123456789abc" > > + /sys/class/mdev/mdev_start > + > +* mdev_stop: (write only) > + This trigger the registration interface to notify the driver to > + release resources of mediated device of target VM. > + Input syntax: > + Example: > + # echo "12345678-1234-1234-1234-123456789abc" > > + /sys/class/mdev/mdev_stop I think it's clearer to create a node per mdev under /sys/class/mdev, and then move start/stop as attributes under each mdev node, e.g: echo "0/1" > /sys/class/mdev/12345678-1234-1234-1234-123456789abc/start Doing this way is more extensible to add more capabilities under each mdev node, and different capability set may be implemented for them. > + > +Mediated device Hotplug: > +--- > + > +To support mediated device hotplug, and can be > +accessed during VM runtime, and the corresponding registration callback is > +invoked to allow driver to support hotplug. 'hotplug' is an action on the mdev user (e.g. the VM), not on mdev itself. You can always create a mdev as long as physical device has enough available resource to support requested config. Destroying a mdev may fail if there is still user on target mdev. Thanks Kevin
Re: [Qemu-devel] [Patch v1 26/29] qmp: add QMP interface "query-cpu-model-baseline"
> +## > +# @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). The QEMU machine QEMU has been started > +# with affects the result if CPU definitions are used that are not static. > +# > +# Note: This interface should not be used when global properties of CPU > classes > +# are changed (e.g. via "-cpu ..."). > +# > +# s390x supports baselining of all CPU models. Other architectures are not > +# supported yet. > +# > +# Returns: a CpuModelBaselineInfo. Returns an error if baselining CPU models > is > +# not supported, if a model cannot be used, if a model contains > +# an unknown cpu definition name, unknown properties or properties > +# with wrong types. > +# > +# Since: 2.8.0 > +## This comment is now: ## # @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). # # This interface can be used by tooling to create a compatible CPU model out # two CPU models. The created CPU model will be identical to or a subset of # both CPU models when comparing them. Therefore, the created CPU model is # guaranteed to run where the given CPU models run. # # The result returned by this command may be affected by: # # * QEMU version: CPU models may look different depending on the QEMU version. # (Except for CPU models reported as "static" in query-cpu-definitions.) # * machine-type: CPU model may look different depending on the machine-type. # (Except for CPU models reported as "static" in query-cpu-definitions.) # * machine options (including accelerator): in some architectures, CPU models # may look different depending on machine and accelerator options. (Except for # CPU models reported as "static" in query-cpu-definitions.) # * "-cpu" arguments and global properties: arguments to the -cpu option and # global properties may affect expansion of CPU models. Using # query-cpu-model-expansion while using these is not advised. # # Some architectures may not support baselining CPU models. s390x supports # baselining CPU models. # # Returns: a CpuModelBaselineInfo. Returns an error if baselining CPU models is # not supported, if a model cannot be used, if a model contains # an unknown cpu definition name, unknown properties or properties # with wrong types. # # Since: 2.8.0 ## David
Re: [Qemu-devel] [Patch v1 25/29] qmp: add QMP interface "query-cpu-model-comparison"
> +##
> +# @CpuModelCompareResult:
> +#
> +# An enumeration of CPU model comparation results.
> +#
> +# @incompatible: both model definition are incompatible
> +#
> +# @identical: model A == model B
> +#
> +# @superset: model A > model B
> +#
> +# @subset: model A < model B
> +#
> +# Since: 2.8.0
> +##
This comment is now:
##
# @CpuModelCompareResult:
#
# An enumeration of CPU model comparation results. The result is usually
# calcualted using e.g. at CPU features or CPU generations.
#
# @incompatible: If model A is incompatible to model B, model A is not
#guaranteed to run where model B runs and the other way around.
#
# @identical: If model A is identical to model B, model A is guaranteed to run
# where model B runs and the other way around.
#
# @superset: If model A is a superset of model B, model B is guaranteed to run
#where model A runs. There are no guarantees about the other way.
#
# @subset: If model A is a subset of model B, model A is guaranteed to run
# where model B runs. There are no guarantees about the other way.
#
# Since: 2.8.0
##
Think it's all about guarantees.
> +{ 'enum': 'CpuModelCompareResult',
> + 'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
> +
> +##
> +# @CpuModelCompareInfo
> +#
> +# The result of a CPU model comparison.
> +#
> +# @result: The result of the compare operation.
> +# @responsible-properties: List of properties that led to the comparison
> result
> +# not being identical.
> +#
> +# @responsible-properties is a list of QOM property names that led to
> +# both CPUs not being detected as identical. For identical models, this
> +# list is empty.
> +# If a QOM property is read-only, that means there's no known way to make the
> +# CPU models identical. If the special property name "type" is included, the
> +# models are by definition not identical and cannot be made identical.
> +#
> +# Since: 2.8.0
> +##
> +{ 'struct': 'CpuModelCompareInfo',
> + 'data': {'result': 'CpuModelCompareResult',
> + 'responsible-properties': ['str']
> + }
> +}
> +
> +##
> +# @query-cpu-model-comparison:
> +#
> +# Compares two CPU models, returning how they compare under a specific QEMU
> +# machine.
> +#
> +# Note: This interface should not be used when global properties of CPU
> classes
> +# are changed (e.g. via "-cpu ...").
> +#
> +# s390x supports comparing of all CPU models. Other architectures are not
> +# supported yet.
> +#
> +# Returns: a CpuModelBaselineInfo. Returns an error if comparing CPU models
> is
> +# not supported, if a model cannot be used, if a model contains
> +# an unknown cpu definition name, unknown properties or properties
> +# with wrong types.
> +#
> +# Since: 2.8.0
> +##
This comment is now:
##
# @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.
#
# Usually, a CPU model is compared against the maximum possible CPU model
# of a ceratin configuration (e.g. the "host" model for KVM). If that CPU
# model is identical or a subset, it will run in that configuration.
#
# The result returned by this command may be affected by:
#
# * QEMU version: CPU models may look different depending on the QEMU version.
# (Except for CPU models reported as "static" in query-cpu-definitions.)
# * machine-type: CPU model ma
Re: [Qemu-devel] [PATCH] virtio-net: allow increasing rx queue size
On Thu, 4 Aug 2016 02:16:14 +0300
"Michael S. Tsirkin" wrote:
> This allows increasing the rx queue size up to 1024: unlike with tx,
> guests don't put in huge S/G lists into RX so the risk of running into
> the max 1024 limitation due to some off-by-one seems small.
>
> It's helpful for users like OVS-DPDK which don't do any buffering on the
> host - 1K roughly matches 500 entries in tun + 256 in the current rx
> queue, which seems to work reasonably well. We could probably make do
> with ~750 entries but virtio spec limits us to powers of two.
> It might be a good idea to specify an s/g size limit in a future
> version.
>
> It also might be possible to make the queue size smaller down the road, 64
> seems like the minimal value which will still work (as guests seem to
> assume a queue full of 1.5K buffers is enough to process the largest
> incoming packet, which is ~64K). No one actually asked for this, and
> with virtio 1 guests can reduce ring size without need for host
> configuration, so don't bother with this for now.
Do we need some kind of sanity check that the guest did not resize
below a reasonable limit?
>
> Signed-off-by: Michael S. Tsirkin
> ---
> include/hw/virtio/virtio-net.h | 1 +
> hw/net/virtio-net.c| 22 +-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> @@ -1716,10 +1717,28 @@ static void virtio_net_device_realize(DeviceState
> *dev, Error **errp)
> VirtIONet *n = VIRTIO_NET(dev);
> NetClientState *nc;
> int i;
> +int min_rx_queue_size;
>
> virtio_net_set_config_size(n, n->host_features);
> virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
>
> +/*
> + * We set a lower limit on RX queue size to what it always was.
> + * Guests that want a smaller ring can always resize it without
> + * help from us (using virtio 1 and up).
> + */
> +min_rx_queue_size = 256;
I'd find it more readable to introduce a #define with the old queue
size as the minimum size...
> +if (n->net_conf.rx_queue_size < min_rx_queue_size ||
> +n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE ||
> +(n->net_conf.rx_queue_size & (n->net_conf.rx_queue_size - 1))) {
> +error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), "
> + "must be a power of 2 between %d and %d.",
> + n->net_conf.rx_queue_size, min_rx_queue_size,
> + VIRTQUEUE_MAX_SIZE);
> +virtio_cleanup(vdev);
> +return;
> +}
> +
> n->max_queues = MAX(n->nic_conf.peers.queues, 1);
> if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) {
> error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
> @@ -1880,6 +1899,7 @@ static Property virtio_net_properties[] = {
> TX_TIMER_INTERVAL),
> DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST),
> DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx),
> +DEFINE_PROP_UINT16("rx_queue_size", VirtIONet, net_conf.rx_queue_size,
> 256),
...and defaulting to that #define (or one derived from the #define
above) here.
> DEFINE_PROP_END_OF_LIST(),
> };
>
Do we need compat handling for the new property?
Re: [Qemu-devel] [PATCH] net: vmxnet: check fragment length during fragmentation
Hello Jason, +-- On Thu, 4 Aug 2016, Jason Wang wrote --+ | The patch doesn't apply cleanly on HEAD, we now move this logic to | hw/net/net_tx_pkt.c. Please resend on top of HEAD and cc Dmitry Fleytman | . I see, that explains why it did not show-up in search. I've sent a revised patch v2. Nevertheless, the patch here would apply to Qemu versions <= 2.6.0. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
On Monday, August 01, 2016 5:20 PM, Gonglei Wrote:
> -Original Message-
> From: Gonglei [mailto:arei.gong...@huawei.com]
> Sent: Monday, August 1, 2016 5:20 PM
> To: qemu-devel@nongnu.org; virtio-...@lists.oasis-open.org
> Cc: peter.huangp...@huawei.com; luoneng...@huawei.com;
> m...@redhat.com; cornelia.h...@de.ibm.com; stefa...@redhat.com;
> denglin...@chinamobile.com; jani.kokko...@huawei.com;
> ola.liljed...@arm.com; varun.se...@freescale.com; Zeng, Xin
> ; Keating, Brian A ; Ma,
> Liang J ; Griffin, John ;
> hanweid...@huawei.com; weidong.hu...@huawei.com; Gonglei
>
> Subject: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
>
> The virtio crypto device is a virtual crypto device (ie. hardware
> crypto accelerator card). The virtio crypto device can provide
> five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
>
> In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
>
> Signed-off-by: Gonglei
> CC: Michael S. Tsirkin
> CC: Cornelia Huck
> CC: Stefan Hajnoczi
> CC: Lingli Deng
> CC: Jani Kokkonen
> CC: Ola Liljedahl
> CC: Varun Sethi
> CC: Zeng Xin
> CC: Keating Brian
> CC: Ma Liang J
> CC: Griffin John
> CC: Hanweidong
> ---
> content.tex | 2 +
> virtio-crypto.tex | 793
> ++
> 2 files changed, 795 insertions(+)
> create mode 100644 virtio-crypto.tex
>
> diff --git a/content.tex b/content.tex
> index 4b45678..ab75f78 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len}, \field{residual},
> \field{status_qualifier}, \field{status}, \field{response} and
> \field{sense} fields.
>
> +\input{virtio-crypto.tex}
> +
> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>
> Currently there are three device-independent feature bits defined:
> diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> new file mode 100644
> index 000..9465de3
> --- /dev/null
> +++ b/virtio-crypto.tex
> @@ -0,0 +1,793 @@
> +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> +
> +The virtio crypto device is a virtual crypto device (ie. hardware
> +crypto accelerator card). The encryption and decryption requests of
> +are placed in the data queue, and handled by the real hardware crypto
> +accelerators finally. The second queue is the control queue, which
> +is used to create or destroy session for symmetric algorithms, and
> +to control some advanced features in the future. The virtio crypto
> +device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
> +KDF, ASYM, PRIMITIVE.
> +
> +\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID}
> +
> +20
> +
> +\subsection{Virtqueues}\label{sec:Device Types / Crypto Device /
> Virtqueues}
> +
> +\begin{description}
> +\item[0] dataq1
> +\item[\ldots]
> +\item[N-1] dataqN
> +\item[N] controlq
> +\end{description}
> +
> +N is set by \field{max_dataqueues} (\field{max_dataqueues} >= 1).
> +
> +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> bits}
> + None currently defined
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / Crypto
> Device / Device configuration layout}
> +
> +Thirteen driver-read-only configuration fields are currently defined.
> +
> +\begin{lstlisting}
> +struct virtio_crypto_config {
> +le32 version;
> +le16 status;
> +le16 max_dataqueues;
> +le32 crypto_services;
> +/* detailed algorithms mask */
> +le32 cipher_algo_l;
> +le32 cipher_algo_h;
> +le32 hash_algo;
> +le32 mac_algo_l;
> +le32 mac_algo_h;
> +le32 asym_algo;
> +le32 kdf_algo;
> +le32 aead_algo;
> +le32 primitive_algo;
> +};
> +\end{lstlisting}
> +
> +The first driver-read-only field, \field{version} specifies the virtio
> crypto's
> +version, which is reserved for back-compatibility in future.It's currently
> +defined for the version field:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_VERSION_1 (1)
Suggest to remove this macro,
Do you think a version which is composed of major version and
minor version is better?
> +\end{lstlisting}
> +
> +One read-only bit (for the device) is currently defined for the
> \field{status}
> +field: VIRTIO_CRYPTO_S_HW_READY.
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_S_HW_READY (1 << 0)
> +\end{lstlisting}
> +
> +The following driver-read-only field, \field{max_dataqueuess} specifies the
> +maximum number of data virtqueues (dataq1\ldots dataqN). The
> crypto_services
> +shows the crypto services the virtio crypto supports. The service currently
> +defined are:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_NO_SERVICE (0) /* cipher services */
Why is this bit needed?
> +#define VIRTIO_CRYPTO_SERVICE_CIPHER (1) /* cipher services */
> +#define VIRTIO_CRYPTO_SERVICE_HASH (2) /* hash service */
> +#define VIRTIO_CRYPTO_SERVICE_MAC (3) /* MAC (Message
> Authentication Codes) service */
> +#define
Re: [Qemu-devel] [PATCH V3] qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext
On Thu, Jul 28, 2016 at 05:12:25PM +0800, Zhang Chen wrote:
> Add qemu_chr_add_handlers_full() API, we can use
> this API pass in a GMainContext,make handler run
> in the context rather than main_loop.
> This comments from Daniel P . Berrange.
>
> Signed-off-by: Zhang Chen
> Signed-off-by: Li Zhijian
> Signed-off-by: Wen Congyang
> ---
> include/sysemu/char.h | 11 -
> qemu-char.c | 117
> +++---
> 2 files changed, 83 insertions(+), 45 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 307fd8f..86888bc 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -65,7 +65,8 @@ struct CharDriverState {
> int (*chr_sync_read)(struct CharDriverState *s,
> const uint8_t *buf, int len);
> GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond);
> -void (*chr_update_read_handler)(struct CharDriverState *s);
> +void (*chr_update_read_handler_full)(struct CharDriverState *s,
> + GMainContext *context);
If I were to nitpick, I'd say we probably didn't need to rename
this struct field - just adding the extra param is enough. Only
the public API needs the _full suffix.
> int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
> int (*get_msgfds)(struct CharDriverState *s, int* fds, int num);
> int (*set_msgfds)(struct CharDriverState *s, int *fds, int num);
> @@ -388,6 +389,14 @@ void qemu_chr_add_handlers(CharDriverState *s,
> IOEventHandler *fd_event,
> void *opaque);
>
> +/* This API can make handler run in the context what you pass to. */
> +void qemu_chr_add_handlers_full(CharDriverState *s,
> +IOCanReadHandler *fd_can_read,
> +IOReadHandler *fd_read,
> +IOEventHandler *fd_event,
> +void *opaque,
> +GMainContext *context);
> +
> void qemu_chr_be_generic_open(CharDriverState *s);
> void qemu_chr_accept_input(CharDriverState *s);
> int qemu_chr_add_client(CharDriverState *s, int fd);
> diff --git a/qemu-char.c b/qemu-char.c
> index b597ee1..c544427 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -448,11 +448,12 @@ void qemu_chr_fe_printf(CharDriverState *s, const char
> *fmt, ...)
>
> static void remove_fd_in_watch(CharDriverState *chr);
>
> -void qemu_chr_add_handlers(CharDriverState *s,
> - IOCanReadHandler *fd_can_read,
> - IOReadHandler *fd_read,
> - IOEventHandler *fd_event,
> - void *opaque)
> +void qemu_chr_add_handlers_full(CharDriverState *s,
> +IOCanReadHandler *fd_can_read,
> +IOReadHandler *fd_read,
> +IOEventHandler *fd_event,
> +void *opaque,
> +GMainContext *context)
> {
> int fe_open;
>
> @@ -466,8 +467,9 @@ void qemu_chr_add_handlers(CharDriverState *s,
> s->chr_read = fd_read;
> s->chr_event = fd_event;
> s->handler_opaque = opaque;
> -if (fe_open && s->chr_update_read_handler)
> -s->chr_update_read_handler(s);
> +if (fe_open && s->chr_update_read_handler_full) {
> +s->chr_update_read_handler_full(s, context);
> +}
>
> if (!s->explicit_fe_open) {
> qemu_chr_fe_set_open(s, fe_open);
> @@ -480,6 +482,16 @@ void qemu_chr_add_handlers(CharDriverState *s,
> }
> }
>
> +void qemu_chr_add_handlers(CharDriverState *s,
> + IOCanReadHandler *fd_can_read,
> + IOReadHandler *fd_read,
> + IOEventHandler *fd_event,
> + void *opaque)
> +{
> +qemu_chr_add_handlers_full(s, fd_can_read, fd_read,
> + fd_event, opaque, NULL);
> +}
> +
> static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
> {
> return len;
> @@ -717,7 +729,8 @@ static void mux_chr_event(void *opaque, int event)
> mux_chr_send_event(d, i, event);
> }
>
> -static void mux_chr_update_read_handler(CharDriverState *chr)
> +static void mux_chr_update_read_handler_full(CharDriverState *chr,
> + GMainContext *context)
Likewise we could avoid the rename of this and similar internal only
methods, which would cut a few lines from the patch.
> {
> MuxDriver *d = chr->opaque;
>
> @@ -731,8 +744,10 @@ static void mux_chr_update_read_handler(CharDriverState
> *chr)
> d->chr_event[d->mux_cnt] = chr->chr_event;
> /* Fix up the real driver with mux routines */
> if (d->mux_cnt == 0) {
> -qemu_chr_add_handlers(d->drv, mux_ch
Re: [Qemu-devel] [PATCH 3/5] sclpconsole: remove bogus check for -EAGAIN
On Wed, 3 Aug 2016 17:22:38 +0100
"Daniel P. Berrange" wrote:
s/sclpconsole/sclpconsolelm/ in the subject, as there are two sclp
consoles :)
> The write_console_data() method in sclpconsole-lm.c checks
> whether the return value of qemu_chr_fe_write() has the
> value of -EAGAIN and if so then increments the buffer offset
> by the value of EAGAIN. Fortunately qemu_chr_fe_write() will
> never return EAGAIN directly, rather it returns -1 with
> errno set to EAGAIN, so this broken code path was not
> reachable. The behaviour on EAGAIN was stil bad though,
> causing the write_console_data() to busy_wait repeatedly
> calling qemu_chr_fe_write() with no sleep between iters.
>
> Just remove all this loop logic and replace with a call
> to qemu_chr_fe_write_all().
>
> Signed-off-by: Daniel P. Berrange
> ---
> hw/char/sclpconsole-lm.c | 18 +++---
> 1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> index a22ad8d..8cb0026 100644
> --- a/hw/char/sclpconsole-lm.c
> +++ b/hw/char/sclpconsole-lm.c
> @@ -201,21 +201,9 @@ static int write_console_data(SCLPEvent *event, const
> uint8_t *buf, int len)
> return len;
> }
>
> -buf_offset = buf;
> -while (len > 0) {
> -ret = qemu_chr_fe_write(scon->chr, buf, len);
> -if (ret == 0) {
> -/* a pty doesn't seem to be connected - no error */
> -len = 0;
> -} else if (ret == -EAGAIN || (ret > 0 && ret < len)) {
> -len -= ret;
> -buf_offset += ret;
> -} else {
> -len = 0;
> -}
> -}
> -
> -return ret;
> +/* XXX this blocks entire thread. Rewrite to use
> + * qemu_chr_fe_write and background I/O callbacks */
> +return qemu_chr_fe_write_all(scon->chr, buf, len);
This is basically the same change that had been done for the sclp vt220
console already in 2e14211 ("s390/sclpconsole: handle char layer busy
conditions"), so this looks fine.
> }
>
> static int process_mdb(SCLPEvent *event, MDBO *mdbo)
Acked-by: Cornelia Huck
Re: [Qemu-devel] [PATCH V3] qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext
On 08/04/2016 04:05 PM, Daniel P. Berrange wrote:
On Thu, Jul 28, 2016 at 05:12:25PM +0800, Zhang Chen wrote:
Add qemu_chr_add_handlers_full() API, we can use
this API pass in a GMainContext,make handler run
in the context rather than main_loop.
This comments from Daniel P . Berrange.
Signed-off-by: Zhang Chen
Signed-off-by: Li Zhijian
Signed-off-by: Wen Congyang
---
include/sysemu/char.h | 11 -
qemu-char.c | 117 +++---
2 files changed, 83 insertions(+), 45 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 307fd8f..86888bc 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -65,7 +65,8 @@ struct CharDriverState {
int (*chr_sync_read)(struct CharDriverState *s,
const uint8_t *buf, int len);
GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond);
-void (*chr_update_read_handler)(struct CharDriverState *s);
+void (*chr_update_read_handler_full)(struct CharDriverState *s,
+ GMainContext *context);
If I were to nitpick, I'd say we probably didn't need to rename
this struct field - just adding the extra param is enough. Only
the public API needs the _full suffix.
OK~ I will fix this in next version.
int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
int (*get_msgfds)(struct CharDriverState *s, int* fds, int num);
int (*set_msgfds)(struct CharDriverState *s, int *fds, int num);
@@ -388,6 +389,14 @@ void qemu_chr_add_handlers(CharDriverState *s,
IOEventHandler *fd_event,
void *opaque);
+/* This API can make handler run in the context what you pass to. */
+void qemu_chr_add_handlers_full(CharDriverState *s,
+IOCanReadHandler *fd_can_read,
+IOReadHandler *fd_read,
+IOEventHandler *fd_event,
+void *opaque,
+GMainContext *context);
+
void qemu_chr_be_generic_open(CharDriverState *s);
void qemu_chr_accept_input(CharDriverState *s);
int qemu_chr_add_client(CharDriverState *s, int fd);
diff --git a/qemu-char.c b/qemu-char.c
index b597ee1..c544427 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -448,11 +448,12 @@ void qemu_chr_fe_printf(CharDriverState *s, const char
*fmt, ...)
static void remove_fd_in_watch(CharDriverState *chr);
-void qemu_chr_add_handlers(CharDriverState *s,
- IOCanReadHandler *fd_can_read,
- IOReadHandler *fd_read,
- IOEventHandler *fd_event,
- void *opaque)
+void qemu_chr_add_handlers_full(CharDriverState *s,
+IOCanReadHandler *fd_can_read,
+IOReadHandler *fd_read,
+IOEventHandler *fd_event,
+void *opaque,
+GMainContext *context)
{
int fe_open;
@@ -466,8 +467,9 @@ void qemu_chr_add_handlers(CharDriverState *s,
s->chr_read = fd_read;
s->chr_event = fd_event;
s->handler_opaque = opaque;
-if (fe_open && s->chr_update_read_handler)
-s->chr_update_read_handler(s);
+if (fe_open && s->chr_update_read_handler_full) {
+s->chr_update_read_handler_full(s, context);
+}
if (!s->explicit_fe_open) {
qemu_chr_fe_set_open(s, fe_open);
@@ -480,6 +482,16 @@ void qemu_chr_add_handlers(CharDriverState *s,
}
}
+void qemu_chr_add_handlers(CharDriverState *s,
+ IOCanReadHandler *fd_can_read,
+ IOReadHandler *fd_read,
+ IOEventHandler *fd_event,
+ void *opaque)
+{
+qemu_chr_add_handlers_full(s, fd_can_read, fd_read,
+ fd_event, opaque, NULL);
+}
+
static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
return len;
@@ -717,7 +729,8 @@ static void mux_chr_event(void *opaque, int event)
mux_chr_send_event(d, i, event);
}
-static void mux_chr_update_read_handler(CharDriverState *chr)
+static void mux_chr_update_read_handler_full(CharDriverState *chr,
+ GMainContext *context)
Likewise we could avoid the rename of this and similar internal only
methods, which would cut a few lines from the patch.
I got your point. will fix it.
{
MuxDriver *d = chr->opaque;
@@ -731,8 +744,10 @@ static void mux_chr_update_read_handler(CharDriverState *chr)
d->chr_event[d->mux_cnt] = chr->chr_event;
/* Fix up the real driver with mux routines */
if (d->mux_cnt == 0) {
-qemu_chr_add_handlers(d->drv, mux_chr_can_read, mux_chr_read,
-
Re: [Qemu-devel] [PATCH 4/5] hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all
On Wed, 3 Aug 2016 17:22:39 +0100
"Daniel P. Berrange" wrote:
> The qemu_chr_fe_write method will return -1 on EAGAIN if the
> chardev backend write would block. Almost no callers of the
> qemu_chr_fe_write() method check the return value, instead
> blindly assuming data was successfully sent. In most cases
> this will lead to silent data loss on interactive consoles,
> but in some cases (eg RNG EGD) it'll just cause corruption
> of the protocol being spoken.
>
> We unfortunately can't fix the virtio-console code, due to
> a bug in the Linux guest drivers, which would cause the
> entire Linux kernel to hang if we delay processing of the
> incoming data in any way. Fixing this requires first fixing
> the guest driver to not hold spinlocks while writing to the
> hvc device backend.
>
> Fixes bug: https://bugs.launchpad.net/qemu/+bug/1586756
>
> Signed-off-by: Daniel P. Berrange
> ---
> backends/rng-egd.c | 4 +++-
> gdbstub.c | 4 +++-
> hw/arm/omap2.c | 8 +---
> hw/arm/pxa2xx.c | 4 +++-
> hw/arm/strongarm.c | 4 +++-
> hw/char/bcm2835_aux.c | 4 +++-
> hw/char/debugcon.c | 4 +++-
> hw/char/digic-uart.c| 2 ++
> hw/char/escc.c | 4 +++-
> hw/char/etraxfs_ser.c | 4 +++-
> hw/char/exynos4210_uart.c | 4 +++-
> hw/char/grlib_apbuart.c | 4 +++-
> hw/char/imx_serial.c| 4 +++-
> hw/char/ipoctal232.c| 4 +++-
> hw/char/lm32_juart.c| 2 ++
> hw/char/lm32_uart.c | 2 ++
> hw/char/mcf_uart.c | 4 +++-
> hw/char/parallel.c | 4 +++-
> hw/char/pl011.c | 4 +++-
> hw/char/sclpconsole-lm.c| 4 +++-
> hw/char/sclpconsole.c | 2 ++
ack for the sclp consoles
> hw/char/sh_serial.c | 4 +++-
> hw/char/spapr_vty.c | 5 +++--
> hw/char/stm32f2xx_usart.c | 2 ++
> hw/char/virtio-console.c| 21 +
> hw/char/xilinx_uartlite.c | 4 +++-
> hw/usb/ccid-card-passthru.c | 7 +--
> hw/usb/dev-serial.c | 4 +++-
> slirp/slirp.c | 4 +++-
> 29 files changed, 104 insertions(+), 27 deletions(-)
>
> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> index 4f0e03d..d44c18c 100644
> --- a/hw/char/virtio-console.c
> +++ b/hw/char/virtio-console.c
> @@ -68,6 +68,27 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
> */
> if (ret < 0)
> ret = 0;
> +
> +/* XXX we should be queuing data to send later for the
> + * console devices too rather than silently dropping
> + * console data on EAGAIN. The Linux virtio-console
> + * hvc driver though does sends with spinlocks held,
> + * so if we enable throttling that'll stall the entire
> + * guest kernel, not merely the process writing to the
> + * console.
> + *
> + * While we could queue data for later write without
> + * enabling throttling, this would result in the guest
> + * being able to trigger arbitrary memory usage in QEMU
> + * buffering data for later writes.
> + *
> + * So fixing this problem likely requires fixing the
> + * Linux virtio-console hvc driver to not hold spinlocks
> + * while writing, and instead merely block the process
> + * that's writing. QEMU would then need some way to detect
> + * if the guest had the fixed driver too, before we can
> + * use throttling on host side.
Probably best done via a new virtio-console feature bit, as this would
be OS agnostic.
> + */
> if (!k->is_console) {
> virtio_serial_throttle_port(port, true);
> if (!vcon->watch) {
Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel
On 04/08/2016 04:38, David Gibson wrote:
> On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
>> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
>> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
>> the compressed dump. It works fine with Macintosh, but not with
>> pseries as the kernel default page size is 64KB.
>>
>> Without this patch, if we generate a compressed dump in the QEMU monitor:
>>
>> (qemu) dump-guest-memory -z qemu.dump
>>
>> This dump cannot be read by crash:
>>
>> # crash vmlinux qemu.dump
>> ...
>> WARNING: cannot translate vmemmap kernel virtual addresses:
>> commands requiring page structure contents will fail
>> ...
>>
>> Signed-off-by: Laurent Vivier
>> ---
>> target-ppc/arch_dump.c | 5 +
>> 1 file changed, 5 insertions(+)
>
> Urgh.. so, really the page size used by the guest kernel is a
> guest-side detail, and it's certainly possible to build a 4kiB page
> guest kernel, although 64kiB is the norm.
virtio-balloon doesn't work with 4K kernel.
> This might be the best we can do, but it'd be nice if we could probe
> or otherwise avoid relying on this assumption about the guest kernel.
I agree with you but none of the other architectures probes for the page
size.
For instance ARM: |I cc: Drew to know how he has chosen the values]
if (arm_feature(env, ARM_FEATURE_AARCH64)) {
...
info->page_size = (1 << 16);
...
} else {
...
info->page_size = (1 << 12);
...
}
In the kernel:
arch/arm64/include/asm/page.h:
#define PAGE_SHIFT CONFIG_ARM64_PAGE_SHIFT
arch/arm64/Kconfig:
config ARM64_PAGE_SHIFT
int
default 16 if ARM64_64K_PAGES
default 14 if ARM64_16K_PAGES
default 12
choice
prompt "Page size"
default ARM64_4K_PAGES
help
Page size (translation granule) configuration.
config ARM64_4K_PAGES
bool "4KB"
help
This feature enables 4KB pages support.
config ARM64_16K_PAGES
bool "16KB"
help
The system will use 16KB pages support. AArch32 emulation
requires applications compiled with 16K (or a multiple of 16K)
aligned segments.
config ARM64_64K_PAGES
bool "64KB"
help
This feature enables 64KB pages support (4KB by default)
allowing only two levels of page tables and faster TLB
look-up. AArch32 emulation requires applications compiled
with 64K aligned segments.
endchoice
I think we can't rely on the CPU state or the memory content as they can
be corrupted.
Thanks,
Laurent
Re: [Qemu-devel] [virtio-dev][RFC v2 2/2] virtio-sdm: new device specification
On Fri, Jul 22, 2016 at 06:18:25PM +0200, Christian Pinto wrote: > Hello Stefan, > > On Tue, Jul 19, 2016 at 10:40 AM, Stefan Hajnoczi > wrote: > > > On Tue, Jul 19, 2016 at 09:47:13AM +0200, Christian Pinto wrote: > > > > > +During the initialization phase the device connects also to the > > > > communication > > > > > +channel. It has to be noted that the behavior of the device is > > > > > +independent from the communication channel used, that is a detail of > > > > each > > > > > +specific implementation of the SDM device. > > > > > > > > How are SDM devices identified? For example, if two SDM devices are > > > > available, how does the driver know which one serves a particular > > > > function? > > > > > > > > > > The master-slave role is supposed to be enough to identify the device. If > > > as an example we consider an AMP system, the master core will only see > > one > > > SDM master device, while the slave processor will only see the slave SDM > > > instance. Then it is up to the implementation of the drivers to define > > the > > > signals served, while the SDM hardware is only in charge of forwarding > > such > > > signals. We do not foresee the need to have one SDM instance for each > > > signal type. > > > > The laissez-faire approach of allowing the implementation to define > > signals breaks in an environment where there can be multiple versions of > > the SDM hardware. > > > > Virtio feature bits cannot be used to define signal-related > > functionality because it's implementation-defined. For example, there > > is no way to express "CUSTOM_SIGNAL_2 is supported". > > > > In a guest OS image that can run on two different types of AMP systems, > > how do you distinguish between the set of signals to use? > > > > I guess we can say that the driver has some external knowledge (e.g. the > > board/chipset type) that allows it to know the meaning of signals and > > which ones are available? > > > > Here I see two possibilities either what you propose, to demand on an > external > knowledge of the driver on the implementation of the SDM device for the > specific board/chipset. Or alternatively we could think to embed the set of > signals > supported by the implementation of the device in the configuration space. > We could univocally define signals in the specification of the device, > statically assigning each signal to an ID. > At devices initialization the driver reads the configuration and retrieves > the set of > signals supported. It is then up-to the user-level software to know the > semantic of each > signal (e.g., the meaning of the payload), that makes sense in my opinion. > We could also envision that at connection time between master and slave > there is > an handshake phase where the slave notifies the master with the set of > signals > it supports, and a slave can get rejected in case of mismatch. > > Does this sound in line with the virtio philosophy? > > Finally, the idea of the SDM is that in each deployment there is only one > master > and multiple slaves. I think the most satisfying approach from the VIRTIO spec perspective is to include the signals in the spec. That way feature bits can be used. Stefan signature.asc Description: PGP signature
[Qemu-devel] [PATCH V4] qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext
Add qemu_chr_add_handlers_full() API, we can use
this API pass in a GMainContext,make handler run
in the context rather than main_loop.
This comments from Daniel P . Berrange.
Signed-off-by: Zhang Chen
Signed-off-by: Li Zhijian
Signed-off-by: Wen Congyang
Reviewed-by: Daniel P. Berrange
---
include/sysemu/char.h | 11 +++-
qemu-char.c | 77 +++
2 files changed, 63 insertions(+), 25 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 307fd8f..f997849 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -65,7 +65,8 @@ struct CharDriverState {
int (*chr_sync_read)(struct CharDriverState *s,
const uint8_t *buf, int len);
GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond);
-void (*chr_update_read_handler)(struct CharDriverState *s);
+void (*chr_update_read_handler)(struct CharDriverState *s,
+GMainContext *context);
int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
int (*get_msgfds)(struct CharDriverState *s, int* fds, int num);
int (*set_msgfds)(struct CharDriverState *s, int *fds, int num);
@@ -388,6 +389,14 @@ void qemu_chr_add_handlers(CharDriverState *s,
IOEventHandler *fd_event,
void *opaque);
+/* This API can make handler run in the context what you pass to. */
+void qemu_chr_add_handlers_full(CharDriverState *s,
+IOCanReadHandler *fd_can_read,
+IOReadHandler *fd_read,
+IOEventHandler *fd_event,
+void *opaque,
+GMainContext *context);
+
void qemu_chr_be_generic_open(CharDriverState *s);
void qemu_chr_accept_input(CharDriverState *s);
int qemu_chr_add_client(CharDriverState *s, int fd);
diff --git a/qemu-char.c b/qemu-char.c
index b597ee1..a926175 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -448,11 +448,12 @@ void qemu_chr_fe_printf(CharDriverState *s, const char
*fmt, ...)
static void remove_fd_in_watch(CharDriverState *chr);
-void qemu_chr_add_handlers(CharDriverState *s,
- IOCanReadHandler *fd_can_read,
- IOReadHandler *fd_read,
- IOEventHandler *fd_event,
- void *opaque)
+void qemu_chr_add_handlers_full(CharDriverState *s,
+IOCanReadHandler *fd_can_read,
+IOReadHandler *fd_read,
+IOEventHandler *fd_event,
+void *opaque,
+GMainContext *context)
{
int fe_open;
@@ -466,8 +467,9 @@ void qemu_chr_add_handlers(CharDriverState *s,
s->chr_read = fd_read;
s->chr_event = fd_event;
s->handler_opaque = opaque;
-if (fe_open && s->chr_update_read_handler)
-s->chr_update_read_handler(s);
+if (fe_open && s->chr_update_read_handler) {
+s->chr_update_read_handler(s, context);
+}
if (!s->explicit_fe_open) {
qemu_chr_fe_set_open(s, fe_open);
@@ -480,6 +482,16 @@ void qemu_chr_add_handlers(CharDriverState *s,
}
}
+void qemu_chr_add_handlers(CharDriverState *s,
+ IOCanReadHandler *fd_can_read,
+ IOReadHandler *fd_read,
+ IOEventHandler *fd_event,
+ void *opaque)
+{
+qemu_chr_add_handlers_full(s, fd_can_read, fd_read,
+ fd_event, opaque, NULL);
+}
+
static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
return len;
@@ -717,7 +729,8 @@ static void mux_chr_event(void *opaque, int event)
mux_chr_send_event(d, i, event);
}
-static void mux_chr_update_read_handler(CharDriverState *chr)
+static void mux_chr_update_read_handler(CharDriverState *chr,
+GMainContext *context)
{
MuxDriver *d = chr->opaque;
@@ -731,8 +744,10 @@ static void mux_chr_update_read_handler(CharDriverState
*chr)
d->chr_event[d->mux_cnt] = chr->chr_event;
/* Fix up the real driver with mux routines */
if (d->mux_cnt == 0) {
-qemu_chr_add_handlers(d->drv, mux_chr_can_read, mux_chr_read,
- mux_chr_event, chr);
+qemu_chr_add_handlers_full(d->drv, mux_chr_can_read,
+ mux_chr_read,
+ mux_chr_event,
+ chr, context);
}
if (d->focus != -1) {
mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
@@ -840,6 +855,7 @@ typedef struct IOWatchPoll
IOCanReadHandler *fd_can_read;
GSourceFunc fd_read;
void *opaque;
+GMainContext *context;
} IOWatchPoll;
static
Re: [Qemu-devel] [PATCH 0/3] trivial changes of timer & cpus
> > Thanks, these can go in 2.8. > > I am fine with the time. Do you want me to repost these after 2.8? No, it's okay. Paolo
Re: [Qemu-devel] [PATCH 2/2] iotests: fix 109
Dear Max, Max Reitz writes: > On 03.08.2016 17:22, Sascha Silbe wrote: [...] >> What are the exact semantics of the "offset" field for >> BLOCK_JOB_COMPLETED? >> >> docs/qmp-events.txt is rather vague. As an API consumer I'd have assumed >> that everything up to offset has been completed successfully. If that >> interpretation is correct, offset must be 0 for this test because the >> very first sector wasn't mirrored successfully. > > As far as I'm aware, it doesn't have any real semantics besides the fact > that $offset / $len is the progress of the block job; so it's the offset > "in the job", but not the offset in the source disk. Ok. Would be nice to mention that in docs/qmp-events.txt, to avoid API consumers relying on more exact semantics (resuming at the block after offset to step over error locations for example). The name 'offset' is pretty suggestive... Sascha -- Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg https://se-silbe.de/ USt-IdNr. DE281696641
Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
Hi Paolo,
On Tue, Aug 2, 2016 at 4:18 PM, Paolo Bonzini wrote:
> - Original Message -
>> From: "vijay kilari"
>> To: qemu-...@nongnu.org, "peter maydell" ,
>> pbonz...@redhat.com
>> Cc: qemu-devel@nongnu.org, "Prasun Kapoor" ,
>> "vijay kilari" ,
>> "Vijaya Kumar K"
>> Sent: Tuesday, August 2, 2016 12:20:15 PM
>> Subject: [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
>>
>> From: Vijaya Kumar K
>>
>> Add helper API to read MIDR_EL1 registers to fetch
>> cpu identification information. This helps in
>> adding errata's and architecture specific features.
>>
>> This is implemented only for arm architecture.
>>
>> Signed-off-by: Vijaya Kumar K
>> ---
>> include/qemu-common.h |1 +
>> util/Makefile.objs|1 +
>> util/cpuinfo.c| 52
>> +
>> 3 files changed, 54 insertions(+)
>>
>> diff --git a/include/qemu-common.h b/include/qemu-common.h
>> index 1f2cb94..62ad674 100644
>> --- a/include/qemu-common.h
>> +++ b/include/qemu-common.h
>> @@ -134,4 +134,5 @@ void page_size_init(void);
>> * returned. */
>> bool dump_in_progress(void);
>>
>> +long int qemu_read_cpuid_info(void);
>
> First, please avoid adding to include/qemu-common.h (it really should
> go away).
>
> Second, this is too generic a name. Please call it something like
> qemu_read_aarch64_midr_el1.
OK
>
> Third, it's probably a bad idea to call this function from generic code, so
> make it static and add the detection function from patch 2/2 already here.
> By making it static, it's also possible to define it only if CONFIG_LINUX
> is defined; the ThunderX detection will then return false if !CONFIG_LINUX.
>
You mean to say, move contents of this patch to util/cutils.c and make it
static and define under __aarch64__ and CONFIG_LINUX?.
> Thanks,
>
> Paolo
>
>> #endif
>> diff --git a/util/Makefile.objs b/util/Makefile.objs
>> index 96cb1e0..9d25a72 100644
>> --- a/util/Makefile.objs
>> +++ b/util/Makefile.objs
>> @@ -35,3 +35,4 @@ util-obj-y += log.o
>> util-obj-y += qdist.o
>> util-obj-y += qht.o
>> util-obj-y += range.o
>> +util-obj-y += cpuinfo.o
>> diff --git a/util/cpuinfo.c b/util/cpuinfo.c
>> new file mode 100644
>> index 000..3ba7194
>> --- /dev/null
>> +++ b/util/cpuinfo.c
>> @@ -0,0 +1,52 @@
>> +/*
>> + * Dealing with arm cpu identification information.
>> + *
>> + * Copyright (C) 2016 Cavium, Inc.
>> + *
>> + * Authors:
>> + * Vijaya Kumar K
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1
>> + * or later. See the COPYING.LIB file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu-common.h"
>> +#include "qemu/cutils.h"
>> +
>> +#if defined(__aarch64__)
>> +
>> +long int qemu_read_cpuid_info(void)
>> +{
>> +FILE *fp;
>> +char *buf;
>> +long int midr = 0;
>> +#define BUF_SIZE 32
>> +
>> +fp = fopen("/sys/devices/system/cpu/cpu0/regs/identification/midr_el1",
>> + "r");
>> +if (!fp) {
>> +return 0;
>> +}
>> +
>> +buf = g_malloc0(BUF_SIZE);
>> +if (!buf) {
>> +fclose(fp);
>> +return 0;
>> +}
>> +
>> +if (buf != fgets(buf, BUF_SIZE - 1, fp)) {
>> +goto out;
>> +}
>> +
>> +if (qemu_strtol(buf, NULL, 0, &midr) < 0) {
>> +goto out;
>> +}
>> +
>> +out:
>> +g_free(buf);
>> +fclose(fp);
>> +
>> +return midr;
>> +}
>> +#endif
>> --
>> 1.7.9.5
>>
>>
Re: [Qemu-devel] [PULL for-2.7 0/3] vnc: fixes for "-vnc none".
On 3 August 2016 at 16:35, Gerd Hoffmann wrote: > Hi, > > vnc patch queue, with three patches fixing "-vnc none" with > connects via monitor fd passing. > > cheers, > Gerd > > The following changes since commit cc0100f464c94bf80ad36cd432f4a1ed58126b60: > > MAINTAINERS: Update the Xilinx maintainers (2016-08-01 15:31:32 +0100) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/pull-vnc-20160803-1 > > for you to fetch changes up to 12e29b1682e0a50ed57c324152addb585ae5ce69: > > vnc: ensure connection sharing/limits is always configured (2016-08-03 > 15:06:32 +0200) > > > vnc: fixes for "-vnc none". > > > Daniel P. Berrange (3): > vnc: don't crash getting server info if lsock is NULL > vnc: fix crash when vnc_server_info_get has an error > vnc: ensure connection sharing/limits is always configured Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v7 02/16] block: Convert bdrv_pwrite_compressed() to BdrvChild
On Fri, Jul 22, 2016 at 11:17:41AM +0300, Denis V. Lunev wrote: > From: Pavel Butsykin > > Signed-off-by: Pavel Butsykin > Signed-off-by: Denis V. Lunev > Reviewed-by: Eric Blake > CC: Jeff Cody > CC: Markus Armbruster > CC: Eric Blake > CC: John Snow > CC: Stefan Hajnoczi > CC: Kevin Wolf > --- > block/block-backend.c | 2 +- > block/io.c| 3 ++- > include/block/block.h | 2 +- > 3 files changed, 4 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v4 1/2] Interface for grant copy operation in libs.
The code looks ok. I have two minor suggestions below.
I would suggest changing the subject line to:
libs/gnttab: introduce grant copy interface
On Tue, Aug 02, 2016 at 04:06:29PM +0200, Paulina Szubarczyk wrote:
> In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
> system call is invoked. In mini-os the operation is yet not
> implemented. For the OSs that does not implement gnttab the
> call of the grant copy operation causes abort.
>
> Signed-off-by: Paulina Szubarczyk
> ---
> Changes since v3:
> - revert to cast from xengnttab_grant_copy_segment_t
> to ioctl_gntdev_grant_copy.
> - added compile-time check to compare the libs
> xengnttab_grant_copy_segment_t with the ioctl structure.
> The patch relies on Wei patch introducing XENGNTTAB_BUILD_BUG_ON
> in libs/gnttab.
I should resubmit that one soon.
> ---
[...]
> +rc = ioctl(fd, IOCTL_GNTDEV_GRANT_COPY, ©);
> +if (rc)
> +{
> +GTERROR(xgt->logger, "ioctl GRANT COPY failed %d ", errno);
> +}
Normally for a single statement you don't need {} around it.
No need to resubmit just because of this patch. I can handle the subject
line change, fix up the style issue and change the comment according to
David's suggestion while committing if you don't object to any of them.
I won't commit this patch right away though. I will wait until the QEMU
patch is acked because I would avoid committing things that have no
users.
If you end up submitting another version you can make those changes
yourself.
Wei.
Re: [Qemu-devel] [PATCH v7 00/16] backup compression
On Fri, Jul 22, 2016 at 11:17:39AM +0300, Denis V. Lunev wrote: > The idea is simple - backup is "written-once" data. It is written block > by block and it is large enough. It would be nice to save storage > space and compress it. > > These patches add the ability to compress data during backup. This > functionality is implemented by means of adding options to the qmp/hmp > commands(drive-backup, blockdev-backup). The implementation is quite > simple, because the responsibility for data compression imposed on the > format driver. > > Changes from v1: > - added unittest for backup compression (12, 13) > > Changes from v2: > - implemented a new .bdrv_co_write_compressed interface to replace the > old .bdrv_write_compressed (2,3,4,5,6) > > Changes from v3: > - added the byte-based interfaces: > blk_pwrite_compressed()/blk_co_pwritev_compressed() (1, 7) > - fix drive/blockdev-backup documentation (10, 11) > > Changes from v4: > - added assert that offset and count are aligned (1) > - reuse RwCo and bdrv_co_pwritev() for write compressed (2) > - converted interfaces to byte-based for format drivers (2, 3, 5, 6) > - move an unrelated cleanup in a separate patches (4, 7) > - turn on dirty_bitmaps for the compressed writes (9) > - added simplify drive/blockdev-backup by using the boxed commands (10, 11) > - reworded drive/blockdev-backup documentation about compression (12, 13) > - fix s/bakup/backup/ (14) > > Changes from v5: > - rebased on master > - fix grammar (5, 8) > > Changes from v6: > - more grammar fixes (1,11) > - assignment cleanup as suggested by Eric in 11 > > Signed-off-by: Pavel Butsykin > Signed-off-by: Denis V. Lunev > CC: Jeff Cody > CC: Markus Armbruster > CC: Eric Blake > CC: John Snow > CC: Stefan Hajnoczi > CC: Kevin Wolf > > Pavel Butsykin (16): > block: switch blk_write_compressed() to byte-based interface > block: Convert bdrv_pwrite_compressed() to BdrvChild > block/io: reuse bdrv_co_pwritev() for write compressed > qcow2: add qcow2_co_pwritev_compressed > qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion > vmdk: add vmdk_co_pwritev_compressed > qcow: add qcow_co_pwritev_compressed > qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion > block: remove BlockDriver.bdrv_write_compressed > block/io: turn on dirty_bitmaps for the compressed writes > block: simplify drive-backup > block: simplify blockdev-backup > drive-backup: added support for data compression > blockdev-backup: added support for data compression > qemu-iotests: test backup compression in 055 > qemu-iotests: add vmdk for test backup compression in 055 > > block/backup.c | 12 ++- > block/block-backend.c | 27 +- > block/io.c | 48 -- > block/qcow.c | 113 +--- > block/qcow2.c | 128 ++- > block/vmdk.c | 55 ++-- > blockdev.c | 193 > ++--- > hmp-commands.hx| 8 +- > hmp.c | 24 ++--- > include/block/block.h | 5 +- > include/block/block_int.h | 5 +- > include/sysemu/block-backend.h | 4 +- > qapi/block-core.json | 18 +++- > qemu-img.c | 8 +- > qemu-io-cmds.c | 2 +- > qmp-commands.hx| 9 +- > tests/qemu-iotests/055 | 118 + > tests/qemu-iotests/055.out | 4 +- > tests/qemu-iotests/iotests.py | 10 +-- > 19 files changed, 366 insertions(+), 425 deletions(-) > > -- > 2.1.4 > Acked-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [Xen-devel] [PATCH v4 1/2] Interface for grant copy operation in libs.
On 03/08/16 15:36, David Vrabel wrote: > On 02/08/16 15:06, Paulina Szubarczyk wrote: >> >> +/** >> + * Copy memory from or to grant references. The information of each >> operations >> + * are contained in 'xengnttab_grant_copy_segment_t'. The @flag value >> indicate >> + * the direction of an operation (GNTCOPY_source_gref\GNTCOPY_dest_gref). >> + * >> + * The sum of fields @offset[i] and @len[i] of >> 'xengnttab_grant_copy_segment_t' >> + * should not exceed XEN_PAGE_SIZE > > "For each segment, @virt may cross a page boundary but @offset + @len > must be less than XEN_PAGE_SIZE." might be better. This should be: "For each segment, @virt may cross a page boundary but @offset + @len must not exceed XEN_PAGE_SIZE." David
Re: [Qemu-devel] [PATCH v24 01/12] unblock backup operations in backing file
On Wed, Jul 27, 2016 at 03:01:42PM +0800, Changlong Xie wrote: > From: Wen Congyang > > Signed-off-by: Wen Congyang > Signed-off-by: Changlong Xie > Signed-off-by: Wang WeiWei > --- > block.c | 17 + > 1 file changed, 17 insertions(+) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v24 02/12] Backup: clear all bitmap when doing block checkpoint
On Wed, Jul 27, 2016 at 03:01:43PM +0800, Changlong Xie wrote: > From: Wen Congyang > > Signed-off-by: Wen Congyang > Signed-off-by: Changlong Xie > Signed-off-by: Wang WeiWei > Signed-off-by: zhanghailiang > Signed-off-by: Gonglei > --- > block/backup.c | 18 ++ > include/block/block_backup.h | 25 + > 2 files changed, 43 insertions(+) > create mode 100644 include/block/block_backup.h Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v24 03/12] Backup: export interfaces for extra serialization
On Wed, Jul 27, 2016 at 03:01:44PM +0800, Changlong Xie wrote:
> Normal backup(sync='none') workflow:
> step 1. NBD peformance I/O write from client to server
>qcow2_co_writev
> bdrv_co_writev
> ...
>bdrv_aligned_pwritev
> notifier_with_return_list_notify -> backup_do_cow
> bdrv_driver_pwritev // write new contents
>
> step 2. drive-backup sync=none
>backup_do_cow
>{
> wait_for_overlapping_requests
> cow_request_begin
> for(; start < end; start++) {
> bdrv_co_readv_no_serialising //read old contents from Secondary
> disk
> bdrv_co_writev // write old contents to hidden-disk
> }
> cow_request_end
>}
>
> step 3. Then roll back to "step 1" to write new contents to Secondary disk.
>
> And for replication, we must make sure that we only read the old contents from
> Secondary disk in order to keep contents consistent.
>
> 1) Replication workflow of Secondary
> virtio-blk
> ^
> ---> 1 NBD |
>|| server 3 replication
>||^^
>||| backing backing |
>|| Secondary disk 6< hidden-disk 5 < active-disk 4
>||| ^
>||'-'
>|| drive-backup sync=none 2
>
> Hence, we need these interfaces to implement coarse-grained serialization
> between
> COW of Secondary disk and the read operation of replication.
>
> Example codes about how to use them:
>
> *#include "block/block_backup.h"
>
> static coroutine_fn int xxx_co_readv()
> {
> CowRequest req;
> BlockJob *job = secondary_disk->bs->job;
>
> if (job) {
> backup_wait_for_overlapping_requests(job, start, end);
> backup_cow_request_begin(&req, job, start, end);
> ret = bdrv_co_readv();
> backup_cow_request_end(&req);
> goto out;
> }
> ret = bdrv_co_readv();
> out:
> return ret;
> }
>
> Signed-off-by: Changlong Xie
> Signed-off-by: Wen Congyang
> Signed-off-by: Wang WeiWei
> ---
> block/backup.c | 41 ++---
> include/block/block_backup.h | 14 ++
> 2 files changed, 48 insertions(+), 7 deletions(-)
Reviewed-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v24 05/12] docs: block replication's description
On Wed, Jul 27, 2016 at 03:01:46PM +0800, Changlong Xie wrote: > From: Wen Congyang > > Signed-off-by: Wen Congyang > Signed-off-by: Changlong Xie > Signed-off-by: Wang WeiWei > Signed-off-by: zhanghailiang > Signed-off-by: Gonglei > --- > docs/block-replication.txt | 239 > + > 1 file changed, 239 insertions(+) > create mode 100644 docs/block-replication.txt Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.7 v2 0/2] Don't allow burst limits to be lower than the normal limits
On Thu, Jul 28, 2016 at 11:08:11AM +0300, Alberto Garcia wrote: > Hello, > > Gu Nini found this problem and reported it in > > https://bugzilla.redhat.com/show_bug.cgi?id=1355665 > > When setting the throttling configuration, the burst limits can be > lower than the normal limits. This does not making any sense and > behaves oddly, so let's forbid it. > > Berto > > v2: > - Simplify error message [Eric] > > Alberto Garcia (2): > throttle: Don't allow burst limits to be lower than the normal limits > throttle: Test burst limits lower than the normal limits > > tests/test-throttle.c | 8 > util/throttle.c | 5 + > 2 files changed, 13 insertions(+) > > -- > 2.8.1 > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v4] block/gluster: add support to choose libgfapi logfile
Prasanna Kumar Kalever writes:
> currently all the libgfapi logs defaults to '/dev/stderr' as it was hardcoded
> in a call to glfs logging api. When the debug level is chosen to DEBUG/TRACE,
> gfapi logs will be huge and fill/overflow the console view.
>
> This patch provides a commandline option to mention log file path which helps
> in logging to the specified file and also help in persisting the gfapi logs.
>
> Usage:
> -
> *URI Style:
> -
> -drive file=gluster://hostname/volname/image.qcow2,file.debug=9,\
> file.logfile=/var/log/qemu/qemu-gfapi.log
>
> *JSON Style:
> --
> 'json:{
>"driver":"qcow2",
>"file":{
> "driver":"gluster",
> "volume":"volname",
> "path":"image.qcow2",
> "debug":"9",
> "logfile":"/var/log/qemu/qemu-gfapi.log",
> "server":[
> {
> "type":"tcp",
> "host":"1.2.3.4",
> "port":24007
> },
> {
> "type":"unix",
> "socket":"/var/run/glusterd.socket"
> }
> ]
>}
> }'
>
> Signed-off-by: Prasanna Kumar Kalever
> ---
> v4: address review comments from Eric Blake on v3.
> v3: rebased on master, which is now QMP compatible.
> v2: address comments from Jeff Cody, thanks Jeff!
> v1: initial patch
> ---
> block/gluster.c | 42 ++
> qapi/block-core.json | 5 -
> 2 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 01b479f..e7bd13c 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -30,6 +30,8 @@
> #define GLUSTER_DEFAULT_PORT24007
> #define GLUSTER_DEBUG_DEFAULT 4
> #define GLUSTER_DEBUG_MAX 9
> +#define GLUSTER_OPT_LOGFILE "logfile"
> +#define GLUSTER_LOGFILE_DEFAULT "-" /* handled in libgfapi as
> /dev/stderr */
>
> #define GERR_INDEX_HINT "hint: check in 'server' array index '%d'\n"
>
> @@ -44,6 +46,7 @@ typedef struct GlusterAIOCB {
> typedef struct BDRVGlusterState {
> struct glfs *glfs;
> struct glfs_fd *fd;
> +char *logfile;
> bool supports_seek_data;
> int debug_level;
> } BDRVGlusterState;
Outside this patch's scope, but have you considered embedding
BlockdevOptionsGluster in BDRVGlusterState instead of selectively
duplicating it there? Mind, I'm not claiming it would be an
improvement, I just wonder whether it would be.
> @@ -73,6 +76,11 @@ static QemuOptsList qemu_gluster_create_opts = {
> .type = QEMU_OPT_NUMBER,
> .help = "Gluster log level, valid range is 0-9",
> },
> +{
> +.name = GLUSTER_OPT_LOGFILE,
> +.type = QEMU_OPT_STRING,
> +.help = "Logfile path of libgfapi",
> +},
> { /* end of list */ }
> }
> };
> @@ -91,6 +99,11 @@ static QemuOptsList runtime_opts = {
> .type = QEMU_OPT_NUMBER,
> .help = "Gluster log level, valid range is 0-9",
> },
> +{
> +.name = GLUSTER_OPT_LOGFILE,
> +.type = QEMU_OPT_STRING,
> +.help = "Logfile path of libgfapi",
> +},
> { /* end of list */ }
> },
> };
> @@ -341,7 +354,7 @@ static struct glfs
> *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
> }
> }
>
> -ret = glfs_set_logging(glfs, "-", gconf->debug_level);
> +ret = glfs_set_logging(glfs, gconf->logfile, gconf->debug_level);
> if (ret < 0) {
> goto out;
> }
> @@ -576,7 +589,9 @@ static struct glfs
> *qemu_gluster_init(BlockdevOptionsGluster *gconf,
> if (ret < 0) {
> error_setg(errp, "invalid URI");
> error_append_hint(errp, "Usage: file=gluster[+transport]://"
> -
> "[host[:port]]/volume/path[?socket=...]\n");
> +"[host[:port]]volume/path[?socket=...]"
> +"[,file.debug=N]"
> +"[,file.logfile=/path/filename.log]\n");
> errno = -ret;
> return NULL;
> }
> @@ -586,7 +601,9 @@ static struct glfs
> *qemu_gluster_init(BlockdevOptionsGluster *gconf,
> error_append_hint(errp, "Usage: "
> "-drive driver=qcow2,file.driver=gluster,"
> "file.volume=testvol,file.path=/path/a.qcow2"
> - "[,file.debug=9],file.server.0.type=tcp,"
> + "[,file.debug=9]"
> + "[,file.logfile=/path/filename.log],"
> + "file.server.0.type=tcp,"
> "file.server.0.host=1.2.3.4,"
> "file.server.0.p
Re: [Qemu-devel] [PULL v2 00/25] Misc QEMU fixes for 2016-08-02
On 3 August 2016 at 18:04, Paolo Bonzini wrote: > The following changes since commit cc0100f464c94bf80ad36cd432f4a1ed58126b60: > > MAINTAINERS: Update the Xilinx maintainers (2016-08-01 15:31:32 +0100) > > are available in the git repository at: > > git://github.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to e911765cbb9e9ddf5d952c88bb52180a62c6cea0: > > util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset' > (2016-08-03 18:44:57 +0200) > > > * xsetbv fix (x86 targets TCG) > * remove unused functions > * qht segfault and memory leak fixes > * NBD fixes > * Fix for non-power-of-2 discard granularity > * Memory hotplug fixes > * Migration regressions > * IOAPIC fixes and (disabled by default) EOI register support > * Various other small fixes > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v4 1/2] Interface for grant copy operation in libs.
On 08/04/2016 11:38 AM, Wei Liu wrote:
The code looks ok. I have two minor suggestions below.
I would suggest changing the subject line to:
libs/gnttab: introduce grant copy interface
On Tue, Aug 02, 2016 at 04:06:29PM +0200, Paulina Szubarczyk wrote:
In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
system call is invoked. In mini-os the operation is yet not
implemented. For the OSs that does not implement gnttab the
call of the grant copy operation causes abort.
Signed-off-by: Paulina Szubarczyk
---
Changes since v3:
- revert to cast from xengnttab_grant_copy_segment_t
to ioctl_gntdev_grant_copy.
- added compile-time check to compare the libs
xengnttab_grant_copy_segment_t with the ioctl structure.
The patch relies on Wei patch introducing XENGNTTAB_BUILD_BUG_ON
in libs/gnttab.
I should resubmit that one soon.
---
[...]
+rc = ioctl(fd, IOCTL_GNTDEV_GRANT_COPY, ©);
+if (rc)
+{
+GTERROR(xgt->logger, "ioctl GRANT COPY failed %d ", errno);
+}
Normally for a single statement you don't need {} around it.
No need to resubmit just because of this patch. I can handle the subject
line change, fix up the style issue and change the comment according to
David's suggestion while committing if you don't object to any of them.
Ok, thank you.
I won't commit this patch right away though. I will wait until the QEMU
patch is acked because I would avoid committing things that have no
users.
If you end up submitting another version you can make those changes
yourself.
Wei.
Paulina
[Qemu-devel] [PATCH] tests: Rename qtests which have names ending "error"
We have three qtest tests which have test names ending with "error".
This is awkward because the output of verbose test runs looks like
/crypto/task/error: OK
/crypto/task/thread_error: OK
which gives false positives if you are grepping build logs for
errors by looking for "error:". Since there are only three tests
with this problem, just rename them all to 'failure' instead.
Signed-off-by: Peter Maydell
---
Per discussion on IRC yesterday. I might throw this one into 2.7,
I dunno.
In particular the grep rune I run over build logs for merges
looks for "error:" among other things, so these false positives
are irritating.
---
tests/test-io-task.c | 8
tests/test-qmp-commands.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/test-io-task.c b/tests/test-io-task.c
index a36cb82..e091c12 100644
--- a/tests/test-io-task.c
+++ b/tests/test-io-task.c
@@ -110,7 +110,7 @@ static void test_task_data_free(void)
}
-static void test_task_error(void)
+static void test_task_failure(void)
{
QIOTask *task;
Object *obj = object_new(TYPE_DUMMY);
@@ -214,7 +214,7 @@ static void test_task_thread_complete(void)
}
-static void test_task_thread_error(void)
+static void test_task_thread_failure(void)
{
QIOTask *task;
Object *obj = object_new(TYPE_DUMMY);
@@ -262,8 +262,8 @@ int main(int argc, char **argv)
type_register_static(&dummy_info);
g_test_add_func("/crypto/task/complete", test_task_complete);
g_test_add_func("/crypto/task/datafree", test_task_data_free);
-g_test_add_func("/crypto/task/error", test_task_error);
+g_test_add_func("/crypto/task/failure", test_task_failure);
g_test_add_func("/crypto/task/thread_complete", test_task_thread_complete);
-g_test_add_func("/crypto/task/thread_error", test_task_thread_error);
+g_test_add_func("/crypto/task/thread_failure", test_task_thread_failure);
return g_test_run();
}
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 5af1a46..261fd9e 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -103,7 +103,7 @@ static void test_dispatch_cmd(void)
}
/* test commands that return an error due to invalid parameters */
-static void test_dispatch_cmd_error(void)
+static void test_dispatch_cmd_failure(void)
{
QDict *req = qdict_new();
QObject *resp;
@@ -253,7 +253,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
g_test_add_func("/0.15/dispatch_cmd", test_dispatch_cmd);
-g_test_add_func("/0.15/dispatch_cmd_error", test_dispatch_cmd_error);
+g_test_add_func("/0.15/dispatch_cmd_failure", test_dispatch_cmd_failure);
g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
--
1.9.1
Re: [Qemu-devel] [PULL 1/2] spapr: Error out when CPU hotplug is attempted on older pseries machines
On Wed, Aug 03, 2016 at 03:25:50PM +1000, David Gibson wrote:
> From: Bharata B Rao
>
> CPU hotplug and coldplug aren't supported prior to pseries-2.7. Further,
> earlier machine types don't use CPU core objects at all. These mean that
> query-hotpluggable-cpus and coldplug on older pseries machines will crash
> QEMU. It also means that hotpluggable_cpus flag in query-machines will
> be incorrectly set to true for pseries < 2.7, since it is based on the
> presence of the query_hotpluggable_cpus hook.
>
> - Don't assign the query_hotpluggable_cpus hook for pseries < 2.7
> - query_hotpluggable_cpus should therefore never be called on pseries <
> 2.7, so add an assert
> - spapr_core_pre_plug() should fail hot/cold plug attempts for pseries <
> 2.7, since core objects are never used there
> - spapr_core_plug() should therefore never be called for pseries < 2.7, so
> add an assert.
>
> Signed-off-by: Bharata B Rao
> [dwg: Change from query_hotpluggable_cpus returning NULL for pseries < 2.7
> to not being called at all, reword commit message for accuracy]
> Signed-off-by: David Gibson
> ---
> hw/ppc/spapr.c | 7 ++-
> hw/ppc/spapr_cpu_core.c | 19 ++-
> 2 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index fbbd051..bce2371 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2376,8 +2376,11 @@ static HotpluggableCPUList
> *spapr_query_hotpluggable_cpus(MachineState *machine)
> int i;
> HotpluggableCPUList *head = NULL;
> sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> int spapr_max_cores = max_cpus / smp_threads;
>
> +g_assert(smc->dr_cpu_enabled);
> +
> for (i = 0; i < spapr_max_cores; i++) {
> HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> @@ -2432,7 +2435,9 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> hc->plug = spapr_machine_device_plug;
> hc->unplug = spapr_machine_device_unplug;
> mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
> -mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> +if (smc->dr_cpu_enabled) {
> +mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> +}
>
> smc->dr_lmb_enabled = true;
> smc->dr_cpu_enabled = true;
Unfortunately smc->dr_cpu_enabled is always false when you set
mc->query_hotpluggable_cpus and will be set to true immediately
afterwards as seen in above hunk.
This leads to query-hotpluggable-cpus being unavailable for all
machine type versions. Your first version of setting
mc->query_hotpluggable_cpus to NULL explicitly for 2.6 and downwards
was correct.
Regards,
Bharata.
[Qemu-devel] [Bug 1490611] Re: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the result file, which Microsoft Azure rejects as invalid
Uploaded to Xenial, thanks. Am I right in thinking that the new option force_size is required to be used with the patch to fix the problem? It's probably worth mentioning this in the Test Case in the bug description; otherwise it will appear to testers that the fix doesn't work. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1490611 Title: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the result file, which Microsoft Azure rejects as invalid Status in QEMU: Fix Released Status in qemu package in Ubuntu: Fix Released Status in qemu source package in Xenial: In Progress Bug description: [Impact] * Starting with a raw disk image, using "qemu-img convert" to convert from raw to VHD results in the output VHD file's virtual size being aligned to the nearest 516096 bytes (16 heads x 63 sectors per head x 512 bytes per sector), instead of preserving the input file's size as the output VHD's virtual disk size. * Microsoft Azure requires that disk images (VHDs) submitted for upload have virtual sizes aligned to a megabyte boundary. (Ex. 4096MB, 4097MB, 4098MB, etc. are OK, 4096.5MB is rejected with an error.) This is reflected in Microsoft's documentation: https://azure.microsoft.com /en-us/documentation/articles/virtual-machines-linux-create-upload- vhd-generic/ * The fix for this bug is a backport from upstream. http://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb9245c2610932d33ce14 [Test Case] * This is reproducible with the following set of commands (including the Azure command line tools from https://github.com/Azure/azure- xplat-cli). For the following example, I used qemu version 2.2.1: $ dd if=/dev/zero of=source-disk.img bs=1M count=4096 $ stat source-disk.img File: ‘source-disk.img’ Size: 4294967296 Blocks: 798656 IO Block: 4096 regular file Device: fc01h/64513dInode: 13247963Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ smkent) Gid: ( 1000/ smkent) Access: 2015-08-18 09:48:02.613988480 -0700 Modify: 2015-08-18 09:48:02.825985646 -0700 Change: 2015-08-18 09:48:02.825985646 -0700 Birth: - $ qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img dest-disk.vhd $ stat dest-disk.vhd File: ‘dest-disk.vhd’ Size: 4296499712 Blocks: 535216 IO Block: 4096 regular file Device: fc01h/64513dInode: 13247964Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ smkent) Gid: ( 1000/ smkent) Access: 2015-08-18 09:50:22.252077624 -0700 Modify: 2015-08-18 09:49:24.424868868 -0700 Change: 2015-08-18 09:49:24.424868868 -0700 Birth: - $ azure vm image create testimage1 dest-disk.vhd -o linux -l "West US" info:Executing command vm image create + Retrieving storage accounts info:VHD size : 4097 MB info:Uploading 4195800.5 KB Requested:100.0% Completed:100.0% Running: 0 Time: 1m 0s Speed: 6744 KB/s info:https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd was uploaded successfully error: The VHD https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd has an unsupported virtual size of 4296499200 bytes. The size must be a whole number (in MBs). info:Error information has been recorded to /home/smkent/.azure/azure.err error: vm image create command failed * A fixed qemu-img will not result in an error during azure image creation. [Regression Potential] * The upstream fix introduces a qemu-img option (-o force_size) which is unset by default. The regression potential is very low, as a result. ... I also ran the above commands using qemu 2.4.0, which resulted in the same error as the conversion behavior is the same. However, qemu 2.1.1 and earlier (including qemu 2.0.0 installed by Ubuntu 14.04) does not pad the virtual disk size during conversion. Using qemu-img convert from qemu versions <=2.1.1 results in a VHD that is exactly the size of the raw input file plus 512 bytes (for the VHD footer). Those qemu versions do not attempt to realign the disk. As a result, Azure accepts VHD files created using those versions of qemu-img convert for upload. Is there a reason why newer qemu realigns the converted VHD file? It would be useful if an option were added to disable this feature, as current versions of qemu cannot be used to create VHD files for Azure using Microsoft's official instructions. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1490611/+subscriptions
Re: [Qemu-devel] [PATCH v2] net: check fragment length during fragmentation
Reviewed-by: Dmitry Fleytman On Thu, Aug 4, 2016 at 12:30 AM, P J P wrote: > From: Prasad J Pandit > > Network transport abstraction layer supports packet fragmentation. > While fragmenting a packet, it checks for more fragments from > packet length and current fragment length. It is susceptible > to an infinite loop, if the current fragment length is zero. > Add check to avoid it. > > Reported-by: Li Qiang > Signed-off-by: Prasad J Pandit > --- > hw/net/net_tx_pkt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Updated as per > -> https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg00751.html > > diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c > index efd43b4..53dfaa2 100644 > --- a/hw/net/net_tx_pkt.c > +++ b/hw/net/net_tx_pkt.c > @@ -590,7 +590,7 @@ static bool net_tx_pkt_do_sw_fragmentation(struct > NetTxPkt *pkt, > > fragment_offset += fragment_len; > > -} while (more_frags); > +} while (fragment_len && more_frags); > > return true; > } > -- > 2.5.5 > >
[Qemu-devel] [PATCH v2 for-2.7] Update ancient copyright string in -version output
Currently the -version command line argument prints a string ending
with "Copyright (c) 2003-2008 Fabrice Bellard". This is now some
eight years out of date; abstract it out of the several places that
print the string and update it to:
Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers
to reflect the work by all the QEMU Project contributors over the
last decade.
Signed-off-by: Peter Maydell
Acked-by: Stefan Hajnoczi
---
The aim here is to (1) update the dates and (2) acknowledge
the work of all our contributors. I'm open to bikeshedding
on the exact wording (or on which header file we should
put the #define in...)
I only pulled out the copyright string proper into the #define
because a GUI About box is going to want just that, with no
leading ',' or trailing newline.
Fabrice: I have cc'd you since this is proposing an update
to your copyright info.
v1->v2 changes: add the qemu-img.c string.
---
bsd-user/main.c | 3 ++-
include/qemu-common.h | 4
linux-user/main.c | 2 +-
qemu-img.c| 2 +-
vl.c | 3 ++-
5 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index bbba43f..b4a0a00 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -667,7 +667,8 @@ void cpu_loop(CPUSPARCState *env)
static void usage(void)
{
-printf("qemu-" TARGET_NAME " version " QEMU_VERSION ", Copyright (c)
2003-2008 Fabrice Bellard\n"
+printf("qemu-" TARGET_NAME " version " QEMU_VERSION QEMU_PKGVERSION
+ ", " QEMU_COPYRIGHT "\n"
"usage: qemu-" TARGET_NAME " [options] program [arguments...]\n"
"BSD CPU emulator (compiled for %s emulation)\n"
"\n"
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 1f2cb94..9e8b0bd 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -18,6 +18,10 @@
#include "qemu/option.h"
+/* Copyright string for -version arguments, About dialogs, etc */
+#define QEMU_COPYRIGHT "Copyright (c) 2003-2016 " \
+"Fabrice Bellard and the QEMU Project developers"
+
/* main function, renamed */
#if defined(CONFIG_COCOA)
int qemu_main(int argc, char **argv, char **envp);
diff --git a/linux-user/main.c b/linux-user/main.c
index 462e820..f2f4d2f 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4000,7 +4000,7 @@ static void handle_arg_strace(const char *arg)
static void handle_arg_version(const char *arg)
{
printf("qemu-" TARGET_NAME " version " QEMU_VERSION QEMU_PKGVERSION
- ", Copyright (c) 2003-2008 Fabrice Bellard\n");
+ ", " QEMU_COPYRIGHT "\n");
exit(EXIT_SUCCESS);
}
diff --git a/qemu-img.c b/qemu-img.c
index 2e40e1f..def9f96 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -44,7 +44,7 @@
#include
#define QEMU_IMG_VERSION "qemu-img version " QEMU_VERSION QEMU_PKGVERSION \
- ", Copyright (c) 2004-2008 Fabrice Bellard\n"
+ ", " QEMU_COPYRIGHT "\n"
typedef struct img_cmd_t {
const char *name;
diff --git a/vl.c b/vl.c
index e7c2c62..57f34d0 100644
--- a/vl.c
+++ b/vl.c
@@ -1914,7 +1914,8 @@ static void main_loop(void)
static void version(void)
{
-printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright
(c) 2003-2008 Fabrice Bellard\n");
+printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", "
+ QEMU_COPYRIGHT "\n");
}
static void help(int exitcode)
--
1.9.1
Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
Hi Xin,
Thanks for your feedback! Please see my embedded reply.
Regards,
-Gonglei
> -Original Message-
> From: Zeng, Xin [mailto:xin.z...@intel.com]
> Sent: Thursday, August 04, 2016 3:48 PM
> To: Gonglei (Arei); qemu-devel@nongnu.org; virtio-...@lists.oasis-open.org
> Cc: Huangpeng (Peter); Luonengjun; m...@redhat.com;
> cornelia.h...@de.ibm.com; stefa...@redhat.com;
> denglin...@chinamobile.com; Jani Kokkonen; ola.liljed...@arm.com;
> varun.se...@freescale.com; Keating, Brian A; Ma, Liang J; Griffin, John;
> Hanweidong (Randy); Huangweidong (C)
> Subject: RE: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> specification
>
> On Monday, August 01, 2016 5:20 PM, Gonglei Wrote:
> > -Original Message-
> > From: Gonglei [mailto:arei.gong...@huawei.com]
> > Sent: Monday, August 1, 2016 5:20 PM
> > To: qemu-devel@nongnu.org; virtio-...@lists.oasis-open.org
> > Cc: peter.huangp...@huawei.com; luoneng...@huawei.com;
> > m...@redhat.com; cornelia.h...@de.ibm.com; stefa...@redhat.com;
> > denglin...@chinamobile.com; jani.kokko...@huawei.com;
> > ola.liljed...@arm.com; varun.se...@freescale.com; Zeng, Xin
> > ; Keating, Brian A ; Ma,
> > Liang J ; Griffin, John ;
> > hanweid...@huawei.com; weidong.hu...@huawei.com; Gonglei
> >
> > Subject: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > specification
> >
> > The virtio crypto device is a virtual crypto device (ie. hardware
> > crypto accelerator card). The virtio crypto device can provide
> > five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
> >
> > In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> >
> > Signed-off-by: Gonglei
> > CC: Michael S. Tsirkin
> > CC: Cornelia Huck
> > CC: Stefan Hajnoczi
> > CC: Lingli Deng
> > CC: Jani Kokkonen
> > CC: Ola Liljedahl
> > CC: Varun Sethi
> > CC: Zeng Xin
> > CC: Keating Brian
> > CC: Ma Liang J
> > CC: Griffin John
> > CC: Hanweidong
> > ---
> > content.tex | 2 +
> > virtio-crypto.tex | 793
> > ++
> > 2 files changed, 795 insertions(+)
> > create mode 100644 virtio-crypto.tex
> >
> > diff --git a/content.tex b/content.tex
> > index 4b45678..ab75f78 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len},
> \field{residual},
> > \field{status_qualifier}, \field{status}, \field{response} and
> > \field{sense} fields.
> >
> > +\input{virtio-crypto.tex}
> > +
> > \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >
> > Currently there are three device-independent feature bits defined:
> > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > new file mode 100644
> > index 000..9465de3
> > --- /dev/null
> > +++ b/virtio-crypto.tex
> > @@ -0,0 +1,793 @@
> > +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> > +
> > +The virtio crypto device is a virtual crypto device (ie. hardware
> > +crypto accelerator card). The encryption and decryption requests of
> > +are placed in the data queue, and handled by the real hardware crypto
> > +accelerators finally. The second queue is the control queue, which
> > +is used to create or destroy session for symmetric algorithms, and
> > +to control some advanced features in the future. The virtio crypto
> > +device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
> > +KDF, ASYM, PRIMITIVE.
> > +
> > +\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID}
> > +
> > +20
> > +
> > +\subsection{Virtqueues}\label{sec:Device Types / Crypto Device /
> > Virtqueues}
> > +
> > +\begin{description}
> > +\item[0] dataq1
> > +\item[\ldots]
> > +\item[N-1] dataqN
> > +\item[N] controlq
> > +\end{description}
> > +
> > +N is set by \field{max_dataqueues} (\field{max_dataqueues} >= 1).
> > +
> > +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> > bits}
> > + None currently defined
> > +
> > +\subsection{Device configuration layout}\label{sec:Device Types / Crypto
> > Device / Device configuration layout}
> > +
> > +Thirteen driver-read-only configuration fields are currently defined.
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_config {
> > +le32 version;
> > +le16 status;
> > +le16 max_dataqueues;
> > +le32 crypto_services;
> > +/* detailed algorithms mask */
> > +le32 cipher_algo_l;
> > +le32 cipher_algo_h;
> > +le32 hash_algo;
> > +le32 mac_algo_l;
> > +le32 mac_algo_h;
> > +le32 asym_algo;
> > +le32 kdf_algo;
> > +le32 aead_algo;
> > +le32 primitive_algo;
> > +};
> > +\end{lstlisting}
> > +
> > +The first driver-read-only field, \field{version} specifies the virtio
> > crypto's
> > +version, which is reserved for back-compatibility in future.It's currently
> > +defined for the version field:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_VERSION_1 (1)
>
> Suggest to remove this macro,
> Do you think a version which
Re: [Qemu-devel] [PATCH] tests: Rename qtests which have names ending "error"
On Thu, Aug 04, 2016 at 11:39:38AM +0100, Peter Maydell wrote:
> We have three qtest tests which have test names ending with "error".
> This is awkward because the output of verbose test runs looks like
> /crypto/task/error: OK
> /crypto/task/thread_error: OK
>
> which gives false positives if you are grepping build logs for
> errors by looking for "error:". Since there are only three tests
> with this problem, just rename them all to 'failure' instead.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Daniel P. Berrange
> ---
> Per discussion on IRC yesterday. I might throw this one into 2.7,
> I dunno.
It has no functional change, so low risk & thus fine to put in 2.7 IMHO
>
> In particular the grep rune I run over build logs for merges
> looks for "error:" among other things, so these false positives
> are irritating.
> ---
> tests/test-io-task.c | 8
> tests/test-qmp-commands.c | 4 ++--
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tests/test-io-task.c b/tests/test-io-task.c
> index a36cb82..e091c12 100644
> --- a/tests/test-io-task.c
> +++ b/tests/test-io-task.c
> @@ -110,7 +110,7 @@ static void test_task_data_free(void)
> }
>
>
> -static void test_task_error(void)
> +static void test_task_failure(void)
> {
> QIOTask *task;
> Object *obj = object_new(TYPE_DUMMY);
> @@ -214,7 +214,7 @@ static void test_task_thread_complete(void)
> }
>
>
> -static void test_task_thread_error(void)
> +static void test_task_thread_failure(void)
> {
> QIOTask *task;
> Object *obj = object_new(TYPE_DUMMY);
> @@ -262,8 +262,8 @@ int main(int argc, char **argv)
> type_register_static(&dummy_info);
> g_test_add_func("/crypto/task/complete", test_task_complete);
> g_test_add_func("/crypto/task/datafree", test_task_data_free);
> -g_test_add_func("/crypto/task/error", test_task_error);
> +g_test_add_func("/crypto/task/failure", test_task_failure);
> g_test_add_func("/crypto/task/thread_complete",
> test_task_thread_complete);
> -g_test_add_func("/crypto/task/thread_error", test_task_thread_error);
> +g_test_add_func("/crypto/task/thread_failure", test_task_thread_failure);
> return g_test_run();
> }
> diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
> index 5af1a46..261fd9e 100644
> --- a/tests/test-qmp-commands.c
> +++ b/tests/test-qmp-commands.c
> @@ -103,7 +103,7 @@ static void test_dispatch_cmd(void)
> }
>
> /* test commands that return an error due to invalid parameters */
> -static void test_dispatch_cmd_error(void)
> +static void test_dispatch_cmd_failure(void)
> {
> QDict *req = qdict_new();
> QObject *resp;
> @@ -253,7 +253,7 @@ int main(int argc, char **argv)
> g_test_init(&argc, &argv, NULL);
>
> g_test_add_func("/0.15/dispatch_cmd", test_dispatch_cmd);
> -g_test_add_func("/0.15/dispatch_cmd_error", test_dispatch_cmd_error);
> +g_test_add_func("/0.15/dispatch_cmd_failure", test_dispatch_cmd_failure);
> g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
> g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
> g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
> --
> 1.9.1
>
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v1 1/1] block/gluster: improve defense over string to int conversion
Cc: Eric, because there's a relation to QAPI even though the patch
doesn't touch the schema.
Prasanna Kumar Kalever writes:
> using atoi() for converting string to int may be error prone in case if
> string supplied in the argument is not a fold of numerical number,
>
> This is not a bug because in the existing code,
>
> static QemuOptsList runtime_tcp_opts = {
> .name = "gluster_tcp",
> .head = QTAILQ_HEAD_INITIALIZER(runtime_tcp_opts.head),
> .desc = {
> ...
> {
> .name = GLUSTER_OPT_PORT,
> .type = QEMU_OPT_NUMBER,
> .help = "port number ...",
> },
> ...
> };
>
> port type is QEMU_OPT_NUMBER, before we actually reaches atoi() port is
> already
> defended by parse_option_number()
Should we use QEMU_OPT_STRING here, for the same reasons we use 'str' in
the QAPI schema?
> However It is a good practice to use function like parse_uint_full()
> over atoi() to keep port self defended
>
> Signed-off-by: Prasanna Kumar Kalever
> ---
> block/gluster.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 01b479f..e2aa0f3 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -14,6 +14,7 @@
> #include "qapi/qmp/qerror.h"
> #include "qemu/uri.h"
> #include "qemu/error-report.h"
> +#include "qemu/cutils.h"
>
> #define GLUSTER_OPT_FILENAME"filename"
> #define GLUSTER_OPT_VOLUME "volume"
> @@ -318,6 +319,7 @@ static struct glfs
> *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
> int ret;
> int old_errno;
> GlusterServerList *server;
> +long long unsigned port;
unsigned long long, please.
>
> glfs = glfs_new(gconf->volume);
> if (!glfs) {
> @@ -330,10 +332,16 @@ static struct glfs
> *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
>
> GlusterTransport_lookup[server->value->type],
> server->value->u.q_unix.path, 0);
> } else {
> +if (parse_uint_full(server->value->u.tcp.port, &port, 0) < 0) {
Should we require decimal by passing 10 rather than 0? Port numbers are
commonly decimal. For what it's worth, JSON numbers are always decimal.
> +error_setg(errp, "can't convert port to a number: %s",
> + server->value->u.tcp.port);
Suggest "'%s' is not a valid port number".
> +errno = EINVAL;
> +goto out;
> +}
Missing: a range check. No need to get fancy, something like this might
do:
if (parse_uint_full(server->value->u.tcp.port, &port, 0) < 0
|| port > 65535) {
> ret = glfs_set_volfile_server(glfs,
>
> GlusterTransport_lookup[server->value->type],
> server->value->u.tcp.host,
> - atoi(server->value->u.tcp.port));
> + (int)port);
> }
>
> if (ret < 0) {
[Qemu-devel] [PATCH for-2.7] block/qdev: Let 'drive' property fall back to node name
If a qdev block device is created with an anonymous BlockBackend (i.e.
a node name rather than a BB name was given for the drive property),
qdev used to return an empty string when the property was read. This
patch fixes it to return the node name instead.
Signed-off-by: Kevin Wolf
---
hw/core/qdev-properties-system.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 2ba2504..25b24aa 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -126,7 +126,13 @@ static void release_drive(Object *obj, const char *name,
void *opaque)
static char *print_drive(void *ptr)
{
-return g_strdup(blk_name(ptr));
+const char *name;
+
+name = blk_name(ptr);
+if (!*name) {
+name = bdrv_get_node_name(blk_bs(ptr));
+}
+return g_strdup(name);
}
static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
--
1.8.3.1
Re: [Qemu-devel] [PATCH] trace: add syslog tracing backend
On Tue, Aug 02, 2016 at 04:06:42PM +0100, Paul Durrant wrote:
> This patch adds a tracing backend which sends output using syslog().
> The syslog backend is limited to POSIX compliant systems.
>
> openlog() is called with facility set to LOG_DAEMON, with the LOG_PID
> option. Trace events are logged at level LOG_INFO.
>
> Signed-off-by: Paul Durrant
> Cc: Stefan Hajnoczi
> ---
> configure | 19
> scripts/tracetool/backend/syslog.py | 45
> +
> trace/control.c | 7 ++
> 3 files changed, 71 insertions(+)
> create mode 100644 scripts/tracetool/backend/syslog.py
Please add a section to docs/tracing.txt. It's worth mentioning the
caveats that Daniel Berrange pointed out in the documentation.
(I do believe that syslog tracing can be useful in some cases and it
will be merged.)
> +def generate_h_begin(events):
> +out('#include "trace/control.h"',
> +'#include ',
Please include system headers before user headers. This makes it clear
the user headers are not defining macros that will affect system
headers (which is sometimes necessary but should be done explicitly).
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH for-2.7 v2] block/qdev: Let 'drive' property fall back to node name
If a qdev block device is created with an anonymous BlockBackend (i.e.
a node name rather than a BB name was given for the drive property),
qdev used to return an empty string when the property was read. This
patch fixes it to return the node name instead.
Signed-off-by: Kevin Wolf
---
v2:
- Check whether the BB has even a BDS inserted. Fixes a segfault with
empty anonymous BlockBackends.
hw/core/qdev-properties-system.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 2ba2504..e55afe6 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -126,7 +126,16 @@ static void release_drive(Object *obj, const char *name,
void *opaque)
static char *print_drive(void *ptr)
{
-return g_strdup(blk_name(ptr));
+const char *name;
+
+name = blk_name(ptr);
+if (!*name) {
+BlockDriverState *bs = blk_bs(ptr);
+if (bs) {
+name = bdrv_get_node_name(bs);
+}
+}
+return g_strdup(name);
}
static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
--
1.8.3.1
Re: [Qemu-devel] [PATCH for-2.7] block: Accept any target node for transactional blockdev-backup
On Tue, Aug 02, 2016 at 07:22:04PM +0200, Kevin Wolf wrote: > Commit 0d978913 changed blockdev-backup to accept arbitrary node names > instead of device names (i.e. root nodes) for the backup target. > However, it forgot to make the same change in transactions and to update > the documentation. This patch fixes these omissions. > > Signed-off-by: Kevin Wolf > --- > blockdev.c | 8 > qapi/block-core.json | 2 +- > 2 files changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.7] block/qdev: Let 'drive' property fall back to node name
Am 04.08.2016 um 14:03 hat Kevin Wolf geschrieben:
> If a qdev block device is created with an anonymous BlockBackend (i.e.
> a node name rather than a BB name was given for the drive property),
> qdev used to return an empty string when the property was read. This
> patch fixes it to return the node name instead.
>
> Signed-off-by: Kevin Wolf
As always, the moment you send out a patch, you notice that it's
broken.
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 2ba2504..25b24aa 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -126,7 +126,13 @@ static void release_drive(Object *obj, const char *name,
> void *opaque)
>
> static char *print_drive(void *ptr)
> {
> -return g_strdup(blk_name(ptr));
> +const char *name;
> +
> +name = blk_name(ptr);
> +if (!*name) {
> +name = bdrv_get_node_name(blk_bs(ptr));
blk_bs(ptr) can be NULL. Self-NACK, v2 is coming.
> +}
> +return g_strdup(name);
> }
Kevin
Re: [Qemu-devel] [RFC PATCH v1 1/2] utils: Add helper to read arm MIDR_EL1 register
> > Third, it's probably a bad idea to call this function from generic code, so > > make it static and add the detection function from patch 2/2 already here. > > By making it static, it's also possible to define it only if CONFIG_LINUX > > is defined; the ThunderX detection will then return false if !CONFIG_LINUX. > > > > You mean to say, move contents of this patch to util/cutils.c and make it > static and define under __aarch64__ and CONFIG_LINUX?. I don't think util/cutils.c is the right file. It should be a new file, something like util/aarch64-cpuid.c. If CONFIG_LINUX is not defined, the ThunderX detection function should return zero. If __aarch64__ is not defined, the function should not be defined at all. Paolo
Re: [Qemu-devel] [PATCH] trace: add syslog tracing backend
> -Original Message-
> From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> Sent: 04 August 2016 13:04
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH] trace: add syslog tracing backend
>
> On Tue, Aug 02, 2016 at 04:06:42PM +0100, Paul Durrant wrote:
> > This patch adds a tracing backend which sends output using syslog().
> > The syslog backend is limited to POSIX compliant systems.
> >
> > openlog() is called with facility set to LOG_DAEMON, with the LOG_PID
> > option. Trace events are logged at level LOG_INFO.
> >
> > Signed-off-by: Paul Durrant
> > Cc: Stefan Hajnoczi
> > ---
> > configure | 19
> > scripts/tracetool/backend/syslog.py | 45
> +
> > trace/control.c | 7 ++
> > 3 files changed, 71 insertions(+)
> > create mode 100644 scripts/tracetool/backend/syslog.py
>
> Please add a section to docs/tracing.txt. It's worth mentioning the
> caveats that Daniel Berrange pointed out in the documentation.
Sure. Will do.
>
> (I do believe that syslog tracing can be useful in some cases and it
> will be merged.)
>
> > +def generate_h_begin(events):
> > +out('#include "trace/control.h"',
> > +'#include ',
>
> Please include system headers before user headers. This makes it clear
> the user headers are not defining macros that will affect system
> headers (which is sometimes necessary but should be done explicitly).
Ok.
Cheers,
Paul
Re: [Qemu-devel] [PATCH] virtio-blk: rename virtio_device_info to virtio_blk_info
On Wed, Aug 03, 2016 at 04:49:07PM +0800, Changlong Xie wrote: > The old one is confusing with @virtio_device_info in virtio.c, > so make it more appropriate. > > Signed-off-by: Changlong Xie > --- > hw/block/virtio-blk.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] docker: Be compatible with older docker
On 04/08/2016 04:33, Fam Zheng wrote: > On Wed, 08/03 13:42, Fam Zheng wrote: >> By not using "--format" with docker images command. >> >> The option is not available on RHEL 7 docker command. Use an awk >> matching command instead. >> >> Reported-by: Paolo Bonzini >> Signed-off-by: Fam Zheng >> --- >> tests/docker/Makefile.include | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) > > Paolo, does this work for you? Yes, it works. Thanks! Paolo
Re: [Qemu-devel] [PATCH v2 1/2] virtio-blk: Release s->rq queue at system_reset
On 03/08/2016 02:52, Fam Zheng wrote: >> > It's also (perhaps especially) because blk_drain() can consume them. Fam's >> > patch to do blk_drain() first would cause a double-free. > That "consume" part is what I don't understand. > > Shouldn't blk_drain() only process submitted requests (and further requests > they dequeue indirectly), while s->rq only contains failed requests. Nevermind, I was confused. virtio_blk_init_request doesn't store the requests in a list, unlike SCSI. Paolo
Re: [Qemu-devel] [PATCH 1/7] util: Add UUID API
On 03/08/2016 04:36, Fam Zheng wrote: > On Tue, 08/02 15:45, Paolo Bonzini wrote: >> >> >> - Original Message - >>> From: "Fam Zheng" >>> To: qemu-devel@nongnu.org >>> Cc: f...@redhat.com, berra...@redhat.com, pbonz...@redhat.com, >>> kw...@redhat.com, mre...@redhat.com, >>> mdr...@linux.vnet.ibm.com, arm...@redhat.com, s...@weilnetz.de, >>> qemu-bl...@nongnu.org >>> Sent: Tuesday, August 2, 2016 11:18:32 AM >>> Subject: [PATCH 1/7] util: Add UUID API >>> >>> A number of different places across the code base use CONFIG_UUID. Some >>> of them are soft dependency, some are not built if libuuid is not >>> available, some come with dummy fallback, some throws runtime error. >>> >>> It is hard to maintain, and hard to reason for users. >>> >>> Since UUID is a simple standard with only a small number of operations, >>> it is cleaner to have a central support in libqemuutil. This patch adds >>> qemu_uuid_* the functions so that all uuid users in the code base can >>> rely on. Except for qemu_uuid_generate which is new code, all other >>> functions are just copy from existing fallbacks from other files. >> >> How is g_random_* seeded? > > According to glib doc: > >> GLib changed the seeding algorithm for the pseudo-random number generator >> Mersenne Twister, as used by GRand. > > The urandom source is /dev/urandom (or time based if unavailable). That's great then. Paolo
Re: [Qemu-devel] [PATCH v3 0/2] Two virtio-blk fixes
On Thu, Aug 04, 2016 at 10:44:12AM +0800, Fam Zheng wrote: > v3: Patch 1: add comment as Laszlo suggested, and add his r-b too. :) > > Fam Zheng (2): > virtio-blk: Release s->rq queue at system_reset > virtio-blk: Remove stale comment about draining > > hw/block/virtio-blk.c | 13 + > 1 file changed, 9 insertions(+), 4 deletions(-) > > -- > 2.7.4 > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
[Qemu-devel] [RESEND PATCH V5 6/6] coroutine: reduce stack size to 64kB
evaluation with the recently introduced maximum stack usage monitoring revealed
that the actual used stack size was never above 4kB so allocating 1MB stack
for each coroutine is a lot of wasted memory. So reduce the stack size to
64kB which should still give enough head room. The guard page added
in qemu_alloc_stack will catch a potential stack overflow introduced
by this commit.
Signed-off-by: Peter Lieven
Reviewed-by: Eric Blake
---
include/qemu/coroutine_int.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index eac323a..f84d777 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,7 +28,7 @@
#include "qemu/queue.h"
#include "qemu/coroutine.h"
-#define COROUTINE_STACK_SIZE (1 << 20)
+#define COROUTINE_STACK_SIZE (1 << 16)
typedef enum {
COROUTINE_YIELD = 1,
--
1.9.1
[Qemu-devel] [RESEND PATCH V5 0/6] coroutine: mmap stack memory and stack size
I decided to split this from the rest of the Qemu RSS usage series as it contains the more or less non contentious patches. I omitted the MAP_GROWSDOWN flag in mmap as we are not 100% sure which side effects it has. I kept the guard page which is now nicely makes the stacks visible in smaps. The old version of the relevent patch lacked the MAP_FIXED flag in the second call to mmap. The last patch which reduces the stack size of coroutines to 64kB may be omitted if its found to risky. v4->v5: - Patch 1: check if _SC_THREAD_STACK_MIN is defined - Patch 1: guard against sysconf(_SC_THREAD_STACK_MIN) returning -1 [Eric] v3->v4: - Patch 1: add a static function to adjust the stack size [Richard] - Patch 1: round up the stack size to multiple of the pagesize. v2->v3: - Patch 1,6: adjusted commit message to mention the guard page [Markus] v1->v2: - Patch 1: added an architecture dependend guard page [Richard] - Patch 1: avoid stacks smaller than _SC_THREAD_STACK_MIN [Richard] - Patch 1: use mmap+mprotect instead of mmap+mmap [Richard] - Patch 5: u_int32_t -> uint32_t [Richard] - Patch 5: only available if stack grows down Peter Lieven (6): oslib-posix: add helpers for stack alloc and free coroutine: add a macro for the coroutine stack size coroutine-ucontext: use helper for allocating stack memory coroutine-sigaltstack: use helper for allocating stack memory oslib-posix: add a configure switch to debug stack usage coroutine: reduce stack size to 64kB configure| 19 ++ include/qemu/coroutine_int.h | 2 ++ include/sysemu/os-posix.h| 23 util/coroutine-sigaltstack.c | 7 ++-- util/coroutine-ucontext.c| 9 +++-- util/coroutine-win32.c | 2 +- util/oslib-posix.c | 83 7 files changed, 135 insertions(+), 10 deletions(-) -- 1.9.1
[Qemu-devel] [RESEND PATCH V5 3/6] coroutine-ucontext: use helper for allocating stack memory
Signed-off-by: Peter Lieven
Reviewed-by: Paolo Bonzini
Reviewed-by: Richard Henderson
---
util/coroutine-ucontext.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 31254ab..b7dea8c 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -82,7 +82,6 @@ static void coroutine_trampoline(int i0, int i1)
Coroutine *qemu_coroutine_new(void)
{
-const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
ucontext_t old_uc, uc;
sigjmp_buf old_env;
@@ -101,17 +100,17 @@ Coroutine *qemu_coroutine_new(void)
}
co = g_malloc0(sizeof(*co));
-co->stack = g_malloc(stack_size);
+co->stack = qemu_alloc_stack(COROUTINE_STACK_SIZE);
co->base.entry_arg = &old_env; /* stash away our jmp_buf */
uc.uc_link = &old_uc;
uc.uc_stack.ss_sp = co->stack;
-uc.uc_stack.ss_size = stack_size;
+uc.uc_stack.ss_size = COROUTINE_STACK_SIZE;
uc.uc_stack.ss_flags = 0;
#ifdef CONFIG_VALGRIND_H
co->valgrind_stack_id =
-VALGRIND_STACK_REGISTER(co->stack, co->stack + stack_size);
+VALGRIND_STACK_REGISTER(co->stack, co->stack + COROUTINE_STACK_SIZE);
#endif
arg.p = co;
@@ -149,7 +148,7 @@ void qemu_coroutine_delete(Coroutine *co_)
valgrind_stack_deregister(co);
#endif
-g_free(co->stack);
+qemu_free_stack(co->stack, COROUTINE_STACK_SIZE);
g_free(co);
}
--
1.9.1
[Qemu-devel] [RESEND PATCH V5 4/6] coroutine-sigaltstack: use helper for allocating stack memory
Signed-off-by: Peter Lieven
Reviewed-by: Paolo Bonzini
Reviewed-by: Richard Henderson
---
util/coroutine-sigaltstack.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index 9c2854c..ccf4861 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -143,7 +143,6 @@ static void coroutine_trampoline(int signal)
Coroutine *qemu_coroutine_new(void)
{
-const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
CoroutineThreadState *coTS;
struct sigaction sa;
@@ -164,7 +163,7 @@ Coroutine *qemu_coroutine_new(void)
*/
co = g_malloc0(sizeof(*co));
-co->stack = g_malloc(stack_size);
+co->stack = qemu_alloc_stack(COROUTINE_STACK_SIZE);
co->base.entry_arg = &old_env; /* stash away our jmp_buf */
coTS = coroutine_get_thread_state();
@@ -189,7 +188,7 @@ Coroutine *qemu_coroutine_new(void)
* Set the new stack.
*/
ss.ss_sp = co->stack;
-ss.ss_size = stack_size;
+ss.ss_size = COROUTINE_STACK_SIZE;
ss.ss_flags = 0;
if (sigaltstack(&ss, &oss) < 0) {
abort();
@@ -253,7 +252,7 @@ void qemu_coroutine_delete(Coroutine *co_)
{
CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
-g_free(co->stack);
+qemu_free_stack(co->stack, COROUTINE_STACK_SIZE);
g_free(co);
}
--
1.9.1
[Qemu-devel] [RESEND PATCH V5 5/6] oslib-posix: add a configure switch to debug stack usage
this adds a knob to track the maximum stack usage of stacks
created by qemu_alloc_stack.
Signed-off-by: Peter Lieven
Reviewed-by: Paolo Bonzini
---
configure | 19 +++
util/oslib-posix.c | 37 +
2 files changed, 56 insertions(+)
diff --git a/configure b/configure
index 5ada56d..a7ee2f3 100755
--- a/configure
+++ b/configure
@@ -296,6 +296,7 @@ libiscsi=""
libnfs=""
coroutine=""
coroutine_pool=""
+debug_stack_usage="no"
seccomp=""
glusterfs=""
glusterfs_xlator_opt="no"
@@ -1005,6 +1006,8 @@ for opt do
;;
--enable-coroutine-pool) coroutine_pool="yes"
;;
+ --enable-debug-stack-usage) debug_stack_usage="yes"
+ ;;
--disable-docs) docs="no"
;;
--enable-docs) docs="yes"
@@ -4302,6 +4305,17 @@ if test "$coroutine" = "gthread" -a "$coroutine_pool" =
"yes"; then
error_exit "'gthread' coroutine backend does not support pool (use
--disable-coroutine-pool)"
fi
+if test "$debug_stack_usage" = "yes"; then
+ if test "$cpu" = "ia64" -o "$cpu" = "hppa"; then
+error_exit "stack usage debugging is not supported for $cpu"
+ fi
+ if test "$coroutine_pool" = "yes"; then
+echo "WARN: disabling coroutine pool for stack usage debugging"
+coroutine_pool=no
+ fi
+fi
+
+
##
# check if we have open_by_handle_at
@@ -4879,6 +4893,7 @@ echo "QGA MSI support $guest_agent_msi"
echo "seccomp support $seccomp"
echo "coroutine backend $coroutine"
echo "coroutine pool$coroutine_pool"
+echo "debug stack usage $debug_stack_usage"
echo "GlusterFS support $glusterfs"
echo "Archipelago support $archipelago"
echo "gcov $gcov_tool"
@@ -5347,6 +5362,10 @@ else
echo "CONFIG_COROUTINE_POOL=0" >> $config_host_mak
fi
+if test "$debug_stack_usage" = "yes" ; then
+ echo "CONFIG_DEBUG_STACK_USAGE=y" >> $config_host_mak
+fi
+
if test "$open_by_handle_at" = "yes" ; then
echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
fi
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 2303ca6..e818d38 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -50,6 +50,10 @@
#include
+#ifdef CONFIG_DEBUG_STACK_USAGE
+#include "qemu/error-report.h"
+#endif
+
int qemu_get_thread_id(void)
{
#if defined(__linux__)
@@ -512,6 +516,9 @@ static size_t adjust_stack_size(size_t sz)
void *qemu_alloc_stack(size_t sz)
{
void *ptr, *guardpage;
+#ifdef CONFIG_DEBUG_STACK_USAGE
+void *ptr2;
+#endif
size_t pagesz = getpagesize();
sz = adjust_stack_size(sz);
@@ -535,11 +542,41 @@ void *qemu_alloc_stack(size_t sz)
abort();
}
+#ifdef CONFIG_DEBUG_STACK_USAGE
+for (ptr2 = ptr + pagesz; ptr2 < ptr + sz; ptr2 += sizeof(uint32_t)) {
+*(uint32_t *)ptr2 = 0xdeadbeaf;
+}
+#endif
+
return ptr;
}
+#ifdef CONFIG_DEBUG_STACK_USAGE
+static __thread unsigned int max_stack_usage;
+#endif
+
void qemu_free_stack(void *stack, size_t sz)
{
+#ifdef CONFIG_DEBUG_STACK_USAGE
+unsigned int usage;
+void *ptr;
+#endif
sz = adjust_stack_size(sz);
+
+#ifdef CONFIG_DEBUG_STACK_USAGE
+for (ptr = stack + getpagesize(); ptr < stack + sz;
+ ptr += sizeof(uint32_t)) {
+if (*(uint32_t *)ptr != 0xdeadbeaf) {
+break;
+}
+}
+usage = sz - (uintptr_t) (ptr - stack);
+if (usage > max_stack_usage) {
+error_report("thread %d max stack usage increased from %u to %u",
+ qemu_get_thread_id(), max_stack_usage, usage);
+max_stack_usage = usage;
+}
+#endif
+
munmap(stack, sz);
}
--
1.9.1
[Qemu-devel] [RESEND PATCH V5 1/6] oslib-posix: add helpers for stack alloc and free
the allocated stack will be adjusted to the minimum supported stack size
by the OS and rounded up to be a multiple of the system pagesize.
Additionally an architecture dependent guard page is added to the stack
to catch stack overflows.
Signed-off-by: Peter Lieven
Reviewed-by: Eric Blake
---
include/sysemu/os-posix.h | 23 +++
util/oslib-posix.c| 46 ++
2 files changed, 69 insertions(+)
diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 9c7dfdf..7630665 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -60,4 +60,27 @@ int qemu_utimens(const char *path, const qemu_timespec
*times);
bool is_daemonized(void);
+/**
+ * qemu_alloc_stack:
+ * @sz: size of required stack in bytes
+ *
+ * Allocate memory that can be used as a stack, for instance for
+ * coroutines. If the memory cannot be allocated, this function
+ * will abort (like g_malloc()).
+ *
+ * The allocated stack must be freed with qemu_free_stack().
+ *
+ * Returns: pointer to (the lowest address of) the stack memory.
+ */
+void *qemu_alloc_stack(size_t sz);
+
+/**
+ * qemu_free_stack:
+ * @stack: stack to free
+ * @sz: size of stack in bytes
+ *
+ * Free a stack allocated via qemu_alloc_stack().
+ */
+void qemu_free_stack(void *stack, size_t sz);
+
#endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e2e1d4d..2303ca6 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -497,3 +497,49 @@ pid_t qemu_fork(Error **errp)
}
return pid;
}
+
+static size_t adjust_stack_size(size_t sz)
+{
+#ifdef _SC_THREAD_STACK_MIN
+/* avoid stacks smaller than _SC_THREAD_STACK_MIN */
+sz = MAX(MAX(sysconf(_SC_THREAD_STACK_MIN), 0), sz);
+#endif
+/* adjust stack size to a multiple of the page size */
+sz = ROUND_UP(sz, getpagesize());
+return sz;
+}
+
+void *qemu_alloc_stack(size_t sz)
+{
+void *ptr, *guardpage;
+size_t pagesz = getpagesize();
+sz = adjust_stack_size(sz);
+
+ptr = mmap(NULL, sz, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+if (ptr == MAP_FAILED) {
+abort();
+}
+
+#if defined(HOST_IA64)
+/* separate register stack */
+guardpage = ptr + (((sz - pagesz) / 2) & ~pagesz);
+#elif defined(HOST_HPPA)
+/* stack grows up */
+guardpage = ptr + sz - pagesz;
+#else
+/* stack grows down */
+guardpage = ptr;
+#endif
+if (mprotect(guardpage, pagesz, PROT_NONE) != 0) {
+abort();
+}
+
+return ptr;
+}
+
+void qemu_free_stack(void *stack, size_t sz)
+{
+sz = adjust_stack_size(sz);
+munmap(stack, sz);
+}
--
1.9.1
Re: [Qemu-devel] [PATCH 1/7] util: Add UUID API
Hi On Thu, Aug 4, 2016 at 4:44 PM Paolo Bonzini wrote: > > > On 03/08/2016 04:36, Fam Zheng wrote: > > On Tue, 08/02 15:45, Paolo Bonzini wrote: > >> > >> > >> - Original Message - > >>> From: "Fam Zheng" > >>> To: qemu-devel@nongnu.org > >>> Cc: f...@redhat.com, berra...@redhat.com, pbonz...@redhat.com, > kw...@redhat.com, mre...@redhat.com, > >>> mdr...@linux.vnet.ibm.com, arm...@redhat.com, s...@weilnetz.de, > qemu-bl...@nongnu.org > >>> Sent: Tuesday, August 2, 2016 11:18:32 AM > >>> Subject: [PATCH 1/7] util: Add UUID API > >>> > >>> A number of different places across the code base use CONFIG_UUID. Some > >>> of them are soft dependency, some are not built if libuuid is not > >>> available, some come with dummy fallback, some throws runtime error. > For info, there has been glib UUID proposal for a while: https://bugzilla.gnome.org/show_bug.cgi?id=639078 Although quite ready, the discussion stopped because some dev believe g_uuid_string_random() is all you need. Anyway, if ever accepted, it would take another 5y or so to be acceptable for qemu :) -- Marc-André Lureau
[Qemu-devel] [RESEND PATCH V5 2/6] coroutine: add a macro for the coroutine stack size
Signed-off-by: Peter Lieven
Reviewed-by: Paolo Bonzini
Reviewed-by: Richard Henderson
---
include/qemu/coroutine_int.h | 2 ++
util/coroutine-sigaltstack.c | 2 +-
util/coroutine-ucontext.c| 2 +-
util/coroutine-win32.c | 2 +-
4 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index 42d6838..eac323a 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,6 +28,8 @@
#include "qemu/queue.h"
#include "qemu/coroutine.h"
+#define COROUTINE_STACK_SIZE (1 << 20)
+
typedef enum {
COROUTINE_YIELD = 1,
COROUTINE_TERMINATE = 2,
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index a7c3366..9c2854c 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -143,7 +143,7 @@ static void coroutine_trampoline(int signal)
Coroutine *qemu_coroutine_new(void)
{
-const size_t stack_size = 1 << 20;
+const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
CoroutineThreadState *coTS;
struct sigaction sa;
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 2bb7e10..31254ab 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -82,7 +82,7 @@ static void coroutine_trampoline(int i0, int i1)
Coroutine *qemu_coroutine_new(void)
{
-const size_t stack_size = 1 << 20;
+const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
ucontext_t old_uc, uc;
sigjmp_buf old_env;
diff --git a/util/coroutine-win32.c b/util/coroutine-win32.c
index 02e28e8..de6bd4f 100644
--- a/util/coroutine-win32.c
+++ b/util/coroutine-win32.c
@@ -71,7 +71,7 @@ static void CALLBACK coroutine_trampoline(void *co_)
Coroutine *qemu_coroutine_new(void)
{
-const size_t stack_size = 1 << 20;
+const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineWin32 *co;
co = g_malloc0(sizeof(*co));
--
1.9.1
[Qemu-devel] [PATCH v1 4/5] target-ppc: add vector bit permute doubleword instruction
Add vbpermd instruction from ISA 3.0.
Signed-off-by: Rajalakshmi Srinivasaraghavan
---
target-ppc/helper.h |1 +
target-ppc/int_helper.c | 20
target-ppc/translate/vmx-impl.c |1 +
target-ppc/translate/vmx-ops.c |1 +
4 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 6e6e7b3..d1d9418 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -335,6 +335,7 @@ DEF_HELPER_2(vpopcntb, void, avr, avr)
DEF_HELPER_2(vpopcnth, void, avr, avr)
DEF_HELPER_2(vpopcntw, void, avr, avr)
DEF_HELPER_2(vpopcntd, void, avr, avr)
+DEF_HELPER_3(vbpermd, void, avr, avr, avr)
DEF_HELPER_3(vbpermq, void, avr, avr, avr)
DEF_HELPER_2(vgbbd, void, avr, avr)
DEF_HELPER_3(vpmsumb, void, avr, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 09f02f8..d8ad56f 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1134,6 +1134,26 @@ void helper_vperm(CPUPPCState *env, ppc_avr_t *r,
ppc_avr_t *a, ppc_avr_t *b,
#define VBPERMQ_DW(index) (((index) & 0x40) == 0)
#endif
+void helper_vbpermd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+int i, j;
+uint64_t perm = 0;
+
+VECTOR_FOR_INORDER_I(i, u64) {
+perm = 0;
+VECTOR_FOR_INORDER_I(j, u16) {
+int index = VBPERMQ_INDEX(b, (i * 8) + j);
+if (index < 64) {
+uint64_t mask = (1ull << (63 - (index & 0x3F)));
+if (a->u64[VBPERMQ_DW(index)] & mask) {
+perm |= (0x80 >> j);
+}
+}
+r->u64[i] = perm;
+}
+}
+}
+
void helper_vbpermq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
{
int i;
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 2cf8c8f..5ddff58 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -754,6 +754,7 @@ GEN_VXFORM_DUAL(vclzw, PPC_NONE, PPC2_ALTIVEC_207, \
vpopcntw, PPC_NONE, PPC2_ALTIVEC_207)
GEN_VXFORM_DUAL(vclzd, PPC_NONE, PPC2_ALTIVEC_207, \
vpopcntd, PPC_NONE, PPC2_ALTIVEC_207)
+GEN_VXFORM(vbpermd, 6, 23);
GEN_VXFORM(vbpermq, 6, 21);
GEN_VXFORM_NOA(vgbbd, 6, 20);
GEN_VXFORM(vpmsumb, 4, 16)
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index 5b2826e..32bd533 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -261,6 +261,7 @@ GEN_VXFORM_DUAL(vclzh, vpopcnth, 1, 29, PPC_NONE,
PPC2_ALTIVEC_207),
GEN_VXFORM_DUAL(vclzw, vpopcntw, 1, 30, PPC_NONE, PPC2_ALTIVEC_207),
GEN_VXFORM_DUAL(vclzd, vpopcntd, 1, 31, PPC_NONE, PPC2_ALTIVEC_207),
+GEN_VXFORM_300(vbpermd, 6, 23),
GEN_VXFORM_207(vbpermq, 6, 21),
GEN_VXFORM_207(vgbbd, 6, 20),
GEN_VXFORM_207(vpmsumb, 4, 16),
--
1.7.1
[Qemu-devel] [PATCH v1 5/5] target-ppc: add vector permute right indexed instruction
Add vpermr instruction from ISA 3.0.
Signed-off-by: Rajalakshmi Srinivasaraghavan
---
target-ppc/helper.h |1 +
target-ppc/int_helper.c | 23 +++
target-ppc/translate/vmx-impl.c | 18 ++
target-ppc/translate/vmx-ops.c |1 +
4 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index d1d9418..3c476c9 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -270,6 +270,7 @@ DEF_HELPER_5(vmsumubm, void, env, avr, avr, avr, avr)
DEF_HELPER_5(vmsummbm, void, env, avr, avr, avr, avr)
DEF_HELPER_5(vsel, void, env, avr, avr, avr, avr)
DEF_HELPER_5(vperm, void, env, avr, avr, avr, avr)
+DEF_HELPER_5(vpermr, void, env, avr, avr, avr, avr)
DEF_HELPER_4(vpkshss, void, env, avr, avr, avr)
DEF_HELPER_4(vpkshus, void, env, avr, avr, avr)
DEF_HELPER_4(vpkswss, void, env, avr, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index d8ad56f..79e4273 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1126,6 +1126,29 @@ void helper_vperm(CPUPPCState *env, ppc_avr_t *r,
ppc_avr_t *a, ppc_avr_t *b,
*r = result;
}
+void helper_vpermr(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b,
+ ppc_avr_t *c)
+{
+ppc_avr_t result;
+int i;
+
+VECTOR_FOR_INORDER_I(i, u8) {
+int s = c->u8[i] & 0x1f;
+#if defined(HOST_WORDS_BIGENDIAN)
+int index = s & 0xf;
+#else
+int index = 15 - (s & 0xf);
+#endif
+
+if (s & 0x10) {
+result.u8[i] = a->u8[15 - index];
+} else {
+result.u8[i] = b->u8[15 - index];
+}
+}
+*r = result;
+}
+
#if defined(HOST_WORDS_BIGENDIAN)
#define VBPERMQ_INDEX(avr, i) ((avr)->u8[(i)])
#define VBPERMQ_DW(index) (((index) & 0x40) != 0)
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 5ddff58..d13640f 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -728,6 +728,24 @@ static void gen_vmladduhm(DisasContext *ctx)
tcg_temp_free_ptr(rd);
}
+static void gen_vpermr(DisasContext *ctx)
+{
+TCGv_ptr ra, rb, rc, rd;
+if (unlikely(!ctx->altivec_enabled)) {
+gen_exception(ctx, POWERPC_EXCP_VPU);
+return;
+}
+ra = gen_avr_ptr(rA(ctx->opcode));
+rb = gen_avr_ptr(rB(ctx->opcode));
+rc = gen_avr_ptr(rC(ctx->opcode));
+rd = gen_avr_ptr(rD(ctx->opcode));
+gen_helper_vpermr(cpu_env, rd, ra, rb, rc);
+tcg_temp_free_ptr(ra);
+tcg_temp_free_ptr(rb);
+tcg_temp_free_ptr(rc);
+tcg_temp_free_ptr(rd);
+}
+
GEN_VAFORM_PAIRED(vmsumubm, vmsummbm, 18)
GEN_VAFORM_PAIRED(vmsumuhm, vmsumuhs, 19)
GEN_VAFORM_PAIRED(vmsumshm, vmsumshs, 20)
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index 32bd533..ad72db5 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -219,6 +219,7 @@ GEN_VXFORM_300_EO(vctzb, 0x01, 0x18, 0x1C),
GEN_VXFORM_300_EO(vctzh, 0x01, 0x18, 0x1D),
GEN_VXFORM_300_EO(vctzw, 0x01, 0x18, 0x1E),
GEN_VXFORM_300_EO(vctzd, 0x01, 0x18, 0x1F),
+GEN_VXFORM_300(vpermr, 0x1D, 0xFF),
#define GEN_VXFORM_NOA(name, opc2, opc3)\
GEN_HANDLER(name, 0x04, opc2, opc3, 0x001f, PPC_ALTIVEC)
--
1.7.1
[Qemu-devel] [PATCH v1 0/5] POWER9 TCG enablement - part3
This series contains 14 new instructions for POWER9 described in ISA3.0. Patches: 01: Adds vector insert instructions. vinsertb - Vector Insert Byte vinserth - Vector Insert Halfword vinsertw - Vector Insert Word vinsertd - Vector Insert Doubleword 02: Adds vector extract instructions. vextractub - Vector Extract Unsigned Byte vextractuh - Vector Extract Unsigned Halfword vextractuw - Vector Extract Unsigned Word vextractd - Vector Extract Unsigned Doubleword 03: Adds vector count trailing zeros instructions. vctzb - Vector Count Trailing Zeros Byte vctzh - Vector Count Trailing Zeros Halfword vctzw - Vector Count Trailing Zeros Word vctzd - Vector Count Trailing Zeros Doubleword 04: Adds vbpermd-vector bit permute doubleword instruction. 05: Adds vpermr-vector permute right indexed instruction. Changelog: v0: * Rename GEN_VXFORM_300_EXT1 to GEN_VXFORM_300_EO. * Rename GEN_VXFORM_DUAL1 to GEN_VXFORM_DUAL_INV. * Remove undef GEN_VXFORM_DUAL1. target-ppc/helper.h | 14 + target-ppc/int_helper.c | 110 +++ target-ppc/translate/vmx-impl.c | 58 target-ppc/translate/vmx-ops.c | 38 +++--- 4 files changed, 212 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH v1 1/5] target-ppc: add vector insert instructions
The following vector insert instructions are added from ISA 3.0.
vinsertb - Vector Insert Byte
vinserth - Vector Insert Halfword
vinsertw - Vector Insert Word
vinsertd - Vector Insert Doubleword
Signed-off-by: Rajalakshmi Srinivasaraghavan
---
target-ppc/helper.h |4
target-ppc/int_helper.c | 21 +
target-ppc/translate/vmx-impl.c | 10 ++
target-ppc/translate/vmx-ops.c | 18 +-
4 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 93ac9e1..0923779 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -250,6 +250,10 @@ DEF_HELPER_2(vspltisw, void, avr, i32)
DEF_HELPER_3(vspltb, void, avr, avr, i32)
DEF_HELPER_3(vsplth, void, avr, avr, i32)
DEF_HELPER_3(vspltw, void, avr, avr, i32)
+DEF_HELPER_3(vinsertb, void, avr, avr, i32)
+DEF_HELPER_3(vinserth, void, avr, avr, i32)
+DEF_HELPER_3(vinsertw, void, avr, avr, i32)
+DEF_HELPER_3(vinsertd, void, avr, avr, i32)
DEF_HELPER_2(vupkhpx, void, avr, avr)
DEF_HELPER_2(vupklpx, void, avr, avr)
DEF_HELPER_2(vupkhsb, void, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 552b2e0..637f0b1 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1790,6 +1790,27 @@ VSPLT(b, u8)
VSPLT(h, u16)
VSPLT(w, u32)
#undef VSPLT
+#if defined(HOST_WORDS_BIGENDIAN)
+#define VINSERT(suffix, element, index) \
+void helper_vinsert##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{ \
+memcpy(&r->u8[SPLAT_ELEMENT(u8)], &b->element[index], \
+ sizeof(r->element[0])); \
+}
+#else
+#define VINSERT(suffix, element, index) \
+void helper_vinsert##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{ \
+memcpy(&r->u8[(16 - splat) - sizeof(r->element[0])],\
+ &b->element[(ARRAY_SIZE(r->element) - index) - 1], \
+ sizeof(r->element[0])); \
+}
+#endif
+VINSERT(b, u8, 7)
+VINSERT(h, u16, 3)
+VINSERT(w, u32, 1)
+VINSERT(d, u64, 0)
+#undef VINSERT
#undef SPLAT_ELEMENT
#undef _SPLAT_MASKED
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index ac78caf..4940ae3 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -626,10 +626,20 @@ static void glue(gen_, name)(DisasContext *ctx)
\
GEN_VXFORM_UIMM(vspltb, 6, 8);
GEN_VXFORM_UIMM(vsplth, 6, 9);
GEN_VXFORM_UIMM(vspltw, 6, 10);
+GEN_VXFORM_UIMM(vinsertb, 6, 12);
+GEN_VXFORM_UIMM(vinserth, 6, 13);
+GEN_VXFORM_UIMM(vinsertw, 6, 14);
+GEN_VXFORM_UIMM(vinsertd, 6, 15);
GEN_VXFORM_UIMM_ENV(vcfux, 5, 12);
GEN_VXFORM_UIMM_ENV(vcfsx, 5, 13);
GEN_VXFORM_UIMM_ENV(vctuxs, 5, 14);
GEN_VXFORM_UIMM_ENV(vctsxs, 5, 15);
+GEN_VXFORM_DUAL(vspltisb, PPC_NONE, PPC2_ALTIVEC_207,
+ vinsertb, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vspltish, PPC_NONE, PPC2_ALTIVEC_207,
+ vinserth, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vspltisw, PPC_NONE, PPC2_ALTIVEC_207,
+ vinsertw, PPC_NONE, PPC2_ISA300);
static void gen_vsldoi(DisasContext *ctx)
{
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index 7449396..ca69e56 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -41,6 +41,9 @@ GEN_HANDLER_E(name, 0x04, opc2, opc3, 0x, PPC_NONE,
PPC2_ALTIVEC_207)
#define GEN_VXFORM_300(name, opc2, opc3)\
GEN_HANDLER_E(name, 0x04, opc2, opc3, 0x, PPC_NONE, PPC2_ISA300)
+#define GEN_VXFORM_300_EXT(name, opc2, opc3, inval) \
+GEN_HANDLER_E(name, 0x04, opc2, opc3, inval, PPC_NONE, PPC2_ISA300)
+
#define GEN_VXFORM_DUAL(name0, name1, opc2, opc3, type0, type1) \
GEN_HANDLER_E(name0##_##name1, 0x4, opc2, opc3, 0x, type0, type1)
@@ -191,11 +194,16 @@ GEN_VXRFORM(vcmpgefp, 3, 7)
GEN_VXRFORM_DUAL(vcmpgtfp, vcmpgtud, 3, 11, PPC_ALTIVEC, PPC_NONE)
GEN_VXRFORM_DUAL(vcmpbfp, vcmpgtsd, 3, 15, PPC_ALTIVEC, PPC_NONE)
-#define GEN_VXFORM_SIMM(name, opc2, opc3) \
-GEN_HANDLER(name, 0x04, opc2, opc3, 0x, PPC_ALTIVEC)
-GEN_VXFORM_SIMM(vspltisb, 6, 12),
-GEN_VXFORM_SIMM(vspltish, 6, 13),
-GEN_VXFORM_SIMM(vspltisw, 6, 14),
+#define GEN_VXFORM_DUAL_INV(name0, name1, opc2, opc3, inval0, inval1, type) \
+GEN_OPCODE_DUAL(name0##_##name1, 0x04, opc2, opc3, inval0, inval1, type, \
+ PPC_NONE)
+GEN_VXFORM_DUAL_INV(vspltisb, vinsertb, 6, 12, 0x, 0x10,
+ PPC2_
[Qemu-devel] [PATCH v1 3/5] target-ppc: add vector count trailing zeros instructions
The following vector count trailing zeros instructions are
added from ISA 3.0.
vctzb - Vector Count Trailing Zeros Byte
vctzh - Vector Count Trailing Zeros Halfword
vctzw - Vector Count Trailing Zeros Word
vctzd - Vector Count Trailing Zeros Doubleword
Signed-off-by: Rajalakshmi Srinivasaraghavan
---
target-ppc/helper.h |4
target-ppc/int_helper.c | 15 +++
target-ppc/translate/vmx-impl.c | 19 +++
target-ppc/translate/vmx-ops.c |8
4 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 59e7b88..6e6e7b3 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -327,6 +327,10 @@ DEF_HELPER_2(vclzb, void, avr, avr)
DEF_HELPER_2(vclzh, void, avr, avr)
DEF_HELPER_2(vclzw, void, avr, avr)
DEF_HELPER_2(vclzd, void, avr, avr)
+DEF_HELPER_2(vctzb, void, avr, avr)
+DEF_HELPER_2(vctzh, void, avr, avr)
+DEF_HELPER_2(vctzw, void, avr, avr)
+DEF_HELPER_2(vctzd, void, avr, avr)
DEF_HELPER_2(vpopcntb, void, avr, avr)
DEF_HELPER_2(vpopcnth, void, avr, avr)
DEF_HELPER_2(vpopcntw, void, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index d545ec6..09f02f8 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2090,6 +2090,21 @@ VGENERIC_DO(clzd, u64)
#undef clzw
#undef clzd
+#define ctzb(v) ((v) ? ctz32((uint32_t)(v)) : 8)
+#define ctzh(v) ((v) ? ctz32((uint32_t)(v)) : 16)
+#define ctzw(v) ctz32((v))
+#define ctzd(v) ctz64((v))
+
+VGENERIC_DO(ctzb, u8)
+VGENERIC_DO(ctzh, u16)
+VGENERIC_DO(ctzw, u32)
+VGENERIC_DO(ctzd, u64)
+
+#undef ctzb
+#undef ctzh
+#undef ctzw
+#undef ctzd
+
#define popcntb(v) ctpop8(v)
#define popcnth(v) ctpop16(v)
#define popcntw(v) ctpop32(v)
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 8bd48f3..2cf8c8f 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -553,6 +553,21 @@ static void glue(gen_, name)(DisasContext *ctx)
\
tcg_temp_free_ptr(rd); \
}
+#define GEN_VXFORM_NOA_2(name, opc2, opc3, opc4)\
+static void glue(gen_, name)(DisasContext *ctx) \
+{ \
+TCGv_ptr rb, rd;\
+if (unlikely(!ctx->altivec_enabled)) { \
+gen_exception(ctx, POWERPC_EXCP_VPU); \
+return; \
+} \
+rb = gen_avr_ptr(rB(ctx->opcode)); \
+rd = gen_avr_ptr(rD(ctx->opcode)); \
+gen_helper_##name(rd, rb); \
+tcg_temp_free_ptr(rb); \
+tcg_temp_free_ptr(rd); \
+}
+
GEN_VXFORM_NOA(vupkhsb, 7, 8);
GEN_VXFORM_NOA(vupkhsh, 7, 9);
GEN_VXFORM_NOA(vupkhsw, 7, 25);
@@ -723,6 +738,10 @@ GEN_VXFORM_NOA(vclzb, 1, 28)
GEN_VXFORM_NOA(vclzh, 1, 29)
GEN_VXFORM_NOA(vclzw, 1, 30)
GEN_VXFORM_NOA(vclzd, 1, 31)
+GEN_VXFORM_NOA_2(vctzb, 1, 24, 28)
+GEN_VXFORM_NOA_2(vctzh, 1, 24, 29)
+GEN_VXFORM_NOA_2(vctzw, 1, 24, 30)
+GEN_VXFORM_NOA_2(vctzd, 1, 24, 31)
GEN_VXFORM_NOA(vpopcntb, 1, 28)
GEN_VXFORM_NOA(vpopcnth, 1, 29)
GEN_VXFORM_NOA(vpopcntw, 1, 30)
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index aafe70b..5b2826e 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -44,6 +44,10 @@ GEN_HANDLER_E(name, 0x04, opc2, opc3, 0x, PPC_NONE,
PPC2_ISA300)
#define GEN_VXFORM_300_EXT(name, opc2, opc3, inval) \
GEN_HANDLER_E(name, 0x04, opc2, opc3, inval, PPC_NONE, PPC2_ISA300)
+#define GEN_VXFORM_300_EO(name, opc2, opc3, opc4) \
+GEN_HANDLER_E_2(name, 0x04, opc2, opc3, opc4, 0x, PPC_NONE, \
+ PPC2_ISA300)
+
#define GEN_VXFORM_DUAL(name0, name1, opc2, opc3, type0, type1) \
GEN_HANDLER_E(name0##_##name1, 0x4, opc2, opc3, 0x, type0, type1)
@@ -211,6 +215,10 @@ GEN_VXFORM_DUAL_INV(vspltish, vinserth, 6, 13, 0x,
0x10,
GEN_VXFORM_DUAL_INV(vspltisw, vinsertw, 6, 14, 0x, 0x10,
PPC2_ALTIVEC_207),
GEN_VXFORM_300_EXT(vinsertd, 6, 15, 0x10),
+GEN_VXFORM_300_EO(vctzb, 0x01, 0x18, 0x1C),
+GEN_VXFORM_300_EO(vctzh, 0x01, 0x18, 0x1D),
+GEN_VXFORM_300_EO(vctzw, 0x01, 0x18, 0x1E),
+GEN_VXFORM_300_EO(vctzd, 0x01, 0x18, 0x1F),
#define GEN_VXFORM_NOA(name, opc2, opc3)\
GEN_HANDLER(name, 0x04, opc2, opc3, 0x001f, PPC_ALTIVEC)
--
1.7.1
[Qemu-devel] [PATCH v1 2/5] target-ppc: add vector extract instructions
The following vector extract instructions are added from ISA 3.0.
vextractub - Vector Extract Unsigned Byte
vextractuh - Vector Extract Unsigned Halfword
vextractuw - Vector Extract Unsigned Word
vextractd - Vector Extract Unsigned Doubleword
Signed-off-by: Rajalakshmi Srinivasaraghavan
---
target-ppc/helper.h |4
target-ppc/int_helper.c | 31 +++
target-ppc/translate/vmx-impl.c | 10 ++
target-ppc/translate/vmx-ops.c | 10 +++---
4 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 0923779..59e7b88 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -250,6 +250,10 @@ DEF_HELPER_2(vspltisw, void, avr, i32)
DEF_HELPER_3(vspltb, void, avr, avr, i32)
DEF_HELPER_3(vsplth, void, avr, avr, i32)
DEF_HELPER_3(vspltw, void, avr, avr, i32)
+DEF_HELPER_3(vextractub, void, avr, avr, i32)
+DEF_HELPER_3(vextractuh, void, avr, avr, i32)
+DEF_HELPER_3(vextractuw, void, avr, avr, i32)
+DEF_HELPER_3(vextractd, void, avr, avr, i32)
DEF_HELPER_3(vinsertb, void, avr, avr, i32)
DEF_HELPER_3(vinserth, void, avr, avr, i32)
DEF_HELPER_3(vinsertw, void, avr, avr, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 637f0b1..d545ec6 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1811,6 +1811,37 @@ VINSERT(h, u16, 3)
VINSERT(w, u32, 1)
VINSERT(d, u64, 0)
#undef VINSERT
+#if defined(HOST_WORDS_BIGENDIAN)
+#define VEXTRACT(suffix, element, index) \
+void helper_vextract##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{\
+int i; \
+ \
+for (i = 0; i < ARRAY_SIZE(r->element); i++) { \
+r->element[i] = 0; \
+}\
+memcpy(&r->element[index], &b->u8[SPLAT_ELEMENT(u8)],\
+ sizeof(r->element[0])); \
+}
+#else
+#define VEXTRACT(suffix, element, index) \
+void helper_vextract##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{\
+int i; \
+ \
+for (i = 0; i < ARRAY_SIZE(r->element); i++) { \
+r->element[i] = 0; \
+}\
+memcpy(&r->element[(ARRAY_SIZE(r->element) - index) - 1],\
+ &b->u8[(16 - splat) - sizeof(r->element[0])], \
+ sizeof(r->element[0])); \
+}
+#endif
+VEXTRACT(ub, u8, 7)
+VEXTRACT(uh, u16, 3)
+VEXTRACT(uw, u32, 1)
+VEXTRACT(d, u64, 0)
+#undef VEXTRACT
#undef SPLAT_ELEMENT
#undef _SPLAT_MASKED
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 4940ae3..8bd48f3 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -626,6 +626,10 @@ static void glue(gen_, name)(DisasContext *ctx)
\
GEN_VXFORM_UIMM(vspltb, 6, 8);
GEN_VXFORM_UIMM(vsplth, 6, 9);
GEN_VXFORM_UIMM(vspltw, 6, 10);
+GEN_VXFORM_UIMM(vextractub, 6, 8);
+GEN_VXFORM_UIMM(vextractuh, 6, 9);
+GEN_VXFORM_UIMM(vextractuw, 6, 10);
+GEN_VXFORM_UIMM(vextractd, 6, 11);
GEN_VXFORM_UIMM(vinsertb, 6, 12);
GEN_VXFORM_UIMM(vinserth, 6, 13);
GEN_VXFORM_UIMM(vinsertw, 6, 14);
@@ -634,6 +638,12 @@ GEN_VXFORM_UIMM_ENV(vcfux, 5, 12);
GEN_VXFORM_UIMM_ENV(vcfsx, 5, 13);
GEN_VXFORM_UIMM_ENV(vctuxs, 5, 14);
GEN_VXFORM_UIMM_ENV(vctsxs, 5, 15);
+GEN_VXFORM_DUAL(vspltb, PPC_NONE, PPC2_ALTIVEC_207,
+ vextractub, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vsplth, PPC_NONE, PPC2_ALTIVEC_207,
+ vextractuh, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vspltw, PPC_NONE, PPC2_ALTIVEC_207,
+ vextractuw, PPC_NONE, PPC2_ISA300);
GEN_VXFORM_DUAL(vspltisb, PPC_NONE, PPC2_ALTIVEC_207,
vinsertb, PPC_NONE, PPC2_ISA300);
GEN_VXFORM_DUAL(vspltish, PPC_NONE, PPC2_ALTIVEC_207,
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index ca69e56..aafe70b 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -197,6 +197,13 @@ GEN_VXRFORM_DUAL(vcmpbfp, vcmpgtsd, 3, 15, PPC_ALTIVEC,
PPC_NONE)
#define GEN_VXFORM_DUAL_INV(name0, name1, opc2, opc3, inval0, inval1, type
Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist
Copying the QGA maintainer.
marcandre.lur...@redhat.com writes:
> From: Marc-André Lureau
>
> Free the list, not just the elements.
>
> Signed-off-by: Marc-André Lureau
> ---
> include/glib-compat.h | 9 +
> qga/main.c| 8 ++--
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/glib-compat.h b/include/glib-compat.h
> index 01aa7b3..6d643b2 100644
> --- a/include/glib-compat.h
> +++ b/include/glib-compat.h
> @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable
> *hash_table, gpointer key)
> } while (0)
> #endif
>
> +/*
> + * A GFunc function helper freeing the first argument (not part of glib)
Well, it's not part of GLib, because non-obsolete GLib has no use for
it! You'd simply pass g_free directly to a _free_full() function
instead of passing a silly wrapper to a _foreach() function.
The commit does a bit more than just plug a leak, it also provides a new
helper for general use. Mention in the commit message?
I wonder how many more of these silly g_free() wrappers we have. A
quick grep reports hits in string-input-visitor.c and
qobject/json-streamer.c. If you add one to a header, you get to hunt
them down :)
> + */
> +static inline void qemu_g_func_free(gpointer data,
> +gpointer user_data)
> +{
> +g_free(data);
> +}
> +
> #endif
> diff --git a/qga/main.c b/qga/main.c
> index 4c3b2c7..868508b 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
> #ifdef CONFIG_FSFREEZE
> g_free(config->fsfreeze_hook);
> #endif
> +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
> +g_list_free(config->blacklist);
Modern GLib code doesn't need silly wrappers:
g_list_free_full(config->blacklist, g_free);
Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
Please explain this in the commit message.
Even better, provide a replacement in glib-compat.h #if
!GLIB_CHECK_VERSION(2, 28, 0). Will facilitate ditching the work-around
when we upgrade to 2.28.
> g_free(config);
> }
>
> @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *config)
> return EXIT_SUCCESS;
> }
>
> -static void free_blacklist_entry(gpointer entry, gpointer unused)
> -{
> -g_free(entry);
> -}
> -
> int main(int argc, char **argv)
> {
> int ret = EXIT_SUCCESS;
> @@ -1379,7 +1376,6 @@ end:
> if (s->channel) {
> ga_channel_free(s->channel);
> }
> -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
> g_free(s->pstate_filepath);
> g_free(s->state_filepath_isfrozen);
If you at least explain why not g_list_free_full() in the commit
message, you can add
Reviewed-by: Markus Armbruster
But I'd like to encourage you to explore providing a replacement for
g_list_free_full().
Re: [Qemu-devel] [PATCH for-2.7 v3 04/36] qga: free remaining leaking state
Copying the QGA maintainer.
marcandre.lur...@redhat.com writes:
> From: Marc-André Lureau
>
> Signed-off-by: Marc-André Lureau
> ---
> qga/guest-agent-command-state.c | 7 +++
> qga/guest-agent-core.h | 1 +
> qga/main.c | 6 ++
> 3 files changed, 14 insertions(+)
>
> diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-state.c
> index 4de229c..31c6368 100644
> --- a/qga/guest-agent-command-state.c
> +++ b/qga/guest-agent-command-state.c
> @@ -71,3 +71,10 @@ GACommandState *ga_command_state_new(void)
> cs->groups = NULL;
> return cs;
> }
> +
> +void ga_command_state_free(GACommandState *cs)
> +{
> +g_slist_foreach(cs->groups, qemu_g_func_free, NULL);
> +g_slist_free(cs->groups);
Remarks on g_list_free_full() in PATCH 03 apply to g_slist_free_full()
here.
> +g_free(cs);
> +}
> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> index 0a49516..63e9d39 100644
> --- a/qga/guest-agent-core.h
> +++ b/qga/guest-agent-core.h
> @@ -28,6 +28,7 @@ void ga_command_state_add(GACommandState *cs,
> void ga_command_state_init_all(GACommandState *cs);
> void ga_command_state_cleanup_all(GACommandState *cs);
> GACommandState *ga_command_state_new(void);
> +void ga_command_state_free(GACommandState *cs);
> bool ga_logging_enabled(GAState *s);
> void ga_disable_logging(GAState *s);
> void ga_enable_logging(GAState *s);
> diff --git a/qga/main.c b/qga/main.c
> index 868508b..0038702 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1372,6 +1372,8 @@ int main(int argc, char **argv)
> end:
> if (s->command_state) {
> ga_command_state_cleanup_all(s->command_state);
> +ga_command_state_free(s->command_state);
> +json_message_parser_destroy(&s->parser);
> }
> if (s->channel) {
> ga_channel_free(s->channel);
> @@ -1384,6 +1386,10 @@ end:
> }
>
> config_free(config);
> +if (s->main_loop) {
> +g_main_loop_unref(s->main_loop);
> +}
> +g_free(s);
>
> return ret;
> }
Not bothering to free memory immediately before terminating the process
is not a leak. The commit message shouldn't claim it is.
Personally, I wouldn't bother with freeing memory there. Applies to
PATCH 03, too. But it looks like the maintainer likes having it done.
If that's true, then doing it completely is probably better.
Leaving actual review to the maintainer.
Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist
Hi
- Original Message -
> Copying the QGA maintainer.
>
> marcandre.lur...@redhat.com writes:
>
> > From: Marc-André Lureau
> >
> > Free the list, not just the elements.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > include/glib-compat.h | 9 +
> > qga/main.c| 8 ++--
> > 2 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/glib-compat.h b/include/glib-compat.h
> > index 01aa7b3..6d643b2 100644
> > --- a/include/glib-compat.h
> > +++ b/include/glib-compat.h
> > @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable
> > *hash_table, gpointer key)
> > } while (0)
> > #endif
> >
> > +/*
> > + * A GFunc function helper freeing the first argument (not part of glib)
>
> Well, it's not part of GLib, because non-obsolete GLib has no use for
> it! You'd simply pass g_free directly to a _free_full() function
> instead of passing a silly wrapper to a _foreach() function.
>
> The commit does a bit more than just plug a leak, it also provides a new
> helper for general use. Mention in the commit message?
>
> I wonder how many more of these silly g_free() wrappers we have. A
> quick grep reports hits in string-input-visitor.c and
> qobject/json-streamer.c. If you add one to a header, you get to hunt
> them down :)
>
> > + */
> > +static inline void qemu_g_func_free(gpointer data,
> > +gpointer user_data)
> > +{
> > +g_free(data);
> > +}
> > +
> > #endif
> > diff --git a/qga/main.c b/qga/main.c
> > index 4c3b2c7..868508b 100644
> > --- a/qga/main.c
> > +++ b/qga/main.c
> > @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
> > #ifdef CONFIG_FSFREEZE
> > g_free(config->fsfreeze_hook);
> > #endif
> > +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
> > +g_list_free(config->blacklist);
>
> Modern GLib code doesn't need silly wrappers:
>
> g_list_free_full(config->blacklist, g_free);
>
> Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
> Please explain this in the commit message.
>
> Even better, provide a replacement in glib-compat.h #if
> !GLIB_CHECK_VERSION(2, 28, 0). Will facilitate ditching the work-around
> when we upgrade to 2.28.
ok
>
> > g_free(config);
> > }
> >
> > @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *config)
> > return EXIT_SUCCESS;
> > }
> >
> > -static void free_blacklist_entry(gpointer entry, gpointer unused)
> > -{
> > -g_free(entry);
> > -}
> > -
> > int main(int argc, char **argv)
> > {
> > int ret = EXIT_SUCCESS;
> > @@ -1379,7 +1376,6 @@ end:
> > if (s->channel) {
> > ga_channel_free(s->channel);
> > }
> > -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
> > g_free(s->pstate_filepath);
> > g_free(s->state_filepath_isfrozen);
>
> If you at least explain why not g_list_free_full() in the commit
> message, you can add
> Reviewed-by: Markus Armbruster
>
> But I'd like to encourage you to explore providing a replacement for
> g_list_free_full().
Such replacement would make use of a (GFunc) cast, glib implementation is
calling g_list_foreach (list, (GFunc) free_func, NULL), but Eric didn't want to
have such cast in qemu code. He suggested to have the static inline helper in
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06561.html
Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name
marcandre.lur...@redhat.com writes:
> From: Marc-André Lureau
>
> Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
> class base init name generation instead. Get rid of some leaks that way.
>
> Signed-off-by: Marc-André Lureau
> ---
> hw/core/machine.c| 1 +
> include/hw/boards.h | 2 +-
> include/hw/i386/pc.h | 1 -
> 3 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index e5a456f..00fbe3e 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass *klass,
> void *data)
> if (mc->compat_props) {
> g_array_free(mc->compat_props, true);
> }
> +g_free(mc->name);
> }
>
> void machine_register_compat_props(MachineState *machine)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3e69eca..e46a744 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -93,7 +93,7 @@ struct MachineClass {
> /*< public >*/
>
> const char *family; /* NULL iff @name identifies a standalone machtype */
> -const char *name;
> +char *name;
> const char *alias;
> const char *desc;
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index eb1d414..afd025a 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t
> *);
> { \
> MachineClass *mc = MACHINE_CLASS(oc); \
> optsfn(mc); \
> -mc->name = namestr; \
> mc->init = initfn; \
> } \
> static const TypeInfo pc_machine_type_##suffix = { \
I guess there are is at least one assignment to mc->name not visible in
this patch that assigns an allocated string, which leaks before the
patch. The commit message seems to provide a clue "class base init name
generation". I could probably find it with some effort, but patches
that take that much work to understand make me grumpy. Please provide
another clue :)
[Qemu-devel] [PATCH v2] trace: add syslog tracing backend
This patch adds a tracing backend which sends output using syslog().
The syslog backend is limited to POSIX compliant systems.
openlog() is called with facility set to LOG_DAEMON, with the LOG_PID
option. Trace events are logged at level LOG_INFO.
Signed-off-by: Paul Durrant
Cc: Stefan Hajnoczi
---
v2:
- Added a section to docs/tracing.txt
- Re-ordered header inclusion in generated code
---
configure | 19
docs/tracing.txt| 12 ++
scripts/tracetool/backend/syslog.py | 45 +
trace/control.c | 7 ++
4 files changed, 83 insertions(+)
create mode 100644 scripts/tracetool/backend/syslog.py
diff --git a/configure b/configure
index 879324b..fce00b8 100755
--- a/configure
+++ b/configure
@@ -4189,6 +4189,18 @@ if compile_prog "" "" ; then
fi
##
+# check if we have posix_syslog
+
+posix_syslog=no
+cat > $TMPC << EOF
+#include
+int main(void) { openlog("qemu", LOG_PID, LOG_DAEMON); syslog(LOG_INFO,
"configure"); return 0; }
+EOF
+if compile_prog "" "" ; then
+posix_syslog=yes
+fi
+
+##
# check if trace backend exists
$python "$source_path/scripts/tracetool.py" "--backends=$trace_backends"
--check-backends > /dev/null 2> /dev/null
@@ -5456,6 +5468,13 @@ if have_backend "ftrace"; then
feature_not_found "ftrace(trace backend)" "ftrace requires Linux"
fi
fi
+if have_backend "syslog"; then
+ if test "$posix_syslog" = "yes" ; then
+echo "CONFIG_TRACE_SYSLOG=y" >> $config_host_mak
+ else
+feature_not_found "syslog(trace backend)" "syslog not available"
+ fi
+fi
echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
if test "$rdma" = "yes" ; then
diff --git a/docs/tracing.txt b/docs/tracing.txt
index 29f2f9a..e62444c 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -192,6 +192,18 @@ After running qemu by root user, you can get the trace:
Restriction: "ftrace" backend is restricted to Linux only.
+=== Syslog ===
+
+The "syslog" backend sends trace events using the POSIX syslog API. The log
+is opened specifying the LOG_DAEMON facility and LOG_PID option (so events
+are tagged with the pid of the particular QEMU process that generated
+them). All events are logged at LOG_INFO level.
+
+NOTE: syslog may squash duplicate consecutive trace events and apply rate
+ limiting.
+
+Restriction: "syslog" backend is restricted to POSIX compliant OS.
+
Monitor commands
* trace-file on|off|flush|set
diff --git a/scripts/tracetool/backend/syslog.py
b/scripts/tracetool/backend/syslog.py
new file mode 100644
index 000..89019bc
--- /dev/null
+++ b/scripts/tracetool/backend/syslog.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Syslog built-in backend.
+"""
+
+__author__ = "Paul Durrant "
+__copyright__ = "Copyright 2016, Citrix Systems Inc."
+__license__= "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__ = "stefa...@redhat.com"
+
+
+from tracetool import out
+
+
+PUBLIC = True
+
+
+def generate_h_begin(events):
+out('#include ',
+'#include "trace/control.h"',
+'')
+
+
+def generate_h(event):
+argnames = ", ".join(event.args.names())
+if len(event.args) > 0:
+argnames = ", " + argnames
+
+if "vcpu" in event.properties:
+# already checked on the generic format code
+cond = "true"
+else:
+cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
+
+out('if (%(cond)s) {',
+'syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
+'}',
+cond=cond,
+name=event.name,
+fmt=event.fmt.rstrip("\n"),
+argnames=argnames)
diff --git a/trace/control.c b/trace/control.c
index d173c09..b179cde 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -19,6 +19,9 @@
#ifdef CONFIG_TRACE_LOG
#include "qemu/log.h"
#endif
+#ifdef CONFIG_TRACE_SYSLOG
+#include
+#endif
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "qemu/config-file.h"
@@ -250,6 +253,10 @@ bool trace_init_backends(void)
}
#endif
+#ifdef CONFIG_TRACE_SYSLOG
+openlog(NULL, LOG_PID, LOG_DAEMON);
+#endif
+
return true;
}
--
2.1.4
Re: [Qemu-devel] [PATCH for-2.7] Xen PCI passthrough: fix passthrough failure when no interrupt pin
On Wed, Jul 27, 2016 at 09:30:48AM -0600, Bruce Rogers wrote:
> Commit 5a11d0f7 mistakenly converted a log message into an error
> condition when no pin interrupt is found for the pci device being
> passed through. Revert that part of the commit.
>
> Signed-off-by: Bruce Rogers
Acked-by: Anthony PERARD
> ---
> hw/xen/xen_pt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index f593b04..b6d71bb 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -842,7 +842,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
> goto err_out;
> }
> if (!scratch) {
> -error_setg(errp, "no pin interrupt");
> +XEN_PT_LOG(d, "no pin interrupt\n");
> goto out;
> }
>
> --
> 1.9.0
>
--
Anthony PERARD
[Qemu-devel] [PULL 3/5] linux-user: Don't write off end of new_utsname buffer
From: Peter Maydell
Use g_strlcpy() rather than strcpy() to copy the uname string
into the structure we return to the guest for the uname syscall.
This avoids overrunning the buffer if the user passed us an
overlong string via the QEMU command line.
We fix a comment typo while we're in the neighbourhood.
Signed-off-by: Peter Maydell
Signed-off-by: Riku Voipio
---
linux-user/syscall.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 092ff4e..5bc42c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9237,12 +9237,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
goto efault;
ret = get_errno(sys_uname(buf));
if (!is_error(ret)) {
-/* Overrite the native machine name with whatever is being
+/* Overwrite the native machine name with whatever is being
emulated. */
strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
/* Allow the user to override the reported release. */
-if (qemu_uname_release && *qemu_uname_release)
- strcpy (buf->release, qemu_uname_release);
+if (qemu_uname_release && *qemu_uname_release) {
+g_strlcpy(buf->release, qemu_uname_release,
+ sizeof(buf->release));
+}
}
unlock_user_struct(buf, arg1, 1);
}
--
2.1.4
[Qemu-devel] [PULL 5/5] linux-user: Handle brk() attempts with very large sizes
From: Peter Maydell
In do_brk(), we were inadvertently truncating the size
of a requested brk() from the guest by putting it into an
'int' variable. This meant that we would incorrectly report
success back to the guest rather than a failed allocation,
typically resulting in the guest then segfaulting. Use
abi_ulong instead.
This fixes a crash in the '31370.cc' test in the gcc libstdc++ test
suite (the test case starts by trying to allocate a very large
size and reduces the size until the allocation succeeds).
Signed-off-by: Peter Maydell
Signed-off-by: Riku Voipio
---
linux-user/syscall.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index df6f2a9..833f853 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -839,7 +839,7 @@ void target_set_brk(abi_ulong new_brk)
abi_long do_brk(abi_ulong new_brk)
{
abi_long mapped_addr;
-intnew_alloc_size;
+abi_ulong new_alloc_size;
DEBUGF_BRK("do_brk(" TARGET_ABI_FMT_lx ") -> ", new_brk);
--
2.1.4
[Qemu-devel] [PULL 1/5] linux-user: Use correct alignment for long long on i386 guests
From: Peter Maydell For i386, the ABI specifies that 'long long' (8 byte values) need only be 4 aligned, but we were requiring them to be 8-aligned. This meant we were laying out the target_epoll_event structure wrongly. Add a suitable ifdef to abitypes.h to specify the i386-specific alignment requirement. Reported-by: Icenowy Zheng Signed-off-by: Peter Maydell Reviewed-by: Laurent Vivier Signed-off-by: Riku Voipio --- include/exec/user/abitypes.h | 4 1 file changed, 4 insertions(+) diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h index a09d6c6..ba18860 100644 --- a/include/exec/user/abitypes.h +++ b/include/exec/user/abitypes.h @@ -15,6 +15,10 @@ #define ABI_LLONG_ALIGNMENT 2 #endif +#if defined(TARGET_I386) && !defined(TARGET_X86_64) +#define ABI_LLONG_ALIGNMENT 4 +#endif + #ifndef ABI_SHORT_ALIGNMENT #define ABI_SHORT_ALIGNMENT 2 #endif -- 2.1.4
[Qemu-devel] [PULL 0/5] linux-user fixes for 2.7
From: Riku Voipio The following changes since commit 09704e6ded83fa0bec14baf32f800f6512156ca0: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2016-08-04 10:24:27 +0100) are available in the git repository at: git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20160804 for you to fetch changes up to ef4330c23bb47b97a859dbdbae1c784fd2ca402f: linux-user: Handle brk() attempts with very large sizes (2016-08-04 16:38:17 +0300) linux-user important fixes for 2.7 Peter Maydell (5): linux-user: Use correct alignment for long long on i386 guests linux-user: Fix memchr() argument in open_self_cmdline() linux-user: Don't write off end of new_utsname buffer linux-user: Fix target_semid_ds structure definition linux-user: Handle brk() attempts with very large sizes include/exec/user/abitypes.h | 4 linux-user/syscall.c | 29 + linux-user/x86_64/target_structs.h | 15 +++ 3 files changed, 36 insertions(+), 12 deletions(-) -- 2.1.4
[Qemu-devel] [PULL 4/5] linux-user: Fix target_semid_ds structure definition
From: Peter Maydell
The target_semid_ds structure is not correct for all
architectures: the padding fields should only exist for:
* 32-bit ABIs
* x86
It is also misnamed, since it is following the kernel
semid64_ds structure (QEMU doesn't support the legacy
semid_ds structure at all). Rename the struct, provide
a correct generic definition and allow the oddball x86
architecture to provide its own version.
This fixes broken SYSV semaphores for all our 64-bit
architectures except x86 and ppc.
Signed-off-by: Peter Maydell
Signed-off-by: Riku Voipio
---
linux-user/syscall.c | 17 ++---
linux-user/x86_64/target_structs.h | 15 +++
2 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5bc42c0..df6f2a9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3754,27 +3754,30 @@ static struct shm_region {
bool in_use;
} shm_regions[N_SHM_REGIONS];
-struct target_semid_ds
+#ifndef TARGET_SEMID64_DS
+/* asm-generic version of this struct */
+struct target_semid64_ds
{
struct target_ipc_perm sem_perm;
abi_ulong sem_otime;
-#if !defined(TARGET_PPC64)
+#if TARGET_ABI_BITS == 32
abi_ulong __unused1;
#endif
abi_ulong sem_ctime;
-#if !defined(TARGET_PPC64)
+#if TARGET_ABI_BITS == 32
abi_ulong __unused2;
#endif
abi_ulong sem_nsems;
abi_ulong __unused3;
abi_ulong __unused4;
};
+#endif
static inline abi_long target_to_host_ipc_perm(struct ipc_perm *host_ip,
abi_ulong target_addr)
{
struct target_ipc_perm *target_ip;
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
if (!lock_user_struct(VERIFY_READ, target_sd, target_addr, 1))
return -TARGET_EFAULT;
@@ -3802,7 +3805,7 @@ static inline abi_long host_to_target_ipc_perm(abi_ulong
target_addr,
struct ipc_perm *host_ip)
{
struct target_ipc_perm *target_ip;
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0))
return -TARGET_EFAULT;
@@ -3829,7 +3832,7 @@ static inline abi_long host_to_target_ipc_perm(abi_ulong
target_addr,
static inline abi_long target_to_host_semid_ds(struct semid_ds *host_sd,
abi_ulong target_addr)
{
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
if (!lock_user_struct(VERIFY_READ, target_sd, target_addr, 1))
return -TARGET_EFAULT;
@@ -3845,7 +3848,7 @@ static inline abi_long target_to_host_semid_ds(struct
semid_ds *host_sd,
static inline abi_long host_to_target_semid_ds(abi_ulong target_addr,
struct semid_ds *host_sd)
{
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0))
return -TARGET_EFAULT;
diff --git a/linux-user/x86_64/target_structs.h
b/linux-user/x86_64/target_structs.h
index 3489827..b6e82a8 100644
--- a/linux-user/x86_64/target_structs.h
+++ b/linux-user/x86_64/target_structs.h
@@ -55,4 +55,19 @@ struct target_shmid_ds {
abi_ulong __unused5;
};
+/* The x86 definition differs from the generic one in that the
+ * two padding fields exist whether the ABI is 32 bits or 64 bits.
+ */
+#define TARGET_SEMID64_DS
+struct target_semid64_ds {
+struct target_ipc_perm sem_perm;
+abi_ulong sem_otime;
+abi_ulong __unused1;
+abi_ulong sem_ctime;
+abi_ulong __unused2;
+abi_ulong sem_nsems;
+abi_ulong __unused3;
+abi_ulong __unused4;
+};
+
#endif
--
2.1.4
[Qemu-devel] [PULL 2/5] linux-user: Fix memchr() argument in open_self_cmdline()
From: Peter Maydell
In open_self_cmdline() we look for a 0 in the buffer we read
from /prc/self/cmdline. We were incorrectly passing the length
of our buf[] array to memchr() as the length to search, rather
than the number of bytes we actually read into it, which could
be shorter. This was spotted by Coverity (because it could
result in our trying to pass a negative length argument to
write()).
Signed-off-by: Peter Maydell
Signed-off-by: Riku Voipio
---
linux-user/syscall.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ca6a2b4..092ff4e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6856,7 +6856,7 @@ static int open_self_cmdline(void *cpu_env, int fd)
if (!word_skipped) {
/* Skip the first string, which is the path to qemu-*-static
instead of the actual command. */
-cp_buf = memchr(buf, 0, sizeof(buf));
+cp_buf = memchr(buf, 0, nb_read);
if (cp_buf) {
/* Null byte found, skip one string */
cp_buf++;
--
2.1.4
Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name
Hi
On Thu, Aug 4, 2016 at 5:58 PM Markus Armbruster wrote:
> marcandre.lur...@redhat.com writes:
>
> > From: Marc-André Lureau
> >
> > Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
> > class base init name generation instead. Get rid of some leaks that way.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > hw/core/machine.c| 1 +
> > include/hw/boards.h | 2 +-
> > include/hw/i386/pc.h | 1 -
> > 3 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index e5a456f..00fbe3e 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass
> *klass, void *data)
> > if (mc->compat_props) {
> > g_array_free(mc->compat_props, true);
> > }
> > +g_free(mc->name);
> > }
> >
> > void machine_register_compat_props(MachineState *machine)
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 3e69eca..e46a744 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -93,7 +93,7 @@ struct MachineClass {
> > /*< public >*/
> >
> > const char *family; /* NULL iff @name identifies a standalone
> machtype */
> > -const char *name;
> > +char *name;
> > const char *alias;
> > const char *desc;
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index eb1d414..afd025a 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> uint64_t *);
> > { \
> > MachineClass *mc = MACHINE_CLASS(oc); \
> > optsfn(mc); \
> > -mc->name = namestr; \
> > mc->init = initfn; \
> > } \
> > static const TypeInfo pc_machine_type_##suffix = { \
>
> I guess there are is at least one assignment to mc->name not visible in
> this patch that assigns an allocated string, which leaks before the
> patch. The commit message seems to provide a clue "class base init name
> generation". I could probably find it with some effort, but patches
> that take that much work to understand make me grumpy. Please provide
> another clue :)
>
Sorry, thanks for reminding me to write better commit messages.
git grep 'mc->name ='
hw/core/machine.c:mc->name = g_strndup(cname,
Is that better:
Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
name generation from machine_class_base_init() instead, and free the
corresponding allocation in machine_class_finalize().
--
Marc-André Lureau
Re: [Qemu-devel] [Patch v1 26/29] qmp: add QMP interface "query-cpu-model-baseline"
On Thu, Aug 04, 2016 at 09:32:22AM +0200, David Hildenbrand wrote: > ## > > # @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). > > # > > # This interface can be used by tooling to create a compatible CPU model out > > # two CPU models. The created CPU model will be identical to or a subset of > > # both CPU models when comparing them. Therefore, the created CPU model is > > # guaranteed to run where the given CPU models run. > > # > > # The result returned by this command may be affected by: > > # > > # * QEMU version: CPU models may look different depending on the QEMU > version. > # (Except for CPU models reported as "static" in query-cpu-definitions.) > > # * machine-type: CPU model may look different depending on the > machine-type. > # (Except for CPU models reported as "static" in query-cpu-definitions.) > > # * machine options (including accelerator): in some architectures, CPU > models > # may look different depending on machine and accelerator options. (Except > for > # CPU models reported as "static" in query-cpu-definitions.) > > # * "-cpu" arguments and global properties: arguments to the -cpu option and > > # global properties may affect expansion of CPU models. Using > > # query-cpu-model-expansion while using these is not advised. > > # > > # Some architectures may not support baselining CPU models. s390x supports > > # baselining CPU models. > > # > > # Returns: a CpuModelBaselineInfo. Returns an error if baselining CPU models > is > # not supported, if a model cannot be used, if a model contains > > # an unknown cpu definition name, unknown properties or properties > > # with wrong types. > > # > > # Since: 2.8.0 > > ## Looks very clear to me, now. Thanks! -- Eduardo
Re: [Qemu-devel] [Patch v1 25/29] qmp: add QMP interface "query-cpu-model-comparison"
On Thu, Aug 04, 2016 at 09:34:16AM +0200, David Hildenbrand wrote: > ## > > # @CpuModelCompareResult: > > # > > # An enumeration of CPU model comparation results. The result is usually > > # calcualted using e.g. at CPU features or CPU generations. > > # > > # @incompatible: If model A is incompatible to model B, model A is not > > #guaranteed to run where model B runs and the other way > around. > # > > # @identical: If model A is identical to model B, model A is guaranteed to > run > # where model B runs and the other way around. > > # > > # @superset: If model A is a superset of model B, model B is guaranteed to > run > #where model A runs. There are no guarantees about the other way. > > # > > # @subset: If model A is a subset of model B, model A is guaranteed to run > > # where model B runs. There are no guarantees about the other way. > > # > > # Since: 2.8.0 > > ## > > Think it's all about guarantees. > [...] > This comment is now: > > ## > > # @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. > > # > > # Usually, a CPU model is compared against the maximum possible CPU model > > # of a ceratin configuration (e.g. the "host" model for KVM). If that CPU > > # model is identical or a subset, it will run in that configuration. > > # > > # The result returned by this command may be affected by: > > # > > # * QEMU version: CPU models may look different depending on the QEMU > version. > # (Except for CPU models reported as "static" in query-cpu-definitions.) > > # * machine-type: CPU model may look different depending on the > machine-type. > # (Except for CPU models reported as "static" in query-cpu-definitions.) > > # * machine options (including accelerator): in some architectures, CPU > models > # may look different depending on machine and accelerator options. (Except > for > # CPU models reported as "static" in query-cpu-definitions.) > > # * "-cpu" arguments and global properties: arguments to the -cpu option and > > # global properties may affect expansion of CPU models. Using > > # query-cpu-model-expansion while using these is not advised. > > # > > # Some architectures may not support comparing CPU models. s390x supports > > # comparing CPU models. > > # > > # Returns: a CpuModelBaselineInfo. Returns an error if comparing CPU models > is > # not supported, if a model cannot be used, if a model contains > > # an unknown cpu definition name, unknown properties or properties > > # with wrong types. > > # > > # Since: 2.8.0 > > ## > > (excluding the remark about s390x in this patch) Both look very clear to me. Thanks! -- Eduardo
Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist
Marc-André Lureau writes:
> Hi
>
> - Original Message -
>> Copying the QGA maintainer.
>>
>> marcandre.lur...@redhat.com writes:
>>
>> > From: Marc-André Lureau
>> >
>> > Free the list, not just the elements.
>> >
>> > Signed-off-by: Marc-André Lureau
>> > ---
>> > include/glib-compat.h | 9 +
>> > qga/main.c| 8 ++--
>> > 2 files changed, 11 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/include/glib-compat.h b/include/glib-compat.h
>> > index 01aa7b3..6d643b2 100644
>> > --- a/include/glib-compat.h
>> > +++ b/include/glib-compat.h
>> > @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable
>> > *hash_table, gpointer key)
>> > } while (0)
>> > #endif
>> >
>> > +/*
>> > + * A GFunc function helper freeing the first argument (not part of glib)
>>
>> Well, it's not part of GLib, because non-obsolete GLib has no use for
>> it! You'd simply pass g_free directly to a _free_full() function
>> instead of passing a silly wrapper to a _foreach() function.
>>
>> The commit does a bit more than just plug a leak, it also provides a new
>> helper for general use. Mention in the commit message?
>>
>> I wonder how many more of these silly g_free() wrappers we have. A
>> quick grep reports hits in string-input-visitor.c and
>> qobject/json-streamer.c. If you add one to a header, you get to hunt
>> them down :)
>>
>> > + */
>> > +static inline void qemu_g_func_free(gpointer data,
>> > +gpointer user_data)
>> > +{
>> > +g_free(data);
>> > +}
>> > +
>> > #endif
>> > diff --git a/qga/main.c b/qga/main.c
>> > index 4c3b2c7..868508b 100644
>> > --- a/qga/main.c
>> > +++ b/qga/main.c
>> > @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
>> > #ifdef CONFIG_FSFREEZE
>> > g_free(config->fsfreeze_hook);
>> > #endif
>> > +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
>> > +g_list_free(config->blacklist);
>>
>> Modern GLib code doesn't need silly wrappers:
>>
>> g_list_free_full(config->blacklist, g_free);
>>
>> Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
>> Please explain this in the commit message.
>>
>> Even better, provide a replacement in glib-compat.h #if
>> !GLIB_CHECK_VERSION(2, 28, 0). Will facilitate ditching the work-around
>> when we upgrade to 2.28.
>
> ok
>
>>
>> > g_free(config);
>> > }
>> >
>> > @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *config)
>> > return EXIT_SUCCESS;
>> > }
>> >
>> > -static void free_blacklist_entry(gpointer entry, gpointer unused)
>> > -{
>> > -g_free(entry);
>> > -}
>> > -
>> > int main(int argc, char **argv)
>> > {
>> > int ret = EXIT_SUCCESS;
>> > @@ -1379,7 +1376,6 @@ end:
>> > if (s->channel) {
>> > ga_channel_free(s->channel);
>> > }
>> > -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
>> > g_free(s->pstate_filepath);
>> > g_free(s->state_filepath_isfrozen);
>>
>> If you at least explain why not g_list_free_full() in the commit
>> message, you can add
>> Reviewed-by: Markus Armbruster
>>
>> But I'd like to encourage you to explore providing a replacement for
>> g_list_free_full().
>
> Such replacement would make use of a (GFunc) cast, glib implementation is
> calling g_list_foreach (list, (GFunc) free_func, NULL), but Eric didn't want
> to have such cast in qemu code. He suggested to have the static inline helper
> in https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06561.html
Pointlessly dirty:
g_list_foreach(list, (GFunc)g_free, NULL)
Trade dirty for cumbersome:
void free_wrapper(gpointer data, gpointer unused)
{
g_free(data)
}
g_list_foreach(list, free_wrapper, NULL);
But this is C. In C, simple things are best done the simple way. Even
when that means we don't get to show off how amazingly well we've been
educated on higher order functions:
for (node = list; node; node = next) {
next = node->next;
g_free(node);
}
Simple, stupid, and not dirty.
Quote: "Note that it is considered perfectly acceptable to access
list->next directly."
Re: [Qemu-devel] [PATCH for-2.7 v3 23/36] pc: keep gsi reference
marcandre.lur...@redhat.com writes:
> From: Marc-André Lureau
>
> Further cleanup would need to call qemu_free_irq() at the appropriate
> time, but for now this silences ASAN about direct leaks.
>
> Signed-off-by: Marc-André Lureau
I looked into cleaning up the qemu_irq allocation mess long ago, but
gave up after the work-in-progress had ballooned to a dozen commits or
so.
> ---
> hw/i386/pc_piix.c| 17 -
> hw/i386/pc_q35.c | 13 ++---
> include/hw/i386/pc.h | 1 +
> 3 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a07dc81..2af 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -74,7 +74,6 @@ static void pc_init1(MachineState *machine,
> ISABus *isa_bus;
> PCII440FXState *i440fx_state;
> int piix3_devfn = -1;
> -qemu_irq *gsi;
> qemu_irq *i8259;
> qemu_irq smi_irq;
> GSIState *gsi_state;
> @@ -185,16 +184,16 @@ static void pc_init1(MachineState *machine,
> gsi_state = g_malloc0(sizeof(*gsi_state));
> if (kvm_ioapic_in_kernel()) {
> kvm_pc_setup_irq_routing(pcmc->pci_enabled);
> -gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> - GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> + GSI_NUM_PINS);
> } else {
> -gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> }
>
> if (pcmc->pci_enabled) {
> pci_bus = i440fx_init(host_type,
>pci_type,
> - &i440fx_state, &piix3_devfn, &isa_bus, gsi,
> + &i440fx_state, &piix3_devfn, &isa_bus,
> pcms->gsi,
>system_memory, system_io, machine->ram_size,
>pcms->below_4g_mem_size,
>pcms->above_4g_mem_size,
> @@ -207,7 +206,7 @@ static void pc_init1(MachineState *machine,
>&error_abort);
> no_hpet = 1;
> }
> -isa_bus_irqs(isa_bus, gsi);
> +isa_bus_irqs(isa_bus, pcms->gsi);
>
> if (kvm_pic_in_kernel()) {
> i8259 = kvm_i8259_init(isa_bus);
> @@ -225,7 +224,7 @@ static void pc_init1(MachineState *machine,
> ioapic_init_gsi(gsi_state, "i440fx");
> }
>
> -pc_register_ferr_irq(gsi[13]);
> +pc_register_ferr_irq(pcms->gsi[13]);
>
> pc_vga_init(isa_bus, pcmc->pci_enabled ? pci_bus : NULL);
>
> @@ -235,7 +234,7 @@ static void pc_init1(MachineState *machine,
> }
>
> /* init basic PC hardware */
> -pc_basic_device_init(isa_bus, gsi, &rtc_state, true,
> +pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, true,
> (pcms->vmport != ON_OFF_AUTO_ON), 0x4);
>
> pc_nic_init(isa_bus, pci_bus);
> @@ -279,7 +278,7 @@ static void pc_init1(MachineState *machine,
> smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
> /* TODO: Populate SPD eeprom data. */
> smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
> - gsi[9], smi_irq,
> + pcms->gsi[9], smi_irq,
>pc_machine_is_smm_enabled(pcms),
>&piix4_pm);
> smbus_eeprom_init(smbus, 8, NULL, 0);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c5e8367..3cbcbb0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -69,7 +69,6 @@ static void pc_q35_init(MachineState *machine)
> MemoryRegion *ram_memory;
> GSIState *gsi_state;
> ISABus *isa_bus;
> -qemu_irq *gsi;
> qemu_irq *i8259;
> int i;
> ICH9LPCState *ich9_lpc;
> @@ -153,10 +152,10 @@ static void pc_q35_init(MachineState *machine)
> gsi_state = g_malloc0(sizeof(*gsi_state));
> if (kvm_ioapic_in_kernel()) {
> kvm_pc_setup_irq_routing(pcmc->pci_enabled);
> -gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> - GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> + GSI_NUM_PINS);
> } else {
> -gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> }
>
> /* create pci host bus */
> @@ -195,7 +194,7 @@ static void pc_q35_init(MachineState *machine)
> ich9_lpc = ICH9_LPC_DEVICE(lpc);
> lpc_dev = DEVICE(lpc);
> for (i = 0; i < GSI_NUM_PINS; i++) {
> -qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, gsi[i]);
> +qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, pcms->gsi[i]);
> }
> pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc
Re: [Qemu-devel] [PATCH for-2.7 v3 35/36] ahci: fix sglist leak on retry
John, can you review this one?
marcandre.lur...@redhat.com writes:
> From: Marc-André Lureau
>
> ahci-test /x86_64/ahci/io/dma/lba28/retry triggers the following leak:
>
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
> #0 0x7fc4b2a25e20 in malloc (/lib64/libasan.so.3+0xc6e20)
> #1 0x7fc4993bce58 in g_malloc (/lib64/libglib-2.0.so.0+0x4ee58)
> #2 0x556a187d4b34 in ahci_populate_sglist hw/ide/ahci.c:896
> #3 0x556a187d8237 in ahci_dma_prepare_buf hw/ide/ahci.c:1367
> #4 0x556a187b5a1a in ide_dma_cb hw/ide/core.c:844
> #5 0x556a187d7eec in ahci_start_dma hw/ide/ahci.c:1333
> #6 0x556a187b650b in ide_start_dma hw/ide/core.c:921
> #7 0x556a187b61e6 in ide_sector_start_dma hw/ide/core.c:911
> #8 0x556a187b9e26 in cmd_write_dma hw/ide/core.c:1486
> #9 0x556a187bd519 in ide_exec_cmd hw/ide/core.c:2027
> #10 0x556a187d71c5 in handle_reg_h2d_fis hw/ide/ahci.c:1204
> #11 0x556a187d7681 in handle_cmd hw/ide/ahci.c:1254
> #12 0x556a187d168a in check_cmd hw/ide/ahci.c:510
> #13 0x556a187d0afc in ahci_port_write hw/ide/ahci.c:314
> #14 0x556a187d105d in ahci_mem_write hw/ide/ahci.c:435
> #15 0x556a1831d959 in memory_region_write_accessor
> /home/elmarco/src/qemu/memory.c:525
> #16 0x556a1831dc35 in access_with_adjusted_size
> /home/elmarco/src/qemu/memory.c:591
> #17 0x556a18323ce3 in memory_region_dispatch_write
> /home/elmarco/src/qemu/memory.c:1262
> #18 0x556a1828cf67 in address_space_write_continue
> /home/elmarco/src/qemu/exec.c:2578
> #19 0x556a1828d20b in address_space_write
> /home/elmarco/src/qemu/exec.c:2635
> #20 0x556a1828d92b in address_space_rw /home/elmarco/src/qemu/exec.c:2737
> #21 0x556a1828daf7 in cpu_physical_memory_rw
> /home/elmarco/src/qemu/exec.c:2746
> #22 0x556a183068d3 in cpu_physical_memory_write
> /home/elmarco/src/qemu/include/exec/cpu-common.h:72
> #23 0x556a18308194 in qtest_process_command
> /home/elmarco/src/qemu/qtest.c:382
> #24 0x556a1830 in qtest_process_inbuf
> /home/elmarco/src/qemu/qtest.c:573
> #25 0x556a18309a4a in qtest_read /home/elmarco/src/qemu/qtest.c:585
> #26 0x556a18598b85 in qemu_chr_be_write_impl
> /home/elmarco/src/qemu/qemu-char.c:387
> #27 0x556a18598c52 in qemu_chr_be_write
> /home/elmarco/src/qemu/qemu-char.c:399
> #28 0x556a185a2afa in tcp_chr_read /home/elmarco/src/qemu/qemu-char.c:2902
> #29 0x556a18cbaf52 in qio_channel_fd_source_dispatch io/channel-watch.c:84
>
> Follow John Snow recommendation:
> Everywhere else ncq_err is used, it is accompanied by a list cleanup
> except for ncq_cb, which is the case you are fixing here.
>
> Move the sglist destruction inside of ncq_err and then delete it from
> the other two locations to keep it tidy.
>
> Call dma_buf_commit in ide_dma_cb after the early return. Though, this
> is also a little wonky because this routine does more than clear the
> list, but it is at the moment the centralized "we're done with the
> sglist" function and none of the other side effects that occur in
> dma_buf_commit will interfere with the reset that occurs from
> ide_restart_bh, I think
>
> Signed-off-by: Marc-André Lureau
> ---
> hw/ide/ahci.c | 3 +--
> hw/ide/core.c | 1 +
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 6defeed..f3438ad 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -919,6 +919,7 @@ static void ncq_err(NCQTransferState *ncq_tfs)
> ide_state->error = ABRT_ERR;
> ide_state->status = READY_STAT | ERR_STAT;
> ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag);
> +qemu_sglist_destroy(&ncq_tfs->sglist);
> ncq_tfs->used = 0;
> }
>
> @@ -1025,7 +1026,6 @@ static void execute_ncq_command(NCQTransferState
> *ncq_tfs)
> default:
> DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n",
> ncq_tfs->cmd);
> -qemu_sglist_destroy(&ncq_tfs->sglist);
> ncq_err(ncq_tfs);
> }
> }
> @@ -1092,7 +1092,6 @@ static void process_ncq_command(AHCIState *s, int port,
> uint8_t *cmd_fis,
> error_report("ahci: PRDT length for NCQ command (0x%zx) "
> "is smaller than the requested size (0x%zx)",
> ncq_tfs->sglist.size, size);
> -qemu_sglist_destroy(&ncq_tfs->sglist);
> ncq_err(ncq_tfs);
> ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW);
> return;
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index f9c8162..82d7f2a 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -822,6 +822,7 @@ static void ide_dma_cb(void *opaque, int ret)
> return;
> }
> if (ret < 0) {
> +dma_buf_commit(s, 0);
> if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
> s->bus->dma->aiocb = NULL;
> return;
Re: [Qemu-devel] [PATCH for-2.7 v3 32/36] tests: add qtest_add_data_func_full
marcandre.lur...@redhat.com writes:
> From: Marc-André Lureau
>
> Allows one to specify a destroy function for the test data.
>
> Signed-off-by: Marc-André Lureau
> ---
> tests/libqtest.c | 15 +++
> tests/libqtest.h | 7 ++-
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index eb00f13..9e2d0cd 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -758,6 +758,21 @@ void qtest_add_func(const char *str, void (*fn)(void))
> g_free(path);
> }
>
> +void qtest_add_data_func_full(const char *str, void *data,
> + void (*fn)(const void *),
> + GDestroyNotify data_free_func)
> +{
> +gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
> +#if GLIB_CHECK_VERSION(2, 34, 0)
> +g_test_add_data_func_full(path, data, fn, data_free_func);
> +#else
> +/* back-compat casts, remove this once we can require new-enough glib */
> +g_test_add_vtable(path, 0, data, NULL,
> + (GTestFixtureFunc) fn, (GTestFixtureFunc)
> data_free_func);
Umm, are these function casts kosher?
I can't find documentation for g_test_add_vtable(). Got a pointer for
me? Sure GLib 2.22 provides it?
> +#endif
> +g_free(path);
> +}
> +
> void qtest_add_data_func(const char *str, const void *data,
> void (*fn)(const void *))
> {
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 37f37ad..e4c9c39 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -413,15 +413,20 @@ const char *qtest_get_arch(void);
> void qtest_add_func(const char *str, void (*fn)(void));
>
> /**
> - * qtest_add_data_func:
> + * qtest_add_data_func_full:
> * @str: Test case path.
> * @data: Test case data
> * @fn: Test case function
> + * @data_free_func: GDestroyNotify for data
> *
> * Add a GTester testcase with the given name, data and function.
> * The path is prefixed with the architecture under test, as
> * returned by qtest_get_arch().
> */
> +void qtest_add_data_func_full(const char *str, void *data,
> + void (*fn)(const void *),
> + GDestroyNotify data_free_func);
> +
> void qtest_add_data_func(const char *str, const void *data,
> void (*fn)(const void *));
Please keep qtest_add_data_func() documented.
Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist
Hi
- Original Message -
> Marc-André Lureau writes:
>
> > Hi
> >
> > - Original Message -
> >> Copying the QGA maintainer.
> >>
> >> marcandre.lur...@redhat.com writes:
> >>
> >> > From: Marc-André Lureau
> >> >
> >> > Free the list, not just the elements.
> >> >
> >> > Signed-off-by: Marc-André Lureau
> >> > ---
> >> > include/glib-compat.h | 9 +
> >> > qga/main.c| 8 ++--
> >> > 2 files changed, 11 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/include/glib-compat.h b/include/glib-compat.h
> >> > index 01aa7b3..6d643b2 100644
> >> > --- a/include/glib-compat.h
> >> > +++ b/include/glib-compat.h
> >> > @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable
> >> > *hash_table, gpointer key)
> >> > } while (0)
> >> > #endif
> >> >
> >> > +/*
> >> > + * A GFunc function helper freeing the first argument (not part of
> >> > glib)
> >>
> >> Well, it's not part of GLib, because non-obsolete GLib has no use for
> >> it! You'd simply pass g_free directly to a _free_full() function
> >> instead of passing a silly wrapper to a _foreach() function.
> >>
> >> The commit does a bit more than just plug a leak, it also provides a new
> >> helper for general use. Mention in the commit message?
> >>
> >> I wonder how many more of these silly g_free() wrappers we have. A
> >> quick grep reports hits in string-input-visitor.c and
> >> qobject/json-streamer.c. If you add one to a header, you get to hunt
> >> them down :)
> >>
> >> > + */
> >> > +static inline void qemu_g_func_free(gpointer data,
> >> > +gpointer user_data)
> >> > +{
> >> > +g_free(data);
> >> > +}
> >> > +
> >> > #endif
> >> > diff --git a/qga/main.c b/qga/main.c
> >> > index 4c3b2c7..868508b 100644
> >> > --- a/qga/main.c
> >> > +++ b/qga/main.c
> >> > @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
> >> > #ifdef CONFIG_FSFREEZE
> >> > g_free(config->fsfreeze_hook);
> >> > #endif
> >> > +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
> >> > +g_list_free(config->blacklist);
> >>
> >> Modern GLib code doesn't need silly wrappers:
> >>
> >> g_list_free_full(config->blacklist, g_free);
> >>
> >> Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
> >> Please explain this in the commit message.
> >>
> >> Even better, provide a replacement in glib-compat.h #if
> >> !GLIB_CHECK_VERSION(2, 28, 0). Will facilitate ditching the work-around
> >> when we upgrade to 2.28.
> >
> > ok
> >
> >>
> >> > g_free(config);
> >> > }
> >> >
> >> > @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig
> >> > *config)
> >> > return EXIT_SUCCESS;
> >> > }
> >> >
> >> > -static void free_blacklist_entry(gpointer entry, gpointer unused)
> >> > -{
> >> > -g_free(entry);
> >> > -}
> >> > -
> >> > int main(int argc, char **argv)
> >> > {
> >> > int ret = EXIT_SUCCESS;
> >> > @@ -1379,7 +1376,6 @@ end:
> >> > if (s->channel) {
> >> > ga_channel_free(s->channel);
> >> > }
> >> > -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
> >> > g_free(s->pstate_filepath);
> >> > g_free(s->state_filepath_isfrozen);
> >>
> >> If you at least explain why not g_list_free_full() in the commit
> >> message, you can add
> >> Reviewed-by: Markus Armbruster
> >>
> >> But I'd like to encourage you to explore providing a replacement for
> >> g_list_free_full().
> >
> > Such replacement would make use of a (GFunc) cast, glib implementation is
> > calling g_list_foreach (list, (GFunc) free_func, NULL), but Eric didn't
> > want to have such cast in qemu code. He suggested to have the static
> > inline helper in
> > https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06561.html
>
> Pointlessly dirty:
>
> g_list_foreach(list, (GFunc)g_free, NULL)
>
> Trade dirty for cumbersome:
>
> void free_wrapper(gpointer data, gpointer unused)
> {
> g_free(data)
> }
>
> g_list_foreach(list, free_wrapper, NULL);
>
> But this is C. In C, simple things are best done the simple way. Even
> when that means we don't get to show off how amazingly well we've been
> educated on higher order functions:
>
> for (node = list; node; node = next) {
> next = node->next;
> g_free(node);
> }
>
> Simple, stupid, and not dirty.
If only we were paid to write more lines of code ;) Anyway, that's fine by me
(it's work after all, I'll write elegant code for fun time ;)
> Quote: "Note that it is considered perfectly acceptable to access
> list->next directly."
Funny that quote is only in GList, but GSList also documents and exposes the
struct.
Re: [Qemu-devel] [PATCH for-2.7 v3 32/36] tests: add qtest_add_data_func_full
Hi
- Original Message -
> marcandre.lur...@redhat.com writes:
>
> > From: Marc-André Lureau
> >
> > Allows one to specify a destroy function for the test data.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > tests/libqtest.c | 15 +++
> > tests/libqtest.h | 7 ++-
> > 2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/libqtest.c b/tests/libqtest.c
> > index eb00f13..9e2d0cd 100644
> > --- a/tests/libqtest.c
> > +++ b/tests/libqtest.c
> > @@ -758,6 +758,21 @@ void qtest_add_func(const char *str, void (*fn)(void))
> > g_free(path);
> > }
> >
> > +void qtest_add_data_func_full(const char *str, void *data,
> > + void (*fn)(const void *),
> > + GDestroyNotify data_free_func)
> > +{
> > +gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
> > +#if GLIB_CHECK_VERSION(2, 34, 0)
> > +g_test_add_data_func_full(path, data, fn, data_free_func);
> > +#else
> > +/* back-compat casts, remove this once we can require new-enough glib
> > */
> > +g_test_add_vtable(path, 0, data, NULL,
> > + (GTestFixtureFunc) fn, (GTestFixtureFunc)
> > data_free_func);
>
> Umm, are these function casts kosher?
>
> I can't find documentation for g_test_add_vtable(). Got a pointer for
> me? Sure GLib 2.22 provides it?
Yes, https://git.gnome.org/browse/glib/tree/glib/gtestutils.h?h=2.22.0#n214
See also: https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg00073.html
>
> > +#endif
> > +g_free(path);
> > +}
> > +
> > void qtest_add_data_func(const char *str, const void *data,
> > void (*fn)(const void *))
> > {
> > diff --git a/tests/libqtest.h b/tests/libqtest.h
> > index 37f37ad..e4c9c39 100644
> > --- a/tests/libqtest.h
> > +++ b/tests/libqtest.h
> > @@ -413,15 +413,20 @@ const char *qtest_get_arch(void);
> > void qtest_add_func(const char *str, void (*fn)(void));
> >
> > /**
> > - * qtest_add_data_func:
> > + * qtest_add_data_func_full:
> > * @str: Test case path.
> > * @data: Test case data
> > * @fn: Test case function
> > + * @data_free_func: GDestroyNotify for data
> > *
> > * Add a GTester testcase with the given name, data and function.
> > * The path is prefixed with the architecture under test, as
> > * returned by qtest_get_arch().
> > */
> > +void qtest_add_data_func_full(const char *str, void *data,
> > + void (*fn)(const void *),
> > + GDestroyNotify data_free_func);
> > +
> > void qtest_add_data_func(const char *str, const void *data,
> > void (*fn)(const void *));
>
> Please keep qtest_add_data_func() documented.
Ok (I thought it was quite enough based on the _full description)
Re: [Qemu-devel] [PATCH v2] balloon: Fix failure of updating guest memory status
On Wed, Aug 3, 2016 at 9:25 AM, Ladi Prosek wrote:
> On Tue, Aug 2, 2016 at 2:11 AM, Michael S. Tsirkin wrote:
>> On Mon, Aug 01, 2016 at 11:59:31PM +, Li, Liang Z wrote:
>>> > On Wed, Jul 06, 2016 at 12:49:06PM +, Li, Liang Z wrote:
>>> > > > > > > After live migration, 'guest-stats' can't get the expected
>>> > > > > > > memory status in the guest. This issue is caused by commit
>>> > 4eae2a657d.
>>> > > > > > > The value of 's->stats_vq_elem' will be NULL after live
>>> > > > > > > migration, and the check in the function
>>> > > > > > > 'balloon_stats_poll_cb()' will prevent the 'virtio_notify()'
>>> > > > > > > from executing. So guest will not update the memory status.
>>> > > > > > >
>>> > > > > > > Commit 4eae2a657d is doing the right thing, but
>>> > > > > > > 's->stats_vq_elem'
>>> > > > > > > should be treated as part of balloon device state and migrated
>>> > > > > > > to destination if it's not NULL to make everything works well.
>>> > > > > > >
>>> > > > > > > Signed-off-by: Liang Li
>>> > > > > > > Suggested-by: Paolo Bonzini
>>> > > > > > > Cc: Michael S. Tsirkin
>>> > > > > > > Cc: Ladi Prosek
>>> > > > > > > Cc: Paolo Bonzini
>>> > > > > >
>>> > > > > > I agree there's an issue but we don't change versions anymore.
>>> > > > > > Breaking migrations for everyone is also not nice.
>>> > > > > >
>>> > > > > > How about queueing virtio_balloon_receive_stats so it will get
>>> > > > > > invoked when vm starts?
>>> > > > > >
>>> > > > >
>>> > > > > Could you give more explanation about how it works? I can't catch
>>> > > > > you.
>>> > > > >
>>> > > > > Thanks!
>>> > > > > Liang
>>> > > >
>>> > > > virtqueue_discard before migration
>>> > > >
>>> > > > virtio_balloon_receive_stats after migration
>>> > > >
>>> > >
>>> > > Sorry, I still can't catch you. Maybe it's easier for you to submit a
>>> > > patch than writing a lot a words to make me understand your idea.
>>> >
>>> > I'm rather busy now. I might look into it towards end of the month.
>>> >
>>> > > I just don't understand why not to use the version to make things
>>> > > easier, is that not the original intent of version id?
>>> >
>>> > This was the original idea but we stopped using version ids since they
>>> > have
>>> > many shortcomings.
>>> >
>>> > > If we want to extend the device and more states are needed, the idea
>>> > > you suggest can be used as a common solution?
>>> > >
>>> > > Thanks!
>>> > > Liang
>>> >
>>> > The idea is to try to avoid adding more state. that's not always possible
>>> > but in
>>> > this case element was seen but not consumed yet, so it should be possible
>>> > for destination to simply get it from the VQ again.
>>> >
>>> > > > --
>>> > > > MST
>>>
>>> Hi Michel,
>>>
>>> Do you have time for this issue recently?
>>>
>>> Thanks!
>>> Liang
>
> Hi Liang,
>
> I should be able to look into it this week if you help me with testing.
>
> Thanks,
> Ladi
Please try the attached patch. I have tested it with a simple
'migrate' to save the state and then '-incoming' to load it back.
One question for you: is it expected that stats_poll_interval is not
preserved by save/load? I had to explicitly set
guest-stats-polling-interval on the receiving VM to start getting
stats again. It's also the reason why the new
virtio_balloon_receive_stats call is not under if
(balloon_stats_enabled(s)) because this condition always evaluates to
false for me.
Thanks!
Ladi
>> Sorry, doesn't look like I will.
>> Idea is to make sure balloon_stats_poll_cb runs
>> on source. This will set stats_vq_elem to NULL.
>>
>>
>> --
>> MST
From f2f779e12f4aa4d3469d1b44e54484e66f82a2d7 Mon Sep 17 00:00:00 2001
From: Ladi Prosek
Date: Thu, 4 Aug 2016 15:22:05 +0200
Subject: [PATCH] balloon: preserve stats virtqueue state across migrations
Signed-off-by: Ladi Prosek
---
hw/virtio/virtio-balloon.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 5af429a..1293be0 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -396,6 +396,19 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
trace_virtio_balloon_to_target(target, dev->num_pages);
}
+static void virtio_balloon_save(QEMUFile *f, void *opaque, size_t size)
+{
+VirtIOBalloon *s = VIRTIO_BALLOON(opaque);
+
+if (s->stats_vq_elem != NULL) {
+virtqueue_discard(s->svq, s->stats_vq_elem, s->stats_vq_offset);
+g_free(s->stats_vq_elem);
+s->stats_vq_elem = NULL;
+}
+
+virtio_save(VIRTIO_DEVICE(opaque), f);
+}
+
static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f)
{
VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
@@ -417,6 +430,9 @@ static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f,
s->num_pages = qemu_get_be32(f);
s->actual = qemu_get_be32(f);
+/* poll the queue for the element we may have discarded on save */
+virtio_balloon_receive_sta
Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name
Marc-André Lureau writes:
> Hi
>
> On Thu, Aug 4, 2016 at 5:58 PM Markus Armbruster wrote:
>
>> marcandre.lur...@redhat.com writes:
>>
>> > From: Marc-André Lureau
>> >
>> > Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
>> > class base init name generation instead. Get rid of some leaks that way.
>> >
>> > Signed-off-by: Marc-André Lureau
>> > ---
>> > hw/core/machine.c| 1 +
>> > include/hw/boards.h | 2 +-
>> > include/hw/i386/pc.h | 1 -
>> > 3 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/hw/core/machine.c b/hw/core/machine.c
>> > index e5a456f..00fbe3e 100644
>> > --- a/hw/core/machine.c
>> > +++ b/hw/core/machine.c
>> > @@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass *klass,
>> > void *data)
>> > if (mc->compat_props) {
>> > g_array_free(mc->compat_props, true);
>> > }
>> > +g_free(mc->name);
>> > }
>> >
>> > void machine_register_compat_props(MachineState *machine)
>> > diff --git a/include/hw/boards.h b/include/hw/boards.h
>> > index 3e69eca..e46a744 100644
>> > --- a/include/hw/boards.h
>> > +++ b/include/hw/boards.h
>> > @@ -93,7 +93,7 @@ struct MachineClass {
>> > /*< public >*/
>> >
>> > const char *family; /* NULL iff @name identifies a standalone
>> > machtype */
>> > -const char *name;
>> > +char *name;
>> > const char *alias;
>> > const char *desc;
>> >
>> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> > index eb1d414..afd025a 100644
>> > --- a/include/hw/i386/pc.h
>> > +++ b/include/hw/i386/pc.h
>> > @@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
>> > uint64_t *);
>> > { \
>> > MachineClass *mc = MACHINE_CLASS(oc); \
>> > optsfn(mc); \
>> > -mc->name = namestr; \
>> > mc->init = initfn; \
>> > } \
>> > static const TypeInfo pc_machine_type_##suffix = { \
>>
>> I guess there are is at least one assignment to mc->name not visible in
>> this patch that assigns an allocated string, which leaks before the
>> patch. The commit message seems to provide a clue "class base init name
>> generation". I could probably find it with some effort, but patches
>> that take that much work to understand make me grumpy. Please provide
>> another clue :)
>>
>
> Sorry, thanks for reminding me to write better commit messages.
Good commit messages are hard.
> git grep 'mc->name ='
> hw/core/machine.c:mc->name = g_strndup(cname,
Aha: the concrete machine type's init function overwrites the strdup()ed
value set by machine_class_base_init(), leaking it. Your fix removes
the overwrites and adds a free.
As far as I can see, you got all such overwrites.
> Is that better:
>
> Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
> name generation from machine_class_base_init() instead, and free the
> corresponding allocation in machine_class_finalize().
Works for me. Alternatively:
machine_class_base_init() member name is allocated by
machine_class_base_init(), but not freed by machine_class_finalize().
Simply freeing there doesn't work, because DEFINE_PC_MACHINE()
overwrites it with a literal string.
Fix DEFINE_PC_MACHINE() not to overwrite it, and add the missing
free to machine_class_finalize().
Use the one you like better, or mix them up to taste.
Reviewed-by: Markus Armbruster
[Qemu-devel] [Bug 1490611] Re: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the result file, which Microsoft Azure rejects as invalid
On 04.08.2016 [10:49:47 -], Robie Basak wrote: > Uploaded to Xenial, thanks. Am I right in thinking that the new option > force_size is required to be used with the patch to fix the problem? > It's probably worth mentioning this in the Test Case in the bug > description; otherwise it will appear to testers that the fix doesn't > work. Agreed, updated. ** Description changed: [Impact] - * Starting with a raw disk image, using "qemu-img convert" to convert + * Starting with a raw disk image, using "qemu-img convert" to convert from raw to VHD results in the output VHD file's virtual size being aligned to the nearest 516096 bytes (16 heads x 63 sectors per head x 512 bytes per sector), instead of preserving the input file's size as the output VHD's virtual disk size. - * Microsoft Azure requires that disk images (VHDs) submitted for upload + * Microsoft Azure requires that disk images (VHDs) submitted for upload have virtual sizes aligned to a megabyte boundary. (Ex. 4096MB, 4097MB, 4098MB, etc. are OK, 4096.5MB is rejected with an error.) This is reflected in Microsoft's documentation: https://azure.microsoft.com/en- us/documentation/articles/virtual-machines-linux-create-upload-vhd- generic/ - * The fix for this bug is a backport from upstream. + * The fix for this bug is a backport from upstream. http://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb9245c2610932d33ce14 [Test Case] - * This is reproducible with the following set of commands (including + * This is reproducible with the following set of commands (including the Azure command line tools from https://github.com/Azure/azure-xplat- cli). For the following example, I used qemu version 2.2.1: $ dd if=/dev/zero of=source-disk.img bs=1M count=4096 $ stat source-disk.img File: ‘source-disk.img’ Size: 4294967296 Blocks: 798656 IO Block: 4096 regular file Device: fc01h/64513dInode: 13247963Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ smkent) Gid: ( 1000/ smkent) Access: 2015-08-18 09:48:02.613988480 -0700 Modify: 2015-08-18 09:48:02.825985646 -0700 Change: 2015-08-18 09:48:02.825985646 -0700 Birth: - $ qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img dest-disk.vhd $ stat dest-disk.vhd File: ‘dest-disk.vhd’ Size: 4296499712 Blocks: 535216 IO Block: 4096 regular file Device: fc01h/64513dInode: 13247964Links: 1 Access: (0644/-rw-r--r--) Uid: ( 1000/ smkent) Gid: ( 1000/ smkent) Access: 2015-08-18 09:50:22.252077624 -0700 Modify: 2015-08-18 09:49:24.424868868 -0700 Change: 2015-08-18 09:49:24.424868868 -0700 Birth: - $ azure vm image create testimage1 dest-disk.vhd -o linux -l "West US" info:Executing command vm image create + Retrieving storage accounts info:VHD size : 4097 MB info:Uploading 4195800.5 KB Requested:100.0% Completed:100.0% Running: 0 Time: 1m 0s Speed: 6744 KB/s info:https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd was uploaded successfully error: The VHD https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd has an unsupported virtual size of 4296499200 bytes. The size must be a whole number (in MBs). info:Error information has been recorded to /home/smkent/.azure/azure.err error: vm image create command failed - * A fixed qemu-img will not result in an error during azure image - creation. + * A fixed qemu-img will not result in an error during azure image + creation. It will require passing -o force_size, which will leverage the + backported functionality. [Regression Potential] - * The upstream fix introduces a qemu-img option (-o force_size) which + * The upstream fix introduces a qemu-img option (-o force_size) which is unset by default. The regression potential is very low, as a result. ... I also ran the above commands using qemu 2.4.0, which resulted in the same error as the conversion behavior is the same. However, qemu 2.1.1 and earlier (including qemu 2.0.0 installed by Ubuntu 14.04) does not pad the virtual disk size during conversion. Using qemu-img convert from qemu versions <=2.1.1 results in a VHD that is exactly the size of the raw input file plus 512 bytes (for the VHD footer). Those qemu versions do not attempt to realign the disk. As a result, Azure accepts VHD files created using those versions of qemu-img convert for upload. Is there a reason why newer qemu realigns the converted VHD file? It would be useful if an option were added to disable this feature, as current versions of qemu cannot be used to create VHD files for Azure using Microsoft's official instructions. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1490611 Title: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the r
Re: [Qemu-devel] [PATCH v2 for-2.7] Update ancient copyright string in -version output
On 08/04/2016 05:14 AM, Peter Maydell wrote: > Currently the -version command line argument prints a string ending > with "Copyright (c) 2003-2008 Fabrice Bellard". This is now some > eight years out of date; abstract it out of the several places that > print the string and update it to: > > Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers > > to reflect the work by all the QEMU Project contributors over the > last decade. > > Signed-off-by: Peter Maydell > Acked-by: Stefan Hajnoczi > --- Reviewed-by: Eric Blake > The aim here is to (1) update the dates and (2) acknowledge > the work of all our contributors. I'm open to bikeshedding > on the exact wording (or on which header file we should > put the #define in...) > > I only pulled out the copyright string proper into the #define > because a GUI About box is going to want just that, with no > leading ',' or trailing newline. > > Fabrice: I have cc'd you since this is proposing an update > to your copyright info. > > v1->v2 changes: add the qemu-img.c string. > --- > bsd-user/main.c | 3 ++- > include/qemu-common.h | 4 > linux-user/main.c | 2 +- > qemu-img.c| 2 +- > vl.c | 3 ++- > 5 files changed, 10 insertions(+), 4 deletions(-) qemu-io has a --version option that probably ought to output copyright information, but as it currently does not do so, this patch need not be the one that tweaks it. qemu-nbd has a --version option, where the copyright info is currently: Copyright (C) 2006 Anthony Liguori . I don't know if we want to rework that one in light of this patch, or if we can just blindly use this code. Anthony? > +/* Copyright string for -version arguments, About dialogs, etc */ > +#define QEMU_COPYRIGHT "Copyright (c) 2003-2016 " \ > +"Fabrice Bellard and the QEMU Project developers" > + -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-block] [PATCH 1/7] util: Add UUID API
On Tue, Aug 02, 2016 at 05:18:32PM +0800, Fam Zheng wrote:
> A number of different places across the code base use CONFIG_UUID. Some
> of them are soft dependency, some are not built if libuuid is not
> available, some come with dummy fallback, some throws runtime error.
>
> It is hard to maintain, and hard to reason for users.
>
> Since UUID is a simple standard with only a small number of operations,
> it is cleaner to have a central support in libqemuutil. This patch adds
> qemu_uuid_* the functions so that all uuid users in the code base can
> rely on. Except for qemu_uuid_generate which is new code, all other
> functions are just copy from existing fallbacks from other files.
>
> Signed-off-by: Fam Zheng
> ---
> arch_init.c | 19 ---
> block/iscsi.c | 2 +-
> hw/smbios/smbios.c | 1 +
> include/qemu/uuid.h | 37 +
> include/sysemu/sysemu.h | 4
> qmp.c | 1 +
> stubs/uuid.c| 2 +-
> util/Makefile.objs | 1 +
> util/uuid.c | 63
> +
> vl.c| 1 +
> 10 files changed, 106 insertions(+), 25 deletions(-)
> create mode 100644 include/qemu/uuid.h
> create mode 100644 util/uuid.c
>
> diff --git a/arch_init.c b/arch_init.c
> index fa05973..5cc58b2 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -235,25 +235,6 @@ void audio_init(void)
> }
> }
>
> -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> -{
> -int ret;
> -
> -if (strlen(str) != 36) {
> -return -1;
> -}
> -
> -ret = sscanf(str, UUID_FMT, &uuid[0], &uuid[1], &uuid[2], &uuid[3],
> - &uuid[4], &uuid[5], &uuid[6], &uuid[7], &uuid[8], &uuid[9],
> - &uuid[10], &uuid[11], &uuid[12], &uuid[13], &uuid[14],
> - &uuid[15]);
> -
> -if (ret != 16) {
> -return -1;
> -}
> -return 0;
> -}
> -
> void do_acpitable_option(const QemuOpts *opts)
> {
> #ifdef TARGET_I386
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 95ce9e1..961ac76 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -36,7 +36,7 @@
> #include "block/block_int.h"
> #include "block/scsi.h"
> #include "qemu/iov.h"
> -#include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
> #include "qmp-commands.h"
> #include "qapi/qmp/qstring.h"
> #include "crypto/secret.h"
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 74c7102..0705eb1 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -20,6 +20,7 @@
> #include "qemu/config-file.h"
> #include "qemu/error-report.h"
> #include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
> #include "sysemu/cpus.h"
> #include "hw/smbios/smbios.h"
> #include "hw/loader.h"
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> new file mode 100644
> index 000..53d572f
> --- /dev/null
> +++ b/include/qemu/uuid.h
> @@ -0,0 +1,37 @@
> +/*
> + * QEMU UUID functions
> + *
> + * Copyright 2016 Red Hat, Inc.,
> + *
> + * Authors:
> + * Fam Zheng
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#ifndef QEMU_UUID_H
> +#define QEMU_UUID_H
> +
> +#include "qemu-common.h"
> +
> +typedef unsigned char qemu_uuid_t[16];
> +
> +#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
> + "%02hhx%02hhx-%02hhx%02hhx-" \
> + "%02hhx%02hhx-" \
> + "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> +#define UUID_NONE "----"
> +
> +void qemu_uuid_generate(qemu_uuid_t out);
> +
> +int qemu_uuid_is_null(const qemu_uuid_t uu);
> +
> +void qemu_uuid_unparse(const qemu_uuid_t uu, char *out);
> +
> +int qemu_uuid_parse(const char *str, uint8_t *uuid);
> +
> +#endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ee7c760..6111950 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -18,10 +18,6 @@ extern const char *bios_name;
> extern const char *qemu_name;
> extern uint8_t qemu_uuid[];
> extern bool qemu_uuid_set;
> -int qemu_uuid_parse(const char *str, uint8_t *uuid);
> -
> -#define UUID_FMT
> "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> -#define UUID_NONE "----"
>
> bool runstate_check(RunState state);
> void runstate_set(RunState new_state);
> diff --git a/qmp.c b/qmp.c
> index b6d531e..7fbde29 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -35,6 +35,7 @@
> #include "qom/object_interfaces.h"
> #include "hw/mem/pc-dimm.h"
> #include "hw/acpi/acpi_dev_interface.h"
> +#include "qemu/uuid.h"
>
> NameInfo *qmp_query_name(Error **errp)
> {
> diff --git a/stubs/uuid.c b/stubs/uuid.c
> index 92ad717..a880de8 10064
Re: [Qemu-devel] [PATCH for-2.7 v2] block/qdev: Let 'drive' property fall back to node name
On 08/04/2016 06:09 AM, Kevin Wolf wrote: > If a qdev block device is created with an anonymous BlockBackend (i.e. > a node name rather than a BB name was given for the drive property), > qdev used to return an empty string when the property was read. This > patch fixes it to return the node name instead. > > Signed-off-by: Kevin Wolf > --- > v2: > - Check whether the BB has even a BDS inserted. Fixes a segfault with > empty anonymous BlockBackends. > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/7] util: Add UUID API
On Tue, Aug 02, 2016 at 05:18:32PM +0800, Fam Zheng wrote: > A number of different places across the code base use CONFIG_UUID. Some > of them are soft dependency, some are not built if libuuid is not > available, some come with dummy fallback, some throws runtime error. > > It is hard to maintain, and hard to reason for users. > > Since UUID is a simple standard with only a small number of operations, > it is cleaner to have a central support in libqemuutil. This patch adds > qemu_uuid_* the functions so that all uuid users in the code base can > rely on. Except for qemu_uuid_generate which is new code, all other > functions are just copy from existing fallbacks from other files. > > Signed-off-by: Fam Zheng > --- > arch_init.c | 19 --- > block/iscsi.c | 2 +- > hw/smbios/smbios.c | 1 + > include/qemu/uuid.h | 37 + > include/sysemu/sysemu.h | 4 > qmp.c | 1 + > stubs/uuid.c| 2 +- > util/Makefile.objs | 1 + > util/uuid.c | 63 > + > vl.c| 1 + > 10 files changed, 106 insertions(+), 25 deletions(-) > create mode 100644 include/qemu/uuid.h > create mode 100644 util/uuid.c It would be nice to see you add a tests/test-uuid.c unit test to exercise all the new utility APIs you're adding & check their corner cases. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive
On 08/01/2016 01:47 PM, Kevin Wolf wrote:
> Am 14.07.2016 um 21:48 hat Eric Blake geschrieben:
>> On 07/14/2016 07:49 AM, Kevin Wolf wrote:
>>> This allows to create an empty ide-cd device without manually creating a
>>> BlockBackend.
>>>
>>> Signed-off-by: Kevin Wolf
>>> ---
>>> hw/ide/qdev.c | 20 +++-
>>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>>> @@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind
>>> kind)
>>> IDEState *s = bus->ifs + dev->unit;
>>> Error *err = NULL;
>>>
>>> +if (!dev->conf.blk) {
>>> +if (kind != IDE_CD) {
>>> +error_report("No drive specified");
>>> +return -1;
>>> +} else {
>>> +/* Anonymous BlockBackend for an empty drive */
>>> +dev->conf.blk = blk_new();
>>
>> So we either fail or dev->conf.blk is set...
>>
>>> +}
>>> +}
>>> +
>>> if (dev->conf.discard_granularity == -1) {
>>> dev->conf.discard_granularity = 512;
>>> } else if (dev->conf.discard_granularity &&
>>> @@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
>>>
>>> static int ide_drive_initfn(IDEDevice *dev)
>>> {
>>> -DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
>>> +DriveInfo *dinfo = NULL;
>>> +
>>> +if (dev->conf.blk) {
>>> +dinfo = blk_legacy_dinfo(dev->conf.blk);
>>> +}
>>>
>>> return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
>>
>> ...yet, this claims dev->conf.blk can be NULL. What am I missing?
>
> That ide_drive_initfn() is the outer function and runs first, before we
> handle the case in ide_dev_initfn()?
Ah, I think I glazed over the difference between 'dev' and 'drive', and
was thinking 'both of these are initfn, what's different?'. I think you
are okay, after all.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2] balloon: Fix failure of updating guest memory status
On Thu, Aug 04, 2016 at 05:14:14PM +0200, Ladi Prosek wrote:
> On Wed, Aug 3, 2016 at 9:25 AM, Ladi Prosek wrote:
> > On Tue, Aug 2, 2016 at 2:11 AM, Michael S. Tsirkin wrote:
> >> On Mon, Aug 01, 2016 at 11:59:31PM +, Li, Liang Z wrote:
> >>> > On Wed, Jul 06, 2016 at 12:49:06PM +, Li, Liang Z wrote:
> >>> > > > > > > After live migration, 'guest-stats' can't get the expected
> >>> > > > > > > memory status in the guest. This issue is caused by commit
> >>> > 4eae2a657d.
> >>> > > > > > > The value of 's->stats_vq_elem' will be NULL after live
> >>> > > > > > > migration, and the check in the function
> >>> > > > > > > 'balloon_stats_poll_cb()' will prevent the 'virtio_notify()'
> >>> > > > > > > from executing. So guest will not update the memory status.
> >>> > > > > > >
> >>> > > > > > > Commit 4eae2a657d is doing the right thing, but
> >>> > > > > > > 's->stats_vq_elem'
> >>> > > > > > > should be treated as part of balloon device state and migrated
> >>> > > > > > > to destination if it's not NULL to make everything works well.
> >>> > > > > > >
> >>> > > > > > > Signed-off-by: Liang Li
> >>> > > > > > > Suggested-by: Paolo Bonzini
> >>> > > > > > > Cc: Michael S. Tsirkin
> >>> > > > > > > Cc: Ladi Prosek
> >>> > > > > > > Cc: Paolo Bonzini
> >>> > > > > >
> >>> > > > > > I agree there's an issue but we don't change versions anymore.
> >>> > > > > > Breaking migrations for everyone is also not nice.
> >>> > > > > >
> >>> > > > > > How about queueing virtio_balloon_receive_stats so it will get
> >>> > > > > > invoked when vm starts?
> >>> > > > > >
> >>> > > > >
> >>> > > > > Could you give more explanation about how it works? I can't
> >>> > > > > catch you.
> >>> > > > >
> >>> > > > > Thanks!
> >>> > > > > Liang
> >>> > > >
> >>> > > > virtqueue_discard before migration
> >>> > > >
> >>> > > > virtio_balloon_receive_stats after migration
> >>> > > >
> >>> > >
> >>> > > Sorry, I still can't catch you. Maybe it's easier for you to submit a
> >>> > > patch than writing a lot a words to make me understand your idea.
> >>> >
> >>> > I'm rather busy now. I might look into it towards end of the month.
> >>> >
> >>> > > I just don't understand why not to use the version to make things
> >>> > > easier, is that not the original intent of version id?
> >>> >
> >>> > This was the original idea but we stopped using version ids since they
> >>> > have
> >>> > many shortcomings.
> >>> >
> >>> > > If we want to extend the device and more states are needed, the idea
> >>> > > you suggest can be used as a common solution?
> >>> > >
> >>> > > Thanks!
> >>> > > Liang
> >>> >
> >>> > The idea is to try to avoid adding more state. that's not always
> >>> > possible but in
> >>> > this case element was seen but not consumed yet, so it should be
> >>> > possible
> >>> > for destination to simply get it from the VQ again.
> >>> >
> >>> > > > --
> >>> > > > MST
> >>>
> >>> Hi Michel,
> >>>
> >>> Do you have time for this issue recently?
> >>>
> >>> Thanks!
> >>> Liang
> >
> > Hi Liang,
> >
> > I should be able to look into it this week if you help me with testing.
> >
> > Thanks,
> > Ladi
>
> Please try the attached patch. I have tested it with a simple
> 'migrate' to save the state and then '-incoming' to load it back.
>
> One question for you: is it expected that stats_poll_interval is not
> preserved by save/load? I had to explicitly set
> guest-stats-polling-interval on the receiving VM to start getting
> stats again. It's also the reason why the new
> virtio_balloon_receive_stats call is not under if
> (balloon_stats_enabled(s)) because this condition always evaluates to
> false for me.
>
> Thanks!
> Ladi
>
> >> Sorry, doesn't look like I will.
> >> Idea is to make sure balloon_stats_poll_cb runs
> >> on source. This will set stats_vq_elem to NULL.
> >>
> >>
> >> --
> >> MST
> From f2f779e12f4aa4d3469d1b44e54484e66f82a2d7 Mon Sep 17 00:00:00 2001
> From: Ladi Prosek
> Date: Thu, 4 Aug 2016 15:22:05 +0200
> Subject: [PATCH] balloon: preserve stats virtqueue state across migrations
>
> Signed-off-by: Ladi Prosek
> ---
> hw/virtio/virtio-balloon.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 5af429a..1293be0 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -396,6 +396,19 @@ static void virtio_balloon_to_target(void *opaque,
> ram_addr_t target)
> trace_virtio_balloon_to_target(target, dev->num_pages);
> }
>
> +static void virtio_balloon_save(QEMUFile *f, void *opaque, size_t size)
> +{
> +VirtIOBalloon *s = VIRTIO_BALLOON(opaque);
> +
> +if (s->stats_vq_elem != NULL) {
> +virtqueue_discard(s->svq, s->stats_vq_elem, s->stats_vq_offset);
> +g_free(s->stats_vq_elem);
> +s->stats_vq_elem = NULL;
> +}
> +
> +virtio_save(VIRTIO_DEVICE(opaque), f);
> +}
> +
> static void virtio_balloon_save_device(VirtIODev
[Qemu-devel] [PATCH v4 for-2.7 0/7] Fixing i686 host / sparc64 guest crash
This is a revision of my "third" attempt, tweaked a bit for Aurelien's review. I've sort of lost track of where we are with the release process, so I'll understand if we've gone too far now. r~ Richard Henderson (7): tcg: Compress liveness data to 16 bits tcg: Reorg TCGOp chaining tcg: Fold life data into TCGOp tcg: Compress dead_temps and mem_temps into a single array tcg: Include liveness info in the dumps tcg: Require liveness analysis tcg: Lower indirect registers in a separate pass include/exec/gen-icount.h | 2 +- include/qemu/log.h| 3 +- tcg/optimize.c| 37 +-- tcg/tcg-op.c | 2 +- tcg/tcg.c | 588 ++ tcg/tcg.h | 52 ++-- util/log.c| 24 +- 7 files changed, 441 insertions(+), 267 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH v4 for-2.7 1/7] tcg: Compress liveness data to 16 bits
This reduces both memory usage and per-insn cacheline usage
during code generation.
Reviewed-by: Aurelien Jarno
Signed-off-by: Richard Henderson
---
tcg/tcg.c | 58 ++
tcg/tcg.h | 16 ++--
2 files changed, 32 insertions(+), 42 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0c46c43..4aa1933 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1341,7 +1341,7 @@ static inline void tcg_la_bb_end(TCGContext *s, uint8_t
*dead_temps,
}
}
-/* Liveness analysis : update the opc_dead_args array to tell if a
+/* Liveness analysis : update the opc_arg_life array to tell if a
given input arguments is dead. Instructions updating dead
temporaries are removed. */
static void tcg_liveness_analysis(TCGContext *s)
@@ -1350,9 +1350,8 @@ static void tcg_liveness_analysis(TCGContext *s)
int oi, oi_prev, nb_ops;
nb_ops = s->gen_next_op_idx;
-s->op_dead_args = tcg_malloc(nb_ops * sizeof(uint16_t));
-s->op_sync_args = tcg_malloc(nb_ops * sizeof(uint8_t));
-
+s->op_arg_life = tcg_malloc(nb_ops * sizeof(TCGLifeData));
+
dead_temps = tcg_malloc(s->nb_temps);
mem_temps = tcg_malloc(s->nb_temps);
tcg_la_func_end(s, dead_temps, mem_temps);
@@ -1361,8 +1360,7 @@ static void tcg_liveness_analysis(TCGContext *s)
int i, nb_iargs, nb_oargs;
TCGOpcode opc_new, opc_new2;
bool have_opc_new2;
-uint16_t dead_args;
-uint8_t sync_args;
+TCGLifeData arg_life = 0;
TCGArg arg;
TCGOp * const op = &s->gen_op_buf[oi];
@@ -1394,15 +1392,13 @@ static void tcg_liveness_analysis(TCGContext *s)
do_not_remove_call:
/* output args are dead */
-dead_args = 0;
-sync_args = 0;
for (i = 0; i < nb_oargs; i++) {
arg = args[i];
if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
}
if (mem_temps[arg]) {
-sync_args |= (1 << i);
+arg_life |= SYNC_ARG << i;
}
dead_temps[arg] = 1;
mem_temps[arg] = 0;
@@ -1423,7 +1419,7 @@ static void tcg_liveness_analysis(TCGContext *s)
arg = args[i];
if (arg != TCG_CALL_DUMMY_ARG) {
if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
}
}
}
@@ -1432,8 +1428,6 @@ static void tcg_liveness_analysis(TCGContext *s)
arg = args[i];
dead_temps[arg] = 0;
}
-s->op_dead_args[oi] = dead_args;
-s->op_sync_args[oi] = sync_args;
}
}
break;
@@ -1544,15 +1538,13 @@ static void tcg_liveness_analysis(TCGContext *s)
} else {
do_not_remove:
/* output args are dead */
-dead_args = 0;
-sync_args = 0;
for (i = 0; i < nb_oargs; i++) {
arg = args[i];
if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
}
if (mem_temps[arg]) {
-sync_args |= (1 << i);
+arg_life |= SYNC_ARG << i;
}
dead_temps[arg] = 1;
mem_temps[arg] = 0;
@@ -1570,7 +1562,7 @@ static void tcg_liveness_analysis(TCGContext *s)
for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
arg = args[i];
if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
}
}
/* input arguments are live for preceding opcodes */
@@ -1578,11 +1570,10 @@ static void tcg_liveness_analysis(TCGContext *s)
arg = args[i];
dead_temps[arg] = 0;
}
-s->op_dead_args[oi] = dead_args;
-s->op_sync_args[oi] = sync_args;
}
break;
}
+s->op_arg_life[oi] = arg_life;
}
}
#else
@@ -1921,11 +1912,11 @@ static void tcg_reg_alloc_bb_end(TCGContext *s,
TCGRegSet allocated_regs)
save_globals(s, allocated_regs);
}
-#define IS_DEAD_ARG(n) ((dead_args >> (n)) & 1)
-#define NEED_SYNC_ARG(n) ((sync_args >> (n)) & 1)
+#define IS_DEAD_ARG(n) (arg_life & (DEAD_ARG << (n)))
+#define NEED_SYNC_ARG(n)
[Qemu-devel] [PATCH v4 for-2.7 2/7] tcg: Reorg TCGOp chaining
Instead of using -1 as end of chain, use 0, and link through the 0
entry as a fully circular double-linked list.
Signed-off-by: Richard Henderson
---
include/exec/gen-icount.h | 2 +-
tcg/optimize.c| 8 ++--
tcg/tcg-op.c | 2 +-
tcg/tcg.c | 35 +++
tcg/tcg.h | 22 --
5 files changed, 31 insertions(+), 38 deletions(-)
diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index 1af03d8..050de59 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -59,7 +59,7 @@ static void gen_tb_end(TranslationBlock *tb, int num_insns)
}
/* Terminate the linked list. */
-tcg_ctx.gen_op_buf[tcg_ctx.gen_last_op_idx].next = -1;
+tcg_ctx.gen_op_buf[tcg_ctx.gen_op_buf[0].prev].next = 0;
}
static inline void gen_io_start(void)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index c0d975b..8df7fc7 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -103,11 +103,7 @@ static TCGOp *insert_op_before(TCGContext *s, TCGOp
*old_op,
.prev = prev,
.next = next
};
-if (prev >= 0) {
-s->gen_op_buf[prev].next = oi;
-} else {
-s->gen_first_op_idx = oi;
-}
+s->gen_op_buf[prev].next = oi;
old_op->prev = oi;
return new_op;
@@ -583,7 +579,7 @@ void tcg_optimize(TCGContext *s)
nb_globals = s->nb_globals;
reset_all_temps(nb_temps);
-for (oi = s->gen_first_op_idx; oi >= 0; oi = oi_next) {
+for (oi = s->gen_op_buf[0].next; oi != 0; oi = oi_next) {
tcg_target_ulong mask, partmask, affected;
int nb_oargs, nb_iargs, i;
TCGArg tmp;
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 293b854..0243c99 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -52,7 +52,7 @@ static void tcg_emit_op(TCGContext *ctx, TCGOpcode opc, int
args)
int pi = oi - 1;
tcg_debug_assert(oi < OPC_BUF_SIZE);
-ctx->gen_last_op_idx = oi;
+ctx->gen_op_buf[0].prev = oi;
ctx->gen_next_op_idx = ni;
ctx->gen_op_buf[oi] = (TCGOp){
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 4aa1933..cd76e42 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -438,9 +438,9 @@ void tcg_func_start(TCGContext *s)
s->goto_tb_issue_mask = 0;
#endif
-s->gen_first_op_idx = 0;
-s->gen_last_op_idx = -1;
-s->gen_next_op_idx = 0;
+s->gen_op_buf[0].next = 1;
+s->gen_op_buf[0].prev = 0;
+s->gen_next_op_idx = 1;
s->gen_next_parm_idx = 0;
s->be = tcg_malloc(sizeof(TCGBackendData));
@@ -869,7 +869,7 @@ void tcg_gen_callN(TCGContext *s, void *func, TCGArg ret,
/* Make sure the calli field didn't overflow. */
tcg_debug_assert(s->gen_op_buf[i].calli == real_args);
-s->gen_last_op_idx = i;
+s->gen_op_buf[0].prev = i;
s->gen_next_op_idx = i + 1;
s->gen_next_parm_idx = pi;
@@ -1021,7 +1021,7 @@ void tcg_dump_ops(TCGContext *s)
TCGOp *op;
int oi;
-for (oi = s->gen_first_op_idx; oi >= 0; oi = op->next) {
+for (oi = s->gen_op_buf[0].next; oi != 0; oi = op->next) {
int i, k, nb_oargs, nb_iargs, nb_cargs;
const TCGOpDef *def;
const TCGArg *args;
@@ -1033,7 +1033,7 @@ void tcg_dump_ops(TCGContext *s)
args = &s->gen_opparam_buf[op->args];
if (c == INDEX_op_insn_start) {
-qemu_log("%s ", oi != s->gen_first_op_idx ? "\n" : "");
+qemu_log("%s ", oi != s->gen_op_buf[0].next ? "\n" : "");
for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
target_ulong a;
@@ -1298,18 +1298,13 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
int next = op->next;
int prev = op->prev;
-if (next >= 0) {
-s->gen_op_buf[next].prev = prev;
-} else {
-s->gen_last_op_idx = prev;
-}
-if (prev >= 0) {
-s->gen_op_buf[prev].next = next;
-} else {
-s->gen_first_op_idx = next;
-}
+/* We should never attempt to remove the list terminator. */
+tcg_debug_assert(op != &s->gen_op_buf[0]);
+
+s->gen_op_buf[next].prev = prev;
+s->gen_op_buf[prev].next = next;
-memset(op, -1, sizeof(*op));
+memset(op, 0, sizeof(*op));
#ifdef CONFIG_PROFILER
s->del_op_count++;
@@ -1356,7 +1351,7 @@ static void tcg_liveness_analysis(TCGContext *s)
mem_temps = tcg_malloc(s->nb_temps);
tcg_la_func_end(s, dead_temps, mem_temps);
-for (oi = s->gen_last_op_idx; oi >= 0; oi = oi_prev) {
+for (oi = s->gen_op_buf[0].prev; oi != 0; oi = oi_prev) {
int i, nb_iargs, nb_oargs;
TCGOpcode opc_new, opc_new2;
bool have_opc_new2;
@@ -2351,7 +2346,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
{
int n;
-n = s->gen_last_op_idx + 1;
+n = s->gen_op_buf[0].prev + 1;
s->op_count += n;
if (n > s->op_count_max) {
s->op_count_max = n;
@@ -2410,7 +2405,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *
[Qemu-devel] [PATCH v4 for-2.7 3/7] tcg: Fold life data into TCGOp
Reduce the size of other bitfields to make room.
This reduces the cache footprint of compilation.
Reviewed-by: Aurelien Jarno
Signed-off-by: Richard Henderson
---
tcg/tcg.c | 9 +++--
tcg/tcg.h | 26 ++
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index cd76e42..6bcf6e5 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1342,10 +1342,7 @@ static inline void tcg_la_bb_end(TCGContext *s, uint8_t
*dead_temps,
static void tcg_liveness_analysis(TCGContext *s)
{
uint8_t *dead_temps, *mem_temps;
-int oi, oi_prev, nb_ops;
-
-nb_ops = s->gen_next_op_idx;
-s->op_arg_life = tcg_malloc(nb_ops * sizeof(TCGLifeData));
+int oi, oi_prev;
dead_temps = tcg_malloc(s->nb_temps);
mem_temps = tcg_malloc(s->nb_temps);
@@ -1568,7 +1565,7 @@ static void tcg_liveness_analysis(TCGContext *s)
}
break;
}
-s->op_arg_life[oi] = arg_life;
+op->life = arg_life;
}
}
#else
@@ -2410,7 +2407,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
TCGArg * const args = &s->gen_opparam_buf[op->args];
TCGOpcode opc = op->opc;
const TCGOpDef *def = &tcg_op_defs[opc];
-TCGLifeData arg_life = s->op_arg_life[oi];
+TCGLifeData arg_life = op->life;
oi_next = op->next;
#ifdef CONFIG_PROFILER
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 007d7bc..ebf6867 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -583,25 +583,30 @@ typedef struct TCGTempSet {
#define SYNC_ARG 1
typedef uint16_t TCGLifeData;
+/* The layout here is designed to avoid crossing of a 32-bit boundary.
+ If we do so, gcc adds padding, expanding the size to 12. */
typedef struct TCGOp {
-TCGOpcode opc : 8;
+TCGOpcode opc : 8;/* 8 */
+
+/* Index of the prev/next op, or 0 for the end of the list. */
+unsigned prev : 10; /* 18 */
+unsigned next : 10; /* 28 */
/* The number of out and in parameter for a call. */
-unsigned callo : 2;
-unsigned calli : 6;
+unsigned calli : 4;/* 32 */
+unsigned callo : 2;/* 34 */
/* Index of the arguments for this op, or 0 for zero-operand ops. */
-unsigned args : 16;
+unsigned args : 14; /* 48 */
-/* Index of the prev/next op, or 0 for the end of the list. */
-unsigned prev : 16;
-unsigned next : 16;
+/* Lifetime data of the operands. */
+unsigned life : 16; /* 64 */
} TCGOp;
/* Make sure operands fit in the bitfields above. */
QEMU_BUILD_BUG_ON(NB_OPS > (1 << 8));
-QEMU_BUILD_BUG_ON(OPC_BUF_SIZE > (1 << 16));
-QEMU_BUILD_BUG_ON(OPPARAM_BUF_SIZE > (1 << 16));
+QEMU_BUILD_BUG_ON(OPC_BUF_SIZE > (1 << 10));
+QEMU_BUILD_BUG_ON(OPPARAM_BUF_SIZE > (1 << 14));
/* Make sure that we don't overflow 64 bits without noticing. */
QEMU_BUILD_BUG_ON(sizeof(TCGOp) > 8);
@@ -619,9 +624,6 @@ struct TCGContext {
uint16_t *tb_jmp_insn_offset; /* tb->jmp_insn_offset if USE_DIRECT_JUMP */
uintptr_t *tb_jmp_target_addr; /* tb->jmp_target_addr if !USE_DIRECT_JUMP
*/
-/* liveness analysis */
-TCGLifeData *op_arg_life;
-
TCGRegSet reserved_regs;
intptr_t current_frame_offset;
intptr_t frame_start;
--
2.7.4
[Qemu-devel] [PATCH v4 for-2.7 6/7] tcg: Require liveness analysis
Reviewed-by: Aurelien Jarno
Signed-off-by: Richard Henderson
---
tcg/tcg.c | 21 -
1 file changed, 21 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index b0a88ba..3c1f526 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -23,7 +23,6 @@
*/
/* define it to use liveness analysis (better code) */
-#define USE_LIVENESS_ANALYSIS
#define USE_TCG_OPTIMIZATIONS
#include "qemu/osdep.h"
@@ -1337,8 +1336,6 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
#endif
}
-#ifdef USE_LIVENESS_ANALYSIS
-
#define TS_DEAD 1
#define TS_MEM 2
@@ -1595,18 +1592,6 @@ static void tcg_liveness_analysis(TCGContext *s)
op->life = arg_life;
}
}
-#else
-/* dummy liveness analysis */
-static void tcg_liveness_analysis(TCGContext *s)
-{
-int nb_ops = s->gen_next_op_idx;
-
-s->op_dead_args = tcg_malloc(nb_ops * sizeof(uint16_t));
-memset(s->op_dead_args, 0, nb_ops * sizeof(uint16_t));
-s->op_sync_args = tcg_malloc(nb_ops * sizeof(uint8_t));
-memset(s->op_sync_args, 0, nb_ops * sizeof(uint8_t));
-}
-#endif
#ifdef CONFIG_DEBUG_TCG
static void dump_regs(TCGContext *s)
@@ -1858,7 +1843,6 @@ static void temp_load(TCGContext *s, TCGTemp *ts,
TCGRegSet desired_regs,
temporary registers needs to be allocated to store a constant. */
static void temp_save(TCGContext *s, TCGTemp *ts, TCGRegSet allocated_regs)
{
-#ifdef USE_LIVENESS_ANALYSIS
/* ??? Liveness does not yet incorporate indirect bases. */
if (!ts->indirect_base) {
/* The liveness analysis already ensures that globals are back
@@ -1866,7 +1850,6 @@ static void temp_save(TCGContext *s, TCGTemp *ts,
TCGRegSet allocated_regs)
tcg_debug_assert(ts->val_type == TEMP_VAL_MEM || ts->fixed_reg);
return;
}
-#endif
temp_sync(s, ts, allocated_regs, 1);
}
@@ -1891,7 +1874,6 @@ static void sync_globals(TCGContext *s, TCGRegSet
allocated_regs)
for (i = 0; i < s->nb_globals; i++) {
TCGTemp *ts = &s->temps[i];
-#ifdef USE_LIVENESS_ANALYSIS
/* ??? Liveness does not yet incorporate indirect bases. */
if (!ts->indirect_base) {
tcg_debug_assert(ts->val_type != TEMP_VAL_REG
@@ -1899,7 +1881,6 @@ static void sync_globals(TCGContext *s, TCGRegSet
allocated_regs)
|| ts->mem_coherent);
continue;
}
-#endif
temp_sync(s, ts, allocated_regs, 0);
}
}
@@ -1915,7 +1896,6 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet
allocated_regs)
if (ts->temp_local) {
temp_save(s, ts, allocated_regs);
} else {
-#ifdef USE_LIVENESS_ANALYSIS
/* ??? Liveness does not yet incorporate indirect bases. */
if (!ts->indirect_base) {
/* The liveness analysis already ensures that temps are dead.
@@ -1923,7 +1903,6 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet
allocated_regs)
tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
continue;
}
-#endif
temp_dead(s, ts);
}
}
--
2.7.4
