[PATCH v2 01/10] qga/channel-posix: Plug memory leak in ga_channel_write_all()

2020-08-31 Thread Pan Nengyuan
Missing g_error_free on error path in ga_channel_write_all(). Fix that.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Li Qiang 
---
Cc: Michael Roth 
---
- V2: no changes in v2
---
 qga/channel-posix.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 8fc205ad21..0b151cb87b 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -249,7 +249,7 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar 
*buf, gsize size)
 buf += written;
 } else if (status != G_IO_STATUS_AGAIN) {
 g_warning("error writing to channel: %s", err->message);
-return status;
+goto out;
 }
 }
 
@@ -261,6 +261,10 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar 
*buf, gsize size)
 g_warning("error flushing channel: %s", err->message);
 }
 
+out:
+if (err) {
+g_error_free(err);
+}
 return status;
 }
 
-- 
2.18.2




[PATCH v2 08/10] blockdev: Fix a memleak in drive_backup_prepare()

2020-08-31 Thread Pan Nengyuan
'local_err' seems forgot to propagate in error path, it'll cause
a memleak. Fix it.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Kevin Wolf 
Reviewed-by: Li Qiang 
---
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Markus Armbruster 
Cc: qemu-bl...@nongnu.org
---
- V2: no changes in v2.
---
 blockdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/blockdev.c b/blockdev.c
index 3848a9c8ab..842ac289c1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1801,6 +1801,7 @@ static void drive_backup_prepare(BlkActionState *common, 
Error **errp)
 if (set_backing_hd) {
 bdrv_set_backing_hd(target_bs, source, &local_err);
 if (local_err) {
+error_propagate(errp, local_err);
 goto unref;
 }
 }
-- 
2.18.2




[PATCH v2 04/10] target/i386/sev: Plug memleak in sev_read_file_base64

2020-08-31 Thread Pan Nengyuan
Missing g_error_free() in sev_read_file_base64() error path.
Fix that.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Li Qiang 
---
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
---
- v2: no changes in v2
---
 target/i386/sev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index c3ecf86704..de4818da6d 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -500,6 +500,7 @@ sev_read_file_base64(const char *filename, guchar **data, 
gsize *len)
 
 if (!g_file_get_contents(filename, &base64, &sz, &error)) {
 error_report("failed to read '%s' (%s)", filename, error->message);
+g_error_free(error);
 return -1;
 }
 
-- 
2.18.2




[PATCH v2 02/10] elf2dmp/qemu_elf: Plug memleak in QEMU_Elf_init

2020-08-31 Thread Pan Nengyuan
Missing g_error_free in QEMU_Elf_init() error path. Fix that.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Viktor Prutyanov 
Reviewed-by: Li Qiang 
---
Cc: Viktor Prutyanov 
---
- v2: no changes in v2
---
 contrib/elf2dmp/qemu_elf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
index 0db7816586..b601b6d7ba 100644
--- a/contrib/elf2dmp/qemu_elf.c
+++ b/contrib/elf2dmp/qemu_elf.c
@@ -126,6 +126,7 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
 qe->gmf = g_mapped_file_new(filename, TRUE, &gerr);
 if (gerr) {
 eprintf("Failed to map ELF dump file \'%s\'\n", filename);
+g_error_free(gerr);
 return 1;
 }
 
-- 
2.18.2




[PATCH v2 00/10] fix some error memleaks

2020-08-31 Thread Pan Nengyuan
This series fix some Error/GError memleaks.

V2: 
  1. remove two patches.(One has aleardy applied. The other has fixed.)
  2. change patch 5/10 and 7/10.

Pan Nengyuan (10):
  qga/channel-posix: Plug memory leak in ga_channel_write_all()
  elf2dmp/qemu_elf: Plug memleak in QEMU_Elf_init
  elf2dmp/pdb: Plug memleak in pdb_init_from_file
  target/i386/sev: Plug memleak in sev_read_file_base64
  ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()
  target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features
  migration/colo: Plug memleaks in colo_process_incoming_thread
  blockdev: Fix a memleak in drive_backup_prepare()
  block/file-posix: fix a possible undefined behavior
  vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

 block/file-posix.c |  2 +-
 blockdev.c |  1 +
 contrib/elf2dmp/pdb.c  |  1 +
 contrib/elf2dmp/qemu_elf.c |  1 +
 migration/colo.c   |  5 -
 qga/channel-posix.c|  6 +-
 target/i386/cpu.c  |  1 +
 target/i386/sev.c  |  1 +
 ui/gtk-gl-area.c   | 11 +++
 ui/vnc-auth-sasl.c |  1 +
 10 files changed, 27 insertions(+), 3 deletions(-)

-- 
2.18.2




[PATCH v2 03/10] elf2dmp/pdb: Plug memleak in pdb_init_from_file

2020-08-31 Thread Pan Nengyuan
Missing g_error_free in pdb_init_from_file() error path. Fix that.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Viktor Prutyanov 
Reviewed-by: Li Qiang 
---
Cc: Viktor Prutyanov 
---
- v2: no changes in v2
---
 contrib/elf2dmp/pdb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index a5bd40c99d..b3a6547068 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -285,6 +285,7 @@ int pdb_init_from_file(const char *name, struct pdb_reader 
*reader)
 reader->gmf = g_mapped_file_new(name, TRUE, &gerr);
 if (gerr) {
 eprintf("Failed to map PDB file \'%s\'\n", name);
+g_error_free(gerr);
 return 1;
 }
 
-- 
2.18.2




[PATCH v2 06/10] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features

2020-08-31 Thread Pan Nengyuan
'err' forgot to free in x86_cpu_class_check_missing_features error path.
Fix that.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Li Qiang 
---
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
---
- V2: no changes in v2.
---
 target/i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 588f32e136..4678aac0b4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4872,6 +4872,7 @@ static void 
x86_cpu_class_check_missing_features(X86CPUClass *xcc,
 new->value = g_strdup("type");
 *next = new;
 next = &new->next;
+error_free(err);
 }
 
 x86_cpu_filter_features(xc, false);
-- 
2.18.2




[PATCH v2 09/10] block/file-posix: fix a possible undefined behavior

2020-08-31 Thread Pan Nengyuan
local_err is not initialized to NULL, it will cause a assert error as below:
qemu/util/error.c:59: error_setv: Assertion `*errp == NULL' failed.

Fixes: c6447510690
Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Stefano Garzarella 
---
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Aarushi Mehta 
Cc: qemu-bl...@nongnu.org
---
- V2: no changes in v2.
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 9a00d4190a..697a7d9eea 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2113,7 +2113,7 @@ static void raw_aio_attach_aio_context(BlockDriverState 
*bs,
 #endif
 #ifdef CONFIG_LINUX_IO_URING
 if (s->use_linux_io_uring) {
-Error *local_err;
+Error *local_err = NULL;
 if (!aio_setup_linux_io_uring(new_context, &local_err)) {
 error_reportf_err(local_err, "Unable to use linux io_uring, "
  "falling back to thread pool: ");
-- 
2.18.2




[PATCH v2 05/10] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

2020-08-31 Thread Pan Nengyuan
Receiving error in local variable err, and forgot to free it.
This patch check the return value of 'gdk_window_create_gl_context'
and 'gdk_gl_context_realize', then free err to fix it.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Cc: Gerd Hoffmann 
---
V2->V1: check the return value of  'gdk_window_create_gl_context'
and 'gdk_gl_context_realize' instead of omitting it(Suggested by Li Qiang)
---
 ui/gtk-gl-area.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 85f9d14c51..98c22d23f5 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -147,10 +147,21 @@ QEMUGLContext 
gd_gl_area_create_context(DisplayChangeListener *dcl,
 gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
 window = gtk_widget_get_window(vc->gfx.drawing_area);
 ctx = gdk_window_create_gl_context(window, &err);
+if (err) {
+g_printerr("Create gdk gl context failed: %s\n", err->message);
+g_error_free(err);
+return NULL;
+}
 gdk_gl_context_set_required_version(ctx,
 params->major_ver,
 params->minor_ver);
 gdk_gl_context_realize(ctx, &err);
+if (err) {
+g_printerr("Realize gdk gl context failed: %s\n", err->message);
+g_error_free(err);
+g_clear_object(&ctx);
+return NULL;
+}
 return ctx;
 }
 
-- 
2.18.2




[PATCH v2 10/10] vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

2020-08-31 Thread Pan Nengyuan
'addr' is forgot to free in vnc_socket_ip_addr_string error path. Fix that.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Li Qiang 
---
Cc: Gerd Hoffmann 
---
- V2: no changes in v2.
---
 ui/vnc-auth-sasl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 7b2b09f242..0517b2ead9 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -522,6 +522,7 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
 
 if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
 error_setg(errp, "Not an inet socket type");
+qapi_free_SocketAddress(addr);
 return NULL;
 }
 ret = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
-- 
2.18.2




Re: [PATCH v2 (BROKEN) 0/6] migration: bring improved savevm/loadvm/delvm to QMP

2020-08-31 Thread Markus Armbruster
Kevin Wolf  writes:

> Am 28.08.2020 um 08:20 hat Markus Armbruster geschrieben:
>> Kevin Wolf  writes:
>> 
>> > Am 27.08.2020 um 13:06 hat Markus Armbruster geschrieben:
>> >> Daniel P. Berrangé  writes:
>> >> 
>> >> > On Wed, Aug 26, 2020 at 07:28:24PM +0100, Daniel P. Berrangé wrote:
>> >> >> On Wed, Aug 26, 2020 at 05:52:06PM +0200, Markus Armbruster wrote:
>> >> >> > Open questions:
>> >> >> > 
>> >> >> > * Do we want the QMP command to delete existing snapshots with
>> >> >> >   conflicting tag / ID, like HMP savevm does?  Or do we want it to 
>> >> >> > fail
>> >> >> >   the transaction?
>> >> >> 
>> >> >> The intent is for the QMP commands to operate exclusively on
>> >> >> 'tags', and never consider "ID".
>> >> >
>> >> > I forgot that even HMP ignores "ID" now and works exclusively in terms
>> >> > of tags since:
>> >> >
>> >> >
>> >> >   commit 6ca080453ea403959ccde661030ca16264acc181
>> >> >   Author: Daniel Henrique Barboza 
>> >> >   Date:   Wed Nov 7 11:09:58 2018 -0200
>> >> >
>> >> > block/snapshot.c: eliminate use of ID input in snapshot operations
>> >> 
>> >> Almost a year after I sent the memo I quoted.  It's an incompatible
>> >> change, but nobody complained, and I'm glad we got this issue out of the
>> >> way.
>> >
>> > FWIW, I would have ignored any complaint about incompatible changes in
>> > HMP. It's not supposed to be a stable API, but UI.
>> 
>> The iffy part is actually the loss of ability to access snapshots that
>> lack a name.  Complaints about that would have been valid, I think.
>> Fortunately, there have been none.
>
> 'loadvm ""' should do the trick for these.

As long as you have at most one.

>The same way as you have to
> use with 'savevm' to create them in non-prehistoric versions of QEMU.
> We stopped creating snapshots with empty names by default in 0.14, so
> they are probably not very relevant any more. (Versioned machine types
> go back "only" to 1.0, so good luck loading a snapshot from an older
> version. And I wouldn't bet money either on a 1.0 snapshot still working
> with that machine type...)

No argument.




Re: [PATCH v6 19/20] tests/acpi: add microvm test

2020-08-31 Thread Igor Mammedov
On Wed, 26 Aug 2020 12:52:53 +0200
Gerd Hoffmann  wrote:

> Signed-off-by: Gerd Hoffmann 
> ---
>  tests/qtest/bios-tables-test.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 058ba3886659..883532e531d6 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1008,6 +1008,20 @@ static void test_acpi_virt_tcg_memhp(void)
>  
>  }
>  
> +static void test_acpi_microvm_tcg(void)
> +{
> +test_data data;
> +
> +memset(&data, 0, sizeof(data));
> +data.machine = "microvm";
> +data.required_struct_types = NULL; /* no smbios */
> +data.required_struct_types_len = 0;
> +data.blkdev = "virtio-blk-device";
> +test_acpi_one(" -machine microvm,acpi=on,rtc=off",
I'm just curious, why rtc is off?


> +  &data);
> +free_test_data(&data);
> +}
> +
>  static void test_acpi_virt_tcg_numamem(void)
>  {
>  test_data data = {
> @@ -1119,6 +1133,7 @@ int main(int argc, char *argv[])
>  qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
>  qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
>  qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
> +qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
>  } else if (strcmp(arch, "aarch64") == 0) {
>  qtest_add_func("acpi/virt", test_acpi_virt_tcg);
>  qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);




Re: [PATCH v7 1/8] Introduce yank feature

2020-08-31 Thread Markus Armbruster
Lukas Straub  writes:

> On Thu, 27 Aug 2020 14:37:00 +0200
> Markus Armbruster  wrote:
>
>> I apologize for not reviewing this much earlier.
>> 
>> Lukas Straub  writes:
>> 
>> > The yank feature allows to recover from hanging qemu by "yanking"
>> > at various parts. Other qemu systems can register themselves and
>> > multiple yank functions. Then all yank functions for selected
>> > instances can be called by the 'yank' out-of-band qmp command.
>> > Available instances can be queried by a 'query-yank' oob command.
>> >
>> > Signed-off-by: Lukas Straub 
>> > Acked-by: Stefan Hajnoczi 
[...]
>> > diff --git a/qapi/misc.json b/qapi/misc.json
>> > index 9d32820dc1..0d6a8f20b7 100644
>> > --- a/qapi/misc.json
>> > +++ b/qapi/misc.json
>> > @@ -1615,3 +1615,48 @@
>> >  ##
>> >  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
>> >
>> > +##
>> > +# @YankInstances:
>> > +#
>> > +# @instances: List of yank instances.
>> > +#
>> > +# Yank instances are named after the following schema:
>> > +# "blockdev:", "chardev:" and "migration"
>> > +#
>> > +# Since: 5.1
>> > +##
>> > +{ 'struct': 'YankInstances', 'data': {'instances': ['str'] } }  
>> 
>> I'm afraid this is a problematic QMP interface.
>> 
>> By making YankInstances a struct, you keep the door open to adding more
>> members, which is good.
>> 
>> But by making its 'instances' member a ['str'], you close the door to
>> using anything but a single string for the individual instances.  Not so
>> good.
>> 
>> The single string encodes information which QMP client will need to
>> parse from the string.  We frown on that in QMP.  Use QAPI complex types
>> capabilities for structured data.
>> 
>> Could you use something like this instead?
>> 
>> { 'enum': 'YankInstanceType',
>>   'data': { 'block-node', 'chardev', 'migration' } }
>> 
>> { 'struct': 'YankInstanceBlockNode',
>>   'data': { 'node-name': 'str' } }
>> 
>> { 'struct': 'YankInstanceChardev',
>>   'data' { 'label': 'str' } }
>> 
>> { 'union': 'YankInstance',
>>   'base': { 'type': 'YankInstanceType' },
>>   'discriminator': 'type',
>>   'data': {
>>   'block-node': 'YankInstanceBlockNode',
>>   'chardev': 'YankInstanceChardev' } }
>> 
>> { 'command': 'yank',
>>   'data': { 'instances': ['YankInstance'] },
>>   'allow-oob': true }
>> 
>> If you're confident nothing will ever be added to YankInstanceBlockNode
>> and YankInstanceChardev, you could use str instead.
>
> As Daniel said, this has already been discussed.

I'll look up that discussion.

[...]
>> The two QMP commands permit out-of-band execution ('allow-oob': true).
>> OOB is easy to get wrong, but I figure you have a legitimate use case.
>> Let's review the restrictions documented in
>> docs/devel/qapi-code-gen.txt:
>> 
>> An OOB-capable command handler must satisfy the following conditions:
>> 
>> - It terminates quickly.
>> - It does not invoke system calls that may block.
>> - It does not access guest RAM that may block when userfaultfd is
>>   enabled for postcopy live migration.
>> - It takes only "fast" locks, i.e. all critical sections protected by
>>   any lock it takes also satisfy the conditions for OOB command
>>   handler code.
>> 
>> Since the command handlers take &lock, the restrictions apply to the
>> other critical sections protected by &lock as well.  I believe these are
>> all okay: they do nothing but allocate, initialize and free memory.
>> 
>> The restrictions also apply to the YankFn callbacks, but you documented
>> that.  Okay.
>> 
>> The one such callback included in this patch is
>> yank_generic_iochannel(), which is a thin wrapper around
>> qio_channel_shutdown(), which in turn runs the io_shutdown method.
>> Thus, the restructions also apply to all the io_shutdown methods.
>> That's not documented.
>> 
>> Daniel, should it be documented?
>> 
> This is already done in patch 6.

Patch 6 adds "This function is thread-safe" to its contract.  The
restrictions on OOB-capable handler code are much more severe than
ordinary thread safety.  For instance, blocking system calls outside
critical sections are thread safe, but not permitted in OOB-capable
handler code.  The contract needs to be more specific.

> Thank you for you review.

Better late than never...  you're welcome!




Re: [PATCH v6 15/20] x86: move cpu hotplug from pc to x86

2020-08-31 Thread Igor Mammedov
On Wed, 26 Aug 2020 12:52:49 +0200
Gerd Hoffmann  wrote:

> The cpu hotplug code handles the iniu hotplug is not supported.
> 
> Move the code from pc to x86, so microvm can use it.
> 
> Move both plug and unplug to keep everything in one place, even
> though microvm needs plug only.

this will conflict with planned revert of EPYC changes
 2e26f4ab3bf8 hw/i386: Move arch_id decode inside x86_cpus_init
so it will mess up otherwise clean revert,
I'd prefer revert being merged first and than this reased on
top of it.


> Signed-off-by: Gerd Hoffmann 
> ---
>  include/hw/i386/x86.h |  10 ++
>  hw/i386/pc.c  | 283 +-
>  hw/i386/x86.c | 268 +++
>  3 files changed, 283 insertions(+), 278 deletions(-)
> 
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index de74c831c3ab..5c3803e74e31 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -102,6 +102,16 @@ CpuInstanceProperties 
> x86_cpu_index_to_props(MachineState *ms,
>   unsigned cpu_index);
>  int64_t x86_get_default_cpu_node_id(const MachineState *ms, int idx);
>  const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms);
> +CPUArchId *x86_find_cpu_slot(MachineState *ms, uint32_t id, int *idx);
> +void x86_rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count);
> +void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp);
> +void x86_cpu_plug(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp);
> +void x86_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
> +   DeviceState *dev, Error **errp);
> +void x86_cpu_unplug_cb(HotplugHandler *hotplug_dev,
> +   DeviceState *dev, Error **errp);
>  
>  void x86_bios_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw);
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0bd6dbbd7bf6..b55369357e5d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -803,19 +803,6 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, 
> Error **errp)
>  }
>  }
>  
> -static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
> -{
> -if (cpus_count > 0xff) {
> -/* If the number of CPUs can't be represented in 8 bits, the
> - * BIOS must use "FW_CFG_NB_CPUS". Set RTC field to 0 just
> - * to make old BIOSes fail more predictably.
> - */
> -rtc_set_memory(rtc, 0x5f, 0);
> -} else {
> -rtc_set_memory(rtc, 0x5f, cpus_count - 1);
> -}
> -}
> -
>  static
>  void pc_machine_done(Notifier *notifier, void *data)
>  {
> @@ -825,7 +812,7 @@ void pc_machine_done(Notifier *notifier, void *data)
>  PCIBus *bus = pcms->bus;
>  
>  /* set the number of CPUs */
> -rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
> +x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
>  
>  if (bus) {
>  int extra_hosts = 0;
> @@ -1373,266 +1360,6 @@ static void pc_memory_unplug(HotplugHandler 
> *hotplug_dev,
>  error_propagate(errp, local_err);
>  }
>  
> -static int pc_apic_cmp(const void *a, const void *b)
> -{
> -   CPUArchId *apic_a = (CPUArchId *)a;
> -   CPUArchId *apic_b = (CPUArchId *)b;
> -
> -   return apic_a->arch_id - apic_b->arch_id;
> -}
> -
> -/* returns pointer to CPUArchId descriptor that matches CPU's apic_id
> - * in ms->possible_cpus->cpus, if ms->possible_cpus->cpus has no
> - * entry corresponding to CPU's apic_id returns NULL.
> - */
> -static CPUArchId *pc_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
> -{
> -CPUArchId apic_id, *found_cpu;
> -
> -apic_id.arch_id = id;
> -found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
> -ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
> -pc_apic_cmp);
> -if (found_cpu && idx) {
> -*idx = found_cpu - ms->possible_cpus->cpus;
> -}
> -return found_cpu;
> -}
> -
> -static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> -DeviceState *dev, Error **errp)
> -{
> -CPUArchId *found_cpu;
> -Error *local_err = NULL;
> -X86CPU *cpu = X86_CPU(dev);
> -PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> -X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
> -
> -if (x86ms->acpi_dev) {
> -hotplug_handler_plug(x86ms->acpi_dev, dev, &local_err);
> -if (local_err) {
> -goto out;
> -}
> -}
> -
> -/* increment the number of CPUs */
> -x86ms->boot_cpus++;
> -if (x86ms->rtc) {
> -rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
> -}
> -if (x86ms->fw_cfg) {
> -fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
> -}
> -
> -found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
> -found_cpu->cpu = OBJECT(dev);
> -out:
> -error_propagate(errp, local_err);
> -}
> -static void pc_cpu_unplug_r

Re: [PATCH v7 0/8] Generalize start-powered-off property from ARM

2020-08-31 Thread David Gibson
On Wed, Aug 26, 2020 at 02:55:27AM -0300, Thiago Jung Bauermann wrote:
> This version fixes `make check` failures in ppc/e500.c, mips/cps.c and
> sparc/sun4m.c. This was done by moving the qdev_realize_and_unref() call as
> close as possible to the object_new() call, in order to keep the CPU object
> construction as similar as possible to the earlier version which used
> cpu_create().
> 
> I also had to change the patch which removed the main_cpu_reset() function
> from sparc/sun4m.c. It was causing a `make check` failure but I can't
> really explain why. See this message for a few more details:
> 
> https://lists.nongnu.org/archive/html/qemu-ppc/2020-08/msg00419.html
> 
> I dropped the Reviewed-by's on the changed patches because of these
> changes.
> 
> Original cover letter below, followed by changelog:
> 
> The ARM code has a start-powered-off property in ARMCPU, which is a
> subclass of CPUState. This property causes arm_cpu_reset() to set
> CPUState::halted to 1, signalling that the CPU should start in a halted
> state. Other architectures also have code which aim to achieve the same
> effect, but without using a property.
> 
> The ppc/spapr version has a bug where QEMU does a KVM_RUN on the vcpu
> before cs->halted is set to 1, causing the vcpu to run while it's still in
> an unitialized state (more details in patch 3).
> 
> Peter Maydell mentioned the ARM start-powered-off property and
> Eduardo Habkost suggested making it generic, so this patch series does
> that, for all cases which I was able to find via grep in the code.
> 
> The only problem is that I was only able to test these changes on a ppc64le
> pseries KVM guest, so except for patches 2 and 3, all others are only
> build-tested. Also, my grasp of QOM lifecycle is basically non-existant so
> please be aware of that when reviewing this series.
> 
> The last patch may be wrong, as pointed out by Eduardo, so I marked it as
> RFC. It may make sense to drop it.

Applied to ppc-for-5.2, thanks.

> Changes since v6:
> 
> Patch "ppc/e500: Use start-powered-off CPUState property"
> Patch "mips/cps: Use start-powered-off CPUState property"
> - Moved setting of start-powered-off property and qdev_realize_and_unref()
>   to right after object_new(machine->cpu_type).
> - Dropped Philippe's Reviewed-by due to this change.
> 
> Patch "sparc/sun4m: Remove main_cpu_reset()"
> - Dropped.
> 
> Patch "sparc/sun4m: Don't set cs->halted = 0 in main_cpu_reset()"
> - New patch.
> 
> Patch "sparc/sun4m: Use start-powered-off CPUState property"
> - Merged secondary_cpu_reset() with main_cpu_reset().
> - Dropped Philippe's Reviewed-by due to this change.
> 
> Changes since v5:
> 
> Patch "ppc/e500: Use start-powered-off CPUState property"
> Patch "mips/cps: Use start-powered-off CPUState property"
> Patch "sparc/sun4m: Remove main_cpu_reset()"
> Patch "target/s390x: Use start-powered-off CPUState property"
> - Added Philippe's Reviewed-by.
> 
> Patch "sparc/sun4m: Use start-powered-off CPUState property"
> - Move call to qdev_realize_and_unref() right after 
> object_property_set_bool(),
>   as suggested by Philippe.
> 
> Changes since v4:
> 
> Patch "ppc/e500: Use start-powered-off CPUState property"
> Patch "sparc/sun4m: Use start-powered-off CPUState property"
> - Use qdev_realize_and_unref() instead of qdev_realize(), as suggested
>   by Igor.
> - Pass &error_fatal to qdev_realize_and_unref() instead of manually
>   reporting the error and exiting QEMU, as suggested by Philippe.
> - Changed object_property_set_bool() to use &error_fatal instead of
>   &error_abort.
> 
> Patch "mips/cps: Use start-powered-off CPUState property"
> - Use qdev_realize_and_unref() instead of qdev_realize(), as suggested
>   by Igor.
> - Use existing errp argument to propagate error back to the caller, as
>   suggested by Philippe.
> - Changed object_property_set_bool() to use existing errp argument to
>   propagate error back to the caller instead of using &error_abort.
> 
> Changes since v3:
> 
> General:
> - Added David's, Greg's and Cornelia's Reviewed-by and Acked-by to some
>   of the patches.
> - Rebased on top of dgibson/ppc-for-5.2.
> 
> Patch "ppc/e500: Use start-powered-off CPUState property"
> Patch "mips/cps: Use start-powered-off CPUState property"
> Patch "sparc/sun4m: Use start-powered-off CPUState property"
> - Initialize CPU object with object_new() and qdev_realize() instead
>   of cpu_create().
> - Removed Reviewed-by's and Acked-by's from these patches because of
>   these changes.
> 
> Changes since v2:
> 
> General:
> - Added Philippe's Reviewed-by to some of the patches.
> 
> Patch "ppc/spapr: Use start-powered-off CPUState property"
> - Set the CPUState::start_powered_off variable directly rather than using
>   object_property_set_bool(). Suggested by Philippe.
> 
> Patch "sparc/sun4m: Remove main_cpu_reset()"
> - New patch. Suggested by Philippe.
> 
> Patch "sparc/sun4m: Use start-powered-off CPUState property"
> - Remove secondary_cpu_reset(). Suggested by

Re: [PATCH v6 14/20] x86: move acpi_dev from pc/microvm

2020-08-31 Thread Igor Mammedov
On Wed, 26 Aug 2020 12:52:48 +0200
Gerd Hoffmann  wrote:

> Both pc and microvm machine types have a acpi_dev field.
> Move it to the common base type.
> 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Igor Mammedov 

> ---
>  include/hw/i386/microvm.h |  1 -
>  include/hw/i386/pc.h  |  1 -
>  include/hw/i386/x86.h |  1 +
>  hw/i386/acpi-build.c  |  2 +-
>  hw/i386/acpi-microvm.c|  5 +++--
>  hw/i386/microvm.c | 10 ++
>  hw/i386/pc.c  | 34 +++---
>  hw/i386/pc_piix.c |  2 +-
>  hw/i386/pc_q35.c  |  2 +-
>  9 files changed, 32 insertions(+), 26 deletions(-)
> 
> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index b6e0d4395af7..b8ec99aeb051 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -66,7 +66,6 @@ typedef struct {
>  bool kernel_cmdline_fixed;
>  Notifier machine_done;
>  Notifier powerdown_req;
> -AcpiDeviceIf *acpi_dev;
>  } MicrovmMachineState;
>  
>  #define TYPE_MICROVM_MACHINE   MACHINE_TYPE_NAME("microvm")
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index fe52e165b27c..0f7da2329b0f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -29,7 +29,6 @@ struct PCMachineState {
>  Notifier machine_done;
>  
>  /* Pointers to devices and objects: */
> -HotplugHandler *acpi_dev;
>  PCIBus *bus;
>  I2CBus *smbus;
>  PFlashCFI01 *flash[2];
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index a350ea3609f5..de74c831c3ab 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -50,6 +50,7 @@ typedef struct {
>  FWCfgState *fw_cfg;
>  qemu_irq *gsi;
>  GMappedFile *initrd_mapped_file;
> +HotplugHandler *acpi_dev;
>  
>  /* RAM information (sizes, addresses, configuration): */
>  ram_addr_t below_4g_mem_size, above_4g_mem_size;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bc2a35..c356cc71fe08 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2431,7 +2431,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
> *machine)
>  
>  acpi_add_table(table_offsets, tables_blob);
>  acpi_build_madt(tables_blob, tables->linker, x86ms,
> -ACPI_DEVICE_IF(pcms->acpi_dev), true);
> +ACPI_DEVICE_IF(x86ms->acpi_dev), true);
>  
>  vmgenid_dev = find_vmgenid_dev();
>  if (vmgenid_dev) {
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index b9ce3768b263..df39c5d3bd90 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -108,7 +108,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
>  sb_scope = aml_scope("_SB");
>  fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
>  isa_build_aml(ISA_BUS(isabus), sb_scope);
> -build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
> +build_ged_aml(sb_scope, GED_DEVICE, x86ms->acpi_dev,
>GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
>  acpi_dsdt_add_power_button(sb_scope);
>  acpi_dsdt_add_virtio(sb_scope, mms);
> @@ -136,6 +136,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
> MicrovmMachineState *mms)
>  {
>  MachineState *machine = MACHINE(mms);
> +X86MachineState *x86ms = X86_MACHINE(mms);
>  GArray *table_offsets;
>  GArray *tables_blob = tables->table_data;
>  unsigned dsdt, xsdt;
> @@ -183,7 +184,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
>  
>  acpi_add_table(table_offsets, tables_blob);
>  acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
> -mms->acpi_dev, false);
> +ACPI_DEVICE_IF(x86ms->acpi_dev), false);
>  
>  xsdt = tables_blob->len;
>  build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 04209eb38fbe..9df15354ce0f 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -143,7 +143,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
>  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> x86ms->gsi[GED_MMIO_IRQ]);
>  sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> -mms->acpi_dev = ACPI_DEVICE_IF(dev);
> +x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
>  }
>  
>  if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {
> @@ -469,11 +469,13 @@ static void microvm_powerdown_req(Notifier *notifier, 
> void *data)
>  {
>  MicrovmMachineState *mms = container_of(notifier, MicrovmMachineState,
>  powerdown_req);
> +X86MachineState *x86ms = X86_MACHINE(mms);
>  
> -if (mms->acpi_dev) {
> -Object *obj = OBJECT(mms->acpi_dev);
> +if (x86ms->acpi_dev) {
> +Object *obj = OBJECT(x86ms->acpi_dev);
>  AcpiDeviceIfClass

Re: [PATCH v6 13/20] x86: constify x86_machine_is_*_enabled

2020-08-31 Thread Igor Mammedov
On Wed, 26 Aug 2020 12:52:47 +0200
Gerd Hoffmann  wrote:

> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Igor Mammedov 

> ---
>  include/hw/i386/x86.h | 4 ++--
>  hw/i386/x86.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index b79f24e28545..a350ea3609f5 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -110,8 +110,8 @@ void x86_load_linux(X86MachineState *x86ms,
>  bool pvh_enabled,
>  bool linuxboot_dma_enabled);
>  
> -bool x86_machine_is_smm_enabled(X86MachineState *x86ms);
> -bool x86_machine_is_acpi_enabled(X86MachineState *x86ms);
> +bool x86_machine_is_smm_enabled(const X86MachineState *x86ms);
> +bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms);
>  
>  /* Global System Interrupts */
>  
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index cf384b974318..31a82885d735 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -845,7 +845,7 @@ void x86_bios_rom_init(MemoryRegion *rom_memory, bool 
> isapc_ram_fw)
>  bios);
>  }
>  
> -bool x86_machine_is_smm_enabled(X86MachineState *x86ms)
> +bool x86_machine_is_smm_enabled(const X86MachineState *x86ms)
>  {
>  bool smm_available = false;
>  
> @@ -887,7 +887,7 @@ static void x86_machine_set_smm(Object *obj, Visitor *v, 
> const char *name,
>  visit_type_OnOffAuto(v, name, &x86ms->smm, errp);
>  }
>  
> -bool x86_machine_is_acpi_enabled(X86MachineState *x86ms)
> +bool x86_machine_is_acpi_enabled(const X86MachineState *x86ms)
>  {
>  if (x86ms->acpi == ON_OFF_AUTO_OFF) {
>  return false;




Re: [PATCH 0/7] target/arm: Add vSPE support to KVM guest

2020-08-31 Thread Auger Eric
Hi Haibo,

On 8/7/20 10:10 AM, Haibo Xu wrote:
> This series add support for SPE(Statistical Profiling Extension)[1]
> in KVM guest. It's based on Andrew Murray's kernel KVM patches V2[2],
> and has been tested to ensure that guest can use SPE with valid data.
> E.g.
> 
> In host:
> $ ./qemu-system-aarch64 \
> -cpu host -M virt,accel=kvm,gic-version=3 -nographic -m 2048M \
> -kernel ./Image-new \
> -initrd /boot/initrd.img-5.6.0-rc2+ \
> -append "root=/dev/vda rw console=ttyAMA0" -nodefaults -serial stdio\
> -drive if=none,file=./xenial.rootfs.ext4,id=hd0,format=raw \
> -device virtio-blk-device,drive=hd0  \
> 
> In guest:
> $ perf record -e arm_spe/ts_enable=1,pa_enable=1,pct_enable=1/ \
> dd if=/dev/zero of=/dev/null count=1000
> $ perf report --dump-raw-trace > spe_buf.txt
> 
> The spe_buf.txt should contain similar data as below:
> 
> . ... ARM SPE data: size 135944 bytes
> .  :  b0 f4 d3 29 10 00 80 ff a0  PC 
> 0xff80001029d3f4 el1 ns=1
> .  0009:  99 0b 00LAT 11 ISSUE
> .  000c:  98 0d 00LAT 13 TOT
> .  000f:  52 16 00EV RETIRED 
> L1D-ACCESS TLB-ACCESS
> .  0012:  49 00   LD
> .  0014:  b2 d0 40 d8 70 00 00 ff 00  VA 
> 0xff70d840d0
> .  001d:  9a 01 00LAT 1 XLAT
> .  0020:  00 00 00PAD
> .  0023:  71 a5 1f b3 20 14 00 00 00  TS 86447955877
> .  002c:  b0 7c f9 29 10 00 80 ff a0  PC 
> 0xff80001029f97c el1 ns=1
> .  0035:  99 02 00LAT 2 ISSUE
> .  0038:  98 03 00LAT 3 TOT
> .  003b:  52 02 00EV RETIRED
> .  003e:  48 00   INSN-OTHER
> .  0040:  00 00 00PAD
> .  0043:  71 ef 1f b3 20 14 00 00 00  TS 86447955951
> .  004c:  b0 f0 e9 29 10 00 80 ff a0  PC 
> 0xff80001029e9f0 el1 ns=1
> .  0055:  99 02 00LAT 2 ISSUE
> .  0058:  98 03 00LAT 3 TOT
> .  005b:  52 02 00EV RETIRED
> 
> If you want to disable the vSPE support, you can use the 'spe=off' cpu
> property:
> 
> ./qemu-system-aarch64 \
> -cpu host,spe=off -M virt,accel=kvm,gic-version=3 -nographic -m 2048M 
> \
> -kernel ./Image-new \
> -initrd /boot/initrd.img-5.6.0-rc2+ \
> -append "root=/dev/vda rw console=ttyAMA0" -nodefaults -serial stdio\
> -drive if=none,file=./xenial.rootfs.ext4,id=hd0,format=raw \
> -device virtio-blk-device,drive=hd0  \
> 
> Note:
> (1) Since the kernel patches are still under review, some of the macros
> in the header files may be changed after merging. We may need to
> update them accordingly.
to be more explicit one needs to replace on the kernel 5.5-rc2 based series

-#define KVM_CAP_ARM_SPE_V1 179
+#define KVM_CAP_ARM_SPE_V1 184

I got misleaded ;-)

+ Andrew in CC as he contributed the kernel part.

For information, I have been working on a kvm unit test series for
testing SPE. I will send an RFC, most probably this week. At the moment
I still face some weirdness such as some unexpected Service state in the
syndrome register. Anyway I will share the existing code so that we can
discuss the issues.

Are there any plans to respin the kernel series

Thanks

Eric

> (2) These patches only add vSPE support in KVM mode, for TCG mode, I'm
> not sure whether we need to support it.
> (3) Just followed the 'pmu' property, we only allow this feature to be
> removed from CPUs which enable it by default. But since the SPE is
> an optional feature extension for Armv8.2, I think a better way may
> be to disable it by default, and only enable it when the host cpu
> do have the feature.
> 
> [1]https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/
>posts/statistical-profiling-extension-for-armv8-a
> [2]https://www.spinics.net/lists/arm-kernel/msg776228.html
> 
> Haibo Xu (7):
>   update Linux headers with new vSPE macros
>   target/arm/kvm: spe: Add helper to detect SPE when using KVM
>   target/arm/cpu: spe: Add an option to turn on/off vSPE support
>   target/arm/kvm: spe: Unify device attr operatioin helper
>   target/arm/kvm: spe: Add device init and set_irq operations
>   hw/arm/virt: spe: Add SPE fdt binding for virt machine
>   target/arm/cpu: spe: Enable spe to work with host cpu
> 
>  hw/arm/virt-acpi-build.c  |  3 +++
>  hw/arm/virt.c | 42 ++
>  include/hw/acpi/

Re: [PATCH v6 08/20] microvm/acpi: add minimal acpi support

2020-08-31 Thread Igor Mammedov
On Wed, 26 Aug 2020 12:52:42 +0200
Gerd Hoffmann  wrote:

> $subject says all.  Can be controlled using -M microvm,acpi=on/off.
> 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Igor Mammedov 

> ---
>  hw/i386/acpi-microvm.h|   8 ++
>  include/hw/i386/microvm.h |   9 ++
>  hw/i386/acpi-microvm.c| 187 ++
>  hw/i386/microvm.c |  40 
>  hw/i386/Kconfig   |   1 +
>  hw/i386/meson.build   |   2 +-
>  6 files changed, 246 insertions(+), 1 deletion(-)
>  create mode 100644 hw/i386/acpi-microvm.h
>  create mode 100644 hw/i386/acpi-microvm.c
> 
> diff --git a/hw/i386/acpi-microvm.h b/hw/i386/acpi-microvm.h
> new file mode 100644
> index ..dfe853690e15
> --- /dev/null
> +++ b/hw/i386/acpi-microvm.h
> @@ -0,0 +1,8 @@
> +#ifndef HW_I386_ACPI_MICROVM_H
> +#define HW_I386_ACPI_MICROVM_H
> +
> +#include "hw/i386/microvm.h"
> +
> +void acpi_setup_microvm(MicrovmMachineState *mms);
> +
> +#endif
> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index 03e735723726..b6e0d4395af7 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -24,12 +24,18 @@
>  
>  #include "hw/boards.h"
>  #include "hw/i386/x86.h"
> +#include "hw/acpi/acpi_dev_interface.h"
>  
>  /* Platform virtio definitions */
>  #define VIRTIO_MMIO_BASE  0xfeb0
>  #define VIRTIO_NUM_TRANSPORTS 8
>  #define VIRTIO_CMDLINE_MAXLEN 64
>  
> +#define GED_MMIO_BASE 0xfea0
> +#define GED_MMIO_BASE_MEMHP   (GED_MMIO_BASE + 0x100)
> +#define GED_MMIO_BASE_REGS(GED_MMIO_BASE + 0x200)
> +#define GED_MMIO_IRQ  9
> +
>  /* Machine type options */
>  #define MICROVM_MACHINE_PIT "pit"
>  #define MICROVM_MACHINE_PIC "pic"
> @@ -58,6 +64,9 @@ typedef struct {
>  /* Machine state */
>  uint32_t virtio_irq_base;
>  bool kernel_cmdline_fixed;
> +Notifier machine_done;
> +Notifier powerdown_req;
> +AcpiDeviceIf *acpi_dev;
>  } MicrovmMachineState;
>  
>  #define TYPE_MICROVM_MACHINE   MACHINE_TYPE_NAME("microvm")
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> new file mode 100644
> index ..06ef33949f5f
> --- /dev/null
> +++ b/hw/i386/acpi-microvm.c
> @@ -0,0 +1,187 @@
> +/* Support for generating ACPI tables and passing them to Guests
> + *
> + * Copyright (C) 2008-2010  Kevin O'Connor 
> + * Copyright (C) 2006 Fabrice Bellard
> + * Copyright (C) 2013 Red Hat Inc
> + *
> + * Author: Michael S. Tsirkin 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +
> +#include "exec/memory.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/bios-linker-loader.h"
> +#include "hw/acpi/generic_event_device.h"
> +#include "hw/acpi/utils.h"
> +#include "hw/boards.h"
> +#include "hw/i386/fw_cfg.h"
> +#include "hw/i386/microvm.h"
> +
> +#include "acpi-common.h"
> +#include "acpi-microvm.h"
> +
> +static void
> +build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
> +   MicrovmMachineState *mms)
> +{
> +X86MachineState *x86ms = X86_MACHINE(mms);
> +Aml *dsdt, *sb_scope, *scope, *pkg;
> +bool ambiguous;
> +Object *isabus;
> +
> +isabus = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
> +assert(isabus);
> +assert(!ambiguous);
> +
> +dsdt = init_aml_allocator();
> +
> +/* Reserve space for header */
> +acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
> +
> +sb_scope = aml_scope("_SB");
> +fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
> +isa_build_aml(ISA_BUS(isabus), sb_scope);
> +build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
> +  GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
> +acpi_dsdt_add_power_button(sb_scope);
> +aml_append(dsdt, sb_scope);
> +
> +/* ACPI 5.0: Table 7-209 System State Package */
> +scope = aml_scope("\\");
> +pkg = aml_package(4);
> +aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5));
> +aml_append(pkg, aml_int(0)); /* ignored */
> +aml_append(pkg, aml_int(0)); /* reserved */
> +aml_append(pkg, aml_int(0)); /* reserved */
> +aml_append(scope, aml_name_decl("_S5", pkg));
> +aml_append(dsdt, scope);
> +
> +/* copy AML table into ACPI tables blob and

Re: Meson build on macOS: undefined symbol treatment

2020-08-31 Thread Emmanuel Blot

On 30 Aug 2020, at 11:35, Paolo Bonzini wrote:

This is done by "b_lundef=false".  I think it was added for modules, 
but
maybe it's not necessary.  Emmanuel, can you try removing it (line 3 
of

meson.build) and seeing if --enable-modules still works for you?


Removing this option does restore the regular behavior on macOS (link 
stage fails whenever some symbols are missing), and QEMU still builds ok 
with —enable-modules option, but I have not actually tested this 
feature - I did not know it even existed. Do you want me to test some 
specific use cases?


Thanks,
Emmanuel.



Re: [PATCH 2/2] gitlab: expand test coverage for crypto builds

2020-08-31 Thread Thomas Huth
On 28/08/2020 15.27, Daniel P. Berrangé wrote:
> Most jobs test the latest nettle library. This adds explicit coverage
> for latest gcrypt using Fedora, and old gcrypt and nettle using
> CentOS-7. The latter does a minimal tools-only build, as we only need to
> validate that the crypto code builds and unit tests pass. Finally a job
> disabling both nettle and gcrypt is provided to validate that gnutls
> still works.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  .gitlab-ci.yml  | 66 +
>  tests/docker/dockerfiles/centos7.docker |  2 +
>  tests/docker/dockerfiles/centos8.docker |  1 +
>  3 files changed, 69 insertions(+)

Acked-by: Thomas Huth 




Re: [PATCH v2 4/4] pc-bios: s390x: Go into disabled wait when encountering a PGM exception

2020-08-31 Thread Christian Borntraeger



On 27.08.20 11:31, Janosch Frank wrote:
> Let's setup a PGM PSW, so we won't load 0s when a program exception
> happens. Instead we'll load a disabled wait PSW.

Makes sense

Reviewed-by: Christian Borntraeger 


> 
> Signed-off-by: Janosch Frank 
> ---
>  pc-bios/s390-ccw/start.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
> index 939aac3a7c..775b45baeb 100644
> --- a/pc-bios/s390-ccw/start.S
> +++ b/pc-bios/s390-ccw/start.S
> @@ -44,6 +44,9 @@ done:
>  larl  %r2, external_new_psw
>  stg   %r1, 8(%r2)
>  mvc   0x1b0(16),0(%r2)
> +/* set up a pgm exception disabled wait psw */
> +larl  %r2, disabled_wait_psw
> +mvc   0x01d0(16), 0(%r2)
>  j  main  /* And call C */
>  
>  memsetxc:
> 



Re: [PATCH v2 1/7] vhost: recheck dev state in the vhost_migration_log routine

2020-08-31 Thread Dima Stepanov
On Thu, Aug 27, 2020 at 09:44:22PM -0400, Raphael Norwitz wrote:
> Generally I’m happy with this. Some comments:
> 
> On Mon, Aug 24, 2020 at 4:40 AM Dima Stepanov  wrote:
> >
> > vhost-user devices can get a disconnect in the middle of the VHOST-USER
> > handshake on the migration start. If disconnect event happened right
> > before sending next VHOST-USER command, then the vhost_dev_set_log()
> > call in the vhost_migration_log() function will return error. This error
> > will lead to the assert() and close the QEMU migration source process.
> > For the vhost-user devices the disconnect event should not break the
> > migration process, because:
> >   - the device will be in the stopped state, so it will not be changed
> > during migration
> >   - if reconnect will be made the migration log will be reinitialized as
> > part of reconnect/init process:
> > #0  vhost_log_global_start (listener=0x563989cf7be0)
> > at hw/virtio/vhost.c:920
> > #1  0x56398603d8bc in listener_add_address_space 
> > (listener=0x563989cf7be0,
> > as=0x563986ea4340 )
> > at softmmu/memory.c:2664
> > #2  0x56398603dd30 in memory_listener_register 
> > (listener=0x563989cf7be0,
> > as=0x563986ea4340 )
> > at softmmu/memory.c:2740
> > #3  0x563985fd6956 in vhost_dev_init (hdev=0x563989cf7bd8,
> > opaque=0x563989cf7e30, backend_type=VHOST_BACKEND_TYPE_USER,
> > busyloop_timeout=0)
> > at hw/virtio/vhost.c:1385
> > #4  0x563985f7d0b8 in vhost_user_blk_connect (dev=0x563989cf7990)
> > at hw/block/vhost-user-blk.c:315
> > #5  0x563985f7d3f6 in vhost_user_blk_event (opaque=0x563989cf7990,
> > event=CHR_EVENT_OPENED)
> > at hw/block/vhost-user-blk.c:379
> > Update the vhost-user-blk device with the internal started field which
> > will be used for initialization and clean up. The disconnect event will
> > set the overall VHOST device to the stopped state, so it can be used by
> > the vhost_migration_log routine.
> > Such approach could be propogated to the other vhost-user devices, but
> > better idea is just to make the same connect/disconnect code for all the
> > vhost-user devices.
> >
> > This migration issue was slightly discussed earlier:
> >   - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html
> >   - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05241.html
> >
> 
> What you’re doing here on a structural level is adding an additional
> flag “started” to VhostUserBlk to track whether the device really
> needs to be stopped and cleaned up on a vhost level on a disconnect.
> Can you make that more clear in the commit message as you have in the
> comments?
Sounds reasonable, will update the commit message.

> 
> 
> > Signed-off-by: Dima Stepanov 
> > ---
> >  hw/block/vhost-user-blk.c  | 19 ---
> >  hw/virtio/vhost.c  | 27 ---
> >  include/hw/virtio/vhost-user-blk.h | 10 ++
> >  3 files changed, 50 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > index a00b854..5573e89 100644
> > --- a/hw/block/vhost-user-blk.c
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -150,6 +150,7 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
> >  error_report("Error starting vhost: %d", -ret);
> >  goto err_guest_notifiers;
> >  }
> > +s->started = true;
> >
> >  /* guest_notifier_mask/pending not used yet, so just unmask
> >   * everything here. virtio-pci will do the right thing by
> > @@ -175,6 +176,11 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
> >  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> >  int ret;
> >
> > +if (!s->started) {
> > +return;
> > +}
> > +s->started = false;
> > +
> >  if (!k->set_guest_notifiers) {
> >  return;
> >  }
> > @@ -341,9 +347,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
> >  }
> >  s->connected = false;
> >
> > -if (s->dev.started) {
> > -vhost_user_blk_stop(vdev);
> > -}
> > +vhost_user_blk_stop(vdev);
> >
> >  vhost_dev_cleanup(&s->dev);
> >  }
> > @@ -399,6 +403,15 @@ static void vhost_user_blk_event(void *opaque, 
> > QEMUChrEvent event)
> >  NULL, NULL, false);
> >  aio_bh_schedule_oneshot(ctx, vhost_user_blk_chr_closed_bh, 
> > opaque);
> >  }
> > +
> > +/*
> > + * Move vhost device to the stopped state. The vhost-user device
> > + * will be clean up and disconnected in BH. This can be useful in
> > + * the vhost migration code. If disconnect was caught there is an
> > + * option for the general vhost code to get the dev state without
> > + * knowing its type (in this case vhost-user).
> > + */
> > +s->dev.started = false;
> >  break;
> >  case CHR_EVENT_BREAK:
> >  case CHR_EVENT_MUX_IN:
>

[PATCH 3/9] vhost-vdpa: remove the unnecessary assert(s->vhost_net)

2020-08-31 Thread Jason Wang
vhost_vdpa_add() can fail before s->vhost_net is set and
net_vhost_vdpa_init() can report error. So there's no need for this
assert. Let's remove it.

Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index b7221beaa1..c4568b885e 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -190,7 +190,7 @@ static int net_vhost_vdpa_init(NetClientState *peer, const 
char *device,
 }
 s->vhost_vdpa.device_fd = vdpa_device_fd;
 ret = vhost_vdpa_add(nc, (void *)&s->vhost_vdpa);
-assert(s->vhost_net);
+
 return ret;
 }
 
-- 
2.20.1




[PATCH 2/9] vhost-vdpa: mandate vhostdev

2020-08-31 Thread Jason Wang
vhost-dev is mandatory for vhost-vdpa to be initialized otherwise
net_vhost_vdpa_init will pass an uninitialized pointer to qemu_open()
which will lead a SIGSEV:

#0  0x55c3a640 in strstart (str=str@entry=0x0, 
val=val@entry=0x55dbcb65 "/dev/fdset/", ptr=ptr@entry=0x7fffdfb8) at 
../util/cutils.c:77
#1  0x55c572a7 in qemu_open (name=name@entry=0x0, flags=flags@entry=2) 
at ../util/osdep.c:294
#2  0x5584314a in net_vhost_vdpa_init (device=0x55c81baa 
"vhost-vdpa", vhostdev=0x0, name=0x56396600 "hn0", peer=0x0) at 
../net/vhost-vdpa.c:187
#3  0x5584314a in net_init_vhost_vdpa (netdev=, 
name=0x56396600 "hn0", peer=0x0, errp=) at 
../net/vhost-vdpa.c:227
#4  0x5587e8c9 in net_client_init1 (netdev=0x56398790, 
is_netdev=is_netdev@entry=true, errp=errp@entry=0x7fffe290) at 
../net/net.c:1008
#5  0x5587ecc7 in net_client_init (opts=0x56192ff0, 
is_netdev=, errp=0x7fffe290) at ../net/net.c:1113
#6  0x55c33082 in qemu_opts_foreach
(list=, func=func@entry=0x5587ed50 , 
opaque=opaque@entry=0x0, errp=errp@entry=0x7fffe290) at 
../util/qemu-option.c:1172
#7  0x55880057 in net_init_clients (errp=errp@entry=0x7fffe290) at 
../net/net.c:1494
#8  0x55a7f8e9 in qemu_init (argc=, argv=, envp=) at ../softmmu/vl.c:4250
#9  0x557f26cd in main (argc=, argv=, 
envp=) at ../softmmu/main.c:49

Cc: qemu-sta...@nongnu.org
Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index bc0e0d2d35..b7221beaa1 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -206,7 +206,7 @@ static int net_vhost_check_net(void *opaque, QemuOpts 
*opts, Error **errp)
 }
 if (strcmp(netdev, name) == 0 &&
 !g_str_has_prefix(driver, "virtio-net-")) {
-error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
+error_setg(errp, "Vhost-vdpa requires frontend driver virtio-net-*");
 return -1;
 }
 return 0;
@@ -224,5 +224,9 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
   (char *)name, errp)) {
 return -1;
 }
+if (!opts->has_vhostdev) {
+error_setg(errp, "vhost-vdpa requires vhostdev to be set");
+return -1;
+}
 return net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, opts->vhostdev);
 }
-- 
2.20.1




[PATCH 0/9] refine vhost-vdpa initialization

2020-08-31 Thread Jason Wang
Hi all:

This series refine vhost-vdpa initialization by:

- fixing SIGSEV when vhostdev is not specificed
- tweak the codes for vhost-vdpa initialization
- allow to accept preopen vhost-vdpa file descriptor

Please review.

Thanks

Jason Wang (9):
  vhost-vdpa: remove the default devname
  vhost-vdpa: mandate vhostdev
  vhost-vdpa: remove the unnecessary assert(s->vhost_net)
  vhost-vdpa: refine info string
  vhost-vdpa: remove the unnecessary initialization
  vhost-vdpa: remove the unnecessary queue index assignment
  vhost-vdpa: squash net_vhost_vdpa_init() into net_init_vhost_vdpa()
  vhost-vdpa: add accurate error string when fail to open vhost vDPA
device
  vhost-vdpa: allow pre-opend file descriptor

 net/vhost-vdpa.c | 63 ++--
 qapi/net.json|  6 +++--
 2 files changed, 43 insertions(+), 26 deletions(-)

-- 
2.20.1




[PATCH 1/9] vhost-vdpa: remove the default devname

2020-08-31 Thread Jason Wang
The code doesn't have a default vhostdev, so remove the default
description in net.json.

Signed-off-by: Jason Wang 
---
 qapi/net.json | 1 -
 1 file changed, 1 deletion(-)

diff --git a/qapi/net.json b/qapi/net.json
index ddb113e5e5..a2a94fad3e 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -438,7 +438,6 @@
 # specifications with a vendor specific control path.
 #
 # @vhostdev: path of vhost-vdpa device
-#(default:'/dev/vhost-vdpa-0')
 #
 # @queues: number of queues to be created for multiqueue vhost-vdpa
 #  (default: 1)
-- 
2.20.1




[PATCH 5/9] vhost-vdpa: remove the unnecessary initialization

2020-08-31 Thread Jason Wang
Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 397d4d3082..bcbf49d55b 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -175,10 +175,10 @@ static NetClientInfo net_vhost_vdpa_info = {
 static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
const char *name, const char *vhostdev)
 {
-NetClientState *nc = NULL;
+NetClientState *nc;
 VhostVDPAState *s;
-int vdpa_device_fd = -1;
-int ret = 0;
+int vdpa_device_fd;
+int ret;
 assert(name);
 nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
 snprintf(nc->info_str, sizeof(nc->info_str), "vhostdev=%s", vhostdev);
-- 
2.20.1




[PATCH 7/9] vhost-vdpa: squash net_vhost_vdpa_init() into net_init_vhost_vdpa()

2020-08-31 Thread Jason Wang
This patch squashes net_vhost_vdpa_init() into
net_init_vhost_vdpa(). This will simplify adding pre open file
descriptor support.

Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 41 +++--
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 1d3ac89135..f5cc4e8326 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -172,27 +172,6 @@ static NetClientInfo net_vhost_vdpa_info = {
 .has_ufo = vhost_vdpa_has_ufo,
 };
 
-static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
-   const char *name, const char *vhostdev)
-{
-NetClientState *nc;
-VhostVDPAState *s;
-int vdpa_device_fd;
-int ret;
-assert(name);
-nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
-snprintf(nc->info_str, sizeof(nc->info_str), "vhostdev=%s", vhostdev);
-s = DO_UPCAST(VhostVDPAState, nc, nc);
-vdpa_device_fd = qemu_open(vhostdev, O_RDWR);
-if (vdpa_device_fd == -1) {
-return -errno;
-}
-s->vhost_vdpa.device_fd = vdpa_device_fd;
-ret = vhost_vdpa_add(nc, (void *)&s->vhost_vdpa);
-
-return ret;
-}
-
 static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
 {
 const char *name = opaque;
@@ -215,6 +194,10 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
 NetClientState *peer, Error **errp)
 {
 const NetdevVhostVDPAOptions *opts;
+NetClientState *nc;
+VhostVDPAState *s;
+int vdpa_device_fd;
+int ret;
 
 assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);
 opts = &netdev->u.vhost_vdpa;
@@ -227,5 +210,19 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
 error_setg(errp, "vhost-vdpa requires vhostdev to be set");
 return -1;
 }
-return net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, opts->vhostdev);
+
+assert(name);
+
+nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, TYPE_VHOST_VDPA, 
name);
+snprintf(nc->info_str, sizeof(nc->info_str), "vhostdev=%s", 
opts->vhostdev);
+
+s = DO_UPCAST(VhostVDPAState, nc, nc);
+vdpa_device_fd = qemu_open(opts->vhostdev, O_RDWR);
+if (vdpa_device_fd == -1) {
+return -errno;
+}
+s->vhost_vdpa.device_fd = vdpa_device_fd;
+ret = vhost_vdpa_add(nc, (void *)&s->vhost_vdpa);
+
+return ret;
 }
-- 
2.20.1




[PATCH 4/9] vhost-vdpa: refine info string

2020-08-31 Thread Jason Wang
We use "vhost-vdpa" as nic info string which is not useful, let's add
the vhostdev path.

before:
info network
v0: index=0,type=vhost-vdpa,vhost-vdpa

after:
info network
v0: index=0,type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0

Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index c4568b885e..397d4d3082 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -181,7 +181,7 @@ static int net_vhost_vdpa_init(NetClientState *peer, const 
char *device,
 int ret = 0;
 assert(name);
 nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
-snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
+snprintf(nc->info_str, sizeof(nc->info_str), "vhostdev=%s", vhostdev);
 nc->queue_index = 0;
 s = DO_UPCAST(VhostVDPAState, nc, nc);
 vdpa_device_fd = qemu_open(vhostdev, O_RDWR);
-- 
2.20.1




[PATCH 6/9] vhost-vdpa: remove the unnecessary queue index assignment

2020-08-31 Thread Jason Wang
The nc is allocated through g_malloc0(), so no need to assign its
queue_index to zero.

Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index bcbf49d55b..1d3ac89135 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -182,7 +182,6 @@ static int net_vhost_vdpa_init(NetClientState *peer, const 
char *device,
 assert(name);
 nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
 snprintf(nc->info_str, sizeof(nc->info_str), "vhostdev=%s", vhostdev);
-nc->queue_index = 0;
 s = DO_UPCAST(VhostVDPAState, nc, nc);
 vdpa_device_fd = qemu_open(vhostdev, O_RDWR);
 if (vdpa_device_fd == -1) {
-- 
2.20.1




[PULL 08/18] hw/usb: Add U2F key base class implementation

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

This patch adds the U2F key base class implementation.

The U2F key base mainly takes care of the HID interfacing with guest.
On the one hand, it retrieves the guest U2FHID packets and transmits
them to the variant associated according to the mode: pass-through
or emulated.
On the other hand, it provides the public API used by its variants to
send U2FHID packets to the guest.

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-5-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/u2f.c | 352 +++
 1 file changed, 352 insertions(+)
 create mode 100644 hw/usb/u2f.c

diff --git a/hw/usb/u2f.c b/hw/usb/u2f.c
new file mode 100644
index ..bc09191f063e
--- /dev/null
+++ b/hw/usb/u2f.c
@@ -0,0 +1,352 @@
+/*
+ * U2F USB device.
+ *
+ * Copyright (c) 2020 César Belley 
+ * Written by César Belley 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/usb.h"
+#include "hw/usb/hid.h"
+#include "migration/vmstate.h"
+#include "desc.h"
+
+#include "u2f.h"
+
+/* U2F key Vendor / Product */
+#define U2F_KEY_VENDOR_NUM 0x46f4 /* CRC16() of "QEMU" */
+#define U2F_KEY_PRODUCT_NUM0x0005
+
+enum {
+STR_MANUFACTURER = 1,
+STR_PRODUCT,
+STR_SERIALNUMBER,
+STR_CONFIG,
+STR_INTERFACE
+};
+
+static const USBDescStrings desc_strings = {
+[STR_MANUFACTURER] = "QEMU",
+[STR_PRODUCT]  = "U2F USB key",
+[STR_SERIALNUMBER] = "0",
+[STR_CONFIG]   = "U2F key config",
+[STR_INTERFACE]= "U2F key interface"
+};
+
+static const USBDescIface desc_iface_u2f_key = {
+.bInterfaceNumber  = 0,
+.bNumEndpoints = 2,
+.bInterfaceClass   = USB_CLASS_HID,
+.bInterfaceSubClass= 0x0,
+.bInterfaceProtocol= 0x0,
+.ndesc = 1,
+.descs = (USBDescOther[]) {
+{
+/* HID descriptor */
+.data = (uint8_t[]) {
+0x09,  /*  u8  bLength */
+USB_DT_HID,/*  u8  bDescriptorType */
+0x10, 0x01,/*  u16 HID_class */
+0x00,  /*  u8  country_code */
+0x01,  /*  u8  num_descriptors */
+USB_DT_REPORT, /*  u8  type: Report */
+0x22, 0,   /*  u16 len */
+},
+},
+},
+.eps = (USBDescEndpoint[]) {
+{
+.bEndpointAddress  = USB_DIR_IN | 0x01,
+.bmAttributes  = USB_ENDPOINT_XFER_INT,
+.wMaxPacketSize= U2FHID_PACKET_SIZE,
+.bInterval = 0x05,
+}, {
+.bEndpointAddress  = USB_DIR_OUT | 0x01,
+.bmAttributes  = USB_ENDPOINT_XFER_INT,
+.wMaxPacketSize= U2FHID_PACKET_SIZE,
+.bInterval = 0x05,
+},
+},
+
+};
+
+static const USBDescDevice desc_device_u2f_key = {
+.bcdUSB= 0x0100,
+.bMaxPacketSize0   = U2FHID_PACKET_SIZE,
+.bNumConfigurations= 1,
+.confs = (USBDescConfig[]) {
+{
+.bNumInterfaces= 1,
+.bConfigurationValue   = 1,
+.iConfiguration= STR_CONFIG,
+.bmAttributes  = USB_CFG_ATT_ONE,
+.bMaxPower = 15,
+.nif = 1,
+.ifs = &desc_iface_u2f_key,
+},
+},
+};
+
+static const USBDesc desc_u2f_key = {
+.id = {
+.idVendor  = U2F_KEY_VENDOR_NUM,
+.idProduct = U2F_KEY_PRODUCT_NUM,
+.bcdDevice = 0,
+.iManufacturer = STR_MANUFACTURER,
+.iProduct  = STR_PRODUCT,
+.iSerialNumber = STR_SERIALNUMBER,
+},
+.full

[PULL 04/18] ehci: drop pointless warn_report for guest bugs.

2020-08-31 Thread Gerd Hoffmann
We have a tracepoint at the same place which can be enabled if needed.

Buglink: https://bugzilla.redhat.com//show_bug.cgi?id=1859236
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200722072613.10390-1-kra...@redhat.com>
---
 hw/usb/hcd-ehci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 4da446d2de6b..2b995443fbfd 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -352,7 +352,6 @@ static void ehci_trace_sitd(EHCIState *s, hwaddr addr,
 static void ehci_trace_guest_bug(EHCIState *s, const char *message)
 {
 trace_usb_ehci_guest_bug(message);
-warn_report("%s", message);
 }
 
 static inline bool ehci_enabled(EHCIState *s)
-- 
2.27.0




[PATCH 8/9] vhost-vdpa: add accurate error string when fail to open vhost vDPA device

2020-08-31 Thread Jason Wang
This patch adds more accurate error string when fail to open vhost
vDPA device.

Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index f5cc4e8326..9a6f0b63d3 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -219,6 +219,7 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
 s = DO_UPCAST(VhostVDPAState, nc, nc);
 vdpa_device_fd = qemu_open(opts->vhostdev, O_RDWR);
 if (vdpa_device_fd == -1) {
+error_setg(errp, "Fail to open vhost-vdpa device %s", opts->vhostdev);
 return -errno;
 }
 s->vhost_vdpa.device_fd = vdpa_device_fd;
-- 
2.20.1




[PULL 02/18] hw: ehci: destroy sglist in error path

2020-08-31 Thread Gerd Hoffmann
From: Li Qiang 

This may cause resource leak.

Signed-off-by: Li Qiang 
Message-Id: <20200812161712.29361-1-liq...@163.com>
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ehci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 1495e8f7fab1..58cceacbf83a 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1445,6 +1445,7 @@ static int ehci_process_itd(EHCIState *ehci,
 dev = ehci_find_device(ehci, devaddr);
 if (dev == NULL) {
 ehci_trace_guest_bug(ehci, "no device found");
+qemu_sglist_destroy(&ehci->isgl);
 return -1;
 }
 pid = dir ? USB_TOKEN_IN : USB_TOKEN_OUT;
-- 
2.27.0




[PULL 03/18] hw: ehci: check return value of 'usb_packet_map'

2020-08-31 Thread Gerd Hoffmann
From: Li Qiang 

If 'usb_packet_map' fails, we should stop to process the usb
request.

Signed-off-by: Li Qiang 
Message-Id: <20200812161727.29412-1-liq...@163.com>
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ehci.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 58cceacbf83a..4da446d2de6b 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1373,7 +1373,10 @@ static int ehci_execute(EHCIPacket *p, const char 
*action)
 spd = (p->pid == USB_TOKEN_IN && NLPTR_TBIT(p->qtd.altnext) == 0);
 usb_packet_setup(&p->packet, p->pid, ep, 0, p->qtdaddr, spd,
  (p->qtd.token & QTD_TOKEN_IOC) != 0);
-usb_packet_map(&p->packet, &p->sgl);
+if (usb_packet_map(&p->packet, &p->sgl)) {
+qemu_sglist_destroy(&p->sgl);
+return -1;
+}
 p->async = EHCI_ASYNC_INITIALIZED;
 }
 
@@ -1453,7 +1456,10 @@ static int ehci_process_itd(EHCIState *ehci,
 if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) {
 usb_packet_setup(&ehci->ipacket, pid, ep, 0, addr, false,
  (itd->transact[i] & ITD_XACT_IOC) != 0);
-usb_packet_map(&ehci->ipacket, &ehci->isgl);
+if (usb_packet_map(&ehci->ipacket, &ehci->isgl)) {
+qemu_sglist_destroy(&ehci->isgl);
+return -1;
+}
 usb_handle_packet(dev, &ehci->ipacket);
 usb_packet_unmap(&ehci->ipacket, &ehci->isgl);
 } else {
-- 
2.27.0




[PULL 16/18] hw/usb: Add U2F device autoscan to passthru mode

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

This patch adds an autoscan to let u2f-passthru choose the first U2F
device it finds.

The autoscan is performed using libudev with an enumeration of all the
hidraw devices present on the host.

The first device which happens to be a U2F device is taken to do the
passtru.

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-13-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 docs/u2f.txt  |   9 
 hw/usb/u2f-passthru.c | 113 +-
 hw/usb/meson.build|   2 +-
 3 files changed, 110 insertions(+), 14 deletions(-)

diff --git a/docs/u2f.txt b/docs/u2f.txt
index f60052882ec3..8f44994818a2 100644
--- a/docs/u2f.txt
+++ b/docs/u2f.txt
@@ -42,6 +42,10 @@ on libu2f-emu: configuring and building:
 
 ./configure --enable-u2f && make
 
+The pass-through mode is built by default on Linux. To take advantage
+of the autoscan option it provides, make sure you have a working libudev
+installed on the host.
+
 
 3. Using u2f-emulated
 
@@ -90,6 +94,11 @@ On the host specify the u2f-passthru device with a suitable 
hidraw:
 
 qemu -usb -device u2f-passthru,hidraw=/dev/hidraw0
 
+Alternately, the u2f-passthru device can autoscan to take the first
+U2F device it finds on the host (this requires a working libudev):
+
+qemu -usb -device u2f-passthru
+
 
 5. Libu2f-emu
 
diff --git a/hw/usb/u2f-passthru.c b/hw/usb/u2f-passthru.c
index 74d4ae6e9294..e9c8aa459586 100644
--- a/hw/usb/u2f-passthru.c
+++ b/hw/usb/u2f-passthru.c
@@ -378,6 +378,84 @@ static bool u2f_passthru_is_u2f_device(int fd)
   sizeof(u2f_hid_report_desc_header)) == 0;
 }
 
+#ifdef CONFIG_LIBUDEV
+static int u2f_passthru_open_from_device(struct udev_device *device)
+{
+const char *devnode = udev_device_get_devnode(device);
+
+int fd = qemu_open(devnode, O_RDWR);
+if (fd < 0) {
+return -1;
+} else if (!u2f_passthru_is_u2f_device(fd)) {
+qemu_close(fd);
+return -1;
+}
+return fd;
+}
+
+static int u2f_passthru_open_from_enumerate(struct udev *udev,
+struct udev_enumerate *enumerate)
+{
+struct udev_list_entry *devices, *entry;
+int ret, fd;
+
+ret = udev_enumerate_scan_devices(enumerate);
+if (ret < 0) {
+return -1;
+}
+
+devices = udev_enumerate_get_list_entry(enumerate);
+udev_list_entry_foreach(entry, devices) {
+struct udev_device *device;
+const char *syspath = udev_list_entry_get_name(entry);
+
+if (syspath == NULL) {
+continue;
+}
+
+device = udev_device_new_from_syspath(udev, syspath);
+if (device == NULL) {
+continue;
+}
+
+fd = u2f_passthru_open_from_device(device);
+udev_device_unref(device);
+if (fd >= 0) {
+return fd;
+}
+}
+return -1;
+}
+
+static int u2f_passthru_open_from_scan(void)
+{
+struct udev *udev;
+struct udev_enumerate *enumerate;
+int ret, fd = -1;
+
+udev = udev_new();
+if (udev == NULL) {
+return -1;
+}
+
+enumerate = udev_enumerate_new(udev);
+if (enumerate == NULL) {
+udev_unref(udev);
+return -1;
+}
+
+ret = udev_enumerate_add_match_subsystem(enumerate, "hidraw");
+if (ret >= 0) {
+fd = u2f_passthru_open_from_enumerate(udev, enumerate);
+}
+
+udev_enumerate_unref(enumerate);
+udev_unref(udev);
+
+return fd;
+}
+#endif
+
 static void u2f_passthru_unrealize(U2FKeyState *base)
 {
 U2FPassthruState *key = PASSTHRU_U2F_KEY(base);
@@ -392,22 +470,31 @@ static void u2f_passthru_realize(U2FKeyState *base, Error 
**errp)
 int fd;
 
 if (key->hidraw == NULL) {
+#ifdef CONFIG_LIBUDEV
+fd = u2f_passthru_open_from_scan();
+if (fd < 0) {
+error_setg(errp, "%s: Failed to find a U2F USB device",
+   TYPE_U2F_PASSTHRU);
+return;
+}
+#else
 error_setg(errp, "%s: Missing hidraw", TYPE_U2F_PASSTHRU);
 return;
-}
+#endif
+} else {
+fd = qemu_open(key->hidraw, O_RDWR);
+if (fd < 0) {
+error_setg(errp, "%s: Failed to open %s", TYPE_U2F_PASSTHRU,
+   key->hidraw);
+return;
+}
 
-fd = qemu_open(key->hidraw, O_RDWR);
-if (fd < 0) {
-error_setg(errp, "%s: Failed to open %s", TYPE_U2F_PASSTHRU,
-   key->hidraw);
-return;
-}
-
-if (!u2f_passthru_is_u2f_device(fd)) {
-qemu_close(fd);
-error_setg(errp, "%s: Passed hidraw does not represent "
-   "a U2F HID device", TYPE_U2F_PASSTHRU);
-return;
+if (!u2f_passthru_is_u2f_device(fd)) {
+qemu_close(fd);
+error_setg(errp, "%s: Passed hidraw does not represent "
+   "a U2F HID device", TYPE_U2F_PASSTHRU);
+return;
+}
 }
 key

[PULL 07/18] hw/usb: Add U2F key base class

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

This patch adds the specification for the U2F key base class.
Used to group the common characteristics, this device class will be
inherited by its two variants, corresponding to the two modes:
passthrough and emulated

This prepares the U2F devices hierarchy which is as follow:
USB device -> u2f-key -> {u2f-passthru, u2f-emulated}.

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-4-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/u2f.h | 92 
 1 file changed, 92 insertions(+)
 create mode 100644 hw/usb/u2f.h

diff --git a/hw/usb/u2f.h b/hw/usb/u2f.h
new file mode 100644
index ..db30f3586bf7
--- /dev/null
+++ b/hw/usb/u2f.h
@@ -0,0 +1,92 @@
+/*
+ * U2F USB device.
+ *
+ * Copyright (c) 2020 César Belley 
+ * Written by César Belley 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef U2F_H
+#define U2F_H
+
+#include "hw/qdev-core.h"
+
+#define U2FHID_PACKET_SIZE 64
+#define U2FHID_PENDING_IN_NUM 32
+
+typedef struct U2FKeyState U2FKeyState;
+typedef struct U2FKeyInfo U2FKeyInfo;
+
+#define TYPE_U2F_KEY "u2f-key"
+#define U2F_KEY(obj) \
+OBJECT_CHECK(U2FKeyState, (obj), TYPE_U2F_KEY)
+#define U2F_KEY_CLASS(klass) \
+OBJECT_CLASS_CHECK(U2FKeyClass, (klass), TYPE_U2F_KEY)
+#define U2F_KEY_GET_CLASS(obj) \
+OBJECT_GET_CLASS(U2FKeyClass, (obj), TYPE_U2F_KEY)
+
+/*
+ * Callbacks to be used by the U2F key base device (i.e. hw/u2f.c)
+ * to interact with its variants (i.e. hw/u2f-*.c)
+ */
+typedef struct U2FKeyClass {
+/*< private >*/
+USBDeviceClass parent_class;
+
+/*< public >*/
+void (*recv_from_guest)(U2FKeyState *key,
+const uint8_t packet[U2FHID_PACKET_SIZE]);
+void (*realize)(U2FKeyState *key, Error **errp);
+void (*unrealize)(U2FKeyState *key);
+} U2FKeyClass;
+
+/*
+ * State of the U2F key base device (i.e. hw/u2f.c)
+ */
+typedef struct U2FKeyState {
+USBDevice dev;
+USBEndpoint *ep;
+uint8_t idle;
+
+/* Pending packets to be send to the guest */
+uint8_t pending_in[U2FHID_PENDING_IN_NUM][U2FHID_PACKET_SIZE];
+uint8_t pending_in_start;
+uint8_t pending_in_end;
+uint8_t pending_in_num;
+} U2FKeyState;
+
+/*
+ * API to be used by the U2F key device variants (i.e. hw/u2f-*.c)
+ * to interact with the the U2F key base device (i.e. hw/u2f.c)
+ */
+void u2f_send_to_guest(U2FKeyState *key,
+   const uint8_t packet[U2FHID_PACKET_SIZE]);
+
+extern const VMStateDescription vmstate_u2f_key;
+
+#define VMSTATE_U2F_KEY(_field, _state) {\
+.name   = (stringify(_field)),   \
+.size   = sizeof(U2FKeyState),   \
+.vmsd   = &vmstate_u2f_key,  \
+.flags  = VMS_STRUCT,\
+.offset = vmstate_offset_value(_state, _field, U2FKeyState), \
+}
+
+#endif /* U2F_H */
-- 
2.27.0




[PATCH 9/9] vhost-vdpa: allow pre-opend file descriptor

2020-08-31 Thread Jason Wang
This patch allows to initialize vhost-vdpa network backend with pre
opened vhost-vdpa file descriptor. This is useful for running
unprivileged qemu through libvirt.

Cc: Eric Blake 
Cc: Markus Armbruster 
Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 24 +++-
 qapi/net.json|  5 -
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 9a6f0b63d3..f6385cd264 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -206,20 +206,34 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
*name,
   (char *)name, errp)) {
 return -1;
 }
-if (!opts->has_vhostdev) {
-error_setg(errp, "vhost-vdpa requires vhostdev to be set");
+if (!(opts->has_vhostdev ^ opts->has_fd)) {
+error_setg(errp, "Vhost-vdpa requires either vhostdev or fd to be 
set");
 return -1;
 }
 
 assert(name);
 
 nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, TYPE_VHOST_VDPA, 
name);
-snprintf(nc->info_str, sizeof(nc->info_str), "vhostdev=%s", 
opts->vhostdev);
+if (opts->has_vhostdev) {
+snprintf(nc->info_str, sizeof(nc->info_str),
+ "vhostdev=%s", opts->vhostdev);
+vdpa_device_fd = qemu_open(opts->vhostdev, O_RDWR);
+if (vdpa_device_fd == -1) {
+error_setg(errp, "Fail to open vhost-vdpa device %s",
+   opts->vhostdev);
+return -errno;
+}
+} else {
+snprintf(nc->info_str, sizeof(nc->info_str), "fd=%s", opts->fd);
+vdpa_device_fd = monitor_fd_param(cur_mon, opts->fd, errp);
+if (vdpa_device_fd == -1) {
+return -1;
+}
+}
 
 s = DO_UPCAST(VhostVDPAState, nc, nc);
-vdpa_device_fd = qemu_open(opts->vhostdev, O_RDWR);
 if (vdpa_device_fd == -1) {
-error_setg(errp, "Fail to open vhost-vdpa device %s", opts->vhostdev);
+
 return -errno;
 }
 s->vhost_vdpa.device_fd = vdpa_device_fd;
diff --git a/qapi/net.json b/qapi/net.json
index a2a94fad3e..5ad60c3045 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -442,12 +442,15 @@
 # @queues: number of queues to be created for multiqueue vhost-vdpa
 #  (default: 1)
 #
+# @fd: file descriptor of an already opened vhost-vdpa (since 5.2)
+#
 # Since: 5.1
 ##
 { 'struct': 'NetdevVhostVDPAOptions',
   'data': {
 '*vhostdev': 'str',
-'*queues':   'int' } }
+'*queues':   'int',
+'*fd':   'str' } }
 
 ##
 # @NetClientDriver:
-- 
2.20.1




[PULL 18/18] usb: fix setup_len init (CVE-2020-14364)

2020-08-31 Thread Gerd Hoffmann
Store calculated setup_len in a local variable, verify it, and only
write it to the struct (USBDevice->setup_len) in case it passed the
sanity checks.

This prevents other code (do_token_{in,out} functions specifically)
from working with invalid USBDevice->setup_len values and overrunning
the USBDevice->setup_buf[] buffer.

Fixes: CVE-2020-14364
Signed-off-by: Gerd Hoffmann 
Tested-by: Gonglei 
Reviewed-by: Li Qiang 
Message-id: 20200825053636.29648-1-kra...@redhat.com
---
 hw/usb/core.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/usb/core.c b/hw/usb/core.c
index 5abd128b6bc5..5234dcc73fea 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -129,6 +129,7 @@ void usb_wakeup(USBEndpoint *ep, unsigned int stream)
 static void do_token_setup(USBDevice *s, USBPacket *p)
 {
 int request, value, index;
+unsigned int setup_len;
 
 if (p->iov.size != 8) {
 p->status = USB_RET_STALL;
@@ -138,14 +139,15 @@ static void do_token_setup(USBDevice *s, USBPacket *p)
 usb_packet_copy(p, s->setup_buf, p->iov.size);
 s->setup_index = 0;
 p->actual_length = 0;
-s->setup_len   = (s->setup_buf[7] << 8) | s->setup_buf[6];
-if (s->setup_len > sizeof(s->data_buf)) {
+setup_len = (s->setup_buf[7] << 8) | s->setup_buf[6];
+if (setup_len > sizeof(s->data_buf)) {
 fprintf(stderr,
 "usb_generic_handle_packet: ctrl buffer too small (%d > 
%zu)\n",
-s->setup_len, sizeof(s->data_buf));
+setup_len, sizeof(s->data_buf));
 p->status = USB_RET_STALL;
 return;
 }
+s->setup_len = setup_len;
 
 request = (s->setup_buf[0] << 8) | s->setup_buf[1];
 value   = (s->setup_buf[3] << 8) | s->setup_buf[2];
@@ -259,26 +261,28 @@ static void do_token_out(USBDevice *s, USBPacket *p)
 static void do_parameter(USBDevice *s, USBPacket *p)
 {
 int i, request, value, index;
+unsigned int setup_len;
 
 for (i = 0; i < 8; i++) {
 s->setup_buf[i] = p->parameter >> (i*8);
 }
 
 s->setup_state = SETUP_STATE_PARAM;
-s->setup_len   = (s->setup_buf[7] << 8) | s->setup_buf[6];
 s->setup_index = 0;
 
 request = (s->setup_buf[0] << 8) | s->setup_buf[1];
 value   = (s->setup_buf[3] << 8) | s->setup_buf[2];
 index   = (s->setup_buf[5] << 8) | s->setup_buf[4];
 
-if (s->setup_len > sizeof(s->data_buf)) {
+setup_len = (s->setup_buf[7] << 8) | s->setup_buf[6];
+if (setup_len > sizeof(s->data_buf)) {
 fprintf(stderr,
 "usb_generic_handle_packet: ctrl buffer too small (%d > 
%zu)\n",
-s->setup_len, sizeof(s->data_buf));
+setup_len, sizeof(s->data_buf));
 p->status = USB_RET_STALL;
 return;
 }
+s->setup_len = setup_len;
 
 if (p->pid == USB_TOKEN_OUT) {
 usb_packet_copy(p, s->data_buf, s->setup_len);
-- 
2.27.0




[PULL 17/18] usb-host: workaround libusb bug

2020-08-31 Thread Gerd Hoffmann
libusb_get_device_speed() does not work for
libusb_wrap_sys_device() devices in v1.0.23.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1871090
Signed-off-by: Gerd Hoffmann 
Message-id: 20200824110057.32089-1-kra...@redhat.com
---
 hw/usb/host-libusb.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index c474551d8456..08604f787fdf 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -39,6 +39,11 @@
 #endif
 #include 
 
+#ifdef CONFIG_LINUX
+#include 
+#include 
+#endif
+
 #include "qapi/error.h"
 #include "migration/vmstate.h"
 #include "monitor/monitor.h"
@@ -885,6 +890,7 @@ static void usb_host_ep_update(USBHostDevice *s)
 static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd)
 {
 USBDevice *udev = USB_DEVICE(s);
+int libusb_speed;
 int bus_num = 0;
 int addr = 0;
 int rc;
@@ -935,7 +941,36 @@ static int usb_host_open(USBHostDevice *s, libusb_device 
*dev, int hostfd)
 usb_ep_init(udev);
 usb_host_ep_update(s);
 
-udev->speed = speed_map[libusb_get_device_speed(dev)];
+libusb_speed = libusb_get_device_speed(dev);
+#ifdef CONFIG_LINUX
+if (hostfd && libusb_speed == 0) {
+/*
+ * Workaround libusb bug: libusb_get_device_speed() does not
+ * work for libusb_wrap_sys_device() devices in v1.0.23.
+ *
+ * Speeds are defined in linux/usb/ch9.h, file not included
+ * due to name conflicts.
+ */
+int rc = ioctl(hostfd, USBDEVFS_GET_SPEED, NULL);
+switch (rc) {
+case 1: /* low */
+libusb_speed = LIBUSB_SPEED_LOW;
+break;
+case 2: /* full */
+libusb_speed = LIBUSB_SPEED_FULL;
+break;
+case 3: /* high */
+case 4: /* wireless */
+libusb_speed = LIBUSB_SPEED_HIGH;
+break;
+case 5: /* super */
+case 6: /* super plus */
+libusb_speed = LIBUSB_SPEED_SUPER;
+break;
+}
+}
+#endif
+udev->speed = speed_map[libusb_speed];
 usb_host_speed_compat(s);
 
 if (s->ddesc.iProduct) {
-- 
2.27.0




[PULL 10/18] hw/usb: Add U2F key emulated mode

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

This patch adds the U2F key emulated mode.

The emulated mode consists of completely emulating the behavior of a
U2F device through software part. Libu2f-emu is used for that.

The emulated mode is associated with a device inheriting from
u2f-key base.

To work, an emulated U2F device must have differents elements which
can be given in different ways. This is detailed in docs/u2f.txt.

The Ephemeral one is the simplest way to configure, it lets the device
generate all the elements it needs for a single use of the lifetime
of the device:

qemu -usb -device u2f-emulated

For more information about libu2f-emu see this page:
https://github.com/MattGorko/libu2f-emu.

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-7-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/u2f-emulated.c | 405 ++
 1 file changed, 405 insertions(+)
 create mode 100644 hw/usb/u2f-emulated.c

diff --git a/hw/usb/u2f-emulated.c b/hw/usb/u2f-emulated.c
new file mode 100644
index ..9e1b829f3d32
--- /dev/null
+++ b/hw/usb/u2f-emulated.c
@@ -0,0 +1,405 @@
+/*
+ * U2F USB Emulated device.
+ *
+ * Copyright (c) 2020 César Belley 
+ * Written by César Belley 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/thread.h"
+#include "qemu/main-loop.h"
+#include "qapi/error.h"
+#include "hw/usb.h"
+#include "hw/qdev-properties.h"
+
+#include 
+
+#include "u2f.h"
+
+/* Counter which sync with a file */
+struct synced_counter {
+/* Emulated device counter */
+struct u2f_emu_vdev_counter vdev_counter;
+
+/* Private attributes */
+uint32_t value;
+FILE *fp;
+};
+
+static void counter_increment(struct u2f_emu_vdev_counter *vdev_counter)
+{
+struct synced_counter *counter = (struct synced_counter *)vdev_counter;
+++counter->value;
+
+/* Write back */
+if (fseek(counter->fp, 0, SEEK_SET) == -1) {
+return;
+}
+fprintf(counter->fp, "%u\n", counter->value);
+}
+
+static uint32_t counter_read(struct u2f_emu_vdev_counter *vdev_counter)
+{
+struct synced_counter *counter = (struct synced_counter *)vdev_counter;
+return counter->value;
+}
+
+typedef struct U2FEmulatedState U2FEmulatedState;
+
+#define PENDING_OUT_NUM 32
+
+struct U2FEmulatedState {
+U2FKeyState base;
+
+/* U2F virtual emulated device */
+u2f_emu_vdev *vdev;
+QemuMutex vdev_mutex;
+
+/* Properties */
+char *dir;
+char *cert;
+char *privkey;
+char *entropy;
+char *counter;
+struct synced_counter synced_counter;
+
+/* Pending packets received from the guest */
+uint8_t pending_out[PENDING_OUT_NUM][U2FHID_PACKET_SIZE];
+uint8_t pending_out_start;
+uint8_t pending_out_end;
+uint8_t pending_out_num;
+QemuMutex pending_out_mutex;
+
+/* Emulation thread and sync */
+QemuCond key_cond;
+QemuMutex key_mutex;
+QemuThread key_thread;
+bool stop_thread;
+EventNotifier notifier;
+};
+
+#define TYPE_U2F_EMULATED "u2f-emulated"
+#define EMULATED_U2F_KEY(obj) \
+OBJECT_CHECK(U2FEmulatedState, (obj), TYPE_U2F_EMULATED)
+
+static void u2f_emulated_reset(U2FEmulatedState *key)
+{
+key->pending_out_start = 0;
+key->pending_out_end = 0;
+key->pending_out_num = 0;
+}
+
+static void u2f_pending_out_add(U2FEmulatedState *key,
+const uint8_t packet[U2FHID_PACKET_SIZE])
+{
+int index;
+
+if (key->pending_out_num >= PENDING_OUT_NUM) {
+return;
+}
+
+index = key->pending_out_end;
+key->pending_out_end = (index + 1) % PENDING_OUT_NUM;
+++key->pending_out_num;
+
+memcpy(&key->pending_out[index], packet, U2FHID_PACKET_SIZE);
+}
+
+static uint8_t *u2f_pending_out_get(U2FEmulatedState *key)
+{
+int index;
+
+if (key->pending_out_num == 0) {
+return NULL;
+}
+
+index  = key-

Re: [PULL 00/18] Usb 20200828 patches

2020-08-31 Thread Gerd Hoffmann
On Fri, Aug 28, 2020 at 04:33:04PM +0200, Paolo Bonzini wrote:
> Il ven 28 ago 2020, 16:14 Peter Maydell  ha
> scritto:
> 
> > Why is Meson trying to use CMake here? I don't think we want
> > to bring in another build tool dependency...
> >
> 
> It's asking cmake if it knows about the package, if pkg-config fails. This
> is because cmake has its own incompatible mechanism to tell users about
> compilation and linking flags, and the "dependency" line doesn't tell Meson
> if u2f-emu has a pkg-config or a cmake description.

It uses pkg-config, so we can explicitly say so.

> We can ask to use pkg-config only since none of our dependencies are
> cmake-only; see https://mesonbuild.com/Dependencies.html#dependency-method.

I'll send a new pull with that addressed (also the other 11/18 comments).

thanks,
  Gerd




[PULL 15/18] hw/usb: Add U2F device check to passthru mode

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

This patchs adds a check to verify that the device passed through the
hidraw property is a U2F device.

The check is done by ensuring that the first values of the report
descriptor (USAGE PAGE and USAGE) correspond to those of a U2F device.

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-12-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/u2f-passthru.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/hw/usb/u2f-passthru.c b/hw/usb/u2f-passthru.c
index 52b44670775d..74d4ae6e9294 100644
--- a/hw/usb/u2f-passthru.c
+++ b/hw/usb/u2f-passthru.c
@@ -34,6 +34,12 @@
 
 #include "u2f.h"
 
+#ifdef CONFIG_LIBUDEV
+#include 
+#endif
+#include 
+#include 
+
 #define NONCE_SIZE 8
 #define BROADCAST_CID 0x
 #define TRANSACTION_TIMEOUT 12
@@ -344,6 +350,34 @@ static void u2f_passthru_recv_from_guest(U2FKeyState *base,
 }
 }
 
+static bool u2f_passthru_is_u2f_device(int fd)
+{
+int ret, rdesc_size;
+struct hidraw_report_descriptor rdesc;
+const uint8_t u2f_hid_report_desc_header[] = {
+0x06, 0xd0, 0xf1, /* Usage Page (FIDO) */
+0x09, 0x01,   /* Usage (FIDO) */
+};
+
+/* Get report descriptor size */
+ret = ioctl(fd, HIDIOCGRDESCSIZE, &rdesc_size);
+if (ret < 0 || rdesc_size < sizeof(u2f_hid_report_desc_header)) {
+return false;
+}
+
+/* Get report descriptor */
+memset(&rdesc, 0x0, sizeof(rdesc));
+rdesc.size = rdesc_size;
+ret = ioctl(fd, HIDIOCGRDESC, &rdesc);
+if (ret < 0) {
+return false;
+}
+
+/* Header bytes cover specific U2F rdesc values */
+return memcmp(u2f_hid_report_desc_header, rdesc.value,
+  sizeof(u2f_hid_report_desc_header)) == 0;
+}
+
 static void u2f_passthru_unrealize(U2FKeyState *base)
 {
 U2FPassthruState *key = PASSTHRU_U2F_KEY(base);
@@ -368,6 +402,13 @@ static void u2f_passthru_realize(U2FKeyState *base, Error 
**errp)
key->hidraw);
 return;
 }
+
+if (!u2f_passthru_is_u2f_device(fd)) {
+qemu_close(fd);
+error_setg(errp, "%s: Passed hidraw does not represent "
+   "a U2F HID device", TYPE_U2F_PASSTHRU);
+return;
+}
 key->hidraw_fd = fd;
 u2f_passthru_reset(key);
 }
-- 
2.27.0




[PULL 13/18] docs/qdev-device-use.txt: Add USB U2F key to the QDEV devices examples

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-10-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 docs/qdev-device-use.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index f8d0d2fe297a..9889521e3c07 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -325,6 +325,7 @@ The new way is -device DEVNAME,DEV-OPTS...  Details depend 
on DRIVER:
 * mouse   -device usb-mouse
 * tablet  -device usb-tablet
 * wacom-tablet-device usb-wacom-tablet
+* u2f -device u2f-{emulated,passthru}
 * braille See "Character Devices"
 
 === Watchdog Devices ===
-- 
2.27.0




[PULL 01/18] hw: xhci: check return value of 'usb_packet_map'

2020-08-31 Thread Gerd Hoffmann
From: Li Qiang 

Currently we don't check the return value of 'usb_packet_map',
this will cause an UAF issue. This is LP#1891341.
Following is the reproducer provided in:
-->https://bugs.launchpad.net/qemu/+bug/1891341

cat << EOF | ./i386-softmmu/qemu-system-i386 -device nec-usb-xhci \
-trace usb\* -device usb-audio -device usb-storage,drive=mydrive \
-drive id=mydrive,file=null-co://,size=2M,format=raw,if=none \
-nodefaults -nographic -qtest stdio
outl 0xcf8 0x80001016
outl 0xcfc 0x3c009f0d
outl 0xcf8 0x80001004
outl 0xcfc 0xc77695e
writel 0x9f0d0040 0x3655
writeq 0x9f0d2000 0xff2f9e00
write 0x1d 0x1 0x27
write 0x2d 0x1 0x2e
write 0x17232 0x1 0x03
write 0x17254 0x1 0x06
write 0x17278 0x1 0x34
write 0x3d 0x1 0x27
write 0x40 0x1 0x2e
write 0x41 0x1 0x72
write 0x42 0x1 0x01
write 0x4d 0x1 0x2e
write 0x4f 0x1 0x01
writeq 0x9f0d2000 0x5c051a01
write 0x34001d 0x1 0x13
write 0x340026 0x1 0x30
write 0x340028 0x1 0x08
write 0x34002c 0x1 0xfe
write 0x34002d 0x1 0x08
write 0x340037 0x1 0x5e
write 0x34003a 0x1 0x05
write 0x34003d 0x1 0x05
write 0x34004d 0x1 0x13
writeq 0x9f0d2000 0xff0001010049
EOF

This patch fixes this.

Buglink: https://bugs.launchpad.net/qemu/+bug/1891341
Reported-by: Alexander Bulekov 
Signed-off-by: Li Qiang 
Message-id: 20200812153139.15146-1-liq...@163.com
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-xhci.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 67a18fe2b64c..46a2186d912a 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1615,7 +1615,10 @@ static int xhci_setup_packet(XHCITransfer *xfer)
 xhci_xfer_create_sgl(xfer, dir == USB_TOKEN_IN); /* Also sets int_req */
 usb_packet_setup(&xfer->packet, dir, ep, xfer->streamid,
  xfer->trbs[0].addr, false, xfer->int_req);
-usb_packet_map(&xfer->packet, &xfer->sgl);
+if (usb_packet_map(&xfer->packet, &xfer->sgl)) {
+qemu_sglist_destroy(&xfer->sgl);
+return -1;
+}
 DPRINTF("xhci: setup packet pid 0x%x addr %d ep %d\n",
 xfer->packet.pid, ep->dev->addr, ep->nr);
 return 0;
-- 
2.27.0




[PULL 11/18] meson: Add U2F key to meson

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-8-cesar.bel...@lse.epita.fr

[ fixes suggested by paolo ]

Signed-off-by: Gerd Hoffmann 
---
 configure  | 8 +++-
 meson_options.txt  | 1 +
 hw/usb/Kconfig | 5 +
 hw/usb/meson.build | 7 +++
 meson.build| 7 +++
 5 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 6ecaff429b1b..8dc981684bb9 100755
--- a/configure
+++ b/configure
@@ -495,6 +495,7 @@ trace_file="trace"
 spice=""
 rbd=""
 smartcard=""
+u2f="auto"
 libusb=""
 usb_redir=""
 opengl=""
@@ -1415,6 +1416,10 @@ for opt do
   ;;
   --enable-smartcard) smartcard="yes"
   ;;
+  --disable-u2f) u2f="disabled"
+  ;;
+  --enable-u2f) u2f="enabled"
+  ;;
   --disable-libusb) libusb="no"
   ;;
   --enable-libusb) libusb="yes"
@@ -1945,6 +1950,7 @@ disabled with --disable-FEATURE, default is enabled if 
available:
   libiscsiiscsi support
   libnfs  nfs support
   smartcard   smartcard support (libcacard)
+  u2f U2F support (u2f-emu)
   libusb  libusb (for usb passthrough)
   live-block-migration   Block migration in the main migration stream
   usb-redir   usb network redirection support
@@ -8229,7 +8235,7 @@ NINJA=${ninja:-$PWD/ninjatool} $meson setup \
 -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo false; 
fi) \
-Dsdl=$sdl -Dsdl_image=$sdl_image \
-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png 
\
-   -Dgettext=$gettext -Dxkbcommon=$xkbcommon \
+   -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f\
 $cross_arg \
 "$PWD" "$source_path"
 
diff --git a/meson_options.txt b/meson_options.txt
index c55f9cd94cb2..aef2de652332 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,6 +1,7 @@
 option('gettext', type : 'boolean', value : true)
 option('sdl', type : 'feature', value : 'auto')
 option('sdl_image', type : 'feature', value : 'auto')
+option('u2f', type : 'feature', value : 'auto')
 option('vnc', type : 'feature', value : 'enabled')
 option('vnc_jpeg', type : 'feature', value : 'auto')
 option('vnc_png', type : 'feature', value : 'auto')
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 5e63dc75f815..3fc8fbe3c74c 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -96,6 +96,11 @@ config USB_STORAGE_MTP
 default y
 depends on USB
 
+config USB_U2F
+bool
+default y
+depends on USB
+
 config IMX_USBPHY
 bool
 default y
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 3c44a1b06954..a25109b88c91 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -50,6 +50,13 @@ if config_host.has_key('CONFIG_SMARTCARD')
   hw_usb_modules += {'smartcard': usbsmartcard_ss}
 endif
 
+# U2F
+softmmu_ss.add(when: 'CONFIG_USB_U2F', if_true: files('u2f.c'))
+softmmu_ss.add(when: ['CONFIG_LINUX', 'CONFIG_USB_U2F'], if_true: 
files('u2f-passthru.c'))
+if u2f.found()
+  softmmu_ss.add(when: 'CONFIG_USB_U2F', if_true: [u2f, 
files('u2f-emulated.c')])
+endif
+
 # usb redirect
 if config_host.has_key('CONFIG_USB_REDIR')
   usbredir_ss = ss.source_set()
diff --git a/meson.build b/meson.build
index 74f8ea0c2e11..1e7aee85e33a 100644
--- a/meson.build
+++ b/meson.build
@@ -377,6 +377,12 @@ if 'CONFIG_SMARTCARD' in config_host
   cacard = declare_dependency(compile_args: 
config_host['SMARTCARD_CFLAGS'].split(),
   link_args: config_host['SMARTCARD_LIBS'].split())
 endif
+u2f = not_found
+if have_system
+  u2f = dependency('u2f-emu', required: get_option('u2f'),
+   method: 'pkg-config',
+   static: enable_static)
+endif
 usbredir = not_found
 if 'CONFIG_USB_REDIR' in config_host
   usbredir = declare_dependency(compile_args: 
config_host['USB_REDIR_CFLAGS'].split(),
@@ -1375,6 +1381,7 @@ summary_info += {'spice support': 
config_host.has_key('CONFIG_SPICE')}
 summary_info += {'rbd support':   config_host.has_key('CONFIG_RBD')}
 summary_info += {'xfsctl support':config_host.has_key('CONFIG_XFS')}
 summary_info += {'smartcard support': config_host.has_key('CONFIG_SMARTCARD')}
+summary_info += {'U2F support':   u2f.found()}
 summary_info += {'libusb':config_host.has_key('CONFIG_USB_LIBUSB')}
 summary_info += {'usb net redir': config_host.has_key('CONFIG_USB_REDIR')}
 summary_info += {'OpenGL support':config_host.has_key('CONFIG_OPENGL')}
-- 
2.27.0




Re: [PATCH] block: always link with zlib

2020-08-31 Thread Thomas Huth
On 28/08/2020 19.32, Paolo Bonzini wrote:
> The qcow2 driver needs the zlib dependency.  While emulators
> provided it through the migration code, this is not true of
> the tools.  Move the dependency from the qcow1 rule directly
> into block_ss so that it is included unconditionally.
> 
> Fixes build with --disable-qcow1.
> 
> Reported-by: Thomas Huth 
> Cc: qemu-bl...@nongnu.org
> Signed-off-by: Paolo Bonzini 
> ---
>  block/meson.build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/meson.build b/block/meson.build
> index 4dbbfe60b4..a3e56b7cd1 100644
> --- a/block/meson.build
> +++ b/block/meson.build
> @@ -40,9 +40,9 @@ block_ss.add(files(
>'vmdk.c',
>'vpc.c',
>'write-threshold.c',
> -), zstd)
> +), zstd, zlib)
>  
> -block_ss.add(when: [zlib, 'CONFIG_QCOW1'], if_true: files('qcow.c'))
> +block_ss.add(when: 'CONFIG_QCOW1', if_true: files('qcow.c'))
>  block_ss.add(when: 'CONFIG_VDI', if_true: files('vdi.c'))
>  block_ss.add(when: 'CONFIG_CLOOP', if_true: files('cloop.c'))
>  block_ss.add(when: 'CONFIG_BOCHS', if_true: files('bochs.c'))

Reviewed-by: Thomas Huth 




[PULL 12/18] docs/system: Add U2F key to the USB devices examples

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-9-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 docs/system/usb.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/system/usb.rst b/docs/system/usb.rst
index ddfa828d74ae..9a2f1927c451 100644
--- a/docs/system/usb.rst
+++ b/docs/system/usb.rst
@@ -81,6 +81,9 @@ option or the ``device_add`` monitor command. Available 
devices are:
 ``usb-audio``
USB audio device
 
+``u2f-{emulated,passthru}``
+   Universal Second Factor device
+
 .. _host_005fusb_005fdevices:
 
 Using host USB devices on a Linux host
-- 
2.27.0




[PULL 00/18] Usb 20200831 patches

2020-08-31 Thread Gerd Hoffmann
The following changes since commit 39335fab59e11cfda9b7cf63929825db2dd3a3e0:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.2-pull-=
request' into staging (2020-08-28 22:30:11 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/usb-20200831-pull-request

for you to fetch changes up to b946434f2659a182afc17e155be6791ebfb302eb:

  usb: fix setup_len init (CVE-2020-14364) (2020-08-31 08:23:39 +0200)


usb: usb_packet_map fixes for ehci and xhci.
usb: setup_len fix (CVE-2020-14364).
usb: u2f key support (GSoC).
 * v2: 32bit build fixed.
 * v3: libu2f-emu dependency fixed.



C=C3=A9sar Belley (12):
  hw/usb: Regroup USB HID protocol values
  docs: Add USB U2F key device documentation
  hw/usb: Add U2F key base class
  hw/usb: Add U2F key base class implementation
  hw/usb: Add U2F key passthru mode
  hw/usb: Add U2F key emulated mode
  meson: Add U2F key to meson
  docs/system: Add U2F key to the USB devices examples
  docs/qdev-device-use.txt: Add USB U2F key to the QDEV devices examples
  scripts: Add u2f-setup-gen script
  hw/usb: Add U2F device check to passthru mode
  hw/usb: Add U2F device autoscan to passthru mode

Gerd Hoffmann (3):
  ehci: drop pointless warn_report for guest bugs.
  usb-host: workaround libusb bug
  usb: fix setup_len init (CVE-2020-14364)

Li Qiang (3):
  hw: xhci: check return value of 'usb_packet_map'
  hw: ehci: destroy sglist in error path
  hw: ehci: check return value of 'usb_packet_map'

 configure|   8 +-
 docs/qdev-device-use.txt |   1 +
 docs/u2f.txt | 110 
 meson_options.txt|   1 +
 scripts/u2f-setup-gen.py | 170 
 hw/usb/u2f.h |  92 +++
 include/hw/usb/hid.h |  17 ++
 hw/usb/core.c|  16 +-
 hw/usb/dev-hid.c |  26 +-
 hw/usb/dev-wacom.c   |  12 +-
 hw/usb/hcd-ehci.c|  12 +-
 hw/usb/hcd-xhci.c|   5 +-
 hw/usb/host-libusb.c |  37 ++-
 hw/usb/u2f-emulated.c| 405 
 hw/usb/u2f-passthru.c| 551 +++
 hw/usb/u2f.c | 352 +
 docs/system/usb.rst  |   3 +
 hw/usb/Kconfig   |   5 +
 hw/usb/meson.build   |   7 +
 meson.build  |   7 +
 20 files changed, 1797 insertions(+), 40 deletions(-)
 create mode 100644 docs/u2f.txt
 create mode 100755 scripts/u2f-setup-gen.py
 create mode 100644 hw/usb/u2f.h
 create mode 100644 include/hw/usb/hid.h
 create mode 100644 hw/usb/u2f-emulated.c
 create mode 100644 hw/usb/u2f-passthru.c
 create mode 100644 hw/usb/u2f.c

--=20
2.27.0





Re: [PATCH v2 05/10] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

2020-08-31 Thread Gerd Hoffmann
On Mon, Aug 31, 2020 at 09:43:10AM -0400, Pan Nengyuan wrote:
> Receiving error in local variable err, and forgot to free it.
> This patch check the return value of 'gdk_window_create_gl_context'
> and 'gdk_gl_context_realize', then free err to fix it.
> 
> Reported-by: Euler Robot 
> Signed-off-by: Pan Nengyuan 
> ---
> Cc: Gerd Hoffmann 

Added to UI queue.

thanks,
  Gerd




[PULL 05/18] hw/usb: Regroup USB HID protocol values

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

Group some HID values that are used pretty much everywhere when
dealing with HID devices.

Signed-off-by: César Belley 
Message-id: 20200812094135.20550-2-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 include/hw/usb/hid.h | 17 +
 hw/usb/dev-hid.c | 26 +++---
 hw/usb/dev-wacom.c   | 12 +++-
 3 files changed, 27 insertions(+), 28 deletions(-)
 create mode 100644 include/hw/usb/hid.h

diff --git a/include/hw/usb/hid.h b/include/hw/usb/hid.h
new file mode 100644
index ..1c142584ffab
--- /dev/null
+++ b/include/hw/usb/hid.h
@@ -0,0 +1,17 @@
+#ifndef HW_USB_HID_H
+#define HW_USB_HID_H
+
+/* HID interface requests */
+#define HID_GET_REPORT   0xa101
+#define HID_GET_IDLE 0xa102
+#define HID_GET_PROTOCOL 0xa103
+#define HID_SET_REPORT   0x2109
+#define HID_SET_IDLE 0x210a
+#define HID_SET_PROTOCOL 0x210b
+
+/* HID descriptor types */
+#define USB_DT_HID0x21
+#define USB_DT_REPORT 0x22
+#define USB_DT_PHY0x23
+
+#endif
diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 89f63b698b8a..c73f7b2fe2c5 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -32,21 +32,9 @@
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "hw/input/hid.h"
+#include "hw/usb/hid.h"
 #include "hw/qdev-properties.h"
 
-/* HID interface requests */
-#define GET_REPORT   0xa101
-#define GET_IDLE 0xa102
-#define GET_PROTOCOL 0xa103
-#define SET_REPORT   0x2109
-#define SET_IDLE 0x210a
-#define SET_PROTOCOL 0x210b
-
-/* HID descriptor types */
-#define USB_DT_HID0x21
-#define USB_DT_REPORT 0x22
-#define USB_DT_PHY0x23
-
 typedef struct USBHIDState {
 USBDevice dev;
 USBEndpoint *intr;
@@ -618,38 +606,38 @@ static void usb_hid_handle_control(USBDevice *dev, 
USBPacket *p,
 goto fail;
 }
 break;
-case GET_REPORT:
+case HID_GET_REPORT:
 if (hs->kind == HID_MOUSE || hs->kind == HID_TABLET) {
 p->actual_length = hid_pointer_poll(hs, data, length);
 } else if (hs->kind == HID_KEYBOARD) {
 p->actual_length = hid_keyboard_poll(hs, data, length);
 }
 break;
-case SET_REPORT:
+case HID_SET_REPORT:
 if (hs->kind == HID_KEYBOARD) {
 p->actual_length = hid_keyboard_write(hs, data, length);
 } else {
 goto fail;
 }
 break;
-case GET_PROTOCOL:
+case HID_GET_PROTOCOL:
 if (hs->kind != HID_KEYBOARD && hs->kind != HID_MOUSE) {
 goto fail;
 }
 data[0] = hs->protocol;
 p->actual_length = 1;
 break;
-case SET_PROTOCOL:
+case HID_SET_PROTOCOL:
 if (hs->kind != HID_KEYBOARD && hs->kind != HID_MOUSE) {
 goto fail;
 }
 hs->protocol = value;
 break;
-case GET_IDLE:
+case HID_GET_IDLE:
 data[0] = hs->idle;
 p->actual_length = 1;
 break;
-case SET_IDLE:
+case HID_SET_IDLE:
 hs->idle = (uint8_t) (value >> 8);
 hid_set_next_idle(hs);
 if (hs->kind == HID_MOUSE || hs->kind == HID_TABLET) {
diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
index 8aba44b8bc3d..76fc5a5dabf3 100644
--- a/hw/usb/dev-wacom.c
+++ b/hw/usb/dev-wacom.c
@@ -29,6 +29,7 @@
 #include "qemu/osdep.h"
 #include "ui/console.h"
 #include "hw/usb.h"
+#include "hw/usb/hid.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "desc.h"
@@ -37,13 +38,6 @@
 #define WACOM_GET_REPORT   0x2101
 #define WACOM_SET_REPORT   0x2109
 
-/* HID interface requests */
-#define HID_GET_REPORT 0xa101
-#define HID_GET_IDLE   0xa102
-#define HID_GET_PROTOCOL   0xa103
-#define HID_SET_IDLE   0x210a
-#define HID_SET_PROTOCOL   0x210b
-
 typedef struct USBWacomState {
 USBDevice dev;
 USBEndpoint *intr;
@@ -86,11 +80,11 @@ static const USBDescIface desc_iface_wacom = {
 /* HID descriptor */
 .data = (uint8_t[]) {
 0x09,  /*  u8  bLength */
-0x21,  /*  u8  bDescriptorType */
+USB_DT_HID,/*  u8  bDescriptorType */
 0x01, 0x10,/*  u16 HID_class */
 0x00,  /*  u8  country_code */
 0x01,  /*  u8  num_descriptors */
-0x22,  /*  u8  type: Report */
+USB_DT_REPORT, /*  u8  type: Report */
 0x6e, 0,   /*  u16 len */
 },
 },
-- 
2.27.0




[PULL 09/18] hw/usb: Add U2F key passthru mode

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

This patch adds the U2F key pass-through mode.

The pass-through mode consists of passing all requests made from the
guest to the physical security key connected to the host machine and
vice versa.

In addition, the dedicated pass-through allows to have a U2F security key
shared on several guests which is not possible with a simple host device
assignment pass-through.

The pass-through mode is associated with a device inheriting from
u2f-key base.

To work, it needs the path to a U2F hidraw, obtained from the Qemu
command line, and passed by the user:

qemu -usb -device u2f-passthru,hidraw=/dev/hidrawX

Autoscan and U2F compatibility checking features are given at the end
of the patch series.

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-6-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/u2f-passthru.c | 423 ++
 1 file changed, 423 insertions(+)
 create mode 100644 hw/usb/u2f-passthru.c

diff --git a/hw/usb/u2f-passthru.c b/hw/usb/u2f-passthru.c
new file mode 100644
index ..52b44670775d
--- /dev/null
+++ b/hw/usb/u2f-passthru.c
@@ -0,0 +1,423 @@
+/*
+ * U2F USB Passthru device.
+ *
+ * Copyright (c) 2020 César Belley 
+ * Written by César Belley 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/main-loop.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/usb.h"
+#include "migration/vmstate.h"
+
+#include "u2f.h"
+
+#define NONCE_SIZE 8
+#define BROADCAST_CID 0x
+#define TRANSACTION_TIMEOUT 12
+
+struct transaction {
+uint32_t cid;
+uint16_t resp_bcnt;
+uint16_t resp_size;
+
+/* Nonce for broadcast isolation */
+uint8_t nonce[NONCE_SIZE];
+};
+
+typedef struct U2FPassthruState U2FPassthruState;
+
+#define CURRENT_TRANSACTIONS_NUM 4
+
+struct U2FPassthruState {
+U2FKeyState base;
+
+/* Host device */
+char *hidraw;
+int hidraw_fd;
+
+/* Current Transactions */
+struct transaction current_transactions[CURRENT_TRANSACTIONS_NUM];
+uint8_t current_transactions_start;
+uint8_t current_transactions_end;
+uint8_t current_transactions_num;
+
+/* Transaction time checking */
+int64_t last_transaction_time;
+QEMUTimer timer;
+};
+
+#define TYPE_U2F_PASSTHRU "u2f-passthru"
+#define PASSTHRU_U2F_KEY(obj) \
+OBJECT_CHECK(U2FPassthruState, (obj), TYPE_U2F_PASSTHRU)
+
+/* Init packet sizes */
+#define PACKET_INIT_HEADER_SIZE 7
+#define PACKET_INIT_DATA_SIZE (U2FHID_PACKET_SIZE - PACKET_INIT_HEADER_SIZE)
+
+/* Cont packet sizes */
+#define PACKET_CONT_HEADER_SIZE 5
+#define PACKET_CONT_DATA_SIZE (U2FHID_PACKET_SIZE - PACKET_CONT_HEADER_SIZE)
+
+struct packet_init {
+uint32_t cid;
+uint8_t cmd;
+uint8_t bcnth;
+uint8_t bcntl;
+uint8_t data[PACKET_INIT_DATA_SIZE];
+} QEMU_PACKED;
+
+static inline uint32_t packet_get_cid(const void *packet)
+{
+return *((uint32_t *)packet);
+}
+
+static inline bool packet_is_init(const void *packet)
+{
+return ((uint8_t *)packet)[4] & (1 << 7);
+}
+
+static inline uint16_t packet_init_get_bcnt(
+const struct packet_init *packet_init)
+{
+uint16_t bcnt = 0;
+bcnt |= packet_init->bcnth << 8;
+bcnt |= packet_init->bcntl;
+
+return bcnt;
+}
+
+static void u2f_passthru_reset(U2FPassthruState *key)
+{
+timer_del(&key->timer);
+qemu_set_fd_handler(key->hidraw_fd, NULL, NULL, key);
+key->last_transaction_time = 0;
+key->current_transactions_start = 0;
+key->current_transactions_end = 0;
+key->current_transactions_num = 0;
+}
+
+static void u2f_timeout_check(void *opaque)
+{
+U2FPassthruState *key = opaque;
+int64_t time = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
+
+if (time > key->last_transaction_time + TRANSACTION_TIMEOUT) {
+u2f_passthru_reset(key);
+} els

Re: [PATCH v2 2/7] vhost: check queue state in the vhost_dev_set_log routine

2020-08-31 Thread Dima Stepanov
On Thu, Aug 27, 2020 at 09:46:03PM -0400, Raphael Norwitz wrote:
> On Mon, Aug 24, 2020 at 4:41 AM Dima Stepanov  wrote:
> >
> > If the vhost-user-blk daemon provides only one virtqueue, but device was
> > added with several queues, then QEMU will send more VHOST-USER command
> > than expected by daemon side. The vhost_virtqueue_start() routine
> > handles such case by checking the return value from the
> > virtio_queue_get_desc_addr() function call. Add the same check to the
> > vhost_dev_set_log() routine.
> >
> > Signed-off-by: Dima Stepanov 
> > ---
> >  hw/virtio/vhost.c | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index ffef7ab..a33ffd4 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -825,12 +825,24 @@ static int vhost_dev_set_features(struct vhost_dev 
> > *dev,
> >  static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
> >  {
> >  int r, i, idx;
> > +hwaddr addr;
> > +
> >  r = vhost_dev_set_features(dev, enable_log);
> >  if (r < 0) {
> >  goto err_features;
> >  }
> >  for (i = 0; i < dev->nvqs; ++i) {
> >  idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
> > +addr = virtio_queue_get_desc_addr(dev->vdev, idx);
> > +if (!addr) {
> > +/*
> > + * The queue might not be ready for start. If this
> > + * is the case there is no reason to continue the process.
> > + * The similar logic is used by the vhost_virtqueue_start()
> > + * routine.
> > + */
> 
> Shouldn’t we goto err_vq here to reset the logging state of any vqs
> which have already been set?
As i understand it, no we shouldn't reset the state of other queues. In
general it is pretty valid case. Let's assume that the backend
vhost-user device supports only two queues. But for instance, the QEMU
command line is using value 4 to define number of virtqueues of such
device. In this case only 2 queues will be initializaed.

I've tried to reflect it in the comment section, that the
vhost_virtqueue_start() routine has been alread made the same:
  "if a queue isn't ready for start, just return 0 without any error"
So i made the same here.

I've found this issue, while testing migration with the default
vhost-user-blk daemon. It fails with assert or sigsegv (don't remember),
because it receives NULL for the queues it doesn't have. In general
the daemon should not fall, because of unexpected VHOST_USER
communication, but also there is no reason for QEMU to send additional
packets.

> 
> > +break;
> > +}
> >  r = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
> >   enable_log);
> >  if (r < 0) {
> > --
> > 2.7.4
> >
> >



[PULL 14/18] scripts: Add u2f-setup-gen script

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

This patch adds the script used to generate setup directories, needed
for the device u2f-emulated configuration in directory mode:

python u2f-setup-gen.py $DIR
qemu -usb -device u2f-emulated,dir=$DIR

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-11-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 scripts/u2f-setup-gen.py | 170 +++
 1 file changed, 170 insertions(+)
 create mode 100755 scripts/u2f-setup-gen.py

diff --git a/scripts/u2f-setup-gen.py b/scripts/u2f-setup-gen.py
new file mode 100755
index ..2122598fed8e
--- /dev/null
+++ b/scripts/u2f-setup-gen.py
@@ -0,0 +1,170 @@
+#!/usr/bin/env python3
+#
+# Libu2f-emu setup directory generator for USB U2F key emulation.
+#
+# Copyright (c) 2020 César Belley 
+# Written by César Belley 
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or, at your option, any later version.  See the COPYING file in
+# the top-level directory.
+
+import sys
+import os
+from random import randint
+from typing import Tuple
+
+from cryptography.hazmat.backends import default_backend
+from cryptography.hazmat.primitives.asymmetric import ec
+from cryptography.hazmat.primitives.serialization import Encoding, \
+NoEncryption, PrivateFormat, PublicFormat
+from OpenSSL import crypto
+
+
+def write_setup_dir(dirpath: str, privkey_pem: bytes, cert_pem: bytes,
+entropy: bytes, counter: int) -> None:
+"""
+Write the setup directory.
+
+Args:
+dirpath: The directory path.
+key_pem: The private key PEM.
+cert_pem: The certificate PEM.
+entropy: The 48 bytes of entropy.
+counter: The counter value.
+"""
+# Directory
+os.mkdir(dirpath)
+
+# Private key
+with open(f'{dirpath}/private-key.pem', 'bw') as f:
+f.write(privkey_pem)
+
+# Certificate
+with open(f'{dirpath}/certificate.pem', 'bw') as f:
+f.write(cert_pem)
+
+# Entropy
+with open(f'{dirpath}/entropy', 'wb') as f:
+f.write(entropy)
+
+# Counter
+with open(f'{dirpath}/counter', 'w') as f:
+f.write(f'{str(counter)}\n')
+
+
+def generate_ec_key_pair() -> Tuple[str, str]:
+"""
+Generate an ec key pair.
+
+Returns:
+The private and public key PEM.
+"""
+# Key generation
+privkey = ec.generate_private_key(ec.SECP256R1, default_backend())
+pubkey = privkey.public_key()
+
+# PEM serialization
+privkey_pem = privkey.private_bytes(encoding=Encoding.PEM,
+
format=PrivateFormat.TraditionalOpenSSL,
+encryption_algorithm=NoEncryption())
+pubkey_pem = pubkey.public_bytes(encoding=Encoding.PEM,
+ format=PublicFormat.SubjectPublicKeyInfo)
+return privkey_pem, pubkey_pem
+
+
+def generate_certificate(privkey_pem: str, pubkey_pem: str) -> str:
+"""
+Generate a x509 certificate from a key pair.
+
+Args:
+privkey_pem: The private key PEM.
+pubkey_pem: The public key PEM.
+
+Returns:
+The certificate PEM.
+"""
+# Convert key pair
+privkey = crypto.load_privatekey(crypto.FILETYPE_PEM, privkey_pem)
+pubkey = crypto.load_publickey(crypto.FILETYPE_PEM, pubkey_pem)
+
+# New x509v3 certificate
+cert = crypto.X509()
+cert.set_version(0x2)
+
+# Serial number
+cert.set_serial_number(randint(1, 2 ** 64))
+
+# Before / After
+cert.gmtime_adj_notBefore(0)
+cert.gmtime_adj_notAfter(4 * (365 * 24 * 60 * 60))
+
+# Public key
+cert.set_pubkey(pubkey)
+
+# Subject name and issueer
+cert.get_subject().CN = "U2F emulated"
+cert.set_issuer(cert.get_subject())
+
+# Extensions
+cert.add_extensions([
+crypto.X509Extension(b"subjectKeyIdentifier",
+ False, b"hash", subject=cert),
+])
+cert.add_extensions([
+crypto.X509Extension(b"authorityKeyIdentifier",
+ False, b"keyid:always", issuer=cert),
+])
+cert.add_extensions([
+crypto.X509Extension(b"basicConstraints", True, b"CA:TRUE")
+])
+
+# Signature
+cert.sign(privkey, 'sha256')
+
+return crypto.dump_certificate(crypto.FILETYPE_PEM, cert)
+
+
+def generate_setup_dir(dirpath: str) -> None:
+"""
+Generates the setup directory.
+
+Args:
+dirpath: The directory path.
+"""
+# Key pair
+privkey_pem, pubkey_pem = generate_ec_key_pair()
+
+# Certificate
+certificate_pem = generate_certificate(privkey_pem, pubkey_pem)
+
+# Entropy
+entropy = os.urandom(48)
+
+# Counter
+counter = 0
+
+# Write
+write_setup_dir(dirpath, privkey_pem, certificate_pem, entropy, counter)
+
+
+def main() -> None:
+"""
+Main function
+"""
+# Dir path
+if len(sys.argv) != 2:
+sys.stderr.write(f'Usage: {sys.argv

[PULL 06/18] docs: Add USB U2F key device documentation

2020-08-31 Thread Gerd Hoffmann
From: César Belley 

Add USB U2F key device documentation:
- USB U2F key device
- Building
- Using u2f-emulated
- Using u2f-passthru
- Libu2f-emu

Signed-off-by: César Belley 
Message-id: 20200826114209.28821-3-cesar.bel...@lse.epita.fr
Signed-off-by: Gerd Hoffmann 
---
 docs/u2f.txt | 101 +++
 1 file changed, 101 insertions(+)
 create mode 100644 docs/u2f.txt

diff --git a/docs/u2f.txt b/docs/u2f.txt
new file mode 100644
index ..f60052882ec3
--- /dev/null
+++ b/docs/u2f.txt
@@ -0,0 +1,101 @@
+QEMU U2F Key Device Documentation.
+
+Contents
+1. USB U2F key device
+2. Building
+3. Using u2f-emulated
+4. Using u2f-passthru
+5. Libu2f-emu
+
+1. USB U2F key device
+
+U2F is an open authentication standard that enables relying parties
+exposed to the internet to offer a strong second factor option for end
+user authentication.
+
+The standard brings many advantages to both parties, client and server,
+allowing to reduce over-reliance on passwords, it increases authentication
+security and simplifies passwords.
+
+The second factor is materialized by a device implementing the U2F
+protocol. In case of a USB U2F security key, it is a USB HID device
+that implements the U2F protocol.
+
+In Qemu, the USB U2F key device offers a dedicated support of U2F, allowing
+guest USB FIDO/U2F security keys operating in two possible modes:
+pass-through and emulated.
+
+The pass-through mode consists of passing all requests made from the guest
+to the physical security key connected to the host machine and vice versa.
+In addition, the dedicated pass-through allows to have a U2F security key
+shared on several guests which is not possible with a simple host device
+assignment pass-through.
+
+The emulated mode consists of completely emulating the behavior of an
+U2F device through software part. Libu2f-emu is used for that.
+
+
+2. Building
+
+To ensure the build of the u2f-emulated device variant which depends
+on libu2f-emu: configuring and building:
+
+./configure --enable-u2f && make
+
+
+3. Using u2f-emulated
+
+To work, an emulated U2F device must have four elements:
+ * ec x509 certificate
+ * ec private key
+ * counter (four bytes value)
+ * 48 bytes of entropy (random bits)
+
+To use this type of device, this one has to be configured, and these
+four elements must be passed one way or another.
+
+Assuming that you have a working libu2f-emu installed on the host.
+There are three possible ways of configurations:
+ * ephemeral
+ * setup directory
+ * manual
+
+Ephemeral is the simplest way to configure, it lets the device generate
+all the elements it needs for a single use of the lifetime of the device.
+
+qemu -usb -device u2f-emulated
+
+Setup directory allows to configure the device from a directory containing
+four files:
+ * certificate.pem: ec x509 certificate
+ * private-key.pem: ec private key
+ * counter: counter value
+ * entropy: 48 bytes of entropy
+
+qemu -usb -device u2f-emulated,dir=$dir
+
+Manual allows to configure the device more finely by specifying each
+of the elements necessary for the device:
+ * cert
+ * priv
+ * counter
+ * entropy
+
+qemu -usb -device 
u2f-emulated,cert=$DIR1/$FILE1,priv=$DIR2/$FILE2,counter=$DIR3/$FILE3,entropy=$DIR4/$FILE4
+
+
+4. Using u2f-passthru
+
+On the host specify the u2f-passthru device with a suitable hidraw:
+
+qemu -usb -device u2f-passthru,hidraw=/dev/hidraw0
+
+
+5. Libu2f-emu
+
+The u2f-emulated device uses libu2f-emu for the U2F key emulation. Libu2f-emu
+implements completely the U2F protocol device part for all specified
+transport given by the FIDO Alliance.
+
+For more information about libu2f-emu see this page:
+https://github.com/MattGorko/libu2f-emu.
-- 
2.27.0




Re: [PATCH v2 10/10] vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

2020-08-31 Thread Gerd Hoffmann
On Mon, Aug 31, 2020 at 09:43:15AM -0400, Pan Nengyuan wrote:
> 'addr' is forgot to free in vnc_socket_ip_addr_string error path. Fix that.
> 
> Reported-by: Euler Robot 
> Signed-off-by: Pan Nengyuan 
> Reviewed-by: Li Qiang 
> ---
> Cc: Gerd Hoffmann 

added to UI queue.

thanks,
  Gerd




[Bug 1893010] Re: qemu linux-user doesn't support OFD fcntl locks

2020-08-31 Thread Thomas Huth
** Changed in: qemu
   Status: New => Fix Released

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

Title:
  qemu linux-user doesn't support OFD fcntl locks

Status in QEMU:
  Fix Released

Bug description:
  "Open file description locks (non-POSIX)", as they are described in
  fcntl(2) man page, aren't supported by qemu-user  and attempting to
  use those results in EINVAL. I'm on Gentoo with latest QEMU version
  currently available (5.0.0-r2), and trying to emulate ppc64 and s390x
  on x86_64.

  Looking at linux-user/syscall.c, I'm guessing the issue is in (at
  least) `target_to_host_fcntl_cmd` where switch reaches the default
  clause as there're no cases for F_OFD_SETLK / F_OFD_SETLKW /
  F_OFD_GETLK.

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



Re: [PATCH] ui: Add more mouse buttons to SPICE

2020-08-31 Thread Gerd Hoffmann
> +[INPUT_BUTTON_SIDE]= 0x40,
> +[INPUT_BUTTON_EXTRA]   = 0x80,

Added to UI patch queue.

thanks,
  Gerd




Re: [PATCH] ui/gtk: Update refresh interval after widget is realized

2020-08-31 Thread Gerd Hoffmann
On Mon, Aug 17, 2020 at 07:23:31PM +0200, Philippe Mathieu-Daudé wrote:
> Nikola reported on Windows when gd_vc_gfx_init() is called, the
> window is not yet realized, so we run gd_refresh_rate_millihz(NULL)
> which returns 0 milli-Hertz.
> When a Widget is realized, it fires a 'realized' event. We already
> have the gd_draw_event() handler registered for this even, so simply
> move the gd_refresh_rate_millihz() there. When the event fires, the
> window is known to exist.
> This completes commit c4c00922cc original intention.

Added to UI queue.

thanks,
  Gerd




[PATCH] configure: do not include ${prefix} in firmwarepath

2020-08-31 Thread Paolo Bonzini
Left out in commit 22a87800e6 ("configure: expand path variables for
meson configure", 2020-08-21), do it now.

Signed-off-by: Paolo Bonzini 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 56ceca2f68..d15ecbee46 100755
--- a/configure
+++ b/configure
@@ -473,7 +473,6 @@ LDFLAGS_SHARED="-shared"
 modules="no"
 module_upgrades="no"
 prefix="/usr/local"
-firmwarepath="\${prefix}/share/qemu-firmware"
 qemu_suffix="qemu"
 slirp=""
 oss_lib=""
@@ -1672,6 +1671,7 @@ for opt do
   esac
 done
 
+firmwarepath="${firmwarepath:-$prefix/share/qemu-firmware}"
 libdir="${libdir:-$prefix/lib}"
 libexecdir="${libexecdir:-$prefix/libexec}"
 includedir="${includedir:-$prefix/include}"
-- 
2.26.2




Re: [PATCH v6 01/12] migration/dirtyrate: setup up query-dirtyrate framwork

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:46 +08, Chuan Zheng wrote:

> Add get_dirtyrate_thread() functions to setup query-dirtyrate
> framework.
>
> Signed-off-by: Chuan Zheng 
> Signed-off-by: YanYing Zhuang 
> Reviewed-by: Dr. David Alan Gilbert 

Modulo the question below...

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.c | 38 ++
>  migration/dirtyrate.h | 29 +
>  migration/meson.build |  2 +-
>  3 files changed, 68 insertions(+), 1 deletion(-)
>  create mode 100644 migration/dirtyrate.c
>  create mode 100644 migration/dirtyrate.h
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> new file mode 100644
> index 000..44d673a
> --- /dev/null
> +++ b/migration/dirtyrate.c
> @@ -0,0 +1,38 @@
> +/*
> + * Dirtyrate implement code
> + *
> + * Copyright (c) 2017-2020 HUAWEI TECHNOLOGIES CO.,LTD.

Idle query, given that I'm not a lawyer, has this code really been
around since 2017?

> + *
> + * Authors:
> + *  Chuan Zheng 
> + *
> + * 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 "qapi/error.h"
> +#include "cpu.h"
> +#include "qemu/config-file.h"
> +#include "exec/memory.h"
> +#include "exec/ramblock.h"
> +#include "exec/target_page.h"
> +#include "qemu/rcu_queue.h"
> +#include "qapi/qapi-commands-migration.h"
> +#include "migration.h"
> +#include "dirtyrate.h"
> +
> +static void calculate_dirtyrate(struct DirtyRateConfig config)
> +{
> +/* todo */
> +return;
> +}
> +
> +void *get_dirtyrate_thread(void *arg)
> +{
> +struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
> +
> +calculate_dirtyrate(config);
> +
> +return NULL;
> +}
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> new file mode 100644
> index 000..5be9714
> --- /dev/null
> +++ b/migration/dirtyrate.h
> @@ -0,0 +1,29 @@
> +/*
> + *  Dirtyrate common functions
> + *
> + *  Copyright (c) 2020 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + *  Authors:
> + *  Chuan Zheng 
> + *
> + *  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 QEMU_MIGRATION_DIRTYRATE_H
> +#define QEMU_MIGRATION_DIRTYRATE_H
> +
> +/*
> + * Sample 512 pages per GB as default.
> + * TODO: Make it configurable.
> + */
> +#define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
> +
> +struct DirtyRateConfig {
> +uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
> +int64_t sample_period_seconds; /* time duration between two sampling */
> +};
> +
> +void *get_dirtyrate_thread(void *arg);
> +#endif
> +
> diff --git a/migration/meson.build b/migration/meson.build
> index ac8ff14..b5b71c8 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -37,4 +37,4 @@ softmmu_ss.add(when: ['CONFIG_RDMA', rdma], if_true: 
> files('rdma.c'))
>  softmmu_ss.add(when: 'CONFIG_LIVE_BLOCK_MIGRATION', if_true: 
> files('block.c'))
>  softmmu_ss.add(when: 'CONFIG_ZSTD', if_true: [files('multifd-zstd.c'), zstd])
>  
> -specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files('ram.c'))
> +specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files('dirtyrate.c', 
> 'ram.c'))
> -- 
> 1.8.3.1

dme.
-- 
Tonight I'm gonna bury that horse in the ground.



Re: [PATCH v6 00/12] *** A Method for evaluating dirty page rate ***

2020-08-31 Thread David Edmondson
Trying to think like a control plane developer and user (of which I am
neither) raised some questions about the overall interface provided
here. If everyone else is happy with the current interface, then I'll
shut up :-)

It seems like it should be possible to query the last measured dirty
rate at any time. In particular, it should be possible to query the
value before any rate has been measured (either returning an error, or
if that is unpalatable perhaps a result with a zero interval to indicate
"this data isn't useful"), but also *during* a subsequent measurement
period.

That is, the result of the previous measurement should always be
available on demand and a measurement becomes "current" when it
completes.

Given that we allow the caller to specify the measurement interval, some
callers might specify a long period. As only one measurement can be
taken at a time, a long running measurement rules out taking a short
measurement. That's probably okay, but does lead me to wonder whether
the API should include a mechanism allowing the cancellation of an
in-progress measurement.

dme.
-- 
I can't explain, you would not understand. This is not how I am.



Re: [PATCH v6 03/12] migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:48 +08, Chuan Zheng wrote:

> Add RamblockDirtyInfo to store sampled page info of each ramblock.
>
> Signed-off-by: Chuan Zheng 
> Reviewed-by: Dr. David Alan Gilbert 

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.h | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index 5be9714..479e222 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -19,11 +19,29 @@
>   */
>  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
>  
> +/*
> + * Record ramblock idstr
> + */
> +#define RAMBLOCK_INFO_MAX_LEN 256
> +
>  struct DirtyRateConfig {
>  uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
>  int64_t sample_period_seconds; /* time duration between two sampling */
>  };
>  
> +/*
> + * Store dirtypage info for each ramblock.
> + */
> +struct RamblockDirtyInfo {
> +char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
> +uint8_t *ramblock_addr; /* base address of ramblock we measure */
> +uint64_t ramblock_pages; /* ramblock size in TARGET_PAGE_SIZE */
> +uint64_t *sample_page_vfn; /* relative offset address for sampled page */
> +uint64_t sample_pages_count; /* count of sampled pages */
> +uint64_t sample_dirty_count; /* count of dirty pages we measure */
> +uint32_t *hash_result; /* array of hash result for sampled pages */
> +};
> +
>  void *get_dirtyrate_thread(void *arg);
>  #endif
>  
> -- 
> 1.8.3.1

dme.
-- 
You make me feel like a natural woman.



Re: [PATCH v6 05/12] migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:50 +08, Chuan Zheng wrote:

> RAMBLOCK_FOREACH_MIGRATABLE is need in dirtyrate measure,
> move the existing definition up into migration/ram.h
>
> Signed-off-by: Chuan Zheng 
> Reviewed-by: Dr. David Alan Gilbert 

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.c |  1 +
>  migration/ram.c   | 11 +--
>  migration/ram.h   | 10 ++
>  3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index fa7a1db..35b5c69 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -20,6 +20,7 @@
>  #include "qemu/rcu_queue.h"
>  #include "qapi/qapi-commands-migration.h"
>  #include "migration.h"
> +#include "ram.h"
>  #include "dirtyrate.h"
>  
>  static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
> diff --git a/migration/ram.c b/migration/ram.c
> index 76d4fee..37ef0da 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -158,21 +158,12 @@ out:
>  return ret;
>  }
>  
> -static bool ramblock_is_ignored(RAMBlock *block)
> +bool ramblock_is_ignored(RAMBlock *block)
>  {
>  return !qemu_ram_is_migratable(block) ||
> (migrate_ignore_shared() && qemu_ram_is_shared(block));
>  }
>  
> -/* Should be holding either ram_list.mutex, or the RCU lock. */
> -#define RAMBLOCK_FOREACH_NOT_IGNORED(block)\
> -INTERNAL_RAMBLOCK_FOREACH(block)   \
> -if (ramblock_is_ignored(block)) {} else
> -
> -#define RAMBLOCK_FOREACH_MIGRATABLE(block) \
> -INTERNAL_RAMBLOCK_FOREACH(block)   \
> -if (!qemu_ram_is_migratable(block)) {} else
> -
>  #undef RAMBLOCK_FOREACH
>  
>  int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque)
> diff --git a/migration/ram.h b/migration/ram.h
> index 2eeaacf..011e854 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -37,6 +37,16 @@ extern MigrationStats ram_counters;
>  extern XBZRLECacheStats xbzrle_counters;
>  extern CompressionStats compression_counters;
>  
> +bool ramblock_is_ignored(RAMBlock *block);
> +/* Should be holding either ram_list.mutex, or the RCU lock. */
> +#define RAMBLOCK_FOREACH_NOT_IGNORED(block)\
> +INTERNAL_RAMBLOCK_FOREACH(block)   \
> +if (ramblock_is_ignored(block)) {} else
> +
> +#define RAMBLOCK_FOREACH_MIGRATABLE(block) \
> +INTERNAL_RAMBLOCK_FOREACH(block)   \
> +if (!qemu_ram_is_migratable(block)) {} else
> +
>  int xbzrle_cache_resize(int64_t new_size, Error **errp);
>  uint64_t ram_bytes_remaining(void);
>  uint64_t ram_bytes_total(void);
> -- 
> 1.8.3.1

dme.
-- 
When you were the brightest star, who were the shadows?



Re: [PATCH v6 03/12] migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:48 +08, Chuan Zheng wrote:

> Add RamblockDirtyInfo to store sampled page info of each ramblock.
>
> Signed-off-by: Chuan Zheng 
> Reviewed-by: Dr. David Alan Gilbert 

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.h | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index 5be9714..479e222 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -19,11 +19,29 @@
>   */
>  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
>  
> +/*
> + * Record ramblock idstr
> + */
> +#define RAMBLOCK_INFO_MAX_LEN 256
> +
>  struct DirtyRateConfig {
>  uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
>  int64_t sample_period_seconds; /* time duration between two sampling */
>  };
>  
> +/*
> + * Store dirtypage info for each ramblock.
> + */
> +struct RamblockDirtyInfo {
> +char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
> +uint8_t *ramblock_addr; /* base address of ramblock we measure */
> +uint64_t ramblock_pages; /* ramblock size in TARGET_PAGE_SIZE */
> +uint64_t *sample_page_vfn; /* relative offset address for sampled page */
> +uint64_t sample_pages_count; /* count of sampled pages */
> +uint64_t sample_dirty_count; /* count of dirty pages we measure */
> +uint32_t *hash_result; /* array of hash result for sampled pages */
> +};
> +
>  void *get_dirtyrate_thread(void *arg);
>  #endif
>  
> -- 
> 1.8.3.1

dme.
-- 
This is the time. This is the record of the time.



Re: [PATCH v6 06/12] migration/dirtyrate: Record hash results for each sampled page

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:51 +08, Chuan Zheng wrote:

> Record hash results for each sampled page, crc32 is taken to calculate
> hash results for each sampled length in TARGET_PAGE_SIZE.
>
> Signed-off-by: Chuan Zheng 
> Signed-off-by: YanYing Zhuang 

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.c | 125 
> ++
>  1 file changed, 125 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 35b5c69..f4967fd 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -10,6 +10,7 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> +#include 
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
> @@ -68,6 +69,130 @@ static void update_dirtyrate(uint64_t msec)
>  DirtyStat.dirty_rate = dirtyrate;
>  }
>  
> +/*
> + * get hash result for the sampled memory with length of TARGET_PAGE_SIZE
> + * in ramblock, which starts from ramblock base address.
> + */
> +static uint32_t get_ramblock_vfn_hash(struct RamblockDirtyInfo *info,
> +  uint64_t vfn)
> +{
> +uint32_t crc;
> +
> +crc = crc32(0, (info->ramblock_addr +
> +vfn * TARGET_PAGE_SIZE), TARGET_PAGE_SIZE);
> +
> +return crc;
> +}
> +
> +static int save_ramblock_hash(struct RamblockDirtyInfo *info)
> +{
> +unsigned int sample_pages_count;
> +int i;
> +GRand *rand;
> +
> +sample_pages_count = info->sample_pages_count;
> +
> +/* ramblock size less than one page, return success to skip this 
> ramblock */
> +if (unlikely(info->ramblock_pages == 0 || sample_pages_count == 0)) {
> +return 0;
> +}
> +
> +info->hash_result = g_try_malloc0_n(sample_pages_count,
> +sizeof(uint32_t));
> +if (!info->hash_result) {
> +return -1;
> +}
> +
> +info->sample_page_vfn = g_try_malloc0_n(sample_pages_count,
> +sizeof(uint64_t));
> +if (!info->sample_page_vfn) {
> +g_free(info->hash_result);
> +return -1;
> +}
> +
> +rand  = g_rand_new();
> +for (i = 0; i < sample_pages_count; i++) {
> +info->sample_page_vfn[i] = g_rand_int_range(rand, 0,
> +info->ramblock_pages - 
> 1);
> +info->hash_result[i] = get_ramblock_vfn_hash(info,
> + 
> info->sample_page_vfn[i]);
> +}
> +g_rand_free(rand);
> +
> +return 0;
> +}
> +
> +static void get_ramblock_dirty_info(RAMBlock *block,
> +struct RamblockDirtyInfo *info,
> +struct DirtyRateConfig *config)
> +{
> +uint64_t sample_pages_per_gigabytes = config->sample_pages_per_gigabytes;
> +
> +/* Right shift 30 bits to calc ramblock size in GB */
> +info->sample_pages_count = (qemu_ram_get_used_length(block) *
> +sample_pages_per_gigabytes) >> 30;
> +/* Right shift TARGET_PAGE_BITS to calc page count */
> +info->ramblock_pages = qemu_ram_get_used_length(block) >>
> +   TARGET_PAGE_BITS;
> +info->ramblock_addr = qemu_ram_get_host_addr(block);
> +strcpy(info->idstr, qemu_ram_get_idstr(block));
> +}
> +
> +static struct RamblockDirtyInfo *
> +alloc_ramblock_dirty_info(int *block_index,
> +  struct RamblockDirtyInfo *block_dinfo)
> +{
> +struct RamblockDirtyInfo *info = NULL;
> +int index = *block_index;
> +
> +if (!block_dinfo) {
> +index = 0;
> +block_dinfo = g_try_new(struct RamblockDirtyInfo, 1);
> +} else {
> +index++;
> +block_dinfo = g_try_realloc(block_dinfo, (index + 1) *
> +sizeof(struct RamblockDirtyInfo));
> +}
> +if (!block_dinfo) {
> +return NULL;
> +}
> +
> +info = &block_dinfo[index];
> +*block_index = index;
> +memset(info, 0, sizeof(struct RamblockDirtyInfo));
> +
> +return block_dinfo;
> +}
> +
> +static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
> + struct DirtyRateConfig config,
> + int *block_index)
> +{
> +struct RamblockDirtyInfo *info = NULL;
> +struct RamblockDirtyInfo *dinfo = NULL;
> +RAMBlock *block = NULL;
> +int index = 0;
> +
> +RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +dinfo = alloc_ramblock_dirty_info(&index, dinfo);
> +if (dinfo == NULL) {
> +return -1;
> +}
> +info = &dinfo[index];
> +get_ramblock_dirty_info(block, info, &config);
> +if (save_ramblock_hash(info) < 0) {
> +*block_dinfo = dinfo;
> +*block_index = index;
> +return -1;
> +}
> +}
> +
> +*block_dinfo = dinfo;
> +*block_index = i

Re: [PATCH v6 07/12] migration/dirtyrate: Compare page hash results for recorded sampled page

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:52 +08, Chuan Zheng wrote:

> Compare page hash results for recorded sampled page.
>
> Signed-off-by: Chuan Zheng 
> Signed-off-by: YanYing Zhuang 
> ---
>  migration/dirtyrate.c | 63 
> +++
>  1 file changed, 63 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index f4967fd..9cc2cbb 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -193,6 +193,69 @@ static int record_ramblock_hash_info(struct 
> RamblockDirtyInfo **block_dinfo,
>  return 0;
>  }
>  
> +static void calc_page_dirty_rate(struct RamblockDirtyInfo *info)
> +{
> +uint32_t crc;
> +int i;
> +
> +for (i = 0; i < info->sample_pages_count; i++) {
> +crc = get_ramblock_vfn_hash(info, info->sample_page_vfn[i]);
> +if (crc != info->hash_result[i]) {
> +info->sample_dirty_count++;
> +}
> +}
> +}
> +
> +static struct RamblockDirtyInfo *
> +find_page_matched(RAMBlock *block, int count,
> +  struct RamblockDirtyInfo *infos)
> +{
> +int i;
> +struct RamblockDirtyInfo *matched;
> +
> +for (i = 0; i < count; i++) {
> +if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
> +break;
> +}
> +}
> +
> +if (i == count) {
> +return NULL;
> +}
> +
> +if (infos[i].ramblock_addr != qemu_ram_get_host_addr(block) ||
> +infos[i].ramblock_pages !=
> +(qemu_ram_get_used_length(block) >> TARGET_PAGE_BITS)) {
> +return NULL;
> +}
> +
> +matched = &infos[i];
> +
> +return matched;
> +}
> +
> +static int compare_page_hash_info(struct RamblockDirtyInfo *info,
> +  int block_index)
> +{
> +struct RamblockDirtyInfo *block_dinfo = NULL;
> +RAMBlock *block = NULL;
> +
> +RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +block_dinfo = find_page_matched(block, block_index + 1, info);
> +if (block_dinfo == NULL) {
> +continue;
> +}
> +calc_page_dirty_rate(block_dinfo);
> +update_dirtyrate_stat(block_dinfo);
> +}
> +
> +if (!DirtyStat.total_sample_count) {

total_sample_count isn't a boolean or pointer - comparing against 0
would be clearer.

> +return -1;
> +}
> +
> +return 0;
> +}
> +
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
>  /* todo */
> -- 
> 1.8.3.1

dme.
-- 
You can't hide from the flipside.



Re: [PATCH v6 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period()

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:54 +08, Chuan Zheng wrote:

> Implement set_sample_page_period()/get_sample_page_period() to sleep
> specific time between sample actions.
>
> Signed-off-by: Chuan Zheng 
> Reviewed-by: Dr. David Alan Gilbert 

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.c | 24 
>  migration/dirtyrate.h |  6 ++
>  2 files changed, 30 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 420fc59..850126d 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -27,6 +27,30 @@
>  static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
>  static struct DirtyRateStat DirtyStat;
>  
> +static int64_t set_sample_page_period(int64_t msec, int64_t initial_time)
> +{
> +int64_t current_time;
> +
> +current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +if ((current_time - initial_time) >= msec) {
> +msec = current_time - initial_time;
> +} else {
> +g_usleep((msec + initial_time - current_time) * 1000);
> +}
> +
> +return msec;
> +}
> +
> +static bool get_sample_page_period(int64_t sec)
> +{
> +if (sec < MIN_FETCH_DIRTYRATE_TIME_SEC ||
> +sec > MAX_FETCH_DIRTYRATE_TIME_SEC) {
> +return false;
> +}
> +
> +return true;
> +}
> +
>  static int dirtyrate_set_state(int *state, int old_state, int new_state)
>  {
>  assert(new_state < DIRTY_RATE_STATUS__MAX);
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index faaf9da..8f9bc80 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -29,6 +29,12 @@
>   */
>  #define MIN_RAMBLOCK_SIZE 128
>  
> +/*
> + * Take 1s as minimum time for calculation duration
> + */
> +#define MIN_FETCH_DIRTYRATE_TIME_SEC  1
> +#define MAX_FETCH_DIRTYRATE_TIME_SEC  60
> +
>  struct DirtyRateConfig {
>  uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
>  int64_t sample_period_seconds; /* time duration between two sampling */
> -- 
> 1.8.3.1

dme.
-- 
Tonight I think I'll walk alone, I'll find my soul as I go home.



Re: [PATCH v6 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:55 +08, Chuan Zheng wrote:

> Implement calculate_dirtyrate() function.
>
> Signed-off-by: Chuan Zheng 
> Signed-off-by: YanYing Zhuang 
> ---
>  migration/dirtyrate.c | 45 +++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 850126d..95ee23e 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -162,6 +162,21 @@ static void get_ramblock_dirty_info(RAMBlock *block,
>  strcpy(info->idstr, qemu_ram_get_idstr(block));
>  }
>  
> +static void free_ramblock_dirty_info(struct RamblockDirtyInfo *infos, int 
> count)
> +{
> +int i;
> +
> +if (!infos) {
> +return;
> +}
> +
> +for (i = 0; i < count; i++) {
> +g_free(infos[i].sample_page_vfn);
> +g_free(infos[i].hash_result);
> +}
> +g_free(infos);
> +}
> +
>  static struct RamblockDirtyInfo *
>  alloc_ramblock_dirty_info(int *block_index,
>struct RamblockDirtyInfo *block_dinfo)
> @@ -301,8 +316,34 @@ static int compare_page_hash_info(struct 
> RamblockDirtyInfo *info,
>  
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
> -/* todo */
> -return;
> +struct RamblockDirtyInfo *block_dinfo = NULL;
> +int block_index = 0;
> +int64_t msec = 0;
> +int64_t initial_time;
> +
> +rcu_register_thread();
> +reset_dirtyrate_stat();
> +rcu_read_lock();
> +initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +if (record_ramblock_hash_info(&block_dinfo, config, &block_index) < 0) {
> +goto out;
> +}
> +rcu_read_unlock();
> +
> +msec = config.sample_period_seconds * 1000;
> +msec = set_sample_page_period(msec, initial_time);
> +
> +rcu_read_lock();
> +if (compare_page_hash_info(block_dinfo, block_index) < 0) {
> +goto out;
> +}
> +
> +update_dirtyrate(msec);
> +
> +out:
> +rcu_read_unlock();

This still holds the RCU lock across update_dirtyrate(), which I
understood to be unnecessary.

> +free_ramblock_dirty_info(block_dinfo, block_index + 1);
> +rcu_unregister_thread();
>  }
>  
>  void *get_dirtyrate_thread(void *arg)
> -- 
> 1.8.3.1

dme.
-- 
I'd come on over but I haven't got a raincoat.



Re: [PATCH v6 08/12] migration/dirtyrate: skip sampling ramblock with size below MIN_RAMBLOCK_SIZE

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:53 +08, Chuan Zheng wrote:

> In order to sample real RAM, skip ramblock with size below MIN_RAMBLOCK_SIZE
> which is set as 128M.
>
> Signed-off-by: Chuan Zheng 

Minor wordsmithing below, but...

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.c | 19 +++
>  migration/dirtyrate.h |  5 +
>  2 files changed, 24 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 9cc2cbb..420fc59 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -164,6 +164,19 @@ alloc_ramblock_dirty_info(int *block_index,
>  return block_dinfo;
>  }
>  
> +static bool skip_sample_ramblock(RAMBlock *block)
> +{
> +/*
> + * Consider ramblock with size larger than 128M is what we
> + * want to sample.

"Sample only blocks larger than MIN_RAMBLOCK_SIZE."

> + */
> +if (qemu_ram_get_used_length(block) < (MIN_RAMBLOCK_SIZE << 10)) {
> +return true;
> +}
> +
> +return false;
> +}
> +
>  static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
>   struct DirtyRateConfig config,
>   int *block_index)
> @@ -174,6 +187,9 @@ static int record_ramblock_hash_info(struct 
> RamblockDirtyInfo **block_dinfo,
>  int index = 0;
>  
>  RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +if (skip_sample_ramblock(block)) {
> +continue;
> +}
>  dinfo = alloc_ramblock_dirty_info(&index, dinfo);
>  if (dinfo == NULL) {
>  return -1;
> @@ -241,6 +257,9 @@ static int compare_page_hash_info(struct 
> RamblockDirtyInfo *info,
>  RAMBlock *block = NULL;
>  
>  RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +if (skip_sample_ramblock(block)) {
> +continue;
> +}
>  block_dinfo = find_page_matched(block, block_index + 1, info);
>  if (block_dinfo == NULL) {
>  continue;
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index a3ee305..faaf9da 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -24,6 +24,11 @@
>   */
>  #define RAMBLOCK_INFO_MAX_LEN 256
>  
> +/*
> + * Minimum RAMBlock size to sample, in megabytes.
> + */
> +#define MIN_RAMBLOCK_SIZE 128
> +
>  struct DirtyRateConfig {
>  uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
>  int64_t sample_period_seconds; /* time duration between two sampling */
> -- 
> 1.8.3.1

dme.
-- 
Jane was in her turtle neck, I was much happier then.



Re: [PATCH v6 12/12] migration/dirtyrate: Add trace_calls to make it easier to debug

2020-08-31 Thread David Edmondson
On Saturday, 2020-08-29 at 10:52:57 +08, Chuan Zheng wrote:

> Add trace_calls to  make it easier to debug
>
> Signed-off-by: Chuan Zheng 
> Reviewed-by: Dr. David Alan Gilbert 

Reviewed-by: David Edmondson 

> ---
>  migration/dirtyrate.c  | 9 +
>  migration/trace-events | 8 
>  2 files changed, 17 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index fa1a12d..2a8f4ff 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -22,6 +22,7 @@
>  #include "qapi/qapi-commands-migration.h"
>  #include "migration.h"
>  #include "ram.h"
> +#include "trace.h"
>  #include "dirtyrate.h"
>  
>  static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
> @@ -54,6 +55,7 @@ static bool get_sample_page_period(int64_t sec)
>  static int dirtyrate_set_state(int *state, int old_state, int new_state)
>  {
>  assert(new_state < DIRTY_RATE_STATUS__MAX);
> +trace_dirtyrate_set_state(DirtyRateStatus_str(new_state));
>  if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
>  return 0;
>  } else {
> @@ -76,6 +78,8 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
>  info->start_time = DirtyStat.start_time;
>  info->calc_time = DirtyStat.calc_time;
>  
> +trace_query_dirty_rate_info(DirtyRateStatus_str(CalculatingState));
> +
>  return info;
>  }
>  
> @@ -123,6 +127,7 @@ static uint32_t get_ramblock_vfn_hash(struct 
> RamblockDirtyInfo *info,
>  crc = crc32(0, (info->ramblock_addr +
>  vfn * TARGET_PAGE_SIZE), TARGET_PAGE_SIZE);
>  
> +trace_get_ramblock_vfn_hash(info->idstr, vfn, crc);
>  return crc;
>  }
>  
> @@ -228,6 +233,8 @@ static bool skip_sample_ramblock(RAMBlock *block)
>   * want to sample.
>   */
>  if (qemu_ram_get_used_length(block) < (MIN_RAMBLOCK_SIZE << 10)) {
> +trace_skip_sample_ramblock(block->idstr,
> +   qemu_ram_get_used_length(block));
>  return true;
>  }
>  
> @@ -274,6 +281,7 @@ static void calc_page_dirty_rate(struct RamblockDirtyInfo 
> *info)
>  for (i = 0; i < info->sample_pages_count; i++) {
>  crc = get_ramblock_vfn_hash(info, info->sample_page_vfn[i]);
>  if (crc != info->hash_result[i]) {
> +trace_calc_page_dirty_rate(info->idstr, crc, 
> info->hash_result[i]);
>  info->sample_dirty_count++;
>  }
>  }
> @@ -299,6 +307,7 @@ find_page_matched(RAMBlock *block, int count,
>  if (infos[i].ramblock_addr != qemu_ram_get_host_addr(block) ||
>  infos[i].ramblock_pages !=
>  (qemu_ram_get_used_length(block) >> TARGET_PAGE_BITS)) {
> +trace_find_page_matched(block->idstr);
>  return NULL;
>  }
>  
> diff --git a/migration/trace-events b/migration/trace-events
> index 4ab0a50..8c2b58f 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -312,3 +312,11 @@ dirty_bitmap_load_bits_zeroes(void) ""
>  dirty_bitmap_load_header(uint32_t flags) "flags 0x%x"
>  dirty_bitmap_load_enter(void) ""
>  dirty_bitmap_load_success(void) ""
> +
> +# dirtyrate.c
> +dirtyrate_set_state(const char *new_state) "new state %s"
> +query_dirty_rate_info(const char *new_state) "current state %s"
> +get_ramblock_vfn_hash(const char *idstr, uint64_t vfn, uint32_t crc) 
> "ramblock name: %s, vfn: %"PRIu64 ", crc: %" PRIu32
> +calc_page_dirty_rate(const char *idstr, uint32_t new_crc, uint32_t old_crc) 
> "ramblock name: %s, new crc: %" PRIu32 ", old crc: %" PRIu32
> +skip_sample_ramblock(const char *idstr, uint64_t ramblock_size) "ramblock 
> name: %s, ramblock size: %" PRIu64
> +find_page_matched(const char *idstr) "ramblock %s addr or size changed"
> -- 
> 1.8.3.1

dme.
-- 
All of us, we're going out tonight. We're gonna walk all over your cars.



[PATCH v3 2/7] vhost: check queue state in the vhost_dev_set_log routine

2020-08-31 Thread Dima Stepanov
If the vhost-user-blk daemon provides only one virtqueue, but device was
added with several queues, then QEMU will send more VHOST-USER command
than expected by daemon side. The vhost_virtqueue_start() routine
handles such case by checking the return value from the
virtio_queue_get_desc_addr() function call. Add the same check to the
vhost_dev_set_log() routine.

Signed-off-by: Dima Stepanov 
---
 hw/virtio/vhost.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ffef7ab..a33ffd4 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -825,12 +825,24 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
 static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
 {
 int r, i, idx;
+hwaddr addr;
+
 r = vhost_dev_set_features(dev, enable_log);
 if (r < 0) {
 goto err_features;
 }
 for (i = 0; i < dev->nvqs; ++i) {
 idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
+addr = virtio_queue_get_desc_addr(dev->vdev, idx);
+if (!addr) {
+/*
+ * The queue might not be ready for start. If this
+ * is the case there is no reason to continue the process.
+ * The similar logic is used by the vhost_virtqueue_start()
+ * routine.
+ */
+break;
+}
 r = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
  enable_log);
 if (r < 0) {
-- 
2.7.4




[PATCH v3 1/7] vhost: recheck dev state in the vhost_migration_log routine

2020-08-31 Thread Dima Stepanov
vhost-user devices can get a disconnect in the middle of the VHOST-USER
handshake on the migration start. If disconnect event happened right
before sending next VHOST-USER command, then the vhost_dev_set_log()
call in the vhost_migration_log() function will return error. This error
will lead to the assert() and close the QEMU migration source process.
For the vhost-user devices the disconnect event should not break the
migration process, because:
  - the device will be in the stopped state, so it will not be changed
during migration
  - if reconnect will be made the migration log will be reinitialized as
part of reconnect/init process:
#0  vhost_log_global_start (listener=0x563989cf7be0)
at hw/virtio/vhost.c:920
#1  0x56398603d8bc in listener_add_address_space 
(listener=0x563989cf7be0,
as=0x563986ea4340 )
at softmmu/memory.c:2664
#2  0x56398603dd30 in memory_listener_register (listener=0x563989cf7be0,
as=0x563986ea4340 )
at softmmu/memory.c:2740
#3  0x563985fd6956 in vhost_dev_init (hdev=0x563989cf7bd8,
opaque=0x563989cf7e30, backend_type=VHOST_BACKEND_TYPE_USER,
busyloop_timeout=0)
at hw/virtio/vhost.c:1385
#4  0x563985f7d0b8 in vhost_user_blk_connect (dev=0x563989cf7990)
at hw/block/vhost-user-blk.c:315
#5  0x563985f7d3f6 in vhost_user_blk_event (opaque=0x563989cf7990,
event=CHR_EVENT_OPENED)
at hw/block/vhost-user-blk.c:379
Update the vhost-user-blk device with the internal started_vu field which
will be used for initialization (vhost_user_blk_start) and clean up
(vhost_user_blk_stop). This additional flag in the VhostUserBlk structure
will be used to track whether the device really needs to be stopped and
cleaned up on a vhost-user level.
The disconnect event will set the overall VHOST device (not vhost-user) to
the stopped state, so it can be used by the general vhost_migration_log
routine.
Such approach could be propogated to the other vhost-user devices, but
better idea is just to make the same connect/disconnect code for all the
vhost-user devices.

This migration issue was slightly discussed earlier:
  - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html
  - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05241.html

Signed-off-by: Dima Stepanov 
---
 hw/block/vhost-user-blk.c  | 19 ---
 hw/virtio/vhost.c  | 27 ---
 include/hw/virtio/vhost-user-blk.h | 10 ++
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 39aec42..a076b1e 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -150,6 +150,7 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
 error_report("Error starting vhost: %d", -ret);
 goto err_guest_notifiers;
 }
+s->started_vu = true;
 
 /* guest_notifier_mask/pending not used yet, so just unmask
  * everything here. virtio-pci will do the right thing by
@@ -175,6 +176,11 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 int ret;
 
+if (!s->started_vu) {
+return;
+}
+s->started_vu = false;
+
 if (!k->set_guest_notifiers) {
 return;
 }
@@ -341,9 +347,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
 }
 s->connected = false;
 
-if (s->dev.started) {
-vhost_user_blk_stop(vdev);
-}
+vhost_user_blk_stop(vdev);
 
 vhost_dev_cleanup(&s->dev);
 }
@@ -399,6 +403,15 @@ static void vhost_user_blk_event(void *opaque, 
QEMUChrEvent event)
 NULL, NULL, false);
 aio_bh_schedule_oneshot(ctx, vhost_user_blk_chr_closed_bh, opaque);
 }
+
+/*
+ * Move vhost device to the stopped state. The vhost-user device
+ * will be clean up and disconnected in BH. This can be useful in
+ * the vhost migration code. If disconnect was caught there is an
+ * option for the general vhost code to get the dev state without
+ * knowing its type (in this case vhost-user).
+ */
+s->dev.started = false;
 break;
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1a1384e..ffef7ab 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -861,21 +861,42 @@ static int vhost_migration_log(MemoryListener *listener, 
bool enable)
 dev->log_enabled = enable;
 return 0;
 }
+
+r = 0;
 if (!enable) {
 r = vhost_dev_set_log(dev, false);
 if (r < 0) {
-return r;
+goto check_dev_state;
 }
 vhost_log_put(dev, false);
 } else {
 vhost_dev_log_resize(dev, vhost_get_log_size(dev));
 r = vhost_dev_set_log(dev, true);
 if (r < 0) {
-return r;
+goto check_dev_state;
 

[PATCH v3 0/7] vhost-user-blk: fix the migration issue and enhance qtests

2020-08-31 Thread Dima Stepanov
v2 -> v3:
  - update commit message for the 
"vhost: recheck dev state in the vhost_migration_log routine" commit
  - rename "started" field of the VhostUserBlk structure to
"started_vu", so there will be no confustion with the VHOST started
field
  - update vhost-user-test.c to always initialize nq local variable
(spotted by patchew)

v1 -> v2:
  - add comments to connected/started fields in the header file
  - move the "s->started" logic from the vhost_user_blk_disconnect
routine to the vhost_user_blk_stop routine

Reference e-mail threads:
  - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html
  - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05241.html

If vhost-user daemon is used as a backend for the vhost device, then we
should consider a possibility of disconnect at any moment. There was a general
question here: should we consider it as an error or okay state for the 
vhost-user
devices during migration process?
I think the disconnect event for the vhost-user devices should not break the
migration process, because:
  - the device will be in the stopped state, so it will not be changed
during migration
  - if reconnect will be made the migration log will be reinitialized as
part of reconnect/init process:
#0  vhost_log_global_start (listener=0x563989cf7be0)
at hw/virtio/vhost.c:920
#1  0x56398603d8bc in listener_add_address_space 
(listener=0x563989cf7be0,
as=0x563986ea4340 )
at softmmu/memory.c:2664
#2  0x56398603dd30 in memory_listener_register (listener=0x563989cf7be0,
as=0x563986ea4340 )
at softmmu/memory.c:2740
#3  0x563985fd6956 in vhost_dev_init (hdev=0x563989cf7bd8,
opaque=0x563989cf7e30, backend_type=VHOST_BACKEND_TYPE_USER,
busyloop_timeout=0)
at hw/virtio/vhost.c:1385
#4  0x563985f7d0b8 in vhost_user_blk_connect (dev=0x563989cf7990)
at hw/block/vhost-user-blk.c:315
#5  0x563985f7d3f6 in vhost_user_blk_event (opaque=0x563989cf7990,
event=CHR_EVENT_OPENED)
at hw/block/vhost-user-blk.c:379
The first patch in the patchset fixes this issue by setting vhost device to the
stopped state in the disconnect handler and check it the vhost_migration_log()
routine before returning from the function.
qtest framework was updated to test vhost-user-blk functionality. The
vhost-user-blk/vhost-user-blk-tests/migrate_reconnect test was added to 
reproduce
the original issue found.

Dima Stepanov (7):
  vhost: recheck dev state in the vhost_migration_log routine
  vhost: check queue state in the vhost_dev_set_log routine
  tests/qtest/vhost-user-test: prepare the tests for adding new dev
class
  tests/qtest/libqos/virtio-blk: add support for vhost-user-blk
  tests/qtest/vhost-user-test: add support for the vhost-user-blk device
  tests/qtest/vhost-user-test: add migrate_reconnect test
  tests/qtest/vhost-user-test: enable the reconnect tests

 hw/block/vhost-user-blk.c  |  19 ++-
 hw/virtio/vhost.c  |  39 -
 include/hw/virtio/vhost-user-blk.h |  10 ++
 tests/qtest/libqos/virtio-blk.c|  14 ++
 tests/qtest/vhost-user-test.c  | 290 +++--
 5 files changed, 323 insertions(+), 49 deletions(-)

-- 
2.7.4




[PATCH v3 3/7] tests/qtest/vhost-user-test: prepare the tests for adding new dev class

2020-08-31 Thread Dima Stepanov
For now only vhost-user-net device is supported by the test. Other
vhost-user devices are not tested. As a first step make source code
refactoring so new devices can reuse the same test routines. To make
this provide a new vhost_user_ops structure with the methods to
initialize device, its command line or make a proper vhost-user
responses.

Signed-off-by: Dima Stepanov 
---
 tests/qtest/vhost-user-test.c | 105 ++
 1 file changed, 76 insertions(+), 29 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index 9ee0f1e..3df5322 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -135,6 +135,10 @@ enum {
 TEST_FLAGS_END,
 };
 
+enum {
+VHOST_USER_NET,
+};
+
 typedef struct TestServer {
 gchar *socket_path;
 gchar *mig_path;
@@ -154,10 +158,25 @@ typedef struct TestServer {
 bool test_fail;
 int test_flags;
 int queues;
+struct vhost_user_ops *vu_ops;
 } TestServer;
 
+struct vhost_user_ops {
+/* Device types. */
+int type;
+void (*append_opts)(TestServer *s, GString *cmd_line,
+const char *chr_opts);
+
+/* VHOST-USER commands. */
+void (*set_features)(TestServer *s, CharBackend *chr,
+VhostUserMsg *msg);
+void (*get_protocol_features)(TestServer *s,
+CharBackend *chr, VhostUserMsg *msg);
+};
+
 static const char *init_hugepagefs(void);
-static TestServer *test_server_new(const gchar *name);
+static TestServer *test_server_new(const gchar *name,
+struct vhost_user_ops *ops);
 static void test_server_free(TestServer *server);
 static void test_server_listen(TestServer *server);
 
@@ -167,7 +186,7 @@ enum test_memfd {
 TEST_MEMFD_NO,
 };
 
-static void append_vhost_opts(TestServer *s, GString *cmd_line,
+static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
  const char *chr_opts)
 {
 g_string_append_printf(cmd_line, QEMU_CMD_CHR QEMU_CMD_NETDEV,
@@ -332,25 +351,15 @@ static void chr_read(void *opaque, const uint8_t *buf, 
int size)
 break;
 
 case VHOST_USER_SET_FEATURES:
-g_assert_cmpint(msg.payload.u64 & (0x1ULL << 
VHOST_USER_F_PROTOCOL_FEATURES),
-!=, 0ULL);
-if (s->test_flags == TEST_FLAGS_DISCONNECT) {
-qemu_chr_fe_disconnect(chr);
-s->test_flags = TEST_FLAGS_BAD;
+if (s->vu_ops->set_features) {
+s->vu_ops->set_features(s, chr, &msg);
 }
 break;
 
 case VHOST_USER_GET_PROTOCOL_FEATURES:
-/* send back features to qemu */
-msg.flags |= VHOST_USER_REPLY_MASK;
-msg.size = sizeof(m.payload.u64);
-msg.payload.u64 = 1 << VHOST_USER_PROTOCOL_F_LOG_SHMFD;
-msg.payload.u64 |= 1 << VHOST_USER_PROTOCOL_F_CROSS_ENDIAN;
-if (s->queues > 1) {
-msg.payload.u64 |= 1 << VHOST_USER_PROTOCOL_F_MQ;
+if (s->vu_ops->get_protocol_features) {
+s->vu_ops->get_protocol_features(s, chr, &msg);
 }
-p = (uint8_t *) &msg;
-qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
 break;
 
 case VHOST_USER_GET_VRING_BASE:
@@ -467,7 +476,8 @@ static const char *init_hugepagefs(void)
 #endif
 }
 
-static TestServer *test_server_new(const gchar *name)
+static TestServer *test_server_new(const gchar *name,
+struct vhost_user_ops *ops)
 {
 TestServer *server = g_new0(TestServer, 1);
 char template[] = "/tmp/vhost-test-XX";
@@ -495,6 +505,7 @@ static TestServer *test_server_new(const gchar *name)
 
 server->log_fd = -1;
 server->queues = 1;
+server->vu_ops = ops;
 
 return server;
 }
@@ -669,11 +680,11 @@ static void vhost_user_test_cleanup(void *s)
 
 static void *vhost_user_test_setup(GString *cmd_line, void *arg)
 {
-TestServer *server = test_server_new("vhost-user-test");
+TestServer *server = test_server_new("vhost-user-test", arg);
 test_server_listen(server);
 
 append_mem_opts(server, cmd_line, 256, TEST_MEMFD_AUTO);
-append_vhost_opts(server, cmd_line, "");
+server->vu_ops->append_opts(server, cmd_line, "");
 
 g_test_queue_destroy(vhost_user_test_cleanup, server);
 
@@ -682,11 +693,11 @@ static void *vhost_user_test_setup(GString *cmd_line, 
void *arg)
 
 static void *vhost_user_test_setup_memfd(GString *cmd_line, void *arg)
 {
-TestServer *server = test_server_new("vhost-user-test");
+TestServer *server = test_server_new("vhost-user-test", arg);
 test_server_listen(server);
 
 append_mem_opts(server, cmd_line, 256, TEST_MEMFD_YES);
-append_vhost_opts(server, cmd_line, "");
+server->vu_ops->append_opts(server, cmd_line, "");
 
 g_test_queue_destroy(vhost_user_test_cleanup, server);
 
@@ -720,7 +731,7 @@ static void test_migrate(void *obj, void *arg, 
QGuestAllocator *alloc)
 return;
 }
 
-dest = test_server_new("dest");
+dest = test_serve

[PATCH v3 7/7] tests/qtest/vhost-user-test: enable the reconnect tests

2020-08-31 Thread Dima Stepanov
For now a QTEST_VHOST_USER_FIXME environment variable is used to
separate reconnect tests for the vhost-user-net device. Looks like the
reconnect functionality is pretty stable, so this separation is
deprecated.
Remove it and enable these tests for the default run.

Signed-off-by: Dima Stepanov 
---
 tests/qtest/vhost-user-test.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index 4b715d3..4b96312 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -1140,20 +1140,17 @@ static void register_vhost_user_test(void)
  "virtio-net",
  test_migrate, &opts);
 
-/* keeps failing on build-system since Aug 15 2017 */
-if (getenv("QTEST_VHOST_USER_FIXME")) {
-opts.before = vhost_user_test_setup_reconnect;
-qos_add_test("vhost-user/reconnect", "virtio-net",
- test_reconnect, &opts);
-
-opts.before = vhost_user_test_setup_connect_fail;
-qos_add_test("vhost-user/connect-fail", "virtio-net",
- test_vhost_user_started, &opts);
-
-opts.before = vhost_user_test_setup_flags_mismatch;
-qos_add_test("vhost-user/flags-mismatch", "virtio-net",
- test_vhost_user_started, &opts);
-}
+opts.before = vhost_user_test_setup_reconnect;
+qos_add_test("vhost-user/reconnect", "virtio-net",
+ test_reconnect, &opts);
+
+opts.before = vhost_user_test_setup_connect_fail;
+qos_add_test("vhost-user/connect-fail", "virtio-net",
+ test_vhost_user_started, &opts);
+
+opts.before = vhost_user_test_setup_flags_mismatch;
+qos_add_test("vhost-user/flags-mismatch", "virtio-net",
+ test_vhost_user_started, &opts);
 
 opts.before = vhost_user_test_setup_multiqueue;
 opts.edge.extra_device_opts = "mq=on";
-- 
2.7.4




[PATCH v3 4/7] tests/qtest/libqos/virtio-blk: add support for vhost-user-blk

2020-08-31 Thread Dima Stepanov
Add support for the vhost-user-blk-pci device. This node can be used by
the vhost-user-blk tests. Tests for the vhost-user-blk device are added
in the following patches.

Signed-off-by: Dima Stepanov 
---
 tests/qtest/libqos/virtio-blk.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/tests/qtest/libqos/virtio-blk.c b/tests/qtest/libqos/virtio-blk.c
index 5da0259..959c5dc 100644
--- a/tests/qtest/libqos/virtio-blk.c
+++ b/tests/qtest/libqos/virtio-blk.c
@@ -36,6 +36,9 @@ static void *qvirtio_blk_get_driver(QVirtioBlk *v_blk,
 if (!g_strcmp0(interface, "virtio")) {
 return v_blk->vdev;
 }
+if (!g_strcmp0(interface, "vhost-user-blk")) {
+return v_blk;
+}
 
 fprintf(stderr, "%s not present in virtio-blk-device\n", interface);
 g_assert_not_reached();
@@ -120,6 +123,17 @@ static void virtio_blk_register_nodes(void)
 qos_node_produces("virtio-blk-pci", "virtio-blk");
 
 g_free(arg);
+
+/* vhost-user-blk-pci */
+arg = g_strdup_printf("id=drv0,chardev=chdev0,addr=%x.%x",
+PCI_SLOT, PCI_FN);
+opts.extra_device_opts = arg;
+add_qpci_address(&opts, &addr);
+qos_node_create_driver("vhost-user-blk-pci", virtio_blk_pci_create);
+qos_node_consumes("vhost-user-blk-pci", "pci-bus", &opts);
+qos_node_produces("vhost-user-blk-pci", "vhost-user-blk");
+
+g_free(arg);
 }
 
 libqos_init(virtio_blk_register_nodes);
-- 
2.7.4




[PATCH v3 5/7] tests/qtest/vhost-user-test: add support for the vhost-user-blk device

2020-08-31 Thread Dima Stepanov
Add vhost_user_ops structure for the vhost-user-blk device class. Add
the test_reconnect and test_migrate tests for this device.

Signed-off-by: Dima Stepanov 
---
 tests/qtest/vhost-user-test.c | 139 +-
 1 file changed, 137 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index 3df5322..a8af613 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -24,6 +24,7 @@
 #include "libqos/libqos.h"
 #include "libqos/pci-pc.h"
 #include "libqos/virtio-pci.h"
+#include "libqos/virtio-blk.h"
 
 #include "libqos/malloc-pc.h"
 #include "hw/virtio/virtio-net.h"
@@ -31,6 +32,7 @@
 #include "standard-headers/linux/vhost_types.h"
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/virtio_net.h"
+#include "standard-headers/linux/virtio_blk.h"
 
 #ifdef CONFIG_LINUX
 #include 
@@ -43,6 +45,7 @@
 " -numa node,memdev=mem"
 #define QEMU_CMD_CHR" -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce"
+#define QEMU_CMD_BLKCHR " -chardev socket,id=chdev0,path=%s%s"
 
 #define HUGETLBFS_MAGIC   0x958458f6
 
@@ -55,6 +58,7 @@
 #define VHOST_USER_PROTOCOL_F_MQ 0
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
+#define VHOST_USER_PROTOCOL_F_CONFIG 9
 
 #define VHOST_LOG_PAGE 0x1000
 
@@ -78,6 +82,8 @@ typedef enum VhostUserRequest {
 VHOST_USER_SET_PROTOCOL_FEATURES = 16,
 VHOST_USER_GET_QUEUE_NUM = 17,
 VHOST_USER_SET_VRING_ENABLE = 18,
+VHOST_USER_GET_CONFIG = 24,
+VHOST_USER_SET_CONFIG = 25,
 VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -99,6 +105,14 @@ typedef struct VhostUserLog {
 uint64_t mmap_offset;
 } VhostUserLog;
 
+#define VHOST_USER_MAX_CONFIG_SIZE 256
+typedef struct VhostUserConfig {
+uint32_t offset;
+uint32_t size;
+uint32_t flags;
+uint8_t region[VHOST_USER_MAX_CONFIG_SIZE];
+} VhostUserConfig;
+
 typedef struct VhostUserMsg {
 VhostUserRequest request;
 
@@ -114,6 +128,7 @@ typedef struct VhostUserMsg {
 struct vhost_vring_addr addr;
 VhostUserMemory memory;
 VhostUserLog log;
+VhostUserConfig config;
 } payload;
 } QEMU_PACKED VhostUserMsg;
 
@@ -137,6 +152,7 @@ enum {
 
 enum {
 VHOST_USER_NET,
+VHOST_USER_BLK,
 };
 
 typedef struct TestServer {
@@ -166,12 +182,15 @@ struct vhost_user_ops {
 int type;
 void (*append_opts)(TestServer *s, GString *cmd_line,
 const char *chr_opts);
+void (*driver_init)(void *obj, QGuestAllocator *alloc);
 
 /* VHOST-USER commands. */
 void (*set_features)(TestServer *s, CharBackend *chr,
 VhostUserMsg *msg);
 void (*get_protocol_features)(TestServer *s,
 CharBackend *chr, VhostUserMsg *msg);
+void (*get_config)(TestServer *s, CharBackend *chr,
+VhostUserMsg *msg);
 };
 
 static const char *init_hugepagefs(void);
@@ -194,6 +213,14 @@ static void append_vhost_net_opts(TestServer *s, GString 
*cmd_line,
chr_opts, s->chr_name);
 }
 
+static void append_vhost_blk_opts(TestServer *s, GString *cmd_line,
+ const char *chr_opts)
+{
+g_string_append_printf(cmd_line, QEMU_CMD_BLKCHR,
+   s->socket_path,
+   chr_opts);
+}
+
 static void append_mem_opts(TestServer *server, GString *cmd_line,
 int size, enum test_memfd memfd)
 {
@@ -425,6 +452,12 @@ static void chr_read(void *opaque, const uint8_t *buf, int 
size)
 qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
 break;
 
+case VHOST_USER_GET_CONFIG:
+if (s->vu_ops->get_config) {
+s->vu_ops->get_config(s, chr, &msg);
+}
+break;
+
 default:
 break;
 }
@@ -727,6 +760,9 @@ static void test_migrate(void *obj, void *arg, 
QGuestAllocator *alloc)
 guint8 *log;
 guint64 size;
 
+if (s->vu_ops->driver_init) {
+s->vu_ops->driver_init(obj, alloc);
+}
 if (!wait_for_fds(s)) {
 return;
 }
@@ -796,6 +832,24 @@ static void test_migrate(void *obj, void *arg, 
QGuestAllocator *alloc)
 g_string_free(dest_cmdline, true);
 }
 
+static void vu_blk_driver_init(void *obj, QGuestAllocator *alloc)
+{
+QVirtioBlk *blk_if;
+QVirtioDevice *dev;
+QVirtQueue *vq;
+uint64_t features;
+
+blk_if = obj;
+dev = blk_if->vdev;
+features = qvirtio_get_features(dev);
+qvirtio_set_features(dev, features);
+
+vq = qvirtqueue_setup(dev, alloc, 0);
+g_assert(vq);
+
+qvirtio_set_driver_ok(dev);
+}
+
 static void wait_for_rings_started(TestServer *s, size_t count)
 {
 gint64 end_time;
@@ -857,12 +911,21 @@ static void test_reconnect(void *obj, void *arg, 
QGuestAllocator *alloc)
 {
 TestServer *s = arg;
 GSource *src;
+int nq;
 

[PATCH v3 6/7] tests/qtest/vhost-user-test: add migrate_reconnect test

2020-08-31 Thread Dima Stepanov
Add new migrate_reconnect test for the vhost-user-blk device. Perform a
disconnect after sending response for the VHOST_USER_SET_LOG_BASE
command.

Signed-off-by: Dima Stepanov 
---
 tests/qtest/vhost-user-test.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index a8af613..4b715d3 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -146,6 +146,7 @@ static VhostUserMsg m __attribute__ ((unused));
 enum {
 TEST_FLAGS_OK,
 TEST_FLAGS_DISCONNECT,
+TEST_FLAGS_MIGRATE_DISCONNECT,
 TEST_FLAGS_BAD,
 TEST_FLAGS_END,
 };
@@ -436,6 +437,15 @@ static void chr_read(void *opaque, const uint8_t *buf, int 
size)
 qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE);
 
 g_cond_broadcast(&s->data_cond);
+/*
+ * Perform disconnect after sending a response. In this
+ * case the next write command on the QEMU side (for now
+ * it is SET_FEATURES will return -1, because of disconnect.
+ */
+if (s->test_flags == TEST_FLAGS_MIGRATE_DISCONNECT) {
+qemu_chr_fe_disconnect(chr);
+s->test_flags = TEST_FLAGS_BAD;
+}
 break;
 
 case VHOST_USER_SET_VRING_BASE:
@@ -737,6 +747,17 @@ static void *vhost_user_test_setup_memfd(GString 
*cmd_line, void *arg)
 return server;
 }
 
+static void *vhost_user_test_setup_migrate_reconnect(GString *cmd_line,
+void *arg)
+{
+TestServer *server;
+
+server = vhost_user_test_setup_memfd(cmd_line, arg);
+server->test_flags = TEST_FLAGS_MIGRATE_DISCONNECT;
+
+return server;
+}
+
 static void test_read_guest_mem(void *obj, void *arg, QGuestAllocator *alloc)
 {
 TestServer *server = arg;
@@ -1150,5 +1171,9 @@ static void register_vhost_user_test(void)
 opts.before = vhost_user_test_setup_memfd;
 qos_add_test("migrate", "vhost-user-blk",
  test_migrate, &opts);
+
+opts.before = vhost_user_test_setup_migrate_reconnect;
+qos_add_test("migrate_reconnect", "vhost-user-blk",
+ test_migrate, &opts);
 }
 libqos_init(register_vhost_user_test);
-- 
2.7.4




Re: [PATCH 2/2] hw/virtio-pci Added AER capability.

2020-08-31 Thread Yan Vugenfirer
Ping.

> On 13 Aug 2020, at 10:19 AM, and...@daynix.com wrote:
> 
> From: Andrew 
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1857668
> Added AER capability for virtio-pci devices.
> Also added property for devices, by default AER is enabled.
> 
> Signed-off-by: Andrew Melnychenko 
> ---
> hw/virtio/virtio-pci.c | 16 
> hw/virtio/virtio-pci.h |  4 
> 2 files changed, 20 insertions(+)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 8e02709605..646dfb8a0d 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1806,6 +1806,12 @@ static void virtio_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
>  */
> pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3);
> 
> +if (proxy->flags & VIRTIO_PCI_FLAG_AER) {
> +pcie_aer_init(pci_dev, PCI_ERR_VER, last_pcie_cap_offset,
> +  PCI_ERR_SIZEOF, NULL);
> +last_pcie_cap_offset += PCI_ERR_SIZEOF;
> +}
> +
> if (proxy->flags & VIRTIO_PCI_FLAG_INIT_DEVERR) {
> /* Init error enabling flags */
> pcie_cap_deverr_init(pci_dev);
> @@ -1847,7 +1853,15 @@ static void virtio_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
> 
> static void virtio_pci_exit(PCIDevice *pci_dev)
> {
> +VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
> +bool pcie_port = pci_bus_is_express(pci_get_bus(pci_dev)) &&
> + !pci_bus_is_root(pci_get_bus(pci_dev));
> +
> msix_uninit_exclusive_bar(pci_dev);
> +if (proxy->flags & VIRTIO_PCI_FLAG_AER && pcie_port &&
> +pci_is_express(pci_dev)) {
> +pcie_aer_exit(pci_dev);
> +}
> }
> 
> static void virtio_pci_reset(DeviceState *qdev)
> @@ -1900,6 +1914,8 @@ static Property virtio_pci_properties[] = {
> VIRTIO_PCI_FLAG_INIT_PM_BIT, true),
> DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags,
> VIRTIO_PCI_FLAG_INIT_FLR_BIT, true),
> +DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags,
> +VIRTIO_PCI_FLAG_AER_BIT, true),
> DEFINE_PROP_END_OF_LIST(),
> };
> 
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index e2eaaa9182..4b2491ff15 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -45,6 +45,7 @@ enum {
> VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT,
> VIRTIO_PCI_FLAG_INIT_PM_BIT,
> VIRTIO_PCI_FLAG_INIT_FLR_BIT,
> +VIRTIO_PCI_FLAG_AER_BIT,
> };
> 
> /* Need to activate work-arounds for buggy guests at vmstate load. */
> @@ -84,6 +85,9 @@ enum {
> /* Init Function Level Reset capability */
> #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT)
> 
> +/* Advanced Error Reporting capability */
> +#define VIRTIO_PCI_FLAG_AER (1 << VIRTIO_PCI_FLAG_AER_BIT)
> +
> typedef struct {
> MSIMessage msg;
> int virq;
> -- 
> 2.27.0
> 
> 




Re: [PATCH v6 19/20] tests/acpi: add microvm test

2020-08-31 Thread Gerd Hoffmann
  Hi,

> > +test_acpi_one(" -machine microvm,acpi=on,rtc=off",
> I'm just curious, why rtc is off?

Just want to have it set explicitly in the testcase instead of depending
on the default.  The rtc is listed in the dsdt if present so the switch
has an effect on the expected dsdt content.

take care,
  Gerd




Re: [PATCH v6 00/12] *** A Method for evaluating dirty page rate ***

2020-08-31 Thread Zheng Chuan



On 2020/8/31 17:05, David Edmondson wrote:
> Trying to think like a control plane developer and user (of which I am
> neither) raised some questions about the overall interface provided
> here. If everyone else is happy with the current interface, then I'll
> shut up :-)
> 
> It seems like it should be possible to query the last measured dirty
> rate at any time. In particular, it should be possible to query the
> value before any rate has been measured (either returning an error, or
> if that is unpalatable perhaps a result with a zero interval to indicate
> "this data isn't useful"), but also *during* a subsequent measurement
> period.
> 
Hi, Thank you for your review.

For now,
i. if we query the value before any rate has been measured, it will return 
unstarted,
   and dirtyrate will return -1.
{"return":{"status":"unstarted","dirty-rate":-1,"start-time":0,"calc-time":0},"id":"libvirt-14"}

ii.if we specify the measurement interval like -1 or 61, it will return error
{"id":"libvirt-13","error":{"class":"GenericError","desc":"calc-time is out of 
range[1, 60]."}}

iii. We can query the last measured dirty rate at any time now as you expected 
in last patch version
 with returning the measurement timestamp and calc-time.

If i have missed some other scenes, please let me know:)

> That is, the result of the previous measurement should always be
> available on demand and a measurement becomes "current" when it
> completes.
> 
> Given that we allow the caller to specify the measurement interval, some
> callers might specify a long period. As only one measurement can be
> taken at a time, a long running measurement rules out taking a short
> measurement. That's probably okay, but does lead me to wonder whether
> the API should include a mechanism allowing the cancellation of an
> in-progress measurement.

That's why we restrict the maximum time to 60s, i think this is enough and also
not too long for user to observe the average dirty rate.
I think it is may a little expensive and hardly used to implement cancellation 
mechanism.

On the other hand, users could call several times with the measurement interval
like 1s, to improve its accuracy otherwise observe a long time.



> 
> dme.
> 




Re: Meson build on macOS: undefined symbol treatment

2020-08-31 Thread Paolo Bonzini
On 31/08/20 10:12, Emmanuel Blot wrote:
> 
>> This is done by "b_lundef=false".  I think it was added for modules, but
>> maybe it's not necessary.  Emmanuel, can you try removing it (line 3 of
>> meson.build) and seeing if --enable-modules still works for you?
> 
> Removing this option does restore the regular behavior on macOS (link
> stage fails whenever some symbols are missing), and QEMU still builds ok
> with --enable-modules option, but I have not actually tested this feature
> - I did not know it even existed. Do you want me to test some specific
> use cases?

That's good enough for now, thanks!

Paolo




Re: [PATCH v6 00/12] *** A Method for evaluating dirty page rate ***

2020-08-31 Thread David Edmondson
On Monday, 2020-08-31 at 17:55:39 +08, Zheng Chuan wrote:

> On 2020/8/31 17:05, David Edmondson wrote:
>> Trying to think like a control plane developer and user (of which I am
>> neither) raised some questions about the overall interface provided
>> here. If everyone else is happy with the current interface, then I'll
>> shut up :-)
>> 
>> It seems like it should be possible to query the last measured dirty
>> rate at any time. In particular, it should be possible to query the
>> value before any rate has been measured (either returning an error, or
>> if that is unpalatable perhaps a result with a zero interval to indicate
>> "this data isn't useful"), but also *during* a subsequent measurement
>> period.
>> 
> Hi, Thank you for your review.
>
> For now,
> i. if we query the value before any rate has been measured, it will return 
> unstarted,
>and dirtyrate will return -1.
> {"return":{"status":"unstarted","dirty-rate":-1,"start-time":0,"calc-time":0},"id":"libvirt-14"}
>
> ii.if we specify the measurement interval like -1 or 61, it will return error
> {"id":"libvirt-13","error":{"class":"GenericError","desc":"calc-time is out 
> of range[1, 60]."}}
>
> iii. We can query the last measured dirty rate at any time now as you 
> expected in last patch version
>  with returning the measurement timestamp and calc-time.
>
> If i have missed some other scenes, please let me know:)

No, I think that you have everything. My aim was to see if other people
agreed with the usage scenarios.

>> That is, the result of the previous measurement should always be
>> available on demand and a measurement becomes "current" when it
>> completes.
>> 
>> Given that we allow the caller to specify the measurement interval, some
>> callers might specify a long period. As only one measurement can be
>> taken at a time, a long running measurement rules out taking a short
>> measurement. That's probably okay, but does lead me to wonder whether
>> the API should include a mechanism allowing the cancellation of an
>> in-progress measurement.
>
> That's why we restrict the maximum time to 60s, i think this is enough and 
> also
> not too long for user to observe the average dirty rate.
> I think it is may a little expensive and hardly used to implement 
> cancellation mechanism.
>
> On the other hand, users could call several times with the measurement 
> interval
> like 1s, to improve its accuracy otherwise observe a long time.

Understood.

dme.
-- 
Modern people tend to dance.



[PATCH] meson: use pkg-config method to find dependencies

2020-08-31 Thread Paolo Bonzini
We do not need to ask cmake for the dependencies, so just use the
pkg-config mechanism.  Keep "auto" for SDL so that it tries using
sdl-config too.

Signed-off-by: Paolo Bonzini 
---
 docs/devel/build-system.rst | 6 +++---
 meson.build | 8 
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
index 58bf392430..4465ea76d6 100644
--- a/docs/devel/build-system.rst
+++ b/docs/devel/build-system.rst
@@ -97,11 +97,11 @@ In meson_options.txt::
 In meson.build::
 
   # Detect dependency
-  sdl = dependency('sdl2',
-   required: get_option('sdl'),
+  sdl = dependency('sdl2', required: get_option('sdl'),
+   method: 'pkg-config',
static: enable_static)
 
-  # Create config-host.h
+  # Create config-host.h (if applicable)
   config_host_data.set('CONFIG_SDL', sdl.found())
 
   # Summary
diff --git a/meson.build b/meson.build
index cfdbdd5e8e..26c2aacb69 100644
--- a/meson.build
+++ b/meson.build
@@ -129,7 +129,7 @@ endif
 pixman = not_found
 if have_system or have_tools
   pixman = dependency('pixman-1', required: have_system, version:'>=0.21.8',
-  static: enable_static)
+  method: 'pkg-config', static: enable_static)
 endif
 pam = not_found
 if 'CONFIG_AUTH_PAM' in config_host
@@ -168,7 +168,7 @@ if get_option('xkbcommon').auto() and not have_system and 
not have_tools
   xkbcommon = not_found
 else
   xkbcommon = dependency('xkbcommon', required: get_option('xkbcommon'),
- static: enable_static)
+ method: 'pkg-config', static: enable_static)
 endif
 slirp = not_found
 if config_host.has_key('CONFIG_SLIRP')
@@ -247,7 +247,7 @@ if sdl.found()
   sdl = declare_dependency(compile_args: '-Wno-undef',
dependencies: sdl)
   sdl_image = dependency('SDL2_image', required: get_option('sdl_image'),
- static: enable_static)
+ method: 'pkg-config', static: enable_static)
 else
   if get_option('sdl_image').enabled()
 error('sdl-image required, but SDL was @0@',
@@ -332,7 +332,7 @@ sasl = not_found
 if get_option('vnc').enabled()
   vnc = declare_dependency() # dummy dependency
   png = dependency('libpng', required: get_option('vnc_png'),
-   static: enable_static)
+   method: 'pkg-config', static: enable_static)
   jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
  required: get_option('vnc_jpeg'),
  static: enable_static)
-- 
2.26.2




[Bug 1893634] [NEW] blk_get_max_transfer() works only with sg

2020-08-31 Thread Tom Yan
Public bug reported:

blk_get_max_transfer() is supposed to be able to get the max_sectors
queue limit of the scsi device on the host side and is used in both
scsi-generic.c (for scsi-generic and scsi-block) and scsi-disk.c (for
scsi-hd) to set/change the max_xfer_len (and opt_xfer_len in the case of
scsi-generic).

However, it only works with the sg driver in doing so. It cannot get the
queue limit with the sd driver and simply returns MAX_INT.

qemu version 5.1.0
kernel version 5.8.5

Btw, is there a particular reason that it doesn't MIN_NON_ZERO against
the original max_xfer_len:
https://github.com/qemu/qemu/blob/v5.1.0/hw/scsi/scsi-generic.c#L172?

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  blk_get_max_transfer() works only with sg

Status in QEMU:
  New

Bug description:
  blk_get_max_transfer() is supposed to be able to get the max_sectors
  queue limit of the scsi device on the host side and is used in both
  scsi-generic.c (for scsi-generic and scsi-block) and scsi-disk.c (for
  scsi-hd) to set/change the max_xfer_len (and opt_xfer_len in the case
  of scsi-generic).

  However, it only works with the sg driver in doing so. It cannot get
  the queue limit with the sd driver and simply returns MAX_INT.

  qemu version 5.1.0
  kernel version 5.8.5

  Btw, is there a particular reason that it doesn't MIN_NON_ZERO against
  the original max_xfer_len:
  https://github.com/qemu/qemu/blob/v5.1.0/hw/scsi/scsi-generic.c#L172?

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



Re: [PATCH] configure: do not include ${prefix} in firmwarepath

2020-08-31 Thread Marc-André Lureau
On Mon, Aug 31, 2020 at 12:50 PM Paolo Bonzini  wrote:

> Left out in commit 22a87800e6 ("configure: expand path variables for
> meson configure", 2020-08-21), do it now.
>
> Signed-off-by: Paolo Bonzini 
>

lgtm

Reviewed-by: Marc-André Lureau 


---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 56ceca2f68..d15ecbee46 100755
> --- a/configure
> +++ b/configure
> @@ -473,7 +473,6 @@ LDFLAGS_SHARED="-shared"
>  modules="no"
>  module_upgrades="no"
>  prefix="/usr/local"
> -firmwarepath="\${prefix}/share/qemu-firmware"
>  qemu_suffix="qemu"
>  slirp=""
>  oss_lib=""
> @@ -1672,6 +1671,7 @@ for opt do
>esac
>  done
>
> +firmwarepath="${firmwarepath:-$prefix/share/qemu-firmware}"
>  libdir="${libdir:-$prefix/lib}"
>  libexecdir="${libexecdir:-$prefix/libexec}"
>  includedir="${includedir:-$prefix/include}"
> --
> 2.26.2
>
>
>

-- 
Marc-André Lureau


Re: [PATCH 01/11] hw/arm/virt-acpi-build:Remove dead assignment in build_madt()

2020-08-31 Thread Michael S. Tsirkin
On Thu, Aug 13, 2020 at 03:37:02PM +0800, Chen Qun wrote:
> Clang static code analyzer show warning:
> hw/arm/virt-acpi-build.c:641:5: warning: Value stored to 'madt' is never read
> madt = acpi_data_push(table_data, sizeof *madt);
> ^  
> 
> Reported-by: Euler Robot 
> Signed-off-by: Chen Qun 

Reviewed-by: Michael S. Tsirkin 

feel free to merge through the trivial tree.

> ---
> Cc: Shannon Zhao 
> Cc: Peter Maydell 
> Cc: "Michael S. Tsirkin" 
> Cc: Igor Mammedov 
> Cc: qemu-...@nongnu.org
> ---
>  hw/arm/virt-acpi-build.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 91f0df7b13..f830f9b779 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -633,12 +633,11 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  int madt_start = table_data->len;
>  const MemMapEntry *memmap = vms->memmap;
>  const int *irqmap = vms->irqmap;
> -AcpiMultipleApicTable *madt;
>  AcpiMadtGenericDistributor *gicd;
>  AcpiMadtGenericMsiFrame *gic_msi;
>  int i;
>  
> -madt = acpi_data_push(table_data, sizeof *madt);
> +acpi_data_push(table_data, sizeof(AcpiMultipleApicTable));
>  
>  gicd = acpi_data_push(table_data, sizeof *gicd);
>  gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR;
> -- 
> 2.23.0




Re: [PATCH 05/11] hw/virtio/vhost-user:Remove dead assignment in scrub_shadow_regions()

2020-08-31 Thread Michael S. Tsirkin
On Thu, Aug 13, 2020 at 03:37:06PM +0800, Chen Qun wrote:
> Clang static code analyzer show warning:
> hw/virtio/vhost-user.c:606:9: warning: Value stored to 'mr' is never read
> mr = vhost_user_get_mr_data(reg->userspace_addr, &offset, &fd);
> ^~
> 
> Reported-by: Euler Robot 
> Signed-off-by: Chen Qun 
> ---
> Cc: "Michael S. Tsirkin" 
> Cc: Raphael Norwitz 

Reviewed-by: Michael S. Tsirkin 

feel free to merge through the trivial tree

> ---
>  hw/virtio/vhost-user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index d7e2423762..9c5b4f7fbc 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -603,7 +603,7 @@ static void scrub_shadow_regions(struct vhost_dev *dev,
>   */
>  for (i = 0; i < dev->mem->nregions; i++) {
>  reg = &dev->mem->regions[i];
> -mr = vhost_user_get_mr_data(reg->userspace_addr, &offset, &fd);
> +vhost_user_get_mr_data(reg->userspace_addr, &offset, &fd);
>  if (fd > 0) {
>  ++fd_num;
>  }
> -- 
> 2.23.0




Re: [PATCH 06/11] hw/net/virtio-net:Remove redundant statement in virtio_net_rsc_tcp_ctrl_check()

2020-08-31 Thread Michael S. Tsirkin
On Thu, Aug 13, 2020 at 03:37:07PM +0800, Chen Qun wrote:
> Clang static code analyzer show warning:
> hw/net/virtio-net.c:2077:5: warning: Value stored to 'tcp_flag' is never read
> tcp_flag &= VIRTIO_NET_TCP_FLAG;
> ^   ~~~
> 
> The 'VIRTIO_NET_TCP_FLAG' is '0x3F'. The last ‘tcp_flag’ assignment statement 
> is
>  the same as that of the first two statements.
> 
> Reported-by: Euler Robot 
> Signed-off-by: Chen Qun 

Reviewed-by: Michael S. Tsirkin 

feel free to merge through the trivial tree.

> ---
> Cc: "Michael S. Tsirkin" 
> Cc: Jason Wang 
> ---
>  hw/net/virtio-net.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a1fe9e9285..cb0d27084c 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -2075,7 +2075,6 @@ static int 
> virtio_net_rsc_tcp_ctrl_check(VirtioNetRscChain *chain,
>  tcp_flag = htons(tcp->th_offset_flags);
>  tcp_hdr = (tcp_flag & VIRTIO_NET_TCP_HDR_LENGTH) >> 10;
>  tcp_flag &= VIRTIO_NET_TCP_FLAG;
> -tcp_flag = htons(tcp->th_offset_flags) & 0x3F;
>  if (tcp_flag & TH_SYN) {
>  chain->stat.tcp_syn++;
>  return RSC_BYPASS;
> -- 
> 2.23.0




[PULL 0/5] Ui 20200831 patches

2020-08-31 Thread Gerd Hoffmann
The following changes since commit 39335fab59e11cfda9b7cf63929825db2dd3a3e0:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.2-pull-=
request' into staging (2020-08-28 22:30:11 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/ui-20200831-pull-request

for you to fetch changes up to e54635710f1cf667191a0a1d9df9a37d1c9b0ad0:

  ui/gtk: Update refresh interval after widget is realized (2020-08-31 10:41:=
43 +0200)


ui: memleak fixes.
gtk: refresh interval fix.
keymaps: don't require qemu-keymap (fix gitlab ci).
spice: add mouse buttons.



Frediano Ziglio (1):
  ui: Add more mouse buttons to SPICE

Gerd Hoffmann (1):
  meson: fix keymaps witout qemu-keymap

Pan Nengyuan (2):
  ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()
  vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

Philippe Mathieu-Daud=C3=A9 (1):
  ui/gtk: Update refresh interval after widget is realized

 ui/gtk-gl-area.c| 11 
 ui/gtk.c| 52 ++---
 ui/spice-input.c|  2 ++
 ui/vnc-auth-sasl.c  |  1 +
 pc-bios/keymaps/meson.build | 28 +---
 5 files changed, 59 insertions(+), 35 deletions(-)

--=20
2.27.0





[PULL 4/5] ui: Add more mouse buttons to SPICE

2020-08-31 Thread Gerd Hoffmann
From: Frediano Ziglio 

Add support for SIDE and EXTRA buttons.

The constants for buttons in both SPICE and QEMU are defined as
  LEFT
  MIDDLE
  RIGHT
  UP
  DOWN
  SIDE
  EXTRA
(same order).

"button_mask" contains for each bit the state of a button. Qemu currently
uses bits 0, 1, 2 respectively as LEFT, RIGHT, MIDDLE; also add bits 4
and 5 as UP and DOWN (using wheel movements). SPICE protocol uses
a bitmask based on the order above where LEFT is bit 0, MIDDLE is
bit 1 and so on till EXTRA being bit 6. To avoid clash with Qemu usage
SPICE bitmask from SIDE are move a bit more resulting respectively
in 0x40 and 0x80 values.

Signed-off-by: Frediano Ziglio 
Message-id: 20200820145851.50846-1-fzig...@redhat.com
Signed-off-by: Gerd Hoffmann 
---
 ui/spice-input.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui/spice-input.c b/ui/spice-input.c
index cd4bb0043fd9..d5bba231c95c 100644
--- a/ui/spice-input.c
+++ b/ui/spice-input.c
@@ -123,6 +123,8 @@ static void spice_update_buttons(QemuSpicePointer *pointer,
 [INPUT_BUTTON_RIGHT]   = 0x02,
 [INPUT_BUTTON_WHEEL_UP]= 0x10,
 [INPUT_BUTTON_WHEEL_DOWN]  = 0x20,
+[INPUT_BUTTON_SIDE]= 0x40,
+[INPUT_BUTTON_EXTRA]   = 0x80,
 };
 
 if (wheel < 0) {
-- 
2.27.0




[PULL 2/5] vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

2020-08-31 Thread Gerd Hoffmann
From: Pan Nengyuan 

'addr' is forgot to free in vnc_socket_ip_addr_string error path. Fix that.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Li Qiang 
Message-Id: <20200831134315.1221-11-pannengy...@huawei.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc-auth-sasl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 7b2b09f24276..0517b2ead9ce 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -522,6 +522,7 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
 
 if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
 error_setg(errp, "Not an inet socket type");
+qapi_free_SocketAddress(addr);
 return NULL;
 }
 ret = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
-- 
2.27.0




[PULL 5/5] ui/gtk: Update refresh interval after widget is realized

2020-08-31 Thread Gerd Hoffmann
From: Philippe Mathieu-Daudé 

Nikola reported on Windows when gd_vc_gfx_init() is called, the
window is not yet realized, so we run gd_refresh_rate_millihz(NULL)
which returns 0 milli-Hertz.
When a Widget is realized, it fires a 'realized' event. We already
have the gd_draw_event() handler registered for this even, so simply
move the gd_refresh_rate_millihz() there. When the event fires, the
window is known to exist.
This completes commit c4c00922cc original intention.

Reported-by: Nikola Pavlica 
Tested-by: Nikola Pavlica 
Signed-off-by: Philippe Mathieu-Daudé 
Message-id: 20200817172331.598255-1-phi...@redhat.com
Suggested-by: Nikola Pavlica 
Tested-by: Nikola Pavlica 
Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Gerd Hoffmann 
---
 ui/gtk.c | 52 ++--
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index b0cc08ad6da7..7a717ce8e5e4 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -744,6 +744,25 @@ static void gd_resize_event(GtkGLArea *area,
 
 #endif
 
+/*
+ * If available, return the refresh rate of the display in milli-Hertz,
+ * else return 0.
+ */
+static int gd_refresh_rate_millihz(GtkWidget *window)
+{
+#ifdef GDK_VERSION_3_22
+GdkWindow *win = gtk_widget_get_window(window);
+
+if (win) {
+GdkDisplay *dpy = gtk_widget_get_display(window);
+GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
+
+return gdk_monitor_get_refresh_rate(monitor);
+}
+#endif
+return 0;
+}
+
 static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
 {
 VirtualConsole *vc = opaque;
@@ -751,6 +770,7 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t 
*cr, void *opaque)
 int mx, my;
 int ww, wh;
 int fbw, fbh;
+int refresh_rate_millihz;
 
 #if defined(CONFIG_OPENGL)
 if (vc->gfx.gls) {
@@ -771,6 +791,12 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t 
*cr, void *opaque)
 return FALSE;
 }
 
+refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
+   vc->window : s->window);
+if (refresh_rate_millihz) {
+vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
+}
+
 fbw = surface_width(vc->gfx.ds);
 fbh = surface_height(vc->gfx.ds);
 
@@ -1949,31 +1975,11 @@ static GtkWidget 
*gd_create_menu_machine(GtkDisplayState *s)
 return machine_menu;
 }
 
-/*
- * If available, return the refresh rate of the display in milli-Hertz,
- * else return 0.
- */
-static int gd_refresh_rate_millihz(GtkWidget *window)
-{
-#ifdef GDK_VERSION_3_22
-GdkWindow *win = gtk_widget_get_window(window);
-
-if (win) {
-GdkDisplay *dpy = gtk_widget_get_display(window);
-GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
-
-return gdk_monitor_get_refresh_rate(monitor);
-}
-#endif
-return 0;
-}
-
 static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,
   QemuConsole *con, int idx,
   GSList *group, GtkWidget *view_menu)
 {
 bool zoom_to_fit = false;
-int refresh_rate_millihz;
 
 vc->label = qemu_console_get_label(con);
 vc->s = s;
@@ -2031,12 +2037,6 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, 
VirtualConsole *vc,
 vc->gfx.kbd = qkbd_state_init(con);
 vc->gfx.dcl.con = con;
 
-refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
-   vc->window : s->window);
-if (refresh_rate_millihz) {
-vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
-}
-
 register_displaychangelistener(&vc->gfx.dcl);
 
 gd_connect_vc_gfx_signals(vc);
-- 
2.27.0




[PULL 1/5] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

2020-08-31 Thread Gerd Hoffmann
From: Pan Nengyuan 

Receiving error in local variable err, and forgot to free it.
This patch check the return value of 'gdk_window_create_gl_context'
and 'gdk_gl_context_realize', then free err to fix it.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Message-Id: <20200831134315.1221-6-pannengy...@huawei.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/gtk-gl-area.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 85f9d14c51f1..98c22d23f501 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -147,10 +147,21 @@ QEMUGLContext 
gd_gl_area_create_context(DisplayChangeListener *dcl,
 gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
 window = gtk_widget_get_window(vc->gfx.drawing_area);
 ctx = gdk_window_create_gl_context(window, &err);
+if (err) {
+g_printerr("Create gdk gl context failed: %s\n", err->message);
+g_error_free(err);
+return NULL;
+}
 gdk_gl_context_set_required_version(ctx,
 params->major_ver,
 params->minor_ver);
 gdk_gl_context_realize(ctx, &err);
+if (err) {
+g_printerr("Realize gdk gl context failed: %s\n", err->message);
+g_error_free(err);
+g_clear_object(&ctx);
+return NULL;
+}
 return ctx;
 }
 
-- 
2.27.0




[PULL 0/8] Linux user for 5.2 patches

2020-08-31 Thread Laurent Vivier
The following changes since commit 39335fab59e11cfda9b7cf63929825db2dd3a3e0:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.2-pull-=
request' into staging (2020-08-28 22:30:11 +0100)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request

for you to fetch changes up to d6676fc62a52e020df32abf927c2e7183781a2e3:

  linux-user: Add support for btrfs ioctls used to scrub a filesystem (2020-0=
8-29 10:14:52 +0200)


Add btrfs support



Filip Bozuta (8):
  linux-user: Add support for a group of btrfs ioctls used for
subvolumes
  linux-user: Add support for a group of btrfs ioctls used for snapshots
  linux-user: Add support for btrfs ioctls used to manipulate with
devices
  linux-user: Add support for btrfs ioctls used to get/set features
  linux-user: Add support for a group of btrfs inode ioctls
  linux-user: Add support for two btrfs ioctls used for subvolume
  linux-user: Add support for btrfs ioctls used to manage quota
  linux-user: Add support for btrfs ioctls used to scrub a filesystem

 configure  |   9 ++
 linux-user/ioctls.h| 124 
 linux-user/syscall.c   |   3 +
 linux-user/syscall_defs.h  |  37 +
 linux-user/syscall_types.h | 163 +
 5 files changed, 336 insertions(+)

--=20
2.26.2




[PULL 3/5] meson: fix keymaps witout qemu-keymap

2020-08-31 Thread Gerd Hoffmann
In case the qemu-keymap tool generating them is neither installed on the
system nor built from sources (due to xkbcommon not being available)
qemu will not find the keymaps when started directly from the build
tree,

This happens because commit ddcf607fa3d6 ("meson: drop keymaps symlink")
removed the symlink to the source tree, and the special handling for
install doesn't help in case we do not install qemu.

Lets fix that by simply copying over the file from the source tree as
fallback.

Reported-by: Thomas Huth 
Signed-off-by: Gerd Hoffmann 
Message-id: 20200827102617.14448-1-kra...@redhat.com
---
 pc-bios/keymaps/meson.build | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build
index b737c8223031..e102dd56b454 100644
--- a/pc-bios/keymaps/meson.build
+++ b/pc-bios/keymaps/meson.build
@@ -38,19 +38,29 @@ if meson.is_cross_build() or 'CONFIG_XKBCOMMON' not in 
config_host
 else
   native_qemu_keymap = qemu_keymap
 endif
+
 t = []
 foreach km, args: keymaps
-  t += custom_target(km,
- build_by_default: true,
- output: km,
- command: [native_qemu_keymap, '-f', '@OUTPUT@', 
args.split()],
- install_dir: config_host['qemu_datadir'] / 'keymaps')
+  if native_qemu_keymap.found()
+# generate with qemu-kvm
+t += custom_target(km,
+   build_by_default: true,
+   output: km,
+   command: [native_qemu_keymap, '-f', '@OUTPUT@', 
args.split()],
+   install_dir: config_host['qemu_datadir'] / 'keymaps')
+  else
+# copy from source tree
+t += custom_target(km,
+   build_by_default: true,
+  input: km,
+   output: km,
+   command: ['cp', '@INPUT@', '@OUTPUT@'],
+   install_dir: config_host['qemu_datadir'] / 'keymaps')
+  endif
 endforeach
-if t.length() > 0
+
+if native_qemu_keymap.found()
   alias_target('update-keymaps', t)
-else
-  # install from the source tree
-  install_data(keymaps.keys(), install_dir: config_host['qemu_datadir'] / 
'keymaps')
 endif
 
 install_data(['sl', 'sv'], install_dir: config_host['qemu_datadir'] / 
'keymaps')
-- 
2.27.0




[PULL 1/8] linux-user: Add support for a group of btrfs ioctls used for subvolumes

2020-08-31 Thread Laurent Vivier
From: Filip Bozuta 

This patch implements functionality of following ioctls:

BTRFS_IOC_SUBVOL_CREATE - Creating a btrfs subvolume

Create a btrfs subvolume. The subvolume is created using the ioctl's
third argument which represents a pointer to a following structure
type:

struct btrfs_ioctl_vol_args {
__s64 fd;
char name[BTRFS_PATH_NAME_MAX + 1];
};

Before calling this ioctl, the fields of this structure should be filled
with aproppriate values. The fd field represents the file descriptor
value of the subvolume and the name field represents the subvolume
path.

BTRFS_IOC_SUBVOL_GETFLAGS - Getting subvolume flags

Read the flags of the btrfs subvolume. The flags are read using
the ioctl's third argument that is a pointer of __u64 (unsigned long).
The third argument represents a bit mask that can be composed of following
values:
BTRFS_SUBVOL_RDONLY   (1ULL << 1)
BTRFS_SUBVOL_QGROUP_INHERIT   (1ULL << 2)
BTRFS_DEVICE_SPEC_BY_ID   (1ULL << 3)
BTRFS_SUBVOL_SPEC_BY_ID   (1ULL << 4)

BTRFS_IOC_SUBVOL_SETFLAGS - Setting subvolume flags

Set the flags of the btrfs subvolume. The flags are set using the
ioctl's third argument that is a pointer of __u64 (unsigned long).
The third argument represents a bit mask that can be composed of same
values as in the case of previous ioctl (BTRFS_IOC_SUBVOL_GETFLAGS).

BTRFS_IOC_SUBVOL_GETINFO - Getting subvolume information

Read information about the subvolume. The subvolume information is
returned in the ioctl's third argument which represents a pointer to
a following structure type:

struct btrfs_ioctl_get_subvol_info_args {
/* Id of this subvolume */
__u64 treeid;

/* Name of this subvolume, used to get the real name at mount point */
char name[BTRFS_VOL_NAME_MAX + 1];

/*
 * Id of the subvolume which contains this subvolume.
 * Zero for top-level subvolume or a deleted subvolume.
 */
__u64 parent_id;

/*
 * Inode number of the directory which contains this subvolume.
 * Zero for top-level subvolume or a deleted subvolume
 */
__u64 dirid;

/* Latest transaction id of this subvolume */
__u64 generation;

/* Flags of this subvolume */
__u64 flags;

/* UUID of this subvolume */
__u8 uuid[BTRFS_UUID_SIZE];

/*
 * UUID of the subvolume of which this subvolume is a snapshot.
 * All zero for a non-snapshot subvolume.
 */
__u8 parent_uuid[BTRFS_UUID_SIZE];

/*
 * UUID of the subvolume from which this subvolume was received.
 * All zero for non-received subvolume.
 */
__u8 received_uuid[BTRFS_UUID_SIZE];

/* Transaction id indicating when change/create/send/receive happened */
__u64 ctransid;
__u64 otransid;
__u64 stransid;
__u64 rtransid;
/* Time corresponding to c/o/s/rtransid */
struct btrfs_ioctl_timespec ctime;
struct btrfs_ioctl_timespec otime;
struct btrfs_ioctl_timespec stime;
struct btrfs_ioctl_timespec rtime;

/* Must be zero */
__u64 reserved[8];
 };

 All of the fields of this structure are filled after the ioctl call.

Implementation notes:

Ioctls BTRFS_IOC_SUBVOL_CREATE and BTRFS_IOC_SUBVOL_GETINFO have structure
types as third arguments. That is the reason why a corresponding definition
are added in file 'linux-user/syscall_types.h'.

The line '#include ' is added in file 'linux-user/syscall.c' 
to
recognise preprocessor definitions for these ioctls. Since the file 
"linux/btrfs.h"
was added in the kernel version 3.9, it is enwrapped in an #ifdef statement
with parameter CONFIG_BTRFS which is defined in 'configure' if the
header file is present.

Signed-off-by: Filip Bozuta 
Tested-by: Daniel P. Berrangé 
Message-Id: <20200823195014.116226-2-filip.boz...@syrmia.com>
Signed-off-by: Laurent Vivier 
---
 configure  |  9 +
 linux-user/ioctls.h| 15 +++
 linux-user/syscall.c   |  3 +++
 linux-user/syscall_defs.h  |  8 
 linux-user/syscall_types.h | 32 
 5 files changed, 67 insertions(+)

diff --git a/configure b/configure
index 6ecaff429b1b..24a6146b14c7 100755
--- a/configure
+++ b/configure
@@ -4939,6 +4939,12 @@ if check_include sys/kcov.h ; then
 kcov=yes
 fi
 
+# check for btrfs filesystem support (kernel must be 3.9+)
+btrfs=no
+if check_include linux/btrfs.h ; then
+btrfs=yes
+fi
+
 # If we're making warnings fatal, apply this to Sphinx runs as well
 sphinx_werror=""
 if test "$werror" = "yes"; then
@@ -6918,6 +6924,9 @@ fi
 if test "$kcov" = "yes" ; then
   echo "CONFIG_KCOV=y" >> $config_host_mak
 fi
+if test "$btrfs" = "yes" ; then
+  echo "CONFIG_BTRFS=y" >> $config

Re: [PATCH] meson: use pkg-config method to find dependencies

2020-08-31 Thread Gerd Hoffmann
  Hi,

> Keep "auto" for SDL so that it tries using
> sdl-config too.

> -  sdl = dependency('sdl2',
> -   required: get_option('sdl'),
> +  sdl = dependency('sdl2', required: get_option('sdl'),
> +   method: 'pkg-config',

code and commit message mismatch here.

take care,
  Gerd




  1   2   3   4   5   >