Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
On 07.04.2017 17:41, Daniel P. Berrange wrote:
On Fri, Apr 07, 2017 at 05:30:31PM +0300, Amarnath Valluri wrote:
This change introduces a new TPM backend driver that can communicate with
swtpm(software TPM emulator) using unix domain socket interface.
Swtpm uses two unix sockets, one for plain TPM commands and responses, and one
for out-of-band control messages.
The swtpm and associated tools can be found here:
https://github.com/stefanberger/swtpm
Usage:
# setup TPM state directory
mkdir /tmp/mytpm
chown -R tss:root /tmp/mytpm
/usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
# Ask qemu to use TPM emulator with given tpm state directory
qemu-system-x86_64 \
[...] \
-tpmdev emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log
\
-device tpm-tis,tpmdev=tpm0 \
[...]
Signed-off-by: Amarnath Valluri
---
configure | 15 +-
hmp.c | 21 ++
hw/tpm/Makefile.objs | 1 +
hw/tpm/tpm_emulator.c | 927 ++
hw/tpm/tpm_ioctl.h| 243 +
qapi-schema.json | 36 +-
qemu-options.hx | 53 ++-
tpm.c | 2 +-
8 files changed, 1289 insertions(+), 9 deletions(-)
create mode 100644 hw/tpm/tpm_emulator.c
create mode 100644 hw/tpm/tpm_ioctl.h
+static int tpm_emulator_spawn_emulator(TPMEmulator *tpm_pt)
+{
+int fds[2] = { -1, -1 };
+int ctrl_fds[2] = { -1, -1 };
+pid_t cpid;
+
+if (!tpm_pt->ops->has_data_path) {
+if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) < 0) {
+return -1;
+}
+}
+
+if (!tpm_pt->ops->has_ctrl_path) {
+if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, ctrl_fds) < 0) {
+if (!tpm_pt->ops->has_data_path) {
+closesocket(fds[0]);
+closesocket(fds[1]);
+}
+return -1;
+}
+}
+
+cpid = qemu_fork(NULL);
+if (cpid < 0) {
+error_report("tpm-emulator: Fork failure: %s", strerror(errno));
+if (!tpm_pt->ops->has_data_path) {
+closesocket(fds[0]);
+closesocket(fds[1]);
+}
+if (!tpm_pt->ops->has_ctrl_path) {
+closesocket(ctrl_fds[0]);
+closesocket(ctrl_fds[1]);
+}
+return -1;
+}
+
+if (cpid == 0) { /* CHILD */
+enum {
+PARAM_PATH,
+PARAM_IFACE,
+PARAM_SERVER, PARAM_SERVER_ARGS,
+PARAM_CTRL,PARAM_CTRL_ARGS,
+PARAM_STATE, PARAM_STATE_ARGS,
+PARAM_PIDFILE, PARAM_PIDFILE_ARGS,
+PARAM_LOG, PARAM_LOG_ARGS,
+PARAM_MAX
+};
+
+int i;
+int data_fd = -1, ctrl_fd = -1;
+char *argv[PARAM_MAX+1];
+
+/* close all unused inherited sockets */
+if (fds[0] >= 0)
+closesocket(fds[0]);
+if (ctrl_fds[0] >= 0)
+closesocket(ctrl_fds[0]);
The 'if' checks are pointless - its already guaranteed by the
fact you check socketpair() status.
socketpairs might not be created in case of data-path & ctrl-path
provided, so i feel these checks are needed.
+i = STDERR_FILENO + 1;
+if (fds[1] >= 0) {
+data_fd = dup2(fds[1], i++);
+if (data_fd < 0) {
+error_report("tpm-emulator: dup2() failure - %s",
+ strerror(errno));
+goto exit_child;
+}
+}
+if (ctrl_fds[1] >= 0) {
+ctrl_fd = dup2(ctrl_fds[1], i++);
+if (ctrl_fd < 0) {
+error_report("tpm-emulator: dup2() failure - %s",
+ strerror(errno));
+goto exit_child;
+}
+}
+for ( ; i < _SC_OPEN_MAX; i++) {
Errr, _SC_OPEN_MAX is not the maximum number of FDs - it is parameter to
use with sysconf() to query the number of files - you must call sysconf().
Ya, thanks for educating me, i will change this.
+closesocket(i);
close, not closesocket - you can't assume these are all sockets.
Does this change makes any difference, as per
include/sysemu/os-posix.h, closesocket() is define as close(), and this
backend is targeted only for "Linux" targets. Please let me know if i am
missing something.
+DPRINT("\n")
+if (execv(tpm_pt->ops->path, (char * const *)argv) < 0) {
+error_report("execv() failure : %s", strerror(errno));
+}
+
+exit_child:
+g_strfreev(argv);
+if (data_fd >= 0)
+closesocket(data_fd);
+if (ctrl_fd >= 0)
+closesocket(ctrl_fd);
+
+exit(0);
You need _exit(), not exit() as you don't want to run atexit() handlers
here. You also want '1' not '0' since this is a failure scenario.
Sure, i will change this.
+} else { /* self */
+struct stat st;
+DPRINTF("child pid: %d", cpid);
+int
Re: [Qemu-devel] [PATCH v9 1/9] memory: add section range info for IOMMU notifier
On Mon, Apr 10, 2017 at 02:39:22PM +1000, David Gibson wrote:
> On Fri, Apr 07, 2017 at 06:59:07PM +0800, Peter Xu wrote:
> > In this patch, IOMMUNotifier.{start|end} are introduced to store section
> > information for a specific notifier. When notification occurs, we not
> > only check the notification type (MAP|UNMAP), but also check whether the
> > notified iova range overlaps with the range of specific IOMMU notifier,
> > and skip those notifiers if not in the listened range.
> >
> > When removing an region, we need to make sure we removed the correct
> > VFIOGuestIOMMU by checking the IOMMUNotifier.start address as well.
> >
> > This patch is solving the problem that vfio-pci devices receive
> > duplicated UNMAP notification on x86 platform when vIOMMU is there. The
> > issue is that x86 IOMMU has a (0, 2^64-1) IOMMU region, which is
> > splitted by the (0xfee0, 0xfeef) IRQ region. AFAIK
> > this (splitted IOMMU region) is only happening on x86.
> >
> > This patch also helps vhost to leverage the new interface as well, so
> > that vhost won't get duplicated cache flushes. In that sense, it's an
> > slight performance improvement.
> >
> > Suggested-by: David Gibson
> > Reviewed-by: Eric Auger
> > Reviewed-by: Michael S. Tsirkin
> > Acked-by: Alex Williamson
> > Signed-off-by: Peter Xu
> > ---
> > hw/vfio/common.c | 12 +---
> > hw/virtio/vhost.c | 10 --
> > include/exec/memory.h | 19 ++-
> > memory.c | 9 +
> > 4 files changed, 44 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index f3ba9b9..6b33b9f 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -478,8 +478,13 @@ static void vfio_listener_region_add(MemoryListener
> > *listener,
> > giommu->iommu_offset = section->offset_within_address_space -
> > section->offset_within_region;
> > giommu->container = container;
> > -giommu->n.notify = vfio_iommu_map_notify;
> > -giommu->n.notifier_flags = IOMMU_NOTIFIER_ALL;
> > +llend = int128_add(int128_make64(section->offset_within_region),
> > + section->size);
> > +llend = int128_sub(llend, int128_one());
> > +iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
> > +IOMMU_NOTIFIER_ALL,
> > +section->offset_within_region,
> > +int128_get64(llend));
>
> Seems to me it would make sense to put the fiddling around to convert
> the MemoryRegionSection into the necessary values would make sense to
> go inside iommu_notifier_init().
But will we always get one MemoryRegionSection if we are not in any of
the region_{add|del} callback? E.g., what if we want to init an IOMMU
notifier that covers just the whole IOMMU region range?
Considering above, I would still slightly prefer current interface.
>
> > QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
> >
> > memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> > @@ -550,7 +555,8 @@ static void vfio_listener_region_del(MemoryListener
> > *listener,
> > VFIOGuestIOMMU *giommu;
> >
> > QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
> > -if (giommu->iommu == section->mr) {
> > +if (giommu->iommu == section->mr &&
> > +giommu->n.start == section->offset_within_region) {
>
> This test should be sufficient, but I'd be a bit more comfortable if
> there was a test and assert() that the end matches as well. I also
> wonder if remove-matching-notifier helper would be useful here.
> Although vhost doesn't appear to ever remove, so maybe it's premature.
Oh... vhost does remove it, but I just forgot to touch it up :( ...
Thanks for pointing out.
Marcel, if this is the only comment, would you mind squash below
change into current patch? Thanks,
8<
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 185b95b..0001e60 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -771,7 +771,8 @@ static void vhost_iommu_region_del(MemoryListener *listener,
}
QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) {
-if (iommu->mr == section->mr) {
+if (iommu->mr == section->mr &&
+iommu->n.start == section->offset_within_region) {
memory_region_unregister_iommu_notifier(iommu->mr,
&iommu->n);
QLIST_REMOVE(iommu, iommu_next);
>8
>
> > memory_region_unregister_iommu_notifier(giommu->iommu,
> > &giommu->n);
> > QLIST_REMOVE(giommu, giommu_next);
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 613494d..185b95b 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Enable MTTCG on PPC64
Nikunj A Dadhania writes: > Alex Bennée writes: > >> luigi burdo writes: >> >>> Hi David and Nikuji, >>> >>> can i suggest to remove the message: >>> >>> >>> Guest not yet converted to MTTCG - you may get unexpected results >>> where the mttcg is enabled? >> >> Have you declared the memory ordering for the guest? > > No, I havent done that yet, will send a patch. You also need to update configure so mttcg="yes" (resulting in TARGET_SUPPORTS_MTTCG being set for the build). > >>> another thing im finding is this message >>> Guest expects a stronger memory ordering than the host provides >>> This may cause strange/hard to debug errors >> >> See ca759f9e387db87e1719911f019bc60c74be9ed8 for an example. > > Regards > Nikunj -- Alex Bennée
Re: [Qemu-devel] [PATCH 15/15] COLO: flush host dirty ram from cache
On 2017/4/8 1:39, Dr. David Alan Gilbert wrote:
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
Don't need to flush all VM's ram from cache, only
flush the dirty pages since last checkpoint
Cc: Juan Quintela
Signed-off-by: Li Zhijian
Signed-off-by: Zhang Chen
Signed-off-by: zhanghailiang
---
migration/ram.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 6227b94..e9ba740 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2702,6 +2702,7 @@ int colo_init_ram_cache(void)
migration_bitmap_rcu = g_new0(struct BitmapRcu, 1);
migration_bitmap_rcu->bmap = bitmap_new(ram_cache_pages);
migration_dirty_pages = 0;
+memory_global_dirty_log_start();
Shouldn't there be a stop somewhere?
(Probably if you failover to the secondary and colo stops?)
Ha, good catch, i forgot to stop the dirty log in secondary side.
return 0;
@@ -2750,6 +2751,15 @@ void colo_flush_ram_cache(void)
void *src_host;
ram_addr_t offset = 0;
+memory_global_dirty_log_sync();
+qemu_mutex_lock(&migration_bitmap_mutex);
+rcu_read_lock();
+QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
+migration_bitmap_sync_range(block->offset, block->used_length);
+}
+rcu_read_unlock();
+qemu_mutex_unlock(&migration_bitmap_mutex);
Again this might have some fun merging with Juan's recent changes - what's
really unusual about your set is that you're using this bitmap on the
destination;
I suspect Juan's recent changes that trickier.
Check 'Creating RAMState for migration' and 'Split migration bitmaps by
ramblock'.
I have reviewed these two series, and i think it's not a big problem
for COLO here, We can still re-use most of the codes.
Thanks,
Hailiang
Dave
trace_colo_flush_ram_cache_begin(migration_dirty_pages);
rcu_read_lock();
block = QLIST_FIRST_RCU(&ram_list.blocks);
--
1.8.3.1
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
.
[Qemu-devel] [Bug 1662050] Re: qemu-img convert a overlay qcow2 image into a entire image
Hi Lidong Chen: I forgot to use this option.I think the way you said is effective.I will try it.Thank you very much for your help -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1662050 Title: qemu-img convert a overlay qcow2 image into a entire image Status in QEMU: Incomplete Bug description: I have a base image file "base.qcow2" and a delta qcow2 image file "delta.qcow2" whose backing file is "base.qcow2". Now I use qemu-img to convert "delta.qcow2" and will get a new image file "new.qcow2" which is entire and equivalent to combination of "base.qcow2" and "delta.qcow2". In fact,I don't want to get a complete image.I just want to convert delta qcow2 image file "A" to a New delta overlay qcow2 image "B" which is equivalent to "A". So the "new.qcow2" is not what i want. I have to admit that this is not bug. Could you please take this as a new feature and enable qemu-img to convert qcow2 overlay itself? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1662050/+subscriptions
Re: [Qemu-devel] [PATCH 01/15] net/colo: Add notifier/callback related helpers for filter
On 2017/4/7 23:46, Dr. David Alan Gilbert wrote:
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
We will use this notifier to help COLO to notify filter object
to do something, like do checkpoint, or process failover event.
Cc: Jason Wang
Signed-off-by: zhanghailiang
---
net/colo.c | 92 ++
net/colo.h | 18
2 files changed, 110 insertions(+)
<..>
+FilterNotifier *filter_noitifier_new(FilterNotifierCallback *cb,
^ Typo - no*i*tifier
Good catch, will fix it in next version.
(I've not looked at this patch much, I'll leave networking stuff to Jason)
OK, thanks.
Dave
+void *opaque, Error **errp)
+{
+FilterNotifier *notify;
+int ret;
+
+notify = (FilterNotifier *)g_source_new(¬ifier_source_funcs,
+sizeof(FilterNotifier));
+ret = event_notifier_init(¬ify->event, false);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "Failed to initialize event notifier");
+goto fail;
+}
+notify->pfd.fd = event_notifier_get_fd(¬ify->event);
+notify->pfd.events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+notify->cb = cb;
+notify->opaque = opaque;
+g_source_add_poll(¬ify->source, ¬ify->pfd);
+
+return notify;
+
+fail:
+g_source_destroy(¬ify->source);
+return NULL;
+}
+
+int filter_notifier_set(FilterNotifier *notify, uint64_t value)
+{
+ssize_t ret;
+
+do {
+ret = write(notify->event.wfd, &value, sizeof(value));
+} while (ret < 0 && errno == EINTR);
+
+/* EAGAIN is fine, a read must be pending. */
+if (ret < 0 && errno != EAGAIN) {
+return -errno;
+}
+return 0;
+}
diff --git a/net/colo.h b/net/colo.h
index cd9027f..00f03b5 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -19,6 +19,7 @@
#include "qemu/jhash.h"
#include "qemu/timer.h"
#include "slirp/tcp.h"
+#include "qemu/event_notifier.h"
#define HASHTABLE_MAX_SIZE 16384
@@ -89,4 +90,21 @@ void connection_hashtable_reset(GHashTable *connection_track_table);
Packet *packet_new(const void *data, int size);
void packet_destroy(void *opaque, void *user_data);
+typedef void FilterNotifierCallback(void *opaque, int value);
+typedef struct FilterNotifier {
+GSource source;
+EventNotifier event;
+GPollFD pfd;
+FilterNotifierCallback *cb;
+void *opaque;
+} FilterNotifier;
+
+FilterNotifier *filter_noitifier_new(FilterNotifierCallback *cb,
+void *opaque, Error **errp);
+int filter_notifier_set(FilterNotifier *notify, uint64_t value);
+
+enum {
+COLO_CHECKPOINT = 2,
+COLO_FAILOVER,
+};
#endif /* QEMU_COLO_PROXY_H */
--
1.8.3.1
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
.
Re: [Qemu-devel] [PATCH 07/15] COLO: Load PVM's dirty pages into SVM's RAM cache temporarily
On 2017/4/8 1:06, Dr. David Alan Gilbert wrote:
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
We should not load PVM's state directly into SVM, because there maybe some
errors happen when SVM is receving data, which will break SVM.
We need to ensure receving all data before load the state into SVM. We use
an extra memory to cache these data (PVM's ram). The ram cache in secondary side
is initially the same as SVM/PVM's memory. And in the process of checkpoint,
we cache the dirty pages of PVM into this ram cache firstly, so this ram cache
always the same as PVM's memory at every checkpoint, then we flush this cached
ram
to SVM after we receive all PVM's state.
You're probably going to find this interesting to merge with Juan's recent RAM
block series.
Probably not too hard, but he's touching a lot of the same code and rearranging
things.
Yes, I'll update this series on top of his series, better to send next version
after his series been merged.
Thanks,
Hailiang
Dave
Cc: Juan Quintela
Signed-off-by: zhanghailiang
Signed-off-by: Li Zhijian
Reviewed-by: Dr. David Alan Gilbert
---
include/exec/ram_addr.h | 1 +
include/migration/migration.h | 4 +++
migration/colo.c | 14 +
migration/ram.c | 73 ++-
4 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 3e79466..44e1190 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -27,6 +27,7 @@ struct RAMBlock {
struct rcu_head rcu;
struct MemoryRegion *mr;
uint8_t *host;
+uint8_t *colo_cache; /* For colo, VM's ram cache */
ram_addr_t offset;
ram_addr_t used_length;
ram_addr_t max_length;
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 1735d66..93c6148 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -379,4 +379,8 @@ int ram_save_queue_pages(MigrationState *ms, const char
*rbname,
PostcopyState postcopy_state_get(void);
/* Set the state and return the old state */
PostcopyState postcopy_state_set(PostcopyState new_state);
+
+/* ram cache */
+int colo_init_ram_cache(void);
+void colo_release_ram_cache(void);
#endif
diff --git a/migration/colo.c b/migration/colo.c
index 1e3e975..edb7f00 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -551,6 +551,7 @@ void *colo_process_incoming_thread(void *opaque)
uint64_t total_size;
uint64_t value;
Error *local_err = NULL;
+int ret;
qemu_sem_init(&mis->colo_incoming_sem, 0);
@@ -572,6 +573,12 @@ void *colo_process_incoming_thread(void *opaque)
*/
qemu_file_set_blocking(mis->from_src_file, true);
+ret = colo_init_ram_cache();
+if (ret < 0) {
+error_report("Failed to initialize ram cache");
+goto out;
+}
+
bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
object_unref(OBJECT(bioc));
@@ -705,11 +712,18 @@ out:
if (fb) {
qemu_fclose(fb);
}
+/*
+ * We can ensure BH is hold the global lock, and will join COLO
+ * incoming thread, so here it is not necessary to lock here again,
+ * Or there will be a deadlock error.
+ */
+colo_release_ram_cache();
/* Hope this not to be too long to loop here */
qemu_sem_wait(&mis->colo_incoming_sem);
qemu_sem_destroy(&mis->colo_incoming_sem);
/* Must be called after failover BH is completed */
+
if (mis->to_src_file) {
qemu_fclose(mis->to_src_file);
}
diff --git a/migration/ram.c b/migration/ram.c
index f289fcd..b588990 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -219,6 +219,7 @@ static RAMBlock *last_sent_block;
static ram_addr_t last_offset;
static QemuMutex migration_bitmap_mutex;
static uint64_t migration_dirty_pages;
+static bool ram_cache_enable;
static uint32_t last_version;
static bool ram_bulk_stage;
@@ -2227,6 +2228,20 @@ static inline void *host_from_ram_block_offset(RAMBlock *block,
return block->host + offset;
}
+static inline void *colo_cache_from_block_offset(RAMBlock *block,
+ ram_addr_t offset)
+{
+if (!offset_in_ramblock(block, offset)) {
+return NULL;
+}
+if (!block->colo_cache) {
+error_report("%s: colo_cache is NULL in block :%s",
+ __func__, block->idstr);
+return NULL;
+}
+return block->colo_cache + offset;
+}
+
/*
* If a page (or a whole RDMA chunk) has been
* determined to be zero, then zap it.
@@ -2542,7 +2557,12 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
RAMBlock *block = ram_block_from_stream(f, flags);
-host = host_from_ram_block_offset(
Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
On 07.04.2017 18:11, Marc-André Lureau wrote:
Hi
On Fri, Apr 7, 2017 at 4:41 PM Daniel P. Berrange
> +.name = "data-path",
> +.type = QEMU_OPT_STRING,
> +.help = "Socket path to use for data exhange",
> +},
> +{
> +.name = "ctrl-path",
> +.type = QEMU_OPT_STRING,
> +.help = "Socket path to use for out-of-band control
messages",
> +},
I'm still not convinced by the need for 2 separate UNIX sockets,
unless
there's a performance reason, but that looks unlikely.
We would also need something more than an implementation of the
protocol that qemu is going to rely on as an external dependency. A
protocol specification would help to start the discussion.
I would agree with you, Can we adopt the current swtpm's control
interface as initial proposal.
Alternatively, I would suggest to not rely on a public protocol,
What do you mean by public protocol here, can you please help me
understanding this.
- Amarnath
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Enable MTTCG on PPC64
Alex Bennée writes: > Nikunj A Dadhania writes: > >> Alex Bennée writes: >> >>> luigi burdo writes: >>> Hi David and Nikuji, can i suggest to remove the message: Guest not yet converted to MTTCG - you may get unexpected results where the mttcg is enabled? >>> >>> Have you declared the memory ordering for the guest? >> >> No, I havent done that yet, will send a patch. > > You also need to update configure so mttcg="yes" (resulting in > TARGET_SUPPORTS_MTTCG being set for the build). Yes, I have that patch for ppc64, will send it to the list. Regards Nikunj
[Qemu-devel] [PATCH for-2.9] iscsi: Fix iscsi_create
Since d5895fcb (iscsi: Split URL into individual options), creating
qcow2 image on an iscsi LUN fails:
qemu-img create -f qcow2 iscsi://$SERVER/$IQN/0 1G
qemu-img: iscsi://$SERVER/$IQN/0: Could not create image: Invalid
argument
The problem is iscsi_open now expects that transport_name, portal and
target are already parsed into structured options by
iscsi_parse_filename, but it is not called in iscsi_create.
Signed-off-by: Fam Zheng
---
block/iscsi.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 716e74a..07596b5 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2092,6 +2092,7 @@ static int iscsi_create(const char *filename, QemuOpts
*opts, Error **errp)
BlockDriverState *bs;
IscsiLun *iscsilun = NULL;
QDict *bs_options;
+Error *local_err = NULL;
bs = bdrv_new();
@@ -2103,7 +2104,13 @@ static int iscsi_create(const char *filename, QemuOpts
*opts, Error **errp)
bs_options = qdict_new();
qdict_put(bs_options, "filename", qstring_from_str(filename));
-ret = iscsi_open(bs, bs_options, 0, NULL);
+iscsi_parse_filename(filename, bs_options, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+} else {
+ret = iscsi_open(bs, bs_options, 0, NULL);
+}
QDECREF(bs_options);
if (ret != 0) {
--
2.9.3
Re: [Qemu-devel] [PATCH fix for-2.9] cpus: fix wrong define name
Nikunj A Dadhania writes:
> While the configure script generates TARGET_SUPPORTS_MTTCG define, one
> of the define is cpus.c is checking wrong name: TARGET_SUPPORT_MTTCG
>
> Signed-off-by: Nikunj A Dadhania
> ---
> cpus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index 68fdbc4..58d90aa 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
> } else if (use_icount) {
> error_setg(errp, "No MTTCG when icount is enabled");
> } else {
> -#ifndef TARGET_SUPPORT_MTTCG
> +#ifndef TARGET_SUPPORTS_MTTCG
> error_report("Guest not yet converted to MTTCG - "
> "you may get unexpected results");
> #endif
Opps that's embarrassing.
Applied to my tree, I'll be sending a pullreq today
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2 4/6] block: Quiesce old aio context during bdrv_set_aio_context
Am 08.04.2017 um 05:43 hat Fam Zheng geschrieben:
> On Fri, 04/07 13:50, Stefan Hajnoczi wrote:
> > On Fri, Apr 07, 2017 at 02:54:12PM +0800, Fam Zheng wrote:
> > > @@ -4413,6 +4416,10 @@ void bdrv_set_aio_context(BlockDriverState *bs,
> > > AioContext *new_context)
> > > aio_context_acquire(new_context);
> > > bdrv_attach_aio_context(bs, new_context);
> > > aio_context_release(new_context);
> > > +if (bs->job) {
> > > +block_job_resume(bs->job);
> > > +}
> >
> > Should this be called before aio_context_release(new_context)?
>
> Yes, and I'm going to replace it with bdrv_parent_drained_begin() as Kevin
> suggested.
I think at the moment bdrv_parent_drained_begin() can't replace it yet,
but you need both.
Kevin
[Qemu-devel] [PATCH for 2.10] tcg: enable MTTCG by default for PPC64 on x86
This enables the multi-threaded system emulation by default for PPC64 guests using the x86_64 TCG back-end. Signed-off-by: Nikunj A Dadhania --- Depends on following patch which fixes the define name: https://patchwork.ozlabs.org/patch/748840/ --- configure| 2 ++ target/ppc/cpu.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/configure b/configure index 4b3b5cd..2a87495 100755 --- a/configure +++ b/configure @@ -6008,12 +6008,14 @@ case "$target_name" in ppc64) TARGET_BASE_ARCH=ppc TARGET_ABI_DIR=ppc +mttcg=yes gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml power-vsx.xml" ;; ppc64le) TARGET_ARCH=ppc64 TARGET_BASE_ARCH=ppc TARGET_ABI_DIR=ppc +mttcg=yes gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml power-vsx.xml" ;; ppc64abi32) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index e0ff041..ece535d 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -30,6 +30,8 @@ #define TARGET_LONG_BITS 64 #define TARGET_PAGE_BITS 12 +#define TCG_GUEST_DEFAULT_MO 0 + /* Note that the official physical address space bits is 62-M where M is implementation dependent. I've not looked up M for the set of cpus we emulate at the system level. */ -- 2.9.3
Re: [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration
On 2017/4/6 21:13, Juan Quintela wrote: Hi This updates patches with all the comments received. I move qdev_unplug() to make linux-user compile. Please, review. [RFC - v1] This series disable hotplug/unplug during migration. Thank to Markus for explaining where I had to put the checks. Why? Because during migration we will fail if there are changes. For instance, in postcopy, if we add a memory region, we would failing. Same for other devices if they are not setup exactly the same on destination. Iidea would be to disable it, andthen enable for the thing that we know that work. This series are on top of my previous RAMState v2 serie. Commets, please? Make sense, this will benefit COLO too :) After the types found by Eric be fixed in patch 5, Reviewed-by: zhanghailiang Thanks, Juan. *** BLURB HERE *** Juan Quintela (5): qdev: qdev_hotplug is really a bool qdev: Export qdev_hot_removed qdev: Move qdev_unplug() to qdev-monitor.c migration: Disable hotplug/unplug during migration ram: Remove migration_bitmap_extend() exec.c | 1 - hw/core/qdev.c | 40 +++- include/exec/ram_addr.h | 2 -- include/hw/qdev-core.h | 3 ++- migration/ram.c | 34 -- qdev-monitor.c | 45 + 6 files changed, 50 insertions(+), 75 deletions(-)
Re: [Qemu-devel] [PATCH 12/15] savevm: split the process of different stages for loadvm/savevm
On 2017/4/8 1:18, Dr. David Alan Gilbert wrote:
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
There are several stages during loadvm/savevm process. In different stage,
migration incoming processes different types of sections.
We want to control these stages more accuracy, it will benefit COLO
performance, we don't have to save type of QEMU_VM_SECTION_START
sections everytime while do checkpoint, besides, we want to separate
the process of saving/loading memory and devices state.
So we add three new helper functions: qemu_loadvm_state_begin(),
qemu_load_device_state() and qemu_savevm_live_state() to achieve
different process during migration.
Besides, we make qemu_loadvm_state_main() and qemu_save_device_state()
public.
Cc: Juan Quintela
Signed-off-by: zhanghailiang
Signed-off-by: Li Zhijian
Reviewed-by: Dr. David Alan Gilbert
---
include/sysemu/sysemu.h | 6 ++
migration/savevm.c | 55 ++---
2 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 7ed665a..95cae41 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -132,7 +132,13 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f,
const char *name,
uint64_t *start_list,
uint64_t *length_list);
+void qemu_savevm_live_state(QEMUFile *f);
+int qemu_save_device_state(QEMUFile *f);
+
int qemu_loadvm_state(QEMUFile *f);
+int qemu_loadvm_state_begin(QEMUFile *f);
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
+int qemu_load_device_state(QEMUFile *f);
extern int autostart;
diff --git a/migration/savevm.c b/migration/savevm.c
index 9c2d239..dac478b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -54,6 +54,7 @@
#include "qemu/cutils.h"
#include "io/channel-buffer.h"
#include "io/channel-file.h"
+#include "migration/colo.h"
#ifndef ETH_P_RARP
#define ETH_P_RARP 0x8035
@@ -1279,13 +1280,21 @@ done:
return ret;
}
-static int qemu_save_device_state(QEMUFile *f)
+void qemu_savevm_live_state(QEMUFile *f)
{
-SaveStateEntry *se;
+/* save QEMU_VM_SECTION_END section */
+qemu_savevm_state_complete_precopy(f, true);
+qemu_put_byte(f, QEMU_VM_EOF);
+}
-qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
-qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+int qemu_save_device_state(QEMUFile *f)
+{
+SaveStateEntry *se;
+if (!migration_in_colo_state()) {
+qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
+qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+}
Note that got split out into qemu_savevm_state_header() at some point.
Do you mean i should use the wrapper qemu_savevm_state_heade() here ?
Dave
cpu_synchronize_all_states();
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
@@ -1336,8 +1345,6 @@ enum LoadVMExitCodes {
LOADVM_QUIT = 1,
};
-static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
-
/* -- incoming postcopy messages -- */
/* 'advise' arrives before any transfers just to tell us that a postcopy
* *might* happen - it might be skipped if precopy transferred everything
@@ -1942,7 +1949,7 @@ qemu_loadvm_section_part_end(QEMUFile *f,
MigrationIncomingState *mis)
return 0;
}
-static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
{
uint8_t section_type;
int ret = 0;
@@ -2080,6 +2087,40 @@ int qemu_loadvm_state(QEMUFile *f)
return ret;
}
+int qemu_loadvm_state_begin(QEMUFile *f)
+{
+MigrationIncomingState *mis = migration_incoming_get_current();
+Error *local_err = NULL;
+int ret;
+
+if (qemu_savevm_state_blocked(&local_err)) {
+error_report_err(local_err);
+return -EINVAL;
+}
+/* Load QEMU_VM_SECTION_START section */
+ret = qemu_loadvm_state_main(f, mis);
+if (ret < 0) {
+error_report("Failed to loadvm begin work: %d", ret);
+}
+return ret;
+}
+
+int qemu_load_device_state(QEMUFile *f)
+{
+MigrationIncomingState *mis = migration_incoming_get_current();
+int ret;
+
+/* Load QEMU_VM_SECTION_FULL section */
+ret = qemu_loadvm_state_main(f, mis);
+if (ret < 0) {
+error_report("Failed to load device state: %d", ret);
+return ret;
+}
+
+cpu_synchronize_all_post_init();
+return 0;
+}
+
int save_vmstate(Monitor *mon, const char *name)
{
BlockDriverState *bs, *bs1;
--
1.8.3.1
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
.
Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
On Mon, Apr 10, 2017 at 10:08:21AM +0300, Amarnath Valluri wrote:
>
>
> On 07.04.2017 17:41, Daniel P. Berrange wrote:
> > On Fri, Apr 07, 2017 at 05:30:31PM +0300, Amarnath Valluri wrote:
> > > This change introduces a new TPM backend driver that can communicate with
> > > swtpm(software TPM emulator) using unix domain socket interface.
> > >
> > > Swtpm uses two unix sockets, one for plain TPM commands and responses,
> > > and one
> > > for out-of-band control messages.
> > >
> > > The swtpm and associated tools can be found here:
> > > https://github.com/stefanberger/swtpm
> > >
> > > Usage:
> > > # setup TPM state directory
> > > mkdir /tmp/mytpm
> > > chown -R tss:root /tmp/mytpm
> > > /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek
> > >
> > > # Ask qemu to use TPM emulator with given tpm state directory
> > > qemu-system-x86_64 \
> > > [...] \
> > > -tpmdev
> > > emulator,id=tpm0,tpmstatedir=/tmp/mytpm,logfile=/tmp/swtpm.log \
> > > -device tpm-tis,tpmdev=tpm0 \
> > > [...]
> > >
> > > Signed-off-by: Amarnath Valluri
> > > ---
> > > configure | 15 +-
> > > hmp.c | 21 ++
> > > hw/tpm/Makefile.objs | 1 +
> > > hw/tpm/tpm_emulator.c | 927
> > > ++
> > > hw/tpm/tpm_ioctl.h| 243 +
> > > qapi-schema.json | 36 +-
> > > qemu-options.hx | 53 ++-
> > > tpm.c | 2 +-
> > > 8 files changed, 1289 insertions(+), 9 deletions(-)
> > > create mode 100644 hw/tpm/tpm_emulator.c
> > > create mode 100644 hw/tpm/tpm_ioctl.h
> > > +static int tpm_emulator_spawn_emulator(TPMEmulator *tpm_pt)
> > > +{
> > > +int fds[2] = { -1, -1 };
> > > +int ctrl_fds[2] = { -1, -1 };
> > > +pid_t cpid;
> > > +
> > > +if (!tpm_pt->ops->has_data_path) {
> > > +if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) < 0) {
> > > +return -1;
> > > +}
> > > +}
> > > +
> > > +if (!tpm_pt->ops->has_ctrl_path) {
> > > +if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, ctrl_fds) < 0) {
> > > +if (!tpm_pt->ops->has_data_path) {
> > > +closesocket(fds[0]);
> > > +closesocket(fds[1]);
> > > +}
> > > +return -1;
> > > +}
> > > +}
> > > +
> > > +cpid = qemu_fork(NULL);
> > > +if (cpid < 0) {
> > > +error_report("tpm-emulator: Fork failure: %s", strerror(errno));
> > > +if (!tpm_pt->ops->has_data_path) {
> > > +closesocket(fds[0]);
> > > +closesocket(fds[1]);
> > > +}
> > > +if (!tpm_pt->ops->has_ctrl_path) {
> > > +closesocket(ctrl_fds[0]);
> > > +closesocket(ctrl_fds[1]);
> > > +}
> > > +return -1;
> > > +}
> > > +
> > > +if (cpid == 0) { /* CHILD */
> > > +enum {
> > > +PARAM_PATH,
> > > +PARAM_IFACE,
> > > +PARAM_SERVER, PARAM_SERVER_ARGS,
> > > +PARAM_CTRL,PARAM_CTRL_ARGS,
> > > +PARAM_STATE, PARAM_STATE_ARGS,
> > > +PARAM_PIDFILE, PARAM_PIDFILE_ARGS,
> > > +PARAM_LOG, PARAM_LOG_ARGS,
> > > +PARAM_MAX
> > > +};
> > > +
> > > +int i;
> > > +int data_fd = -1, ctrl_fd = -1;
> > > +char *argv[PARAM_MAX+1];
> > > +
> > > +/* close all unused inherited sockets */
> > > +if (fds[0] >= 0)
> > > +closesocket(fds[0]);
> > > +if (ctrl_fds[0] >= 0)
> > > +closesocket(ctrl_fds[0]);
> > The 'if' checks are pointless - its already guaranteed by the
> > fact you check socketpair() status.
> socketpairs might not be created in case of data-path & ctrl-path provided,
> so i feel these checks are needed.
> >
> > > +i = STDERR_FILENO + 1;
> > > +if (fds[1] >= 0) {
> > > +data_fd = dup2(fds[1], i++);
> > > +if (data_fd < 0) {
> > > +error_report("tpm-emulator: dup2() failure - %s",
> > > + strerror(errno));
> > > +goto exit_child;
> > > +}
> > > +}
> > > +if (ctrl_fds[1] >= 0) {
> > > +ctrl_fd = dup2(ctrl_fds[1], i++);
> > > +if (ctrl_fd < 0) {
> > > +error_report("tpm-emulator: dup2() failure - %s",
> > > + strerror(errno));
> > > +goto exit_child;
> > > +}
> > > +}
> > > +for ( ; i < _SC_OPEN_MAX; i++) {
> > Errr, _SC_OPEN_MAX is not the maximum number of FDs - it is parameter to
> > use with sysconf() to query the number of files - you must call sysconf().
> Ya, thanks for educating me, i will change this.
> >
> > > +closesocket(i);
> > close, not closesocket - you can't assume these are all sockets.
> Does this change makes any difference, as per include/syse
Re: [Qemu-devel] [PATCH v2] hw/net: convert "dma" property type from ptr to link
On 8 April 2017 at 15:36, Suramya Shah wrote:
> The lance device needs pointer to ISA DMA device to operate. But according to
> qdev-properties.h, properties of pointer type should be avoided.
> A link type property is a good substitution.
>
> Changes since v1
> -changed the code in hw/sparc/sun4m.c which uses the device.
>
> Signed-off-by: Suramya Shah
> ---
> hw/net/lance.c | 8 ++--
> hw/sparc/sun4m.c | 2 +-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/lance.c b/hw/net/lance.c
> index 573d724..3996b9c 100644
> --- a/hw/net/lance.c
> +++ b/hw/net/lance.c
> @@ -44,6 +44,7 @@
> #include "pcnet.h"
> #include "trace.h"
> #include "sysemu/sysemu.h"
> +#include "qapi/error.h"
>
> #define TYPE_LANCE "lance"
> #define SYSBUS_PCNET(obj) \
> @@ -145,10 +146,14 @@ static void lance_instance_init(Object *obj)
> device_add_bootindex_property(obj, &s->conf.bootindex,
>"bootindex", "/ethernet-phy@0",
>DEVICE(obj), NULL);
> +
> +object_property_add_link(obj, "dma", TYPE_LANCE,
> + (Object **)&d->state.dma_opaque,
> + qdev_prop_allow_set_link_before_realize,
> + 0, &error_abort);
> }
>
> static Property lance_properties[] = {
> -DEFINE_PROP_PTR("dma", SysBusPCNetState, state.dma_opaque),
> DEFINE_NIC_PROPERTIES(SysBusPCNetState, state.conf),
> DEFINE_PROP_END_OF_LIST(),
> };
> @@ -164,7 +169,6 @@ static void lance_class_init(ObjectClass *klass, void
> *data)
> dc->reset = lance_reset;
> dc->vmsd = &vmstate_lance;
> dc->props = lance_properties;
> -/* Reason: pointer property "dma" */
> dc->cannot_instantiate_with_device_add_yet = true;
Please don't remove the comment but not the line it refers
to; either the comment needs updating or the cannot_instantiate
setting is no longer correct and needs removing.
thanks
-- PMM
Re: [Qemu-devel] [PATCH] hw/core: fix segmentation fault
On 8 April 2017 at 16:05, Suramya Shah wrote:
> Reproducer:
> $i386-softmmu/qemu-system-i386 -S -machine isapc,accel=tcg -device amd-iommu
> Segmentation fault (core dumped)
>
> Partial bt:
> #0 bus_add_child (child=0x56d4e520, bus=0x0) at hw/core/qdev.c:88
> #1 qdev_set_parent_bus (dev=0x56d4e520, bus=bus@entry=0x0)
> at hw/core/qdev.c:119
>
> Signed-off-by: Suramya Shah
> ---
> hw/core/qdev.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 1e7fb33..07a211b 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -84,7 +84,11 @@ static void bus_add_child(BusState *bus, DeviceState
> *child)
> {
> char name[32];
> BusChild *kid = g_malloc0(sizeof(*kid));
> -
> +
> +if (!bus) {
> +error_report("bus not found ");
> +exit(0);
> +}
> kid->index = bus->max_index++;
> kid->child = child;
> object_ref(OBJECT(kid->child));
Thanks for the patch, but this doesn't look like the right way to fix
this. Exiting with an error in a low-level function like this is not
really any better than crashing. This function is not supposed to
be called with a NULL bus pointer, so the question is: why is that
happening? Something in one of the calling functions, probably
several layers up the stack, has a bug. To fix this bug we need to
look at that code and identify why it is trying to add the child
device to a NULL bus and what bus it ought to be putting it on
instead.
thanks
-- PMM
Re: [Qemu-devel] [RFC] Proposed qcow2 extension: subcluster allocation
Am 07.04.2017 um 19:10 hat Max Reitz geschrieben: > One case I'd be especially interested in are of course 4 kB subclusters > for 64 kB clusters (because 4 kB is a usual page size and can be > configured to be the block size of a guest device; and because 64 kB > simply is the standard cluster size of qcow2 images nowadays[1]...). Why should the current default cluster size be an argument for anything? 64k is a tradeoff between small COW size and large allocation granularity that seemed to work well (and it used to be the maximum cluster size originally, so it seemed safer than 128k). With subclusters, we have a completely different situation and need to find a new default that works best. I'm relatively sure that 64k are too small under the new conditions. Also, undefined reference: [1] (I was hoping to find a better argument there...) Kevin pgpgGWb1zaghS.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] target/arm: Add assertion about FSC format for syndrome registers
On Thu, Apr 06, 2017 at 02:42:32PM +0100, Peter Maydell wrote:
> In tlb_fill() we construct a syndrome register value from a
> fault status register value which is filled in by arm_tlb_fill().
> arm_tlb_fill() returns FSR values which might be in the format
> used with short-format page descriptors, or the format used
> with long-format (LPAE) descriptors. The syndrome register
> always uses LPAE-format FSR status codes.
>
> It isn't actually possible to end up delivering a syndrome
> register value to the guest for a fault which is reported
> with a short-format FSR (that kind of stage 1 fault will only
> happen for an AArch32 translation regime which doesn't have
> a syndrome register, and can never be redirected to an AArch64
> or Hyp exception level). Add an assertion which checks this,
> and adjust the code so that we construct a syndrome with
> an invalid status code, rather than allowing set bits in
> the FSR input to randomly corrupt other fields in the syndrome.
>
> Signed-off-by: Peter Maydell
Tricky but it looks correct as far as I can follow the specs...
Reviewed-by: Edgar E. Iglesias
> ---
> It took me a little while to convince myself that you'd
> never take a short-format FSR to a using-syndrome EL :-)
> ---
> target/arm/op_helper.c | 23 ++-
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index d64c867..156b825 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -130,7 +130,7 @@ void tlb_fill(CPUState *cs, target_ulong addr,
> MMUAccessType access_type,
> if (unlikely(ret)) {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> -uint32_t syn, exc;
> +uint32_t syn, exc, fsc;
> unsigned int target_el;
> bool same_el;
>
> @@ -145,19 +145,32 @@ void tlb_fill(CPUState *cs, target_ulong addr,
> MMUAccessType access_type,
> env->cp15.hpfar_el2 = extract64(fi.s2addr, 12, 47) << 4;
> }
> same_el = arm_current_el(env) == target_el;
> -/* AArch64 syndrome does not have an LPAE bit */
> -syn = fsr & ~(1 << 9);
> +
> +if (fsr & (1 << 9)) {
> +/* LPAE format fault status register : bottom 6 bits are
> + * status code in the same form as needed for syndrome
> + */
> +fsc = extract32(fsr, 0, 6);
> +} else {
> +/* Short format FSR : this fault will never actually be reported
> + * to an EL that uses a syndrome register. Check that here,
> + * and use a (currently) reserved FSR code in case the
> constructed
> + * syndrome does leak into the guest somehow.
> + */
> +assert(target_el != 2 && !arm_el_is_aa64(env, target_el));
> +fsc = 0x3f;
> +}
>
> /* For insn and data aborts we assume there is no instruction
> syndrome
> * information; this is always true for exceptions reported to EL1.
> */
> if (access_type == MMU_INST_FETCH) {
> -syn = syn_insn_abort(same_el, 0, fi.s1ptw, syn);
> +syn = syn_insn_abort(same_el, 0, fi.s1ptw, fsc);
> exc = EXCP_PREFETCH_ABORT;
> } else {
> syn = merge_syn_data_abort(env->exception.syndrome, target_el,
> same_el, fi.s1ptw,
> - access_type == MMU_DATA_STORE, syn);
> + access_type == MMU_DATA_STORE, fsc);
> if (access_type == MMU_DATA_STORE
> && arm_feature(env, ARM_FEATURE_V6)) {
> fsr |= (1 << 11);
> --
> 2.7.4
>
>
Re: [Qemu-devel] [PATCH] target/arm: Add assertion about FSC format for syndrome registers
On Thu, Apr 06, 2017 at 02:42:32PM +0100, Peter Maydell wrote:
> In tlb_fill() we construct a syndrome register value from a
> fault status register value which is filled in by arm_tlb_fill().
> arm_tlb_fill() returns FSR values which might be in the format
> used with short-format page descriptors, or the format used
> with long-format (LPAE) descriptors. The syndrome register
> always uses LPAE-format FSR status codes.
>
> It isn't actually possible to end up delivering a syndrome
> register value to the guest for a fault which is reported
> with a short-format FSR (that kind of stage 1 fault will only
> happen for an AArch32 translation regime which doesn't have
> a syndrome register, and can never be redirected to an AArch64
> or Hyp exception level). Add an assertion which checks this,
> and adjust the code so that we construct a syndrome with
> an invalid status code, rather than allowing set bits in
> the FSR input to randomly corrupt other fields in the syndrome.
BTW,
Have you seen something suspicous when running code or was
this a theoretical issue?
Cheers,
Edgar
>
> Signed-off-by: Peter Maydell
> ---
> It took me a little while to convince myself that you'd
> never take a short-format FSR to a using-syndrome EL :-)
> ---
> target/arm/op_helper.c | 23 ++-
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index d64c867..156b825 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -130,7 +130,7 @@ void tlb_fill(CPUState *cs, target_ulong addr,
> MMUAccessType access_type,
> if (unlikely(ret)) {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> -uint32_t syn, exc;
> +uint32_t syn, exc, fsc;
> unsigned int target_el;
> bool same_el;
>
> @@ -145,19 +145,32 @@ void tlb_fill(CPUState *cs, target_ulong addr,
> MMUAccessType access_type,
> env->cp15.hpfar_el2 = extract64(fi.s2addr, 12, 47) << 4;
> }
> same_el = arm_current_el(env) == target_el;
> -/* AArch64 syndrome does not have an LPAE bit */
> -syn = fsr & ~(1 << 9);
> +
> +if (fsr & (1 << 9)) {
> +/* LPAE format fault status register : bottom 6 bits are
> + * status code in the same form as needed for syndrome
> + */
> +fsc = extract32(fsr, 0, 6);
> +} else {
> +/* Short format FSR : this fault will never actually be reported
> + * to an EL that uses a syndrome register. Check that here,
> + * and use a (currently) reserved FSR code in case the
> constructed
> + * syndrome does leak into the guest somehow.
> + */
> +assert(target_el != 2 && !arm_el_is_aa64(env, target_el));
> +fsc = 0x3f;
> +}
>
> /* For insn and data aborts we assume there is no instruction
> syndrome
> * information; this is always true for exceptions reported to EL1.
> */
> if (access_type == MMU_INST_FETCH) {
> -syn = syn_insn_abort(same_el, 0, fi.s1ptw, syn);
> +syn = syn_insn_abort(same_el, 0, fi.s1ptw, fsc);
> exc = EXCP_PREFETCH_ABORT;
> } else {
> syn = merge_syn_data_abort(env->exception.syndrome, target_el,
> same_el, fi.s1ptw,
> - access_type == MMU_DATA_STORE, syn);
> + access_type == MMU_DATA_STORE, fsc);
> if (access_type == MMU_DATA_STORE
> && arm_feature(env, ARM_FEATURE_V6)) {
> fsr |= (1 << 11);
> --
> 2.7.4
>
>
Re: [Qemu-devel] [Qemu-arm] [PATCH] target/arm: Add missing entries to excnames[] for log strings
On Thu, Apr 06, 2017 at 02:45:40PM +0100, Peter Maydell wrote:
> Recent changes have added new EXCP_ values to ARM but forgot
> to update the excnames[] array which is used to provide
> human-readable strings when printing information about the
> exception for debug logging. Add the missing entries, and
> add a comment to the list of #defines to help avoid the mistake
> being repeated in future.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Edgar E. Iglesias
> ---
> target/arm/cpu.h | 1 +
> target/arm/internals.h | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index a8aabce..e6f05e2 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -58,6 +58,7 @@
> #define EXCP_SEMIHOST 16 /* semihosting call */
> #define EXCP_NOCP 17 /* v7M NOCP UsageFault */
> #define EXCP_INVSTATE 18 /* v7M INVSTATE UsageFault */
> +/* NB: new EXCP_ defines should be added to the excnames[] array too */
>
> #define ARMV7M_EXCP_RESET 1
> #define ARMV7M_EXCP_NMI 2
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index f742a41..97ca034 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -70,6 +70,8 @@ static const char * const excnames[] = {
> [EXCP_VIRQ] = "Virtual IRQ",
> [EXCP_VFIQ] = "Virtual FIQ",
> [EXCP_SEMIHOST] = "Semihosting call",
> +[EXCP_NOCP] = "v7M NOCP UsageFault",
> +[EXCP_INVSTATE] = "v7M INVSTATE UsageFault",
> };
>
> /* Scale factor for generic timers, ie number of ns per tick.
> --
> 2.7.4
>
>
Re: [Qemu-devel] [PATCH v2 4/6] block: Quiesce old aio context during bdrv_set_aio_context
On Mon, 04/10 10:06, Kevin Wolf wrote:
> Am 08.04.2017 um 05:43 hat Fam Zheng geschrieben:
> > On Fri, 04/07 13:50, Stefan Hajnoczi wrote:
> > > On Fri, Apr 07, 2017 at 02:54:12PM +0800, Fam Zheng wrote:
> > > > @@ -4413,6 +4416,10 @@ void bdrv_set_aio_context(BlockDriverState *bs,
> > > > AioContext *new_context)
> > > > aio_context_acquire(new_context);
> > > > bdrv_attach_aio_context(bs, new_context);
> > > > aio_context_release(new_context);
> > > > +if (bs->job) {
> > > > +block_job_resume(bs->job);
> > > > +}
> > >
> > > Should this be called before aio_context_release(new_context)?
> >
> > Yes, and I'm going to replace it with bdrv_parent_drained_begin() as Kevin
> > suggested.
>
> I think at the moment bdrv_parent_drained_begin() can't replace it yet,
> but you need both.
I think we have it already, see 600ac6a0e (blockjob: add devops to blockjob
backends):
bdrv_parent_drained_begin
-> blk_root_drained_begin
-> block_job_drained_begin
-> block_job_pause
Fam
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Enable MTTCG on PPC64
Hi Alex, >Have you declared the memory ordering for the guest? Nope didnt know was necessary i just add the standard -m 2047 >See ca759f9e387db87e1719911f019bc60c74be9ed8 for an example. watching it about Thanks Luigi
Re: [Qemu-devel] [Qemu-block] [PATCH RFC 04/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph
On Tue, Mar 21, 2017 at 11:16:23AM +0800, Fam Zheng wrote:
> @@ -1713,21 +1714,22 @@ void bdrv_format_default_perms(BlockDriverState *bs,
> BdrvChild *c,
> perm |= BLK_PERM_CONSISTENT_READ;
> shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
> } else {
> -/* We want consistent read from backing files if the parent needs it.
> +/* We want consistent read and aio context change from backing files
> if
> + * the parent needs it.
> * No other operations are performed on backing files. */
> -perm &= BLK_PERM_CONSISTENT_READ;
> +perm &= BLK_PERM_CONSISTENT_READ | BLK_PERM_AIO_CONTEXT_CHANGE;
>
> -/* If the parent can deal with changing data, we're okay with a
> +/* If the parent can deal with changing aio context, we're okay too;
> + * If the parent can deal with changing data, we're okay with a
> * writable and resizable backing file. */
> /* TODO Require !(perm & BLK_PERM_CONSISTENT_READ), too? */
> +shared &= BLK_PERM_AIO_CONTEXT_CHANGE | BLK_PERM_WRITE;
> if (shared & BLK_PERM_WRITE) {
> -shared = BLK_PERM_WRITE | BLK_PERM_RESIZE;
> -} else {
> -shared = 0;
> +shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
We already have BLK_PERM_WRITE so we're just adding BLK_PERM_RESIZE.
The following is clearer:
shared |= BLK_PERM_RESIZE;
> }
>
> shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD |
> - BLK_PERM_WRITE_UNCHANGED;
> + BLK_PERM_WRITE_UNCHANGED | BLK_PERM_AIO_CONTEXT_CHANGE;
Why was shared &= BLK_PERM_AIO_CONTEXT_CHANGE necessary above if we
unconditionally OR it here?
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH] target/arm: Add assertion about FSC format for syndrome registers
On 10 April 2017 at 09:44, Edgar E. Iglesias wrote: > Have you seen something suspicous when running code or was > this a theoretical issue? No, I was looking at the code for a different reason and was surprised that the syndrome-generation code wasn't doing anything to handle short-format FSRs. I started out thinking that this was just a corner case we'd forgotten since it only affects AArch32 guests (and even wrote some code to do short-to-long status code conversion), but after some thought I realised that it's not possible to get to a point where we would need the syndrome after such a fault. It's a tricky enough point that it seems worth having an assert and some comments to assure future readers that it isn't just something we forgot to handle... thanks -- PMM
[Qemu-devel] [PATCH for-2.10 03/19] crypto: cipher: introduce qcrypto_cipher_ctx_new for nettle-backend
Extracts qcrypto_cipher_ctx_new() from qcrypto_cipher_new() for
nettle-backend impls.
Signed-off-by: Longpeng(Mike)
---
crypto/cipher-nettle.c | 41 +
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index e04e3a1..e6d6e6c 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -262,12 +262,12 @@ static void nettle_cipher_free_ctx(QCryptoCipherNettle
*ctx)
}
-QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
- QCryptoCipherMode mode,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode,
+ const uint8_t *key,
+ size_t nkey,
+ Error **errp)
{
-QCryptoCipher *cipher;
QCryptoCipherNettle *ctx;
uint8_t *rfbkey;
@@ -287,12 +287,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
return NULL;
}
-cipher = g_new0(QCryptoCipher, 1);
-cipher->alg = alg;
-cipher->mode = mode;
-
ctx = g_new0(QCryptoCipherNettle, 1);
-cipher->opaque = ctx;
switch (alg) {
case QCRYPTO_CIPHER_ALG_DES_RFB:
@@ -436,10 +431,10 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
ctx->iv = g_new0(uint8_t, ctx->blocksize);
-return cipher;
+return ctx;
error:
-qcrypto_cipher_free(cipher);
+nettle_cipher_free_ctx(ctx);
return NULL;
}
@@ -561,3 +556,25 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
memcpy(ctx->iv, iv, niv);
return 0;
}
+
+
+QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
+{
+QCryptoCipher *cipher;
+QCryptoCipherNettle *ctx;
+
+ctx = qcrypto_cipher_ctx_new(alg, mode, key, nkey, errp);
+if (!ctx) {
+return NULL;
+}
+
+cipher = g_new0(QCryptoCipher, 1);
+cipher->alg = alg;
+cipher->mode = mode;
+cipher->opaque = ctx;
+
+return cipher;
+}
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 01/19] crypto: cipher: introduce context free function
Refactors the qcrypto_cipher_free(), splits it into two parts. One
is gcrypt/nettle__cipher_free_ctx() to free the special context.
This makes code more clear, what's more, it would be used by the
later patch.
Signed-off-by: Longpeng(Mike)
---
crypto/cipher-gcrypt.c | 31 ++-
crypto/cipher-nettle.c | 18 ++
2 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index 6487eca..0ecffa2 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -64,6 +64,22 @@ struct QCryptoCipherGcrypt {
uint8_t *iv;
};
+static void gcrypt_cipher_free_ctx(QCryptoCipherGcrypt *ctx,
+ QCryptoCipherMode mode)
+{
+if (!ctx) {
+return;
+}
+
+gcry_cipher_close(ctx->handle);
+if (mode == QCRYPTO_CIPHER_MODE_XTS) {
+gcry_cipher_close(ctx->tweakhandle);
+}
+g_free(ctx->iv);
+g_free(ctx);
+}
+
+
QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
QCryptoCipherMode mode,
const uint8_t *key, size_t nkey,
@@ -228,11 +244,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
return cipher;
error:
-gcry_cipher_close(ctx->handle);
-if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
-gcry_cipher_close(ctx->tweakhandle);
-}
-g_free(ctx);
+gcrypt_cipher_free_ctx(ctx, mode);
g_free(cipher);
return NULL;
}
@@ -240,17 +252,10 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
void qcrypto_cipher_free(QCryptoCipher *cipher)
{
-QCryptoCipherGcrypt *ctx;
if (!cipher) {
return;
}
-ctx = cipher->opaque;
-gcry_cipher_close(ctx->handle);
-if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
-gcry_cipher_close(ctx->tweakhandle);
-}
-g_free(ctx->iv);
-g_free(ctx);
+gcrypt_cipher_free_ctx(cipher->opaque, cipher->mode);
g_free(cipher);
}
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index dfc9030..e04e3a1 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -249,6 +249,19 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
}
+static void nettle_cipher_free_ctx(QCryptoCipherNettle *ctx)
+{
+if (!ctx) {
+return;
+}
+
+g_free(ctx->iv);
+g_free(ctx->ctx);
+g_free(ctx->ctx_tweak);
+g_free(ctx);
+}
+
+
QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
QCryptoCipherMode mode,
const uint8_t *key, size_t nkey,
@@ -440,10 +453,7 @@ void qcrypto_cipher_free(QCryptoCipher *cipher)
}
ctx = cipher->opaque;
-g_free(ctx->iv);
-g_free(ctx->ctx);
-g_free(ctx->ctx_tweak);
-g_free(ctx);
+nettle_cipher_free_ctx(ctx);
g_free(cipher);
}
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 02/19] crypto: cipher: introduce qcrypto_cipher_ctx_new for gcrypt-backend
Extracts qcrypto_cipher_ctx_new() from qcrypto_cipher_new() for
gcrypt-backend impls.
Signed-off-by: Longpeng(Mike)
---
crypto/cipher-gcrypt.c | 50 +-
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index 0ecffa2..871730b 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -80,12 +80,12 @@ static void gcrypt_cipher_free_ctx(QCryptoCipherGcrypt *ctx,
}
-QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
- QCryptoCipherMode mode,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode,
+ const uint8_t *key,
+ size_t nkey,
+ Error **errp)
{
-QCryptoCipher *cipher;
QCryptoCipherGcrypt *ctx;
gcry_error_t err;
int gcryalg, gcrymode;
@@ -162,10 +162,6 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
return NULL;
}
-cipher = g_new0(QCryptoCipher, 1);
-cipher->alg = alg;
-cipher->mode = mode;
-
ctx = g_new0(QCryptoCipherGcrypt, 1);
err = gcry_cipher_open(&ctx->handle, gcryalg, gcrymode, 0);
@@ -174,7 +170,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
gcry_strerror(err));
goto error;
}
-if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
+if (mode == QCRYPTO_CIPHER_MODE_XTS) {
err = gcry_cipher_open(&ctx->tweakhandle, gcryalg, gcrymode, 0);
if (err != 0) {
error_setg(errp, "Cannot initialize cipher: %s",
@@ -183,7 +179,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
}
}
-if (cipher->alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
+if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
/* We're using standard DES cipher from gcrypt, so we need
* to munge the key so that the results are the same as the
* bizarre RFB variant of DES :-)
@@ -193,7 +189,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
g_free(rfbkey);
ctx->blocksize = 8;
} else {
-if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
+if (mode == QCRYPTO_CIPHER_MODE_XTS) {
nkey /= 2;
err = gcry_cipher_setkey(ctx->handle, key, nkey);
if (err != 0) {
@@ -210,7 +206,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
gcry_strerror(err));
goto error;
}
-switch (cipher->alg) {
+switch (alg) {
case QCRYPTO_CIPHER_ALG_AES_128:
case QCRYPTO_CIPHER_ALG_AES_192:
case QCRYPTO_CIPHER_ALG_AES_256:
@@ -230,7 +226,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
}
}
-if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
+if (mode == QCRYPTO_CIPHER_MODE_XTS) {
if (ctx->blocksize != XTS_BLOCK_SIZE) {
error_setg(errp,
"Cipher block size %zu must equal XTS block size %d",
@@ -240,12 +236,10 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
ctx->iv = g_new0(uint8_t, ctx->blocksize);
}
-cipher->opaque = ctx;
-return cipher;
+return ctx;
error:
gcrypt_cipher_free_ctx(ctx, mode);
-g_free(cipher);
return NULL;
}
@@ -385,3 +379,25 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
return 0;
}
+
+
+QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
+{
+QCryptoCipher *cipher;
+QCryptoCipherGcrypt *ctx;
+
+ctx = qcrypto_cipher_ctx_new(alg, mode, key, nkey, errp);
+if (ctx == NULL) {
+return NULL;
+}
+
+cipher = g_new0(QCryptoCipher, 1);
+cipher->alg = alg;
+cipher->mode = mode;
+cipher->opaque = ctx;
+
+return cipher;
+}
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 06/19] crypto: hash: add hash driver framework
1) makes the public APIs in hash-nettle/gcrypt/glib static,
and rename them with "nettle/gcrypt/glib" prefix.
2) introduces hash framework, including QCryptoHashDriver
and new public APIs.
Signed-off-by: Longpeng(Mike)
---
crypto/hash-gcrypt.c | 17 +++--
crypto/hash-glib.c| 17 +++--
crypto/hash-nettle.c | 17 +++--
crypto/hash.c | 12
include/crypto/hash.h | 12
5 files changed, 57 insertions(+), 18 deletions(-)
diff --git a/crypto/hash-gcrypt.c b/crypto/hash-gcrypt.c
index 7690690..21c990f 100644
--- a/crypto/hash-gcrypt.c
+++ b/crypto/hash-gcrypt.c
@@ -44,12 +44,12 @@ gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
}
-int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
-const struct iovec *iov,
-size_t niov,
-uint8_t **result,
-size_t *resultlen,
-Error **errp)
+static int gcrypt_hash_bytesv(QCryptoHashAlgorithm alg,
+ const struct iovec *iov,
+ size_t niov,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
{
int i, ret;
gcry_md_hd_t md;
@@ -107,3 +107,8 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
gcry_md_close(md);
return -1;
}
+
+
+QCryptoHashDriver qcrypto_hash_lib_driver = {
+.hash_bytesv = gcrypt_hash_bytesv,
+};
diff --git a/crypto/hash-glib.c b/crypto/hash-glib.c
index ec99ac9..72be47e 100644
--- a/crypto/hash-glib.c
+++ b/crypto/hash-glib.c
@@ -47,12 +47,12 @@ gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
}
-int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
-const struct iovec *iov,
-size_t niov,
-uint8_t **result,
-size_t *resultlen,
-Error **errp)
+static int glib_hash_bytesv(QCryptoHashAlgorithm alg,
+const struct iovec *iov,
+size_t niov,
+uint8_t **result,
+size_t *resultlen,
+Error **errp)
{
int i, ret;
GChecksum *cs;
@@ -95,3 +95,8 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
g_checksum_free(cs);
return -1;
}
+
+
+QCryptoHashDriver qcrypto_hash_lib_driver = {
+.hash_bytesv = glib_hash_bytesv,
+};
diff --git a/crypto/hash-nettle.c b/crypto/hash-nettle.c
index 6a206dc..9233e10 100644
--- a/crypto/hash-nettle.c
+++ b/crypto/hash-nettle.c
@@ -103,12 +103,12 @@ gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
}
-int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
-const struct iovec *iov,
-size_t niov,
-uint8_t **result,
-size_t *resultlen,
-Error **errp)
+static int nettle_hash_bytesv(QCryptoHashAlgorithm alg,
+ const struct iovec *iov,
+ size_t niov,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
{
int i;
union qcrypto_hash_ctx ctx;
@@ -152,3 +152,8 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
return 0;
}
+
+
+QCryptoHashDriver qcrypto_hash_lib_driver = {
+.hash_bytesv = nettle_hash_bytesv,
+};
diff --git a/crypto/hash.c b/crypto/hash.c
index 0f1ceac..0b0d479 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -38,6 +38,18 @@ size_t qcrypto_hash_digest_len(QCryptoHashAlgorithm alg)
return qcrypto_hash_alg_size[alg];
}
+int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
+const struct iovec *iov,
+size_t niov,
+uint8_t **result,
+size_t *resultlen,
+Error **errp)
+{
+return qcrypto_hash_lib_driver.hash_bytesv(alg, iov, niov,
+ result, resultlen,
+ errp);
+}
+
int qcrypto_hash_bytes(QCryptoHashAlgorithm alg,
const char *buf,
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index ca3267f..00b764e 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -25,6 +25,18 @@
/* See also "QCryptoHashAlgorithm" defined in qapi/crypto.json */
+typedef struct QCryptoHashDriver QCryptoHashDriver;
+struct QCryptoHashDriver {
+int (*hash_bytesv)(QCryptoHashAlgorithm alg,
+ const struct iovec *iov,
+ size_t niov,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp);
+};
+
+extern QCryptoHashDriver qcrypto_hash_lib
[Qemu-devel] [PATCH for-2.10 04/19] crypto: cipher: introduce qcrypto_cipher_ctx_new for builtin-backend
Extracts qcrypto_cipher_ctx_new() from qcrypto_cipher_new() for
glib-backend impls.
Signed-off-by: Longpeng(Mike)
---
crypto/cipher-builtin.c | 101 ++--
1 file changed, 55 insertions(+), 46 deletions(-)
diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
index b4bc2b9..8cf47d1 100644
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -235,23 +235,24 @@ static int qcrypto_cipher_setiv_aes(QCryptoCipher *cipher,
-static int qcrypto_cipher_init_aes(QCryptoCipher *cipher,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoCipherBuiltin *
+qcrypto_cipher_init_aes(QCryptoCipherMode mode,
+const uint8_t *key, size_t nkey,
+Error **errp)
{
QCryptoCipherBuiltin *ctxt;
-if (cipher->mode != QCRYPTO_CIPHER_MODE_CBC &&
-cipher->mode != QCRYPTO_CIPHER_MODE_ECB &&
-cipher->mode != QCRYPTO_CIPHER_MODE_XTS) {
+if (mode != QCRYPTO_CIPHER_MODE_CBC &&
+mode != QCRYPTO_CIPHER_MODE_ECB &&
+mode != QCRYPTO_CIPHER_MODE_XTS) {
error_setg(errp, "Unsupported cipher mode %s",
- QCryptoCipherMode_lookup[cipher->mode]);
-return -1;
+ QCryptoCipherMode_lookup[mode]);
+return NULL;
}
ctxt = g_new0(QCryptoCipherBuiltin, 1);
-if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
+if (mode == QCRYPTO_CIPHER_MODE_XTS) {
if (AES_set_encrypt_key(key, nkey * 4, &ctxt->state.aes.key.enc) != 0)
{
error_setg(errp, "Failed to set encryption key");
goto error;
@@ -291,13 +292,11 @@ static int qcrypto_cipher_init_aes(QCryptoCipher *cipher,
ctxt->encrypt = qcrypto_cipher_encrypt_aes;
ctxt->decrypt = qcrypto_cipher_decrypt_aes;
-cipher->opaque = ctxt;
-
-return 0;
+return ctxt;
error:
g_free(ctxt);
-return -1;
+return NULL;
}
@@ -370,16 +369,17 @@ static int qcrypto_cipher_setiv_des_rfb(QCryptoCipher
*cipher,
}
-static int qcrypto_cipher_init_des_rfb(QCryptoCipher *cipher,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoCipherBuiltin *
+qcrypto_cipher_init_des_rfb(QCryptoCipherMode mode,
+const uint8_t *key, size_t nkey,
+Error **errp)
{
QCryptoCipherBuiltin *ctxt;
-if (cipher->mode != QCRYPTO_CIPHER_MODE_ECB) {
+if (mode != QCRYPTO_CIPHER_MODE_ECB) {
error_setg(errp, "Unsupported cipher mode %s",
- QCryptoCipherMode_lookup[cipher->mode]);
-return -1;
+ QCryptoCipherMode_lookup[mode]);
+return NULL;
}
ctxt = g_new0(QCryptoCipherBuiltin, 1);
@@ -394,9 +394,7 @@ static int qcrypto_cipher_init_des_rfb(QCryptoCipher
*cipher,
ctxt->encrypt = qcrypto_cipher_encrypt_des_rfb;
ctxt->decrypt = qcrypto_cipher_decrypt_des_rfb;
-cipher->opaque = ctxt;
-
-return 0;
+return ctxt;
}
@@ -426,12 +424,13 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
}
-QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
- QCryptoCipherMode mode,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoCipherBuiltin *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+QCryptoCipherMode mode,
+const uint8_t *key,
+size_t nkey,
+Error **errp)
{
-QCryptoCipher *cipher;
+QCryptoCipherBuiltin *ctxt;
switch (mode) {
case QCRYPTO_CIPHER_MODE_ECB:
@@ -444,39 +443,27 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
return NULL;
}
-cipher = g_new0(QCryptoCipher, 1);
-cipher->alg = alg;
-cipher->mode = mode;
-
if (!qcrypto_cipher_validate_key_length(alg, mode, nkey, errp)) {
-goto error;
+return NULL;
}
-switch (cipher->alg) {
+switch (alg) {
case QCRYPTO_CIPHER_ALG_DES_RFB:
-if (qcrypto_cipher_init_des_rfb(cipher, key, nkey, errp) < 0) {
-goto error;
-}
+ctxt = qcrypto_cipher_init_des_rfb(mode, key, nkey, errp);
break;
case QCRYPTO_CIPHER_ALG_AES_128:
case QCRYPTO_CIPHER_ALG_AES_192:
case QCRYPTO_CIPHER_ALG_AES_256:
-if (qcrypto_cipher_init_aes(cipher, key, nkey, errp) < 0) {
-goto error;
-}
+ctxt = qcrypto_cipher_init_aes(mode, key, nkey, errp);
break;
default:
error_setg(errp,
"Unsupported cipher algorithm %s",
- QCryptoCi
[Qemu-devel] [PATCH for-2.10 05/19] crypto: cipher: add cipher driver framework
1) makes the public APIs in cipher-nettle/gcrypt/builtin static,
and rename them with "nettle/gcrypt/builtin" prefix.
2) introduces cipher framework, including QCryptoCipherDriver
and new public APIs.
Signed-off-by: Longpeng(Mike)
---
crypto/cipher-builtin.c | 59 +
crypto/cipher-gcrypt.c | 58 +---
crypto/cipher-nettle.c | 59 +
crypto/cipher.c | 59 +
include/crypto/cipher.h | 22 ++
5 files changed, 141 insertions(+), 116 deletions(-)
diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
index 8cf47d1..a35f461 100644
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -466,25 +466,20 @@ static QCryptoCipherBuiltin
*qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
return ctxt;
}
-void qcrypto_cipher_free(QCryptoCipher *cipher)
+static void builtin_cipher_ctx_free(QCryptoCipher *cipher)
{
QCryptoCipherBuiltin *ctxt;
-if (!cipher) {
-return;
-}
-
ctxt = cipher->opaque;
ctxt->free(cipher);
-g_free(cipher);
}
-int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
- const void *in,
- void *out,
- size_t len,
- Error **errp)
+static int builtin_cipher_encrypt(QCryptoCipher *cipher,
+ const void *in,
+ void *out,
+ size_t len,
+ Error **errp)
{
QCryptoCipherBuiltin *ctxt = cipher->opaque;
@@ -498,11 +493,11 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
}
-int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
- const void *in,
- void *out,
- size_t len,
- Error **errp)
+static int builtin_cipher_decrypt(QCryptoCipher *cipher,
+ const void *in,
+ void *out,
+ size_t len,
+ Error **errp)
{
QCryptoCipherBuiltin *ctxt = cipher->opaque;
@@ -516,9 +511,9 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
}
-int qcrypto_cipher_setiv(QCryptoCipher *cipher,
- const uint8_t *iv, size_t niv,
- Error **errp)
+static int builtin_cipher_setiv(QCryptoCipher *cipher,
+const uint8_t *iv, size_t niv,
+Error **errp)
{
QCryptoCipherBuiltin *ctxt = cipher->opaque;
@@ -526,23 +521,9 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
}
-QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
- QCryptoCipherMode mode,
- const uint8_t *key, size_t nkey,
- Error **errp)
-{
-QCryptoCipher *cipher;
-QCryptoCipherBuiltin *ctxt;
-
-ctxt = qcrypto_cipher_ctx_new(alg, mode, key, nkey, errp);
-if (ctxt == NULL) {
-return NULL;
-}
-
-cipher = g_new0(QCryptoCipher, 1);
-cipher->alg = alg;
-cipher->mode = mode;
-cipher->opaque = ctxt;
-
-return cipher;
-}
+static struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
+.cipher_encrypt = builtin_cipher_encrypt,
+.cipher_decrypt = builtin_cipher_decrypt,
+.cipher_setiv = builtin_cipher_setiv,
+.cipher_free = builtin_cipher_ctx_free,
+};
diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index 871730b..36a0626 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -244,13 +244,9 @@ static QCryptoCipherGcrypt
*qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
}
-void qcrypto_cipher_free(QCryptoCipher *cipher)
+static void gcrypt_cipher_ctx_free(QCryptoCipher *cipher)
{
-if (!cipher) {
-return;
-}
gcrypt_cipher_free_ctx(cipher->opaque, cipher->mode);
-g_free(cipher);
}
@@ -274,11 +270,11 @@ static void qcrypto_gcrypt_xts_decrypt(const void *ctx,
g_assert(err == 0);
}
-int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
- const void *in,
- void *out,
- size_t len,
- Error **errp)
+static int gcrypt_cipher_encrypt(QCryptoCipher *cipher,
+ const void *in,
+ void *out,
+ size_t len,
+ Error **errp)
{
QCryptoCipherGcrypt *ctx = cipher->opaque;
gcry_error_t err;
@@ -309,11 +305,11 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
}
-int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
- const void *in,
[Qemu-devel] [PATCH for-2.10 00/19] crypto: add af_alg-backend support
The AF_ALG socket family is the userspace interface for linux crypto API, users can use it to access hardware accelerators. This patchset adds a afalg-backend for qemu crypto subsystem. Currently when performs encrypt/decrypt, we'll try afalg-backend first and will back to libiary-backend if it failed. In the next step, It would support a command parameter to specifies which backends prefer to and some other improvements. Longpeng(Mike) (19): crypto: cipher: introduce context free function crypto: cipher: introduce qcrypto_cipher_ctx_new for gcrypt-backend crypto: cipher: introduce qcrypto_cipher_ctx_new for nettle-backend crypto: cipher: introduce qcrypto_cipher_ctx_new for builtin-backend crypto: cipher: add cipher driver framework crypto: hash: add hash driver framework crypto: hmac: move crypto/hmac.h into include/crypto/ crypto: hmac: introduce qcrypto_hmac_ctx_new for gcrypt-backend crypto: hmac: introduce qcrypto_hmac_ctx_new for nettle-backend crypto: hmac: introduce qcrypto_hmac_ctx_new for glib-backend crypto: hmac: add hmac driver framework socket: add af_alg family support crypto: introduce some common functions for af_alg backend crypto: cipher: add af_alg cipher support tests: crypto: add cipher speed case crypto: hash: add af_alg hash support tests: crypto: add hash speed case crypto: hmac: add af_alg hmac support tests: crypto: add hmac speed case configure | 21 crypto/Makefile.objs| 3 + crypto/afalg-comm.c | 71 ++ crypto/cipher-afalg.c | 229 +++ crypto/cipher-builtin.c | 120 +++ crypto/cipher-gcrypt.c | 99 ++- crypto/cipher-nettle.c | 78 --- crypto/cipher.c | 87 + crypto/hash-afalg.c | 232 crypto/hash-gcrypt.c| 17 ++-- crypto/hash-glib.c | 17 ++-- crypto/hash-nettle.c| 17 ++-- crypto/hash.c | 22 + crypto/hmac-gcrypt.c| 39 crypto/hmac-glib.c | 58 +-- crypto/hmac-nettle.c| 39 +++- crypto/hmac.c | 65 + crypto/hmac.h | 166 --- include/crypto/afalg-comm.h | 74 ++ include/crypto/cipher.h | 29 ++ include/crypto/hash.h | 13 +++ include/crypto/hmac.h | 199 + include/qemu/sockets.h | 6 ++ qapi-schema.json| 21 +++- tests/test-crypto-cipher.c | 92 -- tests/test-crypto-hash.c| 56 ++- tests/test-crypto-hmac.c| 66 - util/qemu-sockets.c | 91 + 28 files changed, 1599 insertions(+), 428 deletions(-) create mode 100644 crypto/afalg-comm.c create mode 100644 crypto/cipher-afalg.c create mode 100644 crypto/hash-afalg.c delete mode 100644 crypto/hmac.h create mode 100644 include/crypto/afalg-comm.h create mode 100644 include/crypto/hmac.h -- 1.8.3.1
[Qemu-devel] [PATCH for-2.10 14/19] crypto: cipher: add af_alg cipher support
Adds afalg-backend cipher support: introduces some private APIs
firstly, and then intergrates them into qcrypto_cipher_afalg_driver.
Signed-off-by: Longpeng(Mike)
---
crypto/Makefile.objs| 1 +
crypto/cipher-afalg.c | 229
crypto/cipher.c | 30 +-
include/crypto/afalg-comm.h | 11 +++
include/crypto/cipher.h | 7 ++
tests/test-crypto-cipher.c | 10 +-
6 files changed, 286 insertions(+), 2 deletions(-)
create mode 100644 crypto/cipher-afalg.c
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 6f244a3..ad1229b 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -11,6 +11,7 @@ crypto-obj-y += aes.o
crypto-obj-y += desrfb.o
crypto-obj-y += cipher.o
crypto-obj-$(CONFIG_AF_ALG) += afalg-comm.o
+crypto-obj-$(CONFIG_AF_ALG) += cipher-afalg.o
crypto-obj-y += tlscreds.o
crypto-obj-y += tlscredsanon.o
crypto-obj-y += tlscredsx509.o
diff --git a/crypto/cipher-afalg.c b/crypto/cipher-afalg.c
new file mode 100644
index 000..2da972c
--- /dev/null
+++ b/crypto/cipher-afalg.c
@@ -0,0 +1,229 @@
+/*
+ * QEMU Crypto af_alg-backend cipher support
+ *
+ * Copyright (c) 2017 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *Longpeng(Mike)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/sockets.h"
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "crypto/cipher.h"
+#include "crypto/afalg-comm.h"
+#include
+
+static int afalg_cipher_format_name(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode,
+ AfalgSocketAddress *afalg)
+{
+const char *alg_name = NULL;
+const char *mode_name = NULL;
+
+switch (alg) {
+case QCRYPTO_CIPHER_ALG_AES_128:
+case QCRYPTO_CIPHER_ALG_AES_192:
+case QCRYPTO_CIPHER_ALG_AES_256:
+alg_name = "aes";
+break;
+case QCRYPTO_CIPHER_ALG_CAST5_128:
+alg_name = "cast5";
+break;
+case QCRYPTO_CIPHER_ALG_SERPENT_128:
+case QCRYPTO_CIPHER_ALG_SERPENT_192:
+case QCRYPTO_CIPHER_ALG_SERPENT_256:
+alg_name = "serpent";
+break;
+case QCRYPTO_CIPHER_ALG_TWOFISH_128:
+case QCRYPTO_CIPHER_ALG_TWOFISH_192:
+case QCRYPTO_CIPHER_ALG_TWOFISH_256:
+alg_name = "twofish";
+break;
+
+default:
+return -1;
+}
+
+mode_name = QCryptoCipherMode_lookup[mode];
+afalg->name = (char *)g_new0(int8_t, SALG_NAME_LEN_MAX);
+sprintf(afalg->name, "%s(%s)", mode_name, alg_name);
+
+return 0;
+}
+
+QCryptoAfalg *afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode,
+ const uint8_t *key,
+ size_t nkey, Error **errp)
+{
+SocketAddress *saddr = NULL;
+QCryptoAfalg *afalg = NULL;
+size_t except_niv = 0;
+int ret = 0;
+
+saddr = g_new0(SocketAddress, 1);
+saddr->u.afalg.data = g_new0(AfalgSocketAddress, 1);
+saddr->type = SOCKET_ADDRESS_KIND_AFALG;
+ret = afalg_cipher_format_name(alg, mode, saddr->u.afalg.data);
+if (ret != 0) {
+error_setg(errp, "Unsupported cipher mode %s",
+ QCryptoCipherMode_lookup[mode]);
+goto error;
+}
+afalg_comm_format_type(saddr->u.afalg.data, ALG_TYPE_CIPHER);
+
+afalg = afalg_comm_alloc(saddr);
+if (!afalg) {
+error_setg(errp, "Alloc QCryptoAfalg object failed");
+goto error;
+}
+
+/* setkey */
+ret = qemu_setsockopt(afalg->tfmfd, SOL_ALG, ALG_SET_KEY, key,
+ nkey);
+if (ret != 0) {
+error_setg(errp, "Afalg setkey failed");
+goto error;
+}
+
+/* prepare msg header */
+afalg->msg = g_new0(struct msghdr, 1);
+afalg->msg->msg_controllen += CMSG_SPACE(ALG_OPTYPE_LEN);
+except_niv = qcrypto_cipher_get_iv_len(alg, mode);
+if (except_niv) {
+afalg->msg->msg_controllen += CMSG_SPACE(ALG_MSGIV_LEN(except_niv));
+}
+afalg->msg->msg_control = g_new0(uint8_t, afalg->msg->msg_controllen);
+
+/* We use 1st msghdr for crypto-info and 2nd msghdr for IV-info */
+afalg->cmsg = CMSG_FIRSTHDR(afalg->msg);
+afalg->cmsg->cmsg_level = SOL_ALG;
+afalg->cmsg->cmsg_type = ALG_SET_OP;
+afalg->cmsg->cmsg_len = CMSG_SPACE(ALG_OPTYPE_LEN);
+
+cleanup:
+g_free(saddr->u.afalg.data->type);
+g_free(saddr->u.afalg.data->name);
+g_free(saddr->u.afalg.data);
+g_free(saddr);
+return afalg;
+
+error:
+afalg_comm_free(afalg);
+afalg = NULL;
+goto cleanup;
+}
+
+static int afalg_cipher_setiv(QCryptoCipher *cipher,
+ const uint8_t *iv,
+ size_t niv, Error **errp)
+{
+struct af_alg_iv *alg_iv = NULL;
+QCryptoAfalg *afalg = ciphe
[Qemu-devel] [PATCH for-2.10 08/19] crypto: hmac: introduce qcrypto_hmac_ctx_new for gcrypt-backend
1) Fix a handle-leak problem in qcrypto_hmac_new(), doesn't free
ctx->handle if gcry_mac_setkey fails.
2) Extracts qcrypto_hmac_ctx_new() from qcrypto_hmac_new() for
gcrypt-backend impls.
Signed-off-by: Longpeng(Mike)
---
crypto/hmac-gcrypt.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/crypto/hmac-gcrypt.c b/crypto/hmac-gcrypt.c
index 21189e6..42489f3 100644
--- a/crypto/hmac-gcrypt.c
+++ b/crypto/hmac-gcrypt.c
@@ -42,11 +42,11 @@ bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
return false;
}
-QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoHmacGcrypt *
+qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
{
-QCryptoHmac *hmac;
QCryptoHmacGcrypt *ctx;
gcry_error_t err;
@@ -56,9 +56,6 @@ QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
return NULL;
}
-hmac = g_new0(QCryptoHmac, 1);
-hmac->alg = alg;
-
ctx = g_new0(QCryptoHmacGcrypt, 1);
err = gcry_mac_open(&ctx->handle, qcrypto_hmac_alg_map[alg],
@@ -73,15 +70,14 @@ QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
if (err != 0) {
error_setg(errp, "Cannot set key: %s",
gcry_strerror(err));
+gcry_mac_close(ctx->handle);
goto error;
}
-hmac->opaque = ctx;
-return hmac;
+return ctx;
error:
g_free(ctx);
-g_free(hmac);
return NULL;
}
@@ -150,3 +146,22 @@ int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
return 0;
}
+
+QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
+{
+QCryptoHmac *hmac;
+QCryptoHmacGcrypt *ctx;
+
+ctx = qcrypto_hmac_ctx_new(alg, key, nkey, errp);
+if (ctx == NULL) {
+return NULL;
+}
+
+hmac = g_new0(QCryptoHmac, 1);
+hmac->alg = alg;
+hmac->opaque = ctx;
+
+return hmac;
+}
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 07/19] crypto: hmac: move crypto/hmac.h into include/crypto/
Moves crypto/hmac.h into include/crypto/, likes cipher.h and hash.h
Signed-off-by: Longpeng(Mike)
---
crypto/hmac.h | 166 --
include/crypto/hmac.h | 166 ++
2 files changed, 166 insertions(+), 166 deletions(-)
delete mode 100644 crypto/hmac.h
create mode 100644 include/crypto/hmac.h
diff --git a/crypto/hmac.h b/crypto/hmac.h
deleted file mode 100644
index 0d3acd7..000
--- a/crypto/hmac.h
+++ /dev/null
@@ -1,166 +0,0 @@
-/*
- * QEMU Crypto hmac algorithms
- *
- * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or
- * (at your option) any later version. See the COPYING file in the
- * top-level directory.
- *
- */
-
-#ifndef QCRYPTO_HMAC_H
-#define QCRYPTO_HMAC_H
-
-#include "qapi-types.h"
-
-typedef struct QCryptoHmac QCryptoHmac;
-struct QCryptoHmac {
-QCryptoHashAlgorithm alg;
-void *opaque;
-};
-
-/**
- * qcrypto_hmac_supports:
- * @alg: the hmac algorithm
- *
- * Determine if @alg hmac algorithm is supported by
- * the current configured build
- *
- * Returns:
- * true if the algorithm is supported, false otherwise
- */
-bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg);
-
-/**
- * qcrypto_hmac_new:
- * @alg: the hmac algorithm
- * @key: the key bytes
- * @nkey: the length of @key
- * @errp: pointer to a NULL-initialized error object
- *
- * Creates a new hmac object with the algorithm @alg
- *
- * The @key parameter provides the bytes representing
- * the secret key to use. The @nkey parameter specifies
- * the length of @key in bytes
- *
- * Note: must use qcrypto_hmac_free() to release the
- * returned hmac object when no longer required
- *
- * Returns:
- * a new hmac object, or NULL on error
- */
-QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp);
-
-/**
- * qcrypto_hmac_free:
- * @hmac: the hmac object
- *
- * Release the memory associated with @hmac that was
- * previously allocated by qcrypto_hmac_new()
- */
-void qcrypto_hmac_free(QCryptoHmac *hmac);
-
-/**
- * qcrypto_hmac_bytesv:
- * @hmac: the hmac object
- * @iov: the array of memory regions to hmac
- * @niov: the length of @iov
- * @result: pointer to hold output hmac
- * @resultlen: pointer to hold length of @result
- * @errp: pointer to a NULL-initialized error object
- *
- * Computes the hmac across all the memory regions
- * present in @iov. The @result pointer will be
- * filled with raw bytes representing the computed
- * hmac, which will have length @resultlen. The
- * memory pointer in @result must be released
- * with a call to g_free() when no longer required.
- *
- * Returns:
- * 0 on success, -1 on error
- */
-int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
-const struct iovec *iov,
-size_t niov,
-uint8_t **result,
-size_t *resultlen,
-Error **errp);
-
-/**
- * qcrypto_hmac_bytes:
- * @hmac: the hmac object
- * @buf: the memory region to hmac
- * @len: the length of @buf
- * @result: pointer to hold output hmac
- * @resultlen: pointer to hold length of @result
- * @errp: pointer to a NULL-initialized error object
- *
- * Computes the hmac across all the memory region
- * @buf of length @len. The @result pointer will be
- * filled with raw bytes representing the computed
- * hmac, which will have length @resultlen. The
- * memory pointer in @result must be released
- * with a call to g_free() when no longer required.
- *
- * Returns:
- * 0 on success, -1 on error
- */
-int qcrypto_hmac_bytes(QCryptoHmac *hmac,
- const char *buf,
- size_t len,
- uint8_t **result,
- size_t *resultlen,
- Error **errp);
-
-/**
- * qcrypto_hmac_digestv:
- * @hmac: the hmac object
- * @iov: the array of memory regions to hmac
- * @niov: the length of @iov
- * @digest: pointer to hold output hmac
- * @errp: pointer to a NULL-initialized error object
- *
- * Computes the hmac across all the memory regions
- * present in @iov. The @digest pointer will be
- * filled with the printable hex digest of the computed
- * hmac, which will be terminated by '\0'. The
- * memory pointer in @digest must be released
- * with a call to g_free() when no longer required.
- *
- * Returns:
- * 0 on success, -1 on error
- */
-int qcrypto_hmac_digestv(QCryptoHmac *hmac,
- const struct iovec *iov,
- size_t niov,
- char **digest,
- Error **errp);
-
-/**
- * qcrypto_hmac_digest:
- * @hmac: the hmac object
- * @buf: the memory region to hmac
- * @len: the length of @buf
- * @digest: pointer to hold output hmac
- * @errp: pointer to a NULL-initialize
[Qemu-devel] [PATCH for-2.10 16/19] crypto: hash: add af_alg hash support
Adds afalg-backend hash support: introduces some private APIs
firstly, and then intergrates them into qcrypto_hash_afalg_driver.
Signed-off-by: Longpeng(Mike)
---
crypto/Makefile.objs| 1 +
crypto/hash-afalg.c | 150
crypto/hash.c | 16 -
include/crypto/afalg-comm.h | 1 +
include/crypto/hash.h | 1 +
5 files changed, 166 insertions(+), 3 deletions(-)
create mode 100644 crypto/hash-afalg.c
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index ad1229b..d8e8945 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -12,6 +12,7 @@ crypto-obj-y += desrfb.o
crypto-obj-y += cipher.o
crypto-obj-$(CONFIG_AF_ALG) += afalg-comm.o
crypto-obj-$(CONFIG_AF_ALG) += cipher-afalg.o
+crypto-obj-$(CONFIG_AF_ALG) += hash-afalg.o
crypto-obj-y += tlscreds.o
crypto-obj-y += tlscredsanon.o
crypto-obj-y += tlscredsx509.o
diff --git a/crypto/hash-afalg.c b/crypto/hash-afalg.c
new file mode 100644
index 000..d96d7568
--- /dev/null
+++ b/crypto/hash-afalg.c
@@ -0,0 +1,150 @@
+/*
+ * QEMU Crypto af_alg-backend hash support
+ *
+ * Copyright (c) 2017 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *Longpeng(Mike)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "qemu/sockets.h"
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "crypto/hash.h"
+#include "crypto/afalg-comm.h"
+#include
+
+static int afalg_hash_format_name(QCryptoHashAlgorithm alg,
+ AfalgSocketAddress *afalg)
+{
+const char *alg_name = NULL;
+
+switch (alg) {
+case QCRYPTO_HASH_ALG_MD5:
+alg_name = "md5";
+break;
+case QCRYPTO_HASH_ALG_SHA1:
+alg_name = "sha1";
+break;
+case QCRYPTO_HASH_ALG_SHA224:
+alg_name = "sha224";
+break;
+case QCRYPTO_HASH_ALG_SHA256:
+alg_name = "sha256";
+break;
+case QCRYPTO_HASH_ALG_SHA384:
+alg_name = "sha384";
+break;
+case QCRYPTO_HASH_ALG_SHA512:
+alg_name = "sha512";
+break;
+case QCRYPTO_HASH_ALG_RIPEMD160:
+alg_name = "rmd160";
+break;
+
+default:
+return -1;
+}
+
+afalg->name = (char *)g_new0(int8_t, SALG_NAME_LEN_MAX);
+sprintf(afalg->name, "%s", alg_name);
+
+return 0;
+}
+
+static QCryptoAfalg *afalg_hash_ctx_new(QCryptoHashAlgorithm alg)
+{
+SocketAddress *saddr = NULL;
+QCryptoAfalg *afalg = NULL;
+int ret = 0;
+
+saddr = g_new0(SocketAddress, 1);
+saddr->u.afalg.data = g_new0(AfalgSocketAddress, 1);
+saddr->type = SOCKET_ADDRESS_KIND_AFALG;
+ret = afalg_hash_format_name(alg, saddr->u.afalg.data);
+if (ret != 0) {
+goto error;
+}
+afalg_comm_format_type(saddr->u.afalg.data, ALG_TYPE_HASH);
+
+afalg = afalg_comm_alloc(saddr);
+if (!afalg) {
+goto error;
+}
+
+/* prepare msg header */
+afalg->msg = g_new0(struct msghdr, 1);
+
+cleanup:
+g_free(saddr->u.afalg.data->type);
+g_free(saddr->u.afalg.data->name);
+g_free(saddr->u.afalg.data);
+g_free(saddr);
+return afalg;
+
+error:
+afalg_comm_free(afalg);
+afalg = NULL;
+goto cleanup;
+}
+
+static int afalg_hash_bytesv(QCryptoHashAlgorithm alg,
+ const struct iovec *iov,
+ size_t niov, uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
+{
+QCryptoAfalg *afalg = NULL;
+struct iovec outv;
+int ret = 0;
+const int except_len = qcrypto_hash_digest_len(alg);
+
+if (*resultlen == 0) {
+*resultlen = except_len;
+*result = g_new0(uint8_t, *resultlen);
+} else if (*resultlen != except_len) {
+error_setg(errp,
+ "Result buffer size %zu is not match hash %d",
+ *resultlen, except_len);
+return -1;
+}
+
+afalg = afalg_hash_ctx_new(alg);
+if (afalg == NULL) {
+error_setg(errp, "Alloc QCryptoAfalg object failed");
+return -1;
+}
+
+/* send data to kernel's crypto core */
+ret = iov_send_recv(afalg->opfd, iov, niov,
+0, iov_size(iov, niov), true);
+if (ret < 0) {
+error_setg(errp, "Send data to afalg-core failed");
+goto out;
+}
+
+/* hash && get result */
+outv.iov_base = *result;
+outv.iov_len = *resultlen;
+afalg->msg->msg_iov = &outv;
+afalg->msg->msg_iovlen = 1;
+ret = recvmsg(afalg->opfd, afalg->msg, 0);
+if (ret != -1) {
+ret = 0;
+} else {
+error_setg(errp, "Recv result from afalg-core failed");
+}
+
+out:
+afalg_comm_free(afalg);
+return ret;
+}
+
+QCryptoHashDriver qcrypto_hash_afalg_driver = {
+.hash_bytesv
[Qemu-devel] [PATCH for-2.10 11/19] crypto: hmac: add hmac driver framework
1) makes the public APIs in hmac-nettle/gcrypt/glib static,
and rename them with "nettle/gcrypt/glib" prefix.
2) introduces hmac framework, including QCryptoHmacDriver
and new public APIs.
Signed-off-by: Longpeng(Mike)
---
crypto/hmac-gcrypt.c | 48 +++
crypto/hmac-glib.c| 70 ++-
crypto/hmac-nettle.c | 49 +++-
crypto/hmac.c | 39
include/crypto/hmac.h | 20 +++
5 files changed, 112 insertions(+), 114 deletions(-)
diff --git a/crypto/hmac-gcrypt.c b/crypto/hmac-gcrypt.c
index 42489f3..4181b2e 100644
--- a/crypto/hmac-gcrypt.c
+++ b/crypto/hmac-gcrypt.c
@@ -42,10 +42,9 @@ bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
return false;
}
-static QCryptoHmacGcrypt *
-qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp)
+void *qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
{
QCryptoHmacGcrypt *ctx;
gcry_error_t err;
@@ -81,27 +80,22 @@ error:
return NULL;
}
-void qcrypto_hmac_free(QCryptoHmac *hmac)
+static void gcrypt_hmac_ctx_free(QCryptoHmac *hmac)
{
QCryptoHmacGcrypt *ctx;
-if (!hmac) {
-return;
-}
-
ctx = hmac->opaque;
gcry_mac_close(ctx->handle);
g_free(ctx);
-g_free(hmac);
}
-int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
-const struct iovec *iov,
-size_t niov,
-uint8_t **result,
-size_t *resultlen,
-Error **errp)
+static int gcrypt_hmac_bytesv(QCryptoHmac *hmac,
+ const struct iovec *iov,
+ size_t niov,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
{
QCryptoHmacGcrypt *ctx;
gcry_error_t err;
@@ -147,21 +141,7 @@ int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
return 0;
}
-QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp)
-{
-QCryptoHmac *hmac;
-QCryptoHmacGcrypt *ctx;
-
-ctx = qcrypto_hmac_ctx_new(alg, key, nkey, errp);
-if (ctx == NULL) {
-return NULL;
-}
-
-hmac = g_new0(QCryptoHmac, 1);
-hmac->alg = alg;
-hmac->opaque = ctx;
-
-return hmac;
-}
+QCryptoHmacDriver qcrypto_hmac_lib_driver = {
+.hmac_bytesv = gcrypt_hmac_bytesv,
+.hmac_free = gcrypt_hmac_ctx_free,
+};
diff --git a/crypto/hmac-glib.c b/crypto/hmac-glib.c
index d9f88d8..b224be3 100644
--- a/crypto/hmac-glib.c
+++ b/crypto/hmac-glib.c
@@ -49,10 +49,9 @@ bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
return false;
}
-static QCryptoHmacGlib *
-qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp)
+void *qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
{
QCryptoHmacGlib *ctx;
@@ -78,27 +77,22 @@ error:
return NULL;
}
-void qcrypto_hmac_free(QCryptoHmac *hmac)
+static void glib_hmac_ctx_free(QCryptoHmac *hmac)
{
QCryptoHmacGlib *ctx;
-if (!hmac) {
-return;
-}
-
ctx = hmac->opaque;
g_hmac_unref(ctx->ghmac);
g_free(ctx);
-g_free(hmac);
}
-int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
-const struct iovec *iov,
-size_t niov,
-uint8_t **result,
-size_t *resultlen,
-Error **errp)
+static int glib_hmac_bytesv(QCryptoHmac *hmac,
+const struct iovec *iov,
+size_t niov,
+uint8_t **result,
+size_t *resultlen,
+Error **errp)
{
QCryptoHmacGlib *ctx;
int i, ret;
@@ -129,25 +123,6 @@ int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
return 0;
}
-QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp)
-{
-QCryptoHmac *hmac;
-QCryptoHmacGlib *ctx;
-
-ctx = qcrypto_hmac_ctx_new(alg, key, nkey, errp);
-if (ctx == NULL) {
-return NULL;
-}
-
-hmac = g_new0(QCryptoHmac, 1);
-hmac->alg = alg;
-hmac->opaque = ctx;
-
-return hmac;
-}
-
#else
bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
@@ -155,26 +130,31 @@ bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
return false;
}
-QCryptoHmac *qcrypto
[Qemu-devel] [PATCH for-2.10 09/19] crypto: hmac: introduce qcrypto_hmac_ctx_new for nettle-backend
Extracts qcrypto_hmac_ctx_new() from qcrypto_hmac_new() for
nettle-backend impls.
Signed-off-by: Longpeng(Mike)
---
crypto/hmac-nettle.c | 34 --
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/crypto/hmac-nettle.c b/crypto/hmac-nettle.c
index 4a9e6b2..19fbb4f 100644
--- a/crypto/hmac-nettle.c
+++ b/crypto/hmac-nettle.c
@@ -97,11 +97,11 @@ bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
return false;
}
-QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoHmacNettle *
+qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
{
-QCryptoHmac *hmac;
QCryptoHmacNettle *ctx;
if (!qcrypto_hmac_supports(alg)) {
@@ -110,16 +110,11 @@ QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
return NULL;
}
-hmac = g_new0(QCryptoHmac, 1);
-hmac->alg = alg;
-
ctx = g_new0(QCryptoHmacNettle, 1);
qcrypto_hmac_alg_map[alg].setkey(&ctx->u, nkey, key);
-hmac->opaque = ctx;
-
-return hmac;
+return ctx;
}
void qcrypto_hmac_free(QCryptoHmac *hmac)
@@ -173,3 +168,22 @@ int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
return 0;
}
+
+QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
+{
+QCryptoHmac *hmac;
+QCryptoHmacNettle *ctx;
+
+ctx = qcrypto_hmac_ctx_new(alg, key, nkey, errp);
+if (ctx == NULL) {
+return NULL;
+}
+
+hmac = g_new0(QCryptoHmac, 1);
+hmac->alg = alg;
+hmac->opaque = ctx;
+
+return hmac;
+}
--
1.8.3.1
Re: [Qemu-devel] [Qemu-block] [PATCH RFC 00/16] block: Protect AIO context change with perm API
On Tue, Mar 21, 2017 at 11:16:19AM +0800, Fam Zheng wrote: > Eject / change of scsi-cd on a virtio-scsi dataplane bus causes abort() > because > the new BDS doesn't get proper bdrv_set_aio_context(). > > Store the AioContext in BB and do it in blk_insert_bs. That is done by > Vladimir's patch. > > Other patches are to make sure such a bdrv_set_aio_context() doesn't interfere > with other BBs using other nodes from this graph. > > RFC note: > > Unfortunately, a use-after-free crash in iotests 030 appears since patch 7, > which I believe is a latent bug that bdrv_reopen is "reentered" in existing > code, rather than from this series: > > > #4 0x561ab90425a7 in bdrv_reopen > > #5 0x561ab8e1d28e in stream_complete > > #6 0x561ab9048543 in block_job_defer_to_main_loop_bh > > #7 0x561ab91305bc in aio_bh_call > > #8 0x561ab9130659 in aio_bh_poll > > #9 0x561ab9135656 in aio_poll > > #10 0x561ab90a6cf5 in bdrv_flush > > #11 0x561ab904285a in bdrv_reopen_prepare > > #12 0x561ab90423f0 in bdrv_reopen_multiple > > #13 0x561ab90425ef in bdrv_reopen > > #14 0x561ab909fa49 in commit_active_start > > #15 0x561ab8dd6ffb in qmp_block_commit > > #16 0x561ab8ded485 in qmp_marshal_block_commit > > #17 0x561ab9123e6c in do_qmp_dispatch > > #18 0x561ab9123fa4 in qmp_dispatch > > #19 0x561ab8ca26b7 in handle_qmp_command > > I have a fix that I'll post separately. > > The last patches are an alternative to patch 7, to "workaround" this in a > really non-obvious way. Are there qemu-iotests that cover both success and failure scenarios for the new permission? Stefan signature.asc Description: PGP signature
[Qemu-devel] [PATCH for-2.10 10/19] crypto: hmac: introduce qcrypto_hmac_ctx_new for glib-backend
Extracts qcrypto_hmac_ctx_new() from qcrypto_hmac_new() for
glib-backend impls.
Signed-off-by: Longpeng(Mike)
---
crypto/hmac-glib.c | 34 --
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/crypto/hmac-glib.c b/crypto/hmac-glib.c
index 08a1fdd..d9f88d8 100644
--- a/crypto/hmac-glib.c
+++ b/crypto/hmac-glib.c
@@ -49,11 +49,11 @@ bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
return false;
}
-QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
- const uint8_t *key, size_t nkey,
- Error **errp)
+static QCryptoHmacGlib *
+qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
{
-QCryptoHmac *hmac;
QCryptoHmacGlib *ctx;
if (!qcrypto_hmac_supports(alg)) {
@@ -62,9 +62,6 @@ QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
return NULL;
}
-hmac = g_new0(QCryptoHmac, 1);
-hmac->alg = alg;
-
ctx = g_new0(QCryptoHmacGlib, 1);
ctx->ghmac = g_hmac_new(qcrypto_hmac_alg_map[alg],
@@ -74,12 +71,10 @@ QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
goto error;
}
-hmac->opaque = ctx;
-return hmac;
+return ctx;
error:
g_free(ctx);
-g_free(hmac);
return NULL;
}
@@ -134,6 +129,25 @@ int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
return 0;
}
+QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
+{
+QCryptoHmac *hmac;
+QCryptoHmacGlib *ctx;
+
+ctx = qcrypto_hmac_ctx_new(alg, key, nkey, errp);
+if (ctx == NULL) {
+return NULL;
+}
+
+hmac = g_new0(QCryptoHmac, 1);
+hmac->alg = alg;
+hmac->opaque = ctx;
+
+return hmac;
+}
+
#else
bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 12/19] socket: add af_alg family support
The AF_ALG socket family is the userspace interface for linux
crypto API, this patch adds af_alg family support. It'll be used
by afalg-backend crypto later.
Signed-off-by: Longpeng(Mike)
---
configure | 21
include/qemu/sockets.h | 6
qapi-schema.json | 21 +++-
util/qemu-sockets.c| 91 ++
4 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 4b3b5cd..970c9bc 100755
--- a/configure
+++ b/configure
@@ -4737,6 +4737,23 @@ if compile_prog "" "" ; then
have_af_vsock=yes
fi
+##
+# check for usable AF_ALG environment
+hava_af_alg=no
+cat > $TMPC << EOF
+#include
+#include
+#include
+int main(void) {
+int sock;
+sock = socket(AF_ALG, SOCK_SEQPACKET, 0);
+return sock;
+}
+EOF
+if compile_prog "" "" ; then
+have_af_alg=yes
+fi
+
#
# Sparc implicitly links with --relax, which is
# incompatible with -r, so --no-relax should be
@@ -5767,6 +5784,10 @@ if test "$have_af_vsock" = "yes" ; then
echo "CONFIG_AF_VSOCK=y" >> $config_host_mak
fi
+if test "$have_af_alg" = "yes" ; then
+ echo "CONFIG_AF_ALG=y" >> $config_host_mak
+fi
+
if test "$have_sysmacros" = "yes" ; then
echo "CONFIG_SYSMACROS=y" >> $config_host_mak
fi
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 7842f6d..0a4a003 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -51,6 +51,12 @@ int socket_listen(SocketAddress *addr, Error **errp);
void socket_listen_cleanup(int fd, Error **errp);
int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
+#ifdef CONFIG_AF_ALG
+#define SALG_TYPE_LEN_MAX 14
+#define SALG_NAME_LEN_MAX 64
+int socket_bind(SocketAddress *addr, Error **errp);
+#endif
+
/* Old, ipv4 only bits. Don't use for new code. */
int parse_host_port(struct sockaddr_in *saddr, const char *str);
int socket_init(void);
diff --git a/qapi-schema.json b/qapi-schema.json
index 250e4dc..0cb06d3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1516,12 +1516,14 @@
#
# @vsock: vsock family (since 2.8)
#
+# @afalg: af_alg family (since 2.10)
+#
# @unknown: otherwise
#
# Since: 2.1
##
{ 'enum': 'NetworkAddressFamily',
- 'data': [ 'ipv4', 'ipv6', 'unix', 'vsock', 'unknown' ] }
+ 'data': [ 'ipv4', 'ipv6', 'unix', 'vsock', 'afalg', 'unknown' ] }
##
# @VncBasicInfo:
@@ -4119,6 +4121,22 @@
'port': 'str' } }
##
+# @AfalgSocketAddress:
+#
+# Captures a socket address in the af_alg namespace.
+#
+# @type: type of the crypto algogrithms
+#
+# @name: name of the crypto algogrithms
+#
+# Since: 2.10
+##
+{ 'struct': 'AfalgSocketAddress',
+ 'data': {
+'type': 'str',
+'name': 'str' }}
+
+##
# @SocketAddress:
#
# Captures the address of a socket, which could also be a named file descriptor
@@ -4130,6 +4148,7 @@
'inet': 'InetSocketAddress',
'unix': 'UnixSocketAddress',
'vsock': 'VsockSocketAddress',
+'afalg': 'AfalgSocketAddress',
'fd': 'String' } }
##
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 21442c3..258e419 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1151,6 +1151,97 @@ void socket_listen_cleanup(int fd, Error **errp)
qapi_free_SocketAddress(addr);
}
+#ifdef CONFIG_AF_ALG
+
+#include
+
+static bool afalg_parse_bind_saddr(const AfalgSocketAddress *saddr,
+ struct sockaddr_alg *alg,
+ Error **errp)
+{
+memset(alg, 0, sizeof(*alg));
+alg->salg_family = AF_ALG;
+
+if (qemu_strnlen(saddr->type, SALG_TYPE_LEN_MAX) == SALG_TYPE_LEN_MAX) {
+error_setg(errp, "Afalg type(%s) is larger than 14 bytes",
+ saddr->type);
+return false;
+}
+
+if (qemu_strnlen(saddr->name, SALG_NAME_LEN_MAX) == SALG_NAME_LEN_MAX) {
+error_setg(errp, "Afalg name(%s) is larger than 64 bytes",
+ saddr->name);
+return false;
+}
+
+pstrcpy((char *)alg->salg_type, SALG_TYPE_LEN_MAX, saddr->type);
+pstrcpy((char *)alg->salg_name, SALG_NAME_LEN_MAX, saddr->name);
+
+return true;
+}
+
+static int afalg_bind_saddr(const AfalgSocketAddress *saddr,
+Error **errp)
+{
+struct sockaddr_alg alg;
+int sbind;
+
+if (!afalg_parse_bind_saddr(saddr, &alg, errp)) {
+return -1;
+}
+
+sbind = qemu_socket(AF_ALG, SOCK_SEQPACKET, 0);
+if (sbind < 0) {
+error_setg_errno(errp, errno, "Failed to create socket");
+return -1;
+}
+
+if (bind(sbind, (const struct sockaddr *)&alg, sizeof(alg)) != 0) {
+error_setg_errno(errp, errno, "Failed to bind socket");
+closesocket(sbind);
+return -1;
+}
+
+return sbind;
+}
+
+/*
+ * Due to af_alg family doesn't support listen(), so we should
+ * use socket_bind() instead of socket_lis
Re: [Qemu-devel] [PATCH v2 4/6] block: Quiesce old aio context during bdrv_set_aio_context
Am 10.04.2017 um 10:45 hat Fam Zheng geschrieben:
> On Mon, 04/10 10:06, Kevin Wolf wrote:
> > Am 08.04.2017 um 05:43 hat Fam Zheng geschrieben:
> > > On Fri, 04/07 13:50, Stefan Hajnoczi wrote:
> > > > On Fri, Apr 07, 2017 at 02:54:12PM +0800, Fam Zheng wrote:
> > > > > @@ -4413,6 +4416,10 @@ void bdrv_set_aio_context(BlockDriverState
> > > > > *bs, AioContext *new_context)
> > > > > aio_context_acquire(new_context);
> > > > > bdrv_attach_aio_context(bs, new_context);
> > > > > aio_context_release(new_context);
> > > > > +if (bs->job) {
> > > > > +block_job_resume(bs->job);
> > > > > +}
> > > >
> > > > Should this be called before aio_context_release(new_context)?
> > >
> > > Yes, and I'm going to replace it with bdrv_parent_drained_begin() as Kevin
> > > suggested.
> >
> > I think at the moment bdrv_parent_drained_begin() can't replace it yet,
> > but you need both.
>
> I think we have it already, see 600ac6a0e (blockjob: add devops to blockjob
> backends):
>
> bdrv_parent_drained_begin
> -> blk_root_drained_begin
> -> block_job_drained_begin
>-> block_job_pause
Ah, yes, you're right. Somehow I thought this was only for 2.10.
Kevin
[Qemu-devel] [PATCH for-2.10 15/19] tests: crypto: add cipher speed case
Now we have afalg-backend and libiary-backend, it's necessary
to add the speed test in test-crypto-cipher.
We can use "./tests/test-crypto-cipher speed" to do the speed
test.
Signed-off-by: Longpeng(Mike)
---
tests/test-crypto-cipher.c | 82 +-
1 file changed, 74 insertions(+), 8 deletions(-)
diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index 8bb3308..ee0c2ca 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -789,6 +789,64 @@ static void test_cipher_short_plaintext(void)
qcrypto_cipher_free(cipher);
}
+static void test_cipher_speed(const void *opaque)
+{
+QCryptoCipher *cipher;
+Error *err = NULL;
+double total = 0.0;
+size_t chunk_size = (size_t)opaque;
+uint8_t *key = NULL, *iv = NULL;
+uint8_t *plaintext = NULL, *ciphertext = NULL;
+size_t nkey = qcrypto_cipher_get_key_len(QCRYPTO_CIPHER_ALG_AES_128);
+size_t niv = qcrypto_cipher_get_iv_len(QCRYPTO_CIPHER_ALG_AES_128,
+ QCRYPTO_CIPHER_MODE_CBC);
+
+key = g_new0(uint8_t, nkey);
+memset(key, g_test_rand_int(), nkey);
+
+iv = g_new0(uint8_t, niv);
+memset(iv, g_test_rand_int(), niv);
+
+ciphertext = g_new0(uint8_t, chunk_size);
+
+plaintext = g_new0(uint8_t, chunk_size);
+memset(plaintext, g_test_rand_int(), chunk_size);
+
+cipher = qcrypto_cipher_new(QCRYPTO_CIPHER_ALG_AES_128,
+QCRYPTO_CIPHER_MODE_CBC,
+key, nkey, &err);
+g_assert(cipher != NULL);
+
+g_assert(qcrypto_cipher_setiv(cipher,
+ iv, niv,
+ &err) == 0);
+
+g_test_timer_start();
+do {
+g_assert(qcrypto_cipher_encrypt(cipher,
+plaintext,
+ciphertext,
+chunk_size,
+&err) == 0);
+total += chunk_size;
+} while (g_test_timer_elapsed() < 5.0);
+
+total /= 1024 * 1024; /* to MB */
+
+g_print("[drv:%s]", qcrypto_cipher_using_afalg_drv(cipher) ?
+"afalg" : "libs");
+g_print("Testing cbc(aes128): ");
+g_print("Encrypting in chunks of %ld bytes: ", chunk_size);
+g_print("done. %.2f MB in %.2f secs: ", total, g_test_timer_last());
+g_print("%.2f MB/sec\t", total / g_test_timer_last());
+
+qcrypto_cipher_free(cipher);
+g_free(plaintext);
+g_free(ciphertext);
+g_free(iv);
+g_free(key);
+}
+
int main(int argc, char **argv)
{
size_t i;
@@ -797,17 +855,25 @@ int main(int argc, char **argv)
g_assert(qcrypto_init(NULL) == 0);
-for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
-if (qcrypto_cipher_supports(test_data[i].alg, test_data[i].mode)) {
-g_test_add_data_func(test_data[i].path, &test_data[i],
test_cipher);
+if ((argc > 1) && !strcmp(argv[1], "speed")) {
+for (i = 512; i <= (64 * 1204); i *= 2) {
+g_test_add_data_func("/crypto/cipher/speed", (void *)i,
+ test_cipher_speed);
+}
+} else {
+for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
+if (qcrypto_cipher_supports(test_data[i].alg, test_data[i].mode)) {
+g_test_add_data_func(test_data[i].path, &test_data[i],
+ test_cipher);
+}
}
-}
-g_test_add_func("/crypto/cipher/null-iv",
-test_cipher_null_iv);
+g_test_add_func("/crypto/cipher/null-iv",
+test_cipher_null_iv);
-g_test_add_func("/crypto/cipher/short-plaintext",
-test_cipher_short_plaintext);
+g_test_add_func("/crypto/cipher/short-plaintext",
+test_cipher_short_plaintext);
+}
return g_test_run();
}
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 13/19] crypto: introduce some common functions for af_alg backend
This patch introduces some common functions for af_alg backend,
they would be used in af_alg-backend cipher/hash/hmac latter.
Signed-off-by: Longpeng(Mike)
---
crypto/Makefile.objs| 1 +
crypto/afalg-comm.c | 71 +
include/crypto/afalg-comm.h | 61 ++
3 files changed, 133 insertions(+)
create mode 100644 crypto/afalg-comm.c
create mode 100644 include/crypto/afalg-comm.h
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 1f749f2..6f244a3 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -10,6 +10,7 @@ crypto-obj-$(if $(CONFIG_NETTLE),n,$(if
$(CONFIG_GCRYPT_HMAC),n,y)) += hmac-glib
crypto-obj-y += aes.o
crypto-obj-y += desrfb.o
crypto-obj-y += cipher.o
+crypto-obj-$(CONFIG_AF_ALG) += afalg-comm.o
crypto-obj-y += tlscreds.o
crypto-obj-y += tlscredsanon.o
crypto-obj-y += tlscredsx509.o
diff --git a/crypto/afalg-comm.c b/crypto/afalg-comm.c
new file mode 100644
index 000..27bc88c
--- /dev/null
+++ b/crypto/afalg-comm.c
@@ -0,0 +1,71 @@
+/*
+ * QEMU Crypto af_alg support
+ *
+ * Copyright (c) 2017 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *Longpeng(Mike)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "qemu/sockets.h"
+#include "qapi/error.h"
+#include "crypto/afalg-comm.h"
+
+void afalg_comm_format_type(AfalgSocketAddress *afalg,
+const char *type)
+{
+afalg->type = (char *)g_new0(int8_t, SALG_TYPE_LEN_MAX);
+pstrcpy(afalg->type, SALG_TYPE_LEN_MAX, type);
+}
+
+void afalg_comm_free(QCryptoAfalg *afalg)
+{
+if (afalg) {
+if (afalg->msg) {
+g_free(afalg->msg->msg_control);
+g_free(afalg->msg);
+}
+
+if (afalg->tfmfd != -1) {
+closesocket(afalg->tfmfd);
+}
+
+if (afalg->opfd != -1) {
+closesocket(afalg->opfd);
+}
+
+g_free(afalg);
+}
+}
+
+QCryptoAfalg *afalg_comm_alloc(SocketAddress *saddr)
+{
+QCryptoAfalg *afalg = NULL;
+Error *err = NULL;
+
+afalg = g_new0(QCryptoAfalg, 1);
+/* initilize crypto API socket */
+afalg->opfd = -1;
+afalg->tfmfd = socket_bind(saddr, &err);
+if (afalg->tfmfd == -1) {
+goto error;
+}
+
+afalg->opfd = qemu_accept(afalg->tfmfd, NULL, 0);
+if (afalg->opfd == -1) {
+closesocket(afalg->tfmfd);
+goto error;
+}
+
+return afalg;
+
+error:
+error_free(err);
+afalg_comm_free(afalg);
+return NULL;
+}
diff --git a/include/crypto/afalg-comm.h b/include/crypto/afalg-comm.h
new file mode 100644
index 000..b6b9464
--- /dev/null
+++ b/include/crypto/afalg-comm.h
@@ -0,0 +1,61 @@
+/*
+ * QEMU Crypto af_alg support
+ *
+ * Copyright (c) 2017 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *Longpeng(Mike)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ */
+#ifndef QCRYPTO_AFALG_H
+#define QCRYPTO_AFALG_H
+
+#include "qapi-types.h"
+
+#ifndef SOL_ALG
+#define SOL_ALG 279
+#endif
+
+typedef struct QCryptoAfalg QCryptoAfalg;
+struct QCryptoAfalg {
+int tfmfd;
+int opfd;
+struct msghdr *msg;
+struct cmsghdr *cmsg;
+};
+
+
+/**
+ * afalg_comm_format_type:
+ * @afalg: the AfalgSocketAddress object
+ * @type: the type of crypto alg.
+ *
+ * Set the type field of the @afalg according to @type.
+ */
+void afalg_comm_format_type(AfalgSocketAddress *afalg,
+const char *type);
+
+/**
+ * afalg_comm_alloc:
+ * @saddr: the SocketAddress object
+ *
+ * Allocate a QCryptoAfalg object and bind itself to
+ * a AF_ALG socket.
+ *
+ * Returns:
+ * a new QCryptoAfalg object, or NULL in error.
+ */
+QCryptoAfalg *afalg_comm_alloc(SocketAddress *saddr);
+
+/**
+ * afalg_comm_free:
+ * @afalg: the QCryptoAfalg object
+ *
+ * Free the @afalg.
+ */
+void afalg_comm_free(QCryptoAfalg *afalg);
+
+#endif
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 17/19] tests: crypto: add hash speed case
Now we have afalg-backend and libiary-backend, it's necessary
to add the speed test in test-crypto-hash.
We can use "./tests/test-crypto-hash speed" to do the speed test.
Signed-off-by: Longpeng(Mike)
---
tests/test-crypto-hash.c | 56 +++-
1 file changed, 51 insertions(+), 5 deletions(-)
diff --git a/tests/test-crypto-hash.c b/tests/test-crypto-hash.c
index 214a9f7..71199ec 100644
--- a/tests/test-crypto-hash.c
+++ b/tests/test-crypto-hash.c
@@ -241,15 +241,61 @@ static void test_hash_base64(void)
}
}
+static void test_hash_speed(const void *opaque)
+{
+size_t chunk_size = (size_t)opaque;
+uint8_t *in = NULL, *out = NULL;
+size_t out_len = 0;
+double total = 0.0;
+struct iovec iov;
+int ret;
+
+in = g_new0(uint8_t, chunk_size);
+memset(in, g_test_rand_int(), chunk_size);
+
+iov.iov_base = (char *)in;
+iov.iov_len = chunk_size;
+
+g_test_timer_start();
+do {
+ret = qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
+ &iov, 1, &out, &out_len,
+ NULL);
+g_assert(ret == 0);
+
+total += chunk_size;
+} while (g_test_timer_elapsed() < 5.0);
+
+total /= 1024 * 1024; /* to MB */
+g_print("Testing sha256: ");
+g_print("Encrypting in chunks of %ld bytes: ", chunk_size);
+g_print("done. %.2f MB in %.2f secs: ", total, g_test_timer_last());
+g_print("%.2f MB/sec\t", total / g_test_timer_last());
+
+g_free(out);
+g_free(in);
+}
+
int main(int argc, char **argv)
{
+size_t i;
+
g_assert(qcrypto_init(NULL) == 0);
g_test_init(&argc, &argv, NULL);
-g_test_add_func("/crypto/hash/iov", test_hash_iov);
-g_test_add_func("/crypto/hash/alloc", test_hash_alloc);
-g_test_add_func("/crypto/hash/prealloc", test_hash_prealloc);
-g_test_add_func("/crypto/hash/digest", test_hash_digest);
-g_test_add_func("/crypto/hash/base64", test_hash_base64);
+
+if ((argc > 1) && !strcmp(argv[1], "speed")) {
+for (i = 512; i <= (64 * 1204); i *= 2) {
+g_test_add_data_func("/crypto/hash/speed", (void *)i,
+ test_hash_speed);
+}
+} else {
+g_test_add_func("/crypto/hash/iov", test_hash_iov);
+g_test_add_func("/crypto/hash/alloc", test_hash_alloc);
+g_test_add_func("/crypto/hash/prealloc", test_hash_prealloc);
+g_test_add_func("/crypto/hash/digest", test_hash_digest);
+g_test_add_func("/crypto/hash/base64", test_hash_base64);
+}
+
return g_test_run();
}
--
1.8.3.1
[Qemu-devel] [PATCH for-2.10 18/19] crypto: hmac: add af_alg hmac support
Adds afalg-backend hmac support: introduces some private APIs
firstly, and then intergrates them into qcrypto_hmac_afalg_driver.
Signed-off-by: Longpeng(Mike)
---
crypto/hash-afalg.c | 108 ++--
crypto/hmac.c | 28 +++-
include/crypto/afalg-comm.h | 1 +
include/crypto/hmac.h | 13 ++
4 files changed, 136 insertions(+), 14 deletions(-)
diff --git a/crypto/hash-afalg.c b/crypto/hash-afalg.c
index d96d7568..f2c27b1 100644
--- a/crypto/hash-afalg.c
+++ b/crypto/hash-afalg.c
@@ -1,5 +1,5 @@
/*
- * QEMU Crypto af_alg-backend hash support
+ * QEMU Crypto af_alg-backend hash/hmac support
*
* Copyright (c) 2017 HUAWEI TECHNOLOGIES CO., LTD.
*
@@ -16,6 +16,7 @@
#include "qemu-common.h"
#include "qapi/error.h"
#include "crypto/hash.h"
+#include "crypto/hmac.h"
#include "crypto/afalg-comm.h"
#include
@@ -57,7 +58,10 @@ static int afalg_hash_format_name(QCryptoHashAlgorithm alg,
return 0;
}
-static QCryptoAfalg *afalg_hash_ctx_new(QCryptoHashAlgorithm alg)
+static QCryptoAfalg *
+afalg_hash_hmac_ctx_new(QCryptoHashAlgorithm alg,
+const uint8_t *key, size_t nkey,
+bool is_hash)
{
SocketAddress *saddr = NULL;
QCryptoAfalg *afalg = NULL;
@@ -70,13 +74,27 @@ static QCryptoAfalg
*afalg_hash_ctx_new(QCryptoHashAlgorithm alg)
if (ret != 0) {
goto error;
}
-afalg_comm_format_type(saddr->u.afalg.data, ALG_TYPE_HASH);
+
+if (is_hash) {
+afalg_comm_format_type(saddr->u.afalg.data, ALG_TYPE_HASH);
+} else {
+afalg_comm_format_type(saddr->u.afalg.data, ALG_TYPE_HMAC);
+}
afalg = afalg_comm_alloc(saddr);
if (!afalg) {
goto error;
}
+/* HMAC needs setkey */
+if (!is_hash) {
+ret = qemu_setsockopt(afalg->tfmfd, SOL_ALG, ALG_SET_KEY,
+ key, nkey);
+if (ret != 0) {
+goto error;
+}
+}
+
/* prepare msg header */
afalg->msg = g_new0(struct msghdr, 1);
@@ -93,15 +111,38 @@ error:
goto cleanup;
}
-static int afalg_hash_bytesv(QCryptoHashAlgorithm alg,
- const struct iovec *iov,
- size_t niov, uint8_t **result,
- size_t *resultlen,
- Error **errp)
+static QCryptoAfalg *afalg_hash_ctx_new(QCryptoHashAlgorithm alg)
+{
+return afalg_hash_hmac_ctx_new(alg, NULL, 0, true);
+}
+
+QCryptoAfalg *afalg_hmac_ctx_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
+{
+QCryptoAfalg *afalg;
+
+afalg = afalg_hash_hmac_ctx_new(alg, key, nkey, false);
+if (afalg == NULL) {
+error_setg(errp, "Afalg cannot initialize hmac and set key");
+return NULL;
+}
+
+return afalg;
+}
+
+static int
+afalg_hash_hmac_bytesv(QCryptoAfalg *hmac,
+ QCryptoHashAlgorithm alg,
+ const struct iovec *iov,
+ size_t niov, uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
{
QCryptoAfalg *afalg = NULL;
struct iovec outv;
int ret = 0;
+bool is_hmac = (hmac != NULL) ? true : false;
const int except_len = qcrypto_hash_digest_len(alg);
if (*resultlen == 0) {
@@ -114,10 +155,14 @@ static int afalg_hash_bytesv(QCryptoHashAlgorithm alg,
return -1;
}
-afalg = afalg_hash_ctx_new(alg);
-if (afalg == NULL) {
-error_setg(errp, "Alloc QCryptoAfalg object failed");
-return -1;
+if (is_hmac) {
+afalg = hmac;
+} else {
+afalg = afalg_hash_ctx_new(alg);
+if (afalg == NULL) {
+error_setg(errp, "Alloc QCryptoAfalg object failed");
+return -1;
+}
}
/* send data to kernel's crypto core */
@@ -141,10 +186,47 @@ static int afalg_hash_bytesv(QCryptoHashAlgorithm alg,
}
out:
-afalg_comm_free(afalg);
+if (!is_hmac) { /* hash */
+afalg_comm_free(afalg);
+}
return ret;
}
+static int
+afalg_hash_bytesv(QCryptoHashAlgorithm alg,
+ const struct iovec *iov,
+ size_t niov, uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
+{
+return afalg_hash_hmac_bytesv(NULL, alg, iov, niov,
+ result, resultlen, errp);
+}
+
+static int
+afalg_hmac_bytesv(QCryptoHmac *hmac,
+ const struct iovec *iov,
+ size_t niov, uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
+{
+return afalg_hash_hmac_bytesv(hmac->opaque, hmac->alg, iov, niov,
+ result, resultlen, errp);
+}
+
+static void afalg_hmac_ctx_free(QCryptoHmac *hmac)
+{
+QCryptoAfalg *afalg
Re: [Qemu-devel] [PATCH] 9pfs: xattr: fix memory leak in v9fs_list_xattr
On Fri, 7 Apr 2017 03:48:52 -0700
Li Qiang wrote:
> Free 'orig_value' in error path.
>
> Signed-off-by: Li Qiang
> ---
Good catch. I'll send a pull req later today.
Cheers.
--
Greg
> hw/9pfs/9p-xattr.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/9pfs/9p-xattr.c b/hw/9pfs/9p-xattr.c
> index eec160b..d05c1a1 100644
> --- a/hw/9pfs/9p-xattr.c
> +++ b/hw/9pfs/9p-xattr.c
> @@ -108,6 +108,7 @@ ssize_t v9fs_list_xattr(FsContext *ctx, const char *path,
> g_free(name);
> close_preserve_errno(dirfd);
> if (xattr_len < 0) {
> +g_free(orig_value);
> return -1;
> }
>
pgpd03aujw2Cc.pgp
Description: OpenPGP digital signature
[Qemu-devel] [PATCH for-2.10 19/19] tests: crypto: add hmac speed case
Now we have afalg-backend and libiary-backend, it's necessary
to add the speed test in test-crypto-hmac.
We can use "./tests/test-crypto-hmac speed" to do the speed
test.
Signed-off-by: Longpeng(Mike)
---
tests/test-crypto-hmac.c | 66 +---
1 file changed, 62 insertions(+), 4 deletions(-)
diff --git a/tests/test-crypto-hmac.c b/tests/test-crypto-hmac.c
index ee55382..febc556 100644
--- a/tests/test-crypto-hmac.c
+++ b/tests/test-crypto-hmac.c
@@ -251,16 +251,74 @@ static void test_hmac_digest(void)
}
}
+static void test_hmac_speed(const void *opaque)
+{
+size_t chunk_size = (size_t)opaque;
+QCryptoHmac *hmac = NULL;
+uint8_t *in = NULL, *out = NULL;
+size_t out_len = 0;
+double total = 0.0;
+struct iovec iov;
+Error *err = NULL;
+int ret;
+
+if (!qcrypto_hmac_supports(QCRYPTO_HASH_ALG_SHA256)) {
+return;
+}
+
+hmac = qcrypto_hmac_new(QCRYPTO_HASH_ALG_SHA256, (const uint8_t *)KEY,
+strlen(KEY), &err);
+g_assert(err == NULL);
+g_assert(hmac != NULL);
+
+in = g_new0(uint8_t, chunk_size);
+memset(in, g_test_rand_int(), chunk_size);
+
+iov.iov_base = (char *)in;
+iov.iov_len = chunk_size;
+
+g_test_timer_start();
+do {
+ret = qcrypto_hmac_bytesv(hmac, &iov, 1, &out, &out_len, &err);
+g_assert(ret == 0);
+g_assert(err == NULL);
+
+total += chunk_size;
+} while (g_test_timer_elapsed() < 5.0);
+
+total /= 1024 * 1024; /* to MB */
+
+g_print("[drv:%s]", qcrypto_hmac_using_afalg_drv(hmac) ?
+"afalg" : "libs");
+g_print("Testing hmac(sha256): ");
+g_print("Encrypting in chunks of %ld bytes: ", chunk_size);
+g_print("done. %.2f MB in %.2f secs: ", total, g_test_timer_last());
+g_print("%.2f MB/sec\t", total / g_test_timer_last());
+
+qcrypto_hmac_free(hmac);
+g_free(out);
+g_free(in);
+}
+
int main(int argc, char **argv)
{
+size_t i;
+
g_test_init(&argc, &argv, NULL);
g_assert(qcrypto_init(NULL) == 0);
-g_test_add_func("/crypto/hmac/iov", test_hmac_iov);
-g_test_add_func("/crypto/hmac/alloc", test_hmac_alloc);
-g_test_add_func("/crypto/hmac/prealloc", test_hmac_prealloc);
-g_test_add_func("/crypto/hmac/digest", test_hmac_digest);
+if ((argc > 1) && !strcmp(argv[1], "speed")) {
+for (i = 512; i <= (64 * 1204); i *= 2) {
+g_test_add_data_func("/crypto/hmac/speed", (void *)i,
+ test_hmac_speed);
+}
+} else {
+g_test_add_func("/crypto/hmac/iov", test_hmac_iov);
+g_test_add_func("/crypto/hmac/alloc", test_hmac_alloc);
+g_test_add_func("/crypto/hmac/prealloc", test_hmac_prealloc);
+g_test_add_func("/crypto/hmac/digest", test_hmac_digest);
+}
return g_test_run();
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH 11/21] ppc/pnv: scan ISA bus to populate device tree
On 04/10/2017 07:17 AM, David Gibson wrote:
> On Wed, Apr 05, 2017 at 02:41:36PM +0200, Cédric Le Goater wrote:
>> This is an empty shell that we will use to include nodes in the device
>> tree for ISA devices. We expect RTC, UART and IPMI BT devices.
>>
>> Signed-off-by: Cédric Le Goater
>
> This is so simple, I'd probably fold it into the next patch. But
> apart from that.
yes. I agree. The goal was to find an alternative to qbus_walk_children()
which is not a QOM API.
C.
> Reviewed-by: David Gibson
>
>> ---
>> hw/ppc/pnv.c | 32
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 493c7eed7980..a3c8f6594d10 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -281,6 +281,36 @@ static void powernv_populate_chip(PnvChip *chip, void
>> *fdt)
>> g_free(typename);
>> }
>>
>> +typedef struct ForeachPopulateArgs {
>> +void *fdt;
>> +int offset;
>> +} ForeachPopulateArgs;
>> +
>> +static int powernv_populate_isa_device(DeviceState *dev, void *opaque)
>> +{
>> +return 0;
>> +}
>> +
>> +static void powernv_populate_isa(ISABus *bus, void *fdt)
>> +{
>> +int lpc_offset;
>> +ForeachPopulateArgs args;
>> +
>> +lpc_offset = fdt_node_offset_by_compatible(fdt, -1, "ibm,lpc");
>> +if (lpc_offset < 0) {
>> +error_report("Could find the lpc node !?");
>> +return;
>> +}
>> +
>> +args.fdt = fdt;
>> +args.offset = lpc_offset;
>> +
>> +/* ISA devices are not necessarily parented to the ISA bus so we
>> + * can not use object_child_foreach() */
>> +qbus_walk_children(BUS(bus), powernv_populate_isa_device,
>> + NULL, NULL, NULL, &args);
>> +}
>> +
>> static void *powernv_create_fdt(MachineState *machine)
>> {
>> const char plat_compat[] = "qemu,powernv\0ibm,powernv";
>> @@ -328,6 +358,8 @@ static void *powernv_create_fdt(MachineState *machine)
>> for (i = 0; i < pnv->num_chips; i++) {
>> powernv_populate_chip(pnv->chips[i], fdt);
>> }
>> +
>> +powernv_populate_isa(pnv->isa_bus, fdt);
>> return fdt;
>> }
>>
>
Re: [Qemu-devel] [PATCH 03/10] blockjob: introduce block_job_fail
On Thu, Mar 23, 2017 at 06:39:21PM +0100, Paolo Bonzini wrote: > Outside blockjob.c, block_job_unref is only used when a block job fails > to start, and block_job_ref is not used at all. The reference counting > thus is pretty well hidden. Introduce a separate function to be used > by block jobs; because block_job_ref and block_job_unref now become > static, move them earlier in blockjob.c. > > Later on, block_job_fail will also have different locking than > block_job_unref. block_job_fail() sounds like it's *the* job failure API. How about block_job_fail_early()? It indicates the API is only for the beginning of the job's lifecycle. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] qxl: add migration blocker to avoid pre-save assert
Hi
On Mon, Apr 10, 2017 at 8:58 AM Gerd Hoffmann wrote:
> Cc: 1635...@bugs.launchpad.net
> Signed-off-by: Gerd Hoffmann
> ---
> hw/display/qxl.h | 1 +
> hw/display/qxl.c | 22 ++
> 2 files changed, 23 insertions(+)
>
> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> index d2d49dd..77e5a36 100644
> --- a/hw/display/qxl.h
> +++ b/hw/display/qxl.h
> @@ -40,6 +40,7 @@ typedef struct PCIQXLDevice {
> uint32_t cmdlog;
>
> uint32_t guest_bug;
> +Error *migration_blocker;
>
> enum qxl_mode mode;
> uint32_t cmdflags;
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index c31b293..74ebeb9 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -26,6 +26,7 @@
> #include "qemu/queue.h"
> #include "qemu/atomic.h"
> #include "sysemu/sysemu.h"
> +#include "migration/migration.h"
> #include "trace.h"
>
> #include "qxl.h"
> @@ -639,6 +640,27 @@ static int interface_get_command(QXLInstance *sin,
> struct QXLCommandExt *ext)
> qxl->guest_primary.commands++;
> qxl_track_command(qxl, ext);
> qxl_log_command(qxl, "cmd", ext);
> +{
> +/*
> + * Windows 8 drivers place qxl commands in the vram
>
+ * (instead of the ram) bar. We can't live migrate such a
> + * guest, so add a migration blocker in case we detect
> + * this, to avoid triggering the assert in pre_save().
> + *
> + *
> https://cgit.freedesktop.org/spice/win32/qxl-wddm-dod/commit/?id=f6e099db39e7d0787f294d5fd0dce328b5210faa
> + */
> +void *msg = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
> +if (msg != NULL && (
> +msg < (void *)qxl->vga.vram_ptr ||
> +msg > ((void *)qxl->vga.vram_ptr +
> qxl->vga.vram_size))) {
> +if (!qxl->migration_blocker) {
> +Error *local_err = NULL;
> +error_setg(&qxl->migration_blocker,
> + "qxl: guest bug: command not in ram bar");
> +migrate_add_blocker(qxl->migration_blocker,
> &local_err);
> +}
>
Should the blocker be removed on reset?
Looks and works ok otherwise
> +}
> +}
> trace_qxl_ring_command_get(qxl->id,
> qxl_mode_to_string(qxl->mode));
> return true;
> default:
> --
> 2.9.3
>
> --
Marc-André Lureau
Re: [Qemu-devel] [PATCH 15/21] ppc/pnv: add initial IPMI sensors for the BMC simulator
On 04/10/2017 07:31 AM, David Gibson wrote:
> On Wed, Apr 05, 2017 at 02:41:40PM +0200, Cédric Le Goater wrote:
>> Skiboot, the firmware for the PowerNV platform, expects the BMC to
>> provide some specific IPMI sensors. These sensors are exposed in the
>> device tree and their values are updated by the firmware at boot time.
>>
>> Sensors of interest are :
>>
>> "FW Boot Progress"
>> "Boot Count"
>>
>> As such a device is defined on the command line, we can only detect
>> its presence at reset time.
>>
>> Signed-off-by: Cédric Le Goater
>> ---
>> hw/ppc/Makefile.objs | 2 +-
>> hw/ppc/pnv.c | 17 +++
>> hw/ppc/pnv_bmc.c | 81
>>
>> include/hw/ppc/pnv.h | 7 +
>> 4 files changed, 106 insertions(+), 1 deletion(-)
>> create mode 100644 hw/ppc/pnv_bmc.c
>>
>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>> index ef67ea820158..7efc68674819 100644
>> --- a/hw/ppc/Makefile.objs
>> +++ b/hw/ppc/Makefile.objs
>> @@ -6,7 +6,7 @@ obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o
>> spapr_rtas.o
>> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
>> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o
>> # IBM PowerNV
>> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o
>> pnv_occ.o
>> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o
>> pnv_occ.o pnv_bmc.o
>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>> obj-y += spapr_pci_vfio.o
>> endif
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index dfa1cf849b35..c7caecad0aa6 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -35,6 +35,7 @@
>> #include "qapi/visitor.h"
>> #include "monitor/monitor.h"
>> #include "hw/intc/intc.h"
>> +#include "hw/ipmi/ipmi.h"
>>
>> #include "hw/ppc/xics.h"
>> #include "hw/ppc/pnv_xscom.h"
>> @@ -458,16 +459,32 @@ static void *powernv_create_fdt(MachineState *machine)
>> }
>>
>> powernv_populate_isa(pnv->isa_bus, fdt);
>> +
>> +if (pnv->bmc) {
>> +pnv_bmc_populate_sensors(pnv->bmc, fdt);
>> +}
>> +
>> return fdt;
>> }
>>
>> static void ppc_powernv_reset(void)
>> {
>> MachineState *machine = MACHINE(qdev_get_machine());
>> +PnvMachineState *pnv = POWERNV_MACHINE(machine);
>> void *fdt;
>>
>> qemu_devices_reset();
>>
>> +/* OpenPOWER systems have a BMC, which can be defined on the
>> + * command line with:
>> + *
>> + * -device ipmi-bmc-sim,id=bmc0
>> + *
>> + * This is the internal simulator but it could also be an external
>> + * BMC.
>> + */
>> +pnv->bmc = object_resolve_path_type("", TYPE_IPMI_BMC, NULL);
>
> I'd suggest typing the pointer and doing the cast/test here, rather
> than in the functions that need to manipulate it.
OK.
>
>> fdt = powernv_create_fdt(machine);
>>
>> /* Pack resulting tree */
>> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
>> new file mode 100644
>> index ..2998530dc4c0
>> --- /dev/null
>> +++ b/hw/ppc/pnv_bmc.c
>> @@ -0,0 +1,81 @@
>> +/*
>> + * QEMU PowerNV, BMC related functions
>> + *
>> + * Copyright (c) 2016-2017, IBM Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License, version 2, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see
Re: [Qemu-devel] [PATCH 04/10] blockjob: introduce block_job_pause/resume_all
On Thu, Mar 23, 2017 at 06:39:22PM +0100, Paolo Bonzini wrote: > Remove use of block_job_pause/resume from outside blockjob.c, thus > making them static. Again, one reason to do this is that > block_job_pause/resume will have different locking rules compared > to everything else that block.c and block/io.c use. > > Signed-off-by: Paolo Bonzini > --- > block/io.c | 18 +--- > blockjob.c | 114 > --- > include/block/blockjob.h | 14 +++--- > 3 files changed, 77 insertions(+), 69 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 01/10] blockjob: remove unnecessary check
On Thu, Mar 23, 2017 at 06:39:19PM +0100, Paolo Bonzini wrote: > !job is always checked prior to the call, drop it from here. > > Signed-off-by: Paolo Bonzini > --- > blockjob.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 02/10] blockjob: remove iostatus_reset callback
On Thu, Mar 23, 2017 at 06:39:20PM +0100, Paolo Bonzini wrote:
> This is unused since commit 66a0fae ("blockjob: Don't touch BDS iostatus",
> 2016-05-19).
>
> Signed-off-by: Paolo Bonzini
> ---
> blockjob.c | 3 ---
> include/block/blockjob_int.h | 3 ---
> 2 files changed, 6 deletions(-)
Reviewed-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.10 00/19] crypto: add af_alg-backend support
> -Original Message- > From: longpeng > Sent: Monday, April 10, 2017 5:03 PM > To: berra...@redhat.com; kra...@redhat.com; pbonz...@redhat.com; > ebl...@redhat.com; arm...@redhat.com > Cc: Xuquan (Quan Xu); Gonglei (Arei); qemu-devel@nongnu.org; longpeng > Subject: [PATCH for-2.10 00/19] crypto: add af_alg-backend support > > The AF_ALG socket family is the userspace interface for linux > crypto API, users can use it to access hardware accelerators. > > This patchset adds a afalg-backend for qemu crypto subsystem. Currently > when performs encrypt/decrypt, we'll try afalg-backend first and will > back to libiary-backend if it failed. > Cool. You'd better add the performance compared results in the cover letter. :) Regards, -Gonglei > In the next step, It would support a command parameter to specifies > which backends prefer to and some other improvements. > > Longpeng(Mike) (19): > crypto: cipher: introduce context free function > crypto: cipher: introduce qcrypto_cipher_ctx_new for gcrypt-backend > crypto: cipher: introduce qcrypto_cipher_ctx_new for nettle-backend > crypto: cipher: introduce qcrypto_cipher_ctx_new for builtin-backend > crypto: cipher: add cipher driver framework > crypto: hash: add hash driver framework > crypto: hmac: move crypto/hmac.h into include/crypto/ > crypto: hmac: introduce qcrypto_hmac_ctx_new for gcrypt-backend > crypto: hmac: introduce qcrypto_hmac_ctx_new for nettle-backend > crypto: hmac: introduce qcrypto_hmac_ctx_new for glib-backend > crypto: hmac: add hmac driver framework > socket: add af_alg family support > crypto: introduce some common functions for af_alg backend > crypto: cipher: add af_alg cipher support > tests: crypto: add cipher speed case > crypto: hash: add af_alg hash support > tests: crypto: add hash speed case > crypto: hmac: add af_alg hmac support > tests: crypto: add hmac speed case > > configure | 21 > crypto/Makefile.objs| 3 + > crypto/afalg-comm.c | 71 ++ > crypto/cipher-afalg.c | 229 > +++ > crypto/cipher-builtin.c | 120 +++ > crypto/cipher-gcrypt.c | 99 ++- > crypto/cipher-nettle.c | 78 --- > crypto/cipher.c | 87 + > crypto/hash-afalg.c | 232 > > crypto/hash-gcrypt.c| 17 ++-- > crypto/hash-glib.c | 17 ++-- > crypto/hash-nettle.c| 17 ++-- > crypto/hash.c | 22 + > crypto/hmac-gcrypt.c| 39 > crypto/hmac-glib.c | 58 +-- > crypto/hmac-nettle.c| 39 +++- > crypto/hmac.c | 65 + > crypto/hmac.h | 166 --- > include/crypto/afalg-comm.h | 74 ++ > include/crypto/cipher.h | 29 ++ > include/crypto/hash.h | 13 +++ > include/crypto/hmac.h | 199 > + > include/qemu/sockets.h | 6 ++ > qapi-schema.json| 21 +++- > tests/test-crypto-cipher.c | 92 -- > tests/test-crypto-hash.c| 56 ++- > tests/test-crypto-hmac.c| 66 - > util/qemu-sockets.c | 91 + > 28 files changed, 1599 insertions(+), 428 deletions(-) > create mode 100644 crypto/afalg-comm.c > create mode 100644 crypto/cipher-afalg.c > create mode 100644 crypto/hash-afalg.c > delete mode 100644 crypto/hmac.h > create mode 100644 include/crypto/afalg-comm.h > create mode 100644 include/crypto/hmac.h > > -- > 1.8.3.1 >
Re: [Qemu-devel] [PATCH 05/10] blockjob: separate monitor and blockjob APIs
On Thu, Mar 23, 2017 at 06:39:23PM +0100, Paolo Bonzini wrote: > We already have different locking policies for APIs called by the monitor > and the block job. Monitor APIs need consistency across block_job_get > and the actual operation (e.g. block_job_set_speed), so currently there > are explicit aio_context_acquire/release calls in blockdev.c. > > When a block job needs to do something instead it doesn't care about locking, > because the whole coroutine runs under the AioContext lock. When moving > away from the AioContext lock, the monitor will have to call new > block_job_lock/unlock APIs, while blockjob APIs will take care of this > for the users. > > In preparation for that, keep all the blockjob APIs together in the > blockjob.c file. > > Signed-off-by: Paolo Bonzini > --- > blockjob.c | 206 > +++-- > 1 file changed, 105 insertions(+), 101 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 06/10] blockjob: move iostatus reset inside block_job_user_resume
On Thu, Mar 23, 2017 at 06:39:24PM +0100, Paolo Bonzini wrote: > Outside blockjob.c, the block_job_iostatus_reset function is used once > in the monitor and once in BlockBackend. When we introduce the block > job mutex, block_job_iostatus_reset's client is going to be the block > layer (for which blockjob.c will take the block job mutex) rather than > the monitor (which will take the block job mutex by itself). > > The monitor's call to block_job_iostatus_reset from the monitor comes > just before the sole call to block_job_user_resume, so reset the > iostatus directly from block_job_iostatus_reset. This will avoid > the need to introduce separate block_job_iostatus_reset and > block_job_iostatus_reset_locked APIs. > > After making this change, move the function together with the others > that were moved in the previous patch. > > Signed-off-by: Paolo Bonzini > --- > blockdev.c | 1 - > blockjob.c | 11 ++- > 2 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 07/10] blockjob: strengthen a bit test-blockjob-txn
On Thu, Mar 23, 2017 at 06:39:25PM +0100, Paolo Bonzini wrote: > Unlike test-blockjob-txn, QMP releases the reference to the transaction > before the jobs finish. Thus, while working on the next patch, > qemu-iotest 124 showed a failure that the unit tests did not have. > Make the unit test just a little nastier, so that it fails too. > > Signed-off-by: Paolo Bonzini > --- > tests/test-blockjob-txn.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async
On Thu, Mar 23, 2017 at 06:39:26PM +0100, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > blockjob.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.10 00/19] crypto: add af_alg-backend support
On 2017/4/10 17:28, Gonglei (Arei) wrote: > >> -Original Message- >> From: longpeng >> Sent: Monday, April 10, 2017 5:03 PM >> To: berra...@redhat.com; kra...@redhat.com; pbonz...@redhat.com; >> ebl...@redhat.com; arm...@redhat.com >> Cc: Xuquan (Quan Xu); Gonglei (Arei); qemu-devel@nongnu.org; longpeng >> Subject: [PATCH for-2.10 00/19] crypto: add af_alg-backend support >> >> The AF_ALG socket family is the userspace interface for linux >> crypto API, users can use it to access hardware accelerators. >> >> This patchset adds a afalg-backend for qemu crypto subsystem. Currently >> when performs encrypt/decrypt, we'll try afalg-backend first and will >> back to libiary-backend if it failed. >> > Cool. > > You'd better add the performance compared results in the cover letter. :) > OK, I'll add the results in V2. In addition, I find a BUG in patch 18, please review other parts and I'll fix them together in next version. > > Regards, > -Gonglei > >> In the next step, It would support a command parameter to specifies >> which backends prefer to and some other improvements. >> >> Longpeng(Mike) (19): >> crypto: cipher: introduce context free function >> crypto: cipher: introduce qcrypto_cipher_ctx_new for gcrypt-backend >> crypto: cipher: introduce qcrypto_cipher_ctx_new for nettle-backend >> crypto: cipher: introduce qcrypto_cipher_ctx_new for builtin-backend >> crypto: cipher: add cipher driver framework >> crypto: hash: add hash driver framework >> crypto: hmac: move crypto/hmac.h into include/crypto/ >> crypto: hmac: introduce qcrypto_hmac_ctx_new for gcrypt-backend >> crypto: hmac: introduce qcrypto_hmac_ctx_new for nettle-backend >> crypto: hmac: introduce qcrypto_hmac_ctx_new for glib-backend >> crypto: hmac: add hmac driver framework >> socket: add af_alg family support >> crypto: introduce some common functions for af_alg backend >> crypto: cipher: add af_alg cipher support >> tests: crypto: add cipher speed case >> crypto: hash: add af_alg hash support >> tests: crypto: add hash speed case >> crypto: hmac: add af_alg hmac support >> tests: crypto: add hmac speed case >> >> configure | 21 >> crypto/Makefile.objs| 3 + >> crypto/afalg-comm.c | 71 ++ >> crypto/cipher-afalg.c | 229 >> +++ >> crypto/cipher-builtin.c | 120 +++ >> crypto/cipher-gcrypt.c | 99 ++- >> crypto/cipher-nettle.c | 78 --- >> crypto/cipher.c | 87 + >> crypto/hash-afalg.c | 232 >> >> crypto/hash-gcrypt.c| 17 ++-- >> crypto/hash-glib.c | 17 ++-- >> crypto/hash-nettle.c| 17 ++-- >> crypto/hash.c | 22 + >> crypto/hmac-gcrypt.c| 39 >> crypto/hmac-glib.c | 58 +-- >> crypto/hmac-nettle.c| 39 +++- >> crypto/hmac.c | 65 + >> crypto/hmac.h | 166 --- >> include/crypto/afalg-comm.h | 74 ++ >> include/crypto/cipher.h | 29 ++ >> include/crypto/hash.h | 13 +++ >> include/crypto/hmac.h | 199 >> + >> include/qemu/sockets.h | 6 ++ >> qapi-schema.json| 21 +++- >> tests/test-crypto-cipher.c | 92 -- >> tests/test-crypto-hash.c| 56 ++- >> tests/test-crypto-hmac.c| 66 - >> util/qemu-sockets.c | 91 + >> 28 files changed, 1599 insertions(+), 428 deletions(-) >> create mode 100644 crypto/afalg-comm.c >> create mode 100644 crypto/cipher-afalg.c >> create mode 100644 crypto/hash-afalg.c >> delete mode 100644 crypto/hmac.h >> create mode 100644 include/crypto/afalg-comm.h >> create mode 100644 include/crypto/hmac.h >> >> -- >> 1.8.3.1 >> > > > . > -- Regards, Longpeng(Mike)
Re: [Qemu-devel] [PATCH 09/10] blockjob: reorganize block_job_completed_txn_abort
On Thu, Mar 23, 2017 at 06:39:27PM +0100, Paolo Bonzini wrote: > This splits the part that touches job states from the part that invokes > callbacks. It will be a bit simpler to understand once job states will > be protected by a different mutex than the AioContext lock. > > Signed-off-by: Paolo Bonzini > --- > blockjob.c | 165 > - > 1 file changed, 88 insertions(+), 77 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 10/10] blockjob: use deferred_to_main_loop to indicate the coroutine has ended
On Thu, Mar 23, 2017 at 06:39:28PM +0100, Paolo Bonzini wrote: > All block jobs are using block_job_defer_to_main_loop as the final > step just before the coroutine terminates. At this point, > block_job_enter should do nothing, but currently it restarts > the freed coroutine. > > Now, the job->co states should probably be changed to an enum > (e.g. BEFORE_START, STARTED, YIELDED, COMPLETED) subsuming > block_job_started, job->deferred_to_main_loop and job->busy. > For now, this patch eliminates the problematic reenter by > removing the reset of job->deferred_to_main_loop (which served > no purpose, as far as I could see) and checking the flag in > block_job_enter. > > Signed-off-by: Paolo Bonzini > --- > blockjob.c | 10 -- > include/block/blockjob_int.h | 3 ++- > 2 files changed, 10 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.10 15/19] tests: crypto: add cipher speed case
On Mon, Apr 10, 2017 at 05:00:58PM +0800, Longpeng(Mike) wrote: > Now we have afalg-backend and libiary-backend, it's necessary > to add the speed test in test-crypto-cipher. > > We can use "./tests/test-crypto-cipher speed" to do the speed > test. This is not using any of the existing code for the benchmark, so there's no real benefit to having it in this file. I'd suggest creating a separate test program for performance benchmarking, eg tests/benchmark-crypto-cipher, that would be invoked separately as & when needed ie skipped by make check-unit, but run by a 'make check-speed'. Likewise for the other tests you've changed later in this series. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
[Qemu-devel] reminder: last rc for 2.9 is tomorrow
Reminder to maintainers -- I will be cutting the (hopefully) final rc for 2.9 tomorrow. Please get pull requests on the list by Tuesday 12:00 UTC at latest, preferably earlier, or let me know if there's something late-breaking that would make it worth delaying the rc a day or so. thanks -- PMM
Re: [Qemu-devel] [PATCH for 2.10] tcg: enable MTTCG by default for PPC64 on x86
Nikunj A Dadhania writes: > This enables the multi-threaded system emulation by default for PPC64 > guests using the x86_64 TCG back-end. Technically this enables it for all backends that can meet the guests default memory model requirements. So far only the x86 backend defines one as: #define TCG_TARGET_DEFAULT_MO (TCG_MO_ALL & ~TCG_MO_ST_LD) > > Signed-off-by: Nikunj A Dadhania Reviewed-by: Alex Bennée > --- > > Depends on following patch which fixes the define name: > > https://patchwork.ozlabs.org/patch/748840/ > > --- > configure| 2 ++ > target/ppc/cpu.h | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/configure b/configure > index 4b3b5cd..2a87495 100755 > --- a/configure > +++ b/configure > @@ -6008,12 +6008,14 @@ case "$target_name" in >ppc64) > TARGET_BASE_ARCH=ppc > TARGET_ABI_DIR=ppc > +mttcg=yes > gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml > power-spe.xml power-vsx.xml" >;; >ppc64le) > TARGET_ARCH=ppc64 > TARGET_BASE_ARCH=ppc > TARGET_ABI_DIR=ppc > +mttcg=yes > gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml > power-spe.xml power-vsx.xml" >;; >ppc64abi32) > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index e0ff041..ece535d 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -30,6 +30,8 @@ > #define TARGET_LONG_BITS 64 > #define TARGET_PAGE_BITS 12 > > +#define TCG_GUEST_DEFAULT_MO 0 > + > /* Note that the official physical address space bits is 62-M where M > is implementation dependent. I've not looked up M for the set of > cpus we emulate at the system level. */ -- Alex Bennée
Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
Hi
On Mon, Apr 10, 2017 at 9:33 AM Amarnath Valluri
wrote:
>
>
> On 07.04.2017 18:11, Marc-André Lureau wrote:
>
> Hi
>
> On Fri, Apr 7, 2017 at 4:41 PM Daniel P. Berrange
>
> > +.name = "data-path",
> > +.type = QEMU_OPT_STRING,
> > +.help = "Socket path to use for data exhange",
> > +},
> > +{
> > +.name = "ctrl-path",
> > +.type = QEMU_OPT_STRING,
> > +.help = "Socket path to use for out-of-band control messages",
> > +},
>
> I'm still not convinced by the need for 2 separate UNIX sockets, unless
> there's a performance reason, but that looks unlikely.
>
>
> We would also need something more than an implementation of the protocol
> that qemu is going to rely on as an external dependency. A protocol
> specification would help to start the discussion.
>
> I would agree with you, Can we adopt the current swtpm's control interface
> as initial proposal.
>
>
Perhaps, but I think it would be a mistake to rely on it without some
review.
> Alternatively, I would suggest to not rely on a public protocol,
>
> What do you mean by public protocol here, can you please help me
> understanding this.
>
>
By "public protocol", I mean qemu communication with a foreign project,
swtpm or other.
If qemu grows new needs, or if the protocol is found limited or buggy, it
may change. Subtle interactions may break between various implementations.
The minimum would be some versioning or capabilities. A document describing
the states and messages allowed/denied & effects would be quite necessary.
Otoh, there doesn't seem to be other users of this protocol, or other
implementations. So it may make sense to make it qemu-specific, and thus
"private": the protocol and implementation can evolve without risk to break
other users. This gives us a lot more flexibility and control, and doesn't
have to be very strictly documented (although it is still better to be
strict, but requires more effort).
--
Marc-André Lureau
Re: [Qemu-devel] [Qemu-block] [PATCH v2 for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate
On Fri, Apr 07, 2017 at 05:42:16PM +0200, Max Reitz wrote: > On 06.04.2017 15:04, Stefan Hajnoczi wrote: > > On Mon, Apr 03, 2017 at 06:09:33PM +0200, Max Reitz wrote: > >> -/* total size of refcount tables */ > >> -nreftablee = nrefblocke / refblock_size; > >> -nreftablee = align_offset(nreftablee, cluster_size / > >> sizeof(uint64_t)); > >> -meta_size += nreftablee * sizeof(uint64_t); > >> +/* Do not add L1 table size because the only caller of this path > >> + * (qcow2_truncate) has increased its size already. */ > >> > >> -return aligned_total_size + meta_size; > >> +/* Calculate size of the additional refblocks (this assumes that > >> all of > >> + * the existing image is covered by refblocks, which is extremely > >> + * likely); this may result in overallocation because parts of > >> the newly > >> + * added space may be covered by existing refblocks, but that is > >> fine. > >> + * > >> + * This only considers the newly added space. Since we cannot > >> update the > >> + * reftable in-place, we will have to able to store both the old > >> and the > >> + * new one at the same time, though. Therefore, we need to add > >> the size > >> + * of the old reftable here. > >> + */ > >> +creftable_length = ROUND_UP(s->refcount_table_size * > >> sizeof(uint64_t), > >> +cluster_size); > >> +nrefblocke = ((aligned_total_size - aligned_cur_size) + meta_size > >> + > >> + creftable_length + cluster_size) > >> + / (cluster_size - rces - > >> + rces * sizeof(uint64_t) / cluster_size); > >> +meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * > >> cluster_size; > >> + > >> +/* total size of the new refcount table (again, may be too much > >> because > >> + * it assumes that the new area is not covered by any refcount > >> blocks > >> + * yet) */ > >> +nreftablee = s->max_refcount_table_index + 1 + > >> + nrefblocke / refblock_size; > >> +nreftablee = align_offset(nreftablee, cluster_size / > >> sizeof(uint64_t)); > >> +meta_size += nreftablee * sizeof(uint64_t); > >> + > >> +return (aligned_total_size - aligned_cur_size) + meta_size; > > > > This calculation is an approximation. Here is a simpler approximation: > > > > current_usage = qcow2_calc_size_usage(current_size, ...); > > new_usage = qcow2_calc_size_usage(new_size, ...); > > delta = new_usage - current_usage; > > > > (Perhaps the new refcount_table clusters need to be added to new_size > > too.) > > > > Is there an advantage of the more elaborate calculation implemented in > > this patch? > > Now that you mention it... Well, the important thing is it's a > conservative approximation. It's supposed to never underestimate the > correct result. > > Now the existing image doesn't need to be fully allocated. However, your > snippet assumes that it is. Often, this actually wouldn't be an issue. > But it is for clusters that are "shared" between the existing image and > the new area: > > 1. The old final L2 table may point into the new area. Your code assumes > it is already present but it may not be. > > 2. current_size need not be aligned to clusters. If it isn't, the new > area will reuse a data cluster from the existing image that now needs to > be allocated. > > 3. Theoretically: The L1 table may be not be actually allocated. We have > to make sure it is. > > In practice: We call qcow2_grow_l1_table() before doing any > preallocation, and that always fully allocates the (new) L1 table. So > we're good. > > 4. Of course, just as always, it gets *very* funny with refcount > information. Maybe the existing image is sparsely allocated in a way > that its allocated cluster count is aligned to refblocks so that it > completely fills up all the refblocks it has (and those completely fill > up, say, one reftable cluster). Now the calculation above might assume > that we have more allocated clusters and therefore enough free entries > in the last refblock to put everything there. But when actually doing > the allocation... Surprise, you need to allocate one new refblock and a > hole new reftable because that new refblock doesn't fit into the old one. > > So if I implemented your snippet and wanted to keep conservative, I'd > have to add the following cluster counts for each of these: > > 1. 1 > 2. 1 > 3. -- > 4. 1 (refblock) + number of existing reftable clusters + 1 (resized > reftable) > > So this is not really good. We could add checks so we keep the count lower: > > 1. Check whether current_size is aligned to L2 boundaries. If it isn't, > check whether the final L2 table is allocated. If it isn't, add 1. > 2. Check whether current_size is aligned to clusters. If it isn't, check > whether the final cluster is allocated. I
Re: [Qemu-devel] [Qemu-block] [PATCH v3] migration/block:limit the time used for block migration
On Sat, Apr 08, 2017 at 06:09:16PM +0800, Paolo Bonzini wrote: > On 07/04/2017 19:33, Stefan Hajnoczi wrote: > > The migration thread is holding the QEMU global mutex, the AioContext, > > and the qcow2 s->lock while the L2 table is read from disk. > > > > The QEMU global mutex is needed for block layer operations that touch > > the global drives list. bdrv_is_allocated() can be called without the > > global mutex. > > Hi Stefan, > > only virtio-blk and virtio-scsi take the AioContext lock (because they > support dataplane). For block migration to work with devices such as > IDE, it needs to take the iothread lock too. I think there's a comment > about this in migration/block.c. Good point. :( Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.10 05/19] crypto: cipher: add cipher driver framework
On Mon, Apr 10, 2017 at 04:59:46PM +0800, Longpeng(Mike) wrote:
> 1) makes the public APIs in cipher-nettle/gcrypt/builtin static,
>and rename them with "nettle/gcrypt/builtin" prefix.
>
> 2) introduces cipher framework, including QCryptoCipherDriver
>and new public APIs.
>
> Signed-off-by: Longpeng(Mike)
> ---
> crypto/cipher-builtin.c | 59
> +
> crypto/cipher-gcrypt.c | 58 +---
> crypto/cipher-nettle.c | 59
> +
> crypto/cipher.c | 59
> +
> include/crypto/cipher.h | 22 ++
> 5 files changed, 141 insertions(+), 116 deletions(-)
>
> diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
> index bec9f41..32b6065 100644
> --- a/include/crypto/cipher.h
> +++ b/include/crypto/cipher.h
> @@ -23,6 +23,7 @@
>
> #include "qapi-types.h"
>
> +typedef struct QCryptoCipherDriver QCryptoCipherDriver;
> typedef struct QCryptoCipher QCryptoCipher;
>
> /* See also "QCryptoCipherAlgorithm" and "QCryptoCipherMode"
> @@ -76,7 +77,28 @@ typedef struct QCryptoCipher QCryptoCipher;
> *
> */
>
> +struct QCryptoCipherDriver {
> +int (*cipher_encrypt)(QCryptoCipher *cipher,
> + const void *in,
> + void *out,
> + size_t len,
> + Error **errp);
> +
> +int (*cipher_decrypt)(QCryptoCipher *cipher,
> + const void *in,
> + void *out,
> + size_t len,
> + Error **errp);
> +
> +int (*cipher_setiv)(QCryptoCipher *cipher,
> +const uint8_t *iv, size_t niv,
> +Error **errp);
> +
> +void (*cipher_free)(QCryptoCipher *cipher);
> +};
Please put this in a crypto/cipherpriv.h header file, since it is
not something we want to expose to the rest of QEMU source code.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
Re: [Qemu-devel] [PATCH for-2.10 06/19] crypto: hash: add hash driver framework
On Mon, Apr 10, 2017 at 04:59:53PM +0800, Longpeng(Mike) wrote:
> 1) makes the public APIs in hash-nettle/gcrypt/glib static,
>and rename them with "nettle/gcrypt/glib" prefix.
>
> 2) introduces hash framework, including QCryptoHashDriver
>and new public APIs.
>
> Signed-off-by: Longpeng(Mike)
> ---
> crypto/hash-gcrypt.c | 17 +++--
> crypto/hash-glib.c| 17 +++--
> crypto/hash-nettle.c | 17 +++--
> crypto/hash.c | 12
> include/crypto/hash.h | 12
> 5 files changed, 57 insertions(+), 18 deletions(-)
>
> diff --git a/include/crypto/hash.h b/include/crypto/hash.h
> index ca3267f..00b764e 100644
> --- a/include/crypto/hash.h
> +++ b/include/crypto/hash.h
> @@ -25,6 +25,18 @@
>
> /* See also "QCryptoHashAlgorithm" defined in qapi/crypto.json */
>
> +typedef struct QCryptoHashDriver QCryptoHashDriver;
> +struct QCryptoHashDriver {
> +int (*hash_bytesv)(QCryptoHashAlgorithm alg,
> + const struct iovec *iov,
> + size_t niov,
> + uint8_t **result,
> + size_t *resultlen,
> + Error **errp);
> +};
> +
> +extern QCryptoHashDriver qcrypto_hash_lib_driver;
This should be in a crypto/hashpriv.h header file, since again it is not
something we want exposed.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
Re: [Qemu-devel] [PATCH for-2.10 11/19] crypto: hmac: add hmac driver framework
On Mon, Apr 10, 2017 at 05:00:33PM +0800, Longpeng(Mike) wrote:
> 1) makes the public APIs in hmac-nettle/gcrypt/glib static,
>and rename them with "nettle/gcrypt/glib" prefix.
>
> 2) introduces hmac framework, including QCryptoHmacDriver
>and new public APIs.
>
> Signed-off-by: Longpeng(Mike)
> ---
> crypto/hmac-gcrypt.c | 48 +++
> crypto/hmac-glib.c| 70
> ++-
> crypto/hmac-nettle.c | 49 +++-
> crypto/hmac.c | 39
> include/crypto/hmac.h | 20 +++
> 5 files changed, 112 insertions(+), 114 deletions(-)
>
> diff --git a/include/crypto/hmac.h b/include/crypto/hmac.h
> index 0d3acd7..6f63eb8 100644
> --- a/include/crypto/hmac.h
> +++ b/include/crypto/hmac.h
> @@ -14,12 +14,32 @@
>
> #include "qapi-types.h"
>
> +typedef struct QCryptoHmacDriver QCryptoHmacDriver;
> typedef struct QCryptoHmac QCryptoHmac;
> +
> struct QCryptoHmac {
> +QCryptoHmacDriver *driver;
> QCryptoHashAlgorithm alg;
> void *opaque;
> };
>
> +struct QCryptoHmacDriver {
> +int (*hmac_bytesv)(QCryptoHmac *hmac,
> + const struct iovec *iov,
> + size_t niov,
> + uint8_t **result,
> + size_t *resultlen,
> + Error **errp);
> +
> +void (*hmac_free)(QCryptoHmac *hmac);
> +};
> +
> +extern void *qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
> + const uint8_t *key, size_t nkey,
> + Error **errp);
> +extern QCryptoHmacDriver qcrypto_hmac_lib_driver;
Again, we want to have a crypto/hmacpriv.h header file for this stuff.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
Re: [Qemu-devel] [PATCH v2 9/9] tpm: Added support for TPM emulator
On Mon, 2017-04-10 at 09:54 +, Marc-André Lureau wrote: > By "public protocol", I mean qemu communication with a foreign > project, swtpm or other. > > If qemu grows new needs, or if the protocol is found limited or buggy, > it may change. Subtle interactions may break between various > implementations. The minimum would be some versioning or > capabilities. A document describing the states and messages > allowed/denied & effects would be quite necessary. Stefan, is there any documentation besides the source? Just asking, I don't think it is needed because... > Otoh, there doesn't seem to be other users of this protocol, or other > implementations. So it may make sense to make it qemu-specific, and > thus "private": the protocol and implementation can evolve without > risk to break other users. This gives us a lot more flexibility and > control, and doesn't have to be very strictly documented (although it > is still better to be strict, but requires more effort). ... I suspect it falls into this camp. I can't think of any users of the protocol besides swtpm itself and now qemu. Stefan, is that correct? -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter.
Re: [Qemu-devel] [PATCH for-2.10 12/19] socket: add af_alg family support
On Mon, Apr 10, 2017 at 05:00:40PM +0800, Longpeng(Mike) wrote:
> The AF_ALG socket family is the userspace interface for linux
> crypto API, this patch adds af_alg family support. It'll be used
> by afalg-backend crypto later.
>
> Signed-off-by: Longpeng(Mike)
> ---
> configure | 21
> include/qemu/sockets.h | 6
> qapi-schema.json | 21 +++-
> util/qemu-sockets.c| 91
> ++
> 4 files changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 4b3b5cd..970c9bc 100755
> --- a/configure
> +++ b/configure
> @@ -4737,6 +4737,23 @@ if compile_prog "" "" ; then
> have_af_vsock=yes
> fi
>
> +##
> +# check for usable AF_ALG environment
> +hava_af_alg=no
> +cat > $TMPC << EOF
> +#include
> +#include
> +#include
> +int main(void) {
> +int sock;
> +sock = socket(AF_ALG, SOCK_SEQPACKET, 0);
> +return sock;
> +}
> +EOF
> +if compile_prog "" "" ; then
> +have_af_alg=yes
> +fi
> +
> #
> # Sparc implicitly links with --relax, which is
> # incompatible with -r, so --no-relax should be
> @@ -5767,6 +5784,10 @@ if test "$have_af_vsock" = "yes" ; then
>echo "CONFIG_AF_VSOCK=y" >> $config_host_mak
> fi
>
> +if test "$have_af_alg" = "yes" ; then
> + echo "CONFIG_AF_ALG=y" >> $config_host_mak
> +fi
> +
> if test "$have_sysmacros" = "yes" ; then
>echo "CONFIG_SYSMACROS=y" >> $config_host_mak
> fi
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 7842f6d..0a4a003 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -51,6 +51,12 @@ int socket_listen(SocketAddress *addr, Error **errp);
> void socket_listen_cleanup(int fd, Error **errp);
> int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
>
> +#ifdef CONFIG_AF_ALG
> +#define SALG_TYPE_LEN_MAX 14
> +#define SALG_NAME_LEN_MAX 64
> +int socket_bind(SocketAddress *addr, Error **errp);
> +#endif
> +
> /* Old, ipv4 only bits. Don't use for new code. */
> int parse_host_port(struct sockaddr_in *saddr, const char *str);
> int socket_init(void);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 250e4dc..0cb06d3 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1516,12 +1516,14 @@
> #
> # @vsock: vsock family (since 2.8)
> #
> +# @afalg: af_alg family (since 2.10)
> +#
> # @unknown: otherwise
> #
> # Since: 2.1
> ##
> { 'enum': 'NetworkAddressFamily',
> - 'data': [ 'ipv4', 'ipv6', 'unix', 'vsock', 'unknown' ] }
> + 'data': [ 'ipv4', 'ipv6', 'unix', 'vsock', 'afalg', 'unknown' ] }
>
> ##
> # @VncBasicInfo:
> @@ -4119,6 +4121,22 @@
> 'port': 'str' } }
>
> ##
> +# @AfalgSocketAddress:
> +#
> +# Captures a socket address in the af_alg namespace.
> +#
> +# @type: type of the crypto algogrithms
> +#
> +# @name: name of the crypto algogrithms
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'AfalgSocketAddress',
> + 'data': {
> +'type': 'str',
> +'name': 'str' }}
> +
> +##
> # @SocketAddress:
> #
> # Captures the address of a socket, which could also be a named file
> descriptor
> @@ -4130,6 +4148,7 @@
> 'inet': 'InetSocketAddress',
> 'unix': 'UnixSocketAddress',
> 'vsock': 'VsockSocketAddress',
> +'afalg': 'AfalgSocketAddress',
> 'fd': 'String' } }
>
> ##
I really don't think we want to expose any of this in the qapi-schema. It is
a Linux specific internal implementation detail that is not relevant to users
of QAPI.
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 21442c3..258e419 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1151,6 +1151,97 @@ void socket_listen_cleanup(int fd, Error **errp)
> qapi_free_SocketAddress(addr);
> }
>
> +#ifdef CONFIG_AF_ALG
> +
> +#include
> +
> +static bool afalg_parse_bind_saddr(const AfalgSocketAddress *saddr,
> + struct sockaddr_alg *alg,
> + Error **errp)
> +{
> +memset(alg, 0, sizeof(*alg));
> +alg->salg_family = AF_ALG;
> +
> +if (qemu_strnlen(saddr->type, SALG_TYPE_LEN_MAX) == SALG_TYPE_LEN_MAX) {
> +error_setg(errp, "Afalg type(%s) is larger than 14 bytes",
> + saddr->type);
> +return false;
> +}
> +
> +if (qemu_strnlen(saddr->name, SALG_NAME_LEN_MAX) == SALG_NAME_LEN_MAX) {
> +error_setg(errp, "Afalg name(%s) is larger than 64 bytes",
> + saddr->name);
> +return false;
> +}
> +
> +pstrcpy((char *)alg->salg_type, SALG_TYPE_LEN_MAX, saddr->type);
> +pstrcpy((char *)alg->salg_name, SALG_NAME_LEN_MAX, saddr->name);
> +
> +return true;
> +}
> +
> +static int afalg_bind_saddr(const AfalgSocketAddress *saddr,
> +Error **errp)
> +{
> +struct sockaddr_alg alg;
> +int sbind;
> +
> +if (!afalg_parse_bi
Re: [Qemu-devel] [PATCH for-2.10 13/19] crypto: introduce some common functions for af_alg backend
On Mon, Apr 10, 2017 at 05:00:47PM +0800, Longpeng(Mike) wrote:
> This patch introduces some common functions for af_alg backend,
> they would be used in af_alg-backend cipher/hash/hmac latter.
>
> Signed-off-by: Longpeng(Mike)
> ---
> crypto/Makefile.objs| 1 +
> crypto/afalg-comm.c | 71
> +
> include/crypto/afalg-comm.h | 61 ++
> 3 files changed, 133 insertions(+)
> create mode 100644 crypto/afalg-comm.c
> create mode 100644 include/crypto/afalg-comm.h
>
> diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
> index 1f749f2..6f244a3 100644
> --- a/crypto/Makefile.objs
> +++ b/crypto/Makefile.objs
> @@ -10,6 +10,7 @@ crypto-obj-$(if $(CONFIG_NETTLE),n,$(if
> $(CONFIG_GCRYPT_HMAC),n,y)) += hmac-glib
> crypto-obj-y += aes.o
> crypto-obj-y += desrfb.o
> crypto-obj-y += cipher.o
> +crypto-obj-$(CONFIG_AF_ALG) += afalg-comm.o
> crypto-obj-y += tlscreds.o
> crypto-obj-y += tlscredsanon.o
> crypto-obj-y += tlscredsx509.o
> diff --git a/crypto/afalg-comm.c b/crypto/afalg-comm.c
> new file mode 100644
> index 000..27bc88c
> --- /dev/null
> +++ b/crypto/afalg-comm.c
Just calls this crypto/afalg.c
> +void afalg_comm_format_type(AfalgSocketAddress *afalg,
> +const char *type)
> +{
> +afalg->type = (char *)g_new0(int8_t, SALG_TYPE_LEN_MAX);
> +pstrcpy(afalg->type, SALG_TYPE_LEN_MAX, type);
> +}
Needs an 'Error **errp' for reporting if 'type' is too long
> diff --git a/include/crypto/afalg-comm.h b/include/crypto/afalg-comm.h
> new file mode 100644
> index 000..b6b9464
> --- /dev/null
> +++ b/include/crypto/afalg-comm.h
> @@ -0,0 +1,61 @@
> +/*
> + * QEMU Crypto af_alg support
> + *
> + * Copyright (c) 2017 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *Longpeng(Mike)
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version. See the COPYING file in the
> + * top-level directory.
> + */
> +#ifndef QCRYPTO_AFALG_H
> +#define QCRYPTO_AFALG_H
> +
> +#include "qapi-types.h"
> +
> +#ifndef SOL_ALG
> +#define SOL_ALG 279
> +#endif
This can be in the .c file as I don't se it used elsewhere.
> +
> +typedef struct QCryptoAfalg QCryptoAfalg;
> +struct QCryptoAfalg {
Name it 'QCryptoAFAlg' since the letters 'af' are an abbreviation.
> +int tfmfd;
> +int opfd;
> +struct msghdr *msg;
> +struct cmsghdr *cmsg;
> +};
> +
> +
> +/**
> + * afalg_comm_format_type:
> + * @afalg: the AfalgSocketAddress object
> + * @type: the type of crypto alg.
> + *
> + * Set the type field of the @afalg according to @type.
> + */
> +void afalg_comm_format_type(AfalgSocketAddress *afalg,
> +const char *type);
> +
> +/**
> + * afalg_comm_alloc:
> + * @saddr: the SocketAddress object
> + *
> + * Allocate a QCryptoAfalg object and bind itself to
> + * a AF_ALG socket.
> + *
> + * Returns:
> + * a new QCryptoAfalg object, or NULL in error.
> + */
> +QCryptoAfalg *afalg_comm_alloc(SocketAddress *saddr);
> +
> +/**
> + * afalg_comm_free:
> + * @afalg: the QCryptoAfalg object
> + *
> + * Free the @afalg.
> + */
> +void afalg_comm_free(QCryptoAfalg *afalg);
> +
> +#endif
This is all for internal use by the crypto/ code, so should be in a header
crypto/afalgpriv.h
> --
> 1.8.3.1
>
>
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
Re: [Qemu-devel] [PATCH for-2.10 14/19] crypto: cipher: add af_alg cipher support
On Mon, Apr 10, 2017 at 05:00:52PM +0800, Longpeng(Mike) wrote:
> Adds afalg-backend cipher support: introduces some private APIs
> firstly, and then intergrates them into qcrypto_cipher_afalg_driver.
>
> Signed-off-by: Longpeng(Mike)
> ---
> crypto/Makefile.objs| 1 +
> crypto/cipher-afalg.c | 229
>
> crypto/cipher.c | 30 +-
> include/crypto/afalg-comm.h | 11 +++
> include/crypto/cipher.h | 7 ++
> tests/test-crypto-cipher.c | 10 +-
> 6 files changed, 286 insertions(+), 2 deletions(-)
> create mode 100644 crypto/cipher-afalg.c
>
> diff --git a/crypto/cipher-afalg.c b/crypto/cipher-afalg.c
> new file mode 100644
> index 000..2da972c
> --- /dev/null
> +++ b/crypto/cipher-afalg.c
> @@ -0,0 +1,229 @@
> +/*
> + * QEMU Crypto af_alg-backend cipher support
> + *
> + * Copyright (c) 2017 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *Longpeng(Mike)
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version. See the COPYING file in the
> + * top-level directory.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu/sockets.h"
> +#include "qemu-common.h"
> +#include "qapi/error.h"
> +#include "crypto/cipher.h"
> +#include "crypto/afalg-comm.h"
> +#include
> +
> +static int afalg_cipher_format_name(QCryptoCipherAlgorithm alg,
> + QCryptoCipherMode mode,
> + AfalgSocketAddress *afalg)
> +{
> +const char *alg_name = NULL;
> +const char *mode_name = NULL;
> +
> +switch (alg) {
> +case QCRYPTO_CIPHER_ALG_AES_128:
> +case QCRYPTO_CIPHER_ALG_AES_192:
> +case QCRYPTO_CIPHER_ALG_AES_256:
> +alg_name = "aes";
> +break;
> +case QCRYPTO_CIPHER_ALG_CAST5_128:
> +alg_name = "cast5";
> +break;
> +case QCRYPTO_CIPHER_ALG_SERPENT_128:
> +case QCRYPTO_CIPHER_ALG_SERPENT_192:
> +case QCRYPTO_CIPHER_ALG_SERPENT_256:
> +alg_name = "serpent";
> +break;
> +case QCRYPTO_CIPHER_ALG_TWOFISH_128:
> +case QCRYPTO_CIPHER_ALG_TWOFISH_192:
> +case QCRYPTO_CIPHER_ALG_TWOFISH_256:
> +alg_name = "twofish";
> +break;
> +
> +default:
> +return -1;
> +}
You should pass Error **errp into this method & set error in
the default case.
> +
> +mode_name = QCryptoCipherMode_lookup[mode];
> +afalg->name = (char *)g_new0(int8_t, SALG_NAME_LEN_MAX);
> +sprintf(afalg->name, "%s(%s)", mode_name, alg_name);
You're printing into a fixed length buffer here with no bounds
checking. Please use snprintf and report an error if it is too
large to fit.
> +
> +return 0;
> +}
> +
> +QCryptoAfalg *afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
> + QCryptoCipherMode mode,
> + const uint8_t *key,
> + size_t nkey, Error **errp)
> +{
> +SocketAddress *saddr = NULL;
> +QCryptoAfalg *afalg = NULL;
> +size_t except_niv = 0;
> +int ret = 0;
> +
> +saddr = g_new0(SocketAddress, 1);
> +saddr->u.afalg.data = g_new0(AfalgSocketAddress, 1);
> +saddr->type = SOCKET_ADDRESS_KIND_AFALG;
> +ret = afalg_cipher_format_name(alg, mode, saddr->u.afalg.data);
> +if (ret != 0) {
> +error_setg(errp, "Unsupported cipher mode %s",
> + QCryptoCipherMode_lookup[mode]);
> +goto error;
> +}
> +afalg_comm_format_type(saddr->u.afalg.data, ALG_TYPE_CIPHER);
> +
> +afalg = afalg_comm_alloc(saddr);
> +if (!afalg) {
> +error_setg(errp, "Alloc QCryptoAfalg object failed");
The afalg_comm_alloc() function should take an 'Error **errp' so it can
report a more specific message.
> +goto error;
> +}
> +
> +/* setkey */
> +ret = qemu_setsockopt(afalg->tfmfd, SOL_ALG, ALG_SET_KEY, key,
> + nkey);
> +if (ret != 0) {
> +error_setg(errp, "Afalg setkey failed");
error_setg_errno()
> +goto error;
> +}
> +
> +/* prepare msg header */
> +afalg->msg = g_new0(struct msghdr, 1);
> +afalg->msg->msg_controllen += CMSG_SPACE(ALG_OPTYPE_LEN);
> +except_niv = qcrypto_cipher_get_iv_len(alg, mode);
> +if (except_niv) {
> +afalg->msg->msg_controllen += CMSG_SPACE(ALG_MSGIV_LEN(except_niv));
> +}
> +afalg->msg->msg_control = g_new0(uint8_t, afalg->msg->msg_controllen);
> +
> +/* We use 1st msghdr for crypto-info and 2nd msghdr for IV-info */
> +afalg->cmsg = CMSG_FIRSTHDR(afalg->msg);
> +afalg->cmsg->cmsg_level = SOL_ALG;
> +afalg->cmsg->cmsg_type = ALG_SET_OP;
> +afalg->cmsg->cmsg_len = CMSG_SPACE(ALG_OPTYPE_LEN);
> +
> +cleanup:
> +g_free(saddr->u.afalg.data->type);
> +g_free(saddr->u.afalg.data->name);
> +g_free(saddr->u.afalg.data);
> +g_free(saddr);
> +return afalg;
> +
> +error:
> +afalg_comm_free(afalg);
> +
Re: [Qemu-devel] [PATCH for-2.10 16/19] crypto: hash: add af_alg hash support
On Mon, Apr 10, 2017 at 05:01:05PM +0800, Longpeng(Mike) wrote:
> Adds afalg-backend hash support: introduces some private APIs
> firstly, and then intergrates them into qcrypto_hash_afalg_driver.
>
> Signed-off-by: Longpeng(Mike)
> ---
> crypto/Makefile.objs| 1 +
> crypto/hash-afalg.c | 150
>
> crypto/hash.c | 16 -
> include/crypto/afalg-comm.h | 1 +
> include/crypto/hash.h | 1 +
> 5 files changed, 166 insertions(+), 3 deletions(-)
> create mode 100644 crypto/hash-afalg.c
>
> +static int afalg_hash_format_name(QCryptoHashAlgorithm alg,
> + AfalgSocketAddress *afalg)
> +{
> +const char *alg_name = NULL;
> +
> +switch (alg) {
> +case QCRYPTO_HASH_ALG_MD5:
> +alg_name = "md5";
> +break;
> +case QCRYPTO_HASH_ALG_SHA1:
> +alg_name = "sha1";
> +break;
> +case QCRYPTO_HASH_ALG_SHA224:
> +alg_name = "sha224";
> +break;
> +case QCRYPTO_HASH_ALG_SHA256:
> +alg_name = "sha256";
> +break;
> +case QCRYPTO_HASH_ALG_SHA384:
> +alg_name = "sha384";
> +break;
> +case QCRYPTO_HASH_ALG_SHA512:
> +alg_name = "sha512";
> +break;
> +case QCRYPTO_HASH_ALG_RIPEMD160:
> +alg_name = "rmd160";
> +break;
> +
> +default:
> +return -1;
> +}
> +
> +afalg->name = (char *)g_new0(int8_t, SALG_NAME_LEN_MAX);
> +sprintf(afalg->name, "%s", alg_name);
Another printf without any bounds checking.
> +
> +return 0;
> +}
> +
> +static QCryptoAfalg *afalg_hash_ctx_new(QCryptoHashAlgorithm alg)
> +{
> +SocketAddress *saddr = NULL;
> +QCryptoAfalg *afalg = NULL;
> +int ret = 0;
> +
> +saddr = g_new0(SocketAddress, 1);
> +saddr->u.afalg.data = g_new0(AfalgSocketAddress, 1);
> +saddr->type = SOCKET_ADDRESS_KIND_AFALG;
> +ret = afalg_hash_format_name(alg, saddr->u.afalg.data);
> +if (ret != 0) {
> +goto error;
> +}
> +afalg_comm_format_type(saddr->u.afalg.data, ALG_TYPE_HASH);
> +
> +afalg = afalg_comm_alloc(saddr);
> +if (!afalg) {
> +goto error;
> +}
> +
> +/* prepare msg header */
> +afalg->msg = g_new0(struct msghdr, 1);
> +
> +cleanup:
> +g_free(saddr->u.afalg.data->type);
> +g_free(saddr->u.afalg.data->name);
> +g_free(saddr->u.afalg.data);
> +g_free(saddr);
> +return afalg;
> +
> +error:
> +afalg_comm_free(afalg);
> +afalg = NULL;
> +goto cleanup;
> +}
> +
> +static int afalg_hash_bytesv(QCryptoHashAlgorithm alg,
> + const struct iovec *iov,
> + size_t niov, uint8_t **result,
> + size_t *resultlen,
> + Error **errp)
> +{
> +QCryptoAfalg *afalg = NULL;
> +struct iovec outv;
> +int ret = 0;
> +const int except_len = qcrypto_hash_digest_len(alg);
> +
> +if (*resultlen == 0) {
> +*resultlen = except_len;
> +*result = g_new0(uint8_t, *resultlen);
> +} else if (*resultlen != except_len) {
> +error_setg(errp,
> + "Result buffer size %zu is not match hash %d",
> + *resultlen, except_len);
> +return -1;
> +}
> +
> +afalg = afalg_hash_ctx_new(alg);
> +if (afalg == NULL) {
> +error_setg(errp, "Alloc QCryptoAfalg object failed");
Make afalg_hash_ctx_new() report the error
> +return -1;
> +}
> +
> +/* send data to kernel's crypto core */
> +ret = iov_send_recv(afalg->opfd, iov, niov,
> +0, iov_size(iov, niov), true);
> +if (ret < 0) {
> +error_setg(errp, "Send data to afalg-core failed");
error_setg_errno()
> +goto out;
> +}
> +
> +/* hash && get result */
> +outv.iov_base = *result;
> +outv.iov_len = *resultlen;
> +afalg->msg->msg_iov = &outv;
> +afalg->msg->msg_iovlen = 1;
> +ret = recvmsg(afalg->opfd, afalg->msg, 0);
> +if (ret != -1) {
> +ret = 0;
> +} else {
> +error_setg(errp, "Recv result from afalg-core failed");
> +}
> +
> +out:
> +afalg_comm_free(afalg);
> +return ret;
> +}
> +
> +QCryptoHashDriver qcrypto_hash_afalg_driver = {
> +.hash_bytesv = afalg_hash_bytesv,
> +};
All methods should have a qcrypto_afalg_ name prefix.
> diff --git a/crypto/hash.c b/crypto/hash.c
> index 0b0d479..002622c 100644
> --- a/crypto/hash.c
> +++ b/crypto/hash.c
> @@ -45,9 +45,19 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
> size_t *resultlen,
> Error **errp)
> {
> -return qcrypto_hash_lib_driver.hash_bytesv(alg, iov, niov,
> - result, resultlen,
> - errp);
> +int ret;
> +
> +ret = qcrypto_hash_afalg_driver.hash_bytesv(alg, iov, niov,
> +
Re: [Qemu-devel] [PATCH] Removed trailing newline from error_report()
On Sat, Apr 08, 2017 at 11:09:47AM +0530, Ishani Chugh wrote: > Signed-off-by: Ishani Chugh > --- > target/arm/kvm64.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Thanks for the patch! I have CCed Peter Maydell who maintains this source file. I also added the trivial patches mailing list - a patch queue for small patches. Conventially QEMU commit messages are written like this: target-arm: remove trailing newline from error_report() The "target-arm:" prefix names the subsystem that is affected. It makes it easier for maintainer or people searching through patches to find relevant results. The commit message is written in the present imperative - "remove X" instead of "removed X". You do not need to resend the patch. The maintainer can fix up the commit message if they like. Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.10 15/19] tests: crypto: add cipher speed case
Hi Daniel, Ok, I'll take all your suggestions, and fix them in next version. Thanks. On 2017/4/10 17:51, Daniel P. Berrange wrote: > On Mon, Apr 10, 2017 at 05:00:58PM +0800, Longpeng(Mike) wrote: >> Now we have afalg-backend and libiary-backend, it's necessary >> to add the speed test in test-crypto-cipher. >> >> We can use "./tests/test-crypto-cipher speed" to do the speed >> test. > > This is not using any of the existing code for the benchmark, so there's > no real benefit to having it in this file. > > I'd suggest creating a separate test program for performance benchmarking, > eg tests/benchmark-crypto-cipher, that would be invoked separately as & > when needed ie skipped by make check-unit, but run by a 'make check-speed'. > > Likewise for the other tests you've changed later in this series. > > > Regards, > Daniel -- Regards, Longpeng(Mike)
[Qemu-devel] [PATCH v2] qxl: add migration blocker to avoid pre-save assert
Cc: 1635...@bugs.launchpad.net
Signed-off-by: Gerd Hoffmann
---
hw/display/qxl.h | 1 +
hw/display/qxl.c | 28
2 files changed, 29 insertions(+)
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index d2d49dd..77e5a36 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -40,6 +40,7 @@ typedef struct PCIQXLDevice {
uint32_t cmdlog;
uint32_t guest_bug;
+Error *migration_blocker;
enum qxl_mode mode;
uint32_t cmdflags;
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index c31b293..c1f830c 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -26,6 +26,7 @@
#include "qemu/queue.h"
#include "qemu/atomic.h"
#include "sysemu/sysemu.h"
+#include "migration/migration.h"
#include "trace.h"
#include "qxl.h"
@@ -639,6 +640,27 @@ static int interface_get_command(QXLInstance *sin, struct
QXLCommandExt *ext)
qxl->guest_primary.commands++;
qxl_track_command(qxl, ext);
qxl_log_command(qxl, "cmd", ext);
+{
+/*
+ * Windows 8 drivers place qxl commands in the vram
+ * (instead of the ram) bar. We can't live migrate such a
+ * guest, so add a migration blocker in case we detect
+ * this, to avoid triggering the assert in pre_save().
+ *
+ *
https://cgit.freedesktop.org/spice/win32/qxl-wddm-dod/commit/?id=f6e099db39e7d0787f294d5fd0dce328b5210faa
+ */
+void *msg = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+if (msg != NULL && (
+msg < (void *)qxl->vga.vram_ptr ||
+msg > ((void *)qxl->vga.vram_ptr + qxl->vga.vram_size))) {
+if (!qxl->migration_blocker) {
+Error *local_err = NULL;
+error_setg(&qxl->migration_blocker,
+ "qxl: guest bug: command not in ram bar");
+migrate_add_blocker(qxl->migration_blocker, &local_err);
+}
+}
+}
trace_qxl_ring_command_get(qxl->id, qxl_mode_to_string(qxl->mode));
return true;
default:
@@ -1236,6 +1258,12 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
qemu_spice_create_host_memslot(&d->ssd);
qxl_soft_reset(d);
+if (d->migration_blocker) {
+migrate_del_blocker(d->migration_blocker);
+error_free(d->migration_blocker);
+d->migration_blocker = NULL;
+}
+
if (startstop) {
qemu_spice_display_start();
}
--
2.9.3
Re: [Qemu-devel] [PATCH v2] qxl: add migration blocker to avoid pre-save assert
Hi
On Mon, Apr 10, 2017 at 12:27 PM Gerd Hoffmann wrote:
> Cc: 1635...@bugs.launchpad.net
> Signed-off-by: Gerd Hoffmann
>
---
> hw/display/qxl.h | 1 +
> hw/display/qxl.c | 28
> 2 files changed, 29 insertions(+)
>
> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> index d2d49dd..77e5a36 100644
> --- a/hw/display/qxl.h
> +++ b/hw/display/qxl.h
> @@ -40,6 +40,7 @@ typedef struct PCIQXLDevice {
> uint32_t cmdlog;
>
> uint32_t guest_bug;
> +Error *migration_blocker;
>
> enum qxl_mode mode;
> uint32_t cmdflags;
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index c31b293..c1f830c 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -26,6 +26,7 @@
> #include "qemu/queue.h"
> #include "qemu/atomic.h"
> #include "sysemu/sysemu.h"
> +#include "migration/migration.h"
> #include "trace.h"
>
> #include "qxl.h"
> @@ -639,6 +640,27 @@ static int interface_get_command(QXLInstance *sin,
> struct QXLCommandExt *ext)
> qxl->guest_primary.commands++;
> qxl_track_command(qxl, ext);
> qxl_log_command(qxl, "cmd", ext);
> +{
> +/*
> + * Windows 8 drivers place qxl commands in the vram
> + * (instead of the ram) bar. We can't live migrate such a
> + * guest, so add a migration blocker in case we detect
> + * this, to avoid triggering the assert in pre_save().
> + *
> + *
> https://cgit.freedesktop.org/spice/win32/qxl-wddm-dod/commit/?id=f6e099db39e7d0787f294d5fd0dce328b5210faa
> + */
> +void *msg = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
> +if (msg != NULL && (
> +msg < (void *)qxl->vga.vram_ptr ||
> +msg > ((void *)qxl->vga.vram_ptr +
> qxl->vga.vram_size))) {
> +if (!qxl->migration_blocker) {
> +Error *local_err = NULL;
> +error_setg(&qxl->migration_blocker,
> + "qxl: guest bug: command not in ram bar");
> +migrate_add_blocker(qxl->migration_blocker,
> &local_err);
>
What do you do with the local_err? error_report_err() perhaps ?
> +}
> +}
> +}
> trace_qxl_ring_command_get(qxl->id,
> qxl_mode_to_string(qxl->mode));
> return true;
> default:
> @@ -1236,6 +1258,12 @@ static void qxl_hard_reset(PCIQXLDevice *d, int
> loadvm)
> qemu_spice_create_host_memslot(&d->ssd);
> qxl_soft_reset(d);
>
> +if (d->migration_blocker) {
> +migrate_del_blocker(d->migration_blocker);
> +error_free(d->migration_blocker);
> +d->migration_blocker = NULL;
> +}
> +
> if (startstop) {
> qemu_spice_display_start();
> }
> --
> 2.9.3
>
> --
Marc-André Lureau
[Qemu-devel] [Bug 1681398] [NEW] hw/core: segmentation fault
Public bug reported: Reproducer: $i386-softmmu/qemu-system-i386 -S -machine isapc,accel=tcg -device amd-iommu Segmentation fault (core dumped) Partial bt: #0 bus_add_child (child=0x56d4e520, bus=0x0) at hw/core/qdev.c:88 #1 qdev_set_parent_bus (dev=0x56d4e520, bus=bus@entry=0x0) at hw/core/qdev.c:119 ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1681398 Title: hw/core: segmentation fault Status in QEMU: New Bug description: Reproducer: $i386-softmmu/qemu-system-i386 -S -machine isapc,accel=tcg -device amd-iommu Segmentation fault (core dumped) Partial bt: #0 bus_add_child (child=0x56d4e520, bus=0x0) at hw/core/qdev.c:88 #1 qdev_set_parent_bus (dev=0x56d4e520, bus=bus@entry=0x0) at hw/core/qdev.c:119 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1681398/+subscriptions
[Qemu-devel] [PATCH 7/7] arm: Remove workarounds for old M-profile exception return implementation
Now that we've rewritten M-profile exception return so that the magic
PC values are not visible to other parts of QEMU, we can delete the
special casing of them elsewhere.
Signed-off-by: Peter Maydell
---
target/arm/cpu.c | 43 ++-
target/arm/translate.c | 8
2 files changed, 2 insertions(+), 49 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 04b062c..b357aee 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -304,33 +304,6 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
}
#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
-static void arm_v7m_unassigned_access(CPUState *cpu, hwaddr addr,
- bool is_write, bool is_exec, int opaque,
- unsigned size)
-{
-ARMCPU *arm = ARM_CPU(cpu);
-CPUARMState *env = &arm->env;
-
-/* ARMv7-M interrupt return works by loading a magic value into the PC.
- * On real hardware the load causes the return to occur. The qemu
- * implementation performs the jump normally, then does the exception
- * return by throwing a special exception when when the CPU tries to
- * execute code at the magic address.
- */
-if (env->v7m.exception != 0 && addr >= 0xfff0 && is_exec) {
-cpu->exception_index = EXCP_EXCEPTION_EXIT;
-cpu_loop_exit(cpu);
-}
-
-/* In real hardware an attempt to access parts of the address space
- * with nothing there will usually cause an external abort.
- * However our QEMU board models are often missing device models where
- * the guest can boot anyway with the default read-as-zero/writes-ignored
- * behaviour that you get without a QEMU unassigned_access hook.
- * So just return here to retain that default behaviour.
- */
-}
-
static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
CPUClass *cc = CPU_GET_CLASS(cs);
@@ -338,17 +311,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
CPUARMState *env = &cpu->env;
bool ret = false;
-/* ARMv7-M interrupt return works by loading a magic value
- * into the PC. On real hardware the load causes the
- * return to occur. The qemu implementation performs the
- * jump normally, then does the exception return when the
- * CPU tries to execute code at the magic address.
- * This will cause the magic PC value to be pushed to
- * the stack if an interrupt occurred at the wrong time.
- * We avoid this by disabling interrupts when
- * pc contains a magic address.
- *
- * ARMv7-M interrupt masking works differently than -A or -R.
+/* ARMv7-M interrupt masking works differently than -A or -R.
* There is no FIQ/IRQ distinction. Instead of I and F bits
* masking FIQ and IRQ interrupts, an exception is taken only
* if it is higher priority than the current execution priority
@@ -356,8 +319,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
* currently active exception).
*/
if (interrupt_request & CPU_INTERRUPT_HARD
-&& (armv7m_nvic_can_take_pending_exception(env->nvic))
-&& (env->regs[15] < 0xfff0)) {
+&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
cs->exception_index = EXCP_IRQ;
cc->do_interrupt(cs);
ret = true;
@@ -1091,7 +1053,6 @@ static void arm_v7m_class_init(ObjectClass *oc, void
*data)
cc->do_interrupt = arm_v7m_cpu_do_interrupt;
#endif
-cc->do_unassigned_access = arm_v7m_unassigned_access;
cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt;
}
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 156ab46..c85bc6c 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -11914,14 +11914,6 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
dc->is_jmp = DISAS_EXC;
break;
}
-#else
-if (arm_dc_feature(dc, ARM_FEATURE_M)) {
-/* Branches to the magic exception-return addresses should
- * already have been caught via the arm_v7m_unassigned_access hook,
- * and never get here.
- */
-assert(dc->pc < 0xfff0);
-}
#endif
if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) {
--
2.7.4
[Qemu-devel] [PATCH 2/7] arm: Thumb shift operations should not permit interworking branches
In Thumb mode, the only instructions which can cause an interworking branch by writing the PC are BLX, BX, BXJ, LDR, POP and LDM. Unlike ARM mode, data processing instructions which target the PC do not cause interworking branches. When we added support for doing interworking branches on writes to PC from data processing instructions in commit 21aeb3430ce7ba, we accidentally changed a Thumb instruction to have interworking branch behaviour for writes to PC. (MOV, MOVS register-shifted register, encoding T2; this is the standard encoding for LSL/LSR/ASR/ROR (register).) For this encoding, behaviour with Rd == R15 is specified as UNPREDICTABLE, so allowing an interworking branch is within spec, but it's confusing and differs from our handling of this class of UNPREDICTABLE for other Thumb ALU operations. Make it perform a simple (non-interworking) branch like the others. Signed-off-by: Peter Maydell --- target/arm/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index fe3f442..ddc62b6 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -9959,7 +9959,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw gen_arm_shift_reg(tmp, op, tmp2, logic_cc); if (logic_cc) gen_logic_CC(tmp); -store_reg_bx(s, rd, tmp); +store_reg(s, rd, tmp); break; case 1: /* Sign/zero extend. */ op = (insn >> 20) & 7; -- 2.7.4
[Qemu-devel] [PATCH] arm: Move excnames[] array into arm_log_exceptions()
The excnames[] array is defined in internals.h because we used
to use it from two different source files for handling logging
of AArch32 and AArch64 exception entry. Refactoring means that
it's now used only in arm_log_exception() in helper.c, so move
the array into that function.
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Peter Maydell
---
target/arm/cpu.h | 2 +-
target/arm/internals.h | 23 ---
target/arm/helper.c| 19 +++
3 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e6f05e2..ab86943 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -58,7 +58,7 @@
#define EXCP_SEMIHOST 16 /* semihosting call */
#define EXCP_NOCP 17 /* v7M NOCP UsageFault */
#define EXCP_INVSTATE 18 /* v7M INVSTATE UsageFault */
-/* NB: new EXCP_ defines should be added to the excnames[] array too */
+/* NB: add new EXCP_ defines to the array in arm_log_exception() too */
#define ARMV7M_EXCP_RESET 1
#define ARMV7M_EXCP_NMI 2
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 97ca034..1f6efef 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -51,29 +51,6 @@ static inline bool excp_is_internal(int excp)
|| excp == EXCP_SEMIHOST;
}
-/* Exception names for debug logging; note that not all of these
- * precisely correspond to architectural exceptions.
- */
-static const char * const excnames[] = {
-[EXCP_UDEF] = "Undefined Instruction",
-[EXCP_SWI] = "SVC",
-[EXCP_PREFETCH_ABORT] = "Prefetch Abort",
-[EXCP_DATA_ABORT] = "Data Abort",
-[EXCP_IRQ] = "IRQ",
-[EXCP_FIQ] = "FIQ",
-[EXCP_BKPT] = "Breakpoint",
-[EXCP_EXCEPTION_EXIT] = "QEMU v7M exception exit",
-[EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
-[EXCP_HVC] = "Hypervisor Call",
-[EXCP_HYP_TRAP] = "Hypervisor Trap",
-[EXCP_SMC] = "Secure Monitor Call",
-[EXCP_VIRQ] = "Virtual IRQ",
-[EXCP_VFIQ] = "Virtual FIQ",
-[EXCP_SEMIHOST] = "Semihosting call",
-[EXCP_NOCP] = "v7M NOCP UsageFault",
-[EXCP_INVSTATE] = "v7M INVSTATE UsageFault",
-};
-
/* Scale factor for generic timers, ie number of ns per tick.
* This gives a 62.5MHz timer.
*/
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 8cb7a94..8a3e448 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6271,6 +6271,25 @@ static void arm_log_exception(int idx)
{
if (qemu_loglevel_mask(CPU_LOG_INT)) {
const char *exc = NULL;
+static const char * const excnames[] = {
+[EXCP_UDEF] = "Undefined Instruction",
+[EXCP_SWI] = "SVC",
+[EXCP_PREFETCH_ABORT] = "Prefetch Abort",
+[EXCP_DATA_ABORT] = "Data Abort",
+[EXCP_IRQ] = "IRQ",
+[EXCP_FIQ] = "FIQ",
+[EXCP_BKPT] = "Breakpoint",
+[EXCP_EXCEPTION_EXIT] = "QEMU v7M exception exit",
+[EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
+[EXCP_HVC] = "Hypervisor Call",
+[EXCP_HYP_TRAP] = "Hypervisor Trap",
+[EXCP_SMC] = "Secure Monitor Call",
+[EXCP_VIRQ] = "Virtual IRQ",
+[EXCP_VFIQ] = "Virtual FIQ",
+[EXCP_SEMIHOST] = "Semihosting call",
+[EXCP_NOCP] = "v7M NOCP UsageFault",
+[EXCP_INVSTATE] = "v7M INVSTATE UsageFault",
+};
if (idx >= 0 && idx < ARRAY_SIZE(excnames)) {
exc = excnames[idx];
--
2.7.4
[Qemu-devel] [PATCH 3/7] arm: Factor out "generate right kind of step exception"
We currently have two places that do:
if (dc->ss_active) {
gen_step_complete_exception(dc);
} else {
gen_exception_internal(EXCP_DEBUG);
}
Factor this out into its own function, as we're about to add
a third place that needs the same logic.
Signed-off-by: Peter Maydell
---
target/arm/translate.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index ddc62b6..870e320 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -296,6 +296,19 @@ static void gen_step_complete_exception(DisasContext *s)
s->is_jmp = DISAS_EXC;
}
+static void gen_singlestep_exception(DisasContext *s)
+{
+/* Generate the right kind of exception for singlestep, which is
+ * either the architectural singlestep or EXCP_DEBUG for QEMU's
+ * gdb singlestepping.
+ */
+if (s->ss_active) {
+gen_step_complete_exception(s);
+} else {
+gen_exception_internal(EXCP_DEBUG);
+}
+}
+
static void gen_smul_dual(TCGv_i32 a, TCGv_i32 b)
{
TCGv_i32 tmp1 = tcg_temp_new_i32();
@@ -11998,24 +12011,15 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
gen_set_pc_im(dc, dc->pc);
/* fall through */
default:
-if (dc->ss_active) {
-gen_step_complete_exception(dc);
-} else {
-/* FIXME: Single stepping a WFI insn will not halt
- the CPU. */
-gen_exception_internal(EXCP_DEBUG);
-}
+/* FIXME: Single stepping a WFI insn will not halt the CPU. */
+gen_singlestep_exception(dc);
}
if (dc->condjmp) {
/* "Condition failed" instruction codepath. */
gen_set_label(dc->condlabel);
gen_set_condexec(dc);
gen_set_pc_im(dc, dc->pc);
-if (dc->ss_active) {
-gen_step_complete_exception(dc);
-} else {
-gen_exception_internal(EXCP_DEBUG);
-}
+gen_singlestep_exception(dc);
}
} else {
/* While branches must always occur at the end of an IT block,
--
2.7.4
[Qemu-devel] [PATCH 4/7] arm: Move gen_set_condexec() and gen_set_pc_im() up in the file
Move the utility routines gen_set_condexec() and gen_set_pc_im()
up in the file, as we will want to use them from a function
placed earlier in the file than their current location.
Signed-off-by: Peter Maydell
---
target/arm/translate.c | 31 +++
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 870e320..a1a0e73 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -893,6 +893,21 @@ static const uint8_t table_logic_cc[16] = {
1, /* mvn */
};
+static inline void gen_set_condexec(DisasContext *s)
+{
+if (s->condexec_mask) {
+uint32_t val = (s->condexec_cond << 4) | (s->condexec_mask >> 1);
+TCGv_i32 tmp = tcg_temp_new_i32();
+tcg_gen_movi_i32(tmp, val);
+store_cpu_field(tmp, condexec_bits);
+}
+}
+
+static inline void gen_set_pc_im(DisasContext *s, target_ulong val)
+{
+tcg_gen_movi_i32(cpu_R[15], val);
+}
+
/* Set PC and Thumb state from an immediate address. */
static inline void gen_bx_im(DisasContext *s, uint32_t addr)
{
@@ -1069,11 +1084,6 @@ DO_GEN_ST(8, MO_UB)
DO_GEN_ST(16, MO_UW)
DO_GEN_ST(32, MO_UL)
-static inline void gen_set_pc_im(DisasContext *s, target_ulong val)
-{
-tcg_gen_movi_i32(cpu_R[15], val);
-}
-
static inline void gen_hvc(DisasContext *s, int imm16)
{
/* The pre HVC helper handles cases when HVC gets trapped
@@ -1107,17 +1117,6 @@ static inline void gen_smc(DisasContext *s)
s->is_jmp = DISAS_SMC;
}
-static inline void
-gen_set_condexec (DisasContext *s)
-{
-if (s->condexec_mask) {
-uint32_t val = (s->condexec_cond << 4) | (s->condexec_mask >> 1);
-TCGv_i32 tmp = tcg_temp_new_i32();
-tcg_gen_movi_i32(tmp, val);
-store_cpu_field(tmp, condexec_bits);
-}
-}
-
static void gen_exception_internal_insn(DisasContext *s, int offset, int excp)
{
gen_set_condexec(s);
--
2.7.4
[Qemu-devel] [PATCH 1/7] arm: Don't implement BXJ on M-profile CPUs
For M-profile CPUs, the BXJ instruction does not exist at all, and
the encoding should always UNDEF. We were accidentally implementing
it to behave like A-profile BXJ; correct the error.
Signed-off-by: Peter Maydell
---
target/arm/translate.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index e32e38c..fe3f442 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -10485,7 +10485,12 @@ static int disas_thumb2_insn(CPUARMState *env,
DisasContext *s, uint16_t insn_hw
}
break;
case 4: /* bxj */
-/* Trivial implementation equivalent to bx. */
+/* Trivial implementation equivalent to bx.
+ * This instruction doesn't exist at all for M-profile.
+ */
+if (arm_dc_feature(s, ARM_FEATURE_M)) {
+goto illegal_op;
+}
tmp = load_reg(s, rn);
gen_bx(s, tmp);
break;
--
2.7.4
[Qemu-devel] [PATCH 5/7] arm: Move condition-failed codepath generation out of if()
Move the code to generate the "condition failed" instruction
codepath out of the if (singlestepping) {} else {}. This
will allow adding support for handling a new is_jmp type
which can't be neatly split into "singlestepping case"
versus "not singlestepping case".
Signed-off-by: Peter Maydell
---
target/arm/translate.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index a1a0e73..87fd702 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -11988,9 +11988,9 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
/* At this stage dc->condjmp will only be set when the skipped
instruction was a conditional branch or trap, and the PC has
already been written. */
+gen_set_condexec(dc);
if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
/* Unconditional and "condition passed" instruction codepath. */
-gen_set_condexec(dc);
switch (dc->is_jmp) {
case DISAS_SWI:
gen_ss_advance(dc);
@@ -12013,13 +12013,6 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
/* FIXME: Single stepping a WFI insn will not halt the CPU. */
gen_singlestep_exception(dc);
}
-if (dc->condjmp) {
-/* "Condition failed" instruction codepath. */
-gen_set_label(dc->condlabel);
-gen_set_condexec(dc);
-gen_set_pc_im(dc, dc->pc);
-gen_singlestep_exception(dc);
-}
} else {
/* While branches must always occur at the end of an IT block,
there are a few other things that can cause us to terminate
@@ -12029,7 +12022,6 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
- Hardware watchpoints.
Hardware breakpoints have already been handled and skip this code.
*/
-gen_set_condexec(dc);
switch(dc->is_jmp) {
case DISAS_NEXT:
gen_goto_tb(dc, 1, dc->pc);
@@ -12069,11 +12061,17 @@ void gen_intermediate_code(CPUARMState *env,
TranslationBlock *tb)
gen_exception(EXCP_SMC, syn_aa32_smc(), 3);
break;
}
-if (dc->condjmp) {
-gen_set_label(dc->condlabel);
-gen_set_condexec(dc);
+}
+
+if (dc->condjmp) {
+/* "Condition failed" instruction codepath for the branch/trap insn */
+gen_set_label(dc->condlabel);
+gen_set_condexec(dc);
+if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
+gen_set_pc_im(dc, dc->pc);
+gen_singlestep_exception(dc);
+} else {
gen_goto_tb(dc, 1, dc->pc);
-dc->condjmp = 0;
}
}
--
2.7.4
[Qemu-devel] [PATCH 0/7] arm: Implement M profile exception return properly
On M profile, return from exceptions happen when privileged code executes one of the following function call return instructions: * POP or LDM which loads the PC * LDR to PC * BX register and the new PC value is 0xFFxx. QEMU tries to implement this by not treating the instruction specially but then catching the attempt to execute from the magic address value. This is not ideal, because: * there are guest visible differences from the architecturally specified behaviour (for instance jumping to 0xFFxx via a different instruction should not cause an exception return but it will in the QEMU implementation) * we have to account for it in various places (like refusing to take an interrupt if the PC is at a magic value, and making sure that the MPU doesn't deny execution at the magic value addresses) Drop these hacks, and instead implement exception return the way the architecture specifies -- by having the relevant instructions check for the magic value and raise the 'do an exception return' QEMU internal exception immediately. I realised when I was looking at the MPU patches that the current mechanism was a bit awkward and needed still more workarounds in code that really shouldn't have to care about exception-return. And it turns out that implementing it properly doesn't actually have very much runtime cost at all. NB: this is technically speaking a migration compatibility break, if you were unlucky enough to migrate just after an exception return when the PC value was one of the magic values, since the new QEMU will not treat this as "now do exception return". However we don't guarantee migration compatibility for M profile, so that's OK. The effect on the generated code is minor: bx lr, old code (and new code for unprivileged mode): TCG: mov_i32 tmp5,r14 movi_i32 tmp6,$0xfffe and_i32 pc,tmp5,tmp6 movi_i32 tmp6,$0x1 and_i32 tmp5,tmp5,tmp6 st_i32 tmp5,env,$0x218 exit_tb $0x0 x86_64 generated code: 0x7f2aabe87019: mov%ebx,%ebp 0x7f2aabe8701b: and$0xfffe,%ebp 0x7f2aabe8701e: mov%ebp,0x3c(%r14) 0x7f2aabe87022: and$0x1,%ebx 0x7f2aabe87025: mov%ebx,0x218(%r14) 0x7f2aabe8702c: xor%eax,%eax 0x7f2aabe8702e: jmpq 0x7f2aabe7c016 bx lr, new code when privileged: TCG: mov_i32 tmp5,r14 movi_i32 tmp6,$0xfffe and_i32 pc,tmp5,tmp6 movi_i32 tmp6,$0x1 and_i32 tmp5,tmp5,tmp6 st_i32 tmp5,env,$0x218 movi_i32 tmp5,$0xff00 brcond_i32 pc,tmp5,geu,$L1 exit_tb $0x0 set_label $L1 movi_i32 tmp5,$0x8 call exception_internal,$0x0,$0,env,tmp5 x86_64 generated code: 0x7fe8fa1264e3: mov%ebp,%ebx 0x7fe8fa1264e5: and$0xfffe,%ebx 0x7fe8fa1264e8: mov%ebx,0x3c(%r14) 0x7fe8fa1264ec: and$0x1,%ebp 0x7fe8fa1264ef: mov%ebp,0x218(%r14) 0x7fe8fa1264f6: cmp$0xff00,%ebx 0x7fe8fa1264fc: jae0x7fe8fa126509 0x7fe8fa126502: xor%eax,%eax 0x7fe8fa126504: jmpq 0x7fe8fa122016 0x7fe8fa126509: mov%r14,%rdi 0x7fe8fa12650c: mov$0x8,%esi 0x7fe8fa126511: mov$0x56095dbeccf5,%r10 0x7fe8fa12651b: callq *%r10 which is a difference of one cmp/branch-not-taken. This will be lost in the noise of having to exit generated code and look up the next TB anyway. Patches 1 and 2 are minor bug fixes which remove code paths that incorrectly called gen_bx(), so we don't have to consider them when looking at which gen_bx() calls need to be changed to call gen_bx_excret(). Patches 3 to 5 are minor preliminary refactorings; patch 6 is the meat of the series; finally patch 7 removes the old mechanism. thanks -- PMM Peter Maydell (7): arm: Don't implement BXJ on M-profile CPUs arm: Thumb shift operations should not permit interworking branches arm: Factor out "generate right kind of step exception" arm: Move gen_set_condexec() and gen_set_pc_im() up in the file arm: Move condition-failed codepath generation out of if() arm: Implement M profile exception return properly arm: Remove workarounds for old M-profile exception return implementation target/arm/translate.h | 4 ++ target/arm/cpu.c | 43 + target/arm/translate.c | 163 - 3 files changed, 113 insertions(+), 97 deletions(-) -- 2.7.4
Re: [Qemu-devel] [PATCH v2] qxl: add migration blocker to avoid pre-save assert
Hi,
> > +if (!qxl->migration_blocker) {
> > +Error *local_err = NULL;
> > +error_setg(&qxl->migration_blocker,
> > + "qxl: guest bug: command not in ram bar");
> > +migrate_add_blocker(qxl->migration_blocker,
> > &local_err);
> >
>
> What do you do with the local_err? error_report_err() perhaps ?
We can't error out at that point, unlike most migration blockers this
isn't added at device initialization time.
So, yes, we could error_report it, but the message would end up in the
logfile unnoticed, so I'm not sure how useful that is ...
cheers,
Gerd
Re: [Qemu-devel] [PATCH for-2.10 14/19] crypto: cipher: add af_alg cipher support
On 2017/4/10 18:18, Daniel P. Berrange wrote:
> On Mon, Apr 10, 2017 at 05:00:52PM +0800, Longpeng(Mike) wrote:
>> Adds afalg-backend cipher support: introduces some private APIs
>> firstly, and then intergrates them into qcrypto_cipher_afalg_driver.
>>
>> Signed-off-by: Longpeng(Mike)
>> ---
>> crypto/Makefile.objs| 1 +
>> crypto/cipher-afalg.c | 229
>>
>> crypto/cipher.c | 30 +-
>> include/crypto/afalg-comm.h | 11 +++
>> include/crypto/cipher.h | 7 ++
>> tests/test-crypto-cipher.c | 10 +-
>> 6 files changed, 286 insertions(+), 2 deletions(-)
>> create mode 100644 crypto/cipher-afalg.c
>>
>
...
>
>
> All methods in this file should be named 'qcrypto_afalg_...'
>
Hi Daniel,
Now the libiary-backend methods are named as "nettle(gcrypt,glib,builtin)_...",
so if I rename them with "qcrypto_afalg_" prefix , then the libiary-backend
methods need with "qcrypto_nettle(gcrypt,glib,builtin)_" prefix , right ?
>> +
>> +struct QCryptoCipherDriver qcrypto_cipher_afalg_driver = {
>> +.cipher_encrypt = afalg_cipher_encrypt,
>> +.cipher_decrypt = afalg_cipher_decrypt,
>> +.cipher_setiv = afalg_cipher_setiv,
>> +.cipher_free = afalg_comm_ctx_free,
>> +};
>
>
>
>
> Regards,
> Daniel
--
Regards,
Longpeng(Mike)
Re: [Qemu-devel] [PATCH for-2.10 14/19] crypto: cipher: add af_alg cipher support
On Mon, Apr 10, 2017 at 06:52:29PM +0800, Longpeng (Mike) wrote: > > On 2017/4/10 18:18, Daniel P. Berrange wrote: > > > On Mon, Apr 10, 2017 at 05:00:52PM +0800, Longpeng(Mike) wrote: > >> Adds afalg-backend cipher support: introduces some private APIs > >> firstly, and then intergrates them into qcrypto_cipher_afalg_driver. > >> > >> Signed-off-by: Longpeng(Mike) > >> --- > >> crypto/Makefile.objs| 1 + > >> crypto/cipher-afalg.c | 229 > >> > >> crypto/cipher.c | 30 +- > >> include/crypto/afalg-comm.h | 11 +++ > >> include/crypto/cipher.h | 7 ++ > >> tests/test-crypto-cipher.c | 10 +- > >> 6 files changed, 286 insertions(+), 2 deletions(-) > >> create mode 100644 crypto/cipher-afalg.c > >> > > > > ... > > > > > > > All methods in this file should be named 'qcrypto_afalg_...' > > > > > Hi Daniel, > > Now the libiary-backend methods are named as > "nettle(gcrypt,glib,builtin)_...", > so if I rename them with "qcrypto_afalg_" prefix , then the libiary-backend > methods need with "qcrypto_nettle(gcrypt,glib,builtin)_" prefix , right ? Yep, that's right - every method in the crypto/ directory should have a qcrypto_ name prefix, even if it is a static method. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
Re: [Qemu-devel] [Qemu-block] [PATCH RFC 00/16] block: Protect AIO context change with perm API
On Mon, 04/10 10:04, Stefan Hajnoczi wrote: > On Tue, Mar 21, 2017 at 11:16:19AM +0800, Fam Zheng wrote: > > Eject / change of scsi-cd on a virtio-scsi dataplane bus causes abort() > > because > > the new BDS doesn't get proper bdrv_set_aio_context(). > > > > Store the AioContext in BB and do it in blk_insert_bs. That is done by > > Vladimir's patch. > > > > Other patches are to make sure such a bdrv_set_aio_context() doesn't > > interfere > > with other BBs using other nodes from this graph. > > > > RFC note: > > > > Unfortunately, a use-after-free crash in iotests 030 appears since patch 7, > > which I believe is a latent bug that bdrv_reopen is "reentered" in existing > > code, rather than from this series: > > > > > #4 0x561ab90425a7 in bdrv_reopen > > > #5 0x561ab8e1d28e in stream_complete > > > #6 0x561ab9048543 in block_job_defer_to_main_loop_bh > > > #7 0x561ab91305bc in aio_bh_call > > > #8 0x561ab9130659 in aio_bh_poll > > > #9 0x561ab9135656 in aio_poll > > > #10 0x561ab90a6cf5 in bdrv_flush > > > #11 0x561ab904285a in bdrv_reopen_prepare > > > #12 0x561ab90423f0 in bdrv_reopen_multiple > > > #13 0x561ab90425ef in bdrv_reopen > > > #14 0x561ab909fa49 in commit_active_start > > > #15 0x561ab8dd6ffb in qmp_block_commit > > > #16 0x561ab8ded485 in qmp_marshal_block_commit > > > #17 0x561ab9123e6c in do_qmp_dispatch > > > #18 0x561ab9123fa4 in qmp_dispatch > > > #19 0x561ab8ca26b7 in handle_qmp_command > > > > I have a fix that I'll post separately. > > > > The last patches are an alternative to patch 7, to "workaround" this in a > > really non-obvious way. > > Are there qemu-iotests that cover both success and failure scenarios for > the new permission? No, I'll add them. Fam
Re: [Qemu-devel] [PATCH v2] qxl: add migration blocker to avoid pre-save assert
Hi
On Mon, Apr 10, 2017 at 12:51 PM Gerd Hoffmann wrote:
> Hi,
>
> > > +if (!qxl->migration_blocker) {
> > > +Error *local_err = NULL;
> > > +error_setg(&qxl->migration_blocker,
> > > + "qxl: guest bug: command not in ram
> bar");
> > > +migrate_add_blocker(qxl->migration_blocker,
> > > &local_err);
> > >
> >
> > What do you do with the local_err? error_report_err() perhaps ?
>
> We can't error out at that point, unlike most migration blockers this
> isn't added at device initialization time.
>
> So, yes, we could error_report it, but the message would end up in the
> logfile unnoticed, so I'm not sure how useful that is ...
>
Well, it may eventually be read if something breaks. Otherwise, you may
just pass a NULL pointer, no?
thanks
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH for-2.10 14/19] crypto: cipher: add af_alg cipher support
On 2017/4/10 18:56, Daniel P. Berrange wrote: > On Mon, Apr 10, 2017 at 06:52:29PM +0800, Longpeng (Mike) wrote: >> >> On 2017/4/10 18:18, Daniel P. Berrange wrote: >> >>> On Mon, Apr 10, 2017 at 05:00:52PM +0800, Longpeng(Mike) wrote: Adds afalg-backend cipher support: introduces some private APIs firstly, and then intergrates them into qcrypto_cipher_afalg_driver. Signed-off-by: Longpeng(Mike) --- crypto/Makefile.objs| 1 + crypto/cipher-afalg.c | 229 crypto/cipher.c | 30 +- include/crypto/afalg-comm.h | 11 +++ include/crypto/cipher.h | 7 ++ tests/test-crypto-cipher.c | 10 +- 6 files changed, 286 insertions(+), 2 deletions(-) create mode 100644 crypto/cipher-afalg.c >>> >> >> ... >> >>> >>> >>> All methods in this file should be named 'qcrypto_afalg_...' >>> >> >> >> Hi Daniel, >> >> Now the libiary-backend methods are named as >> "nettle(gcrypt,glib,builtin)_...", >> so if I rename them with "qcrypto_afalg_" prefix , then the libiary-backend >> methods need with "qcrypto_nettle(gcrypt,glib,builtin)_" prefix , right ? > > Yep, that's right - every method in the crypto/ directory should have a > qcrypto_ name prefix, even if it is a static method. > Ok, I'll fix them in V2, thx. :） > > Regards, > Daniel -- Regards, Longpeng(Mike)
[Qemu-devel] [Bug 1681404] [NEW] hw/ppc: Aborted (core dumped)
Public bug reported: Reproducable: $ ./ppc64-softmmu/qemu-system-ppc64 -S -machine ppce500,accel=tcg -device spapr-pci-host-bridge qemu/hw/ppc/spapr_pci.c:1567:spapr_phb_realize: Object 0x55bda99744a0 is not an instance of type spapr-machine Aborted (core dumped) ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1681404 Title: hw/ppc: Aborted (core dumped) Status in QEMU: New Bug description: Reproducable: $ ./ppc64-softmmu/qemu-system-ppc64 -S -machine ppce500,accel=tcg -device spapr-pci-host-bridge qemu/hw/ppc/spapr_pci.c:1567:spapr_phb_realize: Object 0x55bda99744a0 is not an instance of type spapr-machine Aborted (core dumped) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1681404/+subscriptions
[Qemu-devel] [PATCH 6/7] arm: Implement M profile exception return properly
On M profile, return from exceptions happen when privileged code
executes one of the following function call return instructions:
* POP or LDM which loads the PC
* LDR to PC
* BX register
and the new PC value is 0xFFxx.
QEMU tries to implement this by not treating the instruction
specially but then catching the attempt to execute from the magic
address value. This is not ideal, because:
* there are guest visible differences from the architecturally
specified behaviour (for instance jumping to 0xFFxx via a
different instruction should not cause an exception return but it
will in the QEMU implementation)
* we have to account for it in various places (like refusing to take
an interrupt if the PC is at a magic value, and making sure that
the MPU doesn't deny execution at the magic value addresses)
Drop these hacks, and instead implement exception return the way the
architecture specifies -- by having the relevant instructions check
for the magic value and raise the 'do an exception return' QEMU
internal exception immediately.
The effect on the generated code is minor:
bx lr, old code (and new code for unprivileged mode):
TCG:
mov_i32 tmp5,r14
movi_i32 tmp6,$0xfffe
and_i32 pc,tmp5,tmp6
movi_i32 tmp6,$0x1
and_i32 tmp5,tmp5,tmp6
st_i32 tmp5,env,$0x218
exit_tb $0x0
set_label $L0
exit_tb $0x7f2aabd61993
x86_64 generated code:
0x7f2aabe87019: mov%ebx,%ebp
0x7f2aabe8701b: and$0xfffe,%ebp
0x7f2aabe8701e: mov%ebp,0x3c(%r14)
0x7f2aabe87022: and$0x1,%ebx
0x7f2aabe87025: mov%ebx,0x218(%r14)
0x7f2aabe8702c: xor%eax,%eax
0x7f2aabe8702e: jmpq 0x7f2aabe7c016
bx lr, new code when privileged:
TCG:
mov_i32 tmp5,r14
movi_i32 tmp6,$0xfffe
and_i32 pc,tmp5,tmp6
movi_i32 tmp6,$0x1
and_i32 tmp5,tmp5,tmp6
st_i32 tmp5,env,$0x218
movi_i32 tmp5,$0xff00
brcond_i32 pc,tmp5,geu,$L1
exit_tb $0x0
set_label $L1
movi_i32 tmp5,$0x8
call exception_internal,$0x0,$0,env,tmp5
x86_64 generated code:
0x7fe8fa1264e3: mov%ebp,%ebx
0x7fe8fa1264e5: and$0xfffe,%ebx
0x7fe8fa1264e8: mov%ebx,0x3c(%r14)
0x7fe8fa1264ec: and$0x1,%ebp
0x7fe8fa1264ef: mov%ebp,0x218(%r14)
0x7fe8fa1264f6: cmp$0xff00,%ebx
0x7fe8fa1264fc: jae0x7fe8fa126509
0x7fe8fa126502: xor%eax,%eax
0x7fe8fa126504: jmpq 0x7fe8fa122016
0x7fe8fa126509: mov%r14,%rdi
0x7fe8fa12650c: mov$0x8,%esi
0x7fe8fa126511: mov$0x56095dbeccf5,%r10
0x7fe8fa12651b: callq *%r10
which is a difference of one cmp/branch-not-taken. This will
be lost in the noise of having to exit generated code and
look up the next TB anyway.
Signed-off-by: Peter Maydell
---
target/arm/translate.h | 4
target/arm/translate.c | 65 +-
2 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index abb0760..c2a5451 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -134,6 +134,10 @@ static void disas_set_insn_syndrome(DisasContext *s,
uint32_t syn)
#define DISAS_HVC 8
#define DISAS_SMC 9
#define DISAS_YIELD 10
+/* M profile branch which might be an exception return (and so needs
+ * custom end-of-TB code)
+ */
+#define DISAS_BX_EXCRET 11
#ifdef TARGET_AARCH64
void a64_translate_init(void);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 87fd702..156ab46 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -932,6 +932,51 @@ static inline void gen_bx(DisasContext *s, TCGv_i32 var)
store_cpu_field(var, thumb);
}
+/* Set PC and Thumb state from var. var is marked as dead.
+ * For M-profile CPUs, include logic to detect exception-return
+ * branches and handle them.
+ * This is needed for Thumb POP/LDM to PC, LDR to PC, and BX reg, and no
others.
+ */
+static inline void gen_bx_excret(DisasContext *s, TCGv_i32 var)
+{
+/* Generate the same code here as for a simple bx, but flag via
+ * s->is_jmp that we need to do the rest of the work later.
+ */
+gen_bx(s, var);
+if (!IS_USER(s) && arm_dc_feature(s, ARM_FEATURE_M)) {
+s->is_jmp = DISAS_BX_EXCRET;
+}
+}
+
+static inline void gen_bx_excret_final_code(DisasContext *s)
+{
+/* Generate the code to finish possible exception return and end the TB */
+TCGLabel *excret_label = gen_new_label();
+
+/* Is the new PC value in the magic range indicating exception return? */
+tcg_gen_brcondi_i32(TCG_COND_GEU, cpu_R[15], 0xff00, excret_label);
+/* No: end the TB as we would for a DISAS_JMP */
+if (s->singlestep_enabled || s->ss_active) {
+gen_singlestep_exception(s);
+} else {
+tcg_gen_exit_tb(0);
+}
+gen_set_label(excret_label);
+/* Yes: this is an exception return.
+ * At this point in runtime env->regs[15] and env->thumb
[Qemu-devel] [PATCH v3] qxl: add migration blocker to avoid pre-save assert
Cc: 1635...@bugs.launchpad.net
Signed-off-by: Gerd Hoffmann
---
hw/display/qxl.h | 1 +
hw/display/qxl.c | 31 +++
2 files changed, 32 insertions(+)
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index d2d49dd..77e5a36 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -40,6 +40,7 @@ typedef struct PCIQXLDevice {
uint32_t cmdlog;
uint32_t guest_bug;
+Error *migration_blocker;
enum qxl_mode mode;
uint32_t cmdflags;
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index c31b293..9feae78 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -26,6 +26,7 @@
#include "qemu/queue.h"
#include "qemu/atomic.h"
#include "sysemu/sysemu.h"
+#include "migration/migration.h"
#include "trace.h"
#include "qxl.h"
@@ -639,6 +640,30 @@ static int interface_get_command(QXLInstance *sin, struct
QXLCommandExt *ext)
qxl->guest_primary.commands++;
qxl_track_command(qxl, ext);
qxl_log_command(qxl, "cmd", ext);
+{
+/*
+ * Windows 8 drivers place qxl commands in the vram
+ * (instead of the ram) bar. We can't live migrate such a
+ * guest, so add a migration blocker in case we detect
+ * this, to avoid triggering the assert in pre_save().
+ *
+ *
https://cgit.freedesktop.org/spice/win32/qxl-wddm-dod/commit/?id=f6e099db39e7d0787f294d5fd0dce328b5210faa
+ */
+void *msg = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+if (msg != NULL && (
+msg < (void *)qxl->vga.vram_ptr ||
+msg > ((void *)qxl->vga.vram_ptr + qxl->vga.vram_size))) {
+if (!qxl->migration_blocker) {
+Error *local_err = NULL;
+error_setg(&qxl->migration_blocker,
+ "qxl: guest bug: command not in ram bar");
+migrate_add_blocker(qxl->migration_blocker, &local_err);
+if (local_err) {
+error_report_err(local_err);
+}
+}
+}
+}
trace_qxl_ring_command_get(qxl->id, qxl_mode_to_string(qxl->mode));
return true;
default:
@@ -1236,6 +1261,12 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
qemu_spice_create_host_memslot(&d->ssd);
qxl_soft_reset(d);
+if (d->migration_blocker) {
+migrate_del_blocker(d->migration_blocker);
+error_free(d->migration_blocker);
+d->migration_blocker = NULL;
+}
+
if (startstop) {
qemu_spice_display_start();
}
--
2.9.3
