date:20180626

John Snow  writes:

> On 06/25/2018 12:57 PM, Philippe Mathieu-Daudé wrote:
>> Use assert() instead of error_setg(&error_abort),
>> as suggested by the "qapi/error.h" documentation:
>> 
>> Please don't error_setg(&error_fatal, ...), use error_report() and
>> exit(), because that's more obvious.
>> Likewise, don't error_setg(&error_abort, ...), use assert().
>> 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> Acked-by: John Snow 
>
> AIUI this series is to be merged by someone else currently, right?
> Nothing needed on my part?

Whatever doesn't get picked up by maintainers of patched code can go
through my tree.

Re: [Qemu-devel] [PATCH] vga: set owner for mmio regions

On 26.06.2018 08:09, Gerd Hoffmann wrote:
> This makes sure the regions are properly cleaned when unplugging -device
> seconday-vga.
> 
> Reported-by: Thomas Huth 
> Signed-off-by: Gerd Hoffmann 
> ---
>  hw/display/vga_int.h|  1 +
>  hw/display/vga-pci.c| 11 ++-
>  hw/display/virtio-vga.c |  2 +-
>  3 files changed, 8 insertions(+), 6 deletions(-)

Thanks, this fixes the issue indeed! Without this patch:

$ valgrind mips-softmmu/qemu-system-mips -accel qtest -monitor stdio
[...]
(qemu) device_add secondary-vga
Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set
(qemu) dump-guest-memory /dev/null 0 4096
==8407== Invalid read of size 8
==8407==at 0x6B0DA5: object_dynamic_cast (object.c:613)
==8407==by 0x6B0DA5: object_resolve_abs_path (object.c:1721)
==8407==by 0x6B0E00: object_resolve_partial_path (object.c:1745)
==8407==by 0x6B0E62: object_resolve_partial_path (object.c:1755)
[...]

With this patch applied, the problem is gone.

Tested-by: Thomas Huth 
Reviewed-by: Thomas Huth

Re: [Qemu-devel] [PATCH] coverity-model: replay data is considered trusted

Paolo Bonzini  writes:

> Replay data is not considered a possible attack vector; add a model that
> does not use getc so that "tainted data" warnings are suppressed.
>
> Signed-off-by: Paolo Bonzini 
> ---
>  scripts/coverity-model.c | 13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
> index c702804f41..576f48de33 100644
> --- a/scripts/coverity-model.c
> +++ b/scripts/coverity-model.c
> @@ -103,6 +103,19 @@ static int get_keysym(const name2keysym_t *table,
>  }
>  }
>  
> +
> +/* Replay data is considered trusted.  */
> +uint8_t replay_get_byte(void)
> +{
> + uint8_t byte = 0;
> + if (replay_file) {
> + uint8_t c;
> + byte = c;
> + }
> + return byte;
> +}
> +
> +
>  /*
>   * GLib memory allocation functions.
>   *

Coverity 2018.06 chokes on this:

$ cov-make-library -of scripts/coverity-model.xmldb 
scripts/coverity-model.c 
output file: scripts/coverity-model.xmldb
Compiling scripts/coverity-model.c with command 
/opt/cov-sa-2018.06/bin/cov-emit --dir 
/tmp/cov-armbru/930a6fb31e5f464fc1a53354b2deb66b/cov-make-library-emit -w 
--no_error_recovery --emit_header_functions --no_implicit_decl --preinclude 
/opt/cov-sa-2018.06/library/decls.h --c scripts/coverity-model.c
"scripts/coverity-model.c", line 110: error #20: identifier "replay_file" is
  undefined
   if (replay_file) {
   ^

Emit for file '/work/armbru/qemu/scripts/coverity-model.c' complete.
[ERROR] 1 error detected in the compilation of "scripts/coverity-model.c".
ERROR: cov-emit returned with code 1

Minimal fix:

diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 48b112393b..f987ce53b8 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -106,6 +106,7 @@ static int get_keysym(const name2keysym_t *table,
 /* Replay data is considered trusted.  */
 uint8_t replay_get_byte(void)
 {
+void *replay_file;
  uint8_t byte = 0;
  if (replay_file) {
  uint8_t c;

Alternatively, dumb down to:

/* Replay data is considered trusted.  */
uint8_t replay_get_byte(void)
{
uint8_t byte;
return byte;
}

Got a preference?

Re: [Qemu-devel] [PATCH] s390/boot: block uncompressed vmlinux booting attempts

2018-06-26 Thread Christian Borntraeger

On 06/25/2018 05:09 PM, Vasily Gorbik wrote:
> Since uncompressed kernel image "vmlinux" elf file is not bootable under
> qemu anymore, add a check which would report that.
> 
> Qemu users are encouraged to use bzImage or
> arch/s390/boot/compressed/vmlinux instead.
> 
> The check relies on s390 linux entry point ABI definition, which is only
> present in bzImage and arch/s390/boot/compressed/vmlinux.
> 
> Signed-off-by: Vasily Gorbik 
Acked-by: Christian Borntraeger 

some proposals regarding the wording below..

[...]
> +
> + sclp_early_printk("The linux kernel boot failure: the image is 
> corrupted or not bootable.\n");
> + sclp_early_printk("Please check that you are using bootable kernel 
> image \"bzImage\".\n");
> + sclp_early_printk("(or alternatively 
> \"arch/s390/boot/compressed/vmlinux\" image for qemu)\n");

What about making this explain things a bit more, e.g. something like

Linux kernel boot failure: The boot image does not contain all necessary
components (like the entry point and decompressor). The plain vmlinux ELF
file no longer carries all necessary parts for starting up. Please use
bzImage or arch/s390/boot/compressed/vmlinux.

Re: [Qemu-devel] [PATCH] migration: invalidate cache before source start

2018-06-26 Thread Vladimir Sementsov-Ogievskiy


25.06.2018 21:03, John Snow wrote:


On 06/25/2018 01:50 PM, Dr. David Alan Gilbert wrote:

* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:

15.06.2018 15:06, Dr. David Alan Gilbert wrote:

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:

Invalidate cache before source start in case of failed migration.

Signed-off-by: Vladimir Sementsov-Ogievskiy 

Why doesn't the code at the bottom of migration_completion,
fail_invalidate:   and the code in migrate_fd_cancel   handle this?

What case did you see it in that those didn't handle?
(Also I guess it probably should set s->block_inactive = false)

on source I see:

81392@1529065750.766289:migrate_set_state new state 7
81392@1529065750.766330:migration_thread_file_err
81392@1529065750.766332:migration_thread_after_loop

so, we are leaving loop on
     if (qemu_file_get_error(s->to_dst_file)) {
     migrate_set_state(&s->state, current_active_state,
MIGRATION_STATUS_FAILED);
trace_migration_thread_file_err();
break;
     }

and skip migration_completion()

Yeh, OK; I'd seen soemthing else a few days ago, where a cancellation
test that had previously ended with a 'cancelled' state has now ended up
in 'failed' (which is the state 7 you have above).
I suspect there's something else going on as well; I think what is
supposed to happen in the case of 'cancel' is that it spins in 'cancelling' for
a while in migrate_fd_cancel and then at the bottom of migrate_fd_cancel
it does the recovery, but because it's going to failed instead, then
it's jumping over that recovery.

Going back and actually looking at the patch again;
can I ask for 1 small change;
Can you set s->block_inactive = false   in the case where you
don't get the local_err (Like we do at the bottom of migrate_fd_cancel)


Does that make sense?

Thanks,

Dave


Vladimir, one more question for you because I'm not as familiar with
this code:

In the normal case we need to invalidate the qcow2 cache as a way to
re-engage the disk (yes?) when we have failed during the late-migration
steps.

In this case, we seem to be observing a failure during the bulk transfer
loop. Why is it important to invalidate the cache at this step -- would
the disk have been inactivated yet? It shouldn't, because it's in the
bulk transfer phase -- or am I missing something?

I feel like this code is behaving in a way that's fairly surprising for
a casual reader so I was hoping you could elaborate for me.

--js


In my case, source is already in postcopy state, when error occured, so 
it is inactivated.


--
Best regards,
Vladimir

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

On Tue, 26 Jun 2018 06:40:56 +0200
Thomas Huth  wrote:

> On 25.06.2018 20:26, Paolo Bonzini wrote:
> > On 25/06/2018 19:30, Eduardo Habkost wrote:  
>  Attentive distros could even replace the wrapper script by a link.  
> >>> If they are okay with replacing the "KVM only" semantics with "KVM or
> >>> TCG", which I think is generally worse.  
> >>
> >> If we can't get agreement on what's the right default for each
> >> QEMU binary, I think that's yet another reason to document that
> >> upstream QEMU won't guarantee ABI compatibility if -accel is
> >> omitted.  
> > 
> > Before that we should ask what the benefit is in changing the default
> > for qemu-system-*.  Nobody is using it in practice to start QEMU with
> > KVM enabled...  
> 
> That's certainly not true. I've seen a couple of times already that
> people ask on IRC why their guests are running so slow, and if you ask
> them about their command line, it's obvious that they simply were not
> aware of "-accel" / "-enable-kvm" yet.
> 
> 
> Maybe we simply should add a "--verbose" command line option that people
> can use to diagnose their problems:
> 
> $ qemu-system-x86_64 --verbose
> QEMU emulator version 2.12.50
> Using 'tcg' accelerator. Use '-accel kvm' to speed things up.
> Machine type is 'pc-i440fx-3.0'. Use 'q35' for a more modern machine.
> 
> 

Not sure how serious you meant that, but I actually quite like the
idea :)

Re: [Qemu-devel] [PATCH] coverity-model: replay data is considered trusted

On 26/06/2018 09:27, Markus Armbruster wrote:
> 
> Minimal fix:
> 
> diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
> index 48b112393b..f987ce53b8 100644
> --- a/scripts/coverity-model.c
> +++ b/scripts/coverity-model.c
> @@ -106,6 +106,7 @@ static int get_keysym(const name2keysym_t *table,
>  /* Replay data is considered trusted.  */
>  uint8_t replay_get_byte(void)
>  {
> +void *replay_file;
>   uint8_t byte = 0;
>   if (replay_file) {
>   uint8_t c;
> 
> Alternatively, dumb down to:
> 
> /* Replay data is considered trusted.  */
> uint8_t replay_get_byte(void)
> {
> uint8_t byte;
> return byte;
> }

I wonder why the online service didn't complain.  I guess the dumber
version is more than enough!

Paolo

[Qemu-devel] [PATCH 0/2] block: Pacify Coverity

Markus Armbruster (2):
  block-qdict: Pacify Coverity after commit f1b34a248e9
  block/crypto: Pacify Coverity after commit f853465aacb

 block/crypto.c|  4 ++--
 qobject/block-qdict.c | 16 
 2 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/2] block-qdict: Pacify Coverity after commit f1b34a248e9

Commit f1b34a248e9 replaced less-than-obvious test in
qdict_flatten_qdict() by the obvious one.  Sadly, it made something
else non-obvious: the fact that @new_key passed to qdict_put_obj()
can't be null, because that depends on the function's precondition
(target == qdict) == !prefix.

Tweak the function some more to help Coverity and human readers alike.

Fixes: CID 1393620
Signed-off-by: Markus Armbruster 
---
 qobject/block-qdict.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c
index 36129e7379..80c653013f 100644
--- a/qobject/block-qdict.c
+++ b/qobject/block-qdict.c
@@ -97,7 +97,7 @@ static void qdict_flatten_qdict(QDict *qdict, QDict *target, 
const char *prefix)
 const QDictEntry *entry, *next;
 QDict *dict_val;
 QList *list_val;
-char *new_key;
+char *key, *new_key;
 
 entry = qdict_first(qdict);
 
@@ -106,10 +106,12 @@ static void qdict_flatten_qdict(QDict *qdict, QDict 
*target, const char *prefix)
 value = qdict_entry_value(entry);
 dict_val = qobject_to(QDict, value);
 list_val = qobject_to(QList, value);
-new_key = NULL;
 
 if (prefix) {
-new_key = g_strdup_printf("%s.%s", prefix, entry->key);
+key = new_key = g_strdup_printf("%s.%s", prefix, entry->key);
+} else {
+key = entry->key;
+new_key = NULL;
 }
 
 /*
@@ -125,19 +127,17 @@ static void qdict_flatten_qdict(QDict *qdict, QDict 
*target, const char *prefix)
  * well advised not to modify them altogether.)
  */
 if (dict_val && qdict_size(dict_val)) {
-qdict_flatten_qdict(dict_val, target,
-new_key ? new_key : entry->key);
+qdict_flatten_qdict(dict_val, target, key);
 if (target == qdict) {
 qdict_del(qdict, entry->key);
 }
 } else if (list_val && !qlist_empty(list_val)) {
-qdict_flatten_qlist(list_val, target,
-new_key ? new_key : entry->key);
+qdict_flatten_qlist(list_val, target, key);
 if (target == qdict) {
 qdict_del(qdict, entry->key);
 }
 } else if (target != qdict) {
-qdict_put_obj(target, new_key, qobject_ref(value));
+qdict_put_obj(target, key, qobject_ref(value));
 }
 
 g_free(new_key);
-- 
2.17.1

[Qemu-devel] [PATCH 2/2] block/crypto: Pacify Coverity after commit f853465aacb

Coverity can't see that qobject_input_visitor_new_flat_confused()
returns non-null when it doesn't set @local_err.  Check the return
value instead, like all the other callers do.

Fixes: CID 1393615
Fixes: CID 1393616
Signed-off-by: Markus Armbruster 
---
 block/crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 82091c5f70..aaa8fb7530 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -160,7 +160,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
 ret->format = format;
 
 v = qobject_input_visitor_new_flat_confused(opts, &local_err);
-if (local_err) {
+if (!v) {
 goto out;
 }
 
@@ -214,7 +214,7 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
 ret->format = format;
 
 v = qobject_input_visitor_new_flat_confused(opts, &local_err);
-if (local_err) {
+if (!v) {
 goto out;
 }
 
-- 
2.17.1

[Qemu-devel] [RFC] arm: Add NRF51 SOC non-volatile memory controller

Signed-off-by: Steffen Görtz 
---
 hw/nvram/Makefile.objs|   1 +
 hw/nvram/nrf51_nvmc.c | 165 ++
 include/hw/nvram/nrf51_nvmc.h |  52 +++
 3 files changed, 218 insertions(+)
 create mode 100644 hw/nvram/nrf51_nvmc.c
 create mode 100644 include/hw/nvram/nrf51_nvmc.h

diff --git a/hw/nvram/Makefile.objs b/hw/nvram/Makefile.objs
index a912d25391..9edd61e8af 100644
--- a/hw/nvram/Makefile.objs
+++ b/hw/nvram/Makefile.objs
@@ -5,3 +5,4 @@ common-obj-y += fw_cfg.o
 common-obj-y += chrp_nvram.o
 common-obj-$(CONFIG_MAC_NVRAM) += mac_nvram.o
 obj-$(CONFIG_PSERIES) += spapr_nvram.o
+obj-$(CONFIG_NRF51_SOC) += nrf51_nvmc.o
diff --git a/hw/nvram/nrf51_nvmc.c b/hw/nvram/nrf51_nvmc.c
new file mode 100644
index 00..839d03d9c4
--- /dev/null
+++ b/hw/nvram/nrf51_nvmc.c
@@ -0,0 +1,165 @@
+/*
+ * nrf51_nvmc.c
+ *
+ * Copyright 2018 Steffen Görtz 
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "hw/nvram/nrf51_nvmc.h"
+#include "exec/address-spaces.h"
+
+#define NRF51_NVMC_SIZE 0x1000
+
+#define NRF51_NVMC_READY0x400
+#define NRF51_NVMC_READY_READY  0x01
+#define NRF51_NVMC_CONFIG   0x504
+#define NRF51_NVMC_CONFIG_MASK  0x03
+#define NRF51_NVMC_CONFIG_WEN   0x01
+#define NRF51_NVMC_CONFIG_EEN   0x02
+#define NRF51_NVMC_ERASEPCR10x508
+#define NRF51_NVMC_ERASEPCR00x510
+#define NRF51_NVMC_ERASEALL 0x50C
+#define NRF51_NVMC_ERASEUICR0x512
+#define NRF51_NVMC_ERASE0x01
+
+#define NRF51_UICR_OFFSET   0x10001000UL
+#define NRF51_UICR_SIZE 0x100
+
+static uint64_t io_read(void *opaque, hwaddr offset, unsigned int size)
+{
+Nrf51NVMCState *s = NRF51_NVMC(opaque);
+uint64_t r = 0;
+
+switch (offset) {
+case NRF51_NVMC_READY:
+r = NRF51_NVMC_READY_READY;
+break;
+case NRF51_NVMC_CONFIG:
+r = s->state.config;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: bad read offset 0x%" HWADDR_PRIx "\n", __func__, offset);
+}
+
+return r;
+}
+
+static void io_write(void *opaque, hwaddr offset, uint64_t value,
+unsigned int size)
+{
+Nrf51NVMCState *s = NRF51_NVMC(opaque);
+
+switch (offset) {
+case NRF51_NVMC_CONFIG:
+s->state.config = value & NRF51_NVMC_CONFIG_MASK;
+break;
+case NRF51_NVMC_ERASEPCR0:
+case NRF51_NVMC_ERASEPCR1:
+value &= ~(s->page_size - 1);
+if (value < (s->code_size * s->page_size)) {
+address_space_write(&s->as, value, MEMTXATTRS_UNSPECIFIED,
+s->empty_page, s->page_size);
+}
+break;
+case NRF51_NVMC_ERASEALL:
+if (value == NRF51_NVMC_ERASE) {
+for (uint32_t i = 0; i < s->code_size; i++) {
+address_space_write(&s->as, i * s->page_size,
+MEMTXATTRS_UNSPECIFIED, s->empty_page, s->page_size);
+}
+address_space_write(&s->as, NRF51_UICR_OFFSET,
+MEMTXATTRS_UNSPECIFIED, s->empty_page, NRF51_UICR_SIZE);
+}
+break;
+case NRF51_NVMC_ERASEUICR:
+if (value == NRF51_NVMC_ERASE) {
+address_space_write(&s->as, NRF51_UICR_OFFSET,
+MEMTXATTRS_UNSPECIFIED, s->empty_page, NRF51_UICR_SIZE);
+}
+break;
+
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: bad write offset 0x%" HWADDR_PRIx "\n", __func__, offset);
+}
+}
+
+static const MemoryRegionOps io_ops = { .read = io_read, .write = io_write,
+.endianness = DEVICE_LITTLE_ENDIAN, };
+
+static void nrf51_nvmc_init(Object *obj)
+{
+Nrf51NVMCState *s = NRF51_NVMC(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+memory_region_init_io(&s->mmio, obj, &io_ops, s,
+TYPE_NRF51_NVMC, NRF51_NVMC_SIZE);
+sysbus_init_mmio(sbd, &s->mmio);
+}
+
+static void nrf51_nvmc_realize(DeviceState *dev, Error **errp)
+{
+Nrf51NVMCState *s = NRF51_NVMC(dev);
+
+if (!s->mr) {
+error_setg(errp, "memory property was not set");
+return;
+}
+
+if (s->page_size < NRF51_UICR_SIZE) {
+error_setg(errp, "page size too small");
+return;
+}
+
+s->empty_page = g_malloc(s->page_size);
+memset(s->empty_page, 0xFF, s->page_size);
+
+address_space_init(&s->as, s->mr, "system-memory");
+}
+
+static void nrf51_nvmc_unrealize(DeviceState *dev, Error **errp)
+{
+Nrf51NVMCState *s = NRF51_NVMC(dev);
+
+g_free(s->empty_page);
+s->empty_page = NULL;
+
+}
+
+static Property nrf51_nvmc_properties[] = {
+DEFINE_PROP_UINT16("page_size", Nrf51NVMCState, page_size, 0x400),
+DEFINE_PROP_UINT32("code_size", Nrf51NVMCState, code_size, 0x100),
+DEFINE_PROP_LINK("memory", Nrf51NVMCState, mr, TYPE_MEMORY_REGION,
+ MemoryRegion *),
+DEFINE_PROP_END_OF_

Re: [Qemu-devel] [PATCH 0/2] block: Pacify Coverity

2018-06-26 Thread Kevin Wolf

Am 26.06.2018 um 10:05 hat Markus Armbruster geschrieben:
> Markus Armbruster (2):
>   block-qdict: Pacify Coverity after commit f1b34a248e9
>   block/crypto: Pacify Coverity after commit f853465aacb

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH] s390/boot: block uncompressed vmlinux booting attempts

On Tue, 26 Jun 2018 09:30:19 +0200
Christian Borntraeger  wrote:

> On 06/25/2018 05:09 PM, Vasily Gorbik wrote:
> > Since uncompressed kernel image "vmlinux" elf file is not bootable under
> > qemu anymore, add a check which would report that.
> > 
> > Qemu users are encouraged to use bzImage or
> > arch/s390/boot/compressed/vmlinux instead.
> > 
> > The check relies on s390 linux entry point ABI definition, which is only
> > present in bzImage and arch/s390/boot/compressed/vmlinux.
> > 
> > Signed-off-by: Vasily Gorbik   
> Acked-by: Christian Borntraeger 
> 
> some proposals regarding the wording below..
> 
> [...]
> > +
> > +   sclp_early_printk("The linux kernel boot failure: the image is 
> > corrupted or not bootable.\n");
> > +   sclp_early_printk("Please check that you are using bootable kernel 
> > image \"bzImage\".\n");
> > +   sclp_early_printk("(or alternatively 
> > \"arch/s390/boot/compressed/vmlinux\" image for qemu)\n");  
> 
> What about making this explain things a bit more, e.g. something like
> 
> Linux kernel boot failure: The boot image does not contain all necessary
> components (like the entry point and decompressor). The plain vmlinux ELF
> file no longer carries all necessary parts for starting up. Please use
> bzImage or arch/s390/boot/compressed/vmlinux.

Yes, that sounds good. With the changed message,

Acked-by: Cornelia Huck

Re: [Qemu-devel] s390 qemu boot failure in -next

On Mon, 25 Jun 2018 10:29:46 +0200
Christian Borntraeger  wrote:

> On 06/25/2018 10:05 AM, Cornelia Huck wrote:
> > On Mon, 25 Jun 2018 09:27:59 +0200
> > Christian Borntraeger  wrote:

> >> Something like this in QEMU 
> >>
> >> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> >> index f278036fa7..14153ce880 100644
> >> --- a/hw/s390x/ipl.c
> >> +++ b/hw/s390x/ipl.c
> >> @@ -187,11 +187,13 @@ static void s390_ipl_realize(DeviceState *dev, Error 
> >> **errp)
> >>   */
> >>  if (pentry == KERN_IMAGE_START || pentry == 0x800) {
> >>  ipl->start_addr = KERN_IMAGE_START;
> >> -/* Overwrite parameters in the kernel image, which are "rom" 
> >> */
> >> -strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> >>  } else {
> >>  ipl->start_addr = pentry;
> >>  }
> >> +   if (ipl->cmdline) {
> >> +/* If there is a command line, put it in the right place */
> >> +strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> >> +   }  
> > 
> > Check for the magic Linux string (like in the non-elf case) first?  
> 
> Even that does not exists in vmlinux but only in bzImage with the latest 
> patchset
> (in next, but not upstream yet)

Ok.

> >   
> >>  
> >>  if (ipl->initrd) {
> >>  ram_addr_t initrd_offset;
> >>
> >> would put the command line in no matter what the start address is.  
> > 
> > I'm for putting that one in (and backporting it to qemu-stable). It's a
> > bit worrying, though, that our ipl code is so fragile...  
> 
> We actually have to combine this with Thomas fix (to check for rom_ptr 
> returning
> something sane). It seems that ipl->commandline is always there, so we have to
> check for strlen!=0 it seems..
> 
> I mean if somebody ask for "-append something" we can certainly always write 
> something
> if there is rom/ram.

Given that the uncompressed image is not supposed to be bootable
anymore, does it make sense to add this anyway?

I'll go ahead and queue Thomas' fix, though.

[Qemu-devel] [PATCH] ramfb: fix overflow

2018-06-26 Thread Gerd Hoffmann

> CID 1393621:(OVERFLOW_BEFORE_WIDEN)
> Potentially overflowing expression "stride * s->height" with type "unsigned
> int" (32 bits, unsigned) is evaluated using +32-bit arithmetic, and then used
> in a context that expects an expression of type "hwaddr" (64 bits, unsigned).

Fix by changing stride from uint32_t to hwaddr.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/ramfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index 6867bce8ae..30f5c8da20 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -36,8 +36,8 @@ static void ramfb_fw_cfg_write(void *dev, off_t offset, 
size_t len)
 {
 RAMFBState *s = dev;
 void *framebuffer;
-uint32_t stride, fourcc, format;
-hwaddr addr, length;
+uint32_t fourcc, format;
+hwaddr stride, addr, length;
 
 s->width  = be32_to_cpu(s->cfg.width);
 s->height = be32_to_cpu(s->cfg.height);
-- 
2.9.3

Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties

Kevin Wolf  writes:

> The error handling policy was traditionally set with -drive, but with
> -blockdev it is no longer possible to set frontend options. scsi-disk
> (and other block devices) have long supported qdev properties to
> configure the error handling policy, so let's add these options to
> usb-storage as well and just forward them to the internal scsi-disk
> instance.
>
> Signed-off-by: Kevin Wolf 

Let's see whether I remember / figure out how this stuff works.

Not all devices support error handling policy, but scsi-disk does.

The preferred way to configure error handling policy is with qdev
properties rerror, werror.  blkconf_apply_backend_options() propagates
them to the associated BlockBackend.

The outmoded way is with -drive parameters rerror, werror.
blockdev_init() propagates them to the BlockBackend.

If you use both ways, qdev properties win.

usb-storage is a hack: it tries to look like a block device, but it's
really a SCSI controller that can serve only a single scsi-disk device,
which it creates automatically.

We want to deprecate -drive.  We also want to deprecate usb-storage, but
I guess we're still not ready for that (it's a complicated story).

To deprecate -drive without also deprecating usb-storage, we need to
preserve features.

The outmoded way works with usb-storage: the automatically created
scsi-disk is associated with the BlockBackend that got its error
handling policy from -drive.

The preferred way doesn't, because usb-storage lacks the qdev
properties.  This patch adds them.

Kind of sad, but it's either that or deprecating usb-storage.

Patch looks good to me, thus
Reviewed-by: Markus Armbruster 

Feel free to work whatever parts of my ramblings you find useful into
your commit message.

Re: [Qemu-devel] [PATCH v3] loader: Check access size when calling rom_ptr() to avoid crashes

On Fri, 15 Jun 2018 11:49:05 +0200
Thomas Huth  wrote:

> The rom_ptr() function allows direct access to the ROM blobs that we
> load during startup. However, there are currently no checks for the
> size of the accesses, so it's currently possible to crash QEMU for
> example with:
> 
> $ echo "Insane in the mainframe" > /tmp/test.txt
> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz
> Segmentation fault (core dumped)
> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt
> Segmentation fault (core dumped)
> $ echo -n HdrS > /tmp/hdr.txt
> $ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/hdr.txt -initrd 
> /tmp/hdr.txt
> Segmentation fault (core dumped)
> 
> We need a possibility to check the size of the ROM area that we want
> to access, thus let's add a size parameter to the rom_ptr() function
> to avoid these problems.
> 
> Signed-off-by: Thomas Huth 

Grmpf, this conflicts with "s390/ipl: fix ipl with -no-reboot" in
s390-next... do you want to rebase it?

> ---
>  v3: Fix the check in find_rom()
> 
>  hw/core/loader.c | 10 +-
>  hw/mips/mips_malta.c |  6 --
>  hw/s390x/ipl.c   | 13 ++---
>  hw/sparc/sun4m.c |  4 ++--
>  hw/sparc64/sun4u.c   |  4 ++--
>  include/hw/loader.h  |  2 +-
>  target/arm/cpu.c |  2 +-
>  7 files changed, 25 insertions(+), 16 deletions(-)

[Qemu-devel] [Bug 1778350] Re: Android-x86 4.4-r5 won't boot on QEMU since v2.11.0-rc2

2018-06-26 Thread navicrej

I don't know if there is another way to do that but I changed the
properties of x-pci-hole64-fix in hw/pci-host/piix.c and hw/pci-
host/q35.c to false on current master and compiled and now it works
again.

Maybe you can add a switch (unless there already is one) so one can
change it on runtime without compiling?

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

Title:
  Android-x86 4.4-r5 won't boot on QEMU since v2.11.0-rc2

Status in QEMU:
  New

Bug description:
  Try to boot from the Android-x86 4.4-r5 ISO won't boot in QEMU after 
2.11.0-rc2. The last known version it works is 2.11.0-rc1.
  It also works on the 2.10.x-line, even the 2.10.2 which was released after 
2.11.0-rc2!

  How to reproduce:
  Download the ISO from
  http://www.android-x86.org/releases/releasenote-4-4-r5 or directly 
https://www.fosshub.com/Android-x86.html/android-x86-4.4-r5.iso

  Start QEMU with this command-line: qemu-system-x86_64 -cdrom
  android-x86-4.4-r5.iso -m 1024

  On 2.11.0-rc1 and 2.10.2 after selecting to boot from CD it shows the Android 
splash after a short while ...
  On 2.11.0-rc2 through the latest 2.12 line it goes to black screen shortly 
right after selecting to boot from CD

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

[Qemu-devel] [PATCH] qapi/job: The next release will be 3.0

2018-06-26 Thread Kevin Wolf

Commit 51f63ec7d tried to change all references to 2.13 into 3.0, but
it failed to achieve this because it was not properly rebased on top of
the series introducing qapi/job.json. Change the references now.

Signed-off-by: Kevin Wolf 
---
 qapi/job.json | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/qapi/job.json b/qapi/job.json
index 9d074eb8d2..a121b615fb 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -104,7 +104,7 @@
 # @id: The job identifier
 # @status: The new job status
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'event': 'JOB_STATUS_CHANGE',
   'data': { 'id': 'str',
@@ -126,7 +126,7 @@
 #
 # @id: The job identifier.
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'command': 'job-pause', 'data': { 'id': 'str' } }
 
@@ -140,7 +140,7 @@
 #
 # @id : The job identifier.
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'command': 'job-resume', 'data': { 'id': 'str' } }
 
@@ -159,7 +159,7 @@
 #
 # @id: The job identifier.
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'command': 'job-cancel', 'data': { 'id': 'str' } }
 
@@ -171,7 +171,7 @@
 #
 # @id: The job identifier.
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'command': 'job-complete', 'data': { 'id': 'str' } }
 
@@ -187,7 +187,7 @@
 #
 # @id: The job identifier.
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'command': 'job-dismiss', 'data': { 'id': 'str' } }
 
@@ -205,7 +205,7 @@
 # @id: The identifier of any job in the transaction, or of a job that is not
 #  part of any transaction.
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'command': 'job-finalize', 'data': { 'id': 'str' } }
 
@@ -237,7 +237,7 @@
 #   the reason for the job failure. It should not be parsed
 #   by applications.
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'struct': 'JobInfo',
   'data': { 'id': 'str', 'type': 'JobType', 'status': 'JobStatus',
@@ -251,6 +251,6 @@
 #
 # Returns: a list with a @JobInfo for each active job
 #
-# Since: 2.13
+# Since: 3.0
 ##
 { 'command': 'query-jobs', 'returns': ['JobInfo'] }
-- 
2.13.6

Re: [Qemu-devel] [PATCH] migration: invalidate cache before source start

2018-06-26 Thread Vladimir Sementsov-Ogievskiy


25.06.2018 20:50, Dr. David Alan Gilbert wrote:

* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:

15.06.2018 15:06, Dr. David Alan Gilbert wrote:

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:

Invalidate cache before source start in case of failed migration.

Signed-off-by: Vladimir Sementsov-Ogievskiy 

Why doesn't the code at the bottom of migration_completion,
fail_invalidate:   and the code in migrate_fd_cancel   handle this?

What case did you see it in that those didn't handle?
(Also I guess it probably should set s->block_inactive = false)

on source I see:

81392@1529065750.766289:migrate_set_state new state 7
81392@1529065750.766330:migration_thread_file_err
81392@1529065750.766332:migration_thread_after_loop

so, we are leaving loop on
     if (qemu_file_get_error(s->to_dst_file)) {
     migrate_set_state(&s->state, current_active_state,
MIGRATION_STATUS_FAILED);
trace_migration_thread_file_err();
break;
     }

and skip migration_completion()



John is right, this ls an unrelated log, here we fail before 
inactivation and there are no problems.


Actual problem is when we fail in postcopy_start, at the end. And source 
log looks like:


84297@1530001796.287344:migrate_set_state new state 1
84297@1530001796.287374:migration_fd_outgoing fd=101
84297@1530001796.287383:migration_set_outgoing_channel 
ioc=0x56363454d630 ioctype=qio-channel-socket hostname=(null)

84297@1530001796.294032:migration_bitmap_sync_start
84297@1530001796.300483:migration_bitmap_sync_end dirty_pages 932
84297@1530001796.300561:migrate_set_state new state 4
84297@1530001796.300588:migration_thread_setup_complete
84297@1530001796.300593:migrate_pending pending size 1107976192 max 0 
(pre = 0 compat=1107976192 post=0)

84297@1530001796.300595:migrate_set_state new state 5
Tap fd 33 disable, ret 0
84297@1530001796.426477:migration_bitmap_sync_start
84297@1530001796.433227:migration_bitmap_sync_end dirty_pages 1091
84297@1530001796.439077:migrate_global_state_pre_save saved state: running
2018-06-26T08:29:56.439134Z qemu-kvm: postcopy_start: Migration stream 
errored -5

84297@1530001796.439141:migrate_set_state new state 7
84297@1530001796.439181:migration_thread_after_loop
Tap fd 33 enable, ret 0
84297@1530001796.453639:migrate_fd_cleanup
qemu-kvm: block/io.c:1655: bdrv_co_pwritev: Assertion `!(bs->open_flags 
& 0x0800)' failed.

2018-06-26 08:29:56.605+: shutting down, reason=crashed



Yeh, OK; I'd seen soemthing else a few days ago, where a cancellation
test that had previously ended with a 'cancelled' state has now ended up
in 'failed' (which is the state 7 you have above).
I suspect there's something else going on as well; I think what is
supposed to happen in the case of 'cancel' is that it spins in 'cancelling' for
a while in migrate_fd_cancel and then at the bottom of migrate_fd_cancel
it does the recovery, but because it's going to failed instead, then
it's jumping over that recovery.

Going back and actually looking at the patch again;
can I ask for 1 small change;
Can you set s->block_inactive = false   in the case where you
don't get the local_err (Like we do at the bottom of migrate_fd_cancel)


Does that make sense?


Ok, I'll resend.

Hm, looks like I'm fixing an outdated version (based on v2.9.0) And my 
reproduce isn't appropriate for upstream.

But looks like current code have a possibility of the same fail:

postcopy_start()
    
    ret = qemu_file_get_error(ms->to_dst_file);
    if (ret) {
    error_report("postcopy_start: Migration stream errored");

leads to "return MIG_ITERATE_SKIP;" in migration_iteration_run

then the loop should finish, as state should be MIGRATION_STATUS_FAILED, 
so we will not call migration_completion.


Hm, I have questions now:

1. should we check s->block_inactive, and if it is false, don't 
invalidate? it is done in migrate_fd_cancel(), but not don in 
migration_completion().
2. should we call qemu_mutex_lock_iothread() like in 
migration_completion()? Why is it needed in migration_completion(), when 
vm is not running?




Thanks,

Dave


Dave


Dave


---

   migration/migration.c | 9 -
   1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 1e99ec9b7e..8f39e0dc02 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2806,7 +2806,14 @@ static void migration_iteration_finish(MigrationState *s)
   case MIGRATION_STATUS_FAILED:
   case MIGRATION_STATUS_CANCELLED:
   if (s->vm_was_running) {
-vm_start();
+Error *local_err = NULL;
+bdrv_invalidate_cache_all(&local_err);
+if (local_err) {
+error_reportf_err(local_err, "Can't invalidate disks before "
+  "source vm start");
+} else {
+vm_start();
+}
   } else {

Re: [Qemu-devel] [RFC] arm: Add NRF51 SOC non-volatile memory controller

On 26.06.2018 10:17, Steffen Görtz wrote:
[...]
> +static const MemoryRegionOps io_ops = { .read = io_read, .write = io_write,
> +.endianness = DEVICE_LITTLE_ENDIAN, };

Could you please put the entries on a separate line each? That would be
better readable.

> +static void nrf51_nvmc_init(Object *obj)
> +{
> +Nrf51NVMCState *s = NRF51_NVMC(obj);
> +SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +
> +memory_region_init_io(&s->mmio, obj, &io_ops, s,
> +TYPE_NRF51_NVMC, NRF51_NVMC_SIZE);
Please indent the second line of the memory_region_init_io statement.

> +sysbus_init_mmio(sbd, &s->mmio);
> +}
[...]
> diff --git a/include/hw/nvram/nrf51_nvmc.h b/include/hw/nvram/nrf51_nvmc.h
> new file mode 100644
> index 00..dfd500b14e
> --- /dev/null
> +++ b/include/hw/nvram/nrf51_nvmc.h
> @@ -0,0 +1,52 @@
> +/*
> + * nrf51_nvmc.h
> + *
> + * Copyright 2018 Steffen Görtz 
> + *
> + * This code is licensed under the GPL version 2 or later.  See
> + * the COPYING file in the top-level directory.
> + *
> + * See Nrf51 reference manual 6 Non-Volatile Memory Controller (NVMC)
> + * See Nrf51 product sheet 8.22 NVMC specifications
> + *
> + * QEMU interface:
> + * + sysbus MMIO regions 0: Memory Region with registers
> + *   to be mapped to the peripherals instance address by the SOC.
> + * + page_size property to set the page size in bytes.
> + * + code_size property to set the code size in number of pages.
> + *
> + * Accuracy of the peripheral model:
> + * + The NVMC is always ready, all requested erase operations succeed
> + *   immediately.
> + * + CONFIG.WEN and CONFIG.EEN flags can be written and read back
> + *   but are not evaluated to check whether a requested write/erase operation
> + *   is legal.
> + * + Code regions (MPU configuration) are disregarded.
> + */
> +#ifndef NRF51_NVMC_H
> +#define NRF51_NVMC_H
> +
> +#include "hw/sysbus.h"
> +#include "qemu/timer.h"

Any reason for including timer.h here? If not, please drop that line.

> +#define TYPE_NRF51_NVMC "nrf51_soc.nvmc"
> +#define NRF51_NVMC(obj) OBJECT_CHECK(Nrf51NVMCState, (obj), TYPE_NRF51_NVMC)
> +
> +typedef struct Nrf51NVMCState {
> +SysBusDevice parent_obj;
> +
> +MemoryRegion mmio;
> +
> +uint32_t code_size;
> +uint16_t page_size;
> +uint8_t *empty_page;
> +MemoryRegion *mr;
> +AddressSpace as;
> +
> +struct {
> +uint32_t config:2;
> +} state;
> +
> +} Nrf51NVMCState;
> +
> +
> +#endif

 Thomas

Re: [Qemu-devel] [PULL 0/9] Target MIPS queue, 2018-06-25

On 25 June 2018 at 21:05, Aleksandar Markovic
 wrote:
> From: Aleksandar Markovic 
>
> The following changes since commit 35e238c9330669882487f9929e0aa97900431853:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
> 15:25:26 +0100)
>
> are available in the git repository at:
>
>   https://github.com/AMarkovic/qemu.git
>
> for you to fetch changes up to 2209731f338baaad3a1ef419fab1928e1e51ec17:
>
>   target/mips: Fix gdbstub to read/write 64 bit FP registers (2018-06-25 
> 21:21:24 +0200)

Hi -- something odd has happened in the creation of this
pull request email. The URL line should include a tag name,
and this one doesn't...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties

On 26/06/2018 10:35, Markus Armbruster wrote:
> We also want to deprecate usb-storage, but
> I guess we're still not ready for that (it's a complicated story).
> 
> To deprecate -drive without also deprecating usb-storage, we need to
> preserve features.

Patch looks good to me too, but indeed why can't we deprecate
usb-storage in favor of usb-bot + scsi-disk?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties

2018-06-26 Thread Daniel P . Berrangé

On Tue, Jun 26, 2018 at 10:48:10AM +0200, Paolo Bonzini wrote:
> On 26/06/2018 10:35, Markus Armbruster wrote:
> > We also want to deprecate usb-storage, but
> > I guess we're still not ready for that (it's a complicated story).
> > 
> > To deprecate -drive without also deprecating usb-storage, we need to
> > preserve features.
> 
> Patch looks good to me too, but indeed why can't we deprecate
> usb-storage in favor of usb-bot + scsi-disk?

Presumably the migration state of usb-storage is different from the
migration state of usb-bot + scsi-disk, so not a case where libvirt
could silently switch from one to the other. It would need to be an
explicit config change by apps/users.


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

Re: [Qemu-devel] s390 qemu boot failure in -next

2018-06-26 Thread Christian Borntraeger




On 06/26/2018 10:29 AM, Cornelia Huck wrote:
[...]
  
  if (ipl->initrd) {
  ram_addr_t initrd_offset;

 would put the command line in no matter what the start address is.  
>>>
>>> I'm for putting that one in (and backporting it to qemu-stable). It's a
>>> bit worrying, though, that our ipl code is so fragile...  
>>
>> We actually have to combine this with Thomas fix (to check for rom_ptr 
>> returning
>> something sane). It seems that ipl->commandline is always there, so we have 
>> to
>> check for strlen!=0 it seems..
>>
>> I mean if somebody ask for "-append something" we can certainly always write 
>> something
>> if there is rom/ram.
> 
> Given that the uncompressed image is not supposed to be bootable
> anymore, does it make sense to add this anyway?

I think we can drop this patch for now.

> 
> I'll go ahead and queue Thomas' fix, though.

yes, please

Re: [Qemu-devel] [qemu-s390x] [PATCH v3] loader: Check access size when calling rom_ptr() to avoid crashes

On 26.06.2018 10:40, Cornelia Huck wrote:
> On Fri, 15 Jun 2018 11:49:05 +0200
> Thomas Huth  wrote:
> 
>> The rom_ptr() function allows direct access to the ROM blobs that we
>> load during startup. However, there are currently no checks for the
>> size of the accesses, so it's currently possible to crash QEMU for
>> example with:
>>
>> $ echo "Insane in the mainframe" > /tmp/test.txt
>> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz
>> Segmentation fault (core dumped)
>> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt
>> Segmentation fault (core dumped)
>> $ echo -n HdrS > /tmp/hdr.txt
>> $ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/hdr.txt -initrd 
>> /tmp/hdr.txt
>> Segmentation fault (core dumped)
>>
>> We need a possibility to check the size of the ROM area that we want
>> to access, thus let's add a size parameter to the rom_ptr() function
>> to avoid these problems.
>>
>> Signed-off-by: Thomas Huth 
> 
> Grmpf, this conflicts with "s390/ipl: fix ipl with -no-reboot" in
> s390-next... do you want to rebase it?

Really? Where is the conflict? I can rebase my patch to your s390-next
branch, but the patch looks identical after that rebase. As far as I can
see, the patches only change different parts of the ipl.c file.

 Thomas

Re: [Qemu-devel] [qemu-s390x] [PATCH v3] loader: Check access size when calling rom_ptr() to avoid crashes

On Tue, 26 Jun 2018 10:55:09 +0200
Thomas Huth  wrote:

> On 26.06.2018 10:40, Cornelia Huck wrote:
> > On Fri, 15 Jun 2018 11:49:05 +0200
> > Thomas Huth  wrote:
> >   
> >> The rom_ptr() function allows direct access to the ROM blobs that we
> >> load during startup. However, there are currently no checks for the
> >> size of the accesses, so it's currently possible to crash QEMU for
> >> example with:
> >>
> >> $ echo "Insane in the mainframe" > /tmp/test.txt
> >> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz
> >> Segmentation fault (core dumped)
> >> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd 
> >> /tmp/test.txt
> >> Segmentation fault (core dumped)
> >> $ echo -n HdrS > /tmp/hdr.txt
> >> $ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/hdr.txt -initrd 
> >> /tmp/hdr.txt
> >> Segmentation fault (core dumped)
> >>
> >> We need a possibility to check the size of the ROM area that we want
> >> to access, thus let's add a size parameter to the rom_ptr() function
> >> to avoid these problems.
> >>
> >> Signed-off-by: Thomas Huth   
> > 
> > Grmpf, this conflicts with "s390/ipl: fix ipl with -no-reboot" in
> > s390-next... do you want to rebase it?  
> 
> Really? Where is the conflict? I can rebase my patch to your s390-next
> branch, but the patch looks identical after that rebase. As far as I can
> see, the patches only change different parts of the ipl.c file.

Yes, but the result does not compile :)

Re: [Qemu-devel] [PATCH] coverity-model: replay data is considered trusted

Paolo Bonzini  writes:

> On 26/06/2018 09:27, Markus Armbruster wrote:
>> 
>> Minimal fix:
>> 
>> diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
>> index 48b112393b..f987ce53b8 100644
>> --- a/scripts/coverity-model.c
>> +++ b/scripts/coverity-model.c
>> @@ -106,6 +106,7 @@ static int get_keysym(const name2keysym_t *table,
>>  /* Replay data is considered trusted.  */
>>  uint8_t replay_get_byte(void)
>>  {
>> +void *replay_file;
>>   uint8_t byte = 0;
>>   if (replay_file) {
>>   uint8_t c;
>> 
>> Alternatively, dumb down to:
>> 
>> /* Replay data is considered trusted.  */
>> uint8_t replay_get_byte(void)
>> {
>> uint8_t byte;
>> return byte;
>> }
>
> I wonder why the online service didn't complain.

Me too.  Of course, I should've run cov-make-library myself.  Next
time...

>   I guess the dumber
> version is more than enough!

I'll post it as a proper patch.  Thanks!

[Qemu-devel] [PATCH] coverity-model: Fix replay_get_byte()

Coverity 2018.06 chokes on replay_get_byte():

$ cov-make-library -of scripts/coverity-model.xmldb scripts/coverity-model.c
output file: scripts/coverity-model.xmldb
Compiling scripts/coverity-model.c with command 
/opt/cov-sa-2018.06/bin/cov-emit --dir 
/tmp/cov-armbru/930a6fb31e5f464fc1a53354b2deb66b/cov-make-library-emit -w 
--no_error_recovery --emit_header_functions --no_implicit_decl --preinclude 
/opt/cov-sa-2018.06/library/decls.h --c scripts/coverity-model.c
"scripts/coverity-model.c", line 110: error #20: identifier "replay_file" is
  undefined
   if (replay_file) {
   ^

Emit for file '/work/armbru/qemu/scripts/coverity-model.c' complete.
[ERROR] 1 error detected in the compilation of "scripts/coverity-model.c".
ERROR: cov-emit returned with code 1

Broken in commit 04a0afe5285.  Fix by dumbing down.

Signed-off-by: Markus Armbruster 
---
 scripts/coverity-model.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 48b112393b..2c0346ff25 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -106,12 +106,8 @@ static int get_keysym(const name2keysym_t *table,
 /* Replay data is considered trusted.  */
 uint8_t replay_get_byte(void)
 {
- uint8_t byte = 0;
- if (replay_file) {
- uint8_t c;
- byte = c;
- }
- return byte;
+uint8_t byte;
+return byte;
 }
 
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/5] i386: Add support for IA32_PRED_CMD and IA32_ARCH_CAPABILITIES MSRs

2018-06-26 Thread Robert Hoo

On Mon, 2018-06-25 at 13:51 +0200, Paolo Bonzini wrote:
> On 25/06/2018 05:39, Robert Hoo wrote:
> > IA32_PRED_CMD MSR gives software a way to issue commands that affect the 
> > state
> > of indirect branch predictors. Enumerated by CPUID.(EAX=7H,ECX=0):EDX[26].
> > IA32_ARCH_CAPABILITIES MSR enumerates architectural features of RDCL_NO and
> > IBRS_ALL. Enumerated by CPUID.(EAX=07H, ECX=0):EDX[29].
> > 
> > https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf
> > 
> > Signed-off-by: Robert Hoo 
> > ---
> >  target/i386/cpu.h |  4 
> >  target/i386/kvm.c | 27 ++-
> >  target/i386/machine.c | 40 
> >  3 files changed, 70 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 89c82be..734a73e 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -352,6 +352,8 @@ typedef enum X86Seg {
> >  #define MSR_TSC_ADJUST  0x003b
> >  #define MSR_IA32_SPEC_CTRL  0x48
> >  #define MSR_VIRT_SSBD   0xc001011f
> > +#define MSR_IA32_PRED_CMD   0x49
> > +#define MSR_IA32_ARCH_CAPABILITIES  0x10a
> >  #define MSR_IA32_TSCDEADLINE0x6e0
> >  
> >  #define FEATURE_CONTROL_LOCKED(1<<0)
> > @@ -1210,6 +1212,8 @@ typedef struct CPUX86State {
> >  
> >  uint64_t spec_ctrl;
> >  uint64_t virt_ssbd;
> > +uint64_t pred_cmd;
> > +uint64_t arch_capabilities;
> >  
> >  /* End of state preserved by INIT (dummy marker).  */
> >  struct {} end_init_save;
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 445e0e0..5232446 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -93,6 +93,8 @@ static bool has_msr_hv_reenlightenment;
> >  static bool has_msr_xss;
> >  static bool has_msr_spec_ctrl;
> >  static bool has_msr_virt_ssbd;
> > +static bool has_msr_pred_cmd;
> > +static bool has_msr_arch_capabilities;
> >  static bool has_msr_smi_count;
> >  
> >  static uint32_t has_architectural_pmu_version;
> > @@ -1258,6 +1260,11 @@ static int kvm_get_supported_msrs(KVMState *s)
> >  break;
> >  case MSR_VIRT_SSBD:
> >  has_msr_virt_ssbd = true;
> > +case MSR_IA32_PRED_CMD:
> > +has_msr_pred_cmd = true;
> > +break;
> > +case MSR_IA32_ARCH_CAPABILITIES:
> > +has_msr_arch_capabilities = true;
> >  break;
> >  }
> >  }
> > @@ -1750,7 +1757,13 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >  if (has_msr_virt_ssbd) {
> >  kvm_msr_entry_add(cpu, MSR_VIRT_SSBD, env->virt_ssbd);
> >  }
> > -
> > +if (has_msr_pred_cmd) {
> > +kvm_msr_entry_add(cpu, MSR_IA32_PRED_CMD, env->pred_cmd);
> > +}
> > +if (has_msr_arch_capabilities) {
> > +kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
> > +env->arch_capabilities);
> > +}
> >  #ifdef TARGET_X86_64
> >  if (lm_capable_kernel) {
> >  kvm_msr_entry_add(cpu, MSR_CSTAR, env->cstar);
> > @@ -2133,6 +2146,13 @@ static int kvm_get_msrs(X86CPU *cpu)
> >  if (has_msr_virt_ssbd) {
> >  kvm_msr_entry_add(cpu, MSR_VIRT_SSBD, 0);
> >  }
> > +if (has_msr_pred_cmd) {
> > +kvm_msr_entry_add(cpu, MSR_IA32_PRED_CMD, 0);
> > +}
> > +if (has_msr_arch_capabilities) {
> > +kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES, 0);
> > +}
> > +
> >  if (!env->tsc_valid) {
> >  kvm_msr_entry_add(cpu, MSR_IA32_TSC, 0);
> >  env->tsc_valid = !runstate_is_running();
> > @@ -2514,6 +2534,11 @@ static int kvm_get_msrs(X86CPU *cpu)
> >  break;
> >  case MSR_VIRT_SSBD:
> >  env->virt_ssbd = msrs[i].data;
> > +case MSR_IA32_PRED_CMD:
> > +env->pred_cmd = msrs[i].data;
> > +break;
> > +case MSR_IA32_ARCH_CAPABILITIES:
> > +env->arch_capabilities = msrs[i].data;
> >  break;
> >  case MSR_IA32_RTIT_CTL:
> >  env->msr_rtit_ctrl = msrs[i].data;
> > diff --git a/target/i386/machine.c b/target/i386/machine.c
> > index 4d98d36..089aba0 100644
> > --- a/target/i386/machine.c
> > +++ b/target/i386/machine.c
> > @@ -879,6 +879,44 @@ static const VMStateDescription vmstate_spec_ctrl = {
> >  }
> >  };
> >  
> > +static bool pred_cmd_needed(void *opaque)
> > +{
> > +X86CPU *cpu = opaque;
> > +CPUX86State *env = &cpu->env;
> > +
> > +return env->pred_cmd != 0;
> > +}
> > +
> > +static const VMStateDescription vmstate_pred_cmd = {
> > +.name = "cpu/pred_cmd",
> > +.version_id = 1,
> > +.minimum_version_id = 1,
> > +.needed = pred_cmd_needed,
> > +.fields = (VMStateField[]){
> > +VMSTATE_UINT64(env.arch_capabilities, X86CPU)

Re: [Qemu-devel] [PATCH 01/23] ppc/pnv: introduce a new intc_create() operation to the chip model

On 22 June 2018 at 05:24, David Gibson  wrote:
> From: Cédric Le Goater 
>
> On Power9, the thread interrupt presenter has a different type and is
> linked to the chip owning the cores.
>
> Signed-off-by: Cédric Le Goater 
> Signed-off-by: David Gibson 
> ---
>  hw/ppc/pnv.c | 21 +++--
>  hw/ppc/pnv_core.c| 18 +-
>  include/hw/ppc/pnv.h |  1 +
>  3 files changed, 29 insertions(+), 11 deletions(-)

Hi; Coverity points out a bug (CID 1393617) in this patch
(which is commit d35aefa9ae150a):

> @@ -143,13 +144,12 @@ static void pnv_core_realize(DeviceState *dev, Error 
> **errp)
>  void *obj;
>  int i, j;
>  char name[32];
> -Object *xi;
> +Object *chip;
>
> -xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
> -if (!xi) {
> -error_setg(errp, "%s: required link 'xics' not found: %s",
> -   __func__, error_get_pretty(local_err));
> -return;
> +chip = object_property_get_link(OBJECT(dev), "chip", &local_err);
> +if (!chip) {
> +error_propagate(errp, local_err);
> +error_prepend(errp, "required link 'chip' not found: ");
>  }

We check for a NULL 'chip' pointer, but forget the 'return', so
execution will plough on through to the code below and eventually
dereference the NULL pointer and segfault.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties

On 26/06/2018 10:53, Daniel P. Berrangé wrote:
> On Tue, Jun 26, 2018 at 10:48:10AM +0200, Paolo Bonzini wrote:
>> On 26/06/2018 10:35, Markus Armbruster wrote:
>>> We also want to deprecate usb-storage, but
>>> I guess we're still not ready for that (it's a complicated story).
>>>
>>> To deprecate -drive without also deprecating usb-storage, we need to
>>> preserve features.
>>
>> Patch looks good to me too, but indeed why can't we deprecate
>> usb-storage in favor of usb-bot + scsi-disk?
> 
> Presumably the migration state of usb-storage is different from the
> migration state of usb-bot + scsi-disk, so not a case where libvirt
> could silently switch from one to the other. It would need to be an
> explicit config change by apps/users.

>From a quick test, migrating

  qemu-kvm \
 -drive if=none,id=usb,file=$HOME/chipsec.img,snapshot=on \
 -usb -device usb-storage,drive=usb -M q35

into

  qemu-kvm \
 -drive if=none,id=usb,file=$HOME/chipsec.img,snapshot=on \
 -usb -device usb-bot -device scsi-disk,drive=usb -M q35

seems to work.  Both of them indeed use vmstate_usb_msd for
migration.

Paolo

Re: [Qemu-devel] [PATCH v3 06/13] 9p: darwin: Ignore O_{NOATIME, DIRECT}

2018-06-26 Thread Greg Kurz

On Sat, 16 Jun 2018 20:56:50 -0400
Keno Fischer  wrote:

> Darwin doesn't have either of these flags. Darwin does have
> F_NOCACHE, which is similar to O_DIRECT, but has different
> enough semantics that other projects don't generally map
> them automatically. In any case, we don't support O_DIRECT
> on Linux at the moment either.
> 
> Signed-off-by: Keno Fischer 
> ---

Reviewed-by: Greg Kurz 

>  hw/9pfs/9p.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 06139c9..e650459 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -123,11 +123,18 @@ static int dotl_to_open_flags(int flags)
>  { P9_DOTL_NONBLOCK, O_NONBLOCK } ,
>  { P9_DOTL_DSYNC, O_DSYNC },
>  { P9_DOTL_FASYNC, FASYNC },
> +#ifndef CONFIG_DARWIN
> +{ P9_DOTL_NOATIME, O_NOATIME },
> +/* On Darwin, we could map to F_NOCACHE, which is
> +   similar, but doesn't quite have the same
> +   semantics. However, we don't support O_DIRECT
> +   even on linux at the moment, so we just ignore
> +   it here. */
>  { P9_DOTL_DIRECT, O_DIRECT },
> +#endif
>  { P9_DOTL_LARGEFILE, O_LARGEFILE },
>  { P9_DOTL_DIRECTORY, O_DIRECTORY },
>  { P9_DOTL_NOFOLLOW, O_NOFOLLOW },
> -{ P9_DOTL_NOATIME, O_NOATIME },
>  { P9_DOTL_SYNC, O_SYNC },
>  };
>  
> @@ -156,10 +163,12 @@ static int get_dotl_openflags(V9fsState *s, int oflags)
>   */
>  flags = dotl_to_open_flags(oflags);
>  flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
> +#ifndef CONFIG_DARWIN
>  /*
>   * Ignore direct disk access hint until the server supports it.
>   */
>  flags &= ~O_DIRECT;
> +#endif
>  return flags;
>  }
>

Re: [Qemu-devel] [PATCH v2 5/7] intc/arm_gic: Add virtualization extensions logic

2018-06-26 Thread Luc Michel



On 06/25/2018 04:23 PM, Peter Maydell wrote:
> On 19 June 2018 at 10:31,   wrote:
>> From: Luc MICHEL 
>>
>> This commit adds the actual implementation of the GICv2 virtualization
>> extensions logic.
>>
>> For the vCPU interfaces, most of the existing CPU interface code is
>> reused. Calls to macros or functions modifying the distributor state are
>> replaced with equivalent generic inline functions that act either on the
>> distributor or on the virtual interface, given that the current CPU is a
>> physical or a virtual one. For vCPU MMIO, the call is simply forwarded
>> to the CPU read/write functions, with the CPU id being incremented by
>> GIC_CPU_NR (vCPU id = CPU id + GIC_CPU_NR), and by faking a secure
>> access (exposed vCPU interfaces do not implement security extensions.
>> Faking a secure access allows to go through the "secure or no security
>> extension" logic in the CPU interface).
>>
>> The gic_update logic is generalized to handle vCPUs. The main difference
>> is the way the best IRQ is selected. This part has been split into two
>> inline functions `gic_get_best_(v)irq`. The other difference is how we
>> determine if IRQ distribution is enabled or not (split in
>> `gic_dist_enabled`).
>>
>> Regarding list registers (LR) handling, some caching is done in the GIC
>> state to avoid repetitive LR iteration. The following information is
>> cached:
>>   * vIRQ to LR mapping, in s->virq_lr_entry,
>>   * the EISR and ELRSR registers,
>>   * the LRs in the pending state, in s->pending_lrs.
>> This cache is kept up-to-date with some maintenance functions, and is
>> recomputed when the GIC state is loaded from a VMState restoration.
> 
> Did you measure to determine whether the caching is worth the effort?
> The kernel has to rewrite the LR registers fairly often as it enters
> and exits the guest, whereas for instance it doesn't ever read the EISR
> at all as far as I can tell, and whenever it reads the ELRSR it then
> writes all the LRs, so LR writes are definitely more frequent than
> ELRSR reads. If you take my suggestion from patch 4 of implementing
> 4 LRs rather than 64 then the overhead of iterating them all is also
> 8x smaller.)
You are probably right, I didn't try to profile this part of the code to
see if the caching is worst the effort. Now that I see the way Xen uses
the GICv2, it's pretty clear that a "on-demand" computation of ELRSR and
EISR would probably be better.

However, I think the "virq_lr_entry" part of the caching is the
interesting one as it allows to resolve a vIRQ → LR mapping in O(1), and
is used in a lot of vCPU interface operations. But as you said, if we
default to 4 LRs, then the LR iteration cost is pretty low.

So, removing the caching mechanism would lead to:
  * no more h_eisr, h_elrsr, pending_lrs in the GIC state,
  * on-demand computation when needed,
  * The vIRQ → LR mapping could be kept, but with 4 LRs it's probably
not worst it.

What do you think? For now I'm preparing the v3 without caching at all.
I'll be able to do some profiling if needed between the two versions.

> 
>> Signed-off-by: Luc MICHEL 
>> ---
>>  hw/intc/arm_gic.c  | 648 +++--
>>  hw/intc/gic_internal.h | 198 +
>>  2 files changed, 754 insertions(+), 92 deletions(-)
> 
> This is painfully large to review. Compare the patchset that
> added virt support to GICv3, which broke things down into
> more digestible pieces (mostly ~250 lines a patch):
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg01243.html
OK :-) Sorry about that, let's split the beast then.
Thanks for the pre-review.

> 
> I've added a few review notes below but just scanned the rest,
> as it's too hard to review as it is.
> 
>> +static inline void gic_get_best_virq(GICState *s, int cpu,
>> + int *best_irq, int *best_prio, int 
>> *group)
>> +{
>> +int lr = 0;
>> +uint64_t lrs_in_use;
>> +
>> +/* In-use LRs have their corresponding bit cleared. */
>> +lrs_in_use = ~(s->h_elrsr[cpu]);
>> +
>> +#if GIC_NR_LR < 64
>> +/* Ignore unimplemented LRs */
>> +lrs_in_use &= (1 << GIC_NR_LR) - 1;
>> +#endif
> 
> MAKE_64BIT_MASK() allows you to avoid the ifdef.
> 
>> +
>> +*best_irq = 1023;
>> +*best_prio = 0x100;
>> +
>> +while (lrs_in_use) {
>> +if (lrs_in_use & 0x1) {
>> +uint32_t lr_entry = s->h_lr[lr][cpu];
>> +int state = GICH_LR_STATE(lr_entry);
>> +
>> +if (state == GICH_LR_STATE_PENDING) {
>> +int prio = GICH_LR_PRIORITY(lr_entry);
>> +
>> +if (prio < *best_prio) {
>> +*best_prio = prio;
>> +*best_irq = GICH_LR_VIRT_ID(lr_entry);
>> +*group = GICH_LR_GROUP(lr_entry);
>> +}
>> +}
>> +}
>> +
>> +lrs_in_use >>= 1;
>> +lr++;
>> +}
>> +}
>> +
>> +/* Return true if IRQ distribution is enabled:
>> + *  - in th

Re: [Qemu-devel] [PATCH 1/5] i386: Add support for IA32_PRED_CMD and IA32_ARCH_CAPABILITIES MSRs

On 26/06/2018 10:58, Robert Hoo wrote:
>> This is not needed, because pred_cmd is write only and arch_capabilities
>> is read only.  The guest cannot modify any of them.
>>
> Thanks Paolo. Yes, indeed unnecessary.
> I looked into the
> 336996-Speculative-Execution-Side-Channel-Mitigations.pdf again, looks
> like the vmstate_spec_ctrl is similar to pred_cmd, wirte only. Shall I
> remove it as well in v2 patch?

No, spec_ctrl is read-write.

Paolo

Re: [Qemu-devel] [PATCH v2 0/7] arm_gic: add virtualization extensions support

2018-06-26 Thread Luc Michel

On 06/25/2018 01:12 PM, Peter Maydell wrote:
> On 25 June 2018 at 05:55, Jan Kiszka  wrote:
>> On 2018-06-19 11:31, luc.mic...@greensocs.com wrote:
>>> From: Luc MICHEL 
>>>
>>> This patch series add support for the virtualization extensions in the
>>> ARM GICv2 interrupt controller.
>>>
>>> The first two commits do some refactoring to prepare for the
>>> implementation. Commits 3 and 4 are the actual implementation. The last
>>> commit updates the ZynqMP implementation to support virtualization.
>>
>> Is it enabled for the virt machine as well? Seems we are missing some
>> mapping of GICV and GICH there.
> 
> I agree that it would be very nice to see this enabled for the
> 'virt' board; it's a bit tricky for me to test otherwise. It
> should just be a matter of mapping in the GICV and GICH,
> wiring up the maintenance interrupt, and advertising it
> correctly in the DTB and ACPI data...
OK, I'll give it a look for the v3 then.
> 
> I'm hoping to get to reviewing this series this afternoon.
> 
> thanks
> -- PMM
> 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] coverity-model: Fix replay_get_byte()

On 26/06/2018 10:56, Markus Armbruster wrote:
> Coverity 2018.06 chokes on replay_get_byte():
> 
> $ cov-make-library -of scripts/coverity-model.xmldb 
> scripts/coverity-model.c
> output file: scripts/coverity-model.xmldb
> Compiling scripts/coverity-model.c with command 
> /opt/cov-sa-2018.06/bin/cov-emit --dir 
> /tmp/cov-armbru/930a6fb31e5f464fc1a53354b2deb66b/cov-make-library-emit -w 
> --no_error_recovery --emit_header_functions --no_implicit_decl --preinclude 
> /opt/cov-sa-2018.06/library/decls.h --c scripts/coverity-model.c
> "scripts/coverity-model.c", line 110: error #20: identifier "replay_file" 
> is
>   undefined
>if (replay_file) {
>^
> 
> Emit for file '/work/armbru/qemu/scripts/coverity-model.c' complete.
> [ERROR] 1 error detected in the compilation of "scripts/coverity-model.c".
> ERROR: cov-emit returned with code 1
> 
> Broken in commit 04a0afe5285.  Fix by dumbing down.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/coverity-model.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
> index 48b112393b..2c0346ff25 100644
> --- a/scripts/coverity-model.c
> +++ b/scripts/coverity-model.c
> @@ -106,12 +106,8 @@ static int get_keysym(const name2keysym_t *table,
>  /* Replay data is considered trusted.  */
>  uint8_t replay_get_byte(void)
>  {
> - uint8_t byte = 0;
> - if (replay_file) {
> - uint8_t c;
> - byte = c;
> - }
> - return byte;
> +uint8_t byte;
> +return byte;
>  }
>  
>  
> 

Reviewed-by: Paolo Bonzini

[Qemu-devel] [PATCH v4] loader: Check access size when calling rom_ptr() to avoid crashes

The rom_ptr() function allows direct access to the ROM blobs that we
load during startup. However, there are currently no checks for the
size of the accesses, so it's currently possible to crash QEMU for
example with:

$ echo "Insane in the mainframe" > /tmp/test.txt
$ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz
Segmentation fault (core dumped)
$ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt
Segmentation fault (core dumped)
$ echo -n HdrS > /tmp/hdr.txt
$ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/hdr.txt -initrd /tmp/hdr.txt
Segmentation fault (core dumped)

We need a possibility to check the size of the ROM area that we want
to access, thus let's add a size parameter to the rom_ptr() function
to avoid these problems.

Acked-by: Christian Borntraeger 
Signed-off-by: Thomas Huth 
---
 v4:
 - Rebase (two more spots were using rom_ptr now)
 - Remove INITRD_PARM_SIZE since it's unused now

 hw/core/loader.c | 10 +-
 hw/mips/mips_malta.c |  6 --
 hw/s390x/ipl.c   | 18 --
 hw/sparc/sun4m.c |  4 ++--
 hw/sparc64/sun4u.c   |  4 ++--
 include/hw/loader.h  |  2 +-
 target/arm/cpu.c |  2 +-
 7 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 06bdbca..bbb6e65 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -191,7 +191,7 @@ void pstrcpy_targphys(const char *name, hwaddr dest, int 
buf_size,
 rom_add_blob_fixed(name, source, (nulp - source) + 1, dest);
 } else {
 rom_add_blob_fixed(name, source, buf_size, dest);
-ptr = rom_ptr(dest + buf_size - 1);
+ptr = rom_ptr(dest + buf_size - 1, sizeof(*ptr));
 *ptr = 0;
 }
 }
@@ -1165,7 +1165,7 @@ void rom_reset_order_override(void)
 fw_cfg_reset_order_override(fw_cfg);
 }
 
-static Rom *find_rom(hwaddr addr)
+static Rom *find_rom(hwaddr addr, size_t size)
 {
 Rom *rom;
 
@@ -1179,7 +1179,7 @@ static Rom *find_rom(hwaddr addr)
 if (rom->addr > addr) {
 continue;
 }
-if (rom->addr + rom->romsize < addr) {
+if (rom->addr + rom->romsize < addr + size) {
 continue;
 }
 return rom;
@@ -1249,11 +1249,11 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
 return (d + l) - dest;
 }
 
-void *rom_ptr(hwaddr addr)
+void *rom_ptr(hwaddr addr, size_t size)
 {
 Rom *rom;
 
-rom = find_rom(addr);
+rom = find_rom(addr, size);
 if (!rom || !rom->data)
 return NULL;
 return rom->data + (addr - rom->addr);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 494f84e..3733e2f 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1133,11 +1133,13 @@ void mips_malta_init(MachineState *machine)
a neat trick which allows bi-endian firmware. */
 #ifndef TARGET_WORDS_BIGENDIAN
 {
-uint32_t *end, *addr = rom_ptr(FLASH_ADDRESS);
+uint32_t *end, *addr;
+const size_t swapsize = MIN(bios_size, 0x3e);
+addr = rom_ptr(FLASH_ADDRESS, swapsize);
 if (!addr) {
 addr = memory_region_get_ram_ptr(bios);
 }
-end = (void *)addr + MIN(bios_size, 0x3e);
+end = (void *)addr + swapsize;
 while (addr < end) {
 bswap32s(addr);
 addr++;
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index f278036..21f64ad 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -33,7 +33,6 @@
 #define KERN_PARM_AREA  0x010480UL
 #define INITRD_START0x80UL
 #define INITRD_PARM_START   0x010408UL
-#define INITRD_PARM_SIZE0x010410UL
 #define PARMFILE_START  0x001000UL
 #define ZIPL_IMAGE_START0x009000UL
 #define IPL_PSW_MASK(PSW_MASK_32 | PSW_MASK_64)
@@ -165,12 +164,12 @@ static void s390_ipl_realize(DeviceState *dev, Error 
**errp)
 goto error;
 }
 /* if this is Linux use KERN_IMAGE_START */
-magic = rom_ptr(LINUX_MAGIC_ADDR);
+magic = rom_ptr(LINUX_MAGIC_ADDR, 6);
 if (magic && !memcmp(magic, "S390EP", 6)) {
 pentry = KERN_IMAGE_START;
 } else {
 /* if not Linux load the address of the (short) IPL PSW */
-ipl_psw = rom_ptr(4);
+ipl_psw = rom_ptr(4, 4);
 if (ipl_psw) {
 pentry = be32_to_cpu(*ipl_psw) & 0x7fffUL;
 } else {
@@ -186,9 +185,12 @@ static void s390_ipl_realize(DeviceState *dev, Error 
**errp)
  * loader) and it won't work. For this case we force it to 0x1, 
too.
  */
 if (pentry == KERN_IMAGE_START || pentry == 0x800) {
+char *parm_area = rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdline) + 
1);
 ipl->start_addr = KERN_IMAGE_START;
 /*

Re: [Qemu-devel] [PATCH v3 4/4] tpm: add a fake ACPI memory clear interface

On Thu, Jun 21, 2018 at 4:33 PM, Igor Mammedov  wrote:
> On Thu, 21 Jun 2018 15:24:44 +0200
> Marc-André Lureau  wrote:
>
>> Hi
>>
>> On Thu, Jun 21, 2018 at 3:02 PM, Igor Mammedov  wrote:
>> > On Tue, 15 May 2018 14:14:33 +0200
>> > Marc-André Lureau  wrote:
>> >
>> >> This allows to pass the last failing test from the Windows HLK TPM 2.0
>> >> TCG PPI 1.3 tests.
>> >>
>> >> The interface is described in the "TCG Platform Reset Attack
>> >> Mitigation Specification", chapter 6 "ACPI _DSM Function". Whether or
>> >> not we should have a real implementation remains an open question to me.
>> > might it cause security issues?
>>
>> Good question. If the guest assumes success of this operation perhaps.
>> I'll check the spec.
>>
>> > What are implications of faking it and how hard it's to implement thing
>> > per spec?
>>
>> Laszlo answerd that in "[Qemu-devel] investigating TPM for
>> OVMF-on-QEMU"  2f2b) TCG Memory Clear Interface
> I get that it's optional, but we probably shouldn't advertise/fake
> feature if it's not supported.

As said in the commit message, the objective was to pass the Windows
HLK test. If we don't want to advertize a fake interface, I am fine
droping this patch. We'll have to revisit with Laszlo the work needed
in the firmware to support it.

>
>>
>> >
>> >
>> >> Signed-off-by: Marc-André Lureau 
>> >> ---
>> >>  hw/i386/acpi-build.c | 9 +
>> >>  1 file changed, 9 insertions(+)
>> >>
>> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> >> index 95be4f0710..392a1e50bd 100644
>> >> --- a/hw/i386/acpi-build.c
>> >> +++ b/hw/i386/acpi-build.c
>> >> @@ -2072,6 +2072,15 @@ build_tpm_ppi(Aml *dev)
>> >>  aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
>> >>  }
>> >>  aml_append(method, ifctx);
>> >> +
>> >> +   /* dummy MOR Memory Clear for the sake of WLK PPI test */
>> >> +ifctx = aml_if(
>> >> +aml_equal(aml_arg(0),
>> >> +  
>> >> aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
>> >> +{
>> >> +aml_append(ifctx, aml_return(aml_int(0)));
>> >> +}
>> >> +aml_append(method, ifctx);
>> >>  }
>> >>  aml_append(dev, method);
>> >>  }
>> >
>> >
>>
>>
>>
>



-- 
Marc-André Lureau

[Qemu-devel] [RFC v2] arm: Add NRF51 SOC non-volatile memory controller

Changes since V1:
- Code style changes

Signed-off-by: Steffen Görtz 
---
 hw/nvram/Makefile.objs|   1 +
 hw/nvram/nrf51_nvmc.c | 168 ++
 include/hw/nvram/nrf51_nvmc.h |  51 +++
 3 files changed, 220 insertions(+)
 create mode 100644 hw/nvram/nrf51_nvmc.c
 create mode 100644 include/hw/nvram/nrf51_nvmc.h

diff --git a/hw/nvram/Makefile.objs b/hw/nvram/Makefile.objs
index a912d25391..9edd61e8af 100644
--- a/hw/nvram/Makefile.objs
+++ b/hw/nvram/Makefile.objs
@@ -5,3 +5,4 @@ common-obj-y += fw_cfg.o
 common-obj-y += chrp_nvram.o
 common-obj-$(CONFIG_MAC_NVRAM) += mac_nvram.o
 obj-$(CONFIG_PSERIES) += spapr_nvram.o
+obj-$(CONFIG_NRF51_SOC) += nrf51_nvmc.o
diff --git a/hw/nvram/nrf51_nvmc.c b/hw/nvram/nrf51_nvmc.c
new file mode 100644
index 00..5dde3088a8
--- /dev/null
+++ b/hw/nvram/nrf51_nvmc.c
@@ -0,0 +1,168 @@
+/*
+ * nrf51_nvmc.c
+ *
+ * Copyright 2018 Steffen Görtz 
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "hw/nvram/nrf51_nvmc.h"
+#include "exec/address-spaces.h"
+
+#define NRF51_NVMC_SIZE 0x1000
+
+#define NRF51_NVMC_READY0x400
+#define NRF51_NVMC_READY_READY  0x01
+#define NRF51_NVMC_CONFIG   0x504
+#define NRF51_NVMC_CONFIG_MASK  0x03
+#define NRF51_NVMC_CONFIG_WEN   0x01
+#define NRF51_NVMC_CONFIG_EEN   0x02
+#define NRF51_NVMC_ERASEPCR10x508
+#define NRF51_NVMC_ERASEPCR00x510
+#define NRF51_NVMC_ERASEALL 0x50C
+#define NRF51_NVMC_ERASEUICR0x512
+#define NRF51_NVMC_ERASE0x01
+
+#define NRF51_UICR_OFFSET   0x10001000UL
+#define NRF51_UICR_SIZE 0x100
+
+static uint64_t io_read(void *opaque, hwaddr offset, unsigned int size)
+{
+Nrf51NVMCState *s = NRF51_NVMC(opaque);
+uint64_t r = 0;
+
+switch (offset) {
+case NRF51_NVMC_READY:
+r = NRF51_NVMC_READY_READY;
+break;
+case NRF51_NVMC_CONFIG:
+r = s->state.config;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: bad read offset 0x%" HWADDR_PRIx "\n", __func__, offset);
+}
+
+return r;
+}
+
+static void io_write(void *opaque, hwaddr offset, uint64_t value,
+unsigned int size)
+{
+Nrf51NVMCState *s = NRF51_NVMC(opaque);
+
+switch (offset) {
+case NRF51_NVMC_CONFIG:
+s->state.config = value & NRF51_NVMC_CONFIG_MASK;
+break;
+case NRF51_NVMC_ERASEPCR0:
+case NRF51_NVMC_ERASEPCR1:
+value &= ~(s->page_size - 1);
+if (value < (s->code_size * s->page_size)) {
+address_space_write(&s->as, value, MEMTXATTRS_UNSPECIFIED,
+s->empty_page, s->page_size);
+}
+break;
+case NRF51_NVMC_ERASEALL:
+if (value == NRF51_NVMC_ERASE) {
+for (uint32_t i = 0; i < s->code_size; i++) {
+address_space_write(&s->as, i * s->page_size,
+MEMTXATTRS_UNSPECIFIED, s->empty_page, s->page_size);
+}
+address_space_write(&s->as, NRF51_UICR_OFFSET,
+MEMTXATTRS_UNSPECIFIED, s->empty_page, NRF51_UICR_SIZE);
+}
+break;
+case NRF51_NVMC_ERASEUICR:
+if (value == NRF51_NVMC_ERASE) {
+address_space_write(&s->as, NRF51_UICR_OFFSET,
+MEMTXATTRS_UNSPECIFIED, s->empty_page, NRF51_UICR_SIZE);
+}
+break;
+
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: bad write offset 0x%" HWADDR_PRIx "\n", __func__, offset);
+}
+}
+
+static const MemoryRegionOps io_ops = {
+.read = io_read,
+.write = io_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void nrf51_nvmc_init(Object *obj)
+{
+Nrf51NVMCState *s = NRF51_NVMC(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+memory_region_init_io(&s->mmio, obj, &io_ops, s,
+  TYPE_NRF51_NVMC, NRF51_NVMC_SIZE);
+sysbus_init_mmio(sbd, &s->mmio);
+}
+
+static void nrf51_nvmc_realize(DeviceState *dev, Error **errp)
+{
+Nrf51NVMCState *s = NRF51_NVMC(dev);
+
+if (!s->mr) {
+error_setg(errp, "memory property was not set");
+return;
+}
+
+if (s->page_size < NRF51_UICR_SIZE) {
+error_setg(errp, "page size too small");
+return;
+}
+
+s->empty_page = g_malloc(s->page_size);
+memset(s->empty_page, 0xFF, s->page_size);
+
+address_space_init(&s->as, s->mr, "system-memory");
+}
+
+static void nrf51_nvmc_unrealize(DeviceState *dev, Error **errp)
+{
+Nrf51NVMCState *s = NRF51_NVMC(dev);
+
+g_free(s->empty_page);
+s->empty_page = NULL;
+
+}
+
+static Property nrf51_nvmc_properties[] = {
+DEFINE_PROP_UINT16("page_size", Nrf51NVMCState, page_size, 0x400),
+DEFINE_PROP_UINT32("code_size", Nrf51NVMCState, code_size, 0x100),
+DEFINE_PROP_LINK("memory", Nrf51NVMCState, mr, TY

Re: [Qemu-devel] [PATCH v2 0/7] arm_gic: add virtualization extensions support

2018-06-26 Thread Luc Michel



On 06/25/2018 06:55 AM, Jan Kiszka wrote:
> On 2018-06-19 11:31, luc.mic...@greensocs.com wrote:
>> From: Luc MICHEL 
>>
>> This patch series add support for the virtualization extensions in the
>> ARM GICv2 interrupt controller.
>>
>> The first two commits do some refactoring to prepare for the
>> implementation. Commits 3 and 4 are the actual implementation. The last
>> commit updates the ZynqMP implementation to support virtualization.
> 
> Is it enabled for the virt machine as well? Seems we are missing some
> mapping of GICV and GICH there.
> 
> And what is the recommended way to get the ZynqMP model running? I've a
> kernel that works on a ZCU102, but just passing it via -kernel to the
> QEMU model seems insufficient.
Hum, I think it should be enough. This is the way I run my tests with Xen:
qemu-system-aarch64 -M xlnx-zcu102,secure=on,virtualization=on \
-m 1024 \
-kernel xen-kernel -nographic -smp 1

-- 
Luc
> 
> Jan
> 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 00/35] target/arm SVE patches

2018-06-26 Thread Alex Bennée



Richard Henderson  writes:

> This is the remainder of the SVE enablement patches,
> with an extra bonus patch to enable ARMv8.2-DotProd.

Have a:

Tested-by: Alex Bennée 

Passes the whole sve-all-short test set with this series applied.

>
>
> r~
>
>
> Richard Henderson (35):
>   target/arm: Implement SVE Memory Contiguous Load Group
>   target/arm: Implement SVE Contiguous Load, first-fault and no-fault
>   target/arm: Implement SVE Memory Contiguous Store Group
>   target/arm: Implement SVE load and broadcast quadword
>   target/arm: Implement SVE integer convert to floating-point
>   target/arm: Implement SVE floating-point arithmetic (predicated)
>   target/arm: Implement SVE FP Multiply-Add Group
>   target/arm: Implement SVE Floating Point Accumulating Reduction Group
>   target/arm: Implement SVE load and broadcast element
>   target/arm: Implement SVE store vector/predicate register
>   target/arm: Implement SVE scatter stores
>   target/arm: Implement SVE prefetches
>   target/arm: Implement SVE gather loads
>   target/arm: Implement SVE first-fault gather loads
>   target/arm: Implement SVE scatter store vector immediate
>   target/arm: Implement SVE floating-point compare vectors
>   target/arm: Implement SVE floating-point arithmetic with immediate
>   target/arm: Implement SVE Floating Point Multiply Indexed Group
>   target/arm: Implement SVE FP Fast Reduction Group
>   target/arm: Implement SVE Floating Point Unary Operations-Unpredicated Group
>   target/arm: Implement SVE FP Compare with Zero Group
>   target/arm: Implement SVE floating-point trig multiply-add coefficient
>   target/arm: Implement SVE floating-point convert precision
>   target/arm: Implement SVE floating-point convert to integer
>   target/arm: Implement SVE floating-point round to integral value
>   target/arm: Implement SVE floating-point unary operations
>   target/arm: Implement SVE MOVPRFX
>   target/arm: Implement SVE floating-point complex add
>   target/arm: Implement SVE fp complex multiply add
>   target/arm: Pass index to AdvSIMD FCMLA (indexed)
>   target/arm: Implement SVE fp complex multiply add (indexed)
>   target/arm: Implement SVE dot product (vectors)
>   target/arm: Implement SVE dot product (indexed)
>   target/arm: Enable SVE for aarch64-linux-user
>   target/arm: Implement ARMv8.2-DotProd
>
>  target/arm/cpu.h   |1 +
>  target/arm/helper-sve.h|  682 ++
>  target/arm/helper.h|   44 +-
>  linux-user/elfload.c   |1 +
>  target/arm/cpu.c   |8 +
>  target/arm/cpu64.c |2 +
>  target/arm/helper.c|2 +-
>  target/arm/sve_helper.c| 1827 
>  target/arm/translate-a64.c |   57 +-
>  target/arm/translate-sve.c | 1691 -
>  target/arm/translate.c |   81 +-
>  target/arm/vec_helper.c|  283 +-
>  target/arm/sve.decode  |  422 +
>  13 files changed, 5039 insertions(+), 62 deletions(-)


--
Alex Bennée

[Qemu-devel] [Bug 1778350] Re: Android-x86 4.4-r5 won't boot on QEMU since v2.11.0-rc2

2018-06-26 Thread navicrej

Nevermind that, just found out how to run:

qemu-system-x86_64 -cdrom android-x86-4.4-r5.iso -m 1024 -global i440FX-
pcihost.x-pci-hole64-fix=off -global q35-pcihost.x-pci-hole64-fix=off

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

Title:
  Android-x86 4.4-r5 won't boot on QEMU since v2.11.0-rc2

Status in QEMU:
  New

Bug description:
  Try to boot from the Android-x86 4.4-r5 ISO won't boot in QEMU after 
2.11.0-rc2. The last known version it works is 2.11.0-rc1.
  It also works on the 2.10.x-line, even the 2.10.2 which was released after 
2.11.0-rc2!

  How to reproduce:
  Download the ISO from
  http://www.android-x86.org/releases/releasenote-4-4-r5 or directly 
https://www.fosshub.com/Android-x86.html/android-x86-4.4-r5.iso

  Start QEMU with this command-line: qemu-system-x86_64 -cdrom
  android-x86-4.4-r5.iso -m 1024

  On 2.11.0-rc1 and 2.10.2 after selecting to boot from CD it shows the Android 
splash after a short while ...
  On 2.11.0-rc2 through the latest 2.12 line it goes to black screen shortly 
right after selecting to boot from CD

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

Re: [Qemu-devel] [PATCH 01/23] ppc/pnv: introduce a new intc_create() operation to the chip model

2018-06-26 Thread Cédric Le Goater

On 06/26/2018 11:07 AM, Peter Maydell wrote:
> On 22 June 2018 at 05:24, David Gibson  wrote:
>> From: Cédric Le Goater 
>>
>> On Power9, the thread interrupt presenter has a different type and is
>> linked to the chip owning the cores.
>>
>> Signed-off-by: Cédric Le Goater 
>> Signed-off-by: David Gibson 
>> ---
>>  hw/ppc/pnv.c | 21 +++--
>>  hw/ppc/pnv_core.c| 18 +-
>>  include/hw/ppc/pnv.h |  1 +
>>  3 files changed, 29 insertions(+), 11 deletions(-)
> 
> Hi; Coverity points out a bug (CID 1393617) in this patch
> (which is commit d35aefa9ae150a):
> 
>> @@ -143,13 +144,12 @@ static void pnv_core_realize(DeviceState *dev, Error 
>> **errp)
>>  void *obj;
>>  int i, j;
>>  char name[32];
>> -Object *xi;
>> +Object *chip;
>>
>> -xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
>> -if (!xi) {
>> -error_setg(errp, "%s: required link 'xics' not found: %s",
>> -   __func__, error_get_pretty(local_err));
>> -return;
>> +chip = object_property_get_link(OBJECT(dev), "chip", &local_err);
>> +if (!chip) {
>> +error_propagate(errp, local_err);
>> +error_prepend(errp, "required link 'chip' not found: ");
>>  }
> 
> We check for a NULL 'chip' pointer, but forget the 'return', so
> execution will plough on through to the code below and eventually
> dereference the NULL pointer and segfault.

arg. My fault. I will send a fix.

Thanks,

C.

Re: [Qemu-devel] [qemu-s390x] [PATCH v3 2/9] s390x/kvm: pass values instead of pointers to kvm_s390_set_clock_*()

On 25.06.2018 18:03, Cornelia Huck wrote:
> On Mon, 25 Jun 2018 17:54:42 +0200
> David Hildenbrand  wrote:
> 
>> On 25.06.2018 17:50, Cornelia Huck wrote:
>>> On Mon, 25 Jun 2018 13:53:45 +0200
>>> David Hildenbrand  wrote:
>>>   
 We are going to factor out the TOD into a separate device and use const
 pointers for device class functions where possible. We are passing right
 now ordinary pointers that should never be touched when setting the TOD.
 Let's just pass the values directly.

 Signed-off-by: David Hildenbrand 
 ---
  target/s390x/cpu.c   |  4 ++--
  target/s390x/kvm-stub.c  |  4 ++--
  target/s390x/kvm.c   | 12 ++--
  target/s390x/kvm_s390x.h |  4 ++--
  4 files changed, 12 insertions(+), 12 deletions(-)

 diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
 index c268065887..68512e3e54 100644
 --- a/target/s390x/cpu.c
 +++ b/target/s390x/cpu.c
 @@ -413,9 +413,9 @@ int s390_set_clock(uint8_t *tod_high, uint64_t 
 *tod_low)  
>>>
>>> Any reason why you keep the pointers here?
>>>   
  int r = 0;
  
  if (kvm_enabled()) {
 -r = kvm_s390_set_clock_ext(tod_high, tod_low);
 +r = kvm_s390_set_clock_ext(*tod_high, *tod_low);
  if (r == -ENXIO) {
 -return kvm_s390_set_clock(tod_high, tod_low);
 +return kvm_s390_set_clock(*tod_high, *tod_low);  
>>>
>>> Especially as it would be more clean to check for !NULL before
>>> dereferencing...  
>>
>> See the next patch :)
>>
>> (I assume that refactoring code in order to rip it out does not make sense)
> 
> Add a comment in the commit message?
> 
> "Note that s390_set_clock() will be removed in a follow-on patch and
> therefore its calling convention is not changed."
> 

Sure I can do that. Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v5 01/35] target/arm: Implement SVE Memory Contiguous Load Group

2018-06-26 Thread Alex Bennée



Richard Henderson  writes:

> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  35 +
>  target/arm/sve_helper.c| 153 +
>  target/arm/translate-sve.c | 121 +
>  target/arm/sve.decode  |  34 +
>  4 files changed, 343 insertions(+)
>
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
> index 2e76084992..fcc9ba5f50 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -719,3 +719,38 @@ DEF_HELPER_FLAGS_5(gvec_rsqrts_s, TCG_CALL_NO_RWG,
> void, ptr, ptr, ptr, ptr, i32)
>  DEF_HELPER_FLAGS_5(gvec_rsqrts_d, TCG_CALL_NO_RWG,
> void, ptr, ptr, ptr, ptr, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ld1bb_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld2bb_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld3bb_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld4bb_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ld1hh_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld2hh_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld3hh_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld4hh_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ld1ss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld2ss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld3ss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld4ss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ld1dd_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld2dd_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld3dd_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld4dd_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ld1bhu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1bsu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1bdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1bhs_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1bss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1bds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ld1hsu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1hdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1hss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1hds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ld1sdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ld1sds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 128bbf9b04..4e6ad282f9 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -2810,3 +2810,156 @@ uint32_t HELPER(sve_while)(void *vd, uint32_t count, 
> uint32_t pred_desc)
>
>  return predtest_ones(d, oprsz, esz_mask);
>  }
> +
> +/*
> + * Load contiguous data, protected by a governing predicate.
> + */
> +#define DO_LD1(NAME, FN, TYPEE, TYPEM, H)  \
> +static void do_##NAME(CPUARMState *env, void *vd, void *vg, \
> +  target_ulong addr, intptr_t oprsz,   \
> +  uintptr_t ra)\
> +{  \
> +intptr_t i = 0;\
> +do {   \
> +uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));\
> +do {   \
> +TYPEM m = 0;   \
> +if (pg & 1) {  \
> +m = FN(env, addr, ra); \
> +}  \
> +*(TYPEE *)(vd + H(i)) = m; \
> +i += sizeof(TYPEE), pg >>= sizeof(TYPEE);  \
> +addr += sizeof(TYPEM); \
> +} while (i & 15);  \
> +} while (i < oprsz);   \
> +}  \
> +void HELPER(NAME)(CPUARMState *env, void *vg,  \
> +  target_ulong addr, uint32_t desc)\
> +{  \
> +do_##NAME(env, &env->vfp.zregs[simd_data(desc)], vg,   \
> +  addr, simd_oprsz(desc), GETPC());\
> +}
> +
> +#define DO_LD2(NAME, FN, TYPEE, TYPEM, H)  \
> +void HELPER(NAME)(CPUARMState *env, void *vg,  \
> +

Re: [Qemu-devel] [qemu-s390x] [PATCH v3 2/9] s390x/kvm: pass values instead of pointers to kvm_s390_set_clock_*()

On Tue, 26 Jun 2018 11:54:32 +0200
David Hildenbrand  wrote:

> On 25.06.2018 18:03, Cornelia Huck wrote:
> > On Mon, 25 Jun 2018 17:54:42 +0200
> > David Hildenbrand  wrote:
> >   
> >> On 25.06.2018 17:50, Cornelia Huck wrote:  
> >>> On Mon, 25 Jun 2018 13:53:45 +0200
> >>> David Hildenbrand  wrote:
> >>> 
>  We are going to factor out the TOD into a separate device and use const
>  pointers for device class functions where possible. We are passing right
>  now ordinary pointers that should never be touched when setting the TOD.
>  Let's just pass the values directly.
> 
>  Signed-off-by: David Hildenbrand 
>  ---
>   target/s390x/cpu.c   |  4 ++--
>   target/s390x/kvm-stub.c  |  4 ++--
>   target/s390x/kvm.c   | 12 ++--
>   target/s390x/kvm_s390x.h |  4 ++--
>   4 files changed, 12 insertions(+), 12 deletions(-)
> 
>  diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>  index c268065887..68512e3e54 100644
>  --- a/target/s390x/cpu.c
>  +++ b/target/s390x/cpu.c
>  @@ -413,9 +413,9 @@ int s390_set_clock(uint8_t *tod_high, uint64_t 
>  *tod_low)
> >>>
> >>> Any reason why you keep the pointers here?
> >>> 
>   int r = 0;
>   
>   if (kvm_enabled()) {
>  -r = kvm_s390_set_clock_ext(tod_high, tod_low);
>  +r = kvm_s390_set_clock_ext(*tod_high, *tod_low);
>   if (r == -ENXIO) {
>  -return kvm_s390_set_clock(tod_high, tod_low);
>  +return kvm_s390_set_clock(*tod_high, *tod_low);
> >>>
> >>> Especially as it would be more clean to check for !NULL before
> >>> dereferencing...
> >>
> >> See the next patch :)
> >>
> >> (I assume that refactoring code in order to rip it out does not make 
> >> sense)  
> > 
> > Add a comment in the commit message?
> > 
> > "Note that s390_set_clock() will be removed in a follow-on patch and
> > therefore its calling convention is not changed."
> >   
> 
> Sure I can do that. Thanks!
> 

Well, no need to respin if nothing else comes up, I can add that myself.

Re: [Qemu-devel] [PATCH v3 1/9] s390x/tcg: avoid overflows in time2tod/tod2time

On 25.06.2018 13:53, David Hildenbrand wrote:
> Big values for the TOD/ns clock can result in some overflows that can be
> avoided. Not all overflows can be handled however, as the conversion either
> multiplies by 4.096 or divided by 4.096.
> 
> Apply the trick used in the Linux kernel in arch/s390/include/asm/timex.h
> for tod_to_ns() and use the same trick also for the conversion in the
> other direction.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/internal.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index e392a02d12..6cf63340bf 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -243,13 +243,14 @@ enum cc_op {
>  /* Converts ns to s390's clock format */
>  static inline uint64_t time2tod(uint64_t ns)
>  {
> -return (ns << 9) / 125;
> +return (ns << 9) / 125 + (((ns & 0xff10ull) / 125) << 9);
> +
>  }
>  
>  /* Converts s390's clock format to ns */
>  static inline uint64_t tod2time(uint64_t t)
>  {
> -return (t * 125) >> 9;
> +return ((t >> 9) * 125) + (((t & 0x1ff) * 125) >> 9);
>  }
>  
>  static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
> 

Looks reasonable.

Reviewed-by: Thomas Huth

Re: [Qemu-devel] [PATCH v5 19/35] target/arm: Implement SVE FP Fast Reduction Group

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 35 ++
>  target/arm/sve_helper.c| 61 ++
>  target/arm/translate-sve.c | 57 +++
>  target/arm/sve.decode  |  8 +
>  4 files changed, 161 insertions(+)
>
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/1] Ide patches

On 25 June 2018 at 22:11, John Snow  wrote:
> The following changes since commit 35e238c9330669882487f9929e0aa97900431853:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
> 15:25:26 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to ae79c2db150e17757ee1be080481be675a15ccea:
>
>   ahci: fix FIS I bit and PIO Setup FIS interrupt (2018-06-25 16:50:48 -0400)
>
> 
> Pull request
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v5 20/35] target/arm: Implement SVE Floating Point Unary Operations - Unpredicated Group

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper.h|  8 +++
>  target/arm/translate-sve.c | 47 ++
>  target/arm/vec_helper.c| 20 
>  target/arm/sve.decode  |  5 
>  4 files changed, 80 insertions(+)
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 07/13] 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX

2018-06-26 Thread Greg Kurz

On Sat, 16 Jun 2018 20:56:51 -0400
Keno Fischer  wrote:

> Signed-off-by: Keno Fischer 
> ---
>  hw/9pfs/9p.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index e650459..abfb8dc 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3374,6 +3374,13 @@ out_nofid:
>  v9fs_string_free(&name);
>  }
>  
> +#if defined(CONFIG_DARWIN) && !defined(XATTR_SIZE_MAX)
> +/* Darwin doesn't seem to define a maximum xattr size in its user
> +   user space header, but looking at the kernel source, HFS supports
> +   up to INT32_MAX, so use that as the maximum.
> +*/
> +#define XATTR_SIZE_MAX INT32_MAX

XATTR_SIZE_MAX is used to cap the size that was received from the client,
and then we do:

xattr_fidp->fs.xattr.value = g_malloc0(size);

so I'd rather have something very much smaller than 4 gigs.

I now think that having an explicit dependency on XATTR_SIZE_MAX was a poor
choice actually. The problem isn't related to what the host supports, but
only to set a reasonable limit on what we accept from guests. We don't want
a buggy 9p client to crash QEMU.

TXATTRCREATE is only present in the 9p2000.L protocol, ie, linux guests,
which I don't expect to create extended attributes greater than 64k (ie,
XATTR_SIZE_MAX in the guest). We've been using this value for nearly two
years and nobody complained.

No need for a define. I guess you can open code the limit at the only
location where it is needed, with a short comment.

> +#endif
>  static void coroutine_fn v9fs_xattrcreate(void *opaque)
>  {
>  int flags, rflags = 0;

Re: [Qemu-devel] [PATCH v5 21/35] target/arm: Implement SVE FP Compare with Zero Group

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 42 +
>  target/arm/sve_helper.c| 43 ++
>  target/arm/translate-sve.c | 43 ++
>  target/arm/sve.decode  | 10 +
>  4 files changed, 138 insertions(+)
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/7] arm_gic: add virtualization extensions support

2018-06-26 Thread Jan Kiszka

On 2018-06-26 11:24, Luc Michel wrote:
> 
> 
> On 06/25/2018 06:55 AM, Jan Kiszka wrote:
>> On 2018-06-19 11:31, luc.mic...@greensocs.com wrote:
>>> From: Luc MICHEL 
>>>
>>> This patch series add support for the virtualization extensions in the
>>> ARM GICv2 interrupt controller.
>>>
>>> The first two commits do some refactoring to prepare for the
>>> implementation. Commits 3 and 4 are the actual implementation. The last
>>> commit updates the ZynqMP implementation to support virtualization.
>>
>> Is it enabled for the virt machine as well? Seems we are missing some
>> mapping of GICV and GICH there.
>>
>> And what is the recommended way to get the ZynqMP model running? I've a
>> kernel that works on a ZCU102, but just passing it via -kernel to the
>> QEMU model seems insufficient.
> Hum, I think it should be enough. This is the way I run my tests with Xen:
> qemu-system-aarch64 -M xlnx-zcu102,secure=on,virtualization=on \
> -m 1024 \
> -kernel xen-kernel -nographic -smp 1
> 

Just debugged this further a bit, and it seems my kernel is not getting
any device tree from qemu:

(gdb) bt
#0  cpu_relax () at ../arch/arm64/include/asm/processor.h:203
#1  setup_machine_fdt (dt_phys=256) at ../arch/arm64/kernel/setup.c:194
#2  setup_arch (cmdline_p=) at ../arch/arm64/kernel/setup.c:258
#3  0xff8008e30bc4 in start_kernel () at ../init/main.c:552
#4  0x in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) lx-dmesg 
[0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
[0.00] Linux version 4.17.0-00033-g67c2a9a37d59 (jan@md1f2u6c) (gcc 
version 7.2.1 20171011 (Linaro GCC 7.2-2017.11)) #17 SMP Tue Jun 26 12:16:56 
CEST 2018
(gdb) l setup_machine_fdt
178 pr_warn("Large number of MPIDR hash buckets 
detected\n");
179 }
180
181 static void __init setup_machine_fdt(phys_addr_t dt_phys)
182 {
183 void *dt_virt = fixmap_remap_fdt(dt_phys);
184 const char *name;
185
186 if (!dt_virt || !early_init_dt_scan(dt_virt)) {
187 pr_crit("\n"
188 "Error: invalid device tree blob at physical 
address %pa (virtual address 0x%p)\n"
189 "The dtb must be 8-byte aligned and must not 
exceed 2 MB in size\n"
190 "\nPlease check your bootloader.",
191 &dt_phys, dt_virt);
192
193 while (true)
194 cpu_relax();
[...]
(gdb) p dt_virt
$1 = (void *) 0x0

Jan

Re: [Qemu-devel] [PATCH v5 22/35] target/arm: Implement SVE floating-point trig multiply-add coefficient

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  4 +++
>  target/arm/sve_helper.c| 70 ++
>  target/arm/translate-sve.c | 27 +++
>  target/arm/sve.decode  |  3 ++
>  4 files changed, 104 insertions(+)


Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v2] crypto: Implement TLS Pre-Shared Keys (PSK).

2018-06-26 Thread Richard W.M. Jones

Pre-Shared Keys (PSK) is a simpler mechanism for enabling TLS
connections than using certificates.  It requires only a simple secret
key:

  $ mkdir -m 0700 /tmp/keys
  $ psktool -u rjones -p /tmp/keys/keys.psk
  $ cat /tmp/keys/keys.psk
  rjones:d543770c15ad93d76443fb56f501a31969235f47e999720ae8d2336f6a13fcbc

The key can be secretly shared between clients and servers.  Clients
must specify the directory containing the "keys.psk" file and a
username (defaults to "qemu").  Servers must specify only the
directory.

Example NBD client:

  $ qemu-img info \
--object 
tls-creds-psk,id=tls0,dir=/tmp/keys,username=rjones,endpoint=client \
--image-opts \

file.driver=nbd,file.host=localhost,file.port=10809,file.tls-creds=tls0,file.export=/

Example NBD server using qemu-nbd:

  $ qemu-nbd -t -x / \
--object tls-creds-psk,id=tls0,endpoint=server,dir=/tmp/keys \
--tls-creds tls0 \
image.qcow2

Example NBD server using nbdkit:

  $ nbdkit -n -e / -fv \
--tls=on --tls-psk=/tmp/keys/keys.psk \
file file=disk.img

Signed-off-by: Richard W.M. Jones 
---
 crypto/Makefile.objs |   1 +
 crypto/tlscredspsk.c | 300 +++
 crypto/tlssession.c  |  38 ++
 crypto/trace-events  |   3 +
 include/crypto/tlscredspsk.h | 106 +++
 qemu-doc.texi|  37 ++
 qemu-options.hx  |  24 
 7 files changed, 509 insertions(+)

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 2b99e08062..756bab111b 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -15,6 +15,7 @@ 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 += tlscredspsk.o
 crypto-obj-y += tlscredsx509.o
 crypto-obj-y += tlssession.o
 crypto-obj-y += secret.o
diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
new file mode 100644
index 00..e9ec23b457
--- /dev/null
+++ b/crypto/tlscredspsk.c
@@ -0,0 +1,300 @@
+/*
+ * QEMU crypto TLS Pre-Shared Keys (PSK) support
+ *
+ * Copyright (c) 2018 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/tlscredspsk.h"
+#include "tlscredspriv.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "trace.h"
+
+
+#ifdef CONFIG_GNUTLS
+
+static int
+lookup_key(const char *pskfile, const char *username, gnutls_datum_t *key,
+   Error **errp)
+{
+FILE *fp;
+char line[1024]; /* Maximum key length in psktool is 512 bytes. */
+size_t ulen = strlen(username);
+size_t len;
+
+fp = fopen(pskfile, "r");
+if (fp == NULL) {
+error_setg_errno(errp, errno, "Cannot open PSK file %s", pskfile);
+return -1;
+}
+while (fgets(line, sizeof line, fp) != NULL) {
+if (strncmp(line, username, ulen) == 0 && line[ulen] == ':') {
+len = strlen(line);
+if (len > 0 && line[len - 1] == '\n') {
+len--;
+line[len] = '\0';
+}
+key->data = (unsigned char *) g_strdup(&line[ulen + 1]);
+key->size = len - ulen - 1;
+fclose(fp);
+return 0;
+}
+}
+fclose(fp);
+error_setg(errp, "Username %s not found in PSK file %s",
+   username, pskfile);
+return -1;
+}
+
+static int
+qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
+   Error **errp)
+{
+char *pskfile = NULL, *dhparams = NULL;
+const char *username;
+int ret;
+int rv = -1;
+gnutls_datum_t key = { .data = NULL };
+
+trace_qcrypto_tls_creds_psk_load(creds,
+creds->parent_obj.dir ? creds->parent_obj.dir : "");
+
+if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
+if (qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_DH_PARAMS,
+   false, &dhparams, errp) < 0 ||
+qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_PSKFILE,
+   true, &pskfile, errp) < 0) {
+goto cleanup;
+}
+
+ret = gnutls_psk_allocate_server_credentials(&

[Qemu-devel] [PATCH v2] crypto: Implement TLS Pre-Shared Keys (PSK).

2018-06-26 Thread Richard W.M. Jones

v1 was here:

  https://lists.nongnu.org/archive/html/qemu-devel/2018-06/threads.html#07252

For v2:

 - Added documentation as suggested by Dan B.

 - Fixed a backwards test of creds->username which slipped into the
   previous version by accident because I was fiddling around with the
   code after I'd tested it but before posting it.  This version
   actually works.

Note also there is an nbdkit patch:

  https://www.redhat.com/archives/libguestfs/2018-June/thread.html#00121

nbdkit has an automated test that qemu TLS-PSK (NBD client)
interoperates with nbdkit (NBD server), so we can be confident that
the client functionality will receive continuous testing in future.

Rich.

Re: [Qemu-devel] [PATCH v2 0/7] arm_gic: add virtualization extensions support

2018-06-26 Thread Jan Kiszka

On 2018-06-26 12:22, Jan Kiszka wrote:
> On 2018-06-26 11:24, Luc Michel wrote:
>>
>>
>> On 06/25/2018 06:55 AM, Jan Kiszka wrote:
>>> On 2018-06-19 11:31, luc.mic...@greensocs.com wrote:
 From: Luc MICHEL 

 This patch series add support for the virtualization extensions in the
 ARM GICv2 interrupt controller.

 The first two commits do some refactoring to prepare for the
 implementation. Commits 3 and 4 are the actual implementation. The last
 commit updates the ZynqMP implementation to support virtualization.
>>>
>>> Is it enabled for the virt machine as well? Seems we are missing some
>>> mapping of GICV and GICH there.
>>>
>>> And what is the recommended way to get the ZynqMP model running? I've a
>>> kernel that works on a ZCU102, but just passing it via -kernel to the
>>> QEMU model seems insufficient.
>> Hum, I think it should be enough. This is the way I run my tests with Xen:
>> qemu-system-aarch64 -M xlnx-zcu102,secure=on,virtualization=on \
>> -m 1024 \
>> -kernel xen-kernel -nographic -smp 1
>>
> 
> Just debugged this further a bit, and it seems my kernel is not getting
> any device tree from qemu:
> 
> (gdb) bt
> #0  cpu_relax () at ../arch/arm64/include/asm/processor.h:203
> #1  setup_machine_fdt (dt_phys=256) at ../arch/arm64/kernel/setup.c:194
> #2  setup_arch (cmdline_p=) at ../arch/arm64/kernel/setup.c:258
> #3  0xff8008e30bc4 in start_kernel () at ../init/main.c:552
> #4  0x in ?? ()
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
> (gdb) lx-dmesg 
> [0.00] Booting Linux on physical CPU 0x00 [0x410fd034]
> [0.00] Linux version 4.17.0-00033-g67c2a9a37d59 (jan@md1f2u6c) (gcc 
> version 7.2.1 20171011 (Linaro GCC 7.2-2017.11)) #17 SMP Tue Jun 26 12:16:56 
> CEST 2018
> (gdb) l setup_machine_fdt
> 178 pr_warn("Large number of MPIDR hash buckets 
> detected\n");
> 179 }
> 180
> 181 static void __init setup_machine_fdt(phys_addr_t dt_phys)
> 182 {
> 183 void *dt_virt = fixmap_remap_fdt(dt_phys);
> 184 const char *name;
> 185
> 186 if (!dt_virt || !early_init_dt_scan(dt_virt)) {
> 187 pr_crit("\n"
> 188 "Error: invalid device tree blob at physical 
> address %pa (virtual address 0x%p)\n"
> 189 "The dtb must be 8-byte aligned and must not 
> exceed 2 MB in size\n"
> 190 "\nPlease check your bootloader.",
> 191 &dt_phys, dt_virt);
> 192
> 193 while (true)
> 194 cpu_relax();
> [...]
> (gdb) p dt_virt
> $1 = (void *) 0x0
> 
> Jan
> 

OK, feeding in -dtb arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revB.dtb
and setting the console to ttyPS0 gives now some output. Looks like this
can take me closer towards a testable setup.

Jan

Re: [Qemu-devel] [qemu-s390x] [PATCH v3 5/9] s390x/tcg: properly implement the TOD

On 25.06.2018 13:53, David Hildenbrand wrote:
> Right now, each CPU has its own TOD. Especially, the TOD will differ
> based on creation time of a CPU - e.g. when hotplugging a CPU the times
> will differ quite a lot, resulting in stall warnings in the guest.
> 
> Let's use a single TOD by implementing our new TOD device. Prepare it
> for TOD-clock epoch extension.
> 
> Most importantly, whenever we set the TOD, we have to update the CKC
> timer.
> 
> Introduce "tcg_s390x.h" just like "kvm_s390x.h" for tcg specific
> function declarations that should not go into cpu.h.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/tod-qemu.c| 46 ++
>  hw/s390x/tod.c | 11 +
>  include/hw/s390x/tod.h | 19 
>  target/s390x/cpu.c |  7 --
>  target/s390x/cpu.h |  1 -
>  target/s390x/internal.h| 16 -
>  target/s390x/misc_helper.c | 30 -
>  target/s390x/tcg_s390x.h   | 18 +++
>  8 files changed, 114 insertions(+), 34 deletions(-)
>  create mode 100644 target/s390x/tcg_s390x.h
> 
> diff --git a/hw/s390x/tod-qemu.c b/hw/s390x/tod-qemu.c
> index 03ea1ce4e8..941e84693e 100644
> --- a/hw/s390x/tod-qemu.c
> +++ b/hw/s390x/tod-qemu.c
> @@ -11,19 +11,43 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "hw/s390x/tod.h"
> +#include "qemu/timer.h"
> +#include "qemu/cutils.h"
> +#include "cpu.h"
> +#include "tcg_s390x.h"
>  
>  static void qemu_s390_tod_get(const S390TODState *td, S390TOD *tod,
>Error **errp)
>  {
> -/* FIXME */
> -tod->high = 0;
> -tod->low = 0;
> +*tod = td->base;
> +
> +tod->low += time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +if (tod->low < td->base.low) {
> +tod->high++;
> +}
>  }
>  
>  static void qemu_s390_tod_set(S390TODState *td, const S390TOD *tod,
>Error **errp)
>  {
> -/* FIXME */
> +CPUState *cpu;
> +
> +td->base = *tod;
> +
> +td->base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> +if (tod->low < td->base.low) {

Just a matter of taste, but I'd rather use "td->base.low > tod->low"
here instead (since the operations before and after the if-statement are
related to td->base).

> +td->base.high--;
> +}
> +
> +/*
> + * The TOD has been changed and we have to recalculate the CKC values
> + * for all CPUs. We do this asynchronously, as "SET CLOCK should be
> + * issued only while all other activity on all CPUs .. has been
> + * suspended".
> + */
> +CPU_FOREACH(cpu) {
> +async_run_on_cpu(cpu, tcg_s390_tod_updated, RUN_ON_CPU_NULL);
> +}
>  }
>  
>  static void qemu_s390_tod_class_init(ObjectClass *oc, void *data)
> @@ -34,10 +58,24 @@ static void qemu_s390_tod_class_init(ObjectClass *oc, 
> void *data)
>  tdc->set = qemu_s390_tod_set;
>  }
>  
> +static void qemu_s390_tod_init(Object *obj)
> +{
> +S390TODState *td = S390_TOD(obj);
> +struct tm tm;
> +
> +qemu_get_timedate(&tm, 0);
> +td->base.high = 0;
> +td->base.low = TOD_UNIX_EPOCH + (time2tod(mktimegm(&tm)) * 
> 10ULL);
> +if (td->base.low < TOD_UNIX_EPOCH) {
> +td->base.high += 1;
> +}
> +}

Nit: It would be sufficient to do this in the realize() function instead.

>  static TypeInfo qemu_s390_tod_info = {
>  .name = TYPE_QEMU_S390_TOD,
>  .parent = TYPE_S390_TOD,
>  .instance_size = sizeof(S390TODState),
> +.instance_init = qemu_s390_tod_init,
>  .class_init = qemu_s390_tod_class_init,
>  .class_size = sizeof(S390TODClass),
>  };
[...]
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index dd5273949b..be341b5295 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -28,6 +28,8 @@
>  #include "qemu/timer.h"
>  #include "exec/exec-all.h"
>  #include "exec/cpu_ldst.h"
> +#include "qapi/error.h"
> +#include "tcg_s390x.h"
>  
>  #if !defined(CONFIG_USER_ONLY)
>  #include "sysemu/cpus.h"
> @@ -39,6 +41,7 @@
>  #include "hw/s390x/ioinst.h"
>  #include "hw/s390x/s390-pci-inst.h"
>  #include "hw/boards.h"
> +#include "hw/s390x/tod.h"
>  #endif
>  
>  /* #define DEBUG_HELPER */
> @@ -138,25 +141,32 @@ void HELPER(spx)(CPUS390XState *env, uint64_t a1)
>  /* Store Clock */
>  uint64_t HELPER(stck)(CPUS390XState *env)
>  {
> -uint64_t time;
> +S390TODState *td = s390_get_todstate();
> +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
> +S390TOD tod;
>  
> -time = env->tod_offset +
> -time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> -
> -return time;
> +tdc->get(td, &tod, &error_abort);
> +return tod.low;
>  }
>  
>  /* Set Clock Comparator */
>  void HELPER(sckc)(CPUS390XState *env, uint64_t time)
>  {
> +S390TODState *td = s390_get_todstate();
> +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
> +S390TOD tod_base;
> +
>  if (time ==

Re: [Qemu-devel] [PATCH v3 2/4] acpi: add fw_cfg file for TPM and PPI virtual memory device

Hi

On Mon, Jun 25, 2018 at 5:20 PM, Laszlo Ersek  wrote:
> On 06/21/18 12:10, Marc-André Lureau wrote:
>> Hi
>>
>> On Thu, Jun 21, 2018 at 12:00 PM, Igor Mammedov  wrote:
>>> On Tue, 15 May 2018 14:14:31 +0200
>>> Marc-André Lureau  wrote:
>>>
 From: Stefan Berger 

 To avoid having to hard code the base address of the PPI virtual
 memory device we introduce a fw_cfg file etc/tpm/config that holds the
 base address of the PPI device, the version of the PPI interface and
 the version of the attached TPM.
>>> is it related to TPM_PPI_ADDR_BASE added in previous patch?
>>>

 Signed-off-by: Stefan Berger 
 [ Marc-André: renamed to etc/tpm/config, made it static, document it ]
 Signed-off-by: Marc-André Lureau 
 ---
  include/hw/acpi/tpm.h |  3 +++
  hw/i386/acpi-build.c  | 17 +
  docs/specs/tpm.txt| 20 
  3 files changed, 40 insertions(+)

 diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
 index c082df7d1d..f79d68a77a 100644
 --- a/include/hw/acpi/tpm.h
 +++ b/include/hw/acpi/tpm.h
 @@ -193,4 +193,7 @@ REG32(CRB_DATA_BUFFER, 0x80)
  #define TPM_PPI_ADDR_SIZE   0x400
  #define TPM_PPI_ADDR_BASE   0xFED45000

 +#define TPM_PPI_VERSION_NONE0
 +#define TPM_PPI_VERSION_1_301
 +
  #endif /* HW_ACPI_TPM_H */
 diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
 index 9bc6d97ea1..f6d447f03a 100644
 --- a/hw/i386/acpi-build.c
 +++ b/hw/i386/acpi-build.c
 @@ -119,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
  bool pcihp_bridge_en;
  } AcpiBuildPciBusHotplugState;

 +typedef struct FWCfgTPMConfig {
 +uint32_t tpmppi_address;
 +uint8_t tpm_version;
 +uint8_t tpmppi_version;
 +} QEMU_PACKED FWCfgTPMConfig;
 +
  static void init_common_fadt_data(Object *o, AcpiFadtData *data)
  {
  uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, 
 NULL);
 @@ -2873,6 +2879,7 @@ void acpi_setup(void)
  AcpiBuildTables tables;
  AcpiBuildState *build_state;
  Object *vmgenid_dev;
 +static FWCfgTPMConfig tpm_config;

  if (!pcms->fw_cfg) {
  ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
 @@ -2907,6 +2914,16 @@ void acpi_setup(void)
  fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
  tables.tcpalog->data, acpi_data_len(tables.tcpalog));

 +if (tpm_find()) {
 +tpm_config = (FWCfgTPMConfig) {
 +.tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
 +.tpm_version = cpu_to_le32(tpm_get_version(tpm_find())),
 +.tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_NONE)
 +};
 +fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
 +&tpm_config, sizeof tpm_config);
 +}
>>> why it's in ACPI part of the code, shouldn't it be a part of device,
>>> could TPM be used without ACPI at all (-noacpi CLI option)?
>>>
>>> Wouldn't adding fwcfg entry unconditionally break migration?
>>
>> Because of unstable entry IDs? that could be problematic. (especially
>> during boot time) What do you think Laszlo?
>>
>> I guess we could have a "ppi" device property, that would imply having
>> the etc/tpm/config fw_cfg entry. We would enable it by default in
>> newer machine types (3.0?)
>
> Can we perhaps draw a parallel with "-device vmcoreinfo" here? For that
> device model, fw_cfg_add_file_callback() is called in the realize
> function, vmcoreinfo_realize(). If libvirt generates the identical
> cmdline on both ends of the migration, and uses the same machine type, I
> think the fw_cfg selectors should end up the same on both sides.

-device vmcoreinfo is a  standalone fw-cfg entry. PPI is tied to a
TPM, the fw-cfg entry is an implementation detail between qemu and
firmware. So I prefer the "ppi" device property. Wrt to migration,
that should be equivalent to -device vmcoreinfo (except that -device
vmcoreinfo is not enabled by default in newer machine types)

> Thanks
> Laszlo
>
>>>
 +
  vmgenid_dev = find_vmgenid_dev();
  if (vmgenid_dev) {
  vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
 diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
 index c230c4c93e..2ddb768084 100644
 --- a/docs/specs/tpm.txt
 +++ b/docs/specs/tpm.txt
 @@ -20,6 +20,26 @@ QEMU files related to TPM TIS interface:
   - hw/tpm/tpm_tis.h


 += fw_cfg interface =
 +
 +The bios/firmware may use the "etc/tpm/config" fw_cfg entry for
 +configuring the guest appropriately.
 +
 +The entry of 6 bytes has the following content, in little-endian:
 +
 +#define TPM_VERSION_UNSPEC  0
 +#define TPM_VERSION_1_2 1
 +#define TPM_VERSION_2_0

Re: [Qemu-devel] [PATCH v5 23/35] target/arm: Implement SVE floating-point convert precision

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 13 +
>  target/arm/sve_helper.c| 27 +++
>  target/arm/translate-sve.c | 30 ++
>  target/arm/sve.decode  |  8 
>  4 files changed, 78 insertions(+)

> +static inline float32 sve_f16_to_f32(float16 f, float_status *s)
> +{
> +return float16_to_float32(f, true, s);
> +}
> +
> +static inline float64 sve_f16_to_f64(float16 f, float_status *s)
> +{
> +return float16_to_float64(f, true, s);
> +}
> +
> +static inline float16 sve_f32_to_f16(float32 f, float_status *s)
> +{
> +return float32_to_float16(f, true, s);
> +}
> +
> +static inline float16 sve_f64_to_f16(float64 f, float_status *s)
> +{
> +return float64_to_float16(f, true, s);
> +}

A comment to the effect that the SVE fp-to-fp conversion
routines always use IEEE format halfprec (ie ignore FPCR.AHP)
would be helpful.

Are you sure we have the FPCR.FZ16 handling right here? That
is, do we need the same "use the not-fp16 fpstatus pointer,
and temporarily clear the flush flag for the fp16 end of
the conversion" behaviour that we have in vfp_fcvt_f16_to_f32
and friends ? The pseudocode FPConvertSVE() calls FPConvert(),
which is the "ignore FZ16" codepath I think. The test case would
be (eg) a conversion where the input f16 is denormal and
FPCR.FZ == 1: this should not do the flush-input-to-zero, right?

thanks
-- PMM

[Qemu-devel] [RFC v2] arm: Add NRF51 random number generator peripheral

Add a model of the NRF51 random number generator peripheral.

Changes since v1:
  - Add implementation access size hints to MemoryRegionOps
  - Fail on error if qcrypto_random_bytes fails
  - Add references to Nrf51 datasheets

Signed-off-by: Steffen Görtz 
---
 hw/misc/Makefile.objs   |   1 +
 hw/misc/nrf51_rng.c | 242 
 include/hw/misc/nrf51_rng.h |  73 +++
 3 files changed, 316 insertions(+)
 create mode 100644 hw/misc/nrf51_rng.c
 create mode 100644 include/hw/misc/nrf51_rng.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 00e834d0f0..fd8cc97249 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -70,3 +70,4 @@ obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
 obj-y += mmio_interface.o
 obj-$(CONFIG_MSF2) += msf2-sysreg.o
+obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
diff --git a/hw/misc/nrf51_rng.c b/hw/misc/nrf51_rng.c
new file mode 100644
index 00..f9fdcd08e6
--- /dev/null
+++ b/hw/misc/nrf51_rng.c
@@ -0,0 +1,242 @@
+/*
+ * nrf51_rng.c
+ *
+ * Copyright 2018 Steffen Görtz 
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/misc/nrf51_rng.h"
+#include "crypto/random.h"
+
+#define NRF51_RNG_SIZE 0x1000
+
+#define NRF51_RNG_TASK_START   0x000
+#define NRF51_RNG_TASK_STOP0x004
+#define NRF51_RNG_EVENT_VALRDY 0x100
+#define NRF51_RNG_REG_SHORTS   0x200
+#define NRF51_RNG_REG_SHORTS_VALRDY_STOP 0
+#define NRF51_RNG_REG_INTEN0x300
+#define NRF51_RNG_REG_INTEN_VALRDY 0
+#define NRF51_RNG_REG_INTENSET 0x304
+#define NRF51_RNG_REG_INTENCLR 0x308
+#define NRF51_RNG_REG_CONFIG   0x504
+#define NRF51_RNG_REG_CONFIG_DECEN 0
+#define NRF51_RNG_REG_VALUE0x508
+
+#define NRF51_TRIGGER_TASK 0x01
+#define NRF51_EVENT_CLEAR  0x00
+
+
+static uint64_t rng_read(void *opaque, hwaddr offset, unsigned int size)
+{
+Nrf51RNGState *s = NRF51_RNG(opaque);
+uint64_t r = 0;
+
+switch (offset) {
+case NRF51_RNG_EVENT_VALRDY:
+r = s->state.event_valrdy;
+break;
+case NRF51_RNG_REG_SHORTS:
+r = s->state.shortcut_stop_on_valrdy;
+break;
+case NRF51_RNG_REG_INTEN:
+case NRF51_RNG_REG_INTENSET:
+case NRF51_RNG_REG_INTENCLR:
+r = s->state.interrupt_enabled;
+break;
+case NRF51_RNG_REG_CONFIG:
+r = s->state.filter_enabled;
+break;
+case NRF51_RNG_REG_VALUE:
+r = s->value;
+break;
+
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: bad read offset 0x%" HWADDR_PRIx "\n",
+  __func__, offset);
+}
+
+return r;
+}
+
+static int64_t calc_next_timeout(Nrf51RNGState *s)
+{
+int64_t timeout = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
+if (s->state.filter_enabled) {
+timeout += s->period_filtered_us;
+} else {
+timeout += s->period_unfiltered_us;
+}
+
+return timeout;
+}
+
+
+static void rng_update_timer(Nrf51RNGState *s)
+{
+if (s->state.active) {
+timer_mod(&s->timer, calc_next_timeout(s));
+} else {
+timer_del(&s->timer);
+}
+}
+
+
+static void rng_write(void *opaque, hwaddr offset,
+   uint64_t value, unsigned int size)
+{
+Nrf51RNGState *s = NRF51_RNG(opaque);
+
+switch (offset) {
+case NRF51_RNG_TASK_START:
+if (value == NRF51_TRIGGER_TASK) {
+s->state.active = 1;
+rng_update_timer(s);
+}
+break;
+case NRF51_RNG_TASK_STOP:
+if (value == NRF51_TRIGGER_TASK) {
+s->state.active = 0;
+rng_update_timer(s);
+}
+break;
+case NRF51_RNG_EVENT_VALRDY:
+if (value == NRF51_EVENT_CLEAR) {
+s->state.event_valrdy = 0;
+qemu_set_irq(s->eep_valrdy, 0);
+}
+break;
+case NRF51_RNG_REG_SHORTS:
+s->state.shortcut_stop_on_valrdy =
+(value & BIT_MASK(NRF51_RNG_REG_SHORTS_VALRDY_STOP)) ? 1 : 0;
+break;
+case NRF51_RNG_REG_INTEN:
+s->state.interrupt_enabled =
+(value & BIT_MASK(NRF51_RNG_REG_INTEN_VALRDY)) ? 1 : 0;
+break;
+case NRF51_RNG_REG_INTENSET:
+if (value & BIT_MASK(NRF51_RNG_REG_INTEN_VALRDY)) {
+s->state.interrupt_enabled = 1;
+}
+break;
+case NRF51_RNG_REG_INTENCLR:
+if (value & BIT_MASK(NRF51_RNG_REG_INTEN_VALRDY)) {
+s->state.interrupt_enabled = 0;
+}
+break;
+case NRF51_RNG_REG_CONFIG:
+s->state.filter_enabled =
+  (value & BIT_MASK(NRF51_RNG_REG_CONFIG_DECEN)) ? 1 : 0;
+break;
+
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: bad write offset 0x%" HWADDR_PRIx "\n",
+  __func__, offset);
+}
+}
+
+static const M

Re: [Qemu-devel] [PATCH v3 2/4] acpi: add fw_cfg file for TPM and PPI virtual memory device

2018-06-26 Thread Laszlo Ersek

On 06/26/18 12:38, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jun 25, 2018 at 5:20 PM, Laszlo Ersek  wrote:
>> On 06/21/18 12:10, Marc-André Lureau wrote:
>>> Hi
>>>
>>> On Thu, Jun 21, 2018 at 12:00 PM, Igor Mammedov  wrote:
 On Tue, 15 May 2018 14:14:31 +0200
 Marc-André Lureau  wrote:

> From: Stefan Berger 
>
> To avoid having to hard code the base address of the PPI virtual
> memory device we introduce a fw_cfg file etc/tpm/config that holds the
> base address of the PPI device, the version of the PPI interface and
> the version of the attached TPM.
 is it related to TPM_PPI_ADDR_BASE added in previous patch?

>
> Signed-off-by: Stefan Berger 
> [ Marc-André: renamed to etc/tpm/config, made it static, document it ]
> Signed-off-by: Marc-André Lureau 
> ---
>  include/hw/acpi/tpm.h |  3 +++
>  hw/i386/acpi-build.c  | 17 +
>  docs/specs/tpm.txt| 20 
>  3 files changed, 40 insertions(+)
>
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index c082df7d1d..f79d68a77a 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -193,4 +193,7 @@ REG32(CRB_DATA_BUFFER, 0x80)
>  #define TPM_PPI_ADDR_SIZE   0x400
>  #define TPM_PPI_ADDR_BASE   0xFED45000
>
> +#define TPM_PPI_VERSION_NONE0
> +#define TPM_PPI_VERSION_1_301
> +
>  #endif /* HW_ACPI_TPM_H */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9bc6d97ea1..f6d447f03a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -119,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
>  bool pcihp_bridge_en;
>  } AcpiBuildPciBusHotplugState;
>
> +typedef struct FWCfgTPMConfig {
> +uint32_t tpmppi_address;
> +uint8_t tpm_version;
> +uint8_t tpmppi_version;
> +} QEMU_PACKED FWCfgTPMConfig;
> +
>  static void init_common_fadt_data(Object *o, AcpiFadtData *data)
>  {
>  uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, 
> NULL);
> @@ -2873,6 +2879,7 @@ void acpi_setup(void)
>  AcpiBuildTables tables;
>  AcpiBuildState *build_state;
>  Object *vmgenid_dev;
> +static FWCfgTPMConfig tpm_config;
>
>  if (!pcms->fw_cfg) {
>  ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
> @@ -2907,6 +2914,16 @@ void acpi_setup(void)
>  fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>  tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>
> +if (tpm_find()) {
> +tpm_config = (FWCfgTPMConfig) {
> +.tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
> +.tpm_version = cpu_to_le32(tpm_get_version(tpm_find())),
> +.tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_NONE)
> +};
> +fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
> +&tpm_config, sizeof tpm_config);
> +}
 why it's in ACPI part of the code, shouldn't it be a part of device,
 could TPM be used without ACPI at all (-noacpi CLI option)?

 Wouldn't adding fwcfg entry unconditionally break migration?
>>>
>>> Because of unstable entry IDs? that could be problematic. (especially
>>> during boot time) What do you think Laszlo?
>>>
>>> I guess we could have a "ppi" device property, that would imply having
>>> the etc/tpm/config fw_cfg entry. We would enable it by default in
>>> newer machine types (3.0?)
>>
>> Can we perhaps draw a parallel with "-device vmcoreinfo" here? For that
>> device model, fw_cfg_add_file_callback() is called in the realize
>> function, vmcoreinfo_realize(). If libvirt generates the identical
>> cmdline on both ends of the migration, and uses the same machine type, I
>> think the fw_cfg selectors should end up the same on both sides.
> 
> -device vmcoreinfo is a  standalone fw-cfg entry. PPI is tied to a
> TPM, the fw-cfg entry is an implementation detail between qemu and
> firmware. So I prefer the "ppi" device property.

Ah, right, sorry I got confused for a moment. The TPM device makes sense
without the (optional) Physical Presence Interface too.

Thanks,
Laszlo

> Wrt to migration,
> that should be equivalent to -device vmcoreinfo (except that -device
> vmcoreinfo is not enabled by default in newer machine types)
> 
>> Thanks
>> Laszlo
>>

> +
>  vmgenid_dev = find_vmgenid_dev();
>  if (vmgenid_dev) {
>  vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index c230c4c93e..2ddb768084 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -20,6 +20,26 @@ QEMU files related to TPM TIS interface:
>   - hw/tpm/tpm_tis.h
>
>
> += fw_cfg interface =
> +
>

Re: [Qemu-devel] [PATCH v5 24/35] target/arm: Implement SVE floating-point convert to integer

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 


>  DO_ZPZ_FP(sve_fcvt_sh, uint32_t, H1_4, sve_f32_to_f16)
>  DO_ZPZ_FP(sve_fcvt_hs, uint32_t, H1_4, sve_f16_to_f32)
>  DO_ZPZ_FP(sve_fcvt_dh, uint64_t, , sve_f64_to_f16)
> @@ -3174,6 +3246,22 @@ DO_ZPZ_FP(sve_fcvt_hd, uint64_t, , sve_f16_to_f64)
>  DO_ZPZ_FP(sve_fcvt_ds, uint64_t, , float64_to_float32)
>  DO_ZPZ_FP(sve_fcvt_sd, uint64_t, , float32_to_float64)
>
> +DO_ZPZ_FP(sve_fcvtzs_hh, uint16_t, H1_2, vfp_float16_to_int16_rtz)
> +DO_ZPZ_FP(sve_fcvtzs_hs, uint32_t, H1_4, helper_vfp_tosizh)
> +DO_ZPZ_FP(sve_fcvtzs_ss, uint32_t, H1_4, helper_vfp_tosizs)
> +DO_ZPZ_FP(sve_fcvtzs_hd, uint64_t, , vfp_float16_to_int64_rtz)
> +DO_ZPZ_FP(sve_fcvtzs_sd, uint64_t, , vfp_float32_to_int64_rtz)
> +DO_ZPZ_FP(sve_fcvtzs_ds, uint64_t, , helper_vfp_tosizd)
> +DO_ZPZ_FP(sve_fcvtzs_dd, uint64_t, , vfp_float64_to_int64_rtz)
> +
> +DO_ZPZ_FP(sve_fcvtzu_hh, uint16_t, H1_2, vfp_float16_to_uint16_rtz)
> +DO_ZPZ_FP(sve_fcvtzu_hs, uint32_t, H1_4, helper_vfp_touizh)
> +DO_ZPZ_FP(sve_fcvtzu_ss, uint32_t, H1_4, helper_vfp_touizs)
> +DO_ZPZ_FP(sve_fcvtzu_hd, uint64_t, , vfp_float16_to_uint64_rtz)
> +DO_ZPZ_FP(sve_fcvtzu_sd, uint64_t, , vfp_float32_to_uint64_rtz)
> +DO_ZPZ_FP(sve_fcvtzu_ds, uint64_t, , helper_vfp_touizd)
> +DO_ZPZ_FP(sve_fcvtzu_dd, uint64_t, , vfp_float64_to_uint64_rtz)
> +

This kind of demonstrates that we've got a bit of an inconsistent
mess of naming with a lot of our fp related helper functions;
maybe we should regularize that someday.

Anyway
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/5] i386: Add CPUID bit and feature words for IA32_ARCH_CAPABILITIES MSR

2018-06-26 Thread Robert Hoo

On Mon, 2018-06-25 at 14:06 +0200, Paolo Bonzini wrote:
> On 25/06/2018 05:39, Robert Hoo wrote:
> > Support of IA32_PRED_CMD MSR already be enumerated by same CPUID bit as
> > SPEC_CTRL.
> > 
> > Signed-off-by: Robert Hoo 
> > ---
> >  target/i386/cpu.c | 2 +-
> >  target/i386/cpu.h | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 1e69e68..3134af4 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -896,7 +896,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
> > = {
> >  NULL, NULL, NULL, NULL,
> >  NULL, NULL, NULL, NULL,
> >  NULL, NULL, "spec-ctrl", NULL,
> > -NULL, NULL, NULL, "ssbd",
> > +NULL, "arch-capabilities", NULL, "ssbd",
> >  },
> >  .cpuid_eax = 7,
> >  .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 734a73e..1ef2040 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -688,6 +688,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> >  #define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2) /* AVX512 Neural Network 
> > Instructions */
> >  #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply 
> > Accumulation Single Precision */
> >  #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
> > +#define CPUID_7_0_EDX_ARCH_CAPABILITIES (1U << 29)  /*Arch Capabilities of 
> > RDCL_NO and IBRS_ALL*/
> >  #define CPUID_7_0_EDX_SPEC_CTRL_SSBD  (1U << 31) /* Speculative Store 
> > Bypass Disable */
> >  
> >  #define CPUID_8000_0008_EBX_IBPB(1U << 12) /* Indirect Branch 
> > Prediction Barrier */
> > 
> 
> For migration to work, you need to add new "features" corresponding to
> the bits in the MSR, and include them in the Icelake-Server and
> Icelake-Client models.  Unfortunately there is no code for this in QEMU
> yet, though the API is there in KVM.
> 
> I have just sent the KVM patch to pass the MSR value down to QEMU ("KVM:
> VMX: support MSR_IA32_ARCH_CAPABILITIES as a feature MSR").
> 
Thanks Paolo.
> Paolo

Re: [Qemu-devel] [PATCH v3 08/13] 9p: darwin: *xattr_nofollow implementations

2018-06-26 Thread Greg Kurz

On Sat, 16 Jun 2018 20:56:52 -0400
Keno Fischer  wrote:

> This implements the darwin equivalent of the functions that were
> moved to 9p-util(-linux) earlier in this series in the new
> 9p-util-darwin file.
> 
> Signed-off-by: Keno Fischer 
> ---
>  hw/9pfs/9p-util-darwin.c | 64 
> 
>  hw/9pfs/Makefile.objs|  1 +
>  2 files changed, 65 insertions(+)
>  create mode 100644 hw/9pfs/9p-util-darwin.c
> 
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> new file mode 100644
> index 000..cdb4c9e
> --- /dev/null
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -0,0 +1,64 @@
> +/*
> + * 9p utilities (Darwin Implementation)
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/xattr.h"
> +#include "9p-util.h"
> +
> +ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char 
> *name,
> + void *value, size_t size)
> +{
> +int ret;
> +int fd = openat_file(dirfd, filename,
> + O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +if (fd == -1) {
> +return -1;
> +}
> +ret = fgetxattr(fd, name, value, size, 0, 0);

Hmm... I had said it looked good, but not that good actually. This isn't
strictly equivalent of what we have with Linux, because it requires the
file to have appropriate access rights.

Why not just use the magic sycall from patch 11 and call getxattr() ?

> +close_preserve_errno(fd);
> +return ret;
> +}
> +
> +ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
> +  char *list, size_t size)
> +{
> +int ret;
> +int fd = openat_file(dirfd, filename,
> + O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +if (fd == -1) {
> +return -1;
> +}
> +ret = flistxattr(fd, list, size, 0);
> +close_preserve_errno(fd);
> +return ret;
> +}
> +
> +ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
> +const char *name)
> +{
> +int ret;
> +int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +if (fd == -1) {
> +return -1;
> +}
> +ret = fremovexattr(fd, name, 0);
> +close_preserve_errno(fd);
> +return ret;
> +}
> +
> +int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> + void *value, size_t size, int flags)
> +{
> +int ret;
> +int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +if (fd == -1) {
> +return -1;
> +}
> +ret = fsetxattr(fd, name, value, size, 0, flags);
> +close_preserve_errno(fd);
> +return ret;
> +}
> diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
> index 95e3bc0..0de39af 100644
> --- a/hw/9pfs/Makefile.objs
> +++ b/hw/9pfs/Makefile.objs
> @@ -1,6 +1,7 @@
>  ifeq ($(call lor,$(CONFIG_VIRTIO_9P),$(CONFIG_XEN)),y)
>  common-obj-y  = 9p.o
>  common-obj-$(CONFIG_LINUX) += 9p-util-linux.o
> +common-obj-$(CONFIG_DARWIN) += 9p-util-darwin.o
>  common-obj-y += 9p-local.o 9p-xattr.o
>  common-obj-y += 9p-xattr-user.o 9p-posix-acl.o
>  common-obj-y += coth.o cofs.o codir.o cofile.o

Re: [Qemu-devel] [PATCH] usb-storage: Add rerror/werror properties

2018-06-26 Thread Gerd Hoffmann

On Tue, Jun 26, 2018 at 10:48:10AM +0200, Paolo Bonzini wrote:
> On 26/06/2018 10:35, Markus Armbruster wrote:
> > We also want to deprecate usb-storage, but
> > I guess we're still not ready for that (it's a complicated story).
> > 
> > To deprecate -drive without also deprecating usb-storage, we need to
> > preserve features.
> 
> Patch looks good to me too, but indeed why can't we deprecate
> usb-storage in favor of usb-bot + scsi-disk?

I think we can.  One blocking issue was hotplug support, but that got fixed a
while ago, and I'm not aware of any other issue ...

cheers,
  Gerd

commit b78ecd0998094a8b7e0f14c4888f3a6b525d14ff
Author: Gerd Hoffmann 
Date:   Wed Jun 15 11:46:58 2016 +0200

usb-bot: hotplug support

This patch marks usb-bot as hot-pluggable device, makes attached
property settable and turns off auto-attach in case the device
was hotplugged.

Hot-plugging a usb-bot device with one or more scsi devices can be
done this way now:

  (1) device-add usb-bot,id=foo
  (2) device-add scsi-{hd,cd},bus=foo.0,lun=0
  (2b) optionally add more devices (luns 0 ... 15).
  (3) qom-set foo.attached = true

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Markus Armbruster 
Message-id: 1465984019-28963-5-git-send-email-kra...@redhat.com

Re: [Qemu-devel] [PATCH] qapi/job: The next release will be 3.0

Kevin Wolf  writes:

> Commit 51f63ec7d tried to change all references to 2.13 into 3.0, but
> it failed to achieve this because it was not properly rebased on top of
> the series introducing qapi/job.json. Change the references now.
>
> Signed-off-by: Kevin Wolf 

Can't see more of them.

Reviewed-by: Markus Armbruster

Re: [Qemu-devel] [PATCH] vga: set owner for mmio regions

2018-06-26 Thread Philippe Mathieu-Daudé

On 06/26/2018 03:09 AM, Gerd Hoffmann wrote:
> This makes sure the regions are properly cleaned when unplugging -device
> seconday-vga.
> 
> Reported-by: Thomas Huth 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  hw/display/vga_int.h|  1 +
>  hw/display/vga-pci.c| 11 ++-
>  hw/display/virtio-vga.c |  2 +-
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
> index 313cff84fc..f8fcf62a56 100644
> --- a/hw/display/vga_int.h
> +++ b/hw/display/vga_int.h
> @@ -193,6 +193,7 @@ extern const MemoryRegionOps vga_mem_ops;
>  
>  /* vga-pci.c */
>  void pci_std_vga_mmio_region_init(VGACommonState *s,
> +  Object *owner,
>MemoryRegion *parent,
>MemoryRegion *subs,
>bool qext);
> diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
> index 700ac58c69..1ea559762a 100644
> --- a/hw/display/vga-pci.c
> +++ b/hw/display/vga-pci.c
> @@ -192,22 +192,23 @@ static const MemoryRegionOps pci_vga_qext_ops = {
>  };
>  
>  void pci_std_vga_mmio_region_init(VGACommonState *s,
> +  Object *owner,
>MemoryRegion *parent,
>MemoryRegion *subs,
>bool qext)
>  {
> -memory_region_init_io(&subs[0], NULL, &pci_vga_ioport_ops, s,
> +memory_region_init_io(&subs[0], owner, &pci_vga_ioport_ops, s,
>"vga ioports remapped", PCI_VGA_IOPORT_SIZE);
>  memory_region_add_subregion(parent, PCI_VGA_IOPORT_OFFSET,
>  &subs[0]);
>  
> -memory_region_init_io(&subs[1], NULL, &pci_vga_bochs_ops, s,
> +memory_region_init_io(&subs[1], owner, &pci_vga_bochs_ops, s,
>"bochs dispi interface", PCI_VGA_BOCHS_SIZE);
>  memory_region_add_subregion(parent, PCI_VGA_BOCHS_OFFSET,
>  &subs[1]);
>  
>  if (qext) {
> -memory_region_init_io(&subs[2], NULL, &pci_vga_qext_ops, s,
> +memory_region_init_io(&subs[2], owner, &pci_vga_qext_ops, s,
>"qemu extended regs", PCI_VGA_QEXT_SIZE);
>  memory_region_add_subregion(parent, PCI_VGA_QEXT_OFFSET,
>  &subs[2]);
> @@ -239,7 +240,7 @@ static void pci_std_vga_realize(PCIDevice *dev, Error 
> **errp)
>  qext = true;
>  pci_set_byte(&d->dev.config[PCI_REVISION_ID], 2);
>  }
> -pci_std_vga_mmio_region_init(s, &d->mmio, d->mrs, qext);
> +pci_std_vga_mmio_region_init(s, OBJECT(dev), &d->mmio, d->mrs, qext);
>  
>  pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, 
> &d->mmio);
>  }
> @@ -275,7 +276,7 @@ static void pci_secondary_vga_realize(PCIDevice *dev, 
> Error **errp)
>  qext = true;
>  pci_set_byte(&d->dev.config[PCI_REVISION_ID], 2);
>  }
> -pci_std_vga_mmio_region_init(s, &d->mmio, d->mrs, qext);
> +pci_std_vga_mmio_region_init(s, OBJECT(dev), &d->mmio, d->mrs, qext);
>  
>  pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram);
>  pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index baa74ba82c..97db6c3372 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -152,7 +152,7 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, 
> Error **errp)
>  }
>  
>  /* add stdvga mmio regions */
> -pci_std_vga_mmio_region_init(vga, &vpci_dev->modern_bar,
> +pci_std_vga_mmio_region_init(vga, OBJECT(vvga), &vpci_dev->modern_bar,
>   vvga->vga_mrs, true);
>  
>  vga->con = g->scanout[0].con;
>

Re: [Qemu-devel] [PATCH v4] loader: Check access size when calling rom_ptr() to avoid crashes

On Tue, 26 Jun 2018 11:35:40 +0200
Thomas Huth  wrote:

> The rom_ptr() function allows direct access to the ROM blobs that we
> load during startup. However, there are currently no checks for the
> size of the accesses, so it's currently possible to crash QEMU for
> example with:
> 
> $ echo "Insane in the mainframe" > /tmp/test.txt
> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz
> Segmentation fault (core dumped)
> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt
> Segmentation fault (core dumped)
> $ echo -n HdrS > /tmp/hdr.txt
> $ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/hdr.txt -initrd 
> /tmp/hdr.txt
> Segmentation fault (core dumped)
> 
> We need a possibility to check the size of the ROM area that we want
> to access, thus let's add a size parameter to the rom_ptr() function
> to avoid these problems.
> 
> Acked-by: Christian Borntraeger 
> Signed-off-by: Thomas Huth 
> ---
>  v4:
>  - Rebase (two more spots were using rom_ptr now)
>  - Remove INITRD_PARM_SIZE since it's unused now
> 
>  hw/core/loader.c | 10 +-
>  hw/mips/mips_malta.c |  6 --
>  hw/s390x/ipl.c   | 18 --
>  hw/sparc/sun4m.c |  4 ++--
>  hw/sparc64/sun4u.c   |  4 ++--
>  include/hw/loader.h  |  2 +-
>  target/arm/cpu.c |  2 +-
>  7 files changed, 27 insertions(+), 19 deletions(-)

Thanks, applied.

Re: [Qemu-devel] [PATCH] qapi/job: The next release will be 3.0

2018-06-26 Thread Eric Blake


On 06/26/2018 03:43 AM, Kevin Wolf wrote:

Commit 51f63ec7d tried to change all references to 2.13 into 3.0, but
it failed to achieve this because it was not properly rebased on top of
the series introducing qapi/job.json. Change the references now.

Signed-off-by: Kevin Wolf 
---
  qapi/job.json | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PULL 00/12] x86 queue, 2018-06-25

On 25 June 2018 at 23:25, Eduardo Habkost  wrote:
> The following changes since commit 35e238c9330669882487f9929e0aa97900431853:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
> 15:25:26 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/x86-next-pull-request
>
> for you to fetch changes up to 0b6e9aa89e02c8b213af019aad816e00ba8243f8:
>
>   Merge branch 'master' of git://git.qemu.org/qemu into x86-next (2018-06-25 
> 14:10:56 -0300)
>
> 
> x86 queue, 2018-06-25
>
> * Add TOPOEXT feature to EPYC CPU model
> * AMD's amd-ssbd and amd-no-ssbd CPUID features
> * Removed unused CPUID flag names: ospke, osxsave
> * Better formatting of '-cpu help'
>
Applied, thanks.

-- PMM

[Qemu-devel] [Bug 1587065] Re: btrfs qemu-ga - multiple mounts block fsfreeze

2018-06-26 Thread  Christian Ehrhardt 

Fix is known and seems backportable, marking as server-next and subscribing for
somebody to take a look.
Also as mentioned it is in 2.9, so newer releases are already fixed.

** Also affects: qemu (Ubuntu Xenial)
Importance: Undecided
Status: New

** Changed in: qemu (Ubuntu)
Status: New => Fix Released

** Changed in: qemu (Ubuntu Xenial)
Status: New => Confirmed

** Tags added: server-next

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

Title:
btrfs qemu-ga - multiple mounts block fsfreeze

Status in QEMU:
Fix Released
Status in qemu package in Ubuntu:
Fix Released
Status in qemu source package in Xenial:
Confirmed

Bug description:
Having two mounts of the same device makes fsfreeze through qemu-ga
impossible.
root@CmsrvMTA:/# mount -l | grep /dev/vda2
/dev/vda2 on / type btrfs (rw,relatime,space_cache,subvolid=257,subvol=/@)
/dev/vda2 on /home type btrfs
(rw,relatime,space_cache,subvolid=258,subvol=/@home)

Having two mounts is rather common with btrfs, so the feature fsfreeze
is unusable on these systems.

Below more information about how we encountered this issue...

Message send to qemu-disc...@nongnu.org:

Message 1:
--
I use external snapshots to backup my guests. I use the 'quiesce' option to
flush and frees the guest file system with the qemu guest agent.

With the exeption of one guest, this procedure works fine. On the 'unwilling'
guest, I get this error message:
"ERROR 2016-05-25 00:51:19 | T25-bakVMSCmsrvVH2 | fout: internal error:
unable to execute QEMU agent command 'guest-fsfreeze-freeze': failed to freeze
/: Device or resource busy"

I don't think this is not some sort of time-out error, because
activation of the fsfreeze and the error message happen immediately
after each other:

$ grep qemu-ga syslog.1
May 25 00:51:19 CmsrvMTA qemu-ga: info: guest-fsfreeze called

This is the only entry of the qemu guest agent in syslog.

$ sudo virsh version
Compiled against library: libvirt 1.3.1
Using library: libvirt 1.3.1
Gebruikte API: QEMU 1.3.1
Draaiende hypervisor: QEMU 2.5.0

$ virsh qemu-agent-command CmsrvMTA '{"execute": "guest-info"}'
{"return":{"version":"2.5.0", ...
,{"enabled":true,"name":"guest-fstrim","success-response":true},{"enabled":true,"name":"guest-fsfreeze-thaw","success-response":true},{"enabled":true,"name":"guest-fsfreeze-status","success-response":true},{"enabled":true,"name":"guest-fsfreeze-freeze-list","success-response":true},{"enabled":true,"name":"guest-fsfreeze-freeze","success-response":true},
... }

For making an external snapshot, I use this command:
$ virsh snapshot-create-as --domain CmsrvMTA sn1 --disk-only --atomic
--quiesce --no-metadata --diskspec vda,file=/srv/poolVMS/CmsrvMTA.sn1

Piece of reply 1:
-
I have encountered this before. Some operating systems
may have bind-mounts that let a device appear multiple times in the mount
list. Unfortunately the guest agent is not smart enough to consider a device
that has been frozen as succesfull and keep going. This causes this specific
error.

Piece of reply 2:
-
Ok, that seems to be it.

I’ve got the ‘/’ and ‘/home’ on the same device formatted as btrfs on
two separate sub volumes.

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

Re: [Qemu-devel] [virtio-dev] Re: [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net

On Tue, 26 Jun 2018 04:46:03 +0300
"Michael S. Tsirkin"  wrote:

> On Mon, Jun 25, 2018 at 11:55:12AM +0200, Cornelia Huck wrote:
> > On Fri, 22 Jun 2018 22:05:50 +0300
> > "Michael S. Tsirkin"  wrote:
> >   
> > > On Fri, Jun 22, 2018 at 05:09:55PM +0200, Cornelia Huck wrote:  
> > > > On Thu, 21 Jun 2018 21:20:13 +0300
> > > > "Michael S. Tsirkin"  wrote:
> > > > 
> > > > > On Thu, Jun 21, 2018 at 04:59:13PM +0200, Cornelia Huck wrote:
> > > > > > OK, so what about the following:
> > > > > > 
> > > > > > - introduce a new feature bit, VIRTIO_NET_F_STANDBY_UUID that 
> > > > > > indicates
> > > > > >   that we have a new uuid field in the virtio-net config space
> > > > > > - in QEMU, add a property for virtio-net that allows to specify a 
> > > > > > uuid,
> > > > > >   offer VIRTIO_NET_F_STANDBY_UUID if set
> > > > > > - when configuring, set the property to the group UUID of the 
> > > > > > vfio-pci
> > > > > >   device
> > > > > > - in the guest, use the uuid from the virtio-net device's config 
> > > > > > space
> > > > > >   if applicable; else, fall back to matching by MAC as done today
> > > > > > 
> > > > > > That should work for all virtio transports.  
> > > > > 
> > > > > True. I'm a bit unhappy that it's virtio net specific though
> > > > > since down the road I expect we'll have a very similar feature
> > > > > for scsi (and maybe others).
> > > > > 
> > > > > But we do not have a way to have fields that are portable
> > > > > both across devices and transports, and I think it would
> > > > > be a useful addition. How would this work though? Any idea?
> > > > 
> > > > Can we introduce some kind of device-independent config space area?
> > > > Pushing back the device-specific config space by a certain value if the
> > > > appropriate feature is negotiated and use that for things like the 
> > > > uuid?
> > > 
> > > So config moves back and forth?
> > > Reminds me of the msi vector mess we had with pci.  
> > 
> > Yes, that would be a bit unfortunate.
> >   
> > > I'd rather have every transport add a new config.  
> > 
> > You mean via different mechanisms?  
> 
> I guess so.

Is there an alternate mechanism for pci to use? (Not so familiar with
it.)

For ccw, this needs more thought. We already introduced two commands
for reading/writing the config space (a concept that does not really
exist on s390). There's the generic read configuration data command,
but the data returned by it is not really generic enough. So we would
need one new command (or two, if we need to write as well). I'm not
sure about that yet.

> 
> > >   
> > > > But regardless of that, I'm not sure whether extending this approach to
> > > > other device types is the way to go. Tying together two different
> > > > devices is creating complicated situations at least in the hypervisor
> > > > (even if it's fairly straightforward in the guest). [I have not come
> > > > around again to look at the "how to handle visibility in QEMU"
> > > > questions due to lack of cycles, sorry about that.]
> > > > 
> > > > So, what's the goal of this approach? Only to allow migration with
> > > > vfio-pci, or also to plug in a faster device and use it instead of an
> > > > already attached paravirtualized device?
> > > 
> > > These are two sides of the same coin, I think the second approach
> > > is closer to what we are doing here.  
> > 
> > Thinking about it, do we need any knob to keep the vfio device
> > invisible if the virtio device is not present? IOW, how does the
> > hypervisor know that the vfio device is supposed to be paired with a
> > virtio device? It seems we need an explicit tie-in.  
> 
> If we are going the way of the bridge, both bridge and
> virtio would have some kind of id.

So the presence of the id would indicate "this is one part of a pair"?

> 
> When pairing using mac, I'm less sure. PAss vfio device mac to qemu
> as a property?

That feels a bit odd. "This is the vfio device's mac, use this instead
of your usual mac property"? As we have not designed the QEMU interface
yet, just go with the id in any case? The guest can still match by mac.

> > > > What about migration of vfio devices that are not easily replaced by a
> > > > paravirtualized device? I'm thinking of vfio-ccw, where our main (and
> > > > currently only) supported device is dasd (disks) -- which can do a lot
> > > > of specialized things that virtio-blk does not support (and should not
> > > > or even cannot support).
> > > 
> > > But maybe virtio-scsi can?  
> > 
> > I don't think so. Dasds have some channel commands that don't map
> > easily to scsi commands.  
> 
> There's always a choice of adding these to the spec.
> E.g. FC extensions were proposed, I don't remember why they
> are still stuck.

FC extensions are a completely different kind of enhancements, though.
For a start, they are not unique to a certain transport.

Also, we have a whole list of special dasd issues. Weird disk layout
for eckd, low-level disk form

Re: [Qemu-devel] [qemu-s390x] [PATCH v3 5/9] s390x/tcg: properly implement the TOD

>> +td->base = *tod;
>> +
>> +td->base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>> +if (tod->low < td->base.low) {
> 
> Just a matter of taste, but I'd rather use "td->base.low > tod->low"
> here instead (since the operations before and after the if-statement are
> related to td->base).

Changed.

> 
>> +td->base.high--;
>> +}
>> +
>> +/*
>> + * The TOD has been changed and we have to recalculate the CKC values
>> + * for all CPUs. We do this asynchronously, as "SET CLOCK should be
>> + * issued only while all other activity on all CPUs .. has been
>> + * suspended".
>> + */
>> +CPU_FOREACH(cpu) {
>> +async_run_on_cpu(cpu, tcg_s390_tod_updated, RUN_ON_CPU_NULL);
>> +}
>>  }
>>  
>>  static void qemu_s390_tod_class_init(ObjectClass *oc, void *data)
>> @@ -34,10 +58,24 @@ static void qemu_s390_tod_class_init(ObjectClass *oc, 
>> void *data)
>>  tdc->set = qemu_s390_tod_set;
>>  }
>>  
>> +static void qemu_s390_tod_init(Object *obj)
>> +{
>> +S390TODState *td = S390_TOD(obj);
>> +struct tm tm;
>> +
>> +qemu_get_timedate(&tm, 0);
>> +td->base.high = 0;
>> +td->base.low = TOD_UNIX_EPOCH + (time2tod(mktimegm(&tm)) * 
>> 10ULL);
>> +if (td->base.low < TOD_UNIX_EPOCH) {
>> +td->base.high += 1;
>> +}
>> +}
> 
> Nit: It would be sufficient to do this in the realize() function instead.

Then I'll have to overwrite the realize function and store the
parent_realize function - something that I want to avoid if not really
necessary.

(for now it was also done in the cpu initfn, so that should be fine)

> 
>>  static TypeInfo qemu_s390_tod_info = {
>>  .name = TYPE_QEMU_S390_TOD,
>>  .parent = TYPE_S390_TOD,
>>  .instance_size = sizeof(S390TODState),
>> +.instance_init = qemu_s390_tod_init,
>>  .class_init = qemu_s390_tod_class_init,
>>  .class_size = sizeof(S390TODClass),
>>  };
> [...]
>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
>> index dd5273949b..be341b5295 100644
>> --- a/target/s390x/misc_helper.c
>> +++ b/target/s390x/misc_helper.c
>> @@ -28,6 +28,8 @@
>>  #include "qemu/timer.h"
>>  #include "exec/exec-all.h"
>>  #include "exec/cpu_ldst.h"
>> +#include "qapi/error.h"
>> +#include "tcg_s390x.h"
>>  
>>  #if !defined(CONFIG_USER_ONLY)
>>  #include "sysemu/cpus.h"
>> @@ -39,6 +41,7 @@
>>  #include "hw/s390x/ioinst.h"
>>  #include "hw/s390x/s390-pci-inst.h"
>>  #include "hw/boards.h"
>> +#include "hw/s390x/tod.h"
>>  #endif
>>  
>>  /* #define DEBUG_HELPER */
>> @@ -138,25 +141,32 @@ void HELPER(spx)(CPUS390XState *env, uint64_t a1)
>>  /* Store Clock */
>>  uint64_t HELPER(stck)(CPUS390XState *env)
>>  {
>> -uint64_t time;
>> +S390TODState *td = s390_get_todstate();
>> +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
>> +S390TOD tod;
>>  
>> -time = env->tod_offset +
>> -time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>> -
>> -return time;
>> +tdc->get(td, &tod, &error_abort);
>> +return tod.low;
>>  }
>>  
>>  /* Set Clock Comparator */
>>  void HELPER(sckc)(CPUS390XState *env, uint64_t time)
>>  {
>> +S390TODState *td = s390_get_todstate();
>> +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
>> +S390TOD tod_base;
>> +
>>  if (time == -1ULL) {
>>  return;
>>  }
>>  
>>  env->ckc = time;
>>  
>> +tdc->get(td, &tod_base, &error_abort);
>> +tod_base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> 
> So the tdc->get first adds the time2tod, and then you subtract it here
> again? Can't you simply use td->base.low directly instead?

That might be a good idea, previously I had tdc->get_base(), but dropped
it because it cannot be implemented for KVM.

Simply accessing the member here should be fine. Thanks!

> 
>>  /* difference between origins */
>> -time -= env->tod_offset;
>> +time -= tod_base.low;
>>  
>>  /* nanoseconds */
>>  time = tod2time(time);
>> @@ -164,6 +174,14 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
>>  timer_mod(env->tod_timer, time);
>>  }
> 
> ... I found only nits, so with or without my suggested modifications:
> 
> Reviewed-by: Thomas Huth 
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v5 25/35] target/arm: Implement SVE floating-point round to integral value

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 14 +++
>  target/arm/sve_helper.c|  8 
>  target/arm/translate-sve.c | 77 ++
>  target/arm/sve.decode  |  9 +
>  4 files changed, 108 insertions(+)
>
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 26/35] target/arm: Implement SVE floating-point unary operations

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h| 14 ++
>  target/arm/sve_helper.c|  8 
>  target/arm/translate-sve.c | 26 ++
>  target/arm/sve.decode  |  4 
>  4 files changed, 52 insertions(+)
Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v5 0/4] Add support for TPM Physical Presence interface

Hi,

The following patches implement the TPM Physical Presence Interface
that allows a user to set a command via ACPI (sysfs entry in Linux)
that, upon the next reboot, the firmware looks for and acts upon by
sending sequences of commands to the TPM.

A dedicated memory region is added to the TPM CRB & TIS devices, at
address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
holds the location for that PPI region and some version details, to
allow for future flexibility.

With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
now runs successfully.

It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
Implement Physical Presence interface for TPM 1.2 and 2")

The edk2 support is merged upstream.

v5:
 - more code documentation (Marc-André)
 - use some explicit named variables to ease reading (Marc-André)
 - use fixed size fields/memory regions, remove PPI struct (Marc-André)
 - only add PPI ACPI methods if PPI is enabled (Marc-André)
 - document the qemu/firmware ACPI memory region (Stefan)
 - remove the dummy ACPI memory clear interface patch

v4:
 - add a "ppi" property, default to true, unless machine <= 2.12
 - pass PPI address to tpm_ppi_init_io()
 - renamed tpm_ppi struct name

Marc-André Lureau (1):
  tpm: add a "ppi" boolean property

Stefan Berger (3):
  tpm: implement virtual memory device for TPM PPI
  acpi: add fw_cfg file for TPM and PPI virtual memory device
  acpi: build TPM Physical Presence interface

 hw/tpm/tpm_ppi.h  |  27 +++
 include/hw/acpi/tpm.h |  17 ++
 include/hw/compat.h   |  10 +
 hw/i386/acpi-build.c  | 440 ++
 hw/tpm/tpm_crb.c  |  10 +
 hw/tpm/tpm_ppi.c  |  57 ++
 hw/tpm/tpm_tis.c  |  10 +
 docs/specs/tpm.txt|  99 ++
 hw/tpm/Makefile.objs  |   2 +-
 hw/tpm/trace-events   |   4 +
 10 files changed, 675 insertions(+), 1 deletion(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

-- 
2.18.0.rc1

[Qemu-devel] [PATCH v5 3/4] acpi: add fw_cfg file for TPM and PPI virtual memory device

From: Stefan Berger 

To avoid having to hard code the base address of the PPI virtual
memory device we introduce a fw_cfg file etc/tpm/config that holds the
base address of the PPI device, the version of the PPI interface and
the version of the attached TPM.

Signed-off-by: Stefan Berger 
[ Marc-André: renamed to etc/tpm/config, made it static, document it ]
Signed-off-by: Marc-André Lureau 

---

v4:
 - add ACPI only if PPI is enabled
v3:
 - renamed to etc/tpm/config, made it static, document it
---
 include/hw/acpi/tpm.h |  3 +++
 hw/i386/acpi-build.c  | 19 +++
 docs/specs/tpm.txt| 20 
 3 files changed, 42 insertions(+)

diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index c082df7d1d..f79d68a77a 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -193,4 +193,7 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_ADDR_SIZE   0x400
 #define TPM_PPI_ADDR_BASE   0xFED45000
 
+#define TPM_PPI_VERSION_NONE0
+#define TPM_PPI_VERSION_1_301
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9bc6d97ea1..d9320845ed 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -119,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
 bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
+typedef struct FWCfgTPMConfig {
+uint32_t tpmppi_address;
+uint8_t tpm_version;
+uint8_t tpmppi_version;
+} QEMU_PACKED FWCfgTPMConfig;
+
 static void init_common_fadt_data(Object *o, AcpiFadtData *data)
 {
 uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
@@ -2873,6 +2879,8 @@ void acpi_setup(void)
 AcpiBuildTables tables;
 AcpiBuildState *build_state;
 Object *vmgenid_dev;
+TPMIf *tpm;
+static FWCfgTPMConfig tpm_config;
 
 if (!pcms->fw_cfg) {
 ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
@@ -2907,6 +2915,17 @@ void acpi_setup(void)
 fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
 tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
+tpm = tpm_find();
+if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+tpm_config = (FWCfgTPMConfig) {
+.tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
+.tpm_version = cpu_to_le32(tpm_get_version(tpm_find())),
+.tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_NONE)
+};
+fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
+&tpm_config, sizeof tpm_config);
+}
+
 vmgenid_dev = find_vmgenid_dev();
 if (vmgenid_dev) {
 vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index c230c4c93e..2ddb768084 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -20,6 +20,26 @@ QEMU files related to TPM TIS interface:
  - hw/tpm/tpm_tis.h
 
 
+= fw_cfg interface =
+
+The bios/firmware may use the "etc/tpm/config" fw_cfg entry for
+configuring the guest appropriately.
+
+The entry of 6 bytes has the following content, in little-endian:
+
+#define TPM_VERSION_UNSPEC  0
+#define TPM_VERSION_1_2 1
+#define TPM_VERSION_2_0 2
+
+#define TPM_PPI_VERSION_NONE0
+#define TPM_PPI_VERSION_1_301
+
+struct FWCfgTPMConfig {
+uint32_t tpmppi_address; /* PPI memory location */
+uint8_t tpm_version; /* TPM version */
+uint8_t tpmppi_version;  /* PPI version */
+};
+
 = ACPI Interface =
 
 The TPM device is defined with ACPI ID "PNP0C31". QEMU builds a SSDT and passes
-- 
2.18.0.rc1

[Qemu-devel] [PATCH v5 2/4] tpm: implement virtual memory device for TPM PPI

From: Stefan Berger 

Implement a virtual memory device for the TPM Physical Presence interface.
The memory is located at 0xFED45000 and used by ACPI to send messages to the
firmware (BIOS) and by the firmware to provide parameters for each one of
the supported codes.

This device should be used by all TPM interfaces on x86 and can be added
by calling tpm_ppi_init_io().

Signed-off-by: Stefan Berger 
Signed-off-by: Marc-André Lureau 

---

v4 (Marc-André):
 - pass TPM_PPI_ADDR_BASE as argument to tpm_ppi_init_io()
 - only enable PPI if property is set

v3 (Marc-André):
 - merge CRB support
 - use trace events instead of DEBUG printf
 - headers inclusion simplification

v2:
 - moved to byte access since an infrequently used device;
   this simplifies code
 - increase size of device to 0x400
 - move device to 0xfffef000 since SeaBIOS has some code at 0x:
   'On the emulators, the bios at 0xf is also at 0x'
---
 hw/tpm/tpm_ppi.h  | 27 
 include/hw/acpi/tpm.h |  6 +
 hw/tpm/tpm_crb.c  |  7 ++
 hw/tpm/tpm_ppi.c  | 57 +++
 hw/tpm/tpm_tis.c  |  7 ++
 hw/tpm/Makefile.objs  |  2 +-
 hw/tpm/trace-events   |  4 +++
 7 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
new file mode 100644
index 00..ac7ad47238
--- /dev/null
+++ b/hw/tpm/tpm_ppi.h
@@ -0,0 +1,27 @@
+/*
+ * TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef TPM_TPM_PPI_H
+#define TPM_TPM_PPI_H
+
+#include "hw/acpi/tpm.h"
+#include "exec/address-spaces.h"
+
+typedef struct TPMPPI {
+MemoryRegion mmio;
+
+uint8_t ram[TPM_PPI_ADDR_SIZE];
+} TPMPPI;
+
+void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
+ hwaddr addr, Object *obj);
+
+#endif /* TPM_TPM_PPI_H */
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 46ac4dc581..c082df7d1d 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -187,4 +187,10 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM2_START_METHOD_MMIO  6
 #define TPM2_START_METHOD_CRB   7
 
+/*
+ * Physical Presence Interface
+ */
+#define TPM_PPI_ADDR_SIZE   0x400
+#define TPM_PPI_ADDR_BASE   0xFED45000
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index d5b0ac5920..37c095f00f 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -29,6 +29,7 @@
 #include "sysemu/reset.h"
 #include "tpm_int.h"
 #include "tpm_util.h"
+#include "tpm_ppi.h"
 #include "trace.h"
 
 typedef struct CRBState {
@@ -43,6 +44,7 @@ typedef struct CRBState {
 size_t be_buffer_size;
 
 bool ppi_enabled;
+TPMPPI ppi;
 } CRBState;
 
 #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
@@ -294,6 +296,11 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
 memory_region_add_subregion(get_system_memory(),
 TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
 
+if (s->ppi_enabled) {
+tpm_ppi_init_io(&s->ppi, get_system_memory(),
+TPM_PPI_ADDR_BASE, OBJECT(s));
+}
+
 qemu_register_reset(tpm_crb_reset, dev);
 }
 
diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
new file mode 100644
index 00..79bec186c7
--- /dev/null
+++ b/hw/tpm/tpm_ppi.c
@@ -0,0 +1,57 @@
+/*
+ * tpm_ppi.c - TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "tpm_ppi.h"
+#include "trace.h"
+
+static uint64_t tpm_ppi_mmio_read(void *opaque, hwaddr addr,
+  unsigned size)
+{
+TPMPPI *s = opaque;
+
+trace_tpm_ppi_mmio_read(addr, size, s->ram[addr]);
+
+return s->ram[addr];
+}
+
+static void tpm_ppi_mmio_write(void *opaque, hwaddr addr,
+   uint64_t val, unsigned size)
+{
+TPMPPI *s = opaque;
+
+trace_tpm_ppi_mmio_write(addr, size, val);
+
+s->ram[addr] = val;
+}
+
+static const MemoryRegionOps tpm_ppi_memory_ops = {
+.read = tpm_ppi_mmio_read,
+.write = tpm_ppi_mmio_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
+};
+
+void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
+ hwaddr addr, Object *obj)
+{
+memory_region_init_io(&tpmppi->mmio, obj, &tpm_ppi_memory_ops,
+  tpmppi, "tpm-ppi-mmio",
+  TPM_PPI_ADDR_SIZE);
+
+memory_region_add_subregion(m, addr, &tpmppi->mmio);
+

Re: [Qemu-devel] [PATCH v3 04/26] vhost-user: simplify vhost_user_init/vhost_user_cleanup

Hi

On Thu, Jun 21, 2018 at 3:27 PM, Tiwei Bie  wrote:
> On Thu, Jun 21, 2018 at 02:48:08PM +0200, Marc-André Lureau wrote:
>> On Thu, Jun 21, 2018 at 2:33 PM, Tiwei Bie  wrote:
>> > On Mon, Jun 18, 2018 at 06:17:07PM +0200, Marc-André Lureau wrote:
>> > [...]
>> >> diff --git a/hw/virtio/vhost-stub.c b/hw/virtio/vhost-stub.c
>> >> index 049089b5e2..323dfcc46a 100644
>> >> --- a/hw/virtio/vhost-stub.c
>> >> +++ b/hw/virtio/vhost-stub.c
>> >> @@ -7,7 +7,7 @@ bool vhost_has_free_slot(void)
>> >>  return true;
>> >>  }
>> >>
>> >> -VhostUserState *vhost_user_init(void)
>> >> +bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error 
>> >> **errp)
>> >>  {
>> >>  return NULL;
>> >
>> > It would be better to return false.
>> >
>>
>> Good catch, fixed.
>>
>> Except that, would you give a reviewed-by?
>
> Sorry, I missed something previously. In below diff:
>
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index a39f9c9974..5e5b8f3fc9 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -291,14 +291,14 @@ static int net_vhost_user_init(NetClientState *peer, 
> const char *device,
>  {
>  Error *err = NULL;
>  NetClientState *nc, *nc0 = NULL;
> -VhostUserState *user = NULL;
>  NetVhostUserState *s = NULL;
> +VhostUserState *user;
>  int i;
>
>  assert(name);
>  assert(queues > 0);
>
> -user = vhost_user_init();
> +user = g_new0(struct VhostUserState, 1);
>  if (!user) {
>  error_report("failed to init vhost_user");
>  goto err;
>
> It might be better to change the error message
> to something like:
>
> "failed to allocate vhost-user state".
>
> Except that, it looks good to me and you can add
> my reviewed-by.

Good point, g_new() aborts on failure, so we can simply remove the error report.

Thanks

>
> Best regards,
> Tiwei Bie
>
>>
>> > Best regards,
>> > Tiwei Bie
>> >
>> >>  }
>> > [...]
>> >
>>
>>
>>
>> --
>> Marc-André Lureau



-- 
Marc-André Lureau

[Qemu-devel] [PATCH v5 1/4] tpm: add a "ppi" boolean property

The following patches implement the TPM Physical Presence Interface,
make use of a new memory region and a fw_cfg entry. Enable PPI by
default with >2.12 machine type, to avoid migration issues.

Signed-off-by: Marc-André Lureau 
---
 include/hw/compat.h | 10 ++
 hw/tpm/tpm_crb.c|  3 +++
 hw/tpm/tpm_tis.c|  3 +++
 3 files changed, 16 insertions(+)

diff --git a/include/hw/compat.h b/include/hw/compat.h
index 44d5964060..01758991a0 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,6 +2,16 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_2_12 \
+{\
+.driver   = "tpm-crb",\
+.property = "ppi",\
+.value= "false",\
+},\
+{\
+.driver   = "tpm-tis",\
+.property = "ppi",\
+.value= "false",\
+},\
 {\
 .driver   = "migration",\
 .property = "decompress-error-check",\
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index a92dd50437..d5b0ac5920 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -41,6 +41,8 @@ typedef struct CRBState {
 MemoryRegion cmdmem;
 
 size_t be_buffer_size;
+
+bool ppi_enabled;
 } CRBState;
 
 #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
@@ -221,6 +223,7 @@ static const VMStateDescription vmstate_tpm_crb = {
 
 static Property tpm_crb_properties[] = {
 DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe),
+DEFINE_PROP_BOOL("ppi", CRBState, ppi_enabled, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 12f5c9a759..d9ddf9b723 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -81,6 +81,8 @@ typedef struct TPMState {
 TPMVersion be_tpm_version;
 
 size_t be_buffer_size;
+
+bool ppi_enabled;
 } TPMState;
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
@@ -950,6 +952,7 @@ static const VMStateDescription vmstate_tpm_tis = {
 static Property tpm_tis_properties[] = {
 DEFINE_PROP_UINT32("irq", TPMState, irq_num, TPM_TIS_IRQ),
 DEFINE_PROP_TPMBE("tpmdev", TPMState, be_driver),
+DEFINE_PROP_BOOL("ppi", TPMState, ppi_enabled, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.18.0.rc1

[Qemu-devel] [PATCH v5 4/4] acpi: build TPM Physical Presence interface

From: Stefan Berger 

The TPM Physical Presence interface consists of an ACPI part, a shared
memory part, and code in the firmware. Users can send messages to the
firmware by writing a code into the shared memory through invoking the
ACPI code. When a reboot happens, the firmware looks for the code and
acts on it by sending sequences of commands to the TPM.

This patch adds the ACPI code. It is similar to the one in EDK2 but doesn't
assume that SMIs are necessary to use. It uses a similar datastructure for
the shared memory as EDK2 does so that EDK2 and SeaBIOS could both make use
of it. I extended the shared memory data structure with an array of 256
bytes, one for each code that could be implemented. The array contains
flags describing the individual codes. This decouples the ACPI implementation
from the firmware implementation.

The underlying TCG specification is accessible from the following page.

https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/

This patch implements version 1.30.

Signed-off-by: Stefan Berger 

---

v6:
 - more code documentation (Marc-André)
 - use some explicit named variables to ease reading (Marc-André)
 - use fixed size fields/memory regions, remove PPI struct (Marc-André)
 - only add PPI ACPI methods if PPI is enabled (Marc-André)
 - document the qemu/firmware ACPI memory region (Stefan)

v5 (Marc-André):
 - /struct tpm_ppi/struct TPMPPIData

v4 (Marc-André):
 - replace 'DerefOf (FUNC [N])' with a function, to fix Windows ACPI
handling.
 - replace 'return Package (..) {} ' with scoped variables, to fix
   Windows ACPI handling.

v3:
 - add support for PPI to CRB
 - split up OperationRegion TPPI into two parts, one containing
   the registers (TPP1) and the other one the flags (TPP2); switched
   the order of the flags versus registers in the code
 - adapted ACPI code to small changes to the array of flags where
   previous flag 0 was removed and now shifting right wasn't always
   necessary anymore

v2:
 - get rid of FAIL variable; function 5 was using it and always
   returns 0; the value is related to the ACPI function call not
   a possible failure of the TPM function call.
 - extend shared memory data structure with per-opcode entries
   holding flags and use those flags to determine what to return
   to caller
 - implement interface version 1.3
---
 include/hw/acpi/tpm.h |   8 +
 hw/i386/acpi-build.c  | 423 +-
 docs/specs/tpm.txt|  79 
 3 files changed, 509 insertions(+), 1 deletion(-)

diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index f79d68a77a..e0bd07862e 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -196,4 +196,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_VERSION_NONE0
 #define TPM_PPI_VERSION_1_301
 
+/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
+#define TPM_PPI_FUNC_NOT_IMPLEMENTED (0 << 0)
+#define TPM_PPI_FUNC_BIOS_ONLY   (1 << 0)
+#define TPM_PPI_FUNC_BLOCKED (2 << 0)
+#define TPM_PPI_FUNC_ALLOWED_USR_REQ (3 << 0)
+#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
+#define TPM_PPI_FUNC_MASK(7 << 0)
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d9320845ed..d815af4eef 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -43,6 +43,7 @@
 #include "hw/acpi/memory_hotplug.h"
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
+#include "hw/tpm/tpm_ppi.h"
 #include "hw/acpi/vmgenid.h"
 #include "sysemu/tpm_backend.h"
 #include "hw/timer/mc146818rtc_regs.h"
@@ -1789,6 +1790,421 @@ static Aml *build_q35_osc_method(void)
 return method;
 }
 
+static void
+build_tpm_ppi(Aml *dev)
+{
+Aml *method, *name, *field, *ifctx, *ifctx2, *ifctx3, *pak;
+int i;
+
+if (!object_property_get_bool(OBJECT(tpm_find()), "ppi", &error_abort)) {
+return;
+}
+/*
+ * TPP1 is for the flags that indicate which PPI operations
+ * are supported by the firmware. The firmware is expected to
+ * write these flags.
+ */
+aml_append(dev,
+   aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
+aml_int(TPM_PPI_ADDR_BASE), 0x100));
+field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
+for (i = 0; i < 0x100; i++) {
+char *tmp = g_strdup_printf("FN%02X", i);
+aml_append(field, aml_named_field(tmp, 8));
+g_free(tmp);
+}
+aml_append(dev, field);
+
+/*
+ * TPP2 is for the registers that ACPI code used to pass
+ * the PPI code and parameter (PPRQ, PPRM) to the firmware.
+ */
+aml_append(dev,
+   aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
+aml_int(TPM_PPI_ADDR_BASE + 0x100),
+0x5A));
+field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
+aml_append(field, aml_named_field("PPI

Re: [Qemu-devel] [qemu-s390x] [PATCH v3 5/9] s390x/tcg: properly implement the TOD

On Tue, 26 Jun 2018 14:06:00 +0200
David Hildenbrand  wrote:

> >> @@ -34,10 +58,24 @@ static void qemu_s390_tod_class_init(ObjectClass *oc, 
> >> void *data)
> >>  tdc->set = qemu_s390_tod_set;
> >>  }
> >>  
> >> +static void qemu_s390_tod_init(Object *obj)
> >> +{
> >> +S390TODState *td = S390_TOD(obj);
> >> +struct tm tm;
> >> +
> >> +qemu_get_timedate(&tm, 0);
> >> +td->base.high = 0;
> >> +td->base.low = TOD_UNIX_EPOCH + (time2tod(mktimegm(&tm)) * 
> >> 10ULL);
> >> +if (td->base.low < TOD_UNIX_EPOCH) {
> >> +td->base.high += 1;
> >> +}
> >> +}  
> > 
> > Nit: It would be sufficient to do this in the realize() function instead.  
> 
> Then I'll have to overwrite the realize function and store the
> parent_realize function - something that I want to avoid if not really
> necessary.

Agreed. I'd just leave it as it is now.

> 
> (for now it was also done in the cpu initfn, so that should be fine)
> 
> >   
> >>  static TypeInfo qemu_s390_tod_info = {
> >>  .name = TYPE_QEMU_S390_TOD,
> >>  .parent = TYPE_S390_TOD,
> >>  .instance_size = sizeof(S390TODState),
> >> +.instance_init = qemu_s390_tod_init,
> >>  .class_init = qemu_s390_tod_class_init,
> >>  .class_size = sizeof(S390TODClass),
> >>  };  
> > [...]  
> >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> >> index dd5273949b..be341b5295 100644
> >> --- a/target/s390x/misc_helper.c
> >> +++ b/target/s390x/misc_helper.c
> >> @@ -28,6 +28,8 @@
> >>  #include "qemu/timer.h"
> >>  #include "exec/exec-all.h"
> >>  #include "exec/cpu_ldst.h"
> >> +#include "qapi/error.h"
> >> +#include "tcg_s390x.h"
> >>  
> >>  #if !defined(CONFIG_USER_ONLY)
> >>  #include "sysemu/cpus.h"
> >> @@ -39,6 +41,7 @@
> >>  #include "hw/s390x/ioinst.h"
> >>  #include "hw/s390x/s390-pci-inst.h"
> >>  #include "hw/boards.h"
> >> +#include "hw/s390x/tod.h"
> >>  #endif
> >>  
> >>  /* #define DEBUG_HELPER */
> >> @@ -138,25 +141,32 @@ void HELPER(spx)(CPUS390XState *env, uint64_t a1)
> >>  /* Store Clock */
> >>  uint64_t HELPER(stck)(CPUS390XState *env)
> >>  {
> >> -uint64_t time;
> >> +S390TODState *td = s390_get_todstate();
> >> +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
> >> +S390TOD tod;
> >>  
> >> -time = env->tod_offset +
> >> -time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> >> -
> >> -return time;
> >> +tdc->get(td, &tod, &error_abort);
> >> +return tod.low;
> >>  }
> >>  
> >>  /* Set Clock Comparator */
> >>  void HELPER(sckc)(CPUS390XState *env, uint64_t time)
> >>  {
> >> +S390TODState *td = s390_get_todstate();
> >> +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
> >> +S390TOD tod_base;
> >> +
> >>  if (time == -1ULL) {
> >>  return;
> >>  }
> >>  
> >>  env->ckc = time;
> >>  
> >> +tdc->get(td, &tod_base, &error_abort);
> >> +tod_base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));  
> > 
> > So the tdc->get first adds the time2tod, and then you subtract it here
> > again? Can't you simply use td->base.low directly instead?  
> 
> That might be a good idea, previously I had tdc->get_base(), but dropped
> it because it cannot be implemented for KVM.
> 
> Simply accessing the member here should be fine. Thanks!

So, we're guaranteed to have td->base.low at the correct value? (Sorry,
having a bit of a hard time following around here.)

> 
> >   
> >>  /* difference between origins */
> >> -time -= env->tod_offset;
> >> +time -= tod_base.low;
> >>  
> >>  /* nanoseconds */
> >>  time = tod2time(time);
> >> @@ -164,6 +174,14 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
> >>  timer_mod(env->tod_timer, time);
> >>  }  
> > 
> > ... I found only nits, so with or without my suggested modifications:
> > 
> > Reviewed-by: Thomas Huth 
> >   
> 
>

Re: [Qemu-devel] [qemu-s390x] [PATCH v3 5/9] s390x/tcg: properly implement the TOD

On 26.06.2018 14:27, Cornelia Huck wrote:
> On Tue, 26 Jun 2018 14:06:00 +0200
> David Hildenbrand  wrote:
> 
 @@ -34,10 +58,24 @@ static void qemu_s390_tod_class_init(ObjectClass *oc, 
 void *data)
  tdc->set = qemu_s390_tod_set;
  }
  
 +static void qemu_s390_tod_init(Object *obj)
 +{
 +S390TODState *td = S390_TOD(obj);
 +struct tm tm;
 +
 +qemu_get_timedate(&tm, 0);
 +td->base.high = 0;
 +td->base.low = TOD_UNIX_EPOCH + (time2tod(mktimegm(&tm)) * 
 10ULL);
 +if (td->base.low < TOD_UNIX_EPOCH) {
 +td->base.high += 1;
 +}
 +}  
>>>
>>> Nit: It would be sufficient to do this in the realize() function instead.  
>>
>> Then I'll have to overwrite the realize function and store the
>> parent_realize function - something that I want to avoid if not really
>> necessary.
> 
> Agreed. I'd just leave it as it is now.
> 
>>
>> (for now it was also done in the cpu initfn, so that should be fine)
>>
>>>   
  static TypeInfo qemu_s390_tod_info = {
  .name = TYPE_QEMU_S390_TOD,
  .parent = TYPE_S390_TOD,
  .instance_size = sizeof(S390TODState),
 +.instance_init = qemu_s390_tod_init,
  .class_init = qemu_s390_tod_class_init,
  .class_size = sizeof(S390TODClass),
  };  
>>> [...]  
 diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
 index dd5273949b..be341b5295 100644
 --- a/target/s390x/misc_helper.c
 +++ b/target/s390x/misc_helper.c
 @@ -28,6 +28,8 @@
  #include "qemu/timer.h"
  #include "exec/exec-all.h"
  #include "exec/cpu_ldst.h"
 +#include "qapi/error.h"
 +#include "tcg_s390x.h"
  
  #if !defined(CONFIG_USER_ONLY)
  #include "sysemu/cpus.h"
 @@ -39,6 +41,7 @@
  #include "hw/s390x/ioinst.h"
  #include "hw/s390x/s390-pci-inst.h"
  #include "hw/boards.h"
 +#include "hw/s390x/tod.h"
  #endif
  
  /* #define DEBUG_HELPER */
 @@ -138,25 +141,32 @@ void HELPER(spx)(CPUS390XState *env, uint64_t a1)
  /* Store Clock */
  uint64_t HELPER(stck)(CPUS390XState *env)
  {
 -uint64_t time;
 +S390TODState *td = s390_get_todstate();
 +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
 +S390TOD tod;
  
 -time = env->tod_offset +
 -time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
 -
 -return time;
 +tdc->get(td, &tod, &error_abort);
 +return tod.low;
  }
  
  /* Set Clock Comparator */
  void HELPER(sckc)(CPUS390XState *env, uint64_t time)
  {
 +S390TODState *td = s390_get_todstate();
 +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
 +S390TOD tod_base;
 +
  if (time == -1ULL) {
  return;
  }
  
  env->ckc = time;
  
 +tdc->get(td, &tod_base, &error_abort);
 +tod_base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));  
>>>
>>> So the tdc->get first adds the time2tod, and then you subtract it here
>>> again? Can't you simply use td->base.low directly instead?  
>>
>> That might be a good idea, previously I had tdc->get_base(), but dropped
>> it because it cannot be implemented for KVM.
>>
>> Simply accessing the member here should be fine. Thanks!
> 
> So, we're guaranteed to have td->base.low at the correct value? (Sorry,
> having a bit of a hard time following around here.)
> 

Yes, just verified, changed and tested. We will have the value at that
place.

I'll give some more time to review the other parts, then I can resend.


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v5 27/35] target/arm: Implement SVE MOVPRFX

On 21 June 2018 at 02:53, Richard Henderson
 wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-sve.c | 60 +-
>  target/arm/sve.decode  |  7 +
>  2 files changed, 66 insertions(+), 1 deletion(-)

> +/*
> + * Move Prefix
> + *
> + * TODO: The implementation so far could handle predicated merging movprfx.
> + * The helper functions as written take an extra source register to
> + * use in the operation, but the result is only written when predication
> + * succeeds.  For unpredicated movprfx, we need to rearrange the helpers
> + * to allow the final write back to the destination to be unconditional.
> + * For predicated zering movprfz, we need to rearrange the helpers to

"zeroing". Should that be "movprfx" or is "movprfz" a thing? (the SVE
spec doesn't mention it.)

> + * allow the final write back to zero inactives.
> + *
> + * In the meantime, just emit the moves.
> + */
> +
> +static bool trans_MOVPRFX(DisasContext *s, arg_MOVPRFX *a, uint32_t insn)
> +{
> +return do_mov_z(s, a->rd, a->rn);
> +}

A bit confusing that do_mov_z() does the sve_access_check() for us
but do_sel_z() and do_movz_zpz() do not...

> +
> +static bool trans_MOVPRFX_m(DisasContext *s, arg_rpr_esz *a, uint32_t insn)
> +{
> +if (sve_access_check(s)) {
> +do_sel_z(s, a->rd, a->rn, a->rd, a->pg, a->esz);
> +}
> +return true;
> +}
> +
> +static bool trans_MOVPRFX_z(DisasContext *s, arg_rpr_esz *a, uint32_t insn)
> +{
> +if (sve_access_check(s)) {
> +do_movz_zpz(s, a->rd, a->rn, a->pg, a->esz);
> +}
> +return true;
> +}

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 6/9] s390x/tcg: SET CLOCK COMPARATOR can clear CKC interrupts

On Mon, 25 Jun 2018 13:53:49 +0200
David Hildenbrand  wrote:

> Let's stop the timer and delete any pending CKC IRQ before doing
> anything else.
> 
> While at it, add a comment while the check for ckc == -1ULL is needed.

s/while/why/

> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/misc_helper.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index be341b5295..4872bdd774 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -156,6 +156,13 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
>  S390TODClass *tdc = S390_TOD_GET_CLASS(td);
>  S390TOD tod_base;
>  
> +/* stop the timer and remove pending CKC IRQs */
> +timer_del(env->tod_timer);
> +qemu_mutex_lock_iothread();
> +env->pending_int &= ~INTERRUPT_EXT_CLOCK_COMPARATOR;
> +qemu_mutex_unlock_iothread();
> +
> +/* the tod has to exceed the ckc, this can never happen if ckc is all 
> 1's */
>  if (time == -1ULL) {
>  return;
>  }

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

2018-06-26 Thread Eduardo Habkost

On Tue, Jun 26, 2018 at 07:57:18AM +0200, Paolo Bonzini wrote:
> On 25/06/2018 21:51, Eduardo Habkost wrote:
> > In either case, I'm not arguing (yet) for changing the default
> > upstream.  I'm just arguing for upstream QEMU to not make any
> > promises about the default.
> 
> It would be a guest ABI breakage for TCG guests, so it would only apply
> to new machine types.  I don't think it's worth the complication.

That's exactly the point: I want to stop promising a stable guest
ABI when the accelerator is omitted, because I see no benefit in
wasting energy on this.

(I don't think we ever kept the guest ABI correctly with TCG, by
the way.)


> 
> BTW, another thing that needs documenting is ABI promises for HAX and
> WHPX.  [...]

Absolutely.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 9/9] s390x/tcg: fix CPU hotplug with single-threaded TCG

On Mon, 25 Jun 2018 13:53:52 +0200
David Hildenbrand  wrote:

> run_on_cpu() doesn't seem to work reliably until the CPU has been fully
> created if the single-threaded TCG main loop is already running.
> 
> Therefore, hotplugging a CPU under single-threaded TCG does currently
> not work. We should use the direct call instead of going via
> run_on_cpu().
> 
> So let's use run_on_cpu() for KVM only - KVM requires it due to the initial
> CPU reset ioctl. As a nice side effect, we get rif of the ifdef.

s/rif/rid/

{Although "we get rif the thing" would be a nice contraction of "rid of" :}

> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 40d6980229..271c5ce652 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -218,11 +218,18 @@ static void s390_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>  #endif
>  s390_cpu_gdb_init(cs);
>  qemu_init_vcpu(cs);
> -#if !defined(CONFIG_USER_ONLY)
> -run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
> -#else
> -cpu_reset(cs);
> -#endif
> +
> +/*
> + * KVM requires the initial CPU reset ioctl to be executed on the target
> + * CPU thread. CPU hotplug under single-threaded TCG will not work with
> + * run_on_cpu(), as run_on_cpu() will not work properly if called while
> + * the main thread is already running but the CPU hasn't been realized.
> + */
> +if (kvm_enabled()) {
> +run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
> +} else {
> +cpu_reset(cs);
> +}
>  
>  scc->parent_realize(dev, &err);
>  out:

Re: [Qemu-devel] [PATCH v3 4/4] tpm: add a fake ACPI memory clear interface

2018-06-26 Thread Igor Mammedov

On Tue, 26 Jun 2018 11:22:26 +0200
Marc-André Lureau  wrote:

> On Thu, Jun 21, 2018 at 4:33 PM, Igor Mammedov  wrote:
> > On Thu, 21 Jun 2018 15:24:44 +0200
> > Marc-André Lureau  wrote:
> >  
> >> Hi
> >>
> >> On Thu, Jun 21, 2018 at 3:02 PM, Igor Mammedov  
> >> wrote:  
> >> > On Tue, 15 May 2018 14:14:33 +0200
> >> > Marc-André Lureau  wrote:
> >> >  
> >> >> This allows to pass the last failing test from the Windows HLK TPM 2.0
> >> >> TCG PPI 1.3 tests.
> >> >>
> >> >> The interface is described in the "TCG Platform Reset Attack
> >> >> Mitigation Specification", chapter 6 "ACPI _DSM Function". Whether or
> >> >> not we should have a real implementation remains an open question to 
> >> >> me.  
> >> > might it cause security issues?  
> >>
> >> Good question. If the guest assumes success of this operation perhaps.
> >> I'll check the spec.
> >>  
> >> > What are implications of faking it and how hard it's to implement thing
> >> > per spec?  
> >>
> >> Laszlo answerd that in "[Qemu-devel] investigating TPM for
> >> OVMF-on-QEMU"  2f2b) TCG Memory Clear Interface  
> > I get that it's optional, but we probably shouldn't advertise/fake
> > feature if it's not supported.  
> 
> As said in the commit message, the objective was to pass the Windows
> HLK test. If we don't want to advertize a fake interface, I am fine
> droping this patch. We'll have to revisit with Laszlo the work needed
> in the firmware to support it.
I think it would be safer to drop this patch.


> >  
> >>  
> >> >
> >> >  
> >> >> Signed-off-by: Marc-André Lureau 
> >> >> ---
> >> >>  hw/i386/acpi-build.c | 9 +
> >> >>  1 file changed, 9 insertions(+)
> >> >>
> >> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> >> index 95be4f0710..392a1e50bd 100644
> >> >> --- a/hw/i386/acpi-build.c
> >> >> +++ b/hw/i386/acpi-build.c
> >> >> @@ -2072,6 +2072,15 @@ build_tpm_ppi(Aml *dev)
> >> >>  aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> >> >>  }
> >> >>  aml_append(method, ifctx);
> >> >> +
> >> >> +   /* dummy MOR Memory Clear for the sake of WLK PPI test */
> >> >> +ifctx = aml_if(
> >> >> +aml_equal(aml_arg(0),
> >> >> +  
> >> >> aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> >> >> +{
> >> >> +aml_append(ifctx, aml_return(aml_int(0)));
> >> >> +}
> >> >> +aml_append(method, ifctx);
> >> >>  }
> >> >>  aml_append(dev, method);
> >> >>  }  
> >> >
> >> >  
> >>
> >>
> >>  
> >  
> 
> 
>

Re: [Qemu-devel] [qemu-s390x] [PATCH v3 5/9] s390x/tcg: properly implement the TOD

On Tue, 26 Jun 2018 14:28:18 +0200
David Hildenbrand  wrote:

> On 26.06.2018 14:27, Cornelia Huck wrote:
> > On Tue, 26 Jun 2018 14:06:00 +0200
> > David Hildenbrand  wrote:

>   /* Set Clock Comparator */
>   void HELPER(sckc)(CPUS390XState *env, uint64_t time)
>   {
>  +S390TODState *td = s390_get_todstate();
>  +S390TODClass *tdc = S390_TOD_GET_CLASS(td);
>  +S390TOD tod_base;
>  +
>   if (time == -1ULL) {
>   return;
>   }
>   
>   env->ckc = time;
>   
>  +tdc->get(td, &tod_base, &error_abort);
>  +tod_base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
> >>>
> >>> So the tdc->get first adds the time2tod, and then you subtract it here
> >>> again? Can't you simply use td->base.low directly instead?
> >>
> >> That might be a good idea, previously I had tdc->get_base(), but dropped
> >> it because it cannot be implemented for KVM.
> >>
> >> Simply accessing the member here should be fine. Thanks!  
> > 
> > So, we're guaranteed to have td->base.low at the correct value? (Sorry,
> > having a bit of a hard time following around here.)
> >   
> 
> Yes, just verified, changed and tested. We will have the value at that
> place.
> 
> I'll give some more time to review the other parts, then I can resend.

Ok, thx.

[Qemu-devel] [PATCH] s390/cpumodel: default enable bpb and ppa15 for z196 and later

2018-06-26 Thread Christian Borntraeger

Most systems and host kernels provide the necessary building blocks for
bpb and ppa15. We can reverse the logic and default enable those
features, while still allowing to disable it via cpu model.

So let us add bpb and ppa15 to z196 and later default CPU model for the
qemu 3.0 machine. (like -cpu z13).  Older machine types (e.g.
s390-ccw-virtio-2.12) will retain the old value and not provide those
bits in the default model.

Signed-off-by: Christian Borntraeger 
---
 hw/s390x/s390-virtio-ccw.c  | 2 ++
 target/s390x/gen-features.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 7ae5fb38dd..f8f58c8acb 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -824,6 +824,8 @@ DEFINE_CCW_MACHINE(3_0, "3.0", true);
 static void ccw_machine_2_12_instance_options(MachineState *machine)
 {
 ccw_machine_3_0_instance_options(machine);
+s390_cpudef_featoff_greater(11, 1, S390_FEAT_PPA15);
+s390_cpudef_featoff_greater(11, 1, S390_FEAT_BPB);
 }
 
 static void ccw_machine_2_12_class_options(MachineClass *mc)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 6c1c636140..5af042c003 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -513,6 +513,8 @@ static uint16_t default_GEN11_GA1[] = {
 S390_FEAT_IPTE_RANGE,
 S390_FEAT_ACCESS_EXCEPTION_FS_INDICATION,
 S390_FEAT_GROUP_MSA_EXT_4,
+S390_FEAT_PPA15,
+S390_FEAT_BPB,
 };
 
 #define default_GEN11_GA2 EmptyFeat
-- 
2.17.0

Re: [Qemu-devel] [PATCH v3 4/4] tpm: add a fake ACPI memory clear interface

2018-06-26 Thread Laszlo Ersek

On 06/26/18 14:34, Igor Mammedov wrote:
> On Tue, 26 Jun 2018 11:22:26 +0200
> Marc-André Lureau  wrote:
> 
>> On Thu, Jun 21, 2018 at 4:33 PM, Igor Mammedov  wrote:
>>> On Thu, 21 Jun 2018 15:24:44 +0200
>>> Marc-André Lureau  wrote:
>>>  
 Hi

 On Thu, Jun 21, 2018 at 3:02 PM, Igor Mammedov  
 wrote:  
> On Tue, 15 May 2018 14:14:33 +0200
> Marc-André Lureau  wrote:
>  
>> This allows to pass the last failing test from the Windows HLK TPM 2.0
>> TCG PPI 1.3 tests.
>>
>> The interface is described in the "TCG Platform Reset Attack
>> Mitigation Specification", chapter 6 "ACPI _DSM Function". Whether or
>> not we should have a real implementation remains an open question to me. 
>>  
> might it cause security issues?  

 Good question. If the guest assumes success of this operation perhaps.
 I'll check the spec.
  
> What are implications of faking it and how hard it's to implement thing
> per spec?  

 Laszlo answerd that in "[Qemu-devel] investigating TPM for
 OVMF-on-QEMU"  2f2b) TCG Memory Clear Interface  
>>> I get that it's optional, but we probably shouldn't advertise/fake
>>> feature if it's not supported.  
>>
>> As said in the commit message, the objective was to pass the Windows
>> HLK test. If we don't want to advertize a fake interface, I am fine
>> droping this patch. We'll have to revisit with Laszlo the work needed
>> in the firmware to support it.
> I think it would be safer to drop this patch.

This is BTW a feature that's very difficult for OVMF to implement, but
(I think) near trivial for QEMU to implement. The feature is about
clearing all of the guest RAM to zero at reboot.

For the firmware, it's difficult to solve, because in the 32-bit PEI
phase, we don't map DRAM beyond 4GB, so we can't re-set memory to zero
via normal addressing. (For physical platforms, this is different,
because their PEI phases have PEI modules that initialize the memory
controller(s), so they have platform-specific means to clear RAM.) For
QEMU, on the other hand, the feature "shouldn't be hard (TM)", just
implement a reset handler that clears all RAMBlocks on the host side (or
some such :) ).

Thanks,
Laszlo

> 
> 
>>>  
  
>
>  
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  hw/i386/acpi-build.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 95be4f0710..392a1e50bd 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -2072,6 +2072,15 @@ build_tpm_ppi(Aml *dev)
>>  aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
>>  }
>>  aml_append(method, ifctx);
>> +
>> +   /* dummy MOR Memory Clear for the sake of WLK PPI test */
>> +ifctx = aml_if(
>> +aml_equal(aml_arg(0),
>> +  
>> aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
>> +{
>> +aml_append(ifctx, aml_return(aml_int(0)));
>> +}
>> +aml_append(method, ifctx);
>>  }
>>  aml_append(dev, method);
>>  }  
>
>  


  
>>>  
>>
>>
>>
>

Re: [Qemu-devel] [PATCH] s390/cpumodel: default enable bpb and ppa15 for z196 and later

s/s390/s390x

I can't think of a scenario where this would be harmful. Migration with
compat machines will still work correctly. -cpu qemu is not affected, so
TCG will also continue to work just fine.

Reviewed-by: David Hildenbrand 

On 26.06.2018 14:38, Christian Borntraeger wrote:
> Most systems and host kernels provide the necessary building blocks for
> bpb and ppa15. We can reverse the logic and default enable those
> features, while still allowing to disable it via cpu model.
> 
> So let us add bpb and ppa15 to z196 and later default CPU model for the
> qemu 3.0 machine. (like -cpu z13).  Older machine types (e.g.
> s390-ccw-virtio-2.12) will retain the old value and not provide those
> bits in the default model.
> 
> Signed-off-by: Christian Borntraeger 
> ---
>  hw/s390x/s390-virtio-ccw.c  | 2 ++
>  target/s390x/gen-features.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 7ae5fb38dd..f8f58c8acb 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -824,6 +824,8 @@ DEFINE_CCW_MACHINE(3_0, "3.0", true);
>  static void ccw_machine_2_12_instance_options(MachineState *machine)
>  {
>  ccw_machine_3_0_instance_options(machine);
> +s390_cpudef_featoff_greater(11, 1, S390_FEAT_PPA15);
> +s390_cpudef_featoff_greater(11, 1, S390_FEAT_BPB);
>  }
>  
>  static void ccw_machine_2_12_class_options(MachineClass *mc)
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 6c1c636140..5af042c003 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -513,6 +513,8 @@ static uint16_t default_GEN11_GA1[] = {
>  S390_FEAT_IPTE_RANGE,
>  S390_FEAT_ACCESS_EXCEPTION_FS_INDICATION,
>  S390_FEAT_GROUP_MSA_EXT_4,
> +S390_FEAT_PPA15,
> +S390_FEAT_BPB,
>  };
>  
>  #define default_GEN11_GA2 EmptyFeat
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v5 02/35] target/arm: Implement SVE Contiguous Load, first-fault and no-fault

2018-06-26 Thread Alex Bennée



Richard Henderson  writes:

> Signed-off-by: Richard Henderson 
> ---
>  target/arm/helper-sve.h|  40 ++
>  target/arm/sve_helper.c| 156 +
>  target/arm/translate-sve.c |  69 
>  target/arm/sve.decode  |   6 ++
>  4 files changed, 271 insertions(+)
>
> diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
> index fcc9ba5f50..7338abbbcf 100644
> --- a/target/arm/helper-sve.h
> +++ b/target/arm/helper-sve.h
> @@ -754,3 +754,43 @@ DEF_HELPER_FLAGS_4(sve_ld1hds_r, TCG_CALL_NO_WG, void, 
> env, ptr, tl, i32)
>
>  DEF_HELPER_FLAGS_4(sve_ld1sdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
>  DEF_HELPER_FLAGS_4(sve_ld1sds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldff1bb_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1bhu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1bsu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1bdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1bhs_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1bss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1bds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldff1hh_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1hsu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1hdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1hss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1hds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldff1ss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1sdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldff1sds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldff1dd_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldnf1bb_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1bhu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1bsu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1bdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1bhs_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1bss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1bds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldnf1hh_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1hsu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1hdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1hss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1hds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldnf1ss_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1sdu_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +DEF_HELPER_FLAGS_4(sve_ldnf1sds_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> +
> +DEF_HELPER_FLAGS_4(sve_ldnf1dd_r, TCG_CALL_NO_WG, void, env, ptr, tl, i32)
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 4e6ad282f9..6e1b539ce3 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -2963,3 +2963,159 @@ DO_LD4(sve_ld4dd_r, cpu_ldq_data_ra, uint64_t, 
> uint64_t, )
>  #undef DO_LD2
>  #undef DO_LD3
>  #undef DO_LD4
> +
> +/*
> + * Load contiguous data, first-fault and no-fault.
> + */
> +
> +#ifdef CONFIG_USER_ONLY
> +
> +/* Fault on byte I.  All bits in FFR from I are cleared.  The vector
> + * result from I is CONSTRAINED UNPREDICTABLE; we choose the MERGE
> + * option, which leaves subsequent data unchanged.
> + */
> +static void __attribute__((cold))
> +record_fault(CPUARMState *env, intptr_t i, intptr_t oprsz)
> +{
> +uint64_t *ffr = env->vfp.pregs[FFR_PRED_NUM].p;
> +if (i & 63) {
> +ffr[i / 64] &= MAKE_64BIT_MASK(0, (i & 63) - 1);
> +i = ROUND_UP(i, 64);
> +}
> +for (; i < oprsz; i += 64) {
> +ffr[i / 64] = 0;
> +}
> +}
> +
> +/* Hold the mmap lock during the operation so that there is no race
> + * between page_check_range and the load operation.  We expect the
> + * usual case to have no faults at all, so we check the whole range
> + * first and if successful defer to the normal load operation.
> + *
> + * TODO: Change mmap_lock to a rwlock so that multiple readers
> + * can run simultaneously.  This will probably help other uses
> + * within QEMU as well.
> + */
> +#define DO_LDFF1(PART, FN, TYPEE, TYPEM, H) \
> +static void do_sve_ldff1##PART(CPUARMState *env, void *vd, void *vg,\
> +   target_ulong addr, intptr_t oprsz,   \
> +   bool f

Re: [Qemu-devel] [PULL 0/2] Machine queue, 2018-06-25

On 25 June 2018 at 23:41, Eduardo Habkost  wrote:
> The following changes since commit 35e238c9330669882487f9929e0aa97900431853:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/audio-20180625-pull-request' into staging (2018-06-25 
> 15:25:26 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
>
> for you to fetch changes up to 08fe68244eb44f3c8ccecd35066eca8392d1345a:
>
>   hw/i386: Deprecate the machine types pc-0.10 and pc-0.11 (2018-06-25 
> 14:10:01 -0300)
>
> 
> Machine queue, 2018-06-25
>
> * Don't support --daemonize and --preconfig together
> * Deprecate machine types pc-0.10 and pc-0.11
>
> 
Applied, thanks.

-- PMM

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

2018-06-26 Thread Eduardo Habkost

On Tue, Jun 26, 2018 at 09:50:04AM +0200, Cornelia Huck wrote:
> On Tue, 26 Jun 2018 06:40:56 +0200
> Thomas Huth  wrote:
> 
> > On 25.06.2018 20:26, Paolo Bonzini wrote:
> > > On 25/06/2018 19:30, Eduardo Habkost wrote:  
> >  Attentive distros could even replace the wrapper script by a link.  
> > >>> If they are okay with replacing the "KVM only" semantics with "KVM or
> > >>> TCG", which I think is generally worse.  
> > >>
> > >> If we can't get agreement on what's the right default for each
> > >> QEMU binary, I think that's yet another reason to document that
> > >> upstream QEMU won't guarantee ABI compatibility if -accel is
> > >> omitted.  
> > > 
> > > Before that we should ask what the benefit is in changing the default
> > > for qemu-system-*.  Nobody is using it in practice to start QEMU with
> > > KVM enabled...  
> > 
> > That's certainly not true. I've seen a couple of times already that
> > people ask on IRC why their guests are running so slow, and if you ask
> > them about their command line, it's obvious that they simply were not
> > aware of "-accel" / "-enable-kvm" yet.
> > 
> > 
> > Maybe we simply should add a "--verbose" command line option that people
> > can use to diagnose their problems:
> > 
> > $ qemu-system-x86_64 --verbose
> > QEMU emulator version 2.12.50
> > Using 'tcg' accelerator. Use '-accel kvm' to speed things up.
> > Machine type is 'pc-i440fx-3.0'. Use 'q35' for a more modern machine.
> > 
> > 
> 
> Not sure how serious you meant that, but I actually quite like the
> idea :)

Also, this mode could be enabled by default if stderr is a tty.

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH 4/4] qemu-options: Do not show -enable-kvm and -enable-hax in the docs anymore

On 26/06/2018 14:29, Eduardo Habkost wrote:
> On Tue, Jun 26, 2018 at 07:57:18AM +0200, Paolo Bonzini wrote:
>> On 25/06/2018 21:51, Eduardo Habkost wrote:
>>> In either case, I'm not arguing (yet) for changing the default
>>> upstream.  I'm just arguing for upstream QEMU to not make any
>>> promises about the default.
>>
>> It would be a guest ABI breakage for TCG guests, so it would only apply
>> to new machine types.  I don't think it's worth the complication.
> 
> That's exactly the point: I want to stop promising a stable guest
> ABI when the accelerator is omitted, because I see no benefit in
> wasting energy on this.

On the other hand I see no benefit in changing a default that people are
obviously not using (since most people use KVM, not TCG).  Distros will
keep shipping, and people will keep using qemu-kvm even if we change the
default.

> (I don't think we ever kept the guest ABI correctly with TCG, by
> the way.)

It would not be any different from KVM.  Less tested and likely to be
more buggy, yes, but not particularly harder.

Paolo

Re: [Qemu-devel] [RFC] arm: Add NRF51 SOC non-volatile memory controller