Re: [Qemu-devel] [PATCH] qemu-thread: Assert locks are initialized before using
On Tue, Jul 4, 2017 at 9:23 AM, Fam Zheng wrote:
> Not all platforms check whether a lock is initialized before used. In
> particular Linux seems to be more permissive than OSX.
>
> Check initialization state explicitly in our code to catch such bugs
> earlier.
>
> Signed-off-by: Fam Zheng
Reviewed-by: Philippe Mathieu-Daudé
> ---
> include/qemu/thread-posix.h | 4
> include/qemu/thread-win32.h | 5 +
> util/qemu-thread-posix.c| 27 +++
> util/qemu-thread-win32.c| 34 +-
> 4 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
> index 09d1e15..e5e3a0f 100644
> --- a/include/qemu/thread-posix.h
> +++ b/include/qemu/thread-posix.h
> @@ -12,10 +12,12 @@ typedef QemuMutex QemuRecMutex;
>
> struct QemuMutex {
> pthread_mutex_t lock;
> +bool initialized;
> };
>
> struct QemuCond {
> pthread_cond_t cond;
> +bool initialized;
> };
>
> struct QemuSemaphore {
> @@ -26,6 +28,7 @@ struct QemuSemaphore {
> #else
> sem_t sem;
> #endif
> +bool initialized;
> };
>
> struct QemuEvent {
> @@ -34,6 +37,7 @@ struct QemuEvent {
> pthread_cond_t cond;
> #endif
> unsigned value;
> +bool initialized;
> };
>
> struct QemuThread {
> diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
> index 4c4a261..3a05e3b 100644
> --- a/include/qemu/thread-win32.h
> +++ b/include/qemu/thread-win32.h
> @@ -5,11 +5,13 @@
>
> struct QemuMutex {
> SRWLOCK lock;
> +bool initialized;
> };
>
> typedef struct QemuRecMutex QemuRecMutex;
> struct QemuRecMutex {
> CRITICAL_SECTION lock;
> +bool initialized;
> };
>
> void qemu_rec_mutex_destroy(QemuRecMutex *mutex);
> @@ -19,15 +21,18 @@ void qemu_rec_mutex_unlock(QemuRecMutex *mutex);
>
> struct QemuCond {
> CONDITION_VARIABLE var;
> +bool initialized;
> };
>
> struct QemuSemaphore {
> HANDLE sema;
> +bool initialized;
> };
>
> struct QemuEvent {
> int value;
> HANDLE event;
> +bool initialized;
> };
>
> typedef struct QemuThreadData QemuThreadData;
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index eacd99e..4e95d27 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -43,12 +43,15 @@ void qemu_mutex_init(QemuMutex *mutex)
> err = pthread_mutex_init(&mutex->lock, NULL);
> if (err)
> error_exit(err, __func__);
> +mutex->initialized = true;
> }
>
> void qemu_mutex_destroy(QemuMutex *mutex)
> {
> int err;
>
> +assert(mutex->initialized);
> +mutex->initialized = false;
> err = pthread_mutex_destroy(&mutex->lock);
> if (err)
> error_exit(err, __func__);
> @@ -58,6 +61,7 @@ void qemu_mutex_lock(QemuMutex *mutex)
> {
> int err;
>
> +assert(mutex->initialized);
> err = pthread_mutex_lock(&mutex->lock);
> if (err)
> error_exit(err, __func__);
> @@ -69,6 +73,7 @@ int qemu_mutex_trylock(QemuMutex *mutex)
> {
> int err;
>
> +assert(mutex->initialized);
> err = pthread_mutex_trylock(&mutex->lock);
> if (err == 0) {
> trace_qemu_mutex_locked(mutex);
> @@ -84,6 +89,7 @@ void qemu_mutex_unlock(QemuMutex *mutex)
> {
> int err;
>
> +assert(mutex->initialized);
> trace_qemu_mutex_unlocked(mutex);
> err = pthread_mutex_unlock(&mutex->lock);
> if (err)
> @@ -102,6 +108,7 @@ void qemu_rec_mutex_init(QemuRecMutex *mutex)
> if (err) {
> error_exit(err, __func__);
> }
> +mutex->initialized = true;
> }
>
> void qemu_cond_init(QemuCond *cond)
> @@ -111,12 +118,15 @@ void qemu_cond_init(QemuCond *cond)
> err = pthread_cond_init(&cond->cond, NULL);
> if (err)
> error_exit(err, __func__);
> +cond->initialized = true;
> }
>
> void qemu_cond_destroy(QemuCond *cond)
> {
> int err;
>
> +assert(cond->initialized);
> +cond->initialized = false;
> err = pthread_cond_destroy(&cond->cond);
> if (err)
> error_exit(err, __func__);
> @@ -126,6 +136,7 @@ void qemu_cond_signal(QemuCond *cond)
> {
> int err;
>
> +assert(cond->initialized);
> err = pthread_cond_signal(&cond->cond);
> if (err)
> error_exit(err, __func__);
> @@ -135,6 +146,7 @@ void qemu_cond_broadcast(QemuCond *cond)
> {
> int err;
>
> +assert(cond->initialized);
> err = pthread_cond_broadcast(&cond->cond);
> if (err)
> error_exit(err, __func__);
> @@ -144,6 +156,7 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
> {
> int err;
>
> +assert(cond->initialized);
> trace_qemu_mutex_unlocked(mutex);
> err = pthread_cond_wait(&cond->cond, &mutex->lock);
> trace_qemu_mutex_locked(mutex);
> @@ -174,12 +187,15 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
> error_exit(errno, __func__);
> }
> #endif
> +sem->initialized = true;
> }
>
[Qemu-devel] [PATCH v4 0/1] virtio-scsi-ccw: fix iotest 068 for s390x
This commit fixes iotest 068 for s390x as s390x uses virtio-scsi-ccw. It's based on commit c324fd0a39c by Stefan Hajnoczi. Thanks! Change history: v4: Got Cornelia Huck's Reviewed-by and take the comment to change the commit message. v3: Take Christian Borntraeger and Cornelia Huck's comment to check if kvm is enabled in s390_assign_subch_ioeventfd instead of kvm_s390_assign_subch_ioeventfd to as the former is a general one. v2: Remove Stefan from sign-off list and change the patch's commit message according to Christian Borntraeger's comment. QingFeng Hao (1): virtio-scsi-ccw: use ioeventfd even when KVM is disabled hw/s390x/virtio-ccw.c | 2 +- target/s390x/cpu.h| 6 +- 2 files changed, 6 insertions(+), 2 deletions(-) -- 2.11.2
[Qemu-devel] [PATCH v4 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")
Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.
This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
Currently we don't have an equivalent to "memory: emulate ioeventfd"
for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
skipping iothread arguments.
I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.
This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.
Signed-off-by: QingFeng Hao
Reviewed-by: Cornelia Huck
---
hw/s390x/virtio-ccw.c | 2 +-
target/s390x/cpu.h| 6 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
Error **errp)
sch->cssid, sch->ssid, sch->schid, sch->devno,
ccw_dev->devno.valid ? "user-configured" : "auto-configured");
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9faca04b52..bdb9bdbc9d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int
s390_assign_subch_ioeventfd(EventNotifier *notifier,
uint32_t sch_id, int vq,
bool assign)
{
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
}
static inline void s390_crypto_reset(void)
--
2.11.2
Re: [Qemu-devel] [PATCH] qemu-img: delete the -e and -6 options from the 'create' command
On Tue, Jul 04, 2017 at 11:34:27AM +0100, Daniel P. Berrange wrote:
> The '-e' and '-6' options to the 'create' command were "deprecated"
> in favour of the more generic '-o' option many years ago:
>
> commit eec77d9e712bd4157a4e1c0b5a9249d168add738
> Author: Jes Sorensen
> Date: Tue Dec 7 17:44:34 2010 +0100
>
> qemu-img: Deprecate obsolete -6 and -e options
>
> Except this was never actually a deprecation, which would imply giving
> the user a warning while the functionality continues to work for a
> number of releases before eventual removal. Instead the options were
> immediately turned into an error + exit. Given that the functionality
> is already broken, there's no point in keeping these psuedo-deprecation
> messages around any longer.
Sounds and looks good.
> Signed-off-by: Daniel P. Berrange
> ---
> qemu-img.c | 10 +-
> 1 file changed, 1 insertion(+), 9 deletions(-)
FWIW:
Reviewed-by: Kashyap Chamarthy
> diff --git a/qemu-img.c b/qemu-img.c
> index 91ad6be..a65239f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -464,7 +464,7 @@ static int img_create(int argc, char **argv)
> {"object", required_argument, 0, OPTION_OBJECT},
> {0, 0, 0, 0}
> };
> -c = getopt_long(argc, argv, ":F:b:f:he6o:q",
> +c = getopt_long(argc, argv, ":F:b:f:ho:q",
> long_options, NULL);
> if (c == -1) {
> break;
> @@ -488,14 +488,6 @@ static int img_create(int argc, char **argv)
> case 'f':
> fmt = optarg;
> break;
> -case 'e':
> -error_report("option -e is deprecated, please use \'-o "
> - "encryption\' instead!");
> -goto fail;
> -case '6':
> -error_report("option -6 is deprecated, please use \'-o "
> - "compat6\' instead!");
> -goto fail;
> case 'o':
> if (!is_valid_option_list(optarg)) {
> error_report("Invalid option list: %s", optarg);
> --
> 2.9.4
>
>
--
/kashyap
Re: [Qemu-devel] [PATCH v3 1/2] docs: document support lifetime for features
On Tue, Jul 04, 2017 at 02:43:29PM +0200, Thomas Huth wrote: > On 04.07.2017 13:14, Daniel P. Berrange wrote: > > There is currently no explicit guidance on the duration of support > > for features such as versioned machine types, which have a finite > > useful lifespan. Thus apps / users cannot predict how much time > > they might be able to use a feature for, before it is removed (if > > ever). > > > > This adds a new appendix that lists items which have finite lifecycles, > > such as machine types. For items which are generally expected to be > > supported indefinitely, it sets out the policy around deprecation > > and removal, should it be needed. > > Great, thanks for tackling this! > > > Signed-off-by: Daniel P. Berrange > > --- > > qemu-doc.texi | 37 + > > 1 file changed, 37 insertions(+) > > > > diff --git a/qemu-doc.texi b/qemu-doc.texi > > index 21079fd..27781e4 100644 > > --- a/qemu-doc.texi > > +++ b/qemu-doc.texi > > @@ -38,6 +38,7 @@ > > * QEMU Guest Agent:: > > * QEMU User space emulator:: > > * Implementation notes:: > > +* Support lifetime:: > > * License:: > > * Index:: > > @end menu > > @@ -3017,6 +3018,42 @@ Run the emulation in single step mode. > > > > @include qemu-tech.texi > > > > +@node Support lifetime > > +@appendix Support lifetime > > + > > +In general features are intended to be supported indefinitely once > > +introduced into QEMU. > > + > > +In the event that a feature needs to be removed, it will be listed > > +in the ``Deprecated features'' appendix of this document. The feature > > +will remain functional for 2 major releases prior to actual removal. > > Maybe say "at least for two major releases" instead? Sometimes we might > forget to remove it in time, or we might decide to keep it alive a > little bit longer in case it was once an important feature... FWIW, I explicitly avoided saying "at least" when I wrote this. I want to make deletion of deprecated features a clearly defined process where we don't have to re-open the debate of whether its ok to now delete it. > > +Deprecated features may also generate warnings on the console when > > +QEMU starts up, or if activated via a monitor command, however, > > +this is not a mandatory requirement. > > + > > +Certain features have an inherently finite lifetime, and thus > > +will be removed on a fixed schedule, without following the normal > > +deprecation process. Such features are listed in the sections > > +that follow. > > + > > +@node Machine types > > +@section Machine types > > + > > +For architectures which aim to support live migration compatibility > > +across releases, each release will introduce a new versioned machine > > +type. For example, the 2.8.0 release introduced machine types > > +``pc-i440fx-2.8'' and ``pc-q35-2.8' 'for the x86_64/i686 architectures. > > Space at wrong location --^ Opps. > > > +To allow live migration of a guest running on a 2.8.0 release to a > > +2.9.0, the QEMU 2.9.0 version must support the ``pc-i440fx-2.8'' and > > +``pc-q35-2.8''. To allow users live migrating VMs to skip multiple > > +intermediate releases when upgrading, new releases of QEMU will > > +support machine types from many previous versions. > > + > > +The supported lifetime for versioned machine types is 12 releases, > > Maybe rather "The minimum supported lifetime ..." ? In case we decide to > support it longer for any reasons... Same point as above - I feel its important to set a clear fixed timeframe to avoid re-opening debate each time we come to delete something. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v3 2/2] docs: document deprecated features in appendix
On Tue, Jul 04, 2017 at 02:59:27PM +0200, Thomas Huth wrote:
> On 04.07.2017 13:14, Daniel P. Berrange wrote:
> > The deprecation of features in QEMU is totally adhoc currently,
> > with no way for the user to get a list of what is deprecated
> > in each release. This adds an appendix to the doc that records
> > when each deprecation was made and provides text explaining
> > what to use instead, if anything.
> >
> > Since there has been no formal policy around removal of deprecated
> > features in the past, any deprecations prior to 2.10.0 are to be
> > treated as if they had been made at the 2.10.0 release. Thus the
> > earliest that existing deprecations will be deleted is the start
> > of the 2.12.0 cycle.
> >
> > Signed-off-by: Daniel P. Berrange
> > ---
> > qemu-doc.texi | 167
> > ++
> > 1 file changed, 167 insertions(+)
> >
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index 27781e4..ba5170c 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -39,6 +39,7 @@
> > * QEMU User space emulator::
> > * Implementation notes::
> > * Support lifetime::
> > +* Deprecated features::
> > * License::
> > * Index::
> > @end menu
> > @@ -3054,6 +3055,172 @@ support machine types from many previous versions.
> [...]
> > +@subsection -monitor default=on (since 2.4.0)
> > +
> > +The ``default'' option to the ``-monitor'' argument is
> > +now ignored. When multiple monitors were enabled, it
> > +indicated which monitor would receive log messages
> > +from the audio subsystem.
>
> The audio subsystem was just the last user. There were other subsystems
> using it before, see e.g. commit 027a79c373954920d5 or commit
> 02d16089802234f.
Ah, right, I'll clarify it.
> [...]
> > +@subsection -net vlan (since 2.9.0)
> > +
> > +The ``-net van=NN'' argument is partially replaced with the
> > +new ``-netdev'' argument. The remaining use cases will no
> > +longer be directly supported in QEMU
>
> Add a period after the last sentence?
Yes
>
> > +@subsection -drive if=scsi (since 2.9.0)
> > +
> > +The ``-drive if=scsi'' argument is replaced by the the
> > +``-device BUS-TYPE'' argument combined with ``-drive if=none''.
> > +
> > +@subsection -net dump (since 2.10.0)
> > +
> > +The ``--net dump'' argument is now a synonym for setting the
> > +``-object filter-dump'' argument instead.
>
> Technically, it's not a synonym, but a replacement ;-)
> ("-net dump" uses the "vlan" concept, while "-object filter-dump" works
> with "-netdev" instead)
Oh, right, I misunderstood that
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PATCH] spapr: Only report host/guest IOMMU page size mismatches on KVM
We print a warning if the spapr IOMMU isn't configured to support a page
size matching the host page size backing RAM. When that's the case we need
more complex logic to translate VFIO mappings, which is slower.
But, it's not so slow that it would be at all noticeable against the
general slowness of TCG. So, only warn when using KVM. This removes some
noisy and unhelpful warnings from make check on hosts with page sizes
which typically differ from those on POWER (e.g. Sparc).
Reported-by: Peter Maydell
Signed-off-by: David Gibson
---
hw/ppc/spapr_pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index cc1588d..a52dcf8 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1745,7 +1745,8 @@ static void spapr_phb_realize(DeviceState *dev, Error
**errp)
}
/* DMA setup */
-if ((sphb->page_size_mask & qemu_getrampagesize()) == 0) {
+if (((sphb->page_size_mask & qemu_getrampagesize()) == 0)
+&& kvm_enabled()) {
error_report("System page size 0x%lx is not enabled in page_size_mask "
"(0x%"PRIx64"). Performance may be slow",
qemu_getrampagesize(), sphb->page_size_mask);
--
2.9.4
Re: [Qemu-devel] [PATCH v4 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
On 07/04/2017 03:23 PM, QingFeng Hao wrote:
> This patch is based on a similar patch from Stefan Hajnoczi -
> commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")
>
> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
>
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.
> Currently we don't have an equivalent to "memory: emulate ioeventfd"
> for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
> skipping iothread arguments.
>
> I have tested that virtio-scsi-ccw works under tcg both with and without
> iothread.
>
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
>
> Signed-off-by: QingFeng Hao
> Reviewed-by: Cornelia Huck
I will take it via the s390-next tree.
thanks applied.
> ---
> hw/s390x/virtio-ccw.c | 2 +-
> target/s390x/cpu.h| 6 +-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice
> *dev, Error **errp)
> sch->cssid, sch->ssid, sch->schid, sch->devno,
> ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>
> -if (!kvm_eventfds_enabled()) {
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
> dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
> }
>
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 9faca04b52..bdb9bdbc9d 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -1264,7 +1264,11 @@ static inline int
> s390_assign_subch_ioeventfd(EventNotifier *notifier,
>uint32_t sch_id, int vq,
>bool assign)
> {
> -return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +if (kvm_enabled()) {
> +return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +} else {
> +return 0;
> +}
> }
>
> static inline void s390_crypto_reset(void)
>
Re: [Qemu-devel] [PATCH] spapr: Only report host/guest IOMMU page size mismatches on KVM
On 04.07.2017 15:54, David Gibson wrote:
> We print a warning if the spapr IOMMU isn't configured to support a page
> size matching the host page size backing RAM. When that's the case we need
> more complex logic to translate VFIO mappings, which is slower.
>
> But, it's not so slow that it would be at all noticeable against the
> general slowness of TCG. So, only warn when using KVM. This removes some
> noisy and unhelpful warnings from make check on hosts with page sizes
> which typically differ from those on POWER (e.g. Sparc).
>
> Reported-by: Peter Maydell
> Signed-off-by: David Gibson
> ---
> hw/ppc/spapr_pci.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index cc1588d..a52dcf8 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1745,7 +1745,8 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> }
>
> /* DMA setup */
> -if ((sphb->page_size_mask & qemu_getrampagesize()) == 0) {
> +if (((sphb->page_size_mask & qemu_getrampagesize()) == 0)
> +&& kvm_enabled()) {
Since you've put me on CC: ... too much parenthesis for my taste ;-)
Apart from that,
Reviewed-by: Thomas Huth
[Qemu-devel] [PATCH 2/7] s390x/3270: fix instruction interception handler
From: Dong Jia Shi
Commit bab482d7405f ("s390x/css: ccw translation infrastructure")
introduced instruction interception handler for different types of
subchannels. For emulated 3270 devices, we should assign the virtual
subchannel handler to them during device realization process, or 3270
will not work.
Fixes: bab482d7405f ("s390x/css: ccw translation infrastructure")
Reviewed-by: Jing Liu
Reviewed-by: Halil Pasic
Signed-off-by: Dong Jia Shi
Signed-off-by: Christian Borntraeger
---
hw/s390x/3270-ccw.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/3270-ccw.c b/hw/s390x/3270-ccw.c
index 6e6eee4..1554aa2 100644
--- a/hw/s390x/3270-ccw.c
+++ b/hw/s390x/3270-ccw.c
@@ -126,6 +126,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds,
Error **errp)
sch->id.cu_type = EMULATED_CCW_3270_CU_TYPE;
css_sch_build_virtual_schib(sch, (uint8_t)chpid,
EMULATED_CCW_3270_CHPID_TYPE);
+sch->do_subchannel_work = do_subchannel_work_virtual;
sch->ccw_cb = emulated_ccw_3270_cb;
ck->init(dev, &err);
--
2.7.4
[Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic
From: Halil Pasic
Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
2016-12-09) introduces a common realize (intended to be common for all
the subclasses) for flic, but fails to make sure the kvm-flic which had
it's own is actually calling this common realize.
This omission fortunately does not result in a grave problem. The common
realize was only supposed to catch a possible programming mistake by
validating a value of a property set via the compat machine macros. Since
there was no programming mistake we don't need this fixed for stable.
Let's fix this problem by making sure kvm flic honors the realize of its
parent class.
Let us also improve on the error message we would hypothetically emit
when the validation fails.
Signed-off-by: Halil Pasic
Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
Reviewed-by: Dong Jia Shi
Reviewed-by: Yi Min Zhao
Signed-off-by: Christian Borntraeger
---
hw/intc/s390_flic.c | 4 ++--
hw/intc/s390_flic_kvm.c | 17 +
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index a99a350..837158b 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev,
Error **errp)
uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
-error_setg(errp, "flic adapter_routes_max_batch too big"
- "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
+error_setg(errp, "flic property adapter_routes_max_batch too big"
+ " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
}
}
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index bea3997..535d99d 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate = {
}
};
+typedef struct KVMS390FLICStateClass {
+S390FLICStateClass parent_class;
+DeviceRealize parent_realize;
+} KVMS390FLICStateClass;
+
+#define KVM_S390_FLIC_GET_CLASS(obj) \
+OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
+
+#define KVM_S390_FLIC_CLASS(klass) \
+OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
+
static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
{
KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
@@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error
**errp)
int ret;
Error *errp_local = NULL;
+KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
+if (errp_local) {
+goto fail;
+}
flic_state->fd = -1;
if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
error_setg_errno(&errp_local, errno, "KVM is missing capability"
@@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void
*data)
DeviceClass *dc = DEVICE_CLASS(oc);
S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
+KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
dc->realize = kvm_s390_flic_realize;
dc->vmsd = &kvm_s390_flic_vmstate;
dc->reset = kvm_s390_flic_reset;
@@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
.name = TYPE_KVM_S390_FLIC,
.parent= TYPE_S390_FLIC_COMMON,
.instance_size = sizeof(KVMS390FLICState),
+.class_size= sizeof(KVMS390FLICStateClass),
.class_init= kvm_s390_flic_class_init,
};
--
2.7.4
[Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize
From: Halil Pasic
>From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
implement floating-interrupt controller device", 2013-07-16) the kvm-flic
is not making realize fail properly in case it's impossible to create the
KVM device which basically serves as a backend and is absolutely
essential for having an operational kvm-flic.
Let's fix this by making sure we do proper error propagation in realize.
Signed-off-by: Halil Pasic
Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
Reviewed-by: Dong Jia Shi
Reviewed-by: Yi Min Zhao
Signed-off-by: Christian Borntraeger
---
hw/intc/s390_flic_kvm.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index b4c61d8..bea3997 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -15,6 +15,7 @@
#include "cpu.h"
#include
#include "qemu/error-report.h"
+#include "qapi/error.h"
#include "hw/sysbus.h"
#include "sysemu/kvm.h"
#include "hw/s390x/s390_flic.h"
@@ -397,18 +398,22 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error
**errp)
struct kvm_create_device cd = {0};
struct kvm_device_attr test_attr = {0};
int ret;
+Error *errp_local = NULL;
flic_state->fd = -1;
if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
+error_setg_errno(&errp_local, errno, "KVM is missing capability"
+ " KVM_CAP_DEVICE_CTRL");
trace_flic_no_device_api(errno);
-return;
+goto fail;
}
cd.type = KVM_DEV_TYPE_FLIC;
ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
if (ret < 0) {
-trace_flic_create_device(errno);
-return;
+error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
+trace_flic_no_device_api(errno);
+goto fail;
}
flic_state->fd = cd.fd;
@@ -417,6 +422,9 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error
**errp)
flic_state->clear_io_supported = !ioctl(flic_state->fd,
KVM_HAS_DEVICE_ATTR, test_attr);
+return;
+fail:
+error_propagate(errp, errp_local);
}
static void kvm_s390_flic_reset(DeviceState *dev)
--
2.7.4
[Qemu-devel] [PATCH 5/7] s390x/MAINTAINERS: Update my email address
From: Cornelia Huck Signed-off-by: Cornelia Huck Message-Id: <20170704092215.13742-2-coh...@redhat.com> Signed-off-by: Christian Borntraeger --- MAINTAINERS | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 839f7ca..4e17216 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -299,7 +299,7 @@ F: target/ppc/kvm.c S390 M: Christian Borntraeger -M: Cornelia Huck +M: Cornelia Huck M: Alexander Graf S: Maintained F: target/s390x/kvm.c @@ -778,7 +778,7 @@ F: include/hw/sparc/grlib.h S390 Machines - S390 Virtio-ccw -M: Cornelia Huck +M: Cornelia Huck M: Christian Borntraeger M: Alexander Graf S: Supported @@ -1006,7 +1006,7 @@ F: hw/vfio/* F: include/hw/vfio/ vfio-ccw -M: Cornelia Huck +M: Cornelia Huck S: Supported F: hw/vfio/ccw.c F: hw/s390x/s390-ccw.c @@ -1048,7 +1048,7 @@ F: tests/virtio-blk-test.c T: git git://github.com/stefanha/qemu.git block virtio-ccw -M: Cornelia Huck +M: Cornelia Huck M: Christian Borntraeger S: Supported F: hw/s390x/virtio-ccw.[hc] -- 2.7.4
[Qemu-devel] [PATCH 6/7] s390x: return unavailable features via query-cpu-definitions
From: Viktor Mihajlovski
The response for query-cpu-definitions didn't include the
unavailable-features field, which is used by libvirt to figure
out whether a certain cpu model is usable on the host.
The unavailable features are now computed by obtaining the host CPU
model and comparing it against the known CPU models. The comparison
takes into account the generation, the GA level and the feature
bitmaps. In the case of a CPU generation/GA level mismatch
a feature called "type" is reported to be missing.
As a result, the output of virsh domcapabilities would change
from something like
...
z10EC-base
z9EC-base
z196.2-base
z900-base
z990
...
to
...
z10EC-base
z9EC-base
z196.2-base
z900-base
z990
...
Signed-off-by: Viktor Mihajlovski
Message-Id: <1499082529-16970-1-git-send-email-mihaj...@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand
Signed-off-by: Christian Borntraeger
---
target/s390x/cpu_models.c | 62 +++
1 file changed, 57 insertions(+), 5 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 63903c2..7cb55dc 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -283,10 +283,41 @@ void s390_cpu_list(FILE *f, fprintf_function print)
}
}
+static S390CPUModel *get_max_cpu_model(Error **errp);
+
#ifndef CONFIG_USER_ONLY
+static void list_add_feat(const char *name, void *opaque);
+
+static void check_unavailable_features(const S390CPUModel *max_model,
+ const S390CPUModel *model,
+ strList **unavailable)
+{
+S390FeatBitmap missing;
+
+/* check general model compatibility */
+if (max_model->def->gen < model->def->gen ||
+(max_model->def->gen == model->def->gen &&
+ max_model->def->ec_ga < model->def->ec_ga)) {
+list_add_feat("type", unavailable);
+}
+
+/* detect missing features if any to properly report them */
+bitmap_andnot(missing, model->features, max_model->features,
+ S390_FEAT_MAX);
+if (!bitmap_empty(missing, S390_FEAT_MAX)) {
+s390_feat_bitmap_to_ascii(missing, unavailable, list_add_feat);
+}
+}
+
+struct CpuDefinitionInfoListData {
+CpuDefinitionInfoList *list;
+S390CPUModel *model;
+};
+
static void create_cpu_model_list(ObjectClass *klass, void *opaque)
{
-CpuDefinitionInfoList **cpu_list = opaque;
+struct CpuDefinitionInfoListData *cpu_list_data = opaque;
+CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
CpuDefinitionInfoList *entry;
CpuDefinitionInfo *info;
char *name = g_strdup(object_class_get_name(klass));
@@ -300,7 +331,19 @@ static void create_cpu_model_list(ObjectClass *klass, void
*opaque)
info->migration_safe = scc->is_migration_safe;
info->q_static = scc->is_static;
info->q_typename = g_strdup(object_class_get_name(klass));
-
+/* check for unavailable features */
+if (cpu_list_data->model) {
+Object *obj;
+S390CPU *sc;
+obj = object_new(object_class_get_name(klass));
+sc = S390_CPU(obj);
+if (sc->model) {
+info->has_unavailable_features = true;
+check_unavailable_features(cpu_list_data->model, sc->model,
+ &info->unavailable_features);
+}
+object_unref(obj);
+}
entry = g_malloc0(sizeof(*entry));
entry->value = info;
@@ -310,11 +353,20 @@ static void create_cpu_model_list(ObjectClass *klass,
void *opaque)
CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
{
-CpuDefinitionInfoList *list = NULL;
+struct CpuDefinitionInfoListData list_data = {
+.list = NULL,
+};
-object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false, &list);
+list_data.model = get_max_cpu_model(errp);
+if (*errp) {
+error_free(*errp);
+*errp = NULL;
+}
-return list;
+object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
+ &list_data);
+
+return list_data.list;
}
static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
--
2.7.4
[Qemu-devel] [PATCH 7/7] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
From: QingFeng Hao
This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")
Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.
This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
Currently we don't have an equivalent to "memory: emulate ioeventfd"
for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
skipping iothread arguments.
I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.
This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.
Signed-off-by: QingFeng Hao
Reviewed-by: Cornelia Huck
Message-Id: <20170704132350.11874-2-ha...@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger
---
hw/s390x/virtio-ccw.c | 2 +-
target/s390x/cpu.h| 6 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index f0e7fc8..e18fd26 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -789,7 +789,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
Error **errp)
sch->cssid, sch->ssid, sch->schid, sch->devno,
ccw_dev->devno.valid ? "user-configured" : "auto-configured");
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9faca04..bdb9bdb 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int
s390_assign_subch_ioeventfd(EventNotifier *notifier,
uint32_t sch_id, int vq,
bool assign)
{
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
}
static inline void s390_crypto_reset(void)
--
2.7.4
[Qemu-devel] [PATCH 0/7] s390x/kvm: pending patches
This is what I have queued for s390x. Cornelia Huck (1): s390x/MAINTAINERS: Update my email address Dong Jia Shi (1): s390x/3270: fix instruction interception handler Halil Pasic (3): s390x: vmstatify config migration for virtio-ccw s390x: fix error propagation in kvm-flic's realize s390x: fix realize inheritance for kvm-flic QingFeng Hao (1): virtio-scsi-ccw: use ioeventfd even when KVM is disabled Viktor Mihajlovski (1): s390x: return unavailable features via query-cpu-definitions MAINTAINERS | 8 +- hw/intc/s390_flic.c | 32 +++- hw/intc/s390_flic_kvm.c | 31 +++- hw/s390x/3270-ccw.c | 1 + hw/s390x/ccw-device.c| 10 ++ hw/s390x/ccw-device.h| 4 + hw/s390x/css.c | 378 +-- hw/s390x/virtio-ccw.c| 160 +- include/hw/s390x/css.h | 12 +- include/hw/s390x/s390_flic.h | 5 + target/s390x/cpu.h | 6 +- target/s390x/cpu_models.c| 62 ++- 12 files changed, 456 insertions(+), 253 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH 1/7] s390x: vmstatify config migration for virtio-ccw
From: Halil Pasic
Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for
flexibility (extending using subsections) and for fun.
To achieve this we need to hack the config_vector, which is VirtIODevice
(that is common virtio) state, in the middle of the VirtioCcwDevice state
representation. This is somewhat ugly, but we have no choice because the
stream format needs to be preserved.
Almost no changes in behavior. Exception is everything that comes with
vmstate like extra bookkeeping about what's in the stream, and maybe some
extra checks and better error reporting.
Signed-off-by: Halil Pasic
Reviewed-by: Dr. David Alan Gilbert
Reviewed-by: Juan Quintela
Reviewed-by: Cornelia Huck
Message-Id: <20170703213414.94298-1-pa...@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger
---
hw/intc/s390_flic.c | 28
hw/s390x/ccw-device.c| 10 ++
hw/s390x/ccw-device.h| 4 +
hw/s390x/css.c | 378 +--
hw/s390x/virtio-ccw.c| 158 +-
include/hw/s390x/css.h | 12 +-
include/hw/s390x/s390_flic.h | 5 +
7 files changed, 358 insertions(+), 237 deletions(-)
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index a26e906..a99a350 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -17,6 +17,7 @@
#include "trace.h"
#include "hw/qdev.h"
#include "qapi/error.h"
+#include "hw/s390x/s390-virtio-ccw.h"
S390FLICState *s390_get_flic(void)
{
@@ -136,3 +137,30 @@ static void qemu_s390_flic_register_types(void)
}
type_init(qemu_s390_flic_register_types)
+
+const VMStateDescription vmstate_adapter_info = {
+.name = "s390_adapter_info",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT64(ind_offset, AdapterInfo),
+/*
+ * We do not have to migrate neither the id nor the addresses.
+ * The id is set by css_register_io_adapter and the addresses
+ * are set based on the IndAddr objects after those get mapped.
+ */
+VMSTATE_END_OF_LIST()
+},
+};
+
+const VMStateDescription vmstate_adapter_routes = {
+
+.name = "s390_adapter_routes",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_STRUCT(adapter, AdapterRoutes, 1, vmstate_adapter_info,
+ AdapterInfo),
+VMSTATE_END_OF_LIST()
+}
+};
diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index fb8d640..f9bfa15 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -50,6 +50,16 @@ static void ccw_device_class_init(ObjectClass *klass, void
*data)
dc->props = ccw_device_properties;
}
+const VMStateDescription vmstate_ccw_dev = {
+.name = "s390_ccw_dev",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_STRUCT_POINTER(sch, CcwDevice, vmstate_subch_dev, SubchDev),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const TypeInfo ccw_device_info = {
.name = TYPE_CCW_DEVICE,
.parent = TYPE_DEVICE,
diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 89c8e5d..4e6af28 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -27,6 +27,10 @@ typedef struct CcwDevice {
CssDevId subch_id;
} CcwDevice;
+extern const VMStateDescription vmstate_ccw_dev;
+#define VMSTATE_CCW_DEVICE(_field, _state) \
+VMSTATE_STRUCT(_field, _state, 1, vmstate_ccw_dev, CcwDevice)
+
typedef struct CCWDeviceClass {
DeviceClass parent_class;
void (*unplug)(HotplugHandler *, DeviceState *, Error **);
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 599805d..d67fffa 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -22,6 +22,7 @@
#include "hw/s390x/css.h"
#include "trace.h"
#include "hw/s390x/s390_flic.h"
+#include "hw/s390x/s390-virtio-ccw.h"
typedef struct CrwContainer {
CRW crw;
@@ -40,6 +41,181 @@ typedef struct SubchSet {
unsigned long devnos_used[BITS_TO_LONGS(MAX_SCHID + 1)];
} SubchSet;
+static const VMStateDescription vmstate_scsw = {
+.name = "s390_scsw",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT16(flags, SCSW),
+VMSTATE_UINT16(ctrl, SCSW),
+VMSTATE_UINT32(cpa, SCSW),
+VMSTATE_UINT8(dstat, SCSW),
+VMSTATE_UINT8(cstat, SCSW),
+VMSTATE_UINT16(count, SCSW),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_pmcw = {
+.name = "s390_pmcw",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(intparm, PMCW),
+VMSTATE_UINT16(flags, PMCW),
+VMSTATE_UINT16(devno, PMCW),
+VMSTATE_UINT8(lpm, PMCW),
+VMSTATE_UINT8(pnom, PMCW),
+VMSTATE_UINT8(lpum, PMCW),
+VMSTATE_UINT8(pim, PMCW),
+VMSTATE_UINT16(mbi, PMCW),
+VMSTATE_UINT8(pom,
Re: [Qemu-devel] [RFC 29/29] vhost-user: Claim support for postcopy
On 06/28/2017 09:00 PM, Dr. David Alan Gilbert (git) wrote:
From: "Dr. David Alan Gilbert"
Tell QEMU we understand the protocol features needed for postcopy.
Signed-off-by: Dr. David Alan Gilbert
---
contrib/libvhost-user/libvhost-user.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/libvhost-user/libvhost-user.c
b/contrib/libvhost-user/libvhost-user.c
index c1716d1a62..1c46aecfb3 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -797,7 +797,8 @@ vu_set_vring_err_exec(VuDev *dev, VhostUserMsg *vmsg)
static bool
vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
{
-uint64_t features = 1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD;
+uint64_t features = 1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD |
+1ULL << VHOST_USER_PROTOCOL_F_POSTCOPY;
Maybe advertising VHOST_USER_PROTOCOL_F_POSTCOPY could be done only
if userfaultfd syscall is supported?
if (dev->iface->get_protocol_features) {
features |= dev->iface->get_protocol_features(dev);
Re: [Qemu-devel] [Xen-devel] [PATCH 2/2 v2] xenfb: Allow vkbd to connect without a DisplayState
On Mon, Jul 3, 2017 at 4:17 PM, Owen Smith wrote:
> If the vkbd device model is registered and the vfb device model
> is not registered, the backend will not transition to connected.
> If there is no DisplayState, then the absolute coordinates cannot
> be scaled, and will remain in the range [0, 0x7fff].
> Backend writes "feature-raw-pointer" to indicate that the backend
> supports reporting absolute position without rescaling.
> The frontend uses "request-raw-pointer" to request raw unscaled
> pointer values. If there is no DisplayState, the absolute values
> are always raw unscaled values.
>
> Signed-off-by: Owen Smith
> ---
> hw/display/xenfb.c | 36 ++--
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 88815df..d40af6e 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -53,6 +53,7 @@ struct common {
> struct XenInput {
> struct common c;
> int abs_pointer_wanted; /* Whether guest supports absolute pointer */
> +int raw_pointer_wanted; /* Whether guest supports unscaled pointer */
> int button_state; /* Last seen pointer button state */
> int extended;
> /* kbd */
> @@ -329,18 +330,22 @@ static void xenfb_mouse_event(void *opaque,
> int dx, int dy, int dz, int button_state)
> {
> struct XenInput *xenfb = opaque;
> -DisplaySurface *surface = qemu_console_surface(xenfb->c.con);
> -int dw = surface_width(surface);
> -int dh = surface_height(surface);
> -int i;
> +int i, x, y;
> +if (xenfb->c.con && xenfb->raw_pointer_wanted != 1) {
> +DisplaySurface *surface = qemu_console_surface(xenfb->c.con);
> +int dw = surface_width(surface);
> +int dh = surface_height(surface);
> +x = dx * (dw - 1) / 0x7fff;
> +y = dy * (dh - 1) / 0x7fff;
> +} else {
> +x = dx;
> +y = dy;
> +}
>
> trace_xenfb_mouse_event(opaque, dx, dy, dz, button_state,
> xenfb->abs_pointer_wanted);
> if (xenfb->abs_pointer_wanted)
> - xenfb_send_position(xenfb,
> - dx * (dw - 1) / 0x7fff,
> - dy * (dh - 1) / 0x7fff,
> - dz);
> +xenfb_send_position(xenfb, x, y, dz);
> else
> xenfb_send_motion(xenfb, dx, dy, dz);
>
> @@ -423,6 +428,7 @@ static void xenfb_legacy_mouse_sync(DeviceState *dev)
> static int input_init(struct XenDevice *xendev)
> {
> xenstore_write_be_int(xendev, "feature-abs-pointer", 1);
> +xenstore_write_be_int(xendev, "feature-raw-pointer", 1);
> return 0;
> }
>
> @@ -432,8 +438,14 @@ static int input_initialise(struct XenDevice *xendev)
> int rc;
>
> if (!in->c.con) {
It doesn't look like proper way. If I understand this condition
correctly, you are trying to
launch the vkbd backend even if there is no fb configured. It means
the vkbd backend
will be always startedeven if it is not needed.
I'm working on the patch which allows to launch vkbd backend separately from fb.
We need it to be able to launch our own backend in user space [1].
I think proper way is:
1. Add standalone vkbd configuration to xl.cfg.
2. Redesign xen_init_display in way it allows to start vkbd without vfb.
The item 1. will be in my patch.
> -xen_pv_printf(xendev, 1, "ds not set (yet)\n");
> -return -1;
> +char *vfb = xenstore_read_str(NULL, "device/vfb");
This xenstore_read_str will not work as expected. I guess
this line will try to read entry from "(null)/device/vfb" which is
always doesn't exist. See xenstore_read_str implementation.
> +if (vfb == NULL) {
> +/* there is no vfb, run vkbd on its own */
> +} else {
> +free(vfb);
> +xen_pv_printf(xendev, 1, "ds not set (yet)\n");
> +return -1;
> +}
> }
>
> rc = common_bind(&in->c);
> @@ -451,6 +463,10 @@ static void input_connected(struct XenDevice *xendev)
> &in->abs_pointer_wanted) == -1) {
> in->abs_pointer_wanted = 0;
> }
> +if (xenstore_read_fe_int(xendev, "request-raw-pointer",
> + &in->raw_pointer_wanted) == -1) {
> +in->raw_pointer_wanted = 0;
> +}
>
> if (in->qkbd) {
> qemu_input_handler_unregister(in->qkbd);
> --
> 2.1.4
>
>
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> https://lists.xen.org/xen-devel
[1] http://marc.info/?l=qemu-devel&m=149266892429889&w=2
[Qemu-devel] [PATCH] vhost: fix a memory leak
vhost exists a call for g_file_get_contents, but not call g_free.
Signed-off-by: Peng Hao
---
hw/virtio/vhost-backend.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 4e31de1..2c481d6 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -52,11 +52,13 @@ static int vhost_kernel_memslots_limit(struct vhost_dev
*dev)
&s, NULL, NULL)) {
uint64_t val = g_ascii_strtoull(s, NULL, 10);
if (!((val == G_MAXUINT64 || !val) && errno)) {
+g_free(s);
return val;
}
error_report("ignoring invalid max_mem_regions value in vhost module:"
" %s", s);
}
+g_free(s);
return limit;
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH v3 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
On Tue, 4 Jul 2017 10:32:31 +0200
QingFeng Hao wrote:
> This patch is based on a similar patch from Stefan Hajnoczi -
> commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)
>
> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
>
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.
It might be good to add a sentence that we don't have an equivalent to
"memory: emulate ioeventfd" for ccw yet, but that this doesn't hurt.
>
> I have tested that virtio-scsi-ccw works under tcg both with and without
> iothread.
>
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
>
> Signed-off-by: QingFeng Hao
> ---
> hw/s390x/virtio-ccw.c | 2 +-
> target/s390x/cpu.h| 6 +-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice
> *dev, Error **errp)
> sch->cssid, sch->ssid, sch->schid, sch->devno,
> ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>
> -if (!kvm_eventfds_enabled()) {
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
> dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
> }
>
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 9faca04b52..bdb9bdbc9d 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -1264,7 +1264,11 @@ static inline int
> s390_assign_subch_ioeventfd(EventNotifier *notifier,
>uint32_t sch_id, int vq,
>bool assign)
> {
> -return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +if (kvm_enabled()) {
> +return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
> +} else {
> +return 0;
> +}
> }
>
> static inline void s390_crypto_reset(void)
Reviewed-by: Cornelia Huck
[Qemu-devel] [PATCH] audio: st_rate_flow exist a dead loop
From: Hao Peng
if audio device is opend for a long time in windows vm, there is a uint32_t
varaible reversal.
It will result to a dead loop.
Signed-off-by: Peng Hao
Reviewed-by: Liu Yun
Reviewed-by: Wang Yechao
---
audio/rate_template.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/audio/rate_template.h b/audio/rate_template.h
index bd4b1c7..1aa6c43 100644
--- a/audio/rate_template.h
+++ b/audio/rate_template.h
@@ -71,6 +71,11 @@ void NAME (void *opaque, struct st_sample *ibuf, struct
st_sample *obuf,
while (rate->ipos <= (rate->opos >> 32)) {
ilast = *ibuf++;
rate->ipos++;
+/* if ipos reversal, there is a dead loop */
+if (rate->ipos == 0x) {
+rate->ipos = 1;
+rate->opos = rate->opos & 0x;
+}
/* See if we finished the input buffer yet */
if (ibuf >= iend) {
goto the_end;
--
1.8.3.1
[Qemu-devel] [PATCH 1/1] Update my email address
Signed-off-by: Cornelia Huck --- MAINTAINERS | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 7df088259b..afc2b8b328 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -299,7 +299,7 @@ F: target/ppc/kvm.c S390 M: Christian Borntraeger -M: Cornelia Huck +M: Cornelia Huck M: Alexander Graf S: Maintained F: target/s390x/kvm.c @@ -778,7 +778,7 @@ F: include/hw/sparc/grlib.h S390 Machines - S390 Virtio-ccw -M: Cornelia Huck +M: Cornelia Huck M: Christian Borntraeger M: Alexander Graf S: Supported @@ -1006,7 +1006,7 @@ F: hw/vfio/* F: include/hw/vfio/ vfio-ccw -M: Cornelia Huck +M: Cornelia Huck S: Supported F: hw/vfio/ccw.c F: hw/s390x/s390-ccw.c @@ -1048,7 +1048,7 @@ F: tests/virtio-blk-test.c T: git git://github.com/stefanha/qemu.git block virtio-ccw -M: Cornelia Huck +M: Cornelia Huck M: Christian Borntraeger S: Supported F: hw/s390x/virtio-ccw.[hc] -- 2.13.0
Re: [Qemu-devel] postcopy migration hangs while loading virtio state
On Tue, 4 Jul 2017 09:48:11 +0200
Christian Borntraeger wrote:
> On 07/03/2017 09:07 PM, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (m...@redhat.com) wrote:
> >> On Fri, Jun 30, 2017 at 05:31:39PM +0100, Dr. David Alan Gilbert wrote:
> >>> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
> On 04/26/2017 01:45 PM, Christian Borntraeger wrote:
>
> >> Hmm, I have a theory, if the flags field has bit 1 set, i.e.
> >> RAM_SAVE_FLAG_COMPRESS
> >> then try changing ram_handle_compressed to always do the memset.
> >
> > FWIW, changing ram_handle_compressed to always memset makes the problem
> > go away.
>
> It is still running fine now with the "always memset change"
> >>>
> >>> Did we ever nail down a fix for this; as I remember Andrea said
> >>> we shouldn't need to do that memset, but we came to the conclusion
> >>> it was something specific to how s390 protection keys worked.
> >>>
> >>> Dave
> >>
> >> No we didn't. Let's merge that for now, with a comment that
> >> we don't really understand what's going on?
> >
> > Hmm no, I don't really want to change the !s390 behaviour, especially
> > since it causes allocation that we otherwise avoid and Andrea's
> > reply tothe original post pointed out it's not necessary.
>
>
> Since storage keys are per physical page we must not have shared pages.
> Therefore in s390_enable_skey we already do a break_ksm (via ksm_madvise),
> in other words we already allocate pages on the keyless->keyed switch.
>
> So why not do the same for zero pages - instead of invalidating the page
> table entry, lets just do a proper COW.
>
> Something like this maybe (Andrea, Martin any better way to do that?)
>
>
> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
> index 4fb3d3c..11475c7 100644
> --- a/arch/s390/mm/gmap.c
> +++ b/arch/s390/mm/gmap.c
> @@ -2149,13 +2149,18 @@ EXPORT_SYMBOL_GPL(s390_enable_sie);
> static int __s390_enable_skey(pte_t *pte, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> + struct page *page;
> /*
> -* Remove all zero page mappings,
> +* Remove all zero page mappings with a COW
> * after establishing a policy to forbid zero page mappings
> * following faults for that page will get fresh anonymous pages
> */
> - if (is_zero_pfn(pte_pfn(*pte)))
> - ptep_xchg_direct(walk->mm, addr, pte, __pte(_PAGE_INVALID));
> + if (is_zero_pfn(pte_pfn(*pte))) {
> + if (get_user_pages(addr, 1, FOLL_WRITE, &page, NULL) == 1)
> + put_page(page);
> + else
> + return -ENOMEM;
> + }
> /* Clear storage key */
> ptep_zap_key(walk->mm, addr, pte);
> return 0;
I do not quite get the problem here. The zero-page mappings are always
marked as _PAGE_SPECIAL. These should be safe to replace with an empty
pte. We do mark all VMAs as unmergeable prior to the page table walk
that replaces all zero-page mappings. What is the get_user_pages() call
supposed to do?
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
[Qemu-devel] [PATCH 0/1] Change mail address
New employer, new address. Make sure people use the right one. Christian, it's probably quickest if you take this. Cornelia Huck (1): Update my email address MAINTAINERS | 8 1 file changed, 4 insertions(+), 4 deletions(-) -- 2.13.0
Re: [Qemu-devel] [PATCH 2/7] s390x/3270: fix instruction interception handler
On Tue, 4 Jul 2017 16:07:54 +0200
Christian Borntraeger wrote:
> From: Dong Jia Shi
>
> Commit bab482d7405f ("s390x/css: ccw translation infrastructure")
> introduced instruction interception handler for different types of
> subchannels. For emulated 3270 devices, we should assign the virtual
> subchannel handler to them during device realization process, or 3270
> will not work.
>
> Fixes: bab482d7405f ("s390x/css: ccw translation infrastructure")
>
> Reviewed-by: Jing Liu
> Reviewed-by: Halil Pasic
> Signed-off-by: Dong Jia Shi
> Signed-off-by: Christian Borntraeger
> ---
> hw/s390x/3270-ccw.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/s390x/3270-ccw.c b/hw/s390x/3270-ccw.c
> index 6e6eee4..1554aa2 100644
> --- a/hw/s390x/3270-ccw.c
> +++ b/hw/s390x/3270-ccw.c
> @@ -126,6 +126,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds,
> Error **errp)
> sch->id.cu_type = EMULATED_CCW_3270_CU_TYPE;
> css_sch_build_virtual_schib(sch, (uint8_t)chpid,
> EMULATED_CCW_3270_CHPID_TYPE);
> +sch->do_subchannel_work = do_subchannel_work_virtual;
> sch->ccw_cb = emulated_ccw_3270_cb;
>
> ck->init(dev, &err);
Reviewed-by: Cornelia Huck
Re: [Qemu-devel] [PATCH] vhost: fix a memory leak
Hi
On Tue, Jul 4, 2017 at 4:16 PM Peng Hao wrote:
> vhost exists a call for g_file_get_contents, but not call g_free.
>
> Signed-off-by: Peng Hao
>
Reviewed-by: Marc-André Lureau
---
> hw/virtio/vhost-backend.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 4e31de1..2c481d6 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -52,11 +52,13 @@ static int vhost_kernel_memslots_limit(struct
> vhost_dev *dev)
> &s, NULL, NULL)) {
> uint64_t val = g_ascii_strtoull(s, NULL, 10);
> if (!((val == G_MAXUINT64 || !val) && errno)) {
> +g_free(s);
> return val;
> }
> error_report("ignoring invalid max_mem_regions value in vhost
> module:"
> " %s", s);
> }
> +g_free(s);
> return limit;
> }
>
> --
> 1.8.3.1
>
>
>
> --
Marc-André Lureau
Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize
On Tue, 4 Jul 2017 16:07:55 +0200
Christian Borntraeger wrote:
> From: Halil Pasic
>
> From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
> implement floating-interrupt controller device", 2013-07-16) the kvm-flic
> is not making realize fail properly in case it's impossible to create the
> KVM device which basically serves as a backend and is absolutely
> essential for having an operational kvm-flic.
>
> Let's fix this by making sure we do proper error propagation in realize.
>
> Signed-off-by: Halil Pasic
> Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
> Reviewed-by: Dong Jia Shi
> Reviewed-by: Yi Min Zhao
> Signed-off-by: Christian Borntraeger
> ---
> hw/intc/s390_flic_kvm.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
(...)
> cd.type = KVM_DEV_TYPE_FLIC;
> ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
> if (ret < 0) {
> -trace_flic_create_device(errno);
> -return;
> +error_setg_errno(&errp_local, errno, "Creating the KVM device
> failed");
> +trace_flic_no_device_api(errno);
Err... this should still be trace_flic_create_device(), no?
> +goto fail;
> }
> flic_state->fd = cd.fd;
Re: [Qemu-devel] [PATCH RESEND v6] qga: Add support network interface statistics in guest-network-get-interfaces command
Hi
On Tue, Jul 4, 2017 at 10:51 AM ZhiPeng Lu wrote:
> we can get the network interface statistics inside a virtual machine by
> guest-network-get-interfaces command. it is very useful for us to monitor
> and analyze network traffic.
>
>
It's nicer if you give v1->v2->..->v6 change summary at each revision
> Signed-off-by: ZhiPeng Lu
> ---
> qga/commands-posix.c | 80
> +++-
> qga/qapi-schema.json | 38 -
> 2 files changed, 116 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 915df9e..233b024 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1638,6 +1638,73 @@ guest_find_interface(GuestNetworkInterfaceList
> *head,
> return head;
> }
>
> +
> +static int str_trim_off(const char *s, int off, int lmt)
> +{
> +for (; off < lmt; ++off) {
> +if (!isspace(s[off])) {
> +break;
> +}
> +}
> +return off;
> +}
> +
> +static int guest_get_network_stats(const char *name,
> + GuestNetworkInterfaceStat *stats)
> +{
> +int name_len;
> +char const *devinfo = "/proc/net/dev";
> +FILE *fp;
> +char *line = NULL, *colon;
> +size_t n;
> +fp = fopen(devinfo, "r");
> +if (!fp) {
> +return -1;
> +}
> +name_len = strlen(name);
> +while (getline(&line, &n, fp) != -1) {
> +long long dummy;
> +long long rx_bytes;
> +long long rx_packets;
> +long long rx_errs;
> +long long rx_dropped;
> +long long tx_bytes;
> +long long tx_packets;
> +long long tx_errs;
> +long long tx_dropped;
>
Why have local variables, and not pass stats->filed directly?
+int trim_off;
> +colon = strchr(line, ':');
> +if (!colon) {
> +continue;
> +}
> +trim_off = str_trim_off(line, 0, strlen(line));
>
Couldn't you call g_strchomp() ?
> +if (colon - name_len - trim_off == line &&
> + strncmp(line + trim_off, name, colon - line - trim_off) == 0) {
> +if (sscanf(colon + 1,
> +"%lld %lld %lld %lld %lld %lld %lld %lld %lld %lld %lld
> %lld %lld %lld %lld %lld",
> + &rx_bytes, &rx_packets, &rx_errs, &rx_dropped,
> + &dummy, &dummy, &dummy, &dummy,
> + &tx_bytes, &tx_packets, &tx_errs, &tx_dropped,
> + &dummy, &dummy, &dummy, &dummy) != 16) {
> +continue;
> +}
> +stats->rx_bytes = rx_bytes;
> +stats->rx_packets = rx_packets;
> +stats->rx_errs = rx_errs;
> +stats->rx_dropped = rx_dropped;
> +stats->tx_bytes = tx_bytes;
> +stats->tx_packets = tx_packets;
> +stats->tx_errs = tx_errs;
> +stats->tx_dropped = tx_dropped;
> +fclose(fp);
> +return 0;
> +}
> +}
> +fclose(fp);
> +g_debug("/proc/net/dev: Interface not found");
> +return -1;
> +}
> +
> /*
> * Build information about guest interfaces
> */
> @@ -1654,6 +1721,7 @@ GuestNetworkInterfaceList
> *qmp_guest_network_get_interfaces(Error **errp)
> for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
> GuestNetworkInterfaceList *info;
> GuestIpAddressList **address_list = NULL, *address_item = NULL;
> +GuestNetworkInterfaceStat *interface_stat = NULL;
> char addr4[INET_ADDRSTRLEN];
> char addr6[INET6_ADDRSTRLEN];
> int sock;
> @@ -1773,7 +1841,17 @@ GuestNetworkInterfaceList
> *qmp_guest_network_get_interfaces(Error **errp)
>
> info->value->has_ip_addresses = true;
>
> -
> +if (!info->value->has_statistics) {
> +interface_stat = g_malloc0(sizeof(*interface_stat));
> +if (guest_get_network_stats(info->value->name,
> +interface_stat) == -1) {
> +info->value->has_statistics = false;
> +g_free(interface_stat);
> +} else {
> +info->value->statistics = interface_stat;
> +info->value->has_statistics = true;
> +}
> +}
> }
>
> freeifaddrs(ifap);
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index a02dbf2..948219b 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -635,6 +635,38 @@
> 'prefix': 'int'} }
>
> ##
> +# @GuestNetworkInterfaceStat:
> +#
> +# @rx-bytes: total bytes received
> +#
> +# @rx-packets: total packets received
> +#
> +# @rx-errs: bad packets received
> +#
> +# @rx-dropped: receiver dropped packets
> +#
> +# @tx-bytes: total bytes transmitted
> +#
> +# @tx-packets: total packets transmitted
> +#
> +# @tx-errs: packet transmit problems
> +#
> +# @tx-dropped: dropped packets transmitted
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'GuestNetworkInterfaceStat',
> + 'data': {'rx-bytes': 'uint64',
>
Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic
On Tue, 4 Jul 2017 16:07:56 +0200
Christian Borntraeger wrote:
> From: Halil Pasic
>
> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
> 2016-12-09) introduces a common realize (intended to be common for all
> the subclasses) for flic, but fails to make sure the kvm-flic which had
> it's own is actually calling this common realize.
s/it's/its/
>
> This omission fortunately does not result in a grave problem. The common
> realize was only supposed to catch a possible programming mistake by
> validating a value of a property set via the compat machine macros. Since
> there was no programming mistake we don't need this fixed for stable.
>
> Let's fix this problem by making sure kvm flic honors the realize of its
> parent class.
>
> Let us also improve on the error message we would hypothetically emit
> when the validation fails.
>
> Signed-off-by: Halil Pasic
> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
> Reviewed-by: Dong Jia Shi
> Reviewed-by: Yi Min Zhao
> Signed-off-by: Christian Borntraeger
> ---
> hw/intc/s390_flic.c | 4 ++--
> hw/intc/s390_flic_kvm.c | 17 +
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> index a99a350..837158b 100644
> --- a/hw/intc/s390_flic.c
> +++ b/hw/intc/s390_flic.c
> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev,
> Error **errp)
> uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
>
> if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
> -error_setg(errp, "flic adapter_routes_max_batch too big"
> - "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
> +error_setg(errp, "flic property adapter_routes_max_batch too big"
> + " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
Unrelated message change?
> }
> }
>
> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> index bea3997..535d99d 100644
> --- a/hw/intc/s390_flic_kvm.c
> +++ b/hw/intc/s390_flic_kvm.c
> @@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate = {
> }
> };
>
> +typedef struct KVMS390FLICStateClass {
> +S390FLICStateClass parent_class;
> +DeviceRealize parent_realize;
> +} KVMS390FLICStateClass;
> +
> +#define KVM_S390_FLIC_GET_CLASS(obj) \
> +OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
> +
> +#define KVM_S390_FLIC_CLASS(klass) \
> +OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
> +
> static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
> {
> KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
> @@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev,
> Error **errp)
> int ret;
> Error *errp_local = NULL;
>
> +KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
I usually prefer to introduce a local variable for that, but don't care
too much.
> +if (errp_local) {
> +goto fail;
> +}
> flic_state->fd = -1;
> if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
> error_setg_errno(&errp_local, errno, "KVM is missing capability"
> @@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc,
> void *data)
> DeviceClass *dc = DEVICE_CLASS(oc);
> S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
>
> +KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
dito
> dc->realize = kvm_s390_flic_realize;
> dc->vmsd = &kvm_s390_flic_vmstate;
> dc->reset = kvm_s390_flic_reset;
> @@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
> .name = TYPE_KVM_S390_FLIC,
> .parent= TYPE_S390_FLIC_COMMON,
> .instance_size = sizeof(KVMS390FLICState),
> +.class_size= sizeof(KVMS390FLICStateClass),
> .class_init= kvm_s390_flic_class_init,
> };
>
Re: [Qemu-devel] [PATCH 6/7] s390x: return unavailable features via query-cpu-definitions
On Tue, 4 Jul 2017 16:07:58 +0200 Christian Borntraeger wrote: > From: Viktor Mihajlovski > > The response for query-cpu-definitions didn't include the > unavailable-features field, which is used by libvirt to figure > out whether a certain cpu model is usable on the host. > > The unavailable features are now computed by obtaining the host CPU > model and comparing it against the known CPU models. The comparison > takes into account the generation, the GA level and the feature > bitmaps. In the case of a CPU generation/GA level mismatch > a feature called "type" is reported to be missing. > > As a result, the output of virsh domcapabilities would change > from something like > ... > > z10EC-base > z9EC-base > z196.2-base > z900-base > z990 > ... > to > ... > > z10EC-base > z9EC-base > z196.2-base > z900-base > z990 > ... > > Signed-off-by: Viktor Mihajlovski > Message-Id: <1499082529-16970-1-git-send-email-mihaj...@linux.vnet.ibm.com> > Reviewed-by: David Hildenbrand > Signed-off-by: Christian Borntraeger > --- > target/s390x/cpu_models.c | 62 > +++ > 1 file changed, 57 insertions(+), 5 deletions(-) Acked-by: Cornelia Huck
Re: [Qemu-devel] [PATCH v2] blockdev: Print a warning for legacy drive options that belong to -device
Am 12.05.2017 um 12:33 hat Thomas Huth geschrieben: > We likely do not want to carry these legacy -drive options along forever. > Let's emit a deprecation warning for the -drive options that have a > replacement with the -device option, so that the (hopefully few) remaining > users are aware of this and can adapt their scripts / behaviour accordingly. > > Signed-off-by: Thomas Huth > --- > v2: > - Check for !qtest_enabled() since tests/hd-geo-test still uses these > - Added "addr" to the list, too > - Also mark the options as deprecated in the documentation Thanks, added the missing deprecation note for 'serial' in the documentation and applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] qemu-doc: Add missing "@c man end" statements
Am 19.06.2017 um 11:16 hat Thomas Huth geschrieben:
> Since commit 3f2ce724f1f1 ("Move the qemu-ga description into a
> separate chapter"), the qemu.1 man page looks pretty much screwed
> up, e.g. the title was "qemu-ga - QEMU Guest Agent" instead of
> "qemu-doc - QEMU Emulator User Documentation". However, that movement
> of the gemu-ga chapter is not the real problem, it just triggered
> another bug in the qemu-doc.texi: There are some parts in the file
> which introduce a "@c man begin OPTIONS" section, but never close
> it again with "@c man end". After adding the proper end tags here,
> the title of the man page is right again and the previously wrongly
> tagged sections now also show up correctly in the man page, too.
>
> Reported-by: Kevin Wolf
> Signed-off-by: Thomas Huth
Michael, do you want to take this through qemu-trivial?
Kevin
Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize
On 07/04/2017 04:31 PM, Cornelia Huck wrote:
> On Tue, 4 Jul 2017 16:07:55 +0200
> Christian Borntraeger wrote:
>
>> From: Halil Pasic
>>
>> From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
>> implement floating-interrupt controller device", 2013-07-16) the kvm-flic
>> is not making realize fail properly in case it's impossible to create the
>> KVM device which basically serves as a backend and is absolutely
>> essential for having an operational kvm-flic.
>>
>> Let's fix this by making sure we do proper error propagation in realize.
>>
>> Signed-off-by: Halil Pasic
>> Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
>> Reviewed-by: Dong Jia Shi
>> Reviewed-by: Yi Min Zhao
>> Signed-off-by: Christian Borntraeger
>> ---
>> hw/intc/s390_flic_kvm.c | 14 +++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>
> (...)
>
>> cd.type = KVM_DEV_TYPE_FLIC;
>> ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>> if (ret < 0) {
>> -trace_flic_create_device(errno);
>> -return;
>> +error_setg_errno(&errp_local, errno, "Creating the KVM device
>> failed");
>> +trace_flic_no_device_api(errno);
>
> Err... this should still be trace_flic_create_device(), no?
I'm afraid you are right! Probably a copy paste error :/
>
>> +goto fail;
>> }
>> flic_state->fd = cd.fd;
>
[Qemu-devel] [RISU PATCH 01/11] risu: make match status take tracing into account
Signed-off-by: Alex Bennée
---
reginfo.c | 14 +-
risu.c| 4 ++--
risu.h| 2 +-
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/reginfo.c b/reginfo.c
index 13879d5..d9d37b3 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -138,7 +138,7 @@ int recv_and_compare_register_info(read_fn read_fn,
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(void)
+int report_match_status(int trace)
{
int resp = 0;
fprintf(stderr, "match status...\n");
@@ -148,7 +148,7 @@ int report_match_status(void)
/* We don't have valid reginfo from the apprentice side
* so stop now rather than printing anything about it.
*/
-fprintf(stderr, "master reginfo:\n");
+fprintf(stderr, "%s reginfo:\n", trace ? "this":"master");
reginfo_dump(&master_ri, stderr);
return 1;
}
@@ -166,11 +166,15 @@ int report_match_status(void)
return 0;
}
-fprintf(stderr, "master reginfo:\n");
+fprintf(stderr, "%s reginfo:\n", trace ? "this":"master");
reginfo_dump(&master_ri, stderr);
-fprintf(stderr, "apprentice reginfo:\n");
+fprintf(stderr, "%s reginfo:\n", trace ? "trace":"apprentice");
reginfo_dump(&apprentice_ri, stderr);
-reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
+if (trace) {
+reginfo_dump_mismatch(&apprentice_ri, &master_ri, stderr);
+} else {
+reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
+}
return resp;
}
diff --git a/risu.c b/risu.c
index 6f213dc..47471c6 100644
--- a/risu.c
+++ b/risu.c
@@ -228,7 +228,7 @@ int master(void)
signal_count);
return 0;
} else {
-return report_match_status();
+return report_match_status(0);
}
}
set_sigill_handler(&master_sigill);
@@ -250,7 +250,7 @@ int apprentice(void)
#endif
close(apprentice_fd);
fprintf(stderr, "finished early after %zd checkpoints\n",
signal_count);
-return report_match_status();
+return report_match_status(1);
}
set_sigill_handler(&apprentice_sigill);
fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
diff --git a/risu.h b/risu.h
index 9f15662..1c8ecee 100644
--- a/risu.h
+++ b/risu.h
@@ -91,7 +91,7 @@ int recv_and_compare_register_info(read_fn read_fn,
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(void);
+int report_match_status(int trace);
/* Interface provided by CPU-specific code: */
--
2.13.0
[Qemu-devel] [RISU PATCH 03/11] README: document --static builds
Signed-off-by: Alex Bennée --- README | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/README b/README index 858a349..e0bf5c5 100644 --- a/README +++ b/README @@ -17,7 +17,7 @@ test blobs (which can be run anywhere), and a Linux executable 'risu' which runs on the target architecture (ie ARM). To build the executable part: -[VAR=VALUE] ... ./configure +[VAR=VALUE] ... ./configure [--static] make where [VAR=VALUE] ... allows you to specify any options. @@ -25,6 +25,10 @@ Most useful is CROSS_PREFIX= which specifies the cross compiler prefix; you'll need this if you're not building on the target system (Example: CROSS_PREFIX=arm-linux-gnueabihf- ) + +Passing --static will build a statically linked binary which is useful +if you don't want to mess around with chroot's to run the binary. + For other possibilities run 'configure --help'. Building into a separate build tree from the source code is supported: @@ -86,8 +90,14 @@ as simple as: However since you actually need to run it under qemu or similar you probably need an ARM chroot to run it in, and to do something -like - sudo chroot /srv/chroot/arm-mav /risu --host ipaddr vqshlimm.out +like: + + sudo chroot /srv/chroot/arm-mav /risu --host ipaddr vqshlimm.out + +If you built the binary statically you can simply run: + + /path/to/qemu ./risu --host ipaddr vqshlimm.out + When the apprentice connects to the master, they will both start running the binary and checking results with each other. When the -- 2.13.0
[Qemu-devel] [RISU PATCH 00/11] Misc fixes, documentation and patterns
Hi Peter, A bit of a mixed set of patches here for you to pick from as you will. The first 2 are additional tracing fixes including a fix for segfaulting when generating a trace. The next two are documentation patches as requested. Then two minor tweaks, one to risu.el and a indent failure of risugen. While I've been working through the half-precision stuff I've been finding the --pattern approach a bit inflexible as you basically end up eyeballing the risu file to build up the regexs. I've added a little syntactic suger to the risu file to enable grouping. This makes slicing a subset a lot easier, e.g.: ./risugen --group AdvSIMDAcrossVector --not-pattern ".*_RES" aarch64.risu You can specify multiple groups in the risu file as the matching is string based, e.g.: @Integer,Logic,Immediate And then: ./risugen --group Logic aarch64.risu foo.bin What do you think? Alex Bennée (11): risu: make match status take tracing into account reginfo.c: always return 1 on OP_TESTEND README: document --static builds README: document record/replay support risu.el: derive from text-mode risugen: fix bad indent risugen: support @GroupName in risu files aarch64.risu: document naming conventions aarch64.risu: remove duplicate AdvSIMD Scalar 3 same block aarch64.risu: remove duplicate AdvSIMD scalar 2 reg misc block aarch64.risu: update AdvancedSIMD across lanes README | 40 -- aarch64.risu | 241 - reginfo.c | 19 +++-- risu.c | 4 +- risu.el| 2 +- risu.h | 2 +- risugen| 17 +++- risugen_arm.pm | 7 ++ 8 files changed, 175 insertions(+), 157 deletions(-) -- 2.13.0
[Qemu-devel] [RISU PATCH 04/11] README: document record/replay support
Signed-off-by: Alex Bennée --- README | 24 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/README b/README index e0bf5c5..5f16f60 100644 --- a/README +++ b/README @@ -107,6 +107,26 @@ mismatch status to its standard output. NB that in the register dump the r15 (pc) value will be given as an offset from the start of the binary, not an absolute value. +While the master/slave setup works well it is a bit fiddly for running +regression tests and other sorts of automation. For this reason risu +supports recording a trace of its execution to a file. For example: + + risu --master FxxV_across_lanes.risu.bin -t FxxV_across_lanes.risu.trace + +And then playback with: + + risu FxxV_across_lanes.risu.bin -t FxxV_across_lanes.risu.trace + +Ideally it should be built with zlib to compress the trace files which +would otherwise be huge. If building with zlib proves too tricky you +can pipe to stdout and an external compression binary using "-t -". + + risu --master FxxV_across_lanes.risu.bin -t - | gzip --best > trace.file + +and: + + gunzip -c trace.file | risu -t - FxxV_across_lanes.risu.bin + File format --- @@ -203,10 +223,6 @@ implementation, for example) but only ARM is tested. * we don't actually compare FP status flags, simply because I'm pretty sure qemu doesn't get them right yet and I'm more interested in fixing gross bugs first. - * there isn't currently any support for a "record and replay -results" mode. This would allow you to record the correct -results from the ARM host once and then test a model implementation -even if you didn't have the corresponding native hardware. * You can compile statically to avoid the requirement for the ARM chroot for qemu testing but you can no longer use gethostbyname() and need to specify your hosts by IP address. -- 2.13.0
[Qemu-devel] [RISU PATCH 05/11] risu.el: derive from text-mode
As RISU files have copious commentary it seems better to derive from text-mode so we can access things like spell-checker short cuts ;-) Signed-off-by: Alex Bennée --- risu.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/risu.el b/risu.el index dff9337..1875d02 100644 --- a/risu.el +++ b/risu.el @@ -39,7 +39,7 @@ ;;; Code -(define-derived-mode risu-mode fundamental-mode "RISU" +(define-derived-mode risu-mode text-mode "RISU" "Major mode for editing RISU control files." (set (make-local-variable 'font-lock-defaults) '(risu-font-lock-keywords))) -- 2.13.0
[Qemu-devel] [RISU PATCH 06/11] risugen: fix bad indent
Signed-off-by: Alex Bennée --- risugen | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/risugen b/risugen index 8b20425..347cf12 100755 --- a/risugen +++ b/risugen @@ -262,7 +262,7 @@ Valid options: These REs are applied after the matching pattern which is useful if you want to exclude a specific instruction from a general set you have excluded. - --no-fp : disable floating point: no fp init, randomization etc. +--no-fp : disable floating point: no fp init, randomization etc. Useful to test before support for FP is available. --be : generate instructions in Big-Endian byte order (ppc64 only). --help : print this message -- 2.13.0
[Qemu-devel] [RISU PATCH 02/11] reginfo.c: always return 1 on OP_TESTEND
In the master/apprentice setup the response byte of 1 is returned by
write_fn. However when tracing it will happily report 0 as it
successfully writes the last bytes. To avoid running of the end when
tracing we just always return 1 at this point.
Signed-off-by: Alex Bennée
---
reginfo.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/reginfo.c b/reginfo.c
index d9d37b3..76f24f9 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -39,7 +39,10 @@ int send_register_info(write_fn write_fn, void *uc)
switch (op) {
case OP_TESTEND:
-return write_fn(&ri, sizeof(ri));
+write_fn(&ri, sizeof(ri));
+/* if we are tracing write_fn will return 0 unlike a remote
+ end, hence we force return of 1 here */
+return 1;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
break;
--
2.13.0
[Qemu-devel] [RISU PATCH 07/11] risugen: support @GroupName in risu files
The existing pattern support is useful but it does get a little
tedious when faced with large groups of instructions. This introduces
the concept of a @GroupName which can be sprinkled in the risu
definition and is attached to all instructions following its
definition until the next group or an empty group "@" is specified.
It can be combined with the existing pattern support to do things
like:
./risugen --group AdvSIMDAcrossVector --not-pattern ".*_RES" aarch64.risu
foo.bin
Signed-off-by: Alex Bennée
---
risugen| 15 +++
risugen_arm.pm | 7 +++
2 files changed, 22 insertions(+)
diff --git a/risugen b/risugen
index 347cf12..97ffa83 100755
--- a/risugen
+++ b/risugen
@@ -30,7 +30,10 @@ my %insn_details;
# The arch will be selected based on .mode directive defined in risu file.
my $arch = "";
+# Current group, updated by @GroupName
+my $insn_group = "";
+my @group = (); # include groups
my @pattern_re = ();# include pattern
my @not_pattern_re = ();# exclude pattern
@@ -118,6 +121,11 @@ sub parse_config_file($)
exit(1);
}
+if ($tokens[0] =~ /^@(.*)/ ) {
+$insn_group = $1;
+next;
+}
+
if ($tokens[0] =~ /^\./) {
parse_risu_directive($file, $seen_pattern, @tokens);
next;
@@ -235,6 +243,9 @@ sub parse_config_file($)
$insnrec->{fixedbits} = $fixedbits;
$insnrec->{fixedbitmask} = $fixedbitmask;
$insnrec->{fields} = [ @fields ];
+if (length $insn_group) {
+$insnrec->{group} = $insn_group;
+}
$insn_details{$insnname} = $insnrec;
}
close(CFILE) or die "can't close $file: $!";
@@ -253,6 +264,7 @@ Valid options:
--fpscr n: set initial FPSCR (arm) or FPCR (aarch64) value (default is
0)
--condprob p : [ARM only] make instructions conditional with probability p
(default is 0, ie all instructions are always executed)
+--group name[,name..]: only use instructions in defined groups
--pattern re[,re...] : only use instructions matching regular expression
Each re must match a full word (that is, we match on
the perl regex '\\b((re)|(re))\\b'). This means that
@@ -281,6 +293,7 @@ sub main()
GetOptions( "help" => sub { usage(); exit(0); },
"numinsns=i" => \$numinsns,
"fpscr=o" => \$fpscr,
+"group=s" => \@group,
"pattern=s" => \@pattern_re,
"not-pattern=s" => \@not_pattern_re,
"condprob=f" => sub {
@@ -295,6 +308,7 @@ sub main()
# allow "--pattern re,re" and "--pattern re --pattern re"
@pattern_re = split(/,/,join(',',@pattern_re));
@not_pattern_re = split(/,/,join(',',@not_pattern_re));
+@group = split(/,/,join(',',@group));
if ($#ARGV != 1) {
usage();
@@ -316,6 +330,7 @@ sub main()
'numinsns' => $numinsns,
'fp_enabled' => $fp_enabled,
'outfile' => $outfile,
+'group' => \@group,
'pattern_re' => \@pattern_re,
'not_pattern_re' => \@not_pattern_re,
'details' => \%insn_details,
diff --git a/risugen_arm.pm b/risugen_arm.pm
index 1024660..8ad208a 100644
--- a/risugen_arm.pm
+++ b/risugen_arm.pm
@@ -895,6 +895,7 @@ sub write_test_code()
my $fp_enabled = $params->{ 'fp_enabled' };
my $outfile = $params->{ 'outfile' };
+my @group = @{ $params->{ 'group' } };
my @pattern_re = @{ $params->{ 'pattern_re' } };
my @not_pattern_re = @{ $params->{ 'not_pattern_re' } };
my %insn_details = %{ $params->{ 'details' } };
@@ -910,6 +911,12 @@ sub write_test_code()
# Get a list of the insn keys which are permitted by the re patterns
my @keys = sort keys %insn_details;
+if (@group) {
+my $re = join("|",@group);
+@keys = grep {
+defined($insn_details{$_}->{group}) &&
+grep /$re/, $insn_details{$_}->{group}} @keys
+}
if (@pattern_re) {
my $re = '\b((' . join(')|(',@pattern_re) . '))\b';
@keys = grep /$re/, @keys;
--
2.13.0
[Qemu-devel] [RISU PATCH 09/11] aarch64.risu: remove duplicate AdvSIMD Scalar 3 same block
A chunk of the AArch64 definitions repeat themselves. Clean that up.
Signed-off-by: Alex Bennée
---
aarch64.risu | 25 +++--
1 file changed, 3 insertions(+), 22 deletions(-)
diff --git a/aarch64.risu b/aarch64.risu
index bfca45f..609021a 100644
--- a/aarch64.risu
+++ b/aarch64.risu
@@ -2130,6 +2130,8 @@ SQDMULL_S3D A64_V 0 1 U 0 size:2 1 rm:5 1101 00 rn:5
rd:5
# 31 30 29 28 27 26 25 24 23 22 21 2016 15 11 10 95 40
# 0 1 U 1 1 1 1 0 size 1 [ Rm ] [ opcode ] 1 [ Rn ] [ Rd ]
#
+@AdvSIMDScalar3Same
+
SQADDA64_V 01 0 0 size:2 1 rm:5 1 1 rn:5 rd:5
SQSUBA64_V 01 0 0 size:2 1 rm:5 00101 1 rn:5 rd:5
CMGT A64_V 01 0 0 size:2 1 rm:5 00110 1 rn:5 rd:5
@@ -2167,28 +2169,7 @@ FCMGTA64_V 01 1 0 1 size:1 1 rm:5 11100 1 rn:5
rd:5
FACGTA64_V 01 1 0 1 size:1 1 rm:5 11101 1 rn:5 rd:5 \
!constraints { $size != 11; }
-CMTSTA64_v 01 0 0 size:2 1 rm:5 10001 1 rn:5 rd:5
-SQDMULH A64_v 01 0 0 size:2 1 rm:5 10110 1 rn:5 rd:5
-FMULXA64_v 01 0 0 0 size:1 1 rm:5 11011 1 rn:5 rd:5
-FCMEQA64_v 01 0 0 0 size:1 1 rm:5 11100 1 rn:5 rd:5
-FRECPS A64_v 01 0 0 0 size:1 1 rm:5 1 1 rn:5 rd:5
-FRSQRTS A64_v 01 0 0 1 size:1 1 rm:5 1 1 rn:5 rd:5
-UQADDA64_v 01 1 0 size:2 1 rm:5 1 1 rn:5 rd:5
-UQSUBA64_v 01 1 0 size:2 1 rm:5 00101 1 rn:5 rd:5
-CMHI A64_v 01 1 0 size:2 1 rm:5 00110 1 rn:5 rd:5
-CMHS A64_v 01 1 0 size:2 1 rm:5 00111 1 rn:5 rd:5
-USHL A64_v 01 1 0 size:2 1 rm:5 01000 1 rn:5 rd:5
-UQSHLA64_v 01 1 0 size:2 1 rm:5 01001 1 rn:5 rd:5
-URSHLA64_v 01 1 0 size:2 1 rm:5 01010 1 rn:5 rd:5
-UQRSHL A64_v 01 1 0 size:2 1 rm:5 01011 1 rn:5 rd:5
-SUBv A64_v 01 1 0 size:2 1 rm:5 1 1 rn:5 rd:5
-CMEQ A64_v 01 1 0 size:2 1 rm:5 10001 1 rn:5 rd:5
-SQRDMULH A64_v 01 1 0 size:2 1 rm:5 10110 1 rn:5 rd:5
-FCMGEA64_v 01 1 0 0 size:1 1 rm:5 11100 1 rn:5 rd:5
-FACGEA64_v 01 1 0 0 size:1 1 rm:5 11101 1 rn:5 rd:5
-FABD A64_v 01 1 0 1 size:1 1 rm:5 11010 1 rn:5 rd:5
-FCMGTA64_v 01 1 0 1 size:1 1 rm:5 11100 1 rn:5 rd:5
-FACGTA64_v 01 1 0 1 size:1 1 rm:5 11101 1 rn:5 rd:5
+@
# C3.6.12 AdvSIMD scalar 2reg misc
CMGTzero A64_V 01 0 0 size:2 1 01000 10 rn:5 rd:5
--
2.13.0
Re: [Qemu-devel] [PATCH 2/5] spapr: Refactor spapr_drc_detach()
On Thu, Jun 22, 2017 at 11:32:15AM +0200, Greg Kurz wrote: > On Wed, 21 Jun 2017 17:18:45 +0800 > David Gibson wrote: > > > This function has two unused parameters - remove them. > > > > It's ok for the d argument but I'm not sure about errp... Indeed it isn't used > in the current code, but looking at the paths below spapr_drc_detach(), we > have: > > spapr_drc_detach() > spapr_drc_release() > spapr_core_release() >hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort); > > and > > spapr_drc_detach() > spapr_drc_release() > spapr_lmb_release() >hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort); > > Shouldn't we pass the errp down to hotplug_handler_unplug() instead of > aborting ? So, at first I thought you were right, and was going to rewrite on that basis. Then I realized that calling hotplug_handler_unplug() is a bizarrely roundabout way of doing what we need anyway (usually the unplug handler is never called if there is an unplug_request() handler). Once we expand those out to what they actually call, the errors disappear again. I'll do that in the next spin. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[Qemu-devel] [RISU PATCH 10/11] aarch64.risu: remove duplicate AdvSIMD scalar 2 reg misc block
While at that also: - sort alphabetically - add to @AdvSIMDScalar2RegMisc group Signed-off-by: Alex Bennée --- aarch64.risu | 114 --- 1 file changed, 39 insertions(+), 75 deletions(-) diff --git a/aarch64.risu b/aarch64.risu index 609021a..5450cd3 100644 --- a/aarch64.risu +++ b/aarch64.risu @@ -2171,85 +2171,49 @@ FACGTA64_V 01 1 0 1 size:1 1 rm:5 11101 1 rn:5 rd:5 \ @ -# C3.6.12 AdvSIMD scalar 2reg misc -CMGTzero A64_V 01 0 0 size:2 1 01000 10 rn:5 rd:5 -CMGEzero A64_V 01 1 0 size:2 1 01000 10 rn:5 rd:5 -CMEQzero A64_V 01 0 0 size:2 1 01001 10 rn:5 rd:5 -CMLEzero A64_V 01 1 0 size:2 1 01001 10 rn:5 rd:5 -CMLTzero A64_V 01 0 0 size:2 1 01010 10 rn:5 rd:5 -ABS A64_V 01 0 0 size:2 1 01011 10 rn:5 rd:5 -NEG A64_V 01 1 0 size:2 1 01011 10 rn:5 rd:5 - -FCMGT_S2MISC A64_V 01 0 0 size:2 1 01100 10 rn:5 rd:5 -FCMEQ_S2MISC A64_V 01 0 0 size:2 1 01101 10 rn:5 rd:5 -FCMLT_S2MISC A64_V 01 0 0 size:2 1 01110 10 rn:5 rd:5 -FCMGE_S2MISC A64_V 01 1 0 size:2 1 01100 10 rn:5 rd:5 -FCMLE_S2MISC A64_V 01 1 0 size:2 1 01101 10 rn:5 rd:5 - -SCVTF_S2MISC A64_V 01 0 0 0 sz 1 11101 10 rn:5 rd:5 -UCVTF_S2MISC A64_V 01 1 0 0 sz 1 11101 10 rn:5 rd:5 - -FCVTNS_S2MISC A64_V 01 0 0 0 sz 1 11010 10 rn:5 rd:5 -FCVTMS_S2MISC A64_V 01 0 0 0 sz 1 11011 10 rn:5 rd:5 -FCVTAS_S2MISC A64_V 01 0 0 0 sz 1 11100 10 rn:5 rd:5 -FCVTPS_S2MISC A64_V 01 0 0 1 sz 1 11010 10 rn:5 rd:5 -FCVTZS_S2MISC A64_V 01 0 0 1 sz 1 11011 10 rn:5 rd:5 - -FCVTNU_S2MISC A64_V 01 1 0 0 sz 1 11010 10 rn:5 rd:5 -FCVTMU_S2MISC A64_V 01 1 0 0 sz 1 11011 10 rn:5 rd:5 -FCVTAU_S2MISC A64_V 01 1 0 0 sz 1 11100 10 rn:5 rd:5 -FCVTPU_S2MISC A64_V 01 1 0 1 sz 1 11010 10 rn:5 rd:5 -FCVTZU_S2MISC A64_V 01 1 0 1 sz 1 11011 10 rn:5 rd:5 - -FCVTXN_S2MISC A64_V 01 1 0 0 sz 1 10110 10 rn:5 rd:5 - -SUQADD_S2MISC A64_V 01 0 0 size:2 1 00011 10 rn:5 rd:5 -USQADD_S2MISC A64_V 01 1 0 size:2 1 00011 10 rn:5 rd:5 -SQABS_S2MISC A64_V 01 0 0 size:2 1 00111 10 rn:5 rd:5 -SQNEG_S2MISC A64_V 01 1 0 size:2 1 00111 10 rn:5 rd:5 - -# XXX lots of others in this group - # C3.6.12 AdvSIMD scalar two-reg misc # 31 30 29 28 27 26 25 24 23 22 21 20 16 12 11 10 9 5 40 # 0 1 U 1 1 1 1 0 size 1 0 0 0 0 [ opcode ] 1 0 [ Rn ] [ Rd ] # U size opcode -SUQADDs A64_V 01 0 0 size:2 1 00011 10 rn:5 rd:5 -SQABSs A64_V 01 0 0 size:2 1 00111 10 rn:5 rd:5 -CMGTzs A64_V 01 0 0 size:2 1 01000 10 rn:5 rd:5 -CMEQzs A64_V 01 0 0 size:2 1 01001 10 rn:5 rd:5 -CMLTzs A64_V 01 0 0 size:2 1 01010 10 rn:5 rd:5 -ABSs A64_V 01 0 0 size:2 1 01011 10 rn:5 rd:5 -SQXTN_SQXTN2s A64_V01 0 0 size:2 1 10100 10 rn:5 rd:5 -FCVTNSvs A64_V 01 0 0 0 size:1 1 11010 10 rn:5 rd:5 -FCVTMSvs A64_V 01 0 0 0 size:1 1 11011 10 rn:5 rd:5 -FCVTASvs A64_V 01 0 0 0 size:1 1 11100 10 rn:5 rd:5 -SCVTFvis A64_V 01 0 0 0 size:1 1 11101 10 rn:5 rd:5 -FCMGTzs A64_V 01 0 0 1 size:1 1 01100 10 rn:5 rd:5 -FCMEQzs A64_V 01 0 0 1 size:1 1 01101 10 rn:5 rd:5 -FCMLTzs A64_V 01 0 0 1 size:1 1 01110 10 rn:5 rd:5 -FCVTPSvs A64_V 01 0 0 1 size:1 1 11010 10 rn:5 rd:5 -FCVTZSvis A64_V01 0 0 1 size:1 1 11011 10 rn:5 rd:5 -FRECPEs A64_V 01 0 0 1 size:1 1 11101 10 rn:5 rd:5 -FRECPX A64_V 01 0 0 1 size:1 1 1 10 rn:5 rd:5 -USQADDs A64_V 01 1 0 size:2 1 00011 10 rn:5 rd:5 -SQNEGs A64_V 01 1 0 size:2 1 00111 10 rn:5 rd:5 -CMGzs A64_V01 1 0 size:2 1 01000 10 rn:5 rd:5 -CMLEzs A64_V 01 1 0 size:2 1 01001 10 rn:5 rd:5 -NEGvs A64_V01 1 0 size:2 1 01011 10 rn:5 rd:5 -SQXTUN_SQXTUN2s A64_V 01 1 0 size:2 1 10010 10 rn:5 rd:5 -UQXTN_UQXTN2s A64_V01 1 0 size:2 1 10100 10 rn:5 rd:5 -FCVTXN_FCVTXN2s A64_V 01 1 0 0 size:1 1 10110 10 rn:5 rd:5 -FCVTNUvs A64_V 01 1 0 0 size:1 1 11010 10 rn:5 rd:5 -FCVTMUvs A64_V 01 1 0 0 size:1 1 11011 10 rn:5 rd:5 -FCVTAUvs A64_V 01 1 0 0 size:1 1 11100 10 rn:5 rd:5 -UCVTFvis A64_V 01 1 0 0 size:1 1 11101 10 rn:5 rd:5 -FCMGEzs A64_V 01 1 0 1 size:1 1 01100 10 rn:5 rd:5 -FCMLEzs A64_V 01 1 0 1 size:1 1 01101 10 rn:5 rd:5 -FCVTPUvs A64_V 01 1 0 1 size:1 1 11010 10 rn:5 rd:5 -FCVTZUvis A64_V01 1 0 1 size:1 1 11011 10 rn:5 rd:5 -FRSQRTE
[Qemu-devel] [RISU PATCH 08/11] aarch64.risu: document naming conventions
Signed-off-by: Alex Bennée --- aarch64.risu | 10 ++ 1 file changed, 10 insertions(+) diff --git a/aarch64.risu b/aarch64.risu index 2f3341c..bfca45f 100644 --- a/aarch64.risu +++ b/aarch64.risu @@ -7,6 +7,7 @@ # # Contributors: # Claudio Fontana - initial implementation +# Alex Bennée - a number of additions including v8.2 FP16 # based on arm.risu by Peter Maydell ### @@ -19,6 +20,15 @@ # XXX NIY: SP-related instructions # XXX NIY: floating point and SIMD specific insns +# Instruction suffixes to identify variants +# m - memory (loads/stores) +# s - scalar +# v - vector +# z - zero (e.g. compare to zero) +# f - fixed point +# +# _FP16 for ARMv8.2 half-precision extensions + # - - - - 1 - 0 - - - - - - - - - - - - - - - Loads and stores # C3.3 Loads and stores -- 2.13.0
[Qemu-devel] [RISU PATCH 11/11] aarch64.risu: update AdvancedSIMD across lanes
- sorted alphabetically
- aligned the instructions patterns
- adding half-precision F[MAX|MIN][NMV|V]
- add @AdvSIMDAcrossVector group
Signed-off-by: Alex Bennée
---
aarch64.risu | 90 +---
1 file changed, 49 insertions(+), 41 deletions(-)
diff --git a/aarch64.risu b/aarch64.risu
index 5450cd3..215882e 100644
--- a/aarch64.risu
+++ b/aarch64.risu
@@ -1955,50 +1955,58 @@ ZIP2 A64_V 0 Q:1 001110 size:2 0 rm:5 0 111 10 rn:5
rd:5 \
# ReservedValue: break the !($size == 3 && $Q == 0) constraint
ZIP2_RES A64_V 0 0 001110 11 0 rm:5 0 111 10 rn:5 rd:5
-# C3.6.4 AdvSIMD across lanes
+# C4-286 AdvSIMD across vector lanes
# 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 12 11 10 9 5 4 0
# 0 Q U 0 1 1 1 0 size 1 1 0 0 0 opcode 1 0 Rn Rd
+@AdvSIMDAcrossVector
+
+ADDV A64_V 0 Q:1 0 01110 s:2 11000 11011 10 rn:5 rd:5 \
+!constraints { $s < 2 || ($s == 2 && $Q == 1); }
+# ReservedValue: break the constraint (s==2) => (Q=1)
+ADDV_RES A64_V 0 0 0 01110 10 11000 11011 10 rn:5 rd:5
+
+FMAXNMV A64_V 0 1 1 01110 00 11000 01100 10 rn:5 rd:5
+FMAXVA64_V 0 1 1 01110 00 11000 0 10 rn:5 rd:5
+FMINNMV A64_V 0 1 1 01110 10 11000 01100 10 rn:5 rd:5
+FMINVA64_V 0 1 1 01110 10 11000 0 10 rn:5 rd:5
+
+# ARMv8.2 Half-precision variants
+FMAXNMV_FP16 A64_V 0 q:1 0 01110 00 11000 01100 10 rn:5 rd:5
+FMAXV_FP16 A64_V 0 q:1 0 01110 00 11000 0 10 rn:5 rd:5
+FMINNMV_FP16 A64_V 0 q:1 0 01110 10 11000 01100 10 rn:5 rd:5
+FMINV_FP16 A64_V 0 q:1 0 01110 10 11000 0 10 rn:5 rd:5
+
+SADDLV A64_V 0 Q:1 0 01110 s:2 11000 00011 10 rn:5 rd:5 \
+!constraints { $s < 2 || ($s == 2 && $Q == 1); }
+# ReservedValue: break the constraint (s==2) => (Q=1)
+SADDLV_RES A64_V 0 0 0 01110 10 11000 00011 10 rn:5 rd:5
+
+SMAXVA64_V 0 Q:1 0 01110 s:2 11000 01010 10 rn:5 rd:5 \
+!constraints { $s < 2 || ($s == 2 && $Q == 1); }
+# ReservedValue: break the constraint (s==2) => (Q=1)
+SMAXV_RESA64_V 0 0 0 01110 10 11000 01010 10 rn:5 rd:5
+
+SMINVA64_V 0 Q:1 0 01110 s:2 11000 11010 10 rn:5 rd:5 \
+!constraints { $s < 2 || ($s == 2 && $Q == 1); }
+# ReservedValue: break the constraint (s==2) => (Q=1)
+SMINV_RESA64_V 0 0 0 01110 10 11000 11010 10 rn:5 rd:5
+
+UADDLV A64_V 0 Q:1 1 01110 s:2 11000 00011 10 rn:5 rd:5 \
+!constraints { $s < 2 || ($s == 2 && $Q == 1); }
+# ReservedValue: break the constraint (s==2) => (Q=1)
+UADDLV_RES A64_V 0 0 1 01110 10 11000 00011 10 rn:5 rd:5
+
+UMAXVA64_V 0 Q:1 1 01110 s:2 11000 01010 10 rn:5 rd:5 \
+!constraints { $s < 2 || ($s == 2 && $Q == 1); }
+# ReservedValue: break the constraint (s==2) => (Q=1)
+UMAXV_RESA64_V 0 0 1 01110 10 11000 01010 10 rn:5 rd:5
+
+UMINVA64_V 0 Q:1 1 01110 s:2 11000 11010 10 rn:5 rd:5 \
+!constraints { $s < 2 || ($s == 2 && $Q == 1); }
+# ReservedValue: break the constraint (s==2) => (Q=1)
+UMINV_RESA64_V 0 0 1 01110 10 11000 11010 10 rn:5 rd:5
-SADDLV A64_V 0 Q:1 0 01110 size:2 11000 00011 10 rn:5 rd:5 \
-!constraints { $size < 2 || ($size == 2 && $Q == 1); }
-# ReservedValue: break the constraint (size==2) => (Q=1)
-SADDLV_RES A64_V 0 0 0 01110 10 11000 00011 10 rn:5 rd:5
-
-SMAXV A64_V 0 Q:1 0 01110 size:2 11000 01010 10 rn:5 rd:5 \
-!constraints { $size < 2 || ($size == 2 && $Q == 1); }
-# ReservedValue: break the constraint (size==2) => (Q=1)
-SMAXV_RES A64_V 0 0 0 01110 10 11000 01010 10 rn:5 rd:5
-
-SMINV A64_V 0 Q:1 0 01110 size:2 11000 11010 10 rn:5 rd:5 \
-!constraints { $size < 2 || ($size == 2 && $Q == 1); }
-# ReservedValue: break the constraint (size==2) => (Q=1)
-SMINV_RES A64_V 0 0 0 01110 10 11000 11010 10 rn:5 rd:5
-
-ADDV A64_V 0 Q:1 0 01110 size:2 11000 11011 10 rn:5 rd:5 \
-!constraints { $size < 2 || ($size == 2 && $Q == 1); }
-# ReservedValue: break the constraint (size==2) => (Q=1)
-ADDV_RES A64_V 0 0 0 01110 10 11000 11011 10 rn:5 rd:5
-
-UADDLV A64_V 0 Q:1 1 01110 size:2 11000 00011 10 rn:5 rd:5 \
-!constraints { $size < 2 || ($size == 2 && $Q == 1); }
-# ReservedValue: break the constraint (size==2) => (Q=1)
-UADDLV_RES A64_V 0 0 1 01110 10 11000 00011 10 rn:5 rd:5
-
-UMAXV A64_V 0 Q:1 1 01110 size:2 11000 01010 10 rn:5 rd:5 \
-!constraints { $size < 2 || ($size == 2 && $Q == 1); }
-# ReservedValue: break the constraint (size==2) => (Q=1)
-UMAXV_RES A64_V 0 0 1 01110 10 11000 01010 10 rn:5 rd:5
-
-UMINV A64_V 0 Q:1 1 01110 size:2 11000 11010 10 rn:5 rd:5 \
-!constraints { $size < 2 || ($size == 2 && $Q == 1); }
-# ReservedValue: break the constraint (size==2) => (Q=1)
-UMINV_RES A64_V 0 0 1 01110 10 11000 11010 10 rn:5 rd:5
-
-FMAXNMV A64_V 0 1 1 01110 00 11000 01100 10 rn:5 rd:5
-FMAXV A64_V 0 1 1 01110 00 11000 0 10 rn:5 rd:5
-
-FMINNMV A64_V 0 1 1 01110 10 11000 01100 10 rn:5 rd:5
-FMINV A64_V 0 1 1 01110 10 11000 0 10 rn:5 rd:5
+@
# C3.6.5 AdvSIMD copy
# 31 30 29 28 27 26 25 24 23 22 21 2016 15 1411 10 9 5 4 0
--
Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic
On 07/04/2017 04:37 PM, Cornelia Huck wrote:
> On Tue, 4 Jul 2017 16:07:56 +0200
> Christian Borntraeger wrote:
>
>> From: Halil Pasic
>>
>> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
>> 2016-12-09) introduces a common realize (intended to be common for all
>> the subclasses) for flic, but fails to make sure the kvm-flic which had
>> it's own is actually calling this common realize.
>
> s/it's/its/
>
Valid. Sorry.
>>
>> This omission fortunately does not result in a grave problem. The common
>> realize was only supposed to catch a possible programming mistake by
>> validating a value of a property set via the compat machine macros. Since
>> there was no programming mistake we don't need this fixed for stable.
>>
>> Let's fix this problem by making sure kvm flic honors the realize of its
>> parent class.
>>
>> Let us also improve on the error message we would hypothetically emit
>> when the validation fails.
>>
>> Signed-off-by: Halil Pasic
>> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
>> Reviewed-by: Dong Jia Shi
>> Reviewed-by: Yi Min Zhao
>> Signed-off-by: Christian Borntraeger
>> ---
>> hw/intc/s390_flic.c | 4 ++--
>> hw/intc/s390_flic_kvm.c | 17 +
>> 2 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
>> index a99a350..837158b 100644
>> --- a/hw/intc/s390_flic.c
>> +++ b/hw/intc/s390_flic.c
>> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev,
>> Error **errp)
>> uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
>>
>> if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
>> -error_setg(errp, "flic adapter_routes_max_batch too big"
>> - "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>> +error_setg(errp, "flic property adapter_routes_max_batch too big"
>> + " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>
> Unrelated message change?
>
I've mentioned it in the commit message. It was also introduced by the
patch I'm fixing. But yes strictly it's two different problems.
>> }
>> }
>>
>> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
>> index bea3997..535d99d 100644
>> --- a/hw/intc/s390_flic_kvm.c
>> +++ b/hw/intc/s390_flic_kvm.c
>> @@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate =
>> {
>> }
>> };
>>
>> +typedef struct KVMS390FLICStateClass {
>> +S390FLICStateClass parent_class;
>> +DeviceRealize parent_realize;
>> +} KVMS390FLICStateClass;
>> +
>> +#define KVM_S390_FLIC_GET_CLASS(obj) \
>> +OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
>> +
>> +#define KVM_S390_FLIC_CLASS(klass) \
>> +OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
>> +
>> static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>> {
>> KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
>> @@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev,
>> Error **errp)
>> int ret;
>> Error *errp_local = NULL;
>>
>> +KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
>
> I usually prefer to introduce a local variable for that, but don't care
> too much.
>
>> +if (errp_local) {
>> +goto fail;
>> +}
>> flic_state->fd = -1;
>> if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>> error_setg_errno(&errp_local, errno, "KVM is missing capability"
>> @@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc,
>> void *data)
>> DeviceClass *dc = DEVICE_CLASS(oc);
>> S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
>>
>> +KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
>
> dito
>
>> dc->realize = kvm_s390_flic_realize;
>> dc->vmsd = &kvm_s390_flic_vmstate;
>> dc->reset = kvm_s390_flic_reset;
>> @@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
>> .name = TYPE_KVM_S390_FLIC,
>> .parent= TYPE_S390_FLIC_COMMON,
>> .instance_size = sizeof(KVMS390FLICState),
>> +.class_size= sizeof(KVMS390FLICStateClass),
>> .class_init= kvm_s390_flic_class_init,
>> };
>>
>
>
Re: [Qemu-devel] [PATCH v7 2/4] net/socket: Convert several helper functions to Error
Mao Zhongyi writes:
> Currently, net_socket_mcast_create(), net_socket_fd_init_dgram() and
> net_socket_fd_init() use the function such as fprintf(), perror() to
> report an error message.
>
> Now, convert these functions to Error.
>
> Cc: jasow...@redhat.com
> Cc: arm...@redhat.com
> Cc: berra...@redhat.com
> Signed-off-by: Mao Zhongyi
> Reviewed-by: Daniel P. Berrange
> ---
> net/socket.c | 81
> +---
> 1 file changed, 50 insertions(+), 31 deletions(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index 7d05e70..44fb504 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -209,7 +209,9 @@ static void net_socket_send_dgram(void *opaque)
> }
> }
>
> -static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct
> in_addr *localaddr)
> +static int net_socket_mcast_create(struct sockaddr_in *mcastaddr,
> + struct in_addr *localaddr,
> + Error **errp)
> {
> struct ip_mreq imr;
> int fd;
> @@ -221,16 +223,16 @@ static int net_socket_mcast_create(struct sockaddr_in
> *mcastaddr, struct in_addr
> #endif
>
> if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
> -fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) "
> -"does not contain a multicast address\n",
> -inet_ntoa(mcastaddr->sin_addr),
> -(int)ntohl(mcastaddr->sin_addr.s_addr));
> +error_setg(errp, "specified mcastaddr %s (0x%08x) "
> + "does not contain a multicast address",
> + inet_ntoa(mcastaddr->sin_addr),
> + (int)ntohl(mcastaddr->sin_addr.s_addr));
> return -1;
>
You could drop the empty line here, and ...
> }
... maybe insert one here.
> fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
> if (fd < 0) {
> -perror("socket(PF_INET, SOCK_DGRAM)");
> +error_setg_errno(errp, errno, "failed to create datagram socket");
I suggested "can't create datagram socket" in my review of v5. I prefer
"can't " over "failed to " myself, but grep
shows more strings starting "failed to" than with "can't". I guess
reporting "failed to " is more common. Okay.
> return -1;
> }
>
> @@ -242,13 +244,15 @@ static int net_socket_mcast_create(struct sockaddr_in
> *mcastaddr, struct in_addr
> val = 1;
> ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
> if (ret < 0) {
> -perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
> +error_setg_errno(errp, errno, "set the socket 'SO_REUSEADDR'"
> + " attribute failed");
Adapting my review of v5 to the next instance: I'm not a native speaker,
but "set FOO failed" doesn't feel right to me. Where's the subject?
"Create" is a verb. "Setting FOO failed" has a subject, but doesn't
feel right. What about "can't set socket option SO_REUSEADDR"?
Also, please avoid breaking the line in the middle of an argument when
you could just as well break it between arguments, like this:
error_setg_errno(errp, errno,
"can't set socket attribute SO_REUSEADDR");
> goto fail;
> }
>
> ret = bind(fd, (struct sockaddr *)mcastaddr, sizeof(*mcastaddr));
> if (ret < 0) {
> -perror("bind");
> +error_setg_errno(errp, errno, "bind ip=%s to socket failed",
> + inet_ntoa(mcastaddr->sin_addr));
Likewise, "can't bind ip=%s to socket".
> goto fail;
> }
>
> @@ -263,7 +267,8 @@ static int net_socket_mcast_create(struct sockaddr_in
> *mcastaddr, struct in_addr
> ret = qemu_setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
>&imr, sizeof(struct ip_mreq));
> if (ret < 0) {
> -perror("setsockopt(IP_ADD_MEMBERSHIP)");
> +error_setg_errno(errp, errno, "add socket to multicast group %s"
> + " failed", inet_ntoa(imr.imr_multiaddr));
Likewise.
> goto fail;
> }
>
> @@ -272,7 +277,8 @@ static int net_socket_mcast_create(struct sockaddr_in
> *mcastaddr, struct in_addr
> ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP,
>&loop, sizeof(loop));
> if (ret < 0) {
> -perror("setsockopt(SOL_IP, IP_MULTICAST_LOOP)");
> +error_setg_errno(errp, errno, "force multicast message to loopback"
> + " failed");
Likwise.
> goto fail;
> }
>
> @@ -281,7 +287,8 @@ static int net_socket_mcast_create(struct sockaddr_in
> *mcastaddr, struct in_addr
> ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,
>localaddr, sizeof(*localaddr));
> if (ret < 0) {
> -perror("setsockopt(IP_MULTICAST_IF)");
> +error_setg_errno(errp, errno, "set the default network send "
> +
Re: [Qemu-devel] [PATCH v3 04/20] stream: Switch stream_run() to byte-based
Am 27.06.2017 um 21:24 hat Eric Blake geschrieben: > We are gradually converting to byte-based interfaces, as they are > easier to reason about than sector-based. Change the internal > loop iteration of streaming to track by bytes instead of sectors > (although we are still guaranteed that we iterate by steps that > are sector-aligned). > > Signed-off-by: Eric Blake > Reviewed-by: John Snow > > --- > v2: no change > --- > block/stream.c | 24 ++-- > 1 file changed, 10 insertions(+), 14 deletions(-) > > diff --git a/block/stream.c b/block/stream.c > index 746d525..2f9618b 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -108,12 +108,11 @@ static void coroutine_fn stream_run(void *opaque) > BlockBackend *blk = s->common.blk; > BlockDriverState *bs = blk_bs(blk); > BlockDriverState *base = s->base; > -int64_t sector_num = 0; > -int64_t end = -1; Here, end was initialised to -1. This made a differnce for early 'goto out' paths because otherwise data->reached_end would incorrectly be true in stream_complete. Because we also check data->ret, I think the only case where it actually makes a difference is for the !bs->backing case: This used to result in data->reached_end == false, but now it ends up as true. This is because s->common.len hasn't been set yet, so it is still 0. Kevin
Re: [Qemu-devel] [PATCH v7 3/4] net/net: Convert parse_host_port() to Error
Mao Zhongyi writes: > Cc: berra...@redhat.com > Cc: kra...@redhat.com > Cc: pbonz...@redhat.com > Cc: jasow...@redhat.com > Cc: arm...@redhat.com > Cc: ebl...@redhat.com > Signed-off-by: Mao Zhongyi Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize
On 07/04/2017 04:46 PM, Halil Pasic wrote:
>
>
> On 07/04/2017 04:31 PM, Cornelia Huck wrote:
>> On Tue, 4 Jul 2017 16:07:55 +0200
>> Christian Borntraeger wrote:
>>
>>> From: Halil Pasic
>>>
>>> From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
>>> implement floating-interrupt controller device", 2013-07-16) the kvm-flic
>>> is not making realize fail properly in case it's impossible to create the
>>> KVM device which basically serves as a backend and is absolutely
>>> essential for having an operational kvm-flic.
>>>
>>> Let's fix this by making sure we do proper error propagation in realize.
>>>
>>> Signed-off-by: Halil Pasic
>>> Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller
>>> device"
>>> Reviewed-by: Dong Jia Shi
>>> Reviewed-by: Yi Min Zhao
>>> Signed-off-by: Christian Borntraeger
>>> ---
>>> hw/intc/s390_flic_kvm.c | 14 +++---
>>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>
>> (...)
>>
>>> cd.type = KVM_DEV_TYPE_FLIC;
>>> ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>>> if (ret < 0) {
>>> -trace_flic_create_device(errno);
>>> -return;
>>> +error_setg_errno(&errp_local, errno, "Creating the KVM device
>>> failed");
>>> +trace_flic_no_device_api(errno);
>>
>> Err... this should still be trace_flic_create_device(), no?
>
> I'm afraid you are right! Probably a copy paste error :/
>
Do you think the traces are still appropriate once we have
proper error propagation?
I did not feel comfortable removing them but thinking again,
than might be the thing to do.
@Christian:
I think we should really fix this the one way or the other.
Can you tell me what is the proper procedure?
>>
>>> +goto fail;
>>> }
>>> flic_state->fd = cd.fd;
>>
>
>
Re: [Qemu-devel] [PATCH] qemu-doc: Add missing "@c man end" statements
On 04/07/2017 16:46, Kevin Wolf wrote:
> Am 19.06.2017 um 11:16 hat Thomas Huth geschrieben:
>> Since commit 3f2ce724f1f1 ("Move the qemu-ga description into a
>> separate chapter"), the qemu.1 man page looks pretty much screwed
>> up, e.g. the title was "qemu-ga - QEMU Guest Agent" instead of
>> "qemu-doc - QEMU Emulator User Documentation". However, that movement
>> of the gemu-ga chapter is not the real problem, it just triggered
>> another bug in the qemu-doc.texi: There are some parts in the file
>> which introduce a "@c man begin OPTIONS" section, but never close
>> it again with "@c man end". After adding the proper end tags here,
>> the title of the man page is right again and the previously wrongly
>> tagged sections now also show up correctly in the man page, too.
>>
>> Reported-by: Kevin Wolf
>> Signed-off-by: Thomas Huth
>
> Michael, do you want to take this through qemu-trivial?
FWIW, I'm sending it out tomorrow. Already tested the pull request, but
I'd like Richard to take another look at the final version of the
TCG-disabling patches and not rush them out.
Paolo
Re: [Qemu-devel] [PATCH v3 02/20] trace: Show blockjob actions via bytes, not sectors
Am 27.06.2017 um 21:24 hat Eric Blake geschrieben: > Upcoming patches are going to switch to byte-based interfaces > instead of sector-based. Even worse, trace_backup_do_cow_enter() > had a weird mix of cluster and sector indices. > > The trace interface is low enough that there are no stability > guarantees, and therefore nothing wrong with changing our units, > even in cases like trace_backup_do_cow_skip() where we are not > changing the trace output. So make the tracing uniformly use > bytes. > > Signed-off-by: Eric Blake > Reviewed-by: John Snow Reviewed-by: Kevin Wolf
Re: [Qemu-devel] [PATCH v3 01/20] blockjob: Track job ratelimits via bytes, not sectors
Am 27.06.2017 um 21:24 hat Eric Blake geschrieben: > The user interface specifies job rate limits in bytes/second. > It's pointless to have our internal representation track things > in sectors/second, particularly since we want to move away from > sector-based interfaces. > > Fix up a doc typo found while verifying that the ratelimit > code handles the scaling difference. > > Repetition of expressions like 'n * BDRV_SECTOR_SIZE' will be > cleaned up later when functions are converted to iterate over > images by bytes rather than by sectors. > > Signed-off-by: Eric Blake > Reviewed-by: John Snow Reviewed-by: Kevin Wolf
Re: [Qemu-devel] [PATCH v3 05/20] commit: Switch commit_populate() to byte-based
Am 27.06.2017 um 21:24 hat Eric Blake geschrieben: > We are gradually converting to byte-based interfaces, as they are > easier to reason about than sector-based. Start by converting an > internal function (no semantic change). > > Signed-off-by: Eric Blake > Reviewed-by: John Snow Reviewed-by: Kevin Wolf
Re: [Qemu-devel] [PATCH v3 06/20] commit: Switch commit_run() to byte-based
Am 27.06.2017 um 21:24 hat Eric Blake geschrieben: > We are gradually converting to byte-based interfaces, as they are > easier to reason about than sector-based. Change the internal > loop iteration of committing to track by bytes instead of sectors > (although we are still guaranteed that we iterate by steps that > are sector-aligned). > > Signed-off-by: Eric Blake > Reviewed-by: John Snow Reviewed-by: Kevin Wolf
Re: [Qemu-devel] [PATCH v3 03/20] stream: Switch stream_populate() to byte-based
Am 27.06.2017 um 21:24 hat Eric Blake geschrieben: > We are gradually converting to byte-based interfaces, as they are > easier to reason about than sector-based. Start by converting an > internal function (no semantic change). > > Signed-off-by: Eric Blake > Reviewed-by: John Snow Reviewed-by: Kevin Wolf
[Qemu-devel] [PATCH v7 4/6] hmp: create a throttle initialization function for code reusability
This patch creates a throttle initialization function to maximize the
code reusability. The same code is also used by fsdev.
Acked-by: Dr. David Alan Gilbert
Signed-off-by: Pradeep Jagadeesh
---
hmp.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/hmp.c b/hmp.c
index 8c72c58..3f76073 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1749,20 +1749,27 @@ void hmp_change(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict)
+{
+iot->bps = qdict_get_int(qdict, "bps");
+iot->bps_rd = qdict_get_int(qdict, "bps_rd");
+iot->bps_wr = qdict_get_int(qdict, "bps_wr");
+iot->iops = qdict_get_int(qdict, "iops");
+iot->iops_rd = qdict_get_int(qdict, "iops_rd");
+iot->iops_wr = qdict_get_int(qdict, "iops_wr");
+}
+
void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
+IOThrottle *iothrottle;
BlockIOThrottle throttle = {
.has_device = true,
.device = (char *) qdict_get_str(qdict, "device"),
-.bps = qdict_get_int(qdict, "bps"),
-.bps_rd = qdict_get_int(qdict, "bps_rd"),
-.bps_wr = qdict_get_int(qdict, "bps_wr"),
-.iops = qdict_get_int(qdict, "iops"),
-.iops_rd = qdict_get_int(qdict, "iops_rd"),
-.iops_wr = qdict_get_int(qdict, "iops_wr"),
};
+iothrottle = qapi_BlockIOThrottle_base(&throttle);
+hmp_initialize_io_throttle(iothrottle, qdict);
qmp_block_set_io_throttle(&throttle, &err);
hmp_handle_error(mon, &err);
}
--
1.8.3.1
[Qemu-devel] [PATCH v7 3/6] throttle: move out function to reuse the code
This patch move out the throttle code to util/throttle.c to maximize
the reusability of the code.The same code is also used by fsdev.
Signed-off-by: Pradeep Jagadeesh
---
blockdev.c | 53 +++-
include/qemu/throttle-options.h | 3 +++
util/throttle.c | 59 +
3 files changed, 65 insertions(+), 50 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a95ea41..8090798 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2579,6 +2579,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg,
Error **errp)
BlockDriverState *bs;
BlockBackend *blk;
AioContext *aio_context;
+IOThrottle *iothrottle;
blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
arg->has_id ? arg->id : NULL,
@@ -2596,56 +2597,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg,
Error **errp)
goto out;
}
-throttle_config_init(&cfg);
-cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
-cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd;
-cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
-
-cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
-cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd;
-cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
-
-if (arg->has_bps_max) {
-cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
-}
-if (arg->has_bps_rd_max) {
-cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
-}
-if (arg->has_bps_wr_max) {
-cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
-}
-if (arg->has_iops_max) {
-cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
-}
-if (arg->has_iops_rd_max) {
-cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
-}
-if (arg->has_iops_wr_max) {
-cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
-}
-
-if (arg->has_bps_max_length) {
-cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
-}
-if (arg->has_bps_rd_max_length) {
-cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
-}
-if (arg->has_bps_wr_max_length) {
-cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
-}
-if (arg->has_iops_max_length) {
-cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
-}
-if (arg->has_iops_rd_max_length) {
-cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
-}
-if (arg->has_iops_wr_max_length) {
-cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
-}
-
-if (arg->has_iops_size) {
-cfg.op_size = arg->iops_size;
-}
+iothrottle = qapi_BlockIOThrottle_base(arg);
+throttle_set_io_limits(&cfg, iothrottle);
if (!throttle_is_valid(&cfg, errp)) {
goto out;
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index f63d38c..a9deb8e 100644
--- a/include/qemu/throttle-options.h
+++ b/include/qemu/throttle-options.h
@@ -11,6 +11,7 @@
#define THROTTLE_OPTIONS_H
#include "typedefs.h"
+#include "qapi-types.h"
#define THROTTLE_OPTS \
{ \
@@ -93,4 +94,6 @@
void throttle_parse_options(ThrottleConfig *, QemuOpts *);
+void throttle_set_io_limits(ThrottleConfig *, IOThrottle *);
+
#endif
diff --git a/util/throttle.c b/util/throttle.c
index a751126..2cf9ec5 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -565,3 +565,62 @@ void throttle_parse_options(ThrottleConfig *throttle_cfg,
QemuOpts *opts)
throttle_cfg->op_size =
qemu_opt_get_number(opts, "throttling.iops-size", 0);
}
+
+/* set the throttle limits
+ *
+ * @arg: iothrottle limits
+ * @cfg: throttle configuration
+ */
+void throttle_set_io_limits(ThrottleConfig *cfg, IOThrottle *arg)
+{
+throttle_config_init(cfg);
+cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
+cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_rd;
+cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
+
+cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
+cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_rd;
+cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
+
+if (arg->has_bps_max) {
+cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
+}
+if (arg->has_bps_rd_max) {
+cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
+}
+if (arg->has_bps_wr_max) {
+cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
+}
+if (arg->has_iops_max) {
+cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
+}
+if (arg->has_iops_rd_max) {
+cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
+}
+if (arg->has_iops_wr_max) {
+cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
+}
+
+if (arg->has_bps_max_length) {
+cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
+
[Qemu-devel] [PATCH v7 1/6] throttle: factor out duplicate code
This patch factor out the duplicate throttle code that was present in
block and fsdev devices.
Signed-off-by: Pradeep Jagadeesh
Reviewed-by: Alberto Garcia
---
blockdev.c | 44 +--
fsdev/qemu-fsdev-throttle.c | 44 ++-
include/qemu/throttle-options.h | 4
include/qemu/throttle.h | 4 ++--
include/qemu/typedefs.h | 1 +
util/throttle.c | 51 +
6 files changed, 61 insertions(+), 87 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index f92dcf2..a95ea41 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts
*opts, int *bdrv_flags,
}
if (throttle_cfg) {
-throttle_config_init(throttle_cfg);
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.bps-total", 0);
-throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
-qemu_opt_get_number(opts, "throttling.bps-read", 0);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.bps-write", 0);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.iops-total", 0);
-throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
-qemu_opt_get_number(opts, "throttling.iops-read", 0);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-throttle_cfg->buckets[THROTTLE_BPS_READ].max =
-qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
-qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_READ].max =
-qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
-qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
-qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
-qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
-qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-
-throttle_cfg->op_size =
-qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+throttle_parse_options(throttle_cfg, opts);
if (!throttle_is_valid(throttle_cfg, errp)) {
return;
}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 7ae4e86..c5e2499 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,6 +16,7 @@
#include "qemu/error-report.h"
#include "qemu-fsdev-throttle.h"
#include "qemu/iov.h"
+#include "qemu/throttle-options.h"
static void fsdev_throttle_read_timer_cb(void *opaque)
{
@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
{
-throttle_config_init(&fst->cfg);
-fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.bps-total", 0);
-fst->cfg.buckets[THROTTLE_BPS_READ].avg =
-qemu_opt_get_number(opts, "throttling.bps-read", 0);
-fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.bps-write", 0);
-fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.iops-total", 0);
-fst->cfg.buckets[THROTTLE_OPS_READ].avg =
-qemu_opt_get_number(opts, "throttling.iops-read", 0);
-fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-fst->cfg.buckets[THROTTLE_BPS_READ].max =
-qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
-qe
[Qemu-devel] [PATCH v7 5/6] fsdev: hmp interface for throttling
This patch introduces hmp interfaces for the fsdev
devices.
Signed-off-by: Pradeep Jagadeesh
---
hmp-commands-info.hx | 18 +++
hmp-commands.hx | 19
hmp.c| 62
hmp.h| 4
4 files changed, 103 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ae16901..5e4ea51 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -84,6 +84,24 @@ STEXI
Show block device statistics.
ETEXI
+#if defined(CONFIG_VIRTFS)
+
+{
+.name = "fsdev_iothrottle",
+.args_type = "",
+.params = "",
+.help = "show fsdev iothrottle information",
+.cmd= hmp_info_fsdev_iothrottle,
+},
+
+#endif
+
+STEXI
+@item info fsdev_iothrottle
+@findex fsdev_iothrottle
+Show fsdev device throttle info.
+ETEXI
+
{
.name = "block-jobs",
.args_type = "",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e763606..b7eb9a6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1662,6 +1662,25 @@ STEXI
Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd}
@var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
ETEXI
+#if defined(CONFIG_VIRTFS)
+
+{
+.name = "fsdev_set_io_throttle",
+.args_type =
"device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
+.params = "device bps bps_rd bps_wr iops iops_rd iops_wr",
+.help = "change I/O throttle limits for a fs devices",
+.cmd= hmp_fsdev_set_io_throttle,
+},
+
+#endif
+
+STEXI
+@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr}
@var{iops} @var{iops_rd} @var{iops_wr}
+@findex fsdev_set_io_throttle
+Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd}
@var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+ETEXI
+
+
{
.name = "set_password",
.args_type = "protocol:s,password:s,connected:s?",
diff --git a/hmp.c b/hmp.c
index 3f76073..97014d4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1774,6 +1774,68 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict
*qdict)
hmp_handle_error(mon, &err);
}
+#ifdef CONFIG_VIRTFS
+
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+IOThrottle throttle = {
+.has_id = true,
+.id = (char *) qdict_get_str(qdict, "device"),
+};
+
+hmp_initialize_io_throttle(&throttle, qdict);
+qmp_fsdev_set_io_throttle(&throttle, &err);
+hmp_handle_error(mon, &err);
+}
+
+static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
+ Error *err)
+{
+monitor_printf(mon, "%s", fscfg->id);
+monitor_printf(mon, "I/O throttling:"
+ " bps=%" PRId64
+ " bps_rd=%" PRId64 " bps_wr=%" PRId64
+ " bps_max=%" PRId64
+ " bps_rd_max=%" PRId64
+ " bps_wr_max=%" PRId64
+ " iops=%" PRId64 " iops_rd=%" PRId64
+ " iops_wr=%" PRId64
+ " iops_max=%" PRId64
+ " iops_rd_max=%" PRId64
+ " iops_wr_max=%" PRId64
+ " iops_size=%" PRId64
+ "\n",
+ fscfg->bps,
+ fscfg->bps_rd,
+ fscfg->bps_wr,
+ fscfg->bps_max,
+ fscfg->bps_rd_max,
+ fscfg->bps_wr_max,
+ fscfg->iops,
+ fscfg->iops_rd,
+ fscfg->iops_wr,
+ fscfg->iops_max,
+ fscfg->iops_rd_max,
+ fscfg->iops_wr_max,
+ fscfg->iops_size);
+ hmp_handle_error(mon, &err);
+}
+
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+IOThrottleList *fsdev_list, *info;
+fsdev_list = qmp_query_fsdev_io_throttle(&err);
+
+for (info = fsdev_list; info; info = info->next) {
+print_fsdev_throttle_config(mon, info->value, err);
+}
+qapi_free_IOThrottleList(fsdev_list);
+}
+
+#endif
+
void hmp_block_stream(Monitor *mon, const QDict *qdict)
{
Error *error = NULL;
diff --git a/hmp.h b/hmp.h
index d8b94ce..dbf024d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -81,6 +81,10 @@ void hmp_set_password(Monitor *mon, const QDict *qdict);
void hmp_expire_password(Monitor *mon, const QDict *qdict);
void hmp_eject(Monitor *mon, const QDict *qdict);
void hmp_change(Monitor *mon, const QDict *qdict);
+#ifdef CONFIG_VIRTFS
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict);
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict);
+#endif
void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
void hmp_block_stream(Monitor *mon, const QDict *qdict);
void hmp_block_job_set_speed(Monitor *mon, const QD
[Qemu-devel] [PATCH v7 2/6] qmp: Create IOThrottle structure
This patch enables qmp interfaces for the fsdev
devices. This provides two interfaces one
for querying info of all the fsdev devices. The second one
to set the IO limits for the required fsdev device.
Signed-off-by: Pradeep Jagadeesh
Reviewed-by: Greg Kurz
Reviewed-by: Eric Blake
Reviewed-by: Alberto Garcia
---
qapi/block-core.json | 76 ++---
qapi/iothrottle.json | 88
2 files changed, 91 insertions(+), 73 deletions(-)
create mode 100644 qapi/iothrottle.json
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f85c223..9320974 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -6,6 +6,7 @@
# QAPI common definitions
{ 'include': 'common.json' }
+{ 'include': 'iothrottle.json' }
##
# @SnapshotInfo:
@@ -1761,84 +1762,13 @@
#
# @device: Block device name (deprecated, use @id instead)
#
-# @id: The name or QOM path of the guest device (since: 2.8)
-#
-# @bps: total throughput limit in bytes per second
-#
-# @bps_rd: read throughput limit in bytes per second
-#
-# @bps_wr: write throughput limit in bytes per second
-#
-# @iops: total I/O operations per second
-#
-# @iops_rd: read I/O operations per second
-#
-# @iops_wr: write I/O operations per second
-#
-# @bps_max: total throughput limit during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_rd_max: read throughput limit during bursts,
-#in bytes (Since 1.7)
-#
-# @bps_wr_max: write throughput limit during bursts,
-#in bytes (Since 1.7)
-#
-# @iops_max: total I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @iops_rd_max: read I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @iops_wr_max: write I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_max_length: maximum length of the @bps_max burst
-#period, in seconds. It must only
-#be set if @bps_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @bps_rd_max_length: maximum length of the @bps_rd_max
-# burst period, in seconds. It must only
-# be set if @bps_rd_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @bps_wr_max_length: maximum length of the @bps_wr_max
-# burst period, in seconds. It must only
-# be set if @bps_wr_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_max_length: maximum length of the @iops burst
-# period, in seconds. It must only
-# be set if @iops_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_rd_max_length: maximum length of the @iops_rd_max
-#burst period, in seconds. It must only
-#be set if @iops_rd_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @iops_wr_max_length: maximum length of the @iops_wr_max
-#burst period, in seconds. It must only
-#be set if @iops_wr_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @iops_size: an I/O size in bytes (Since 1.7)
-#
# @group: throttle group name (Since 2.4)
#
# Since: 1.1
##
{ 'struct': 'BlockIOThrottle',
- 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
-'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
-'*bps_max': 'int', '*bps_rd_max': 'int',
-'*bps_wr_max': 'int', '*iops_max': 'int',
-'*iops_rd_max': 'int', '*iops_wr_max': 'int',
-'*bps_max_length': 'int', '*bps_rd_max_length': 'int',
-'*bps_wr_max_length': 'int', '*iops_max_length': 'int',
-'*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-'*iops_size': 'int', '*group': 'str' } }
+ 'base': 'IOThrottle',
+ 'data': { '*device': 'str', '*group': 'str' } }
##
# @block-stream:
diff --git a/qapi/iothrottle.json b/qapi/iothrottle.json
new file mode 100644
index 000..0f067c3
--- /dev/null
+++ b/qapi/iothrottle.json
@@ -0,0 +1,88 @@
+# -*- Mode: Python -*-
+
+##
+# == QAPI IOThrottle definitions
+##
+
+##
+# @IOThrottle:
+#
+# A set of parameters describing IO throttling
+#
+# @id: The name or QOM path of the guest device (since: 2.8)
+#
+# @bps: total throughput limit in bytes per second
+#
+# @bps_rd: read throughput limit in bytes per second
+#
+# @bps_wr: write throughput limit in bytes per second
+#
+# @iops: total I/O operations per second
+#
+# @iops_rd: read I/O operations per second
+#
+# @iops_wr: write I/O operat
[Qemu-devel] [PATCH v7 6/6] fsdev: QMP interface for throttling
This patch introduces qmp interfaces for the fsdev
devices. This provides two interfaces one
for querying info of all the fsdev devices. The second one
to set the IO limits for the required fsdev device.
Signed-off-by: Pradeep Jagadeesh
---
Makefile| 4 +++
fsdev/qemu-fsdev-dummy.c| 10 ++
fsdev/qemu-fsdev-throttle.c | 76 +++-
fsdev/qemu-fsdev-throttle.h | 13 +++
fsdev/qemu-fsdev.c | 37
monitor.c | 5 +++
qapi-schema.json| 3 ++
qapi/fsdev.json | 84 +
qmp.c | 14
9 files changed, 245 insertions(+), 1 deletion(-)
create mode 100644 qapi/fsdev.json
diff --git a/Makefile b/Makefile
index 16a0430..4fd7625 100644
--- a/Makefile
+++ b/Makefile
@@ -416,6 +416,10 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json
$(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/crypto.json $(SRC_PATH)/qapi/rocker.json \
$(SRC_PATH)/qapi/trace.json
+ifdef CONFIG_VIRTFS
+qapi-modules += $(SRC_PATH)/qapi/fsdev.json
+endif
+
qapi-types.c qapi-types.h :\
$(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 6dc0fbc..f33305d 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -19,3 +19,13 @@ int qemu_fsdev_add(QemuOpts *opts)
{
return 0;
}
+
+void qmp_fsdev_set_io_throttle(IOThrottle *arg, Error **errp)
+{
+ return;
+}
+
+IOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+abort();
+}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index c5e2499..4483533 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,7 +16,6 @@
#include "qemu/error-report.h"
#include "qemu-fsdev-throttle.h"
#include "qemu/iov.h"
-#include "qemu/throttle-options.h"
static void fsdev_throttle_read_timer_cb(void *opaque)
{
@@ -30,6 +29,81 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
qemu_co_enter_next(&fst->throttled_reqs[true]);
}
+void fsdev_set_io_throttle(IOThrottle *arg, FsThrottle *fst, Error **errp)
+{
+ThrottleConfig cfg;
+
+throttle_set_io_limits(&cfg, arg);
+
+if (throttle_is_valid(&cfg, errp)) {
+fst->cfg = cfg;
+fsdev_throttle_init(fst);
+}
+}
+
+void fsdev_get_io_throttle(FsThrottle *fst, IOThrottle **fs9pcfg,
+ char *fsdevice, Error **errp)
+{
+
+ThrottleConfig cfg = fst->cfg;
+IOThrottle *fscfg = g_malloc0(sizeof(*fscfg));
+
+fscfg->has_id = true;
+fscfg->id = g_strdup(fsdevice);
+fscfg->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg;
+fscfg->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg;
+fscfg->bps_wr = cfg.buckets[THROTTLE_BPS_WRITE].avg;
+
+fscfg->iops = cfg.buckets[THROTTLE_OPS_TOTAL].avg;
+fscfg->iops_rd = cfg.buckets[THROTTLE_OPS_READ].avg;
+fscfg->iops_wr = cfg.buckets[THROTTLE_OPS_WRITE].avg;
+
+fscfg->has_bps_max = cfg.buckets[THROTTLE_BPS_TOTAL].max;
+fscfg->bps_max = cfg.buckets[THROTTLE_BPS_TOTAL].max;
+fscfg->has_bps_rd_max = cfg.buckets[THROTTLE_BPS_READ].max;
+fscfg->bps_rd_max = cfg.buckets[THROTTLE_BPS_READ].max;
+fscfg->has_bps_wr_max = cfg.buckets[THROTTLE_BPS_WRITE].max;
+fscfg->bps_wr_max = cfg.buckets[THROTTLE_BPS_WRITE].max;
+
+fscfg->has_iops_max= cfg.buckets[THROTTLE_OPS_TOTAL].max;
+fscfg->iops_max= cfg.buckets[THROTTLE_OPS_TOTAL].max;
+fscfg->has_iops_rd_max = cfg.buckets[THROTTLE_OPS_READ].max;
+fscfg->iops_rd_max = cfg.buckets[THROTTLE_OPS_READ].max;
+fscfg->has_iops_wr_max = cfg.buckets[THROTTLE_OPS_WRITE].max;
+fscfg->iops_wr_max = cfg.buckets[THROTTLE_OPS_WRITE].max;
+
+fscfg->has_bps_max_length = fscfg->has_bps_max;
+fscfg->bps_max_length =
+ cfg.buckets[THROTTLE_BPS_TOTAL].burst_length;
+fscfg->has_bps_rd_max_length = fscfg->has_bps_rd_max;
+fscfg->bps_rd_max_length =
+ cfg.buckets[THROTTLE_BPS_READ].burst_length;
+fscfg->has_bps_wr_max_length = fscfg->has_bps_wr_max;
+fscfg->bps_wr_max_length =
+ cfg.buckets[THROTTLE_BPS_WRITE].burst_length;
+
+fscfg->has_iops_max_length= fscfg->has_iops_max;
+fscfg->iops_max_length=
+ cfg.buckets[THROTTLE_OPS_TOTAL].burst_length;
+fscfg->has_iops_rd_max_length = fscfg->has_iops_rd_max;
+fscfg->iops_rd_max_length =
+ cfg.buckets[THROTTLE_OPS_READ].burst_length;
+fscfg->has_iops_wr_max_length = fscfg->has_iops_wr_max;
+fscfg->iops_wr_max_length =
+ cfg.buckets[THROTTLE_OPS_WRITE].burst_length;
+
+fscfg->bps_max_length = cfg.buckets[THROTTLE_BPS_TOTAL].burst_length;
+fscfg->bps_rd_max_length = cfg.buckets[THROTTLE_BPS_READ].burst_lengt
[Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling
These patches provide the qmp interface, to query the io throttle status of the all fsdev devices that are present in a vm. also, it provides an interface to set the io throttle parameters of a fsdev to a required value. some of the patches also remove the duplicate code that was present in block and fsdev files. Pradeep Jagadeesh (6): throttle: factor out duplicate code qmp: Create IOThrottle structure throttle: move out function to reuse the code hmp: create a throttle initialization function for code reusability fsdev: hmp interface for throttling fsdev: QMP interface for throttling Makefile| 4 ++ blockdev.c | 97 ++--- fsdev/qemu-fsdev-dummy.c| 10 fsdev/qemu-fsdev-throttle.c | 118 ++-- fsdev/qemu-fsdev-throttle.h | 13 + fsdev/qemu-fsdev.c | 37 + hmp-commands-info.hx| 18 ++ hmp-commands.hx | 19 +++ hmp.c | 81 +-- hmp.h | 4 ++ include/qemu/throttle-options.h | 7 +++ include/qemu/throttle.h | 4 +- include/qemu/typedefs.h | 1 + monitor.c | 5 ++ qapi-schema.json| 3 + qapi/block-core.json| 76 +- qapi/fsdev.json | 84 qapi/iothrottle.json| 88 ++ qmp.c | 14 + util/throttle.c | 110 + 20 files changed, 577 insertions(+), 216 deletions(-) create mode 100644 qapi/fsdev.json create mode 100644 qapi/iothrottle.json v0 -> v1: Addressed comments from Eric Blake, Greg Kurz and Daniel P.Berrange Mainly renaming the functions and removing the redundant code. v1 -> v2: Addressed comments from Eric Blake and Greg Kurz. As per the suggestion I split the patches into smaller patches. Removed some more duplicate code. v2 -> v3: Addresssed comments from Alberto Garcia. Changed the comment from block to iothrottle in the iothrottle.json Added the dummy functions in qemu-fsdev-dummy.c to address the compilation issues that were observed. v3 -> v4: Addressed comments from Eric Blake and Greg Kurz Re-ordered the patches Added the dummy functions in qmp.c to address the cross compilation issues v4 -> v5: Addressed comments from Eric Blake and Greg Kurz Split the fsdev qmp patch into hmp and qmp related patches Moved the common functionalities to throttle.c instead of creating a new file v5 -> v6: Addressed comments from Greg Kurz and Markus Armbruster Split the commits to specific to hmp and throttle as suggested by Greg Moved ThrottleConfig typedef to qemu/typedefs.h Addressed compilation issue on FreeBSD by adding flags in qmp.c v6 -> v7: Addressed comments from Albert Garcia and Dr. David Alan Gilbert Fixed the hmp-commands-info.hx and hmp-commands.hx as per Dr. David's comments. Fixed the bug with the hmp fsdev_set_io_throttle and info fsdev_iothrottle -- 1.8.3.1
Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it
On Wed, Jun 28, 2017 at 03:33:49PM -0500, Eric Blake wrote: > On 06/28/2017 03:15 PM, Alberto Garcia wrote: > > On Wed 28 Jun 2017 04:58:00 PM CEST, Kashyap Chamarthy wrote: > >> This patch documents (including their QMP invocations) all the four > >> major kinds of live block operations: > >> > >> - `block-stream` > >> - `block-commit` > >> - `drive-mirror` (& `blockdev-mirror`) > >> - `drive-backup` (& `blockdev-backup`) > > > > This is excellent work, thanks for doing this! Thanks! > > I haven't had the time to review the complete document yet, but here are > > my comments from the first half: [I'm responding out of order -- as I first replied to your other email, which gave feedback about the sceond half.] > >> +Disk image backing chain notation > >> +- > > [...] > >> +.. important:: > >> +The base disk image can be raw format; however, all the overlay > >> +files must be of QCOW2 format. > > > > This is not quite like that: overlay files must be in a format that > > supports backing files. QCOW2 is the most common one, but there are > > others (qed). Grep for 'supports_backing' in the source code. > > At the same time, other image formats are not as frequently tested, or > may be read-only. Maybe a compromise of "The overlay files can > generally be any format that supports a backing file, although qcow2 is > the preferred format and the one used in this document". Yes, I'll use Eric's wording [thanks] here, that makes the intent clearer. > >> +(1) ``block-stream``: Live copy of data from backing files into overlay > >> +files (with the optional goal of removing the backing file from the > >> +chain). > > > > optional? The backing file is removed from the chain as soon as the > > operation finishes, although the image file is not removed from the > > disk. Maybe you meant that? > > Hmm, you're right. In this case, qemu ALWAYS rewrites the backing chain > to get rid of the (now-streamed) backing image. Correct. I will clarify the wording here. > >> +(2) ``block-commit``: Live merge of data from overlay files into backing > >> +files (with the optional goal of removing the overlay file from the > >> +chain). Since QEMU 2.0, this includes "active ``block-commit``" > >> +(i.e. merge the current active layer into the base image). > > > > Same question about the 'optional' here. > > Here, optional is a bit more correct. With non-active (intermediate) > commit, qemu ALWAYS rewrites the backing chain to be shorter; but with > live commit, you can chose whether to pivot to the now-shorter chain > (job-complete) or whether to keep the active image intact (starting to > collect a new delta from the point-in-time of the just-completed commit, > by job-cancel). Right. I'll workout a way to mention this, too. [Me will not wonder whether it will confuse the reader about mentioning it so early -- as the said reader will be using these primitives to make higher level tools, and they can easily figure out the semantics.] > >> +writing to it. (The rule of thumb is: live QEMU will always be pointing > >> +to the right-most image in a disk image chain.) > > > > I think it's 'rightmost', without the hyphen. > > Sadly, I think this is one case where both spellings work to a native > reader, and where I don't know of a specific style-guide preference. I > probably would have written with the hyphen. Heh, indeed. Peter Maydell has said Oxford English Dictionary has mentions for both variants. But Alberto is correct that the non-hyphen variant, "rightmost", is much more widely used. > >> +(3) Intermediate streaming (available since QEMU 2.8): Starting afresh > >> +with the original example disk image chain, with a total of four > >> +images, it is possible to copy contents from image [B] into image > >> +[C]. Once the copy is finished, image [B] can now be (optionally) > >> +discarded; and the backing file pointer of image [C] will be > >> +adjusted to point to [A]. > > > > The 'optional' usage again. [B] will be removed from the chain and can > > be (optionally) removed from the disk, but that you have to do yourself, > > QEMU won't do that. > > Indeed, we may need to be specifically clear of the cases where qemu > shortens the chain, but where disk images that are no longer used by the > chain (whether they are still viable [as in stream], or invalidated [as > in commit crossing more than one element of the chain]) are still left > on the disk for the user to discard separately from qemu. Yes, I'll keep this in mind for v5. > >> +The ``block-commit`` command lets you to live merge data from overlay > >> +images into backing file(s). > > > > I don't think "lets you to live merge data" is correct English. > > Probably better as "lets you merge live data from overlay..." Yes. Will fix. [...] > >> +(1) Commit content from only image [B] into image [A]. The resulting > >> +chain is the following
[Qemu-devel] [PATCH v2 4/4] xen: don't use xenstore to save/restore physmap anymore
If we have a system with xenforeignmemory_map2() implemented
we don't need to save/restore physmap on suspend/restore
anymore. In case we resume a VM without physmap - try to
recreate the physmap during memory region restore phase and
remap map cache entries accordingly. The old code is left
for compatibility reasons.
Signed-off-by: Igor Druzhinin
---
hw/i386/xen/xen-hvm.c | 48 ++---
include/hw/xen/xen_common.h | 1 +
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index d259cf7..d24ca47 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -289,6 +289,7 @@ static XenPhysmap *get_physmapping(XenIOState *state,
return NULL;
}
+#ifdef XEN_COMPAT_PHYSMAP
static hwaddr xen_phys_offset_to_gaddr(hwaddr start_addr,
ram_addr_t size, void
*opaque)
{
@@ -334,6 +335,12 @@ static int xen_save_physmap(XenIOState *state, XenPhysmap
*physmap)
}
return 0;
}
+#else
+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
+{
+return 0;
+}
+#endif
static int xen_add_to_physmap(XenIOState *state,
hwaddr start_addr,
@@ -368,6 +375,26 @@ go_physmap:
DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
start_addr, start_addr + size);
+mr_name = memory_region_name(mr);
+
+physmap = g_malloc(sizeof (XenPhysmap));
+
+physmap->start_addr = start_addr;
+physmap->size = size;
+physmap->name = mr_name;
+physmap->phys_offset = phys_offset;
+
+QLIST_INSERT_HEAD(&state->physmap, physmap, list);
+
+if (runstate_check(RUN_STATE_INMIGRATE)) {
+/* Now when we have a physmap entry we can replace a dummy mapping with
+ * a real one of guest foreign memory. */
+uint8_t *p = xen_replace_cache_entry(phys_offset, start_addr, size);
+assert(p && p == memory_region_get_ram_ptr(mr));
+
+return 0;
+}
+
pfn = phys_offset >> TARGET_PAGE_BITS;
start_gpfn = start_addr >> TARGET_PAGE_BITS;
for (i = 0; i < size >> TARGET_PAGE_BITS; i++) {
@@ -382,17 +409,6 @@ go_physmap:
}
}
-mr_name = memory_region_name(mr);
-
-physmap = g_malloc(sizeof (XenPhysmap));
-
-physmap->start_addr = start_addr;
-physmap->size = size;
-physmap->name = mr_name;
-physmap->phys_offset = phys_offset;
-
-QLIST_INSERT_HEAD(&state->physmap, physmap, list);
-
xc_domain_pin_memory_cacheattr(xen_xc, xen_domid,
start_addr >> TARGET_PAGE_BITS,
(start_addr + size - 1) >> TARGET_PAGE_BITS,
@@ -1158,6 +1174,7 @@ static void xen_exit_notifier(Notifier *n, void *data)
xs_daemon_close(state->xenstore);
}
+#ifdef XEN_COMPAT_PHYSMAP
static void xen_read_physmap(XenIOState *state)
{
XenPhysmap *physmap = NULL;
@@ -1205,6 +1222,11 @@ static void xen_read_physmap(XenIOState *state)
}
free(entries);
}
+#else
+static void xen_read_physmap(XenIOState *state)
+{
+}
+#endif
static void xen_wakeup_notifier(Notifier *notifier, void *data)
{
@@ -1331,7 +1353,11 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
**ram_memory)
state->bufioreq_local_port = rc;
/* Init RAM management */
+#ifdef XEN_COMPAT_PHYSMAP
xen_map_cache_init(xen_phys_offset_to_gaddr, state);
+#else
+xen_map_cache_init(NULL, state);
+#endif
xen_ram_init(pcms, ram_size, ram_memory);
qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state);
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 70a5cad..c04c5c9 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -80,6 +80,7 @@ extern xenforeignmemory_handle *xen_fmem;
#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41000
+#define XEN_COMPAT_PHYSMAP
#define xenforeignmemory_map2(h, d, a, p, f, ps, ar, e) \
xenforeignmemory_map(h, d, p, ps, ar, e)
--
2.7.4
[Qemu-devel] [PATCH v2 0/4] xen: don't save/restore the physmap on VM save/restore
Saving/restoring the physmap to/from xenstore was introduced to QEMU majorly in order to cover up the VRAM region restore issue. The sequence of restore operations implies that we should know the effective guest VRAM address *before* we have the VRAM region restored (which happens later). Unfortunately, in Xen environment VRAM memory does actually belong to a guest - not QEMU itself - which means the position of this region is unknown beforehand and can't be mapped into QEMU address space immediately. Previously, recreating xenstore keys, holding the physmap, by the toolstack helped to get this information in place at the right moment ready to be consumed by QEMU to map the region properly. But using xenstore for it has certain disadvantages: toolstack needs to be aware of these keys and save/restore them accordingly; accessing xenstore requires extra privileges which hinders QEMU sandboxing. The previous attempt to get rid of that was to remember all the VRAM pointers during QEMU initialization phase and then update them all at once when an actual foreign mapping is established. Unfortunately, this approach worked only for VRAM and only for a predefined set of devices - stdvga and cirrus. QXL and other possible future devices using a moving emulated MMIO region would be equally broken. The new approach leverages xenforeignmemory_map2() call recently introduced in libxenforeignmemory. It allows to create a dummy anonymous mapping for QEMU during its initialization and change it to a real one later during machine state restore. --- Changed in v2: * Patch 2: set dummy flag in a new flags field in struct MapCacheEntry * Patch 3: change xen_remap_cache_entry name and signature * Patch 3: gate ram_block_notify_* functions in xen_remap_bucket * Patch 3: rewrite the logic of xen_replace_cache_entry_unlocked to reuse the existing entry instead of allocating a new one * Patch 4: don't use xen_phys_offset_to_gaddr in non-compat mode --- Igor Druzhinin (4): xen: move physmap saving into a separate function xen/mapcache: add an ability to create dummy mappings xen/mapcache: introduce xen_replace_cache_entry() xen: don't use xenstore to save/restore physmap anymore configure | 18 +++ hw/i386/xen/xen-hvm.c | 105 ++--- hw/i386/xen/xen-mapcache.c| 107 ++ include/hw/xen/xen_common.h | 8 include/sysemu/xen-mapcache.h | 11 - 5 files changed, 201 insertions(+), 48 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH v2 1/4] xen: move physmap saving into a separate function
Non-functional change.
Signed-off-by: Igor Druzhinin
---
hw/i386/xen/xen-hvm.c | 57 ---
1 file changed, 31 insertions(+), 26 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index cffa7e2..d259cf7 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -305,6 +305,36 @@ static hwaddr xen_phys_offset_to_gaddr(hwaddr start_addr,
return start_addr;
}
+static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
+{
+char path[80], value[17];
+
+snprintf(path, sizeof(path),
+"/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
+xen_domid, (uint64_t)physmap->phys_offset);
+snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->start_addr);
+if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+return -1;
+}
+snprintf(path, sizeof(path),
+"/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
+xen_domid, (uint64_t)physmap->phys_offset);
+snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
+if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+return -1;
+}
+if (physmap->name) {
+snprintf(path, sizeof(path),
+"/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
+xen_domid, (uint64_t)physmap->phys_offset);
+if (!xs_write(state->xenstore, 0, path,
+ physmap->name, strlen(physmap->name))) {
+return -1;
+}
+}
+return 0;
+}
+
static int xen_add_to_physmap(XenIOState *state,
hwaddr start_addr,
ram_addr_t size,
@@ -316,7 +346,6 @@ static int xen_add_to_physmap(XenIOState *state,
XenPhysmap *physmap = NULL;
hwaddr pfn, start_gpfn;
hwaddr phys_offset = memory_region_get_ram_addr(mr);
-char path[80], value[17];
const char *mr_name;
if (get_physmapping(state, start_addr, size)) {
@@ -368,31 +397,7 @@ go_physmap:
start_addr >> TARGET_PAGE_BITS,
(start_addr + size - 1) >> TARGET_PAGE_BITS,
XEN_DOMCTL_MEM_CACHEATTR_WB);
-
-snprintf(path, sizeof(path),
-"/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
-xen_domid, (uint64_t)phys_offset);
-snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)start_addr);
-if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
-return -1;
-}
-snprintf(path, sizeof(path),
-"/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
-xen_domid, (uint64_t)phys_offset);
-snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)size);
-if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
-return -1;
-}
-if (mr_name) {
-snprintf(path, sizeof(path),
-"/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
-xen_domid, (uint64_t)phys_offset);
-if (!xs_write(state->xenstore, 0, path, mr_name, strlen(mr_name))) {
-return -1;
-}
-}
-
-return 0;
+return xen_save_physmap(state, physmap);
}
static int xen_remove_from_physmap(XenIOState *state,
--
2.7.4
[Qemu-devel] [PATCH v2 2/4] xen/mapcache: add an ability to create dummy mappings
Dummys are simple anonymous mappings that are placed instead
of regular foreign mappings in certain situations when we need
to postpone the actual mapping but still have to give a
memory region to QEMU to play with.
This is planned to be used for restore on Xen.
Signed-off-by: Igor Druzhinin
---
hw/i386/xen/xen-mapcache.c | 40
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index e60156c..cd4e746 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -53,6 +53,8 @@ typedef struct MapCacheEntry {
uint8_t *vaddr_base;
unsigned long *valid_mapping;
uint8_t lock;
+#define XEN_MAPCACHE_ENTRY_DUMMY (1 << 0)
+uint8_t flags;
hwaddr size;
struct MapCacheEntry *next;
} MapCacheEntry;
@@ -150,7 +152,8 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void
*opaque)
static void xen_remap_bucket(MapCacheEntry *entry,
hwaddr size,
- hwaddr address_index)
+ hwaddr address_index,
+ bool dummy)
{
uint8_t *vaddr_base;
xen_pfn_t *pfns;
@@ -177,11 +180,27 @@ static void xen_remap_bucket(MapCacheEntry *entry,
pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT)) + i;
}
-vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
PROT_READ|PROT_WRITE,
- nb_pfn, pfns, err);
-if (vaddr_base == NULL) {
-perror("xenforeignmemory_map");
-exit(-1);
+if (!dummy) {
+vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
+ PROT_READ|PROT_WRITE,
+ nb_pfn, pfns, err);
+if (vaddr_base == NULL) {
+perror("xenforeignmemory_map");
+exit(-1);
+}
+entry->flags &= ~(XEN_MAPCACHE_ENTRY_DUMMY);
+} else {
+/*
+ * We create dummy mappings where we are unable to create a foreign
+ * mapping immediately due to certain circumstances (i.e. on resume
now)
+ */
+vaddr_base = mmap(NULL, size, PROT_READ|PROT_WRITE,
+ MAP_ANON|MAP_SHARED, -1, 0);
+if (vaddr_base == NULL) {
+perror("mmap");
+exit(-1);
+}
+entry->flags |= XEN_MAPCACHE_ENTRY_DUMMY;
}
entry->vaddr_base = vaddr_base;
@@ -211,6 +230,7 @@ static uint8_t *xen_map_cache_unlocked(hwaddr phys_addr,
hwaddr size,
hwaddr cache_size = size;
hwaddr test_bit_size;
bool translated = false;
+bool dummy = false;
tryagain:
address_index = phys_addr >> MCACHE_BUCKET_SHIFT;
@@ -262,14 +282,14 @@ tryagain:
if (!entry) {
entry = g_malloc0(sizeof (MapCacheEntry));
pentry->next = entry;
-xen_remap_bucket(entry, cache_size, address_index);
+xen_remap_bucket(entry, cache_size, address_index, dummy);
} else if (!entry->lock) {
if (!entry->vaddr_base || entry->paddr_index != address_index ||
entry->size != cache_size ||
!test_bits(address_offset >> XC_PAGE_SHIFT,
test_bit_size >> XC_PAGE_SHIFT,
entry->valid_mapping)) {
-xen_remap_bucket(entry, cache_size, address_index);
+xen_remap_bucket(entry, cache_size, address_index, dummy);
}
}
@@ -282,6 +302,10 @@ tryagain:
translated = true;
goto tryagain;
}
+if (!dummy && runstate_check(RUN_STATE_INMIGRATE)) {
+dummy = true;
+goto tryagain;
+}
trace_xen_map_cache_return(NULL);
return NULL;
}
--
2.7.4
[Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry()
This new call is trying to update a requested map cache entry
according to the changes in the physmap. The call is searching
for the entry, unmaps it and maps again at the same place using
a new guest address. If the mapping is dummy this call will
make it real.
This function makes use of a new xenforeignmemory_map2() call
with an extended interface that was recently introduced in
libxenforeignmemory [1].
[1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg113007.html
Signed-off-by: Igor Druzhinin
---
configure | 18 ++
hw/i386/xen/xen-mapcache.c| 79 ++-
include/hw/xen/xen_common.h | 7
include/sysemu/xen-mapcache.h | 11 +-
4 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index c571ad1..ad6156b 100755
--- a/configure
+++ b/configure
@@ -2021,6 +2021,24 @@ EOF
# Xen unstable
elif
cat > $TMPC <
+int main(void) {
+ xenforeignmemory_handle *xfmem;
+
+ xfmem = xenforeignmemory_open(0, 0);
+ xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
+
+ return 0;
+}
+EOF
+compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
+ then
+ xen_stable_libs="-lxendevicemodel $xen_stable_libs"
+ xen_ctrl_version=41000
+ xen=yes
+elif
+cat > $TMPC <
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index cd4e746..a988be7 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -151,6 +151,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void
*opaque)
}
static void xen_remap_bucket(MapCacheEntry *entry,
+ void *vaddr,
hwaddr size,
hwaddr address_index,
bool dummy)
@@ -167,7 +168,9 @@ static void xen_remap_bucket(MapCacheEntry *entry,
err = g_malloc0(nb_pfn * sizeof (int));
if (entry->vaddr_base != NULL) {
-ram_block_notify_remove(entry->vaddr_base, entry->size);
+if (entry->vaddr_base != vaddr) {
+ram_block_notify_remove(entry->vaddr_base, entry->size);
+}
if (munmap(entry->vaddr_base, entry->size) != 0) {
perror("unmap fails");
exit(-1);
@@ -181,11 +184,11 @@ static void xen_remap_bucket(MapCacheEntry *entry,
}
if (!dummy) {
-vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
- PROT_READ|PROT_WRITE,
+vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid, vaddr,
+ PROT_READ|PROT_WRITE, 0,
nb_pfn, pfns, err);
if (vaddr_base == NULL) {
-perror("xenforeignmemory_map");
+perror("xenforeignmemory_map2");
exit(-1);
}
entry->flags &= ~(XEN_MAPCACHE_ENTRY_DUMMY);
@@ -194,7 +197,7 @@ static void xen_remap_bucket(MapCacheEntry *entry,
* We create dummy mappings where we are unable to create a foreign
* mapping immediately due to certain circumstances (i.e. on resume
now)
*/
-vaddr_base = mmap(NULL, size, PROT_READ|PROT_WRITE,
+vaddr_base = mmap(vaddr, size, PROT_READ|PROT_WRITE,
MAP_ANON|MAP_SHARED, -1, 0);
if (vaddr_base == NULL) {
perror("mmap");
@@ -203,13 +206,16 @@ static void xen_remap_bucket(MapCacheEntry *entry,
entry->flags |= XEN_MAPCACHE_ENTRY_DUMMY;
}
+if (entry->vaddr_base == NULL || entry->vaddr_base != vaddr) {
+ram_block_notify_add(vaddr_base, size);
+}
+
entry->vaddr_base = vaddr_base;
entry->paddr_index = address_index;
entry->size = size;
entry->valid_mapping = (unsigned long *) g_malloc0(sizeof(unsigned long) *
BITS_TO_LONGS(size >> XC_PAGE_SHIFT));
-ram_block_notify_add(entry->vaddr_base, entry->size);
bitmap_zero(entry->valid_mapping, nb_pfn);
for (i = 0; i < nb_pfn; i++) {
if (!err[i]) {
@@ -282,14 +288,14 @@ tryagain:
if (!entry) {
entry = g_malloc0(sizeof (MapCacheEntry));
pentry->next = entry;
-xen_remap_bucket(entry, cache_size, address_index, dummy);
+xen_remap_bucket(entry, NULL, cache_size, address_index, dummy);
} else if (!entry->lock) {
if (!entry->vaddr_base || entry->paddr_index != address_index ||
entry->size != cache_size ||
!test_bits(address_offset >> XC_PAGE_SHIFT,
test_bit_size >> XC_PAGE_SHIFT,
entry->valid_mapping)) {
-xen_remap_bucket(entry, cache_size, address_index, dummy);
+xen_remap_bucket(entry, NULL, cache_size, address_index, dummy);
}
}
@@ -486,3 +492,60 @@ void xen_invalidate_map_cache(void)
mapcache_unlock();
}
+
+static uint8_t *xen_replace_cache_entry_unlocked
Re: [Qemu-devel] [PATCH v2 1/4] xen: move physmap saving into a separate function
> -Original Message-
> From: Igor Druzhinin
> Sent: 04 July 2017 16:48
> To: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org
> Cc: Igor Druzhinin ; sstabell...@kernel.org;
> Anthony Perard ; Paul Durrant
> ; pbonz...@redhat.com
> Subject: [PATCH v2 1/4] xen: move physmap saving into a separate function
>
> Non-functional change.
>
> Signed-off-by: Igor Druzhinin
Reviewed-by: Paul Durrant
> ---
> hw/i386/xen/xen-hvm.c | 57
> ---
> 1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index cffa7e2..d259cf7 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -305,6 +305,36 @@ static hwaddr xen_phys_offset_to_gaddr(hwaddr
> start_addr,
> return start_addr;
> }
>
> +static int xen_save_physmap(XenIOState *state, XenPhysmap *physmap)
> +{
> +char path[80], value[17];
> +
> +snprintf(path, sizeof(path),
> +"/local/domain/0/device-
> model/%d/physmap/%"PRIx64"/start_addr",
> +xen_domid, (uint64_t)physmap->phys_offset);
> +snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap-
> >start_addr);
> +if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
> +return -1;
> +}
> +snprintf(path, sizeof(path),
> +"/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
> +xen_domid, (uint64_t)physmap->phys_offset);
> +snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)physmap->size);
> +if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
> +return -1;
> +}
> +if (physmap->name) {
> +snprintf(path, sizeof(path),
> +"/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
> +xen_domid, (uint64_t)physmap->phys_offset);
> +if (!xs_write(state->xenstore, 0, path,
> + physmap->name, strlen(physmap->name))) {
> +return -1;
> +}
> +}
> +return 0;
> +}
> +
> static int xen_add_to_physmap(XenIOState *state,
>hwaddr start_addr,
>ram_addr_t size,
> @@ -316,7 +346,6 @@ static int xen_add_to_physmap(XenIOState *state,
> XenPhysmap *physmap = NULL;
> hwaddr pfn, start_gpfn;
> hwaddr phys_offset = memory_region_get_ram_addr(mr);
> -char path[80], value[17];
> const char *mr_name;
>
> if (get_physmapping(state, start_addr, size)) {
> @@ -368,31 +397,7 @@ go_physmap:
> start_addr >> TARGET_PAGE_BITS,
> (start_addr + size - 1) >>
> TARGET_PAGE_BITS,
> XEN_DOMCTL_MEM_CACHEATTR_WB);
> -
> -snprintf(path, sizeof(path),
> -"/local/domain/0/device-
> model/%d/physmap/%"PRIx64"/start_addr",
> -xen_domid, (uint64_t)phys_offset);
> -snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)start_addr);
> -if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
> -return -1;
> -}
> -snprintf(path, sizeof(path),
> -"/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
> -xen_domid, (uint64_t)phys_offset);
> -snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)size);
> -if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
> -return -1;
> -}
> -if (mr_name) {
> -snprintf(path, sizeof(path),
> -"/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
> -xen_domid, (uint64_t)phys_offset);
> -if (!xs_write(state->xenstore, 0, path, mr_name, strlen(mr_name))) {
> -return -1;
> -}
> -}
> -
> -return 0;
> +return xen_save_physmap(state, physmap);
> }
>
> static int xen_remove_from_physmap(XenIOState *state,
> --
> 2.7.4
Re: [Qemu-devel] [PATCH v2 2/4] xen/mapcache: add an ability to create dummy mappings
> -Original Message-
> From: Igor Druzhinin
> Sent: 04 July 2017 16:48
> To: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org
> Cc: Igor Druzhinin ; sstabell...@kernel.org;
> Anthony Perard ; Paul Durrant
> ; pbonz...@redhat.com
> Subject: [PATCH v2 2/4] xen/mapcache: add an ability to create dummy
> mappings
>
> Dummys are simple anonymous mappings that are placed instead
> of regular foreign mappings in certain situations when we need
> to postpone the actual mapping but still have to give a
> memory region to QEMU to play with.
>
> This is planned to be used for restore on Xen.
>
> Signed-off-by: Igor Druzhinin
Reviewed-by: Paul Durrant
> ---
> hw/i386/xen/xen-mapcache.c | 40
>
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
> index e60156c..cd4e746 100644
> --- a/hw/i386/xen/xen-mapcache.c
> +++ b/hw/i386/xen/xen-mapcache.c
> @@ -53,6 +53,8 @@ typedef struct MapCacheEntry {
> uint8_t *vaddr_base;
> unsigned long *valid_mapping;
> uint8_t lock;
> +#define XEN_MAPCACHE_ENTRY_DUMMY (1 << 0)
> +uint8_t flags;
> hwaddr size;
> struct MapCacheEntry *next;
> } MapCacheEntry;
> @@ -150,7 +152,8 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
> void *opaque)
>
> static void xen_remap_bucket(MapCacheEntry *entry,
> hwaddr size,
> - hwaddr address_index)
> + hwaddr address_index,
> + bool dummy)
> {
> uint8_t *vaddr_base;
> xen_pfn_t *pfns;
> @@ -177,11 +180,27 @@ static void xen_remap_bucket(MapCacheEntry
> *entry,
> pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT))
> + i;
> }
>
> -vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
> PROT_READ|PROT_WRITE,
> - nb_pfn, pfns, err);
> -if (vaddr_base == NULL) {
> -perror("xenforeignmemory_map");
> -exit(-1);
> +if (!dummy) {
> +vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
> + PROT_READ|PROT_WRITE,
> + nb_pfn, pfns, err);
> +if (vaddr_base == NULL) {
> +perror("xenforeignmemory_map");
> +exit(-1);
> +}
> +entry->flags &= ~(XEN_MAPCACHE_ENTRY_DUMMY);
> +} else {
> +/*
> + * We create dummy mappings where we are unable to create a foreign
> + * mapping immediately due to certain circumstances (i.e. on resume
> now)
> + */
> +vaddr_base = mmap(NULL, size, PROT_READ|PROT_WRITE,
> + MAP_ANON|MAP_SHARED, -1, 0);
> +if (vaddr_base == NULL) {
> +perror("mmap");
> +exit(-1);
> +}
> +entry->flags |= XEN_MAPCACHE_ENTRY_DUMMY;
> }
>
> entry->vaddr_base = vaddr_base;
> @@ -211,6 +230,7 @@ static uint8_t *xen_map_cache_unlocked(hwaddr
> phys_addr, hwaddr size,
> hwaddr cache_size = size;
> hwaddr test_bit_size;
> bool translated = false;
> +bool dummy = false;
>
> tryagain:
> address_index = phys_addr >> MCACHE_BUCKET_SHIFT;
> @@ -262,14 +282,14 @@ tryagain:
> if (!entry) {
> entry = g_malloc0(sizeof (MapCacheEntry));
> pentry->next = entry;
> -xen_remap_bucket(entry, cache_size, address_index);
> +xen_remap_bucket(entry, cache_size, address_index, dummy);
> } else if (!entry->lock) {
> if (!entry->vaddr_base || entry->paddr_index != address_index ||
> entry->size != cache_size ||
> !test_bits(address_offset >> XC_PAGE_SHIFT,
> test_bit_size >> XC_PAGE_SHIFT,
> entry->valid_mapping)) {
> -xen_remap_bucket(entry, cache_size, address_index);
> +xen_remap_bucket(entry, cache_size, address_index, dummy);
> }
> }
>
> @@ -282,6 +302,10 @@ tryagain:
> translated = true;
> goto tryagain;
> }
> +if (!dummy && runstate_check(RUN_STATE_INMIGRATE)) {
> +dummy = true;
> +goto tryagain;
> +}
> trace_xen_map_cache_return(NULL);
> return NULL;
> }
> --
> 2.7.4
Re: [Qemu-devel] [PATCH v7 4/4] net/socket: Improve -net socket error reporting
Mao Zhongyi writes:
> When -net socket fails, it first reports a specific error, then
> a generic one, like this:
>
> $ qemu-system-x86_64 -net socket,
> qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=,
> mcast= or udp= is required
> qemu-system-x86_64: -net socket: Device 'socket' could not be initialized
>
> Convert net_socket_*_init() to Error to get rid of the superfluous second
> error message. After the patch, the effect like this:
>
> $ qemu-system-x86_64 -net socket,
> qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=,
> mcast= or udp= is required
>
> At the same time, add many explicit error handling message when it fails.
>
> Cc: jasow...@redhat.com
> Cc: arm...@redhat.com
> Cc: berra...@redhat.com
> Signed-off-by: Mao Zhongyi
> ---
> net/socket.c | 94
> +---
> 1 file changed, 45 insertions(+), 49 deletions(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index bd80b3c..a891c3a 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -494,22 +494,21 @@ static void net_socket_accept(void *opaque)
> static int net_socket_listen_init(NetClientState *peer,
>const char *model,
>const char *name,
> - const char *host_str)
> + const char *host_str,
> + Error **errp)
> {
> NetClientState *nc;
> NetSocketState *s;
> struct sockaddr_in saddr;
> int fd, ret;
> -Error *err = NULL;
>
> -if (parse_host_port(&saddr, host_str, &err) < 0) {
> -error_report_err(err);
> +if (parse_host_port(&saddr, host_str, errp) < 0) {
> return -1;
> }
>
> fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (fd < 0) {
> -perror("socket");
> +error_setg_errno(errp, errno, "failed to create stream socket");
> return -1;
> }
> qemu_set_nonblock(fd);
> @@ -518,13 +517,14 @@ static int net_socket_listen_init(NetClientState *peer,
>
> ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
> if (ret < 0) {
> -perror("bind");
> +error_setg_errno(errp, errno, "bind ip=%s to socket failed",
> + inet_ntoa(saddr.sin_addr));
Comment on the same error message in PATCH 2 applies.
> closesocket(fd);
> return -1;
> }
> ret = listen(fd, 0);
> if (ret < 0) {
> -perror("listen");
> +error_setg_errno(errp, errno, "listen socket failed");
Suggest "can't listen on socket".
> closesocket(fd);
> return -1;
> }
> @@ -543,21 +543,20 @@ static int net_socket_listen_init(NetClientState *peer,
> static int net_socket_connect_init(NetClientState *peer,
> const char *model,
> const char *name,
> - const char *host_str)
> + const char *host_str,
> + Error **errp)
> {
> NetSocketState *s;
> int fd, connected, ret;
> struct sockaddr_in saddr;
> -Error *err = NULL;
>
> -if (parse_host_port(&saddr, host_str, &err) < 0) {
> -error_report_err(err);
> +if (parse_host_port(&saddr, host_str, errp) < 0) {
> return -1;
> }
>
> fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (fd < 0) {
> -perror("socket");
> +error_setg_errno(errp, errno, "failed to create stream socket");
> return -1;
> }
> qemu_set_nonblock(fd);
> @@ -573,7 +572,7 @@ static int net_socket_connect_init(NetClientState *peer,
> errno == EINVAL) {
> break;
> } else {
> -perror("connect");
> +error_setg_errno(errp, errno, "connection failed");
Suggest "can't connect socket".
> closesocket(fd);
> return -1;
> }
> @@ -582,9 +581,8 @@ static int net_socket_connect_init(NetClientState *peer,
> break;
> }
> }
> -s = net_socket_fd_init(peer, model, name, fd, connected, &err);
> +s = net_socket_fd_init(peer, model, name, fd, connected, errp);
> if (!s) {
> -error_report_err(err);
> return -1;
> }
> snprintf(s->nc.info_str, sizeof(s->nc.info_str),
> @@ -597,36 +595,36 @@ static int net_socket_mcast_init(NetClientState *peer,
> const char *model,
> const char *name,
> const char *host_str,
> - const char *localaddr_str)
> + const char *localaddr_str,
> + Error **errp)
> {
> NetSocketState *s;
> int fd;
> struct sockaddr_in
Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry()
> -Original Message-
> From: Igor Druzhinin
> Sent: 04 July 2017 16:48
> To: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org
> Cc: Igor Druzhinin ; sstabell...@kernel.org;
> Anthony Perard ; Paul Durrant
> ; pbonz...@redhat.com
> Subject: [PATCH v2 3/4] xen/mapcache: introduce
> xen_replace_cache_entry()
>
> This new call is trying to update a requested map cache entry
> according to the changes in the physmap. The call is searching
> for the entry, unmaps it and maps again at the same place using
> a new guest address. If the mapping is dummy this call will
> make it real.
>
> This function makes use of a new xenforeignmemory_map2() call
> with an extended interface that was recently introduced in
> libxenforeignmemory [1].
I don't understand how the compat layer works here. If xenforeignmemory_map2()
is not available then you can't control the placement in virtual address space.
Paul
>
> [1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg113007.html
>
> Signed-off-by: Igor Druzhinin
> ---
> configure | 18 ++
> hw/i386/xen/xen-mapcache.c| 79
> ++-
> include/hw/xen/xen_common.h | 7
> include/sysemu/xen-mapcache.h | 11 +-
> 4 files changed, 106 insertions(+), 9 deletions(-)
>
> diff --git a/configure b/configure
> index c571ad1..ad6156b 100755
> --- a/configure
> +++ b/configure
> @@ -2021,6 +2021,24 @@ EOF
> # Xen unstable
> elif
> cat > $TMPC < +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +#include
> +int main(void) {
> + xenforeignmemory_handle *xfmem;
> +
> + xfmem = xenforeignmemory_open(0, 0);
> + xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
> +
> + return 0;
> +}
> +EOF
> +compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
> + then
> + xen_stable_libs="-lxendevicemodel $xen_stable_libs"
> + xen_ctrl_version=41000
> + xen=yes
> +elif
> +cat > $TMPC < #undef XC_WANT_COMPAT_DEVICEMODEL_API
> #define __XEN_TOOLS__
> #include
> diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
> index cd4e746..a988be7 100644
> --- a/hw/i386/xen/xen-mapcache.c
> +++ b/hw/i386/xen/xen-mapcache.c
> @@ -151,6 +151,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
> void *opaque)
> }
>
> static void xen_remap_bucket(MapCacheEntry *entry,
> + void *vaddr,
> hwaddr size,
> hwaddr address_index,
> bool dummy)
> @@ -167,7 +168,9 @@ static void xen_remap_bucket(MapCacheEntry
> *entry,
> err = g_malloc0(nb_pfn * sizeof (int));
>
> if (entry->vaddr_base != NULL) {
> -ram_block_notify_remove(entry->vaddr_base, entry->size);
> +if (entry->vaddr_base != vaddr) {
> +ram_block_notify_remove(entry->vaddr_base, entry->size);
> +}
> if (munmap(entry->vaddr_base, entry->size) != 0) {
> perror("unmap fails");
> exit(-1);
> @@ -181,11 +184,11 @@ static void xen_remap_bucket(MapCacheEntry
> *entry,
> }
>
> if (!dummy) {
> -vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
> - PROT_READ|PROT_WRITE,
> +vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid,
> vaddr,
> + PROT_READ|PROT_WRITE, 0,
> nb_pfn, pfns, err);
> if (vaddr_base == NULL) {
> -perror("xenforeignmemory_map");
> +perror("xenforeignmemory_map2");
> exit(-1);
> }
> entry->flags &= ~(XEN_MAPCACHE_ENTRY_DUMMY);
> @@ -194,7 +197,7 @@ static void xen_remap_bucket(MapCacheEntry
> *entry,
> * We create dummy mappings where we are unable to create a foreign
> * mapping immediately due to certain circumstances (i.e. on resume
> now)
> */
> -vaddr_base = mmap(NULL, size, PROT_READ|PROT_WRITE,
> +vaddr_base = mmap(vaddr, size, PROT_READ|PROT_WRITE,
>MAP_ANON|MAP_SHARED, -1, 0);
> if (vaddr_base == NULL) {
> perror("mmap");
> @@ -203,13 +206,16 @@ static void xen_remap_bucket(MapCacheEntry
> *entry,
> entry->flags |= XEN_MAPCACHE_ENTRY_DUMMY;
> }
>
> +if (entry->vaddr_base == NULL || entry->vaddr_base != vaddr) {
> +ram_block_notify_add(vaddr_base, size);
> +}
> +
> entry->vaddr_base = vaddr_base;
> entry->paddr_index = address_index;
> entry->size = size;
> entry->valid_mapping = (unsigned long *) g_malloc0(sizeof(unsigned long)
> *
> BITS_TO_LONGS(size >> XC_PAGE_SHIFT));
>
> -ram_block_notify_add(entry->vaddr_base, entry->size);
> bitmap_zero(entry->valid_mapping, nb_pfn);
> for (i = 0; i < nb_pfn; i++) {
> if (!err[i]) {
> @@ -282,14 +288,14 @@ tryagain:
> if
Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry()
On 04/07/17 17:27, Paul Durrant wrote:
>> -Original Message-
>> From: Igor Druzhinin
>> Sent: 04 July 2017 16:48
>> To: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org
>> Cc: Igor Druzhinin ; sstabell...@kernel.org;
>> Anthony Perard ; Paul Durrant
>> ; pbonz...@redhat.com
>> Subject: [PATCH v2 3/4] xen/mapcache: introduce
>> xen_replace_cache_entry()
>>
>> This new call is trying to update a requested map cache entry
>> according to the changes in the physmap. The call is searching
>> for the entry, unmaps it and maps again at the same place using
>> a new guest address. If the mapping is dummy this call will
>> make it real.
>>
>> This function makes use of a new xenforeignmemory_map2() call
>> with an extended interface that was recently introduced in
>> libxenforeignmemory [1].
>
> I don't understand how the compat layer works here. If
> xenforeignmemory_map2() is not available then you can't control the placement
> in virtual address space.
>
If it's not 4.10 or newer xenforeignmemory_map2() doesn't exist and is
going to be defined as xenforeignmemory_map(). At the same time
XEN_COMPAT_PHYSMAP is defined and the entry replace function (which
relies on xenforeignmemory_map2 functionality) is never going to be called.
If you mean that I should incorporate this into the description I can do it.
Igor
> Paul
>
>>
>> [1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg113007.html
>>
>> Signed-off-by: Igor Druzhinin
>> ---
>> configure | 18 ++
>> hw/i386/xen/xen-mapcache.c| 79
>> ++-
>> include/hw/xen/xen_common.h | 7
>> include/sysemu/xen-mapcache.h | 11 +-
>> 4 files changed, 106 insertions(+), 9 deletions(-)
>>
>> diff --git a/configure b/configure
>> index c571ad1..ad6156b 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2021,6 +2021,24 @@ EOF
>> # Xen unstable
>> elif
>> cat > $TMPC <> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
>> +#include
>> +int main(void) {
>> + xenforeignmemory_handle *xfmem;
>> +
>> + xfmem = xenforeignmemory_open(0, 0);
>> + xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
>> +
>> + return 0;
>> +}
>> +EOF
>> +compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
>> + then
>> + xen_stable_libs="-lxendevicemodel $xen_stable_libs"
>> + xen_ctrl_version=41000
>> + xen=yes
>> +elif
>> +cat > $TMPC <> #undef XC_WANT_COMPAT_DEVICEMODEL_API
>> #define __XEN_TOOLS__
>> #include
>> diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
>> index cd4e746..a988be7 100644
>> --- a/hw/i386/xen/xen-mapcache.c
>> +++ b/hw/i386/xen/xen-mapcache.c
>> @@ -151,6 +151,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
>> void *opaque)
>> }
>>
>> static void xen_remap_bucket(MapCacheEntry *entry,
>> + void *vaddr,
>> hwaddr size,
>> hwaddr address_index,
>> bool dummy)
>> @@ -167,7 +168,9 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>> err = g_malloc0(nb_pfn * sizeof (int));
>>
>> if (entry->vaddr_base != NULL) {
>> -ram_block_notify_remove(entry->vaddr_base, entry->size);
>> +if (entry->vaddr_base != vaddr) {
>> +ram_block_notify_remove(entry->vaddr_base, entry->size);
>> +}
>> if (munmap(entry->vaddr_base, entry->size) != 0) {
>> perror("unmap fails");
>> exit(-1);
>> @@ -181,11 +184,11 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>> }
>>
>> if (!dummy) {
>> -vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
>> - PROT_READ|PROT_WRITE,
>> +vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid,
>> vaddr,
>> + PROT_READ|PROT_WRITE, 0,
>> nb_pfn, pfns, err);
>> if (vaddr_base == NULL) {
>> -perror("xenforeignmemory_map");
>> +perror("xenforeignmemory_map2");
>> exit(-1);
>> }
>> entry->flags &= ~(XEN_MAPCACHE_ENTRY_DUMMY);
>> @@ -194,7 +197,7 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>> * We create dummy mappings where we are unable to create a foreign
>> * mapping immediately due to certain circumstances (i.e. on resume
>> now)
>> */
>> -vaddr_base = mmap(NULL, size, PROT_READ|PROT_WRITE,
>> +vaddr_base = mmap(vaddr, size, PROT_READ|PROT_WRITE,
>>MAP_ANON|MAP_SHARED, -1, 0);
>> if (vaddr_base == NULL) {
>> perror("mmap");
>> @@ -203,13 +206,16 @@ static void xen_remap_bucket(MapCacheEntry
>> *entry,
>> entry->flags |= XEN_MAPCACHE_ENTRY_DUMMY;
>> }
>>
>> +if (entry->vaddr_base == NULL || entry->vaddr_base != vaddr) {
>> +
Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry()
> -Original Message-
> From: Igor Druzhinin
> Sent: 04 July 2017 17:34
> To: Paul Durrant ; xen-de...@lists.xenproject.org;
> qemu-devel@nongnu.org
> Cc: sstabell...@kernel.org; Anthony Perard ;
> pbonz...@redhat.com
> Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce
> xen_replace_cache_entry()
>
> On 04/07/17 17:27, Paul Durrant wrote:
> >> -Original Message-
> >> From: Igor Druzhinin
> >> Sent: 04 July 2017 16:48
> >> To: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org
> >> Cc: Igor Druzhinin ; sstabell...@kernel.org;
> >> Anthony Perard ; Paul Durrant
> >> ; pbonz...@redhat.com
> >> Subject: [PATCH v2 3/4] xen/mapcache: introduce
> >> xen_replace_cache_entry()
> >>
> >> This new call is trying to update a requested map cache entry
> >> according to the changes in the physmap. The call is searching
> >> for the entry, unmaps it and maps again at the same place using
> >> a new guest address. If the mapping is dummy this call will
> >> make it real.
> >>
> >> This function makes use of a new xenforeignmemory_map2() call
> >> with an extended interface that was recently introduced in
> >> libxenforeignmemory [1].
> >
> > I don't understand how the compat layer works here. If
> xenforeignmemory_map2() is not available then you can't control the
> placement in virtual address space.
> >
>
> If it's not 4.10 or newer xenforeignmemory_map2() doesn't exist and is
> going to be defined as xenforeignmemory_map(). At the same time
> XEN_COMPAT_PHYSMAP is defined and the entry replace function (which
> relies on xenforeignmemory_map2 functionality) is never going to be called.
>
> If you mean that I should incorporate this into the description I can do it.
AFAICT XEN_COMPAT_PHYSMAP is not introduced until patch #4 though.
The problem really comes down to defining xenforeignmemory_map2() in terms of
xenforeignmemory_map(). It basically can't be safely done. Could you define
xenforeignmemory_map2() as abort() in the compat case instead?
Paul
>
> Igor
>
> > Paul
> >
> >>
> >> [1] https://www.mail-archive.com/xen-
> de...@lists.xen.org/msg113007.html
> >>
> >> Signed-off-by: Igor Druzhinin
> >> ---
> >> configure | 18 ++
> >> hw/i386/xen/xen-mapcache.c| 79
> >> ++-
> >> include/hw/xen/xen_common.h | 7
> >> include/sysemu/xen-mapcache.h | 11 +-
> >> 4 files changed, 106 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/configure b/configure
> >> index c571ad1..ad6156b 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -2021,6 +2021,24 @@ EOF
> >> # Xen unstable
> >> elif
> >> cat > $TMPC < >> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> >> +#include
> >> +int main(void) {
> >> + xenforeignmemory_handle *xfmem;
> >> +
> >> + xfmem = xenforeignmemory_open(0, 0);
> >> + xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
> >> +
> >> + return 0;
> >> +}
> >> +EOF
> >> +compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
> >> + then
> >> + xen_stable_libs="-lxendevicemodel $xen_stable_libs"
> >> + xen_ctrl_version=41000
> >> + xen=yes
> >> +elif
> >> +cat > $TMPC < >> #undef XC_WANT_COMPAT_DEVICEMODEL_API
> >> #define __XEN_TOOLS__
> >> #include
> >> diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
> >> index cd4e746..a988be7 100644
> >> --- a/hw/i386/xen/xen-mapcache.c
> >> +++ b/hw/i386/xen/xen-mapcache.c
> >> @@ -151,6 +151,7 @@ void
> xen_map_cache_init(phys_offset_to_gaddr_t f,
> >> void *opaque)
> >> }
> >>
> >> static void xen_remap_bucket(MapCacheEntry *entry,
> >> + void *vaddr,
> >> hwaddr size,
> >> hwaddr address_index,
> >> bool dummy)
> >> @@ -167,7 +168,9 @@ static void xen_remap_bucket(MapCacheEntry
> >> *entry,
> >> err = g_malloc0(nb_pfn * sizeof (int));
> >>
> >> if (entry->vaddr_base != NULL) {
> >> -ram_block_notify_remove(entry->vaddr_base, entry->size);
> >> +if (entry->vaddr_base != vaddr) {
> >> +ram_block_notify_remove(entry->vaddr_base, entry->size);
> >> +}
> >> if (munmap(entry->vaddr_base, entry->size) != 0) {
> >> perror("unmap fails");
> >> exit(-1);
> >> @@ -181,11 +184,11 @@ static void xen_remap_bucket(MapCacheEntry
> >> *entry,
> >> }
> >>
> >> if (!dummy) {
> >> -vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
> >> - PROT_READ|PROT_WRITE,
> >> +vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid,
> >> vaddr,
> >> + PROT_READ|PROT_WRITE, 0,
> >> nb_pfn, pfns, err);
> >> if (vaddr_base == NULL) {
> >> -perror("xenforeignmemory_map");
> >> +perror("xenforeignmemory_map2");
Re: [Qemu-devel] [PATCH v2 3/4] xen/mapcache: introduce xen_replace_cache_entry()
On 04/07/17 17:42, Paul Durrant wrote:
>> -Original Message-
>> From: Igor Druzhinin
>> Sent: 04 July 2017 17:34
>> To: Paul Durrant ; xen-de...@lists.xenproject.org;
>> qemu-devel@nongnu.org
>> Cc: sstabell...@kernel.org; Anthony Perard ;
>> pbonz...@redhat.com
>> Subject: Re: [PATCH v2 3/4] xen/mapcache: introduce
>> xen_replace_cache_entry()
>>
>> On 04/07/17 17:27, Paul Durrant wrote:
-Original Message-
From: Igor Druzhinin
Sent: 04 July 2017 16:48
To: xen-de...@lists.xenproject.org; qemu-devel@nongnu.org
Cc: Igor Druzhinin ; sstabell...@kernel.org;
Anthony Perard ; Paul Durrant
; pbonz...@redhat.com
Subject: [PATCH v2 3/4] xen/mapcache: introduce
xen_replace_cache_entry()
This new call is trying to update a requested map cache entry
according to the changes in the physmap. The call is searching
for the entry, unmaps it and maps again at the same place using
a new guest address. If the mapping is dummy this call will
make it real.
This function makes use of a new xenforeignmemory_map2() call
with an extended interface that was recently introduced in
libxenforeignmemory [1].
>>>
>>> I don't understand how the compat layer works here. If
>> xenforeignmemory_map2() is not available then you can't control the
>> placement in virtual address space.
>>>
>>
>> If it's not 4.10 or newer xenforeignmemory_map2() doesn't exist and is
>> going to be defined as xenforeignmemory_map(). At the same time
>> XEN_COMPAT_PHYSMAP is defined and the entry replace function (which
>> relies on xenforeignmemory_map2 functionality) is never going to be called.
>>
>> If you mean that I should incorporate this into the description I can do it.
>
> AFAICT XEN_COMPAT_PHYSMAP is not introduced until patch #4 though.
>
> The problem really comes down to defining xenforeignmemory_map2() in terms of
> xenforeignmemory_map(). It basically can't be safely done. Could you define
> xenforeignmemory_map2() as abort() in the compat case instead?
>
xen_replace_cache_entry() is not called in patch #3. Which means it's
safe to use a fallback version (xenforeignmemory_map) in
xen_remap_bucket here.
Igor
> Paul
>
>>
>> Igor
>>
>>> Paul
>>>
[1] https://www.mail-archive.com/xen-
>> de...@lists.xen.org/msg113007.html
Signed-off-by: Igor Druzhinin
---
configure | 18 ++
hw/i386/xen/xen-mapcache.c| 79
++-
include/hw/xen/xen_common.h | 7
include/sysemu/xen-mapcache.h | 11 +-
4 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index c571ad1..ad6156b 100755
--- a/configure
+++ b/configure
@@ -2021,6 +2021,24 @@ EOF
# Xen unstable
elif
cat > $TMPC <>>> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
+#include
+int main(void) {
+ xenforeignmemory_handle *xfmem;
+
+ xfmem = xenforeignmemory_open(0, 0);
+ xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
+
+ return 0;
+}
+EOF
+compile_prog "" "$xen_libs -lxendevicemodel $xen_stable_libs"
+ then
+ xen_stable_libs="-lxendevicemodel $xen_stable_libs"
+ xen_ctrl_version=41000
+ xen=yes
+elif
+cat > $TMPC <>>> #undef XC_WANT_COMPAT_DEVICEMODEL_API
#define __XEN_TOOLS__
#include
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index cd4e746..a988be7 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -151,6 +151,7 @@ void
>> xen_map_cache_init(phys_offset_to_gaddr_t f,
void *opaque)
}
static void xen_remap_bucket(MapCacheEntry *entry,
+ void *vaddr,
hwaddr size,
hwaddr address_index,
bool dummy)
@@ -167,7 +168,9 @@ static void xen_remap_bucket(MapCacheEntry
*entry,
err = g_malloc0(nb_pfn * sizeof (int));
if (entry->vaddr_base != NULL) {
-ram_block_notify_remove(entry->vaddr_base, entry->size);
+if (entry->vaddr_base != vaddr) {
+ram_block_notify_remove(entry->vaddr_base, entry->size);
+}
if (munmap(entry->vaddr_base, entry->size) != 0) {
perror("unmap fails");
exit(-1);
@@ -181,11 +184,11 @@ static void xen_remap_bucket(MapCacheEntry
*entry,
}
if (!dummy) {
-vaddr_base = xenforeignmemory_map(xen_fmem, xen_domid,
- PROT_READ|PROT_WRITE,
+vaddr_base = xenforeignmemory_map2(xen_fmem, xen_domid,
vaddr,
+
Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize
On Tue, 4 Jul 2017 17:08:52 +0200
Halil Pasic wrote:
> >>> cd.type = KVM_DEV_TYPE_FLIC;
> >>> ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
> >>> if (ret < 0) {
> >>> -trace_flic_create_device(errno);
> >>> -return;
> >>> +error_setg_errno(&errp_local, errno, "Creating the KVM device
> >>> failed");
> >>> +trace_flic_no_device_api(errno);
> >>
> >> Err... this should still be trace_flic_create_device(), no?
> >
> > I'm afraid you are right! Probably a copy paste error :/
> >
>
> Do you think the traces are still appropriate once we have
> proper error propagation?
They add less value, but I'd just keep them.
>
> I did not feel comfortable removing them but thinking again,
> than might be the thing to do.
>
> @Christian:
> I think we should really fix this the one way or the other.
> Can you tell me what is the proper procedure?
I'd vote for just fixing the trace event. Smaller change, and we can
revisit this later.
[Qemu-devel] [PATCH 2/3] memory.h: Add new utility function memory_region_allocate_aux_memory()
Add a new utility function memory_region_allocate_aux_memory()
for board code to use to create auxiliary memory regions (such
as display RAM or static RAMs). This parallels the existing
memory_region_allocate_system_memory() and wraps up the very
common sequence of:
memory_region_init_ram(sram, NULL, "sram", 0x1, &error_fatal);
vmstate_register_ram_global(sram);
Although this is the parallel function to the existing
memory_region_allocate_system_memory(), we don't put it in
boards.h/numa.c with it because this function is useful for
devices, not just boards, and it has nothing to do with NUMA.
Signed-off-by: Peter Maydell
---
include/exec/memory.h | 23 +++
include/hw/boards.h | 3 ++-
memory.c | 8
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8503685..77fc777 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -412,6 +412,12 @@ void memory_region_init_io(MemoryRegion *mr,
*must be unique within any device
* @size: size of the region.
* @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Note that it is the caller's responsibility to ensure that the contents
+ * of the RAM are migrated (by calling vmstate_register_ram_global(), or
+ * otherwise). The utility function memory_region_allocate_aux_memory()
+ * may be used to combine the "initialize MR" and "register contents for
+ * migration" steps.
*/
void memory_region_init_ram(MemoryRegion *mr,
struct Object *owner,
@@ -631,6 +637,23 @@ void memory_region_init_iommu(MemoryRegion *mr,
uint64_t size);
/**
+ * memory_region_allocate_aux_memory - Allocate auxiliary (non-main) memory
+ * @mr: the #MemoryRegion to be initialized
+ * @owner: the object that tracks the region's reference count
+ * @name: name of the memory region
+ * @ram_size: size of the region in bytes
+ *
+ * This function allocates RAM for a board model or device, and
+ * arranges for it to be migrated (by calling vmstate_register_ram_global()).
+ * Board models should call memory_region_allocate_system_memory()
+ * exactly once to allocate the memory for the primary or largest RAM
+ * area on the board, and then can use this function for smaller
+ * pieces of memory such as display RAM or static RAMs.
+ */
+void memory_region_allocate_aux_memory(MemoryRegion *mr, Object *owner,
+ const char *name, uint64_t ram_size);
+
+/**
* memory_region_owner: get a memory region's owner.
*
* @mr: the memory region being queried.
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 1bc5389..a127a97 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -35,7 +35,8 @@
*
* Smaller pieces of memory (display RAM, static RAMs, etc) don't need
* to be backed via the -mem-path memory backend and can simply
- * be created via memory_region_init_ram().
+ * be created via memory_region_allocate_aux_memory() or
+ * memory_region_init_ram().
*/
void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
const char *name,
diff --git a/memory.c b/memory.c
index 1044bba..25ac7d7 100644
--- a/memory.c
+++ b/memory.c
@@ -32,6 +32,7 @@
#include "sysemu/sysemu.h"
#include "hw/misc/mmio_interface.h"
#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
//#define DEBUG_UNASSIGNED
@@ -2817,6 +2818,13 @@ void mtree_info(fprintf_function mon_printf, void *f,
bool flatview)
}
}
+void memory_region_allocate_aux_memory(MemoryRegion *mr, Object *owner,
+ const char *name, uint64_t ram_size)
+{
+memory_region_init_ram(mr, owner, name, ram_size, &error_fatal);
+vmstate_register_ram_global(mr);
+}
+
static const TypeInfo memory_region_info = {
.parent = TYPE_OBJECT,
.name = TYPE_MEMORY_REGION,
--
2.7.4
[Qemu-devel] [PATCH 1/3] include/hw/boards.h: Document memory_region_allocate_system_memory()
Add a documentation comment for memory_region_allocate_system_memory(). In particular, the reason for this function's existence and the requirement on board code to call it exactly once are non-obvious. Signed-off-by: Peter Maydell --- include/hw/boards.h | 28 1 file changed, 28 insertions(+) diff --git a/include/hw/boards.h b/include/hw/boards.h index 76ce021..1bc5389 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -9,6 +9,34 @@ #include "qom/object.h" #include "qom/cpu.h" +/** + * memory_region_allocate_system_memory - Allocate a board's main memory + * @mr: the #MemoryRegion to be initialized + * @owner: the object that tracks the region's reference count + * @name: name of the memory region + * @ram_size: size of the region in bytes + * + * This function allocates the main memory for a board model, and + * initializes @mr appropriately. It also arranges for the memory + * to be migrated (by calling vmstate_register_ram_global()). + * + * Memory allocated via this function will be backed with the memory + * backend the user provided using -mem-path if appropriate; this + * is typically used to cause host huge pages to be used. + * This function should therefore be called by a board exactly once, + * for the primary or largest RAM area it implements. + * + * For boards where the major RAM is split into two parts in the memory + * map, you can deal with this by calling memory_region_allocate_system_memory() + * once to get a MemoryRegion with enough RAM for both parts, and then + * creating alias MemoryRegions via memory_region_init_alias() which + * alias into different parts of the RAM MemoryRegion and can be mapped + * into the memory map in the appropriate places. + * + * Smaller pieces of memory (display RAM, static RAMs, etc) don't need + * to be backed via the -mem-path memory backend and can simply + * be created via memory_region_init_ram(). + */ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, const char *name, uint64_t ram_size); -- 2.7.4
[Qemu-devel] [PATCH 0/3] Add new utility function memory_region_allocate_aux_memory()
Many board models and several devices need to create auxiliary regions of RAM (in addition to the main lump of 'system' memory), to model static RAMs, video memory, ROMs, etc. Currently they do this with a sequence like: memory_region_init_ram(sram, NULL, "sram", 0x1, &error_fatal); vmstate_register_ram_global(sram); but the need for the second function call is not obvious and if omitted the bug is subtle (possible migration failure). This series wraps the two calls up into a single new function memory_region_allocate_aux_memory(), named to parallel the existing memory_region_allocate_system_memory(). Patch 1 documents memory_region_allocate_system_memory() which has a couple of non-obvious wrinkles. Patch 2 implements the new utility function. Patch 3 changes a lot of callsites to use it with the aid of the included coccinelle patch. (I think most of the remaining callsites of vmstate_register_ram_global() are not changed just because they report the error up to their caller rather than using error_fatal. I'm not sure what kind of error you might get back that wasn't "out of memory", which we usually make fatal anyway, so perhaps we could change those callsites too.) thanks -- PMM Peter Maydell (3): include/hw/boards.h: Document memory_region_allocate_system_memory() memory.h: Add new utility function memory_region_allocate_aux_memory() hw: Use new memory_region_allocate_aux_memory() function include/exec/memory.h | 23 +++ include/hw/boards.h | 29 + hw/arm/exynos4210.c | 10 -- hw/arm/exynos4_boards.c | 12 +--- hw/arm/integratorcp.c | 5 ++--- hw/arm/mainstone.c| 5 ++--- hw/arm/musicpal.c | 5 ++--- hw/arm/omap1.c| 5 ++--- hw/arm/omap2.c| 5 ++--- hw/arm/omap_sx1.c | 10 -- hw/arm/palm.c | 4 +--- hw/arm/pxa2xx.c | 20 hw/arm/realview.c | 14 +- hw/arm/spitz.c| 3 +-- hw/arm/stellaris.c| 9 +++-- hw/arm/stm32f205_soc.c| 10 +++--- hw/arm/tosa.c | 3 +-- hw/arm/vexpress.c | 12 +++- hw/arm/virt.c | 3 +-- hw/arm/xilinx_zynq.c | 5 ++--- hw/arm/xlnx-zynqmp.c | 5 ++--- hw/block/onenand.c| 5 ++--- hw/cris/axis_dev88.c | 5 ++--- hw/display/cg3.c | 5 ++--- hw/display/sm501.c| 5 ++--- hw/display/tc6393xb.c | 5 ++--- hw/display/tcx.c | 5 ++--- hw/display/vmware_vga.c | 5 ++--- hw/i386/pc.c | 5 ++--- hw/i386/pc_sysfw.c| 8 +++- hw/i386/xen/xen-hvm.c | 4 +--- hw/input/milkymist-softusb.c | 10 -- hw/m68k/an5206.c | 3 +-- hw/m68k/mcf5208.c | 3 +-- hw/microblaze/petalogix_ml605_mmu.c | 11 +-- hw/microblaze/petalogix_s3adsp1800_mmu.c | 12 +--- hw/mips/mips_fulong2e.c | 4 +--- hw/mips/mips_jazz.c | 10 -- hw/mips/mips_mipssim.c| 5 ++--- hw/mips/mips_r4k.c| 5 ++--- hw/moxie/moxiesim.c | 6 ++ hw/net/milkymist-minimac2.c | 6 +++--- hw/openrisc/openrisc_sim.c| 3 +-- hw/pci-host/prep.c| 5 + hw/ppc/mac_newworld.c | 5 ++--- hw/ppc/mac_oldworld.c | 5 ++--- hw/ppc/ppc405_boards.c| 14 +- hw/ppc/ppc405_uc.c| 5 ++--- hw/s390x/sclp.c | 5 ++--- hw/sh4/r2d.c | 3 +-- hw/sh4/shix.c | 13 + hw/sparc/leon3.c | 3 +-- hw/sparc/sun4m.c | 13 + hw/sparc64/sun4u.c| 10 -- hw/tricore/tricore_testboard.c| 30 -- hw/unicore32/puv3.c | 4 +--- hw/xtensa/sim.c | 6 ++ hw/xtensa/xtfpga.c| 14 +- memory.c | 8 scripts/coccinelle/allocate_aux_mem.cocci | 14 ++ 60 files changed, 228 insertions(+), 256 deletions(-) create mode 100644 scripts/coccinelle/allocate_aux_mem.cocci -- 2.7.4
[Qemu-devel] [PATCH 3/3] hw: Use new memory_region_allocate_aux_memory() function
Use the new utility function memory_region_allocate_aux_memory() instead of manually calling memory_region_init_ram() and then vmstate_register_ram_global(). Patch automatically created using the included coccinelle script: spatch --in-place -sp_file scripts/coccinelle/allocate_aux_mem.cocci -dir hw Signed-off-by: Peter Maydell --- hw/arm/exynos4210.c | 10 -- hw/arm/exynos4_boards.c | 12 +--- hw/arm/integratorcp.c | 5 ++--- hw/arm/mainstone.c| 5 ++--- hw/arm/musicpal.c | 5 ++--- hw/arm/omap1.c| 5 ++--- hw/arm/omap2.c| 5 ++--- hw/arm/omap_sx1.c | 10 -- hw/arm/palm.c | 4 +--- hw/arm/pxa2xx.c | 20 hw/arm/realview.c | 14 +- hw/arm/spitz.c| 3 +-- hw/arm/stellaris.c| 9 +++-- hw/arm/stm32f205_soc.c| 10 +++--- hw/arm/tosa.c | 3 +-- hw/arm/vexpress.c | 12 +++- hw/arm/virt.c | 3 +-- hw/arm/xilinx_zynq.c | 5 ++--- hw/arm/xlnx-zynqmp.c | 5 ++--- hw/block/onenand.c| 5 ++--- hw/cris/axis_dev88.c | 5 ++--- hw/display/cg3.c | 5 ++--- hw/display/sm501.c| 5 ++--- hw/display/tc6393xb.c | 5 ++--- hw/display/tcx.c | 5 ++--- hw/display/vmware_vga.c | 5 ++--- hw/i386/pc.c | 5 ++--- hw/i386/pc_sysfw.c| 8 +++- hw/i386/xen/xen-hvm.c | 4 +--- hw/input/milkymist-softusb.c | 10 -- hw/m68k/an5206.c | 3 +-- hw/m68k/mcf5208.c | 3 +-- hw/microblaze/petalogix_ml605_mmu.c | 11 +-- hw/microblaze/petalogix_s3adsp1800_mmu.c | 12 +--- hw/mips/mips_fulong2e.c | 4 +--- hw/mips/mips_jazz.c | 10 -- hw/mips/mips_mipssim.c| 5 ++--- hw/mips/mips_r4k.c| 5 ++--- hw/moxie/moxiesim.c | 6 ++ hw/net/milkymist-minimac2.c | 6 +++--- hw/openrisc/openrisc_sim.c| 3 +-- hw/pci-host/prep.c| 5 + hw/ppc/mac_newworld.c | 5 ++--- hw/ppc/mac_oldworld.c | 5 ++--- hw/ppc/ppc405_boards.c| 14 +- hw/ppc/ppc405_uc.c| 5 ++--- hw/s390x/sclp.c | 5 ++--- hw/sh4/r2d.c | 3 +-- hw/sh4/shix.c | 13 + hw/sparc/leon3.c | 3 +-- hw/sparc/sun4m.c | 13 + hw/sparc64/sun4u.c| 10 -- hw/tricore/tricore_testboard.c| 30 -- hw/unicore32/puv3.c | 4 +--- hw/xtensa/sim.c | 6 ++ hw/xtensa/xtfpga.c| 14 +- scripts/coccinelle/allocate_aux_mem.cocci | 14 ++ 57 files changed, 168 insertions(+), 256 deletions(-) create mode 100644 scripts/coccinelle/allocate_aux_mem.cocci diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c index 0050626..92c57bb 100644 --- a/hw/arm/exynos4210.c +++ b/hw/arm/exynos4210.c @@ -276,9 +276,8 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem) &s->chipid_mem); /* Internal ROM */ -memory_region_init_ram(&s->irom_mem, NULL, "exynos4210.irom", - EXYNOS4210_IROM_SIZE, &error_fatal); -vmstate_register_ram_global(&s->irom_mem); +memory_region_allocate_aux_memory(&s->irom_mem, NULL, "exynos4210.irom", + EXYNOS4210_IROM_SIZE); memory_region_set_readonly(&s->irom_mem, true); memory_region_add_subregion(system_mem, EXYNOS4210_IROM_BASE_ADDR, &s->irom_mem); @@ -292,9 +291,8 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem) &s->irom_alias_mem); /* Internal RAM */ -memory_region_init_ram(&s->iram_mem, NULL, "exynos4210.iram", - EXYNOS4210_IRAM_SIZE, &error_fatal); -vmstate_register_ram_global(&s->iram_mem); +memory_region_allocate_aux_memory(&s->iram_mem, NULL, "exynos4210.iram", + EXYNOS4210_IRAM_SIZE); memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_B
Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize
On 07/04/2017 06:59 PM, Cornelia Huck wrote:
> On Tue, 4 Jul 2017 17:08:52 +0200
> Halil Pasic wrote:
>
> cd.type = KVM_DEV_TYPE_FLIC;
> ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
> if (ret < 0) {
> -trace_flic_create_device(errno);
> -return;
> +error_setg_errno(&errp_local, errno, "Creating the KVM device
> failed");
> +trace_flic_no_device_api(errno);
Err... this should still be trace_flic_create_device(), no?
>>>
>>> I'm afraid you are right! Probably a copy paste error :/
>>>
>>
>> Do you think the traces are still appropriate once we have
>> proper error propagation?
>
> They add less value, but I'd just keep them.
>
>>
>> I did not feel comfortable removing them but thinking again,
>> than might be the thing to do.
>>
>> @Christian:
>> I think we should really fix this the one way or the other.
>> Can you tell me what is the proper procedure?
>
> I'd vote for just fixing the trace event. Smaller change, and we can
> revisit this later.
>
+1
Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers
On 03/07/17 10:39, Igor Mammedov wrote:
> On Thu, 29 Jun 2017 15:07:19 +0100
> Mark Cave-Ayland wrote:
>
>> When looking to instantiate a TYPE_FW_CFG_MEM or TYPE_FW_CFG_IO device to be
>> able to wire it up differently, it is much more convenient for the caller to
>> instantiate the device and have the fw_cfg default files already preloaded
>> during realize.
>>
>> Move fw_cfg_init1() to the end of both the fw_cfg_mem_realize() and
>> fw_cfg_io_realize() functions so it no longer needs to be called manually
>> when instantiating the device, and also rename it to fw_cfg_common_realize()
>> which better describes its new purpose.
>>
>> Since it is now the responsibility of the machine to wire up the fw_cfg
>> device
>> it is necessary to introduce a object_property_add_child() call into
>> fw_cfg_init_io() and fw_cfg_init_mem() to link the fw_cfg device to the root
>> machine object as before.
>>
>> Finally we can now convert the asserts() preventing multiple fw_cfg devices
>> and unparented fw_cfg devices being instantiated and replace them with proper
>> error reporting at realize time. This allows us to remove FW_CFG_NAME and
>> FW_CFG_PATH since they are no longer required.
>>
>> Signed-off-by: Mark Cave-Ayland
>> ---
>> hw/nvram/fw_cfg.c | 41 +
>> 1 file changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
>> index 2291121..31029ac 100644
>> --- a/hw/nvram/fw_cfg.c
>> +++ b/hw/nvram/fw_cfg.c
>> @@ -37,9 +37,6 @@
>>
>> #define FW_CFG_FILE_SLOTS_DFLT 0x20
>>
>> -#define FW_CFG_NAME "fw_cfg"
>> -#define FW_CFG_PATH "/machine/" FW_CFG_NAME
>> -
>> #define TYPE_FW_CFG "fw_cfg"
>> #define TYPE_FW_CFG_IO "fw_cfg_io"
>> #define TYPE_FW_CFG_MEM "fw_cfg_mem"
>> @@ -920,19 +917,22 @@ static int fw_cfg_unattached_at_realize(void)
>> }
>>
>>
>> -static void fw_cfg_init1(DeviceState *dev)
>> +static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
>> {
>> FWCfgState *s = FW_CFG(dev);
>> MachineState *machine = MACHINE(qdev_get_machine());
>> uint32_t version = FW_CFG_VERSION;
>>
>> -assert(!object_resolve_path(FW_CFG_PATH, NULL));
>> -
>> -object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s),
>> NULL);
>> -
>> -qdev_init_nofail(dev);
>> +if (!fw_cfg_find()) {
> maybe add comment that here, that fw_cfg_find() will return NULL if more than
> 1 device is found.
This should be the same behaviour as before, i.e. a NULL means that the
fw_cfg device couldn't be found. It seems intuitive to me from the name
of the function exactly what it does, so I don't find the lack of
comment too confusing. Does anyone else have any thoughts here?
>
>> +error_setg(errp, "at most one %s device is permitted", TYPE_FW_CFG);
> s/TYPE_FW_CFG/object_get_typename()/
> so it would print leaf type name
Previously the code would add the device to the machine at FW_CFG_PATH
which was fixed at /machine/fw_cfg regardless of whether it was
fw_cfg_io or fw_cfg_mem type (see the top of fw_cfg.c).
This was a deliberate attempt to preserve the existing behaviour and if
we were to give the leaf type name I think this would be misleading,
since it implies you could have one fw_cfg_io and one fw_cfg_mem which
isn't correct.
>
>> +return;
>> +}
>>
>> -assert(!fw_cfg_unattached_at_realize());
>> +if (fw_cfg_unattached_at_realize()) {
> as I pointed out in v6, this condition will always be false,
> I suggest to drop 4/6 patch and this hunk here so it won't to confuse
> readers with assumption that condition might succeed.
Definitely look more closely at the fw_cfg_unattached_at_realize()
implementation in patch 4. You'll see that the check to determine if the
device is attached does not check obj->parent but checks to see if the
device exists under /machine/unattached which is what the
device_set_realised() does if the device doesn't have a parent.
I can confirm that I've given this a fairly good test with parented and
non-parented objects and it seems to work well here. Does it not work
for you in testing?
>
>> +error_setg(errp, "%s device must be added as a child device before "
>> + "realize", TYPE_FW_CFG);
>> +return;
>> +}
>>
>> fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
>> fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16);
>> @@ -965,7 +965,9 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t
>> dma_iobase,
>> qdev_prop_set_bit(dev, "dma_enabled", false);
>> }
>>
>> -fw_cfg_init1(dev);
>> +object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
>> + OBJECT(dev), NULL);
>> +qdev_init_nofail(dev);
>>
>> sbd = SYS_BUS_DEVICE(dev);
>> ios = FW_CFG_IO(dev);
>> @@ -1003,7 +1005,9 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
>> qdev_prop_set_bit(dev, "dma_enabled", false);
>> }
>
Re: [Qemu-devel] [PATCHv7 3/6] fw_cfg: switch fw_cfg_find() to locate the fw_cfg device by type rather than path
On 03/07/17 10:42, Igor Mammedov wrote:
> On Thu, 29 Jun 2017 15:07:17 +0100
> Mark Cave-Ayland wrote:
>
>> This will enable the fw_cfg device to be placed anywhere within the QOM tree
>> regardless of its machine location.
>>
>> Signed-off-by: Mark Cave-Ayland
>> ---
>> hw/nvram/fw_cfg.c |2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
>> index 99bdbc2..0fe7404 100644
>> --- a/hw/nvram/fw_cfg.c
>> +++ b/hw/nvram/fw_cfg.c
>> @@ -1017,7 +1017,7 @@ FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr
>> data_addr)
>>
>> FWCfgState *fw_cfg_find(void)
>> {
>> -return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
>> +return FW_CFG(object_resolve_path_type("", TYPE_FW_CFG, NULL));
> I insist on using ambiguous argument
>
> see why it's needed
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg460692.html
Yes I did take into account your previous comment about the ambiguous
argument, but with the fw_cfg_unattached_at_realize() check added in the
v7 patchset, it is actually impossible to realize more than one fw_cfg
device, and at realize time that device must have been linked as a child
device to a parent device.
Hence it should be impossible for the above situation where more than
one fw_cfg device can exist in the QOM tree to occur. Have I missed
something in the logic which would prevent this from happening?
ATB,
Mark.
[Qemu-devel] Managing architectural restrictions with -device and libvirt
Hi all, I've been working on a patchset that brings the sun4u machine on qemu-system-sparc64 much closer to a real Ultra 5, however due to various design restrictions I need to be able to restrict how devices are added to the machine with -device. On a real Ultra 5, the root PCI bus (sabre) has 2 PCI bridges (simba A and simba B) with the onboard devices attached to simba A with 2 free slots, and an initially empty simba B. Firstly, is it possible to restrict the machine so that devices cannot be directly plugged into the root PCI bus, but only behind one of the PCI bridges? There is also an additional restriction in that slot 0 behind simba A must be left empty to ensure that the ebus (containing the onboard devices) is the first device allocated. Secondly, how does libvirt handle these type of restrictions? Is it able to get the information from QEMU or is there some kind of libvirt profile that needs to be updated? And do newer versions of libvirt have the ability to attach devices behind PCI bridges using a GUI such as virt-manager, or is that only something that can only be done by directly editing the domain XML? ATB, Mark.
Re: [Qemu-devel] [PATCH 14/22] tcg: add CONFIG_TCG guards in headers
On 07/04/2017 01:12 AM, Paolo Bonzini wrote: From: Yang Zhong Add the CONFIG_TCG for exec-all.h. Since function tlb_set_page_with_attrs() is defined in ./accel/tcg/cputlb.c, which will be disabled if tcg is disabled. This function need be implemented in accel/stubs/tcg-stub.c for disable-tcg. Signed-off-by: Yang Zhong Signed-off-by: Paolo Bonzini --- v2: do not touch include/exec/helper-proto.h [Richard] include/exec/cpu-defs.h | 4 +++- include/exec/cputlb.h | 2 +- include/exec/exec-all.h | 53 ++--- 3 files changed, 32 insertions(+), 27 deletions(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH 16/22] target/i386: move cpu_sync_bndcs_hflags() function
On 07/04/2017 01:12 AM, Paolo Bonzini wrote: From: Yang Zhong Move cpu_sync_bndcs_hflags() function from mpx_helper.c to helper.c because mpx_helper.c need be disabled when tcg is disabled. Signed-off-by: Yang Zhong Signed-off-by: Paolo Bonzini --- v2: moved cpu_report_tpr_access hunk later [Richard] target/i386/helper.c | 30 ++ target/i386/mpx_helper.c | 30 -- 2 files changed, 30 insertions(+), 30 deletions(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH 18/22] target/i386: split cpu_set_mxcsr() and make cpu_set_fpuc() inline
On 07/04/2017 01:12 AM, Paolo Bonzini wrote: From: Yang Zhong Split the cpu_set_mxcsr() and make cpu_set_fpuc() inline with specific tcg code. Signed-off-by: Yang Zhong Signed-off-by: Paolo Bonzini --- v2: renamed tcg_update_mxcsr [Richard], added missing call to cpu_post_load Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH 20/22] target/i386: add the tcg_enabled() in target/i386/
On 07/04/2017 01:12 AM, Paolo Bonzini wrote: From: Yang Zhong Add the tcg_enabled() where the x86 target needs to disable TCG-specific code. Signed-off-by: Yang Zhong Signed-off-by: Paolo Bonzini --- v2: do not touch bpt_helper.c, adjust caller in machine.c [Richard] Reviewed-by: Richard Henderson r~
[Qemu-devel] [PATCH 2/5] migration: Close file on failed migration load
From: "Dr. David Alan Gilbert"
Closing the file before exit on a failure allows
the source to cleanup better, especially with RDMA.
Partial fix for https://bugs.launchpad.net/qemu/+bug/1545052
Signed-off-by: Dr. David Alan Gilbert
---
migration/migration.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration/migration.c b/migration/migration.c
index 51ccd1a4c5..21d6902a29 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -355,6 +355,7 @@ static void process_incoming_migration_co(void *opaque)
MIGRATION_STATUS_FAILED);
error_report("load of migration failed: %s", strerror(-ret));
migrate_decompress_threads_join();
+qemu_fclose(mis->from_src_file);
exit(EXIT_FAILURE);
}
--
2.13.0
[Qemu-devel] [PATCH 1/5] migration/rdma: Fix race on source
From: "Dr. David Alan Gilbert"
Fix a race where the destination might try and send the source a
WRID_READY before the source has done a post-recv for it.
rdma_post_recv has to happen after the qp exists, and we're
OK since we've already called qemu_rdma_source_init that calls
qemu_alloc_qp.
This corresponds to:
https://bugzilla.redhat.com/show_bug.cgi?id=1285044
The race can be triggered by adding a few ms wait before this
post_recv_control (which was originally due to me turning on loads of
debug).
Signed-off-by: Dr. David Alan Gilbert
---
migration/rdma.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index c6bc607a03..6111e10c70 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2365,6 +2365,12 @@ static int qemu_rdma_connect(RDMAContext *rdma, Error
**errp)
caps_to_network(&cap);
+ret = qemu_rdma_post_recv_control(rdma, RDMA_WRID_READY);
+if (ret) {
+ERROR(errp, "posting second control recv");
+goto err_rdma_source_connect;
+}
+
ret = rdma_connect(rdma->cm_id, &conn_param);
if (ret) {
perror("rdma_connect");
@@ -2405,12 +2411,6 @@ static int qemu_rdma_connect(RDMAContext *rdma, Error
**errp)
rdma_ack_cm_event(cm_event);
-ret = qemu_rdma_post_recv_control(rdma, RDMA_WRID_READY);
-if (ret) {
-ERROR(errp, "posting second control recv!");
-goto err_rdma_source_connect;
-}
-
rdma->control_ready_expected = 1;
rdma->nb_sent = 0;
return 0;
--
2.13.0
[Qemu-devel] [PATCH 4/5] migration/rdma: Safely convert control types
From: "Dr. David Alan Gilbert"
control_desc[] is an array of strings that correspond to a
series of message types; they're used only for error messages, but if
the message type is seriously broken then we could go off the end of
the array.
Convert the array to a function control_desc() that bound checks.
Signed-off-by: Dr. David Alan Gilbert
---
migration/rdma.c | 54 --
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 7273ae9929..bfb0a43740 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -165,20 +165,6 @@ enum {
RDMA_CONTROL_UNREGISTER_FINISHED, /* unpinning finished */
};
-static const char *control_desc[] = {
-[RDMA_CONTROL_NONE] = "NONE",
-[RDMA_CONTROL_ERROR] = "ERROR",
-[RDMA_CONTROL_READY] = "READY",
-[RDMA_CONTROL_QEMU_FILE] = "QEMU FILE",
-[RDMA_CONTROL_RAM_BLOCKS_REQUEST] = "RAM BLOCKS REQUEST",
-[RDMA_CONTROL_RAM_BLOCKS_RESULT] = "RAM BLOCKS RESULT",
-[RDMA_CONTROL_COMPRESS] = "COMPRESS",
-[RDMA_CONTROL_REGISTER_REQUEST] = "REGISTER REQUEST",
-[RDMA_CONTROL_REGISTER_RESULT] = "REGISTER RESULT",
-[RDMA_CONTROL_REGISTER_FINISHED] = "REGISTER FINISHED",
-[RDMA_CONTROL_UNREGISTER_REQUEST] = "UNREGISTER REQUEST",
-[RDMA_CONTROL_UNREGISTER_FINISHED] = "UNREGISTER FINISHED",
-};
/*
* Memory and MR structures used to represent an IB Send/Recv work request.
@@ -251,6 +237,30 @@ typedef struct QEMU_PACKED RDMADestBlock {
uint32_t padding;
} RDMADestBlock;
+static const char *control_desc(unsigned int rdma_control)
+{
+static const char *strs[] = {
+[RDMA_CONTROL_NONE] = "NONE",
+[RDMA_CONTROL_ERROR] = "ERROR",
+[RDMA_CONTROL_READY] = "READY",
+[RDMA_CONTROL_QEMU_FILE] = "QEMU FILE",
+[RDMA_CONTROL_RAM_BLOCKS_REQUEST] = "RAM BLOCKS REQUEST",
+[RDMA_CONTROL_RAM_BLOCKS_RESULT] = "RAM BLOCKS RESULT",
+[RDMA_CONTROL_COMPRESS] = "COMPRESS",
+[RDMA_CONTROL_REGISTER_REQUEST] = "REGISTER REQUEST",
+[RDMA_CONTROL_REGISTER_RESULT] = "REGISTER RESULT",
+[RDMA_CONTROL_REGISTER_FINISHED] = "REGISTER FINISHED",
+[RDMA_CONTROL_UNREGISTER_REQUEST] = "UNREGISTER REQUEST",
+[RDMA_CONTROL_UNREGISTER_FINISHED] = "UNREGISTER FINISHED",
+};
+
+if (rdma_control > RDMA_CONTROL_UNREGISTER_FINISHED) {
+return "??BAD CONTROL VALUE??";
+}
+
+return strs[rdma_control];
+}
+
static uint64_t htonll(uint64_t v)
{
union { uint32_t lv[2]; uint64_t llv; } u;
@@ -1632,7 +1642,7 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma,
uint8_t *buf,
.num_sge = 1,
};
-trace_qemu_rdma_post_send_control(control_desc[head->type]);
+trace_qemu_rdma_post_send_control(control_desc(head->type));
/*
* We don't actually need to do a memcpy() in here if we used
@@ -1711,16 +1721,16 @@ static int qemu_rdma_exchange_get_response(RDMAContext
*rdma,
network_to_control((void *) rdma->wr_data[idx].control);
memcpy(head, rdma->wr_data[idx].control, sizeof(RDMAControlHeader));
-trace_qemu_rdma_exchange_get_response_start(control_desc[expecting]);
+trace_qemu_rdma_exchange_get_response_start(control_desc(expecting));
if (expecting == RDMA_CONTROL_NONE) {
-trace_qemu_rdma_exchange_get_response_none(control_desc[head->type],
+trace_qemu_rdma_exchange_get_response_none(control_desc(head->type),
head->type);
} else if (head->type != expecting || head->type == RDMA_CONTROL_ERROR) {
error_report("Was expecting a %s (%d) control message"
", but got: %s (%d), length: %d",
-control_desc[expecting], expecting,
-control_desc[head->type], head->type, head->len);
+control_desc(expecting), expecting,
+control_desc(head->type), head->type, head->len);
if (head->type == RDMA_CONTROL_ERROR) {
rdma->received_error = true;
}
@@ -1830,7 +1840,7 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma,
RDMAControlHeader *head,
}
}
-trace_qemu_rdma_exchange_send_waiting(control_desc[resp->type]);
+trace_qemu_rdma_exchange_send_waiting(control_desc(resp->type));
ret = qemu_rdma_exchange_get_response(rdma, resp,
resp->type, RDMA_WRID_DATA);
@@ -1842,7 +1852,7 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma,
RDMAControlHeader *head,
if (resp_idx) {
*resp_idx = RDMA_WRID_DATA;
}
-trace_qemu_rdma_exchange_send_received(control_desc[resp->type]);
+trace_qemu_rdma_exchange_send_received(control_desc(resp->type));
}
rdma->control_ready_expected = 1;
@@ -3392,7 +3402,7 @@ static int qemu_rdma_registration_h
[Qemu-devel] [PATCH 0/5] A bunch of RDMA fixes
From: "Dr. David Alan Gilbert" Hi, This is a bunch of RDMA fixes, the first is a race I spotted a while ago that you don't hit during normal operation; the rest are to do with migration failure and cancellation that I started looking at because of lp1545052 which is a failure to recover on the source if the destination fails. I'm pretty sure there are other cases where the source might hang waiting for a failed destination; particularly if the destination hangs rather than fails completely; one I know of is waiting for the event after the rdma_disconnect but I don't have a good fix for it. Suggestions welcome. (I sent the 'Fix race on source' patch yesterday, but this set supersedes it). Dave Dr. David Alan Gilbert (5): migration/rdma: Fix race on source migration: Close file on failed migration load migration/rdma: Allow cancelling while waiting for wrid migration/rdma: Safely convert control types migration/rdma: Send error during cancelling migration/migration.c | 1 + migration/rdma.c | 124 -- 2 files changed, 90 insertions(+), 35 deletions(-) -- 2.13.0
[Qemu-devel] [PATCH 3/5] migration/rdma: Allow cancelling while waiting for wrid
From: "Dr. David Alan Gilbert"
When waiting for a WRID, if the other side dies we end up waiting
for ever with no way to cancel the migration.
Cure this by poll()ing the fd first with a timeout and checking
error flags and migration state.
Signed-off-by: Dr. David Alan Gilbert
---
migration/rdma.c | 54 --
1 file changed, 48 insertions(+), 6 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 6111e10c70..7273ae9929 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1466,6 +1466,52 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma,
uint64_t *wr_id_out,
return 0;
}
+/* Wait for activity on the completion channel.
+ * Returns 0 on success, none-0 on error.
+ */
+static int qemu_rdma_wait_comp_channel(RDMAContext *rdma)
+{
+/*
+ * Coroutine doesn't start until migration_fd_process_incoming()
+ * so don't yield unless we know we're running inside of a coroutine.
+ */
+if (rdma->migration_started_on_destination) {
+yield_until_fd_readable(rdma->comp_channel->fd);
+} else {
+/* This is the source side, we're in a separate thread
+ * or destination prior to migration_fd_process_incoming()
+ * we can't yield; so we have to poll the fd.
+ * But we need to be able to handle 'cancel' or an error
+ * without hanging forever.
+ */
+while (!rdma->error_state && !rdma->error_reported &&
+ !rdma->received_error) {
+GPollFD pfds[1];
+pfds[0].fd = rdma->comp_channel->fd;
+pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+/* 0.5s timeout, should be fine for a 'cancel' */
+switch (qemu_poll_ns(pfds, 1, 500 * 1000 * 1000)) {
+case 1: /* fd active */
+return 0;
+
+case 0: /* Timeout, go around again */
+break;
+
+default: /* Error of some type */
+return -1;
+}
+
+if (migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) {
+/* Bail out and let the cancellation happen */
+return -EPIPE;
+}
+}
+}
+
+return rdma->error_state || rdma->error_reported ||
+ rdma->received_error;
+}
+
/*
* Block until the next work request has completed.
*
@@ -1513,12 +1559,8 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma,
int wrid_requested,
}
while (1) {
-/*
- * Coroutine doesn't start until migration_fd_process_incoming()
- * so don't yield unless we know we're running inside of a coroutine.
- */
-if (rdma->migration_started_on_destination) {
-yield_until_fd_readable(rdma->comp_channel->fd);
+if (qemu_rdma_wait_comp_channel(rdma)) {
+goto err_block_for_wrid;
}
if (ibv_get_cq_event(rdma->comp_channel, &cq, &cq_ctx)) {
--
2.13.0
[Qemu-devel] [PATCH 5/5] migration/rdma: Send error during cancelling
From: "Dr. David Alan Gilbert"
When we issue a cancel and clean up the RDMA channel
send a CONTROL_ERROR to get the destination to quit.
The rdma_cleanup code waits for the event to come back
from the rdma_disconnect; but that wont happen until the
destination quits and there's currently nothing to force
it.
Note this makes the case of a cancel work while the destination
is alive, and it already works if the destination is
truly dead. Note it doesn't fix the case where the destination
is hung (we get stuck waiting for the rdma_disconnect event).
Signed-off-by: Dr. David Alan Gilbert
---
migration/rdma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index bfb0a43740..3d17db3a23 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2260,7 +2260,9 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
int ret, idx;
if (rdma->cm_id && rdma->connected) {
-if (rdma->error_state && !rdma->received_error) {
+if ((rdma->error_state ||
+ migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) &&
+!rdma->received_error) {
RDMAControlHeader head = { .len = 0,
.type = RDMA_CONTROL_ERROR,
.repeat = 1,
--
2.13.0
Re: [Qemu-devel] [PATCH v11 04/29] target: [tcg] Add generic translation framework
Richard Henderson writes: > On 06/28/2017 05:32 AM, Lluís Vilanova wrote: >> +void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); >> +void (*init_globals)(DisasContextBase *db, CPUState *cpu); >> +void (*tb_start)(DisasContextBase *db, CPUState *cpu); >> +void (*insn_start)(DisasContextBase *db, CPUState *cpu); >> +BreakpointCheckType (*breakpoint_check)(DisasContextBase *db, CPUState >> *cpu, >> +const CPUBreakpoint *bp); >> +target_ulong (*translate_insn)(DisasContextBase *db, CPUState *cpu); >> +void (*tb_stop)(DisasContextBase *db, CPUState *cpu); >> +void (*disas_log)(const DisasContextBase *db, CPUState *cpu); > Any reason not to stuff the cpu pointer into the DisasContextBase instead of > passing it around separately? None, really. I'll move it from DisasContext (in targets where it's present) into DisasContextBase, and use that one everywhere. > Otherwise, > Reviewed-by: Richard Henderson Thanks, Lluis
[Qemu-devel] [Bug 1545052] Re: RDMA migration will hang forever if target QEMU fails to load vmstate
Fix series posted upstream: 0001-migration-rdma-Fix-race-on-source.patch 0002-migration-Close-file-on-failed-migration-load.patch 0003-migration-rdma-Allow-cancelling-while-waiting-for-wr.patch 0004-migration-rdma-Safely-convert-control-types.patch 0005-migration-rdma-Send-error-during-cancelling.patch ** Changed in: qemu Status: Confirmed => In Progress -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1545052 Title: RDMA migration will hang forever if target QEMU fails to load vmstate Status in QEMU: In Progress Bug description: Get a pair of machines with infiniband support. On one host run $ qemu-system-x86_64 -monitor stdio -incoming rdma:ibme: -vnc :1 -m 1000 To start an incoming migration. Now on the other host, run QEMU with an intentionally different configuration (ie different RAM size) $ qemu-system-x86_64 -monitor stdio -vnc :1 -m 2000 Now trigger a migration on this source host (qemu) migrate rdma:ibpair: You will see on the target host, that it failed to load migration: dest_init RDMA Device opened: kernel name mlx4_0 uverbs device name uverbs0, infiniband_verbs class device path /sys/class/infiniband_verbs/uverbs0, infiniband class device path /sys/class/infiniband/mlx4_0, transport: (2) Ethernet qemu-system-x86_64: Length mismatch: pc.ram: 0x7d00 in != 0x3e80: Invalid argument qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram' This is to be expected, however, at this point QEMU has hung and no longer responds to the monitor GDB shows the target host is stuck in this callpath #0 0x739141cd in write () at ../sysdeps/unix/syscall-template.S:81 #1 0x727fe795 in rdma_get_cm_event.part.15 () from /lib64/librdmacm.so.1 #2 0x5593e445 in qemu_rdma_cleanup (rdma=0x7fff9647e010) at migration/rdma.c:2210 #3 0x5593ea45 in qemu_rdma_close (opaque=0x57796770) at migration/rdma.c:2652 #4 0x559397cc in qemu_fclose (f=f@entry=0x564b1450) at migration/qemu-file.c:270 #5 0x55936b88 in process_incoming_migration_co (opaque=0x564b1450) at migration/migration.c:361 #6 0x55a25a1a in coroutine_trampoline (i0=, i1=) at util/coroutine-ucontext.c:79 #7 0x7fffef5b3110 in ?? () from /lib64/libc.so.6 Now, back on the source host again, you would expect to see that the migrate command failed. Instead, this QEMU is hung too. GDB shows the source host, migrate thread, is stuck in this callpath: #0 0x7391522d in read#1 0x700efd93 in ibv_get_cq_event () at /lib64/libibverbs.so.1 #2 0x559403f2 in qemu_rdma_block_for_wrid (rdma=rdma@entry=0x7fff3d07e010, wrid_requested=wrid_requested@entry=4000, byte_len=byte_len@entry=0x7fff39de370c) at migration/rdma.c:1511 #3 0x5594058a in qemu_rdma_exchange_get_response (rdma=0x7fff3d07e010, head=head@entry=0x7fff39de3780, expecting=expecting@entry=2, idx=idx@entry=0) at migration/rdma.c:1648 #4 0x55941e71 in qemu_rdma_exchange_send (rdma=0x7fff3d07e010, head=0x7fff39de3840, data=0x0, resp=0x7fff39de3870, resp_idx=0x7fff39de3880, callback=0x0) at migration/rdma.c:1725 #5 0x559447e4 in qemu_rdma_registration_stop (f=, opaque=, flags=0, data=) at migration/rdma.c:3302 #6 0x5593bc4b in ram_control_after_iterate (f=f@entry=0x564c20f0, flags=flags@entry=0) at migration/qemu-file.c:157 #7 0x55740b59 in ram_save_setup (f=0x564c20f0, opaque=) at /home/berrange/src/virt/qemu/migration/ram.c:1959 #8 0x557451c1 in qemu_savevm_state_begin (f=0x564c20f0, params=params@entry=0x55f6f048 ) at /home/berrange/src/virt/qemu/migration/savevm.c:919 #9 0x559381a5 in migration_thread (opaque=0x55f6f000 ) at migration/migration.c:1633 #10 0x7390edc5 in start_thread (arg=0x7fff39de4700) at pthread_create.c:308 It should have aborted migrate and set the status to failed. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1545052/+subscriptions
Re: [Qemu-devel] [PATCH v11 04/29] target: [tcg] Add generic translation framework
On 4 July 2017 at 19:59, Lluís Vilanova wrote: > Richard Henderson writes: > >> On 06/28/2017 05:32 AM, Lluís Vilanova wrote: >>> +void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); >>> +void (*init_globals)(DisasContextBase *db, CPUState *cpu); >>> +void (*tb_start)(DisasContextBase *db, CPUState *cpu); >>> +void (*insn_start)(DisasContextBase *db, CPUState *cpu); >>> +BreakpointCheckType (*breakpoint_check)(DisasContextBase *db, CPUState >>> *cpu, >>> +const CPUBreakpoint *bp); >>> +target_ulong (*translate_insn)(DisasContextBase *db, CPUState *cpu); >>> +void (*tb_stop)(DisasContextBase *db, CPUState *cpu); >>> +void (*disas_log)(const DisasContextBase *db, CPUState *cpu); > >> Any reason not to stuff the cpu pointer into the DisasContextBase instead of >> passing it around separately? > > None, really. I'll move it from DisasContext (in targets where it's present) > into DisasContextBase, and use that one everywhere. I kind of like not having CPUState* in DisasContext, because it enforces the rule that you can't read from fields of it inside the target translate.c code without jumping through a hoop (ie copying the info from CPUState->foo to DisasContext->foo). That then acts as a useful flag in code review (or when writing the code) to confirm that foo really is constant for the life of the simulation (or to recommend using a TB flag instead). thanks -- PMM
