Re: [RFC PATCH 24/66] Hexagon generator phase 2 - op_attribs_generated.h

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/11/20 1:40 AM, Taylor Simpson wrote:

Lists all the attributes associated with each instruction

Signed-off-by: Taylor Simpson 
---
  target/hexagon/do_qemu.py | 13 +
  1 file changed, 13 insertions(+)

diff --git a/target/hexagon/do_qemu.py b/target/hexagon/do_qemu.py
index 5439964..f297931 100755
--- a/target/hexagon/do_qemu.py
+++ b/target/hexagon/do_qemu.py
@@ -797,3 +797,16 @@ realf.write(f.getvalue())
  realf.close()
  f.close()
  
+##

+## Generate the op_attribs_generated.h file
+## Lists all the attributes associated with each instruction
+##
+f = StringIO()
+for tag in tags:
+f.write('OP_ATTRIB(%s,ATTRIBS(%s))\n' % \
+(tag,string.join(sorted(attribdict[tag]),",")))
+realf = open('op_attribs_generated.h', 'wt')
+realf.write(f.getvalue())
+realf.close()
+f.close()
+



This fails with Python 3:

  GEN Hexagon generated files
Traceback (most recent call last):
  File "target/hexagon/do_qemu.py", line 952, in 
(tag,string.join(sorted(attribdict[tag]),",")))
AttributeError: module 'string' has no attribute 'join'




Re: [RFC PATCH 28/66] Hexagon generater phase 4 - Decode tree

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/11/20 8:37 AM, Philippe Mathieu-Daudé wrote:

On 2/11/20 1:40 AM, Taylor Simpson wrote:

Python script that emits the decode tree in dectree_generated.h.

Signed-off-by: Taylor Simpson 
---
  target/hexagon/dectree.py | 354 
++

  1 file changed, 354 insertions(+)
  create mode 100755 target/hexagon/dectree.py

diff --git a/target/hexagon/dectree.py b/target/hexagon/dectree.py
new file mode 100755
index 000..a0435c9
--- /dev/null
+++ b/target/hexagon/dectree.py
@@ -0,0 +1,354 @@
+#!/usr/bin/env python


python3


+
+from __future__ import print_function


Not needed anymore.


+##
+##  Copyright (c) 2019 Qualcomm Innovation Center, Inc. All Rights 
Reserved.

+##
+##  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 .
+##
+
+import cStringIO


Another Python 3 failure:

  GEN Hexagon decode tree
Traceback (most recent call last):
  File "target/hexagon/dectree.py", line 20, in 
import cStringIO
ModuleNotFoundError: No module named 'cStringIO'


+import re
+
+import sys
+sys.path.insert(0, sys.argv[1])
+import iset
+
+encs = {tag : ''.join(reversed(iset.iset[tag]['enc'].replace(' ', '')))
+    for tag in iset.tags if iset.iset[tag]['enc'] != 'MISSING ENCODING'}
+
+enc_classes = set([iset.iset[tag]['enc_class'] for tag in encs.keys()])
+subinsn_enc_classes = \
+    set([enc_class for enc_class in enc_classes \
+    if enc_class.startswith('SUBINSN_')])
+ext_enc_classes = \
+    set([enc_class for enc_class in enc_classes \
+    if enc_class not in ('NORMAL', '16BIT') and \
+   not enc_class.startswith('SUBINSN_')])
+
+try:
+    subinsn_groupings = iset.subinsn_groupings
+except AttributeError:
+    subinsn_groupings = {}
+
+for (tag, subinsn_grouping) in subinsn_groupings.items():
+    encs[tag] = ''.join(reversed(subinsn_grouping['enc'].replace(' ', 
'')))

+
+dectree_normal = {'leaves' : set()}
+dectree_16bit = {'leaves' : set()}
+dectree_subinsn_groupings = {'leaves' : set()}
+dectree_subinsns = {name : {'leaves' : set()} for name in 
subinsn_enc_classes}
+dectree_extensions = {name : {'leaves' : set()} for name in 
ext_enc_classes}

+
+for tag in encs.keys():
+    if tag in subinsn_groupings:
+    dectree_subinsn_groupings['leaves'].add(tag)
+    continue
+    enc_class = iset.iset[tag]['enc_class']
+    if enc_class.startswith('SUBINSN_'):
+    if len(encs[tag]) != 32:
+    encs[tag] = encs[tag] + '0' * (32 - len(encs[tag]))
+    dectree_subinsns[enc_class]['leaves'].add(tag)
+    elif  enc_class == '16BIT':
+    if len(encs[tag]) != 16:
+    raise Exception('Tag "{}" has enc_class "{}" and not an 
encoding ' +

+    'width of 16 bits!'.format(tag, enc_class))
+    dectree_16bit['leaves'].add(tag)
+    else:
+    if len(encs[tag]) != 32:
+    raise Exception('Tag "{}" has enc_class "{}" and not an 
encoding ' +

+    'width of 32 bits!'.format(tag, enc_class))
+    if enc_class == 'NORMAL':
+    dectree_normal['leaves'].add(tag)
+    else:
+    dectree_extensions[enc_class]['leaves'].add(tag)
+
+faketags = set()
+for (tag, enc) in iset.enc_ext_spaces.items():
+    faketags.add(tag)
+    encs[tag] = ''.join(reversed(enc.replace(' ', '')))
+    dectree_normal['leaves'].add(tag)
+
+faketags |= set(subinsn_groupings.keys())
+
+def every_bit_counts(bitset):
+    for i in range(1, len(next(iter(bitset:
+    if len(set([bits[:i] + bits[i+1:] for bits in bitset])) == 
len(bitset):

+    return False
+    return True
+
+def auto_separate(node):
+    tags = node['leaves']
+    if len(tags) <= 1:
+    return
+    enc_width = len(encs[next(iter(tags))])
+    opcode_bit_for_all = \
+    [all([encs[tag][i] in '01' \
+    for tag in tags]) for i in range(enc_width)]
+    opcode_bit_is_0_for_all = \
+    [opcode_bit_for_all[i] and all([encs[tag][i] == '0' \
+    for tag in tags]) for i in range(enc_width)]
+    opcode_bit_is_1_for_all = \
+    [opcode_bit_for_all[i] and all([encs[tag][i] == '1' \
+    for tag in tags]) for i in range(enc_width)]
+    differentiator_opcode_bit = \
+    [opcode_bit_for_all[i] and \
+ not (opcode_bit_is_0_for_all[i] or \
+ opcode_bit_is_1_for_all[i]) \
+    for i in range(enc_width)]
+    best_width = 0
+    

Re: [PATCH v2] hw/arm: ast2600: Wire up EHCI controllers

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/7/20 11:48 PM, Guenter Roeck wrote:

On Fri, Feb 07, 2020 at 02:04:09PM -0800, no-re...@patchew.org wrote:

Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-li...@roeck-us.net/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.


I forgot to mention that the patch depends on the similar
patch for ast2400/ast2500. Sorry for that. Not sure though how
to tell that to the test build system.


You mean the "Aspeed: machine extensions and fixes" series?

Use the 'based-on' tag with the series cover message-id in your cover/patch:

Based-on: <20190904070506.1052-1-...@kaod.org>




Re: [PULL 0/6] 9p patches 2020-02-08

2020-02-11 Thread Christian Schoenebeck
On Montag, 10. Februar 2020 18:08:38 CET Peter Maydell wrote:
> On Sat, 8 Feb 2020 at 10:45, Greg Kurz  wrote:
> > The following changes since commit 
42ccca1bd9456568f996d5646b2001faac96944b:
> >   Merge remote-tracking branch
> >   'remotes/berrange/tags/misc-fixes-pull-request' into staging
> >   (2020-02-07 15:01:23 +)> 
> > are available in the Git repository at:
> >   https://github.com/gkurz/qemu.git tags/9p-next-2020-02-08
> > 
> > for you to fetch changes up to 2822602cbe2be98229b882101dfdb9d3a738c611:
> >   MAINTAINERS: 9pfs: Add myself as reviewer (2020-02-08 09:29:04 +0100)
> > 
> > 
> > 9p patches:
> > - some more protocol sanity checks
> > - qtest for readdir
> > - Christian Schoenebeck now official reviewer
> > 
> > 
> 
> Applied, thanks.
> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
> for any user-visible changes.
> 
> -- PMM

I.e. msize >= 4096 now being required. AFAICS I cannot update the wiki myself.

Best regards,
Christian Schoenebeck





Re: [RFC 1/2] tpm: Let the TPM TIS device be usable on ARM

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/10/20 2:15 PM, Eric Auger wrote:

Implement support for TPM on aarch64 by using the
TPM TIS MMIO frontend. Instead of being an ISA device,
the TPM TIS device becomes a sysbus device on ARM. It is
bound to be dynamically instantiated.

Signed-off-by: Eric Auger 

---

I am aware such kind of #ifde'fy is frown upon but this is just
for starting the discussion


I doubt this can be accepted upstream, because a target has to choose 
between using sysbus OR isa devices, not both.



---
  hw/tpm/Kconfig   |  2 +-
  hw/tpm/tpm_tis.c | 16 
  2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 9e67d990e8..326c89e6df 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -4,7 +4,7 @@ config TPMDEV
  
  config TPM_TIS

  bool
-depends on TPM && ISA_BUS
+depends on TPM
  select TPMDEV
  
  config TPM_CRB

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 31facb896d..cfc840942f 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -30,6 +30,7 @@
  
  #include "hw/acpi/tpm.h"

  #include "hw/pci/pci_ids.h"
+#include "hw/sysbus.h"
  #include "hw/qdev-properties.h"
  #include "migration/vmstate.h"
  #include "sysemu/tpm_backend.h"
@@ -65,7 +66,11 @@ typedef struct TPMLocality {
  } TPMLocality;
  
  typedef struct TPMState {

+#ifdef CONFIG_ISA_BUS
  ISADevice busdev;
+#else
+SysBusDevice busdev;
+#endif
  MemoryRegion mmio;
  
  unsigned char buffer[TPM_TIS_BUFFER_MAX];

@@ -967,6 +972,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error 
**errp)
  error_setg(errp, "'tpmdev' property is required");
  return;
  }
+#ifdef CONFIG_ISA_BUS
  if (s->irq_num > 15) {
  error_setg(errp, "IRQ %d is outside valid range of 0 to 15",
 s->irq_num);
@@ -982,6 +988,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error 
**errp)
  tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
   TPM_PPI_ADDR_BASE, OBJECT(s));
  }
+#endif
  }
  
  static void tpm_tis_initfn(Object *obj)

@@ -991,6 +998,10 @@ static void tpm_tis_initfn(Object *obj)
  memory_region_init_io(&s->mmio, OBJECT(s), &tpm_tis_memory_ops,
s, "tpm-tis-mmio",
TPM_TIS_NUM_LOCALITIES << TPM_TIS_LOCALITY_SHIFT);
+#ifndef CONFIG_ISA_BUS
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+#endif
  }
  
  static void tpm_tis_class_init(ObjectClass *klass, void *data)

@@ -1002,6 +1013,7 @@ static void tpm_tis_class_init(ObjectClass *klass, void 
*data)
  device_class_set_props(dc, tpm_tis_properties);
  dc->reset = tpm_tis_reset;
  dc->vmsd  = &vmstate_tpm_tis;


With your #ifde'fy in mind, you probably need to restrict this to sysbus:

  #ifndef CONFIG_ISA_BUS


+dc->user_creatable = true;


  #endif


  tc->model = TPM_MODEL_TPM_TIS;
  tc->get_version = tpm_tis_get_tpm_version;
  tc->request_completed = tpm_tis_request_completed;
@@ -1009,7 +1021,11 @@ static void tpm_tis_class_init(ObjectClass *klass, void 
*data)
  
  static const TypeInfo tpm_tis_info = {

  .name = TYPE_TPM_TIS,
+#ifdef CONFIG_ISA_BUS
  .parent = TYPE_ISA_DEVICE,
+#else
+.parent = TYPE_SYS_BUS_DEVICE,
+#endif
  .instance_size = sizeof(TPMState),
  .instance_init = tpm_tis_initfn,
  .class_init  = tpm_tis_class_init,



From the sysbus device PoV the patch looks OK.

You don't need much to remove the RFC tag:

- rename TYPE_TPM_TIS as TYPE_TPM_TIS_ISA
- rename TPMState as TPMCommonState, add an abstract TYPE_TPM_TIS 
parent, let TYPE_TPM_TIS_ISA be a child

- add TYPE_TPM_TIS_SYSBUS also child.




Re: [PATCH] gitlab-ci.yml: Add .gitlab-ci.d directory for GitLab specific files

2020-02-11 Thread Thomas Huth
On 11/02/2020 07.50, Philippe Mathieu-Daudé wrote:
> As we plan to let maintainers managing their own GitLab CI jobs,
> add a single directory to contain all the new files (to keep the
> root directory cleaner).

The title and description is a little bit confusing, since the directory
is already there and you just move the YML file into it... so I'd
suggest to rather talk about moving that file instead of adding the
directory.

With the patch description updated:
Acked-by: Thomas Huth 




[PATCH] migration-test: fix some memleaks in migration-test

2020-02-11 Thread pannengyuan
From: Pan Nengyuan 

spotted by asan, 'check-qtest-aarch64' runs fail if sanitizers is enabled.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 tests/qtest/migration-test.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index cf27ebbc9d..2bb214c87f 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -498,11 +498,13 @@ static int test_migrate_start(QTestState **from, 
QTestState **to,
 const char *arch = qtest_get_arch();
 const char *machine_opts = NULL;
 const char *memory_size;
+int ret = 0;
 
 if (args->use_shmem) {
 if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
 g_test_skip("/dev/shm is not supported");
-return -1;
+ret = -1;
+goto out;
 }
 }
 
@@ -611,8 +613,9 @@ static int test_migrate_start(QTestState **from, QTestState 
**to,
 g_free(shmem_path);
 }
 
+out:
 migrate_start_destroy(args);
-return 0;
+return ret;
 }
 
 static void test_migrate_end(QTestState *from, QTestState *to, bool test_dest)
@@ -1134,6 +1137,8 @@ static void test_validate_uuid(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_source);
+g_free(args->opts_target);
 args->opts_source = g_strdup("-uuid ----");
 args->opts_target = g_strdup("-uuid ----");
 do_test_validate_uuid(args, false);
@@ -1143,6 +1148,8 @@ static void test_validate_uuid_error(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_source);
+g_free(args->opts_target);
 args->opts_source = g_strdup("-uuid ----");
 args->opts_target = g_strdup("-uuid ----");
 args->hide_stderr = true;
@@ -1153,6 +1160,7 @@ static void test_validate_uuid_src_not_set(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_target);
 args->opts_target = g_strdup("-uuid ----");
 args->hide_stderr = true;
 do_test_validate_uuid(args, false);
@@ -1162,6 +1170,7 @@ static void test_validate_uuid_dst_not_set(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_source);
 args->opts_source = g_strdup("-uuid ----");
 args->hide_stderr = true;
 do_test_validate_uuid(args, false);
@@ -1379,6 +1388,7 @@ static void test_multifd_tcp_cancel(void)
 "  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
 qobject_unref(rsp);
 
+g_free(uri);
 uri = migrate_get_socket_address(to2, "socket-address");
 
 wait_for_migration_status(from, "cancelled", NULL);
-- 
2.18.1




Re: [PATCH] tests/migration: Add some slack to auto converge

2020-02-11 Thread Juan Quintela
"Dr. David Alan Gilbert (git)"  wrote:
> From: "Dr. David Alan Gilbert" 
>
> There's an assert in autoconverge that checks that we quit the
> iteration when we go below the expected threshold.  Philippe
> saw a case where this assert fired with the measured value
> slightly over the threshold. (about 3k out of a few million).
>
> I can think of two reasons:
>   a) Rounding errors
>   b) That after we make the decision to quit iteration we do one
> more sync and that sees a few more dirty pages.
>
> So add 1% slack to the assertion, that should cover a and
> most cases of b, probably all we'll see for the test.
>
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Juan Quintela 

It shouldn't matter really.  And if we are seeing that problem.




Re: [RFC 1/2] tpm: Let the TPM TIS device be usable on ARM

2020-02-11 Thread Auger Eric
Hi Philippe,

On 2/11/20 9:25 AM, Philippe Mathieu-Daudé wrote:
> On 2/10/20 2:15 PM, Eric Auger wrote:
>> Implement support for TPM on aarch64 by using the
>> TPM TIS MMIO frontend. Instead of being an ISA device,
>> the TPM TIS device becomes a sysbus device on ARM. It is
>> bound to be dynamically instantiated.
>>
>> Signed-off-by: Eric Auger 
>>
>> ---
>>
>> I am aware such kind of #ifde'fy is frown upon but this is just
>> for starting the discussion
> 
> I doubt this can be accepted upstream, because a target has to choose
> between using sysbus OR isa devices, not both.
yep that was a first shot to show how this can be derived for ARM but
this deserves some additional care.

Anyway it currently breaks the x86 code because CONFIG_ISA_BUS is never
defined :-( config-devices.h should be included to fix that. Meaning
that the tpm-tis.o should be compiled with additional -I options. In
that prospect tpm-tis.o should be an obj-y and not a common-obj (Connie).
> 
>> ---
>>   hw/tpm/Kconfig   |  2 +-
>>   hw/tpm/tpm_tis.c | 16 
>>   2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
>> index 9e67d990e8..326c89e6df 100644
>> --- a/hw/tpm/Kconfig
>> +++ b/hw/tpm/Kconfig
>> @@ -4,7 +4,7 @@ config TPMDEV
>>     config TPM_TIS
>>   bool
>> -    depends on TPM && ISA_BUS
>> +    depends on TPM
>>   select TPMDEV
>>     config TPM_CRB
>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>> index 31facb896d..cfc840942f 100644
>> --- a/hw/tpm/tpm_tis.c
>> +++ b/hw/tpm/tpm_tis.c
>> @@ -30,6 +30,7 @@
>>     #include "hw/acpi/tpm.h"
>>   #include "hw/pci/pci_ids.h"
>> +#include "hw/sysbus.h"
>>   #include "hw/qdev-properties.h"
>>   #include "migration/vmstate.h"
>>   #include "sysemu/tpm_backend.h"
>> @@ -65,7 +66,11 @@ typedef struct TPMLocality {
>>   } TPMLocality;
>>     typedef struct TPMState {
>> +#ifdef CONFIG_ISA_BUS
>>   ISADevice busdev;
>> +#else
>> +    SysBusDevice busdev;
>> +#endif
>>   MemoryRegion mmio;
>>     unsigned char buffer[TPM_TIS_BUFFER_MAX];
>> @@ -967,6 +972,7 @@ static void tpm_tis_realizefn(DeviceState *dev,
>> Error **errp)
>>   error_setg(errp, "'tpmdev' property is required");
>>   return;
>>   }
>> +#ifdef CONFIG_ISA_BUS
>>   if (s->irq_num > 15) {
>>   error_setg(errp, "IRQ %d is outside valid range of 0 to 15",
>>  s->irq_num);
>> @@ -982,6 +988,7 @@ static void tpm_tis_realizefn(DeviceState *dev,
>> Error **errp)
>>   tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
>>    TPM_PPI_ADDR_BASE, OBJECT(s));
>>   }
>> +#endif
>>   }
>>     static void tpm_tis_initfn(Object *obj)
>> @@ -991,6 +998,10 @@ static void tpm_tis_initfn(Object *obj)
>>   memory_region_init_io(&s->mmio, OBJECT(s), &tpm_tis_memory_ops,
>>     s, "tpm-tis-mmio",
>>     TPM_TIS_NUM_LOCALITIES <<
>> TPM_TIS_LOCALITY_SHIFT);
>> +#ifndef CONFIG_ISA_BUS
>> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>> +    sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
>> +#endif
>>   }
>>     static void tpm_tis_class_init(ObjectClass *klass, void *data)
>> @@ -1002,6 +1013,7 @@ static void tpm_tis_class_init(ObjectClass
>> *klass, void *data)
>>   device_class_set_props(dc, tpm_tis_properties);
>>   dc->reset = tpm_tis_reset;
>>   dc->vmsd  = &vmstate_tpm_tis;
> 
> With your #ifde'fy in mind, you probably need to restrict this to sysbus:
> 
>   #ifndef CONFIG_ISA_BUS
> 
>> +    dc->user_creatable = true;
> 
>   #endif
yes you're right, this only applies to sysbus
> 
>>   tc->model = TPM_MODEL_TPM_TIS;
>>   tc->get_version = tpm_tis_get_tpm_version;
>>   tc->request_completed = tpm_tis_request_completed;
>> @@ -1009,7 +1021,11 @@ static void tpm_tis_class_init(ObjectClass
>> *klass, void *data)
>>     static const TypeInfo tpm_tis_info = {
>>   .name = TYPE_TPM_TIS,
>> +#ifdef CONFIG_ISA_BUS
>>   .parent = TYPE_ISA_DEVICE,
>> +#else
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +#endif
>>   .instance_size = sizeof(TPMState),
>>   .instance_init = tpm_tis_initfn,
>>   .class_init  = tpm_tis_class_init,
>>
> 
> From the sysbus device PoV the patch looks OK.
> 
> You don't need much to remove the RFC tag:
> 
> - rename TYPE_TPM_TIS as TYPE_TPM_TIS_ISA
> - rename TPMState as TPMCommonState, add an abstract TYPE_TPM_TIS
> parent, let TYPE_TPM_TIS_ISA be a child
> - add TYPE_TPM_TIS_SYSBUS also child.
Yes I tried my luck without too much hopes ;-)

Thanks!

Eric
> 




Re: [PULL 0/6] 9p patches 2020-02-08

2020-02-11 Thread Greg Kurz
On Tue, 11 Feb 2020 09:15:41 +0100
Christian Schoenebeck  wrote:

> On Montag, 10. Februar 2020 18:08:38 CET Peter Maydell wrote:
> > On Sat, 8 Feb 2020 at 10:45, Greg Kurz  wrote:
> > > The following changes since commit 
> 42ccca1bd9456568f996d5646b2001faac96944b:
> > >   Merge remote-tracking branch
> > >   'remotes/berrange/tags/misc-fixes-pull-request' into staging
> > >   (2020-02-07 15:01:23 +)> 
> > > are available in the Git repository at:
> > >   https://github.com/gkurz/qemu.git tags/9p-next-2020-02-08
> > > 
> > > for you to fetch changes up to 2822602cbe2be98229b882101dfdb9d3a738c611:
> > >   MAINTAINERS: 9pfs: Add myself as reviewer (2020-02-08 09:29:04 +0100)
> > > 
> > > 
> > > 9p patches:
> > > - some more protocol sanity checks
> > > - qtest for readdir
> > > - Christian Schoenebeck now official reviewer
> > > 
> > > 
> > 
> > Applied, thanks.
> > 
> > Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
> > for any user-visible changes.
> > 
> > -- PMM
> 
> I.e. msize >= 4096 now being required. AFAICS I cannot update the wiki myself.
> 

I've updated the wiki.

> Best regards,
> Christian Schoenebeck
> 
> 




RE: [PATCH V4 0/5] Introduce Advanced Watch Dog module

2020-02-11 Thread Zhang, Chen


> -Original Message-
> From: Jason Wang 
> Sent: Monday, January 20, 2020 10:57 AM
> To: Zhang, Chen ; Paolo Bonzini
> ; Philippe Mathieu-Daudé ;
> qemu-dev 
> Cc: Zhang Chen 
> Subject: Re: [PATCH V4 0/5] Introduce Advanced Watch Dog module
> 
> 
> On 2020/1/19 下午5:10, Zhang, Chen wrote:
> > Hi~
> >
> > Anyone have comments about this module?
> 
> 
> Hi Chen:
> 
> I will take a look at this series.

Sorry for slow reply due to CNY and extend leave.
OK, waiting your comments~ Thanks~

> 
> Two general questions:
> 
> - if it can detect more than network stall, it should not belong to /net

This module use network connection status to detect all the issue(Host to 
Guest/Host to Host/Host to Admin...).
The target is more than network but all use network way. So it is looks a 
tricky problem.

> - need to convince libvirt guys for this proposal, since usually it's the 
> duty of
> upper layer instead of qemu itself
> 

Yes, It looks a upper layer responsibility, but In the cover latter I have 
explained the reason why we need this in Qemu.
 try to make this module as simple as possible. This module give upper layer 
software a new way to connect/monitoring Qemu.
And due to all the COLO code implement in Qemu side, Many customer want to use 
this FT solution without other dependencies,
it is very easy to integrated to real product. 

Thanks
Zhang Chen

> Thanks
> 
> 
> > We have some clients already try to use this module with COLO. Please
> review this part.
> > If no one want to maintain this module, I can maintain this module myself.
> >
> > Thanks
> > Zhang Chen
> >
> >> -Original Message-
> >> From: Qemu-devel  >> bounces+chen.zhang=intel@nongnu.org> On Behalf Of Zhang, Chen
> >> Sent: Tuesday, January 7, 2020 12:33 PM
> >> To: Jason Wang ; Paolo Bonzini
> >> ; Philippe Mathieu-Daudé
> ;
> >> qemu-dev 
> >> Cc: Zhang Chen 
> >> Subject: Re: [PATCH V4 0/5] Introduce Advanced Watch Dog module
> >>
> >> Hi All,
> >>
> >> No news for a while about this series.
> >>
> >> This version already add new docs to address Paolo's comments.
> >>
> >> Please give me more comments.
> >>
> >>
> >> Thanks
> >>
> >> Zhang Chen
> >>
> >>
> >> On 12/17/2019 8:45 PM, Zhang, Chen wrote:
> >>> From: Zhang Chen 
> >>>
> >>> Advanced Watch Dog is an universal monitoring module on VMM side, it
> >>> can be used to detect network down(VMM to guest, VMM to VMM,
> VMM
> >> to
> >>> another remote server) and do previously set operation. Current AWD
> >>> patch just accept any input as the signal to refresh the watchdog
> >>> timer, and we can also make a certain interactive protocol here. For
> >>> the outputs, user can pre-write some command or some messages in
> the
> >>> AWD opt-script. We noticed that there is no way for VMM communicate
> >>> directly, maybe some people think we don't need such things(up layer
> >>> software like openstack can handle it). so we engaged with real
> >>> customer found that they need a lightweight and efficient mechanism
> >>> to solve some practical problems,
> >>>
> >>> For example Edge Computing cases(they think high level software is
> >>> too heavy to use in Edge or it is hard to manage and combine with VM
> instance).
> >>> It make user have basic VM/Host network monitoring tools and basic
> >>> false tolerance and recovery solution..
> >>>
> >>> Please see the detail documentation in the last patch.
> >>>
> >>> V4:
> >>>- Add more introduction in qemu-options.hx
> >>>- Addressed Paolo's comments add docs/awd.txt for the AWD module
> >> detail.
> >>> V3:
> >>>- Rebased on Qemu 4.2.0-rc1 code.
> >>>- Fix commit message issue.
> >>>
> >>> V2:
> >>>- Addressed Philippe comments add configure selector for AWD.
> >>>
> >>> Initial:
> >>>- Initial version.
> >>>
> >>>
> >>> Zhang Chen (5):
> >>> net/awd.c: Introduce Advanced Watch Dog module framework
> >>> net/awd.c: Initailize input/output chardev
> >>> net/awd.c: Load advanced watch dog worker thread job
> >>> vl.c: Make Advanced Watch Dog delayed initialization
> >>> docs/awd.txt: Add doc to introduce Advanced WatchDog(AWD)
> module
> >>>
> >>>configure |   9 +
> >>>docs/awd.txt  |  88 +
> >>>net/Makefile.objs |   1 +
> >>>net/awd.c | 491
> >> ++
> >>>qemu-options.hx   |  20 ++
> >>>vl.c  |   7 +
> >>>6 files changed, 616 insertions(+)
> >>>create mode 100644 docs/awd.txt
> >>>create mode 100644 net/awd.c
> >>>



Re: [PATCH] migration/rdma: rdma_accept_incoming_migration fix error handling

2020-02-11 Thread Dr. David Alan Gilbert
* Peter Xu (pet...@redhat.com) wrote:
> On Mon, Feb 10, 2020 at 07:44:59PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > rdma_accept_incoming_migration is called from an fd handler and
> > can't return an Error * anywhere.
> > Currently it's leaking Error's in errp/local_err - there's
> > no point putting them in there unless we can report them.
> > 
> > Turn most into fprintf's, and the last into an error_reportf_err
> > where it's coming up from another function.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  migration/rdma.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index 2379b8345b..f67161c98f 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -3980,13 +3980,13 @@ static void rdma_accept_incoming_migration(void 
> > *opaque)
> >  RDMAContext *rdma = opaque;
> >  int ret;
> >  QEMUFile *f;
> > -Error *local_err = NULL, **errp = &local_err;
> > +Error *local_err = NULL;
> >  
> >  trace_qemu_rdma_accept_incoming_migration();
> >  ret = qemu_rdma_accept(rdma);
> >  
> >  if (ret) {
> > -ERROR(errp, "RDMA Migration initialization failed!");
> > +fprintf(stderr, "RDMA ERROR: Migration initialization failed");
> 
> Is there any reason to explictly use stderr instead of the
> error_reportf_err() below (then we simply jump to that for error
> paths)?  The only difference of error_reportf_err() and stderr should
> be when there's one HMP, while shall we always suggest to use
> error_reportf_err() rather than stderr?

Because the error_reportf_err is taking an Error* (from an error
reported by migration_fd_process_incoming) where as we don't have an
Error* at the earlier points.

Dave

> Thanks,
> 
> >  return;
> >  }
> >  
> > @@ -3998,13 +3998,16 @@ static void rdma_accept_incoming_migration(void 
> > *opaque)
> >  
> >  f = qemu_fopen_rdma(rdma, "rb");
> >  if (f == NULL) {
> > -ERROR(errp, "could not qemu_fopen_rdma!");
> > +fprintf(stderr, "RDMA ERROR: could not qemu_fopen_rdma");
> >  qemu_rdma_cleanup(rdma);
> >  return;
> >  }
> >  
> >  rdma->migration_started_on_destination = 1;
> > -migration_fd_process_incoming(f, errp);
> > +migration_fd_process_incoming(f, &local_err);
> > +if (local_err) {
> > +error_reportf_err(local_err, "RDMA ERROR:");
> > +}
> >  }
> >  
> >  void rdma_start_incoming_migration(const char *host_port, Error **errp)
> > -- 
> > 2.24.1
> > 
> 
> -- 
> Peter Xu
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




Re: [PATCH v2 0/5] virtio mmio specification enhancement

2020-02-11 Thread Chao Peng
On Mon, Feb 10, 2020 at 06:44:50AM -0500, Michael S. Tsirkin wrote:
> On Mon, Feb 10, 2020 at 05:05:16PM +0800, Zha Bin wrote:
> > We have compared the number of files and the lines of code between
> > virtio-mmio and virio-pci.
> > 
> > Virtio-PCI  Virtio-MMIO 
> > number of files(Linux)  161 1
> > lines of code(Linux)78237   538
> 
> 
> 
> Something's very wrong here. virtio PCI is 161 files?
> Are you counting the whole PCI subsystem?

Right, that is just a rough statistics. Surely enough, some drivers will
never get enabled in a typcial config.

> Sure enough:
> 
> $ find drivers/pci -name '*c' |wc -l
> 150

and plus:
$ find arch/x86/pci/ -name '*c' |wc -l
22

> 
> That's not reasonable, this includes a bunch of drivers that
> never run on a typical hypervisor.
> 
> MMIO is also not as small as you are trying to show:
> 
> $ cloc drivers/virtio/virtio_mmio.c include/uapi/linux/virtio_mmio.h
>2 text files.
>2 unique files.  
>0 files ignored.
> 
> github.com/AlDanial/cloc v 1.82  T=0.01 s (230.7 files/s, 106126.5 lines/s)
> ---
> Language files  blankcomment   
> code
> ---
> C1144100
> 535
> C/C++ Header 1 39 66 
> 36
> ---
> SUM: 2183166
> 571
> ---
> 
> 
> I don't doubt MMIO is smaller than PCI. Of course that's because it has
> no features to speak of - just this patch already doubles it's size. If
> we keep doing that because we want the features then they will reach
> the same size in about 4 iterations.

Since current virtio-mmio size is small enough, so adding any notable
feature would easily double it. I have no objection that it may one day
reach the same level of PCI, but in this patch some are actually
generic changes and for MSI specific code we provide the option to
confige away.

Thanks,
Chao

> 
> 
> -- 
> MST



[PATCH] tests/acceptance: Add boot tests for sh4 and mips64 QEMU advent calendar images

2020-02-11 Thread Thomas Huth
Now that we can select the second serial console in the acceptance tests
(see commit 746f244d9720 "Allow to use other serial consoles than default"),
we can also test the sh4 image from the QEMU advent calendar 2018.

And another recent commit (ec860426dfbe "Fix handling of LL/SC instructions")
fixed a problem with qemu-system-mips64, so the mips64 from the advent
calendar now works again and can be used for acceptance testing, too.

Signed-off-by: Thomas Huth 
---
 .travis.yml|  2 +-
 tests/acceptance/boot_linux_console.py | 23 +--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 5887055951..71a0097878 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -313,7 +313,7 @@ matrix:
 # Acceptance (Functional) tests
 - name: "GCC check-acceptance"
   env:
-- 
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
+- 
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
 - TEST_CMD="make check-acceptance"
   after_script:
 - python3 -c 'import json; r = 
json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for 
t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 34d37eba3b..a38ee004b1 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -591,12 +591,12 @@ class BootLinuxConsole(Test):
 console_pattern = 'No filesystem could mount root'
 self.wait_for_console_pattern(console_pattern)
 
-def do_test_advcal_2018(self, day, tar_hash, kernel_name):
+def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
 tar_url = ('https://www.qemu-advent-calendar.org'
'/2018/download/day' + day + '.tar.xz')
 file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
 archive.extract(file_path, self.workdir)
-self.vm.set_console()
+self.vm.set_console(console_index=console)
 self.vm.add_args('-kernel',
  self.workdir + '/day' + day + '/' + kernel_name)
 self.vm.launch()
@@ -670,6 +670,25 @@ class BootLinuxConsole(Test):
 self.vm.add_args('-M', 'graphics=off')
 self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')
 
+def test_mips64_malta(self):
+"""
+:avocado: tags=arch:mips64
+:avocado: tags=machine:malta
+:avocado: tags=endian:big
+"""
+tar_hash = '81b030201ec3f28cb1925297f6017d3a20d7ced5'
+self.vm.add_args('-hda', self.workdir + '/day22/' + 'ri-li.qcow2',
+ '-append', 'root=/dev/hda')
+self.do_test_advcal_2018('22', tar_hash, 'vmlinux')
+
+def test_sh4_r2d(self):
+"""
+:avocado: tags=arch:sh4
+:avocado: tags=machine:r2d
+"""
+tar_hash = 'fe06a4fd8ccbf2e27928d64472939d47829d4c7e'
+self.do_test_advcal_2018('09', tar_hash, 'zImage', console=1)
+
 def test_sparc_ss20(self):
 """
 :avocado: tags=arch:sparc
-- 
2.18.1




[PATCH 3/3] iotests: Test copy offloading with external data file

2020-02-11 Thread Kevin Wolf
This adds a test for 'qemu-img convert' with copy offloading where the
target image has an external data file. If the test hosts supports it,
it tests both the case where copy offloading is supported and the case
where it isn't (otherwise we just test unsupported twice).

More specifically, the case with unsupported copy offloading tests
qcow2_alloc_cluster_abort() with external data files.

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/244 | 14 ++
 tests/qemu-iotests/244.out |  6 ++
 2 files changed, 20 insertions(+)

diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244
index 0d1efee6ef..2ec1815e6f 100755
--- a/tests/qemu-iotests/244
+++ b/tests/qemu-iotests/244
@@ -197,6 +197,20 @@ $QEMU_IO -c 'read -P 0x11 0 1M' -f $IMGFMT "$TEST_IMG" | 
_filter_qemu_io
 $QEMU_IMG map --output=human "$TEST_IMG" | _filter_testdir
 $QEMU_IMG map --output=json "$TEST_IMG"
 
+echo
+echo "=== Copy offloading ==="
+echo
+
+# Make use of copy offloading if the test host can provide it
+_make_test_img -o "data_file=$TEST_IMG.data" 64M
+$QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n -C "$TEST_IMG.src" "$TEST_IMG"
+$QEMU_IMG compare -f $IMGFMT -F $IMGFMT "$TEST_IMG.src" "$TEST_IMG"
+
+# blkdebug doesn't support copy offloading, so this tests the error path
+$QEMU_IMG amend -f $IMGFMT -o "data_file=blkdebug::$TEST_IMG.data" "$TEST_IMG"
+$QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n -C "$TEST_IMG.src" "$TEST_IMG"
+$QEMU_IMG compare -f $IMGFMT -F $IMGFMT "$TEST_IMG.src" "$TEST_IMG"
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/244.out b/tests/qemu-iotests/244.out
index 6a3d0067cc..e6f4dc7993 100644
--- a/tests/qemu-iotests/244.out
+++ b/tests/qemu-iotests/244.out
@@ -122,4 +122,10 @@ Offset  Length  Mapped to   File
 0   0x100   TEST_DIR/t.qcow2.data
 [{ "start": 0, "length": 1048576, "depth": 0, "zero": false, "data": true, 
"offset": 0},
 { "start": 1048576, "length": 66060288, "depth": 0, "zero": true, "data": 
false}]
+
+=== Copy offloading ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
data_file=TEST_DIR/t.IMGFMT.data
+Images are identical.
+Images are identical.
 *** done
-- 
2.20.1




[PATCH 0/3] qcow2: Fix write/copy error path with data file

2020-02-11 Thread Kevin Wolf
Kevin Wolf (3):
  qcow2: update_refcount(): Reset old_table_index after
qcow2_cache_put()
  qcow2: Fix qcow2_alloc_cluster_abort() for external data file
  iotests: Test copy offloading with external data file

 block/qcow2-cluster.c  |  7 +--
 block/qcow2-refcount.c |  1 +
 tests/qemu-iotests/244 | 14 ++
 tests/qemu-iotests/244.out |  6 ++
 4 files changed, 26 insertions(+), 2 deletions(-)

-- 
2.20.1




[PATCH 2/3] qcow2: Fix qcow2_alloc_cluster_abort() for external data file

2020-02-11 Thread Kevin Wolf
For external data file, cluster allocations return an offset in the data
file and are not refcounted. In this case, there is nothing to do for
qcow2_alloc_cluster_abort(). Freeing the same offset in the qcow2 file
is wrong and causes crashes in the better case or image corruption in
the worse case.

Signed-off-by: Kevin Wolf 
---
 block/qcow2-cluster.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 8982b7b762..dc3c270226 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1015,8 +1015,11 @@ err:
 void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m)
 {
 BDRVQcow2State *s = bs->opaque;
-qcow2_free_clusters(bs, m->alloc_offset, m->nb_clusters << s->cluster_bits,
-QCOW2_DISCARD_NEVER);
+if (!has_data_file(bs)) {
+qcow2_free_clusters(bs, m->alloc_offset,
+m->nb_clusters << s->cluster_bits,
+QCOW2_DISCARD_NEVER);
+}
 }
 
 /*
-- 
2.20.1




[PATCH 1/3] qcow2: update_refcount(): Reset old_table_index after qcow2_cache_put()

2020-02-11 Thread Kevin Wolf
In the case that update_refcount() frees a refcount block, it evicts it
from the metadata cache. Before doing so, however, it returns the
currently used refcount block to the cache because it might be the same.
Returning the refcount block early means that we need to reset
old_table_index so that we reload the refcount block in the next
iteration if it is actually still in use.

Fixes: f71c08ea8e60f035485a512fd2af8908567592f0
Signed-off-by: Kevin Wolf 
---
 block/qcow2-refcount.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index f67ac6b2d8..b06a9fa9ce 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -889,6 +889,7 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 offset);
 if (table != NULL) {
 qcow2_cache_put(s->refcount_block_cache, &refcount_block);
+old_table_index = -1;
 qcow2_cache_discard(s->refcount_block_cache, table);
 }
 
-- 
2.20.1




[PATCH v2] gitlab-ci: Move EDK2 YAML config to .gitlab-ci.d directory

2020-02-11 Thread Philippe Mathieu-Daudé
We plan to let maintainers managing their own GitLab CI jobs.
The .gitlab-ci.d directory will contain all the new GitLab files,
to keep the root directory cleaner.

The EDK2 job was introduced before .gitlab-ci.d was suggested as
a common directory. Move the YAML file, update its references.

Suggested-by: Wainer dos Santos Moschetta 
Reviewed-by: Laszlo Ersek 
Acked-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
Supersedes: <20200211065022.11134-1-phi...@redhat.com>
v2: Reworded subject/description (Thomas)
---
 .gitignore   | 1 +
 .gitlab-ci-edk2.yml => .gitlab-ci.d/edk2.yml | 2 +-
 .gitlab-ci.yml   | 2 +-
 MAINTAINERS  | 3 +--
 4 files changed, 4 insertions(+), 4 deletions(-)
 rename .gitlab-ci-edk2.yml => .gitlab-ci.d/edk2.yml (98%)

diff --git a/.gitignore b/.gitignore
index bc0a035f9c..18288eacd1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -95,6 +95,7 @@
 *.tp
 *.vr
 *.d
+!/.gitlab-ci.d
 !/scripts/qemu-guest-agent/fsfreeze-hook.d
 *.o
 .sdk
diff --git a/.gitlab-ci-edk2.yml b/.gitlab-ci.d/edk2.yml
similarity index 98%
rename from .gitlab-ci-edk2.yml
rename to .gitlab-ci.d/edk2.yml
index 088ba4b43a..a9990b7147 100644
--- a/.gitlab-ci-edk2.yml
+++ b/.gitlab-ci.d/edk2.yml
@@ -2,7 +2,7 @@ docker-edk2:
  stage: build
  rules: # Only run this job when the Dockerfile is modified
  - changes:
-   - .gitlab-ci-edk2.yml
+   - .gitlab-ci.d/edk2.yml
- .gitlab-ci.d/edk2/Dockerfile
when: always
  image: docker:19.03.1
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c15e394f09..dae6045d78 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,5 +1,5 @@
 include:
-  - local: '/.gitlab-ci-edk2.yml'
+  - local: '/.gitlab-ci.d/edk2.yml'
 
 before_script:
  - apt-get update -qq
diff --git a/MAINTAINERS b/MAINTAINERS
index c7717df720..fb00a55f41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2391,8 +2391,7 @@ F: roms/edk2
 F: roms/edk2-*
 F: tests/data/uefi-boot-images/
 F: tests/uefi-test-tools/
-F: .gitlab-ci-edk2.yml
-F: .gitlab-ci.d/edk2/
+F: .gitlab-ci.d/edk2*
 
 Usermode Emulation
 --
-- 
2.21.1




Re: [PATCH 1/2] tests/acceptance: Extract boot_integratorcp() from test_integratorcp()

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/1/20 8:06 AM, Thomas Huth wrote:

On 31/01/2020 22.11, Philippe Mathieu-Daudé wrote:

As we want to re-use this code, extract it as a new function.
Since we are using the PL011 serial console, add a Avocado tag
to ease filtering of tests.

Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/acceptance/machine_arm_integratorcp.py | 16 ++--
  1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/machine_arm_integratorcp.py 
b/tests/acceptance/machine_arm_integratorcp.py
index 4f9ab40f2f..748819916d 100644
--- a/tests/acceptance/machine_arm_integratorcp.py
+++ b/tests/acceptance/machine_arm_integratorcp.py
@@ -19,12 +19,7 @@ class IntegratorMachine(Test):
  
  timeout = 90
  
-@skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code')

-def test_integratorcp(self):
-"""
-:avocado: tags=arch:arm
-:avocado: tags=machine:integratorcp
-"""
+def boot_integratorcp(self):
  kernel_url = ('https://github.com/zayac/qemu-arm/raw/master/'
'arm-test/kernel/zImage.integrator')
  kernel_hash = '0d7adba893c503267c946a3cbdc63b4b54f25468'
@@ -40,4 +35,13 @@ class IntegratorMachine(Test):
   '-initrd', initrd_path,
   '-append', 'printk.time=0 console=ttyAMA0')


I wonder whether you might want to move the "console=ttyAMA0" to the
test_integratorcp(), too, to get the text in the framebuffer in the
second test instead?


This is not a test, but a common method used by both tests, so both use it.




  self.vm.launch()
+
+@skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
+def test_integratorcp(self):
+"""
+:avocado: tags=arch:arm
+:avocado: tags=machine:integratorcp
+:avocado: tags=device:pl011
+"""
+self.boot_integratorcp()
  wait_for_console_pattern(self, 'Log in as root')



Anyway, patch looks fine,

Reviewed-by: Thomas Huth 


Thanks!

Peter, Thomas previous patch and these series are reviewed. Can you 
queue them in your target-arm.next tree?





Re: [PATCH v2 7/7] tests/bios-tables-test: Update arm/virt memhp test

2020-02-11 Thread Michael S. Tsirkin
On Fri, Jan 17, 2020 at 05:45:22PM +, Shameer Kolothum wrote:
> Since we now have both pc-dimm and nvdimm support, update
> test_acpi_virt_tcg_memhp() to include those.
> 
> Signed-off-by: Shameer Kolothum 

And you can add a last patch on top updating the
expected files and blowing out the allowed diff file.
include the ASL changes in the log ...

> ---
>  tests/data/acpi/virt/NFIT.memhp | 0
>  tests/data/acpi/virt/SSDT.memhp | 0
>  tests/qtest/bios-tables-test.c  | 9 +++--
>  3 files changed, 7 insertions(+), 2 deletions(-)
>  create mode 100644 tests/data/acpi/virt/NFIT.memhp
>  create mode 100644 tests/data/acpi/virt/SSDT.memhp
> 
> diff --git a/tests/data/acpi/virt/NFIT.memhp b/tests/data/acpi/virt/NFIT.memhp
> new file mode 100644
> index 00..e69de29bb2
> diff --git a/tests/data/acpi/virt/SSDT.memhp b/tests/data/acpi/virt/SSDT.memhp
> new file mode 100644
> index 00..e69de29bb2
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index f1ac2d7e96..695d2e7fac 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -913,12 +913,17 @@ static void test_acpi_virt_tcg_memhp(void)
>  };
>  
>  data.variant = ".memhp";
> -test_acpi_one(" -cpu cortex-a57"
> +test_acpi_one(" -machine nvdimm=on"
> +  " -cpu cortex-a57"
>" -m 256M,slots=3,maxmem=1G"
>" -object memory-backend-ram,id=ram0,size=128M"
>" -object memory-backend-ram,id=ram1,size=128M"
>" -numa node,memdev=ram0 -numa node,memdev=ram1"
> -  " -numa dist,src=0,dst=1,val=21",
> +  " -numa dist,src=0,dst=1,val=21"
> +  " -object memory-backend-ram,id=ram2,size=128M"
> +  " -object memory-backend-ram,id=nvm0,size=128M"
> +  " -device pc-dimm,id=dimm0,memdev=ram2,node=0"
> +  " -device nvdimm,id=dimm1,memdev=nvm0,node=1",
>&data);
>  
>  free_test_data(&data);
> -- 
> 2.17.1
> 




Re: [PATCH v1 2/4] virtio: increase virtuqueue size for virtio-scsi and virtio-blk

2020-02-11 Thread Michael S. Tsirkin
On Mon, Feb 10, 2020 at 06:34:15PM +0300, Denis Plotnikov wrote:
> 
> 
> On 09.02.2020 10:49, Michael S. Tsirkin wrote:
> > On Fri, Feb 07, 2020 at 11:48:05AM +0300, Denis Plotnikov wrote:
> > > 
> > > On 05.02.2020 14:19, Stefan Hajnoczi wrote:
> > > > On Tue, Feb 04, 2020 at 12:59:04PM +0300, Denis Plotnikov wrote:
> > > > > On 30.01.2020 17:58, Stefan Hajnoczi wrote:
> > > > > > On Wed, Jan 29, 2020 at 05:07:00PM +0300, Denis Plotnikov wrote:
> > > > > > > The goal is to reduce the amount of requests issued by a guest on
> > > > > > > 1M reads/writes. This rises the performance up to 4% on that kind 
> > > > > > > of
> > > > > > > disk access pattern.
> > > > > > > 
> > > > > > > The maximum chunk size to be used for the guest disk accessing is
> > > > > > > limited with seg_max parameter, which represents the max amount of
> > > > > > > pices in the scatter-geather list in one guest disk request.
> > > > > > > 
> > > > > > > Since seg_max is virqueue_size dependent, increasing the virtqueue
> > > > > > > size increases seg_max, which, in turn, increases the maximum size
> > > > > > > of data to be read/write from guest disk.
> > > > > > > 
> > > > > > > More details in the original problem statment:
> > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
> > > > > > > 
> > > > > > > Suggested-by: Denis V. Lunev 
> > > > > > > Signed-off-by: Denis Plotnikov 
> > > > > > > ---
> > > > > > > hw/core/machine.c  | 3 +++
> > > > > > > include/hw/virtio/virtio.h | 2 +-
> > > > > > > 2 files changed, 4 insertions(+), 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > > > > index 3e288bfceb..8bc401d8b7 100644
> > > > > > > --- a/hw/core/machine.c
> > > > > > > +++ b/hw/core/machine.c
> > > > > > > @@ -28,6 +28,9 @@
> > > > > > > #include "hw/mem/nvdimm.h"
> > > > > > > GlobalProperty hw_compat_4_2[] = {
> > > > > > > +{ "virtio-blk-device", "queue-size", "128"},
> > > > > > > +{ "virtio-scsi-device", "virtqueue_size", "128"},
> > > > > > > +{ "vhost-blk-device", "virtqueue_size", "128"},
> > > > > > vhost-blk-device?!  Who has this?  It's not in qemu.git so please 
> > > > > > omit
> > > > > > this line. ;-)
> > > > > So in this case the line:
> > > > > 
> > > > > { "vhost-blk-device", "seg_max_adjust", "off"},
> > > > > 
> > > > > introduced by my patch:
> > > > > 
> > > > > commit 1bf8a989a566b2ba41c197004ec2a02562a766a4
> > > > > Author: Denis Plotnikov 
> > > > > Date:   Fri Dec 20 17:09:04 2019 +0300
> > > > > 
> > > > >       virtio: make seg_max virtqueue size dependent
> > > > > 
> > > > > is also wrong. It should be:
> > > > > 
> > > > > { "vhost-scsi-device", "seg_max_adjust", "off"},
> > > > > 
> > > > > Am I right?
> > > > It's just called "vhost-scsi":
> > > > 
> > > > include/hw/virtio/vhost-scsi.h:#define TYPE_VHOST_SCSI "vhost-scsi"
> > > > 
> > > > > > On the other hand, do you want to do this for the vhost-user-blk,
> > > > > > vhost-user-scsi, and vhost-scsi devices that exist in qemu.git?  
> > > > > > Those
> > > > > > devices would benefit from better performance too.
> > > After thinking about that for a while, I think we shouldn't extend queue
> > > sizes for vhost-user-blk, vhost-user-scsi and vhost-scsi.
> > > This is because increasing the queue sizes seems to be just useless for
> > > them: the all thing is about increasing the queue sizes for increasing
> > > seg_max (it limits the max block query size from the guest). For
> > > virtio-blk-device and virtio-scsi-device it makes sense, since they have
> > > seg-max-adjust property which, if true, sets seg_max to virtqueue_size-2.
> > > vhost-scsi also have this property but it seems the property just doesn't
> > > affect anything (remove it?).
> > > Also vhost-user-blk, vhost-user-scsi and vhost-scsi don't do any seg_max
> > > settings. If I understand correctly, their backends are ment to be
> > > responsible for doing that.
> > The queue size is set by qemu IIRC.
> > 
> > > So, what about changing the queue sizes just for virtio-blk-device and
> > > virtio-scsi-device?
> > 
> > Hmm that would break ability to migrate between userspace and vhost
> > backends, would it not?
> I'm not sure I've understood what you meant.
> Just for the record, I was going to change virtqueue-size for
> virtio-blk-device and virtio-scsi-device



If virtqueue size is different between virtio-blk-device and
vhost-user-blk then one can not migrate the former to the later.

> since they can adjust seg_max to
> the specified queue size and I don't want to touch vhost-s and vhost-user-s
> since they don't have adjustable seg_max for now.
> 
> Denis



If I just grep seg_max_adjust I see them for vhost devices too.
Is that code ineffective somehow? What's it doing then?

> > 
> > 
> > > Denis
> > > 
> > > > > It seems to be so. We also have the test checking those settings:
> > > > > tests/acceptance/virtio_seg_max_adjust.py
> > > > > For now 

[PATCH] tests/acceptance/virtio_check_params: Only disable the test on CI

2020-02-11 Thread Philippe Mathieu-Daudé
Commit 2d6a6e238a incorrectly totally disabled this test.
The original intention was to only disable on continuous integration.

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/acceptance/virtio_check_params.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/virtio_check_params.py 
b/tests/acceptance/virtio_check_params.py
index 87e6c839d1..015582cf9c 100644
--- a/tests/acceptance/virtio_check_params.py
+++ b/tests/acceptance/virtio_check_params.py
@@ -25,7 +25,7 @@ import logging
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.machine import QEMUMachine
 from avocado_qemu import Test
-from avocado import skip
+from avocado import skipIf
 
 #list of machine types and virtqueue properties to test
 VIRTIO_SCSI_PROPS = {'seg_max_adjust': 'seg_max_adjust'}
@@ -117,7 +117,7 @@ class VirtioMaxSegSettingsCheck(Test):
 return True
 return False
 
-@skip("break multi-arch CI")
+@skipIf(os.getenv('CONTINUOUS_INTEGRATION'), 'Break multi-arch CI')
 def test_machine_types(self):
 # collect all machine types except 'none', 'isapc', 'microvm'
 with QEMUMachine(self.qemu_bin) as vm:
-- 
2.21.1




Re: [PATCH v2 1/5] virtio-mmio: add notify feature for per-queue

2020-02-11 Thread Michael S. Tsirkin
On Mon, Feb 10, 2020 at 05:05:17PM +0800, Zha Bin wrote:
> From: Liu Jiang 
> 
> The standard virtio-mmio devices use notification register to signal
> backend. This will cause vmexits and slow down the performance when we
> passthrough the virtio-mmio devices to guest virtual machines.
> We proposed to update virtio over MMIO spec to add the per-queue
> notify feature VIRTIO_F_MMIO_NOTIFICATION[1]. It can allow the VMM to
> configure notify location for each queue.
> 
> [1] https://lkml.org/lkml/2020/1/21/31
> 
> Signed-off-by: Liu Jiang 
> Co-developed-by: Zha Bin 
> Signed-off-by: Zha Bin 
> Co-developed-by: Jing Liu 
> Signed-off-by: Jing Liu 
> Co-developed-by: Chao Peng 
> Signed-off-by: Chao Peng 

So this is for pass-through for nested virt?
OK but let's split this out please, and benchmark separately.


> ---
>  drivers/virtio/virtio_mmio.c   | 37 +++--
>  include/uapi/linux/virtio_config.h |  8 +++-
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 97d5725..1733ab97 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -90,6 +90,9 @@ struct virtio_mmio_device {
>   /* a list of queues so we can dispatch IRQs */
>   spinlock_t lock;
>   struct list_head virtqueues;
> +
> + unsigned short notify_base;
> + unsigned short notify_multiplier;
>  };
>  
>  struct virtio_mmio_vq_info {
> @@ -98,6 +101,9 @@ struct virtio_mmio_vq_info {
>  
>   /* the list node for the virtqueues list */
>   struct list_head node;
> +
> + /* Notify Address*/
> + unsigned int notify_addr;
>  };
>  
>  
> @@ -119,13 +125,23 @@ static u64 vm_get_features(struct virtio_device *vdev)
>   return features;
>  }
>  
> +static void vm_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + if (features & BIT_ULL(VIRTIO_F_MMIO_NOTIFICATION))
> + __virtio_set_bit(vdev, VIRTIO_F_MMIO_NOTIFICATION);
> +}
> +
>  static int vm_finalize_features(struct virtio_device *vdev)
>  {
>   struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> + u64 features = vdev->features;
>  
>   /* Give virtio_ring a chance to accept features. */
>   vring_transport_features(vdev);
>  
> + /* Give virtio_mmio a chance to accept features. */
> + vm_transport_features(vdev, features);
> +
>   /* Make sure there is are no mixed devices */
>   if (vm_dev->version == 2 &&
>   !__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> @@ -272,10 +288,13 @@ static void vm_reset(struct virtio_device *vdev)
>  static bool vm_notify(struct virtqueue *vq)
>  {
>   struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev);
> + struct virtio_mmio_vq_info *info = vq->priv;
>  
> - /* We write the queue's selector into the notification register to
> + /* We write the queue's selector into the Notify Address to
>* signal the other end */
> - writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
> + if (info)
> + writel(vq->index, vm_dev->base + info->notify_addr);
> +
>   return true;
>  }
>  
> @@ -434,6 +453,12 @@ static struct virtqueue *vm_setup_vq(struct 
> virtio_device *vdev, unsigned index,
>   vq->priv = info;
>   info->vq = vq;
>  
> + if (__virtio_test_bit(vdev, VIRTIO_F_MMIO_NOTIFICATION))
> + info->notify_addr = vm_dev->notify_base +
> + vm_dev->notify_multiplier * vq->index;
> + else
> + info->notify_addr = VIRTIO_MMIO_QUEUE_NOTIFY;
> +
>   spin_lock_irqsave(&vm_dev->lock, flags);
>   list_add(&info->node, &vm_dev->virtqueues);
>   spin_unlock_irqrestore(&vm_dev->lock, flags);
> @@ -471,6 +496,14 @@ static int vm_find_vqs(struct virtio_device *vdev, 
> unsigned nvqs,
>   return irq;
>   }
>  
> + if (__virtio_test_bit(vdev, VIRTIO_F_MMIO_NOTIFICATION)) {
> + unsigned int notify = readl(vm_dev->base +
> + VIRTIO_MMIO_QUEUE_NOTIFY);
> +
> + vm_dev->notify_base = notify & 0x;
> + vm_dev->notify_multiplier = (notify >> 16) & 0x;
> + }
> +
>   err = request_irq(irq, vm_interrupt, IRQF_SHARED,
>   dev_name(&vdev->dev), vm_dev);
>   if (err)
> diff --git a/include/uapi/linux/virtio_config.h 
> b/include/uapi/linux/virtio_config.h
> index ff8e7dc..5d93c01 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -52,7 +52,7 @@
>   * rest are per-device feature bits.
>   */
>  #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END   38
> +#define VIRTIO_TRANSPORT_F_END   40
>  
>  #ifndef VIRTIO_CONFIG_NO_LEGACY
>  /* Do we get callbacks when the ring is completely used, even if we've
> @@ -88,4 +88,10 @@
>   * Does the device support Single Ro

Re: [PATCH] tests/acceptance/virtio_check_params: Only disable the test on CI

2020-02-11 Thread Cornelia Huck
On Tue, 11 Feb 2020 11:49:38 +0100
Philippe Mathieu-Daudé  wrote:

> Commit 2d6a6e238a incorrectly totally disabled this test.
> The original intention was to only disable on continuous integration.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/acceptance/virtio_check_params.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/virtio_check_params.py 
> b/tests/acceptance/virtio_check_params.py
> index 87e6c839d1..015582cf9c 100644
> --- a/tests/acceptance/virtio_check_params.py
> +++ b/tests/acceptance/virtio_check_params.py
> @@ -25,7 +25,7 @@ import logging
>  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 
> 'python'))
>  from qemu.machine import QEMUMachine
>  from avocado_qemu import Test
> -from avocado import skip
> +from avocado import skipIf
>  
>  #list of machine types and virtqueue properties to test
>  VIRTIO_SCSI_PROPS = {'seg_max_adjust': 'seg_max_adjust'}
> @@ -117,7 +117,7 @@ class VirtioMaxSegSettingsCheck(Test):
>  return True
>  return False
>  
> -@skip("break multi-arch CI")
> +@skipIf(os.getenv('CONTINUOUS_INTEGRATION'), 'Break multi-arch CI')
>  def test_machine_types(self):
>  # collect all machine types except 'none', 'isapc', 'microvm'
>  with QEMUMachine(self.qemu_bin) as vm:

Acked-by: Cornelia Huck 




Re: [GSoC/Outreachy] Arduino complete setup visualization and emulation

2020-02-11 Thread Stefan Hajnoczi
On Mon, Feb 10, 2020 at 08:58:28PM +0100, Philippe Mathieu-Daudé wrote:

Cool, thanks for proposing this idea!

> [*] Goal:
> 
> Be able to use a visual virtual arduino board, and program it with
> the Arduino IDE. The result should be easily usable by newcomers to
> the Arduino world.

Out of curiosity, how does this compare to existing Arduino simulators?

> [*] Prerequisite:
> 
> - AVR port and Arduino machines merged upstream
> - AVR flash device working (for firmware upload via IDE)

How likely is it that these dependencies will be merged by May 18th
(start of the coding period)?  If they are not merged then the intern
will not be able to get their own patches into qemu.git.

> This works applies to a specific circuit configuration represented as
> a netlist.
> 
> [*] Deliverables
> 
> - IDE Integration
>   Configure QEMU with the Arduino IDE (using chardev UART0).

This (https://www.arduino.cc/en/Main/Software) Java program?  Please add
links to relevant code bases, hardware specs, file formats, etc so that
someone considering the project idea can research it.

I also see Python mentioned in this project idea.  Does the intern need
to be fluent in C, Python, and Java?  Please include skill/language
requirements in the text.

>   Compile program and upload via serial.
> 
> - UI: Python:
>   Connect UART1 (via QMP or chardev), display as textbox
>   (input is not important at this point).
> 
> - QEMU: GPIO
>   Produce script to extract GPIO devices used from the netlist.
>   Configure QEMU devices to use the previous names/values.
>   Publish GPIO events (name, tension as float) via a QMP socket
>   (JSON form?).
>   Write test which runs FreeRTOS test to generates a stable output.
> 
> - UI interface (Python)
>   Connect to the QMP socket and display the GPIO events.
>   Now GPIOs are connected to LEDs. Represent graphical LEDS as ON/OFF.
>   Add an oscilloscope representation (matplotlib widget). Each GPIO
>   can be plugged into the oscilloscope channels.
>   Add Switch and PushButton to the UI, generating QMP events which
>   trigger GPIO input.
>   Add push button for arduino reset (already on board) signaling the
>   core, and switch for general power (for QEMU shutdown and start).
> 
> - Test with the arduino examples
>   Basic: "Blink: Turn an LED on and off."
> 
> - QEMU: PWM
>   Modify script to extract PWM devices used from the netlist.
>   Configure QEMU devices to use the previous names/values.
>   Use QEMU sound API to generate a stream of PWM values (as a wav).
>   Add a QMP command to lookup the PWM wav stream.
>   Write a FreeRTOS test producing a sinusoidal via PWM, verify the
>   wav form.
> 
> - UI interface (Python)
>   Lookup wav stream via QMP socket, connect to it, display to
>   oscilloscope view.
>   Add graphical representation of the LED intensity to the LED.
> 
> - Test with the arduino examples
>   Analog: "Fading: Use an analog output (PWM pin) to fade an LED."

The tasks above could already take 12 weeks.  Especially the new QMP
commands and the UI code could be non-trivial.

I think newcomers will have a hard time designing QMP commands.  It
would help to provide the QMP command documentation so the intern
doesn't need to make design decisions in a space they are unfamiliar
with.

When Steffen Görtz experimented with similar things using the micro:bit
emulation I remember emulation timing issues were a little tricky.
Emitting LED/PWM output and displaying it without timing glitches is
non-trivial since QEMU is not very precise and guest software might be
bit banging.

> - QEMU: ADC
>   Modify script to extract ADC devices used from the netlist.
>   Similarly to the PWM, use sound wav stream to read ADC samples.
> 
> - UI: Python
>   Add a textbox to set the ambient temperature (A thermometer is
>   connected to some ADC pin).
>   Use slider to set the tension sampled by the ADC (as a potentiometer)
> 
> - Test with the arduino examples
>   Analog: "Analog Input: Use a potentiometer to control the blinking
>   of an LED."
> 
> - QEMU: Other communication protocols
>   Modify script to extract RTC (via I2C) and SD card (via SPI) from
>   the netlist.
> 
> - Propose examples to Arduino IDE for these use cases.
> 
> - QEMU: Match physical electrical characteristics (extra)
>   Use imprecise VOL/VOH output
>   Check input within VIL/VIH range
>   Mark input dead when out of range
> 
> - Extra (fun):
>   Connect 2 QEMU Arduino interacting with each other
> 
> - UI: Python (extra++):
>   Add Seven-Segment Display
>   Add SSD1306 128×32 display controller or Nokia 5110 Graphic LCD
>   Propose examples to Arduino IDE for these use cases.

The scope of the project seems large for 12 weeks.  It could scare off
applicants or be unrealistic for an intern without lots of experience.
You could remove some tasks from the project idea and if the intern is
really quick then you can always give them additional tasks later.

> Co-mentor: Philippe Mathieu-Daudé 
> Co-mentor: Joaquín De Andr

Re: [PATCH v5 2/8] migration: Add support for modules

2020-02-11 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> So we don't have to compile everything in, or have ifdefs
> 
> Signed-off-by: Juan Quintela 

As far as I can tell this matches the way all the rest of the module
stuff works, so:

Reviewed-by: Dr. David Alan Gilbert 

(Although I wish it was documented somewhere).

Dave

> ---
>  include/qemu/module.h | 2 ++
>  vl.c  | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 65ba596e46..907cb5c0a5 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -40,6 +40,7 @@ static void __attribute__((constructor)) do_qemu_init_ ## 
> function(void)\
>  #endif
>  
>  typedef enum {
> +MODULE_INIT_MIGRATION,
>  MODULE_INIT_BLOCK,
>  MODULE_INIT_OPTS,
>  MODULE_INIT_QOM,
> @@ -56,6 +57,7 @@ typedef enum {
>  #define xen_backend_init(function) module_init(function, \
> MODULE_INIT_XEN_BACKEND)
>  #define libqos_init(function) module_init(function, MODULE_INIT_LIBQOS)
> +#define migration_init(function) module_init(function, MODULE_INIT_MIGRATION)
>  
>  #define block_module_load_one(lib) module_load_one("block-", lib)
>  #define ui_module_load_one(lib) module_load_one("ui-", lib)
> diff --git a/vl.c b/vl.c
> index b0f52c4d6e..9f8577955a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2890,6 +2890,7 @@ int main(int argc, char **argv, char **envp)
>  qemu_init_exec_dir(argv[0]);
>  
>  module_call_init(MODULE_INIT_QOM);
> +module_call_init(MODULE_INIT_MIGRATION);
>  
>  qemu_add_opts(&qemu_drive_opts);
>  qemu_add_drive_opts(&qemu_legacy_drive_opts);
> -- 
> 2.24.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




Re: [RFC 1/2] tpm: Let the TPM TIS device be usable on ARM

2020-02-11 Thread Peter Maydell
On Tue, 11 Feb 2020 at 08:35, Auger Eric  wrote:
>
> Hi Philippe,
>
> On 2/11/20 9:25 AM, Philippe Mathieu-Daudé wrote:
> > You don't need much to remove the RFC tag:
> >
> > - rename TYPE_TPM_TIS as TYPE_TPM_TIS_ISA
> > - rename TPMState as TPMCommonState, add an abstract TYPE_TPM_TIS
> > parent, let TYPE_TPM_TIS_ISA be a child
> > - add TYPE_TPM_TIS_SYSBUS also child.
> Yes I tried my luck without too much hopes ;-)

There should be a few existing examples in the tree
of devices that we provide in a sysbus and also
an isa or pci flavour, that you can use as templates
for how to structure the device.

-- PMM



Re: [PATCH v2 0/5] virtio mmio specification enhancement

2020-02-11 Thread Michael S. Tsirkin
On Tue, Feb 11, 2020 at 04:05:41PM +, Chao Peng wrote:
> On Mon, Feb 10, 2020 at 06:44:50AM -0500, Michael S. Tsirkin wrote:
> > On Mon, Feb 10, 2020 at 05:05:16PM +0800, Zha Bin wrote:
> > > We have compared the number of files and the lines of code between
> > > virtio-mmio and virio-pci.
> > > 
> > >   Virtio-PCI  Virtio-MMIO 
> > >   number of files(Linux)  161 1
> > >   lines of code(Linux)78237   538
> > 
> > 
> > 
> > Something's very wrong here. virtio PCI is 161 files?
> > Are you counting the whole PCI subsystem?
> 
> Right, that is just a rough statistics.

Please try not to make them look so wrong then.
E.g. you don't include drivers/base/platform-msi.c for
mmio do you? Your patch brings a bunch of code in there.

> Surely enough, some drivers will
> never get enabled in a typcial config.
> 
> > Sure enough:
> > 
> > $ find drivers/pci -name '*c' |wc -l
> > 150
> 
> and plus:
> $ find arch/x86/pci/ -name '*c' |wc -l
> 22

But what's the point? This is code that is maintained by PCI core
people anyway.

> > 
> > That's not reasonable, this includes a bunch of drivers that
> > never run on a typical hypervisor.
> > 
> > MMIO is also not as small as you are trying to show:
> > 
> > $ cloc drivers/virtio/virtio_mmio.c include/uapi/linux/virtio_mmio.h
> >2 text files.
> >2 unique files.  
> >0 files ignored.
> > 
> > github.com/AlDanial/cloc v 1.82  T=0.01 s (230.7 files/s, 106126.5 lines/s)
> > ---
> > Language files  blankcomment   
> > code
> > ---
> > C1144100
> > 535
> > C/C++ Header 1 39 66
> >  36
> > ---
> > SUM: 2183166
> > 571
> > ---
> > 
> > 
> > I don't doubt MMIO is smaller than PCI. Of course that's because it has
> > no features to speak of - just this patch already doubles it's size. If
> > we keep doing that because we want the features then they will reach
> > the same size in about 4 iterations.
> 
> Since current virtio-mmio size is small enough, so adding any notable
> feature would easily double it.

But really unlike PCI this is just PV stuff that is not reused by
anyone. We end up maintaining all this by ourselves.

> I have no objection that it may one day
> reach the same level of PCI, but in this patch some are actually
> generic changes and for MSI specific code we provide the option to
> confige away.
> 
> Thanks,
> Chao

The option will make it fall down at runtime but
it does not actually seem to remove all of the overhead.



> > 
> > 
> > -- 
> > MST




Re: [qemu-web PATCH] Force background color

2020-02-11 Thread Thomas Huth
On 08/02/2020 01.46, athurh wrote:
> Browsers can have a different default background color defined.
> Setting the background image overwrites other background definitions.
> ---
>  assets/css/style-desktop.css | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/assets/css/style-desktop.css b/assets/css/style-desktop.css
> index 44ea0c7..b6c5aa1 100644
> --- a/assets/css/style-desktop.css
> +++ b/assets/css/style-desktop.css
> @@ -33,12 +33,14 @@
>   {
>   /* 90% transparent */
>   background: url(../images/qemu_head_200.png) no-repeat fixed 
> 2em 80px;
> + background-color: #FFF;
>   }
>  
>   body.homepage
>   {
>   /* 90% transparent */
>   background: url(../images/qemu_head_400_faint.png) no-repeat 
> fixed center 60%;
> + background-color: #FFF;
>   }
>  
>   body,input,textarea,select
> 

Thanks, applied.

 Thomas




Re: [PATCH] tests/acceptance: Add boot tests for sh4 and mips64 QEMU advent calendar images

2020-02-11 Thread Thomas Huth
On 11/02/2020 10.42, Thomas Huth wrote:
> Now that we can select the second serial console in the acceptance tests
> (see commit 746f244d9720 "Allow to use other serial consoles than default"),
> we can also test the sh4 image from the QEMU advent calendar 2018.
> 
> And another recent commit (ec860426dfbe "Fix handling of LL/SC instructions")
> fixed a problem with qemu-system-mips64, so the mips64 from the advent
> calendar now works again and can be used for acceptance testing, too.
> 
> Signed-off-by: Thomas Huth 
> ---
>  .travis.yml|  2 +-
>  tests/acceptance/boot_linux_console.py | 23 +--
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 5887055951..71a0097878 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -313,7 +313,7 @@ matrix:
>  # Acceptance (Functional) tests
>  - name: "GCC check-acceptance"
>env:
> -- 
> CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
> +- 
> CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
>  - TEST_CMD="make check-acceptance"
>after_script:
>  - python3 -c 'import json; r = 
> json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) 
> for t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> index 34d37eba3b..a38ee004b1 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -591,12 +591,12 @@ class BootLinuxConsole(Test):
>  console_pattern = 'No filesystem could mount root'
>  self.wait_for_console_pattern(console_pattern)
>  
> -def do_test_advcal_2018(self, day, tar_hash, kernel_name):
> +def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
>  tar_url = ('https://www.qemu-advent-calendar.org'
> '/2018/download/day' + day + '.tar.xz')
>  file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
>  archive.extract(file_path, self.workdir)
> -self.vm.set_console()
> +self.vm.set_console(console_index=console)
>  self.vm.add_args('-kernel',
>   self.workdir + '/day' + day + '/' + kernel_name)
>  self.vm.launch()
> @@ -670,6 +670,25 @@ class BootLinuxConsole(Test):
>  self.vm.add_args('-M', 'graphics=off')
>  self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')
>  
> +def test_mips64_malta(self):
> +"""
> +:avocado: tags=arch:mips64
> +:avocado: tags=machine:malta
> +:avocado: tags=endian:big
> +"""
> +tar_hash = '81b030201ec3f28cb1925297f6017d3a20d7ced5'
> +self.vm.add_args('-hda', self.workdir + '/day22/' + 'ri-li.qcow2',
> + '-append', 'root=/dev/hda')
> +self.do_test_advcal_2018('22', tar_hash, 'vmlinux')

It's maybe nicer to place the malta test alphabetically earlier ... I'll
send a v2...

> +def test_sh4_r2d(self):
> +"""
> +:avocado: tags=arch:sh4
> +:avocado: tags=machine:r2d
> +"""
> +tar_hash = 'fe06a4fd8ccbf2e27928d64472939d47829d4c7e'
> +self.do_test_advcal_2018('09', tar_hash, 'zImage', console=1)
> +
>  def test_sparc_ss20(self):
>  """
>  :avocado: tags=arch:sparc
> 

 Thomas




Re: [PATCH] migration/rdma: rdma_accept_incoming_migration fix error handling

2020-02-11 Thread Juan Quintela
"Dr. David Alan Gilbert (git)"  wrote:
> From: "Dr. David Alan Gilbert" 
>
> rdma_accept_incoming_migration is called from an fd handler and
> can't return an Error * anywhere.
> Currently it's leaking Error's in errp/local_err - there's
> no point putting them in there unless we can report them.
>
> Turn most into fprintf's, and the last into an error_reportf_err
> where it's coming up from another function.
>
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  migration/rdma.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 2379b8345b..f67161c98f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3980,13 +3980,13 @@ static void rdma_accept_incoming_migration(void 
> *opaque)
>  RDMAContext *rdma = opaque;
>  int ret;
>  QEMUFile *f;
> -Error *local_err = NULL, **errp = &local_err;
> +Error *local_err = NULL;
>  
>  trace_qemu_rdma_accept_incoming_migration();
>  ret = qemu_rdma_accept(rdma);
>  
>  if (ret) {
> -ERROR(errp, "RDMA Migration initialization failed!");
> +fprintf(stderr, "RDMA ERROR: Migration initialization failed");
>  return;

this should end in "\n" right?

Appart from that.

Reviewed-by: Juan Quintela 

BTW, I can include the \n when I queue the patch, no need to resend


>  }
>  
> @@ -3998,13 +3998,16 @@ static void rdma_accept_incoming_migration(void 
> *opaque)
>  
>  f = qemu_fopen_rdma(rdma, "rb");
>  if (f == NULL) {
> -ERROR(errp, "could not qemu_fopen_rdma!");
> +fprintf(stderr, "RDMA ERROR: could not qemu_fopen_rdma");
>  qemu_rdma_cleanup(rdma);
>  return;
>  }
>  
>  rdma->migration_started_on_destination = 1;
> -migration_fd_process_incoming(f, errp);
> +migration_fd_process_incoming(f, &local_err);
> +if (local_err) {
> +error_reportf_err(local_err, "RDMA ERROR:");
> +}
>  }
>  
>  void rdma_start_incoming_migration(const char *host_port, Error **errp)




[PATCH] tracing: only allow -trace to override -D if set

2020-02-11 Thread Alex Bennée
Otherwise any -D settings the user may have made get ignored.

Signed-off-by: Alex Bennée 
---
 trace/control.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/trace/control.c b/trace/control.c
index 6c775e68eba..2ffe0008184 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -226,10 +226,15 @@ void trace_init_file(const char *file)
 #ifdef CONFIG_TRACE_SIMPLE
 st_set_trace_file(file);
 #elif defined CONFIG_TRACE_LOG
-/* If both the simple and the log backends are enabled, "--trace file"
- * only applies to the simple backend; use "-D" for the log backend.
+/*
+ * If both the simple and the log backends are enabled, "--trace file"
+ * only applies to the simple backend; use "-D" for the log
+ * backend. However we should only override -D if we actually have
+ * something to override it with.
  */
-qemu_set_log_filename(file, &error_fatal);
+if (file) {
+qemu_set_log_filename(file, &error_fatal);
+}
 #else
 if (file) {
 fprintf(stderr, "error: --trace file=...: "
-- 
2.20.1




Re: [PATCH v2 4/5] virtio-mmio: add MSI interrupt feature support

2020-02-11 Thread Michael S. Tsirkin
On Mon, Feb 10, 2020 at 05:05:20PM +0800, Zha Bin wrote:
> From: Liu Jiang 
> 
> Userspace VMMs (e.g. Qemu microvm, Firecracker) take advantage of using
> virtio over mmio devices as a lightweight machine model for modern
> cloud. The standard virtio over MMIO transport layer only supports one
> legacy interrupt, which is much heavier than virtio over PCI transport
> layer using MSI. Legacy interrupt has long work path and causes specific
> VMExits in following cases, which would considerably slow down the
> performance:
> 
> 1) read interrupt status register
> 2) update interrupt status register
> 3) write IOAPIC EOI register
> 
> We proposed to add MSI support for virtio over MMIO via new feature
> bit VIRTIO_F_MMIO_MSI[1] which increases the interrupt performance.
> 
> With the VIRTIO_F_MMIO_MSI feature bit supported, the virtio-mmio MSI
> uses msi_sharing[1] to indicate the event and vector mapping.
> Bit 1 is 0: device uses non-sharing and fixed vector per event mapping.
> Bit 1 is 1: device uses sharing mode and dynamic mapping.
> 
> [1] https://lkml.org/lkml/2020/1/21/31
> 
> Signed-off-by: Liu Jiang 
> Co-developed-by: Zha Bin 
> Signed-off-by: Zha Bin 
> Co-developed-by: Jing Liu 
> Signed-off-by: Jing Liu 
> Co-developed-by: Chao Peng 
> Signed-off-by: Chao Peng 
> ---
>  drivers/virtio/virtio_mmio.c| 299 
> ++--
>  drivers/virtio/virtio_mmio_common.h |   8 +
>  drivers/virtio/virtio_mmio_msi.h|  82 ++
>  include/uapi/linux/virtio_config.h  |   7 +-
>  include/uapi/linux/virtio_mmio.h|  31 
>  5 files changed, 411 insertions(+), 16 deletions(-)

So that's > 100 exatra lines in headers that you for some reason
don't count when comparing code size :).


I think an effort can be made to reduce the complexity here.
Otherwise you will end up with a clone of PCI
sooner than you think. In fact, disabling all legacy:

$ wc -l drivers/virtio/virtio_pci_modern.c
734 drivers/virtio/virtio_pci_modern.c
$ wc -l drivers/virtio/virtio_pci_common.c
635 drivers/virtio/virtio_pci_common.c

So you have almost matched that with your patch.


> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 41e1c93..b24ce21 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -67,8 +67,7 @@
>  #include 
>  #include 
>  #include "virtio_mmio_common.h"
> -
> -
> +#include "virtio_mmio_msi.h"
>  
>  /* The alignment to use between consumer and producer parts of vring.
>   * Currently hardcoded to the page size. */
> @@ -85,9 +84,13 @@ struct virtio_mmio_vq_info {
>  
>   /* Notify Address*/
>   unsigned int notify_addr;
> -};
>  
> + /* MSI vector (or none) */
> + unsigned int msi_vector;
> +};
>  
> +static void vm_free_msi_irqs(struct virtio_device *vdev);
> +static int vm_request_msi_vectors(struct virtio_device *vdev, int nirqs);
>  
>  /* Configuration interface */
>  
> @@ -110,6 +113,9 @@ static void vm_transport_features(struct virtio_device 
> *vdev, u64 features)
>  {
>   if (features & BIT_ULL(VIRTIO_F_MMIO_NOTIFICATION))
>   __virtio_set_bit(vdev, VIRTIO_F_MMIO_NOTIFICATION);
> +
> + if (features & BIT_ULL(VIRTIO_F_MMIO_MSI))
> + __virtio_set_bit(vdev, VIRTIO_F_MMIO_MSI);
>  }
>  
>  static int vm_finalize_features(struct virtio_device *vdev)
> @@ -307,7 +313,33 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
>   return ret;
>  }
>  
> +static irqreturn_t vm_vring_interrupt(int irq, void *opaque)
> +{
> + struct virtio_mmio_device *vm_dev = opaque;
> + struct virtio_mmio_vq_info *info;
> + irqreturn_t ret = IRQ_NONE;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&vm_dev->lock, flags);
> + list_for_each_entry(info, &vm_dev->virtqueues, node) {
> + if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
> + ret = IRQ_HANDLED;
> + }
> + spin_unlock_irqrestore(&vm_dev->lock, flags);
> +
> + return ret;
> +}
> +
> +
> +/* Handle a configuration change */
> +static irqreturn_t vm_config_changed(int irq, void *opaque)
> +{
> + struct virtio_mmio_device *vm_dev = opaque;
> +
> + virtio_config_changed(&vm_dev->vdev);
>  
> + return IRQ_HANDLED;
> +}
>  
>  static void vm_del_vq(struct virtqueue *vq)
>  {
> @@ -316,6 +348,15 @@ static void vm_del_vq(struct virtqueue *vq)
>   unsigned long flags;
>   unsigned int index = vq->index;
>  
> + if (vm_dev->msi_enabled && !vm_dev->msi_share) {
> + if (info->msi_vector != VIRTIO_MMIO_MSI_NO_VECTOR) {
> + int irq = mmio_msi_irq_vector(&vq->vdev->dev,
> + info->msi_vector);
> +
> + free_irq(irq, vq);
> + }
> + }
> +
>   spin_lock_irqsave(&vm_dev->lock, flags);
>   list_del(&info->node);
>   spin_unlock_irqrestore(&vm_dev->lock, flags);
> @@ -334,20 +375,56 @@ static void vm_del_vq(struct virtqueue *vq)

Re: [PATCH v2 5/5] x86: virtio-mmio: support virtio-mmio with MSI for x86

2020-02-11 Thread Michael S. Tsirkin
On Mon, Feb 10, 2020 at 05:05:21PM +0800, Zha Bin wrote:
> From: Liu Jiang 
> 
> virtio-mmio supports a generic MSI irq domain for all archs. This
> patch adds the x86 architecture support.
> 
> Signed-off-by: Liu Jiang 
> Co-developed-by: Zha Bin 
> Signed-off-by: Zha Bin 
> Co-developed-by: Jing Liu 
> Signed-off-by: Jing Liu 
> Co-developed-by: Chao Peng 
> Signed-off-by: Chao Peng 
> ---
>  arch/x86/kernel/apic/msi.c | 11 ++-

Patches like this need to CC x86 maintainers.


>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
> index 159bd0c..2fcd602 100644
> --- a/arch/x86/kernel/apic/msi.c
> +++ b/arch/x86/kernel/apic/msi.c
> @@ -45,7 +45,11 @@ static void __irq_msi_compose_msg(struct irq_cfg *cfg, 
> struct msi_msg *msg)
>   MSI_DATA_VECTOR(cfg->vector);
>  }
>  
> -static void irq_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
> +/*
> + * x86 PCI-MSI/HPET/DMAR related method.
> + * Also can be used as arch specific method for virtio-mmio MSI.
> + */
> +void irq_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
>  {
>   __irq_msi_compose_msg(irqd_cfg(data), msg);
>  }
> @@ -166,6 +170,11 @@ static void irq_msi_update_msg(struct irq_data *irqd, 
> struct irq_cfg *cfg)
>   return ret;
>  }
>  
> +struct irq_domain *arch_msi_root_irq_domain(void)
> +{
> + return x86_vector_domain;
> +}
> +
>  /*
>   * IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
>   * which implement the MSI or MSI-X Capability Structure.


So there's a new non-static functions with no callers here,
and a previously static irq_msi_compose_msg is not longer
static. No callers so how does this help anyone?
And how does this achieve the goal of enabling virtio mmio?


> -- 
> 1.8.3.1




Re: [PATCH] migration/rdma: rdma_accept_incoming_migration fix error handling

2020-02-11 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)"  wrote:
> > From: "Dr. David Alan Gilbert" 
> >
> > rdma_accept_incoming_migration is called from an fd handler and
> > can't return an Error * anywhere.
> > Currently it's leaking Error's in errp/local_err - there's
> > no point putting them in there unless we can report them.
> >
> > Turn most into fprintf's, and the last into an error_reportf_err
> > where it's coming up from another function.
> >
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  migration/rdma.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index 2379b8345b..f67161c98f 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -3980,13 +3980,13 @@ static void rdma_accept_incoming_migration(void 
> > *opaque)
> >  RDMAContext *rdma = opaque;
> >  int ret;
> >  QEMUFile *f;
> > -Error *local_err = NULL, **errp = &local_err;
> > +Error *local_err = NULL;
> >  
> >  trace_qemu_rdma_accept_incoming_migration();
> >  ret = qemu_rdma_accept(rdma);
> >  
> >  if (ret) {
> > -ERROR(errp, "RDMA Migration initialization failed!");
> > +fprintf(stderr, "RDMA ERROR: Migration initialization failed");
> >  return;
> 
> this should end in "\n" right?

Oops thanks; and in the one below.

> Appart from that.
> 
> Reviewed-by: Juan Quintela 
> 
> BTW, I can include the \n when I queue the patch, no need to resend

Thanks

Dave

> 
> >  }
> >  
> > @@ -3998,13 +3998,16 @@ static void rdma_accept_incoming_migration(void 
> > *opaque)
> >  
> >  f = qemu_fopen_rdma(rdma, "rb");
> >  if (f == NULL) {
> > -ERROR(errp, "could not qemu_fopen_rdma!");
> > +fprintf(stderr, "RDMA ERROR: could not qemu_fopen_rdma");
> >  qemu_rdma_cleanup(rdma);
> >  return;
> >  }
> >  
> >  rdma->migration_started_on_destination = 1;
> > -migration_fd_process_incoming(f, errp);
> > +migration_fd_process_incoming(f, &local_err);
> > +if (local_err) {
> > +error_reportf_err(local_err, "RDMA ERROR:");
> > +}
> >  }
> >  
> >  void rdma_start_incoming_migration(const char *host_port, Error **errp)
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




[PATCH v2] tests/acceptance: Add boot tests for sh4 and mips64 QEMU advent calendar images

2020-02-11 Thread Thomas Huth
Now that we can select the second serial console in the acceptance tests
(see commit 746f244d9720 "Allow to use other serial consoles than default"),
we can also test the sh4 image from the QEMU advent calendar 2018.

And another recent commit (ec860426dfbe "Fix handling of LL/SC instructions")
fixed a problem with qemu-system-mips64, so the mips64 from the advent
calendar now works again and can be used for acceptance testing, too.

Signed-off-by: Thomas Huth 
---
 v2: Moved the mips test earlier so that it matches alphabetially the
 other advent calendar tests

 .travis.yml|  2 +-
 tests/acceptance/boot_linux_console.py | 23 +--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 5887055951..71a0097878 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -313,7 +313,7 @@ matrix:
 # Acceptance (Functional) tests
 - name: "GCC check-acceptance"
   env:
-- 
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
+- 
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
 - TEST_CMD="make check-acceptance"
   after_script:
 - python3 -c 'import json; r = 
json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for 
t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 34d37eba3b..a8486d6231 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -591,12 +591,12 @@ class BootLinuxConsole(Test):
 console_pattern = 'No filesystem could mount root'
 self.wait_for_console_pattern(console_pattern)
 
-def do_test_advcal_2018(self, day, tar_hash, kernel_name):
+def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
 tar_url = ('https://www.qemu-advent-calendar.org'
'/2018/download/day' + day + '.tar.xz')
 file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
 archive.extract(file_path, self.workdir)
-self.vm.set_console()
+self.vm.set_console(console_index=console)
 self.vm.add_args('-kernel',
  self.workdir + '/day' + day + '/' + kernel_name)
 self.vm.launch()
@@ -627,6 +627,17 @@ class BootLinuxConsole(Test):
 tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
 self.do_test_advcal_2018('17', tar_hash, 'ballerina.bin')
 
+def test_mips64_malta(self):
+"""
+:avocado: tags=arch:mips64
+:avocado: tags=machine:malta
+:avocado: tags=endian:big
+"""
+tar_hash = '81b030201ec3f28cb1925297f6017d3a20d7ced5'
+self.vm.add_args('-hda', self.workdir + '/day22/' + 'ri-li.qcow2',
+ '-append', 'root=/dev/hda')
+self.do_test_advcal_2018('22', tar_hash, 'vmlinux')
+
 def test_or1k_sim(self):
 """
 :avocado: tags=arch:or1k
@@ -670,6 +681,14 @@ class BootLinuxConsole(Test):
 self.vm.add_args('-M', 'graphics=off')
 self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')
 
+def test_sh4_r2d(self):
+"""
+:avocado: tags=arch:sh4
+:avocado: tags=machine:r2d
+"""
+tar_hash = 'fe06a4fd8ccbf2e27928d64472939d47829d4c7e'
+self.do_test_advcal_2018('09', tar_hash, 'zImage', console=1)
+
 def test_sparc_ss20(self):
 """
 :avocado: tags=arch:sparc
-- 
2.18.1




Re: [PATCH v2 3/5] virtio-mmio: create a generic MSI irq domain

2020-02-11 Thread Michael S. Tsirkin
On Mon, Feb 10, 2020 at 05:05:19PM +0800, Zha Bin wrote:
> From: Liu Jiang 
> 
> Create a generic irq domain for all architectures which
> supports virtio-mmio. The device offering VIRTIO_F_MMIO_MSI
> feature bit can use this irq domain.
> 
> Signed-off-by: Liu Jiang 
> Co-developed-by: Zha Bin 
> Signed-off-by: Zha Bin 
> Co-developed-by: Jing Liu 
> Signed-off-by: Jing Liu 
> Co-developed-by: Chao Peng 
> Signed-off-by: Chao Peng 
> ---
>  drivers/base/platform-msi.c  |  4 +-
>  drivers/virtio/Kconfig   |  9 
>  drivers/virtio/virtio_mmio_msi.h | 93 
> 
>  include/linux/msi.h  |  1 +
>  4 files changed, 105 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/virtio/virtio_mmio_msi.h


This patch needs to copy maintainers for drivers/base/platform-msi.c and
include/linux/msi.h

> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
> index 8da314b..45752f1 100644
> --- a/drivers/base/platform-msi.c
> +++ b/drivers/base/platform-msi.c
> @@ -31,12 +31,11 @@ struct platform_msi_priv_data {
>  /* The devid allocator */
>  static DEFINE_IDA(platform_msi_devid_ida);
>  
> -#ifdef GENERIC_MSI_DOMAIN_OPS
>  /*
>   * Convert an msi_desc to a globaly unique identifier (per-device
>   * devid + msi_desc position in the msi_list).
>   */
> -static irq_hw_number_t platform_msi_calc_hwirq(struct msi_desc *desc)
> +irq_hw_number_t platform_msi_calc_hwirq(struct msi_desc *desc)
>  {
>   u32 devid;
>  
> @@ -45,6 +44,7 @@ static irq_hw_number_t platform_msi_calc_hwirq(struct 
> msi_desc *desc)
>   return (devid << (32 - DEV_ID_SHIFT)) | desc->platform.msi_index;
>  }
>  
> +#ifdef GENERIC_MSI_DOMAIN_OPS
>  static void platform_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc 
> *desc)
>  {
>   arg->desc = desc;
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 078615c..551a9f7 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -84,6 +84,15 @@ config VIRTIO_MMIO
>  
>If unsure, say N.
>  
> +config VIRTIO_MMIO_MSI
> + bool "Memory-mapped virtio device MSI"
> + depends on VIRTIO_MMIO && GENERIC_MSI_IRQ_DOMAIN && GENERIC_MSI_IRQ
> + help
> +  This allows device drivers to support msi interrupt handling for
> +  virtio-mmio devices. It can improve performance greatly.
> +
> +  If unsure, say N.
> +
>  config VIRTIO_MMIO_CMDLINE_DEVICES
>   bool "Memory mapped virtio devices parameter parsing"
>   depends on VIRTIO_MMIO
> diff --git a/drivers/virtio/virtio_mmio_msi.h 
> b/drivers/virtio/virtio_mmio_msi.h
> new file mode 100644
> index 000..27cb2af
> --- /dev/null
> +++ b/drivers/virtio/virtio_mmio_msi.h
> @@ -0,0 +1,93 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _DRIVERS_VIRTIO_VIRTIO_MMIO_MSI_H
> +#define _DRIVERS_VIRTIO_VIRTIO_MMIO_MSI_H
> +
> +#ifdef CONFIG_VIRTIO_MMIO_MSI
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static irq_hw_number_t mmio_msi_hwirq;
> +static struct irq_domain *mmio_msi_domain;
> +
> +struct irq_domain *__weak arch_msi_root_irq_domain(void)
> +{
> + return NULL;
> +}
> +
> +void __weak irq_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> +}
> +
> +static void mmio_msi_mask_irq(struct irq_data *data)
> +{
> +}
> +
> +static void mmio_msi_unmask_irq(struct irq_data *data)
> +{
> +}
> +
> +static struct irq_chip mmio_msi_controller = {
> + .name   = "VIRTIO-MMIO-MSI",
> + .irq_mask   = mmio_msi_mask_irq,
> + .irq_unmask = mmio_msi_unmask_irq,
> + .irq_ack= irq_chip_ack_parent,
> + .irq_retrigger  = irq_chip_retrigger_hierarchy,
> + .irq_compose_msi_msg= irq_msi_compose_msg,
> + .flags  = IRQCHIP_SKIP_SET_WAKE,
> +};
> +
> +static int mmio_msi_prepare(struct irq_domain *domain, struct device *dev,
> + int nvec, msi_alloc_info_t *arg)
> +{
> + memset(arg, 0, sizeof(*arg));
> + return 0;
> +}
> +
> +static void mmio_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
> +{
> + mmio_msi_hwirq = platform_msi_calc_hwirq(desc);
> +}


This call isn't exported to modules. How will it work when virtio is
modular?

> +
> +static irq_hw_number_t mmio_msi_get_hwirq(struct msi_domain_info *info,
> +   msi_alloc_info_t *arg)
> +{
> + return mmio_msi_hwirq;
> +}
> +
> +static struct msi_domain_ops mmio_msi_domain_ops = {
> + .msi_prepare= mmio_msi_prepare,
> + .set_desc   = mmio_msi_set_desc,
> + .get_hwirq  = mmio_msi_get_hwirq,
> +};
> +
> +static struct msi_domain_info mmio_msi_domain_info = {
> + .flags  = MSI_FLAG_USE_DEF_DOM_OPS |
> +   MSI_FLAG_USE_DEF_CHIP_OPS |
> +   MSI_FLAG_ACTIVATE_EARLY,
> + .ops= &mmio_msi_domain_ops,
> + .chip   = &mmio_msi_controller

Re: [PATCH v2 2/5] virtio-mmio: refactor common functionality

2020-02-11 Thread Michael S. Tsirkin
On Mon, Feb 10, 2020 at 05:05:18PM +0800, Zha Bin wrote:
> From: Liu Jiang 
> 
> Common functionality is refactored into virtio_mmio_common.h
> in order to MSI support in later patch set.
> 
> Signed-off-by: Liu Jiang 
> Co-developed-by: Zha Bin 
> Signed-off-by: Zha Bin 
> Co-developed-by: Jing Liu 
> Signed-off-by: Jing Liu 
> Co-developed-by: Chao Peng 
> Signed-off-by: Chao Peng 

What does this proliferation of header files achieve?
common with what?

> ---
>  drivers/virtio/virtio_mmio.c| 21 +
>  drivers/virtio/virtio_mmio_common.h | 31 +++
>  2 files changed, 32 insertions(+), 20 deletions(-)
>  create mode 100644 drivers/virtio/virtio_mmio_common.h
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 1733ab97..41e1c93 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -61,13 +61,12 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> +#include "virtio_mmio_common.h"
>  
>  
>  
> @@ -77,24 +76,6 @@
>  
>  
>  
> -#define to_virtio_mmio_device(_plat_dev) \
> - container_of(_plat_dev, struct virtio_mmio_device, vdev)
> -
> -struct virtio_mmio_device {
> - struct virtio_device vdev;
> - struct platform_device *pdev;
> -
> - void __iomem *base;
> - unsigned long version;
> -
> - /* a list of queues so we can dispatch IRQs */
> - spinlock_t lock;
> - struct list_head virtqueues;
> -
> - unsigned short notify_base;
> - unsigned short notify_multiplier;
> -};
> -
>  struct virtio_mmio_vq_info {
>   /* the actual virtqueue */
>   struct virtqueue *vq;
> diff --git a/drivers/virtio/virtio_mmio_common.h 
> b/drivers/virtio/virtio_mmio_common.h
> new file mode 100644
> index 000..90cb304
> --- /dev/null
> +++ b/drivers/virtio/virtio_mmio_common.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _DRIVERS_VIRTIO_VIRTIO_MMIO_COMMON_H
> +#define _DRIVERS_VIRTIO_VIRTIO_MMIO_COMMON_H
> +/*
> + * Virtio MMIO driver - common functionality for all device versions
> + *
> + * This module allows virtio devices to be used over a memory-mapped device.
> + */
> +
> +#include 
> +#include 
> +
> +#define to_virtio_mmio_device(_plat_dev) \
> + container_of(_plat_dev, struct virtio_mmio_device, vdev)
> +
> +struct virtio_mmio_device {
> + struct virtio_device vdev;
> + struct platform_device *pdev;
> +
> + void __iomem *base;
> + unsigned long version;
> +
> + /* a list of queues so we can dispatch IRQs */
> + spinlock_t lock;
> + struct list_head virtqueues;
> +
> + unsigned short notify_base;
> + unsigned short notify_multiplier;
> +};
> +
> +#endif
> -- 
> 1.8.3.1




Re: [PATCH v5 3/8] multifd: Make no compression operations into its own structure

2020-02-11 Thread Juan Quintela
"Dr. David Alan Gilbert"  wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> It will be used later.
>> 
>> Signed-off-by: Juan Quintela 
>
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index d8b0205977..c7fea4914c 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -25,6 +25,10 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, 
>> ram_addr_t offset);
>>  
>>  #define MULTIFD_FLAG_SYNC (1 << 0)
>>  
>> +/* We reserve 3 bits for METHODS */
>> +#define MULTIFD_FLAG_METHOD_MASK (7 << 1)
>> +#define MULTIFD_FLAG_NOCOMP (1 << 1)
>> +
>
> Doesn't the 'NOCOMP' value have to be 0 for it to not break
> compatibility with existing multifd?

You are right.  fixing on next resend.

Thanks.




Re: [virtio-dev] Re: [PATCH v2 4/5] virtio-mmio: add MSI interrupt feature support

2020-02-11 Thread Michael S. Tsirkin
On Tue, Feb 11, 2020 at 11:35:43AM +0800, Liu, Jing2 wrote:
> 
> On 2/11/2020 11:17 AM, Jason Wang wrote:
> > 
> > On 2020/2/10 下午5:05, Zha Bin wrote:
> > > From: Liu Jiang
> > > 
> > > Userspace VMMs (e.g. Qemu microvm, Firecracker) take advantage of using
> > > virtio over mmio devices as a lightweight machine model for modern
> > > cloud. The standard virtio over MMIO transport layer only supports one
> > > legacy interrupt, which is much heavier than virtio over PCI transport
> > > layer using MSI. Legacy interrupt has long work path and causes specific
> > > VMExits in following cases, which would considerably slow down the
> > > performance:
> > > 
> > > 1) read interrupt status register
> > > 2) update interrupt status register
> > > 3) write IOAPIC EOI register
> > > 
> > > We proposed to add MSI support for virtio over MMIO via new feature
> > > bit VIRTIO_F_MMIO_MSI[1] which increases the interrupt performance.
> > > 
> > > With the VIRTIO_F_MMIO_MSI feature bit supported, the virtio-mmio MSI
> > > uses msi_sharing[1] to indicate the event and vector mapping.
> > > Bit 1 is 0: device uses non-sharing and fixed vector per event mapping.
> > > Bit 1 is 1: device uses sharing mode and dynamic mapping.
> > 
> > 
> > I believe dynamic mapping should cover the case of fixed vector?
> > 
> Actually this bit *aims* for msi sharing or msi non-sharing.
> 
> It means, when msi sharing bit is 1, device doesn't want vector per queue
> 
> (it wants msi vector sharing as name) and doesn't want a high interrupt
> rate.
> 
> So driver turns to !per_vq_vectors and has to do dynamical mapping.
> 
> So they are opposite not superset.
> 
> Thanks!
> 
> Jing

what's the point of all this flexibility? the cover letter
complains about the code size of pci, then you go back
and add a ton of options with no justification.



> 
> > Thanks
> > 
> > 
> > 
> > -
> > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
> > 




Re: [PATCH v2 1/5] virtio-mmio: add notify feature for per-queue

2020-02-11 Thread Michael S. Tsirkin
On Mon, Feb 10, 2020 at 05:05:17PM +0800, Zha Bin wrote:
> From: Liu Jiang 
> 
> The standard virtio-mmio devices use notification register to signal
> backend. This will cause vmexits and slow down the performance when we
> passthrough the virtio-mmio devices to guest virtual machines.
> We proposed to update virtio over MMIO spec to add the per-queue
> notify feature VIRTIO_F_MMIO_NOTIFICATION[1]. It can allow the VMM to
> configure notify location for each queue.
> 
> [1] https://lkml.org/lkml/2020/1/21/31
> 
> Signed-off-by: Liu Jiang 
> Co-developed-by: Zha Bin 
> Signed-off-by: Zha Bin 
> Co-developed-by: Jing Liu 
> Signed-off-by: Jing Liu 
> Co-developed-by: Chao Peng 
> Signed-off-by: Chao Peng 


Hmm. Any way to make this static so we don't need
base and multiplier?

> ---
>  drivers/virtio/virtio_mmio.c   | 37 +++--
>  include/uapi/linux/virtio_config.h |  8 +++-
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 97d5725..1733ab97 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -90,6 +90,9 @@ struct virtio_mmio_device {
>   /* a list of queues so we can dispatch IRQs */
>   spinlock_t lock;
>   struct list_head virtqueues;
> +
> + unsigned short notify_base;
> + unsigned short notify_multiplier;
>  };
>  
>  struct virtio_mmio_vq_info {
> @@ -98,6 +101,9 @@ struct virtio_mmio_vq_info {
>  
>   /* the list node for the virtqueues list */
>   struct list_head node;
> +
> + /* Notify Address*/
> + unsigned int notify_addr;
>  };
>  
>  
> @@ -119,13 +125,23 @@ static u64 vm_get_features(struct virtio_device *vdev)
>   return features;
>  }
>  
> +static void vm_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + if (features & BIT_ULL(VIRTIO_F_MMIO_NOTIFICATION))
> + __virtio_set_bit(vdev, VIRTIO_F_MMIO_NOTIFICATION);
> +}
> +
>  static int vm_finalize_features(struct virtio_device *vdev)
>  {
>   struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> + u64 features = vdev->features;
>  
>   /* Give virtio_ring a chance to accept features. */
>   vring_transport_features(vdev);
>  
> + /* Give virtio_mmio a chance to accept features. */
> + vm_transport_features(vdev, features);
> +
>   /* Make sure there is are no mixed devices */
>   if (vm_dev->version == 2 &&
>   !__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> @@ -272,10 +288,13 @@ static void vm_reset(struct virtio_device *vdev)
>  static bool vm_notify(struct virtqueue *vq)
>  {
>   struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev);
> + struct virtio_mmio_vq_info *info = vq->priv;
>  
> - /* We write the queue's selector into the notification register to
> + /* We write the queue's selector into the Notify Address to
>* signal the other end */
> - writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
> + if (info)
> + writel(vq->index, vm_dev->base + info->notify_addr);
> +
>   return true;
>  }
>  
> @@ -434,6 +453,12 @@ static struct virtqueue *vm_setup_vq(struct 
> virtio_device *vdev, unsigned index,
>   vq->priv = info;
>   info->vq = vq;
>  
> + if (__virtio_test_bit(vdev, VIRTIO_F_MMIO_NOTIFICATION))
> + info->notify_addr = vm_dev->notify_base +
> + vm_dev->notify_multiplier * vq->index;
> + else
> + info->notify_addr = VIRTIO_MMIO_QUEUE_NOTIFY;
> +
>   spin_lock_irqsave(&vm_dev->lock, flags);
>   list_add(&info->node, &vm_dev->virtqueues);
>   spin_unlock_irqrestore(&vm_dev->lock, flags);
> @@ -471,6 +496,14 @@ static int vm_find_vqs(struct virtio_device *vdev, 
> unsigned nvqs,
>   return irq;
>   }
>  
> + if (__virtio_test_bit(vdev, VIRTIO_F_MMIO_NOTIFICATION)) {
> + unsigned int notify = readl(vm_dev->base +
> + VIRTIO_MMIO_QUEUE_NOTIFY);


that register is documented as:

/* Queue notifier - Write Only */
#define VIRTIO_MMIO_QUEUE_NOTIFY0x050

so at least you need to update the doc.

> +
> + vm_dev->notify_base = notify & 0x;
> + vm_dev->notify_multiplier = (notify >> 16) & 0x;

are 16 bit base/limit always enough?
In fact won't we be short on 16 bit address space
in a rather short order if queues use up a page
of space at a time?


> + }
> +
>   err = request_irq(irq, vm_interrupt, IRQF_SHARED,
>   dev_name(&vdev->dev), vm_dev);
>   if (err)
> diff --git a/include/uapi/linux/virtio_config.h 
> b/include/uapi/linux/virtio_config.h
> index ff8e7dc..5d93c01 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -52,7 +52,7 @@
>   * rest are per-device feature bits.
>   */
>  #define VIRTIO_TRANSPORT_F_START

Re: [virtio-dev] Re: [PATCH v2 4/5] virtio-mmio: add MSI interrupt feature support

2020-02-11 Thread Michael S. Tsirkin
On Tue, Feb 11, 2020 at 03:40:23PM +0800, Jason Wang wrote:
> 
> On 2020/2/11 下午2:02, Liu, Jing2 wrote:
> > 
> > 
> > On 2/11/2020 12:02 PM, Jason Wang wrote:
> > > 
> > > On 2020/2/11 上午11:35, Liu, Jing2 wrote:
> > > > 
> > > > On 2/11/2020 11:17 AM, Jason Wang wrote:
> > > > > 
> > > > > On 2020/2/10 下午5:05, Zha Bin wrote:
> > > > > > From: Liu Jiang
> > > > > > 
> > > > > > Userspace VMMs (e.g. Qemu microvm, Firecracker) take
> > > > > > advantage of using
> > > > > > virtio over mmio devices as a lightweight machine model for modern
> > > > > > cloud. The standard virtio over MMIO transport layer
> > > > > > only supports one
> > > > > > legacy interrupt, which is much heavier than virtio over
> > > > > > PCI transport
> > > > > > layer using MSI. Legacy interrupt has long work path and
> > > > > > causes specific
> > > > > > VMExits in following cases, which would considerably slow down the
> > > > > > performance:
> > > > > > 
> > > > > > 1) read interrupt status register
> > > > > > 2) update interrupt status register
> > > > > > 3) write IOAPIC EOI register
> > > > > > 
> > > > > > We proposed to add MSI support for virtio over MMIO via new feature
> > > > > > bit VIRTIO_F_MMIO_MSI[1] which increases the interrupt performance.
> > > > > > 
> > > > > > With the VIRTIO_F_MMIO_MSI feature bit supported, the virtio-mmio 
> > > > > > MSI
> > > > > > uses msi_sharing[1] to indicate the event and vector mapping.
> > > > > > Bit 1 is 0: device uses non-sharing and fixed vector per
> > > > > > event mapping.
> > > > > > Bit 1 is 1: device uses sharing mode and dynamic mapping.
> > > > > 
> > > > > 
> > > > > I believe dynamic mapping should cover the case of fixed vector?
> > > > > 
> > > > Actually this bit *aims* for msi sharing or msi non-sharing.
> > > > 
> > > > It means, when msi sharing bit is 1, device doesn't want vector
> > > > per queue
> > > > 
> > > > (it wants msi vector sharing as name) and doesn't want a high
> > > > interrupt rate.
> > > > 
> > > > So driver turns to !per_vq_vectors and has to do dynamical mapping.
> > > > 
> > > > So they are opposite not superset.
> > > > 
> > > > Thanks!
> > > > 
> > > > Jing
> > > 
> > > 
> > > I think you need add more comments on the command.
> > > 
> > > E.g if I want to map vector 0 to queue 1, how do I need to do?
> > > 
> > > write(1, queue_sel);
> > > write(0, vector_sel);
> > 
> > That's true. Besides, two commands are used for msi sharing mode,
> > 
> > VIRTIO_MMIO_MSI_CMD_MAP_CONFIG and VIRTIO_MMIO_MSI_CMD_MAP_QUEUE.
> > 
> > "To set up the event and vector mapping for MSI sharing mode, driver
> > SHOULD write a valid MsiVecSel followed by
> > VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE command to
> > map the configuration change/selected queue events respectively.  " (See
> > spec patch 5/5)
> > 
> > So if driver detects the msi sharing mode, when it does setup vq, writes
> > the queue_sel (this already exists in setup vq), vector sel and then
> > MAP_QUEUE command to do the queue event mapping.
> > 
> 
> So actually the per vq msix could be done through this. I don't get why you
> need to introduce MSI_SHARING_MASK which is the charge of driver instead of
> device. The interrupt rate should have no direct relationship with whether
> it has been shared or not.
> 
> Btw, you introduce mask/unmask without pending, how to deal with the lost
> interrupt during the masking then?

pending can be an internal device register. as long as device
does not lose interrupts while masked, all's well.

There's value is being able to say "this queue sends no
interrupts do not bother checking used notification area".
so we need way to say that. So I guess an enable interrupts
register might have some value...
But besides that, it's enough to have mask/unmask/address/data
per vq.


> 
> > For msi non-sharing mode, no special action is needed because we make
> > the rule of per_vq_vector and fixed relationship.
> > 
> > Correct me if this is not that clear for spec/code comments.
> > 
> 
> The ABI is not as straightforward as PCI did. Why not just reuse the PCI
> layout?
> 
> E.g having
> 
> queue_sel
> queue_msix_vector
> msix_config
> 
> for configuring map between msi vector and queues/config
> 
> Then
> 
> vector_sel
> address
> data
> pending
> mask
> unmask
> 
> for configuring msi table?
> 
> Thanks
> 
> 
> > Thanks!
> > 
> > Jing
> > 
> > 
> > > 
> > > ?
> > > 
> > > Thanks
> > > 
> > > 
> > > > 
> > > > 
> > > > > Thanks
> > > > > 
> > > > > 
> > > > > 
> > > > > -
> > > > > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> > > > > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
> > > > > 
> > > > 
> > > 




Re: [virtio-dev] Re: [PATCH v2 4/5] virtio-mmio: add MSI interrupt feature support

2020-02-11 Thread Jason Wang



On 2020/2/11 下午7:58, Michael S. Tsirkin wrote:

On Tue, Feb 11, 2020 at 03:40:23PM +0800, Jason Wang wrote:

On 2020/2/11 下午2:02, Liu, Jing2 wrote:

On 2/11/2020 12:02 PM, Jason Wang wrote:

On 2020/2/11 上午11:35, Liu, Jing2 wrote:

On 2/11/2020 11:17 AM, Jason Wang wrote:

On 2020/2/10 下午5:05, Zha Bin wrote:

From: Liu Jiang

Userspace VMMs (e.g. Qemu microvm, Firecracker) take
advantage of using
virtio over mmio devices as a lightweight machine model for modern
cloud. The standard virtio over MMIO transport layer
only supports one
legacy interrupt, which is much heavier than virtio over
PCI transport
layer using MSI. Legacy interrupt has long work path and
causes specific
VMExits in following cases, which would considerably slow down the
performance:

1) read interrupt status register
2) update interrupt status register
3) write IOAPIC EOI register

We proposed to add MSI support for virtio over MMIO via new feature
bit VIRTIO_F_MMIO_MSI[1] which increases the interrupt performance.

With the VIRTIO_F_MMIO_MSI feature bit supported, the virtio-mmio MSI
uses msi_sharing[1] to indicate the event and vector mapping.
Bit 1 is 0: device uses non-sharing and fixed vector per
event mapping.
Bit 1 is 1: device uses sharing mode and dynamic mapping.

I believe dynamic mapping should cover the case of fixed vector?


Actually this bit*aims*  for msi sharing or msi non-sharing.

It means, when msi sharing bit is 1, device doesn't want vector
per queue

(it wants msi vector sharing as name) and doesn't want a high
interrupt rate.

So driver turns to !per_vq_vectors and has to do dynamical mapping.

So they are opposite not superset.

Thanks!

Jing

I think you need add more comments on the command.

E.g if I want to map vector 0 to queue 1, how do I need to do?

write(1, queue_sel);
write(0, vector_sel);

That's true. Besides, two commands are used for msi sharing mode,

VIRTIO_MMIO_MSI_CMD_MAP_CONFIG and VIRTIO_MMIO_MSI_CMD_MAP_QUEUE.

"To set up the event and vector mapping for MSI sharing mode, driver
SHOULD write a valid MsiVecSel followed by
VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE command to
map the configuration change/selected queue events respectively.  " (See
spec patch 5/5)

So if driver detects the msi sharing mode, when it does setup vq, writes
the queue_sel (this already exists in setup vq), vector sel and then
MAP_QUEUE command to do the queue event mapping.


So actually the per vq msix could be done through this. I don't get why you
need to introduce MSI_SHARING_MASK which is the charge of driver instead of
device. The interrupt rate should have no direct relationship with whether
it has been shared or not.

Btw, you introduce mask/unmask without pending, how to deal with the lost
interrupt during the masking then?

pending can be an internal device register. as long as device
does not lose interrupts while masked, all's well.



You meant raise the interrupt during unmask automatically?




There's value is being able to say "this queue sends no
interrupts do not bother checking used notification area".
so we need way to say that. So I guess an enable interrupts
register might have some value...
But besides that, it's enough to have mask/unmask/address/data
per vq.



Just to check, do you mean "per vector" here?

Thanks










Re: [virtio-dev] Re: [PATCH v2 4/5] virtio-mmio: add MSI interrupt feature support

2020-02-11 Thread Michael S. Tsirkin
On Tue, Feb 11, 2020 at 08:04:24PM +0800, Jason Wang wrote:
> 
> On 2020/2/11 下午7:58, Michael S. Tsirkin wrote:
> > On Tue, Feb 11, 2020 at 03:40:23PM +0800, Jason Wang wrote:
> > > On 2020/2/11 下午2:02, Liu, Jing2 wrote:
> > > > On 2/11/2020 12:02 PM, Jason Wang wrote:
> > > > > On 2020/2/11 上午11:35, Liu, Jing2 wrote:
> > > > > > On 2/11/2020 11:17 AM, Jason Wang wrote:
> > > > > > > On 2020/2/10 下午5:05, Zha Bin wrote:
> > > > > > > > From: Liu Jiang
> > > > > > > > 
> > > > > > > > Userspace VMMs (e.g. Qemu microvm, Firecracker) take
> > > > > > > > advantage of using
> > > > > > > > virtio over mmio devices as a lightweight machine model for 
> > > > > > > > modern
> > > > > > > > cloud. The standard virtio over MMIO transport layer
> > > > > > > > only supports one
> > > > > > > > legacy interrupt, which is much heavier than virtio over
> > > > > > > > PCI transport
> > > > > > > > layer using MSI. Legacy interrupt has long work path and
> > > > > > > > causes specific
> > > > > > > > VMExits in following cases, which would considerably slow down 
> > > > > > > > the
> > > > > > > > performance:
> > > > > > > > 
> > > > > > > > 1) read interrupt status register
> > > > > > > > 2) update interrupt status register
> > > > > > > > 3) write IOAPIC EOI register
> > > > > > > > 
> > > > > > > > We proposed to add MSI support for virtio over MMIO via new 
> > > > > > > > feature
> > > > > > > > bit VIRTIO_F_MMIO_MSI[1] which increases the interrupt 
> > > > > > > > performance.
> > > > > > > > 
> > > > > > > > With the VIRTIO_F_MMIO_MSI feature bit supported, the 
> > > > > > > > virtio-mmio MSI
> > > > > > > > uses msi_sharing[1] to indicate the event and vector mapping.
> > > > > > > > Bit 1 is 0: device uses non-sharing and fixed vector per
> > > > > > > > event mapping.
> > > > > > > > Bit 1 is 1: device uses sharing mode and dynamic mapping.
> > > > > > > I believe dynamic mapping should cover the case of fixed vector?
> > > > > > > 
> > > > > > Actually this bit*aims*  for msi sharing or msi non-sharing.
> > > > > > 
> > > > > > It means, when msi sharing bit is 1, device doesn't want vector
> > > > > > per queue
> > > > > > 
> > > > > > (it wants msi vector sharing as name) and doesn't want a high
> > > > > > interrupt rate.
> > > > > > 
> > > > > > So driver turns to !per_vq_vectors and has to do dynamical mapping.
> > > > > > 
> > > > > > So they are opposite not superset.
> > > > > > 
> > > > > > Thanks!
> > > > > > 
> > > > > > Jing
> > > > > I think you need add more comments on the command.
> > > > > 
> > > > > E.g if I want to map vector 0 to queue 1, how do I need to do?
> > > > > 
> > > > > write(1, queue_sel);
> > > > > write(0, vector_sel);
> > > > That's true. Besides, two commands are used for msi sharing mode,
> > > > 
> > > > VIRTIO_MMIO_MSI_CMD_MAP_CONFIG and VIRTIO_MMIO_MSI_CMD_MAP_QUEUE.
> > > > 
> > > > "To set up the event and vector mapping for MSI sharing mode, driver
> > > > SHOULD write a valid MsiVecSel followed by
> > > > VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE command to
> > > > map the configuration change/selected queue events respectively.  " (See
> > > > spec patch 5/5)
> > > > 
> > > > So if driver detects the msi sharing mode, when it does setup vq, writes
> > > > the queue_sel (this already exists in setup vq), vector sel and then
> > > > MAP_QUEUE command to do the queue event mapping.
> > > > 
> > > So actually the per vq msix could be done through this. I don't get why 
> > > you
> > > need to introduce MSI_SHARING_MASK which is the charge of driver instead 
> > > of
> > > device. The interrupt rate should have no direct relationship with whether
> > > it has been shared or not.
> > > 
> > > Btw, you introduce mask/unmask without pending, how to deal with the lost
> > > interrupt during the masking then?
> > pending can be an internal device register. as long as device
> > does not lose interrupts while masked, all's well.
> 
> 
> You meant raise the interrupt during unmask automatically?
> 


yes - that's also what pci does.

the guest visible pending bit is partially implemented in qemu
as per pci spec but it's unused.

> > 
> > There's value is being able to say "this queue sends no
> > interrupts do not bother checking used notification area".
> > so we need way to say that. So I guess an enable interrupts
> > register might have some value...
> > But besides that, it's enough to have mask/unmask/address/data
> > per vq.
> 
> 
> Just to check, do you mean "per vector" here?
> 
> Thanks
> 

No, per VQ. An indirection VQ -> vector -> address/data isn't
necessary for PCI either, but that ship has sailed.

-- 
MST




[Bug 1862167] Re: Variation of SVE register size (qemu-user-aarch64)

2020-02-11 Thread Laurent Vivier
This is already managed by a cpu property.

0df9142d27d5 ("target/arm/cpu64: max cpu: Introduce sve properties")

See docs/arm-cpu-features.rst

Try "-cpu max,sve512=on".

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

Title:
  Variation of SVE register size (qemu-user-aarch64)

Status in QEMU:
  New

Bug description:
  Specification of ARMv8-A SVE extention allows various values ​​for the
  size of the SVE register. On the other hand, it seems that the current
  qemu-aarch64 supports only the maximum length of 2048 bits as the SVE
  register size. I am writing an assembler program for a CPU that is
  compliant with ARMv8-A + SVE and has a 512-bit SVE register, but when
  this is run with qemu-user-aarch64, a 2048-bit load / store
  instruction is executed This causes a segmentation fault. Shouldn't
  qeum-user-aarch64 have an option to specify the SVE register size?

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



Re: [virtio-dev] Re: [PATCH v2 4/5] virtio-mmio: add MSI interrupt feature support

2020-02-11 Thread Jason Wang



On 2020/2/11 下午8:08, Michael S. Tsirkin wrote:

On Tue, Feb 11, 2020 at 08:04:24PM +0800, Jason Wang wrote:

On 2020/2/11 下午7:58, Michael S. Tsirkin wrote:

On Tue, Feb 11, 2020 at 03:40:23PM +0800, Jason Wang wrote:

On 2020/2/11 下午2:02, Liu, Jing2 wrote:

On 2/11/2020 12:02 PM, Jason Wang wrote:

On 2020/2/11 上午11:35, Liu, Jing2 wrote:

On 2/11/2020 11:17 AM, Jason Wang wrote:

On 2020/2/10 下午5:05, Zha Bin wrote:

From: Liu Jiang

Userspace VMMs (e.g. Qemu microvm, Firecracker) take
advantage of using
virtio over mmio devices as a lightweight machine model for modern
cloud. The standard virtio over MMIO transport layer
only supports one
legacy interrupt, which is much heavier than virtio over
PCI transport
layer using MSI. Legacy interrupt has long work path and
causes specific
VMExits in following cases, which would considerably slow down the
performance:

1) read interrupt status register
2) update interrupt status register
3) write IOAPIC EOI register

We proposed to add MSI support for virtio over MMIO via new feature
bit VIRTIO_F_MMIO_MSI[1] which increases the interrupt performance.

With the VIRTIO_F_MMIO_MSI feature bit supported, the virtio-mmio MSI
uses msi_sharing[1] to indicate the event and vector mapping.
Bit 1 is 0: device uses non-sharing and fixed vector per
event mapping.
Bit 1 is 1: device uses sharing mode and dynamic mapping.

I believe dynamic mapping should cover the case of fixed vector?


Actually this bit*aims*  for msi sharing or msi non-sharing.

It means, when msi sharing bit is 1, device doesn't want vector
per queue

(it wants msi vector sharing as name) and doesn't want a high
interrupt rate.

So driver turns to !per_vq_vectors and has to do dynamical mapping.

So they are opposite not superset.

Thanks!

Jing

I think you need add more comments on the command.

E.g if I want to map vector 0 to queue 1, how do I need to do?

write(1, queue_sel);
write(0, vector_sel);

That's true. Besides, two commands are used for msi sharing mode,

VIRTIO_MMIO_MSI_CMD_MAP_CONFIG and VIRTIO_MMIO_MSI_CMD_MAP_QUEUE.

"To set up the event and vector mapping for MSI sharing mode, driver
SHOULD write a valid MsiVecSel followed by
VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE command to
map the configuration change/selected queue events respectively.  " (See
spec patch 5/5)

So if driver detects the msi sharing mode, when it does setup vq, writes
the queue_sel (this already exists in setup vq), vector sel and then
MAP_QUEUE command to do the queue event mapping.


So actually the per vq msix could be done through this. I don't get why you
need to introduce MSI_SHARING_MASK which is the charge of driver instead of
device. The interrupt rate should have no direct relationship with whether
it has been shared or not.

Btw, you introduce mask/unmask without pending, how to deal with the lost
interrupt during the masking then?

pending can be an internal device register. as long as device
does not lose interrupts while masked, all's well.


You meant raise the interrupt during unmask automatically?



yes - that's also what pci does.

the guest visible pending bit is partially implemented in qemu
as per pci spec but it's unused.



Ok.





There's value is being able to say "this queue sends no
interrupts do not bother checking used notification area".
so we need way to say that. So I guess an enable interrupts
register might have some value...
But besides that, it's enough to have mask/unmask/address/data
per vq.


Just to check, do you mean "per vector" here?

Thanks


No, per VQ. An indirection VQ -> vector -> address/data isn't
necessary for PCI either, but that ship has sailed.



Yes, it can work but it may bring extra effort when you want to mask a 
vector which is was shared by a lot of queues.


Thanks








[Bug 1862167] Re: Variation of SVE register size (qemu-user-aarch64)

2020-02-11 Thread Peter Maydell
Note also that the vector length in SVE is not fixed -- you should be
writing your guest code to support arbitrary vector lengths, because
otherwise it will not run on all SVE-supporting CPUs.

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

Title:
  Variation of SVE register size (qemu-user-aarch64)

Status in QEMU:
  New

Bug description:
  Specification of ARMv8-A SVE extention allows various values ​​for the
  size of the SVE register. On the other hand, it seems that the current
  qemu-aarch64 supports only the maximum length of 2048 bits as the SVE
  register size. I am writing an assembler program for a CPU that is
  compliant with ARMv8-A + SVE and has a 512-bit SVE register, but when
  this is run with qemu-user-aarch64, a 2048-bit load / store
  instruction is executed This causes a segmentation fault. Shouldn't
  qeum-user-aarch64 have an option to specify the SVE register size?

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



Re: KVM call for agenda for 2020-02-11

2020-02-11 Thread Juan Quintela
Juan Quintela  wrote:
> Hi

As there are no topics, call gets cancelled.

Have a good week.

>
> Please, send any topic that you are interested in covering.
>
> At the end of Monday I will send an email with the agenda or the
> cancellation of the call, so hurry up.
>
> After discussions on the QEMU Summit, we are going to have always open a
> KVM call where you can add topics.
>
>  Call details:
>
> By popular demand, a google calendar public entry with it
>
>   
> https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ
>
> (Let me know if you have any problems with the calendar entry.  I just
> gave up about getting right at the same time CEST, CET, EDT and DST).
>
> If you need phone number details,  contact me privately
>
> Thanks, Juan.




KVM call for agenda for 2020-02-25

2020-02-11 Thread Juan Quintela



Hi

Please, send any topic that you are interested in covering.

At the end of Monday I will send an email with the agenda or the
cancellation of the call, so hurry up.

After discussions on the QEMU Summit, we are going to have always open a
KVM call where you can add topics.

 Call details:

By popular demand, a google calendar public entry with it

  
https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ

(Let me know if you have any problems with the calendar entry.  I just
gave up about getting right at the same time CEST, CET, EDT and DST).

If you need phone number details,  contact me privately

Thanks, Juan.




[Bug 1862167] Re: Variation of SVE register size (qemu-user-aarch64)

2020-02-11 Thread Alex Bennée
** Tags added: arm

** Changed in: qemu
   Status: New => Invalid

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

Title:
  Variation of SVE register size (qemu-user-aarch64)

Status in QEMU:
  Invalid

Bug description:
  Specification of ARMv8-A SVE extention allows various values ​​for the
  size of the SVE register. On the other hand, it seems that the current
  qemu-aarch64 supports only the maximum length of 2048 bits as the SVE
  register size. I am writing an assembler program for a CPU that is
  compliant with ARMv8-A + SVE and has a 512-bit SVE register, but when
  this is run with qemu-user-aarch64, a 2048-bit load / store
  instruction is executed This causes a segmentation fault. Shouldn't
  qeum-user-aarch64 have an option to specify the SVE register size?

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



[PATCH] tests/acceptance/ppc_prep_40p: Do not run NetBSD test by default

2020-02-11 Thread Philippe Mathieu-Daudé
The ftp.netbsd.org server is slow and downloading the NetBSD ISO
takes too long. Do not include this test in the default suite.

Similarly to commit 471c97a69b:

  Currently the Avocado framework does not distinct the time spent
  downloading assets vs. the time spent running a test. With big
  assets (like a full VM image) the tests likely fail.

  This is a limitation known by the Avocado team.
  Until this issue get fixed, do not run this tests automatically.

  Tests can still be run setting the AVOCADO_TIMEOUT_EXPECTED
  environment variable.

Reported-by: Alex Bennée 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/acceptance/ppc_prep_40p.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
index b27572f212..efe06037ba 100644
--- a/tests/acceptance/ppc_prep_40p.py
+++ b/tests/acceptance/ppc_prep_40p.py
@@ -61,6 +61,7 @@ def test_openbios_192m(self):
 wait_for_console_pattern(self, '>> CPU type PowerPC,604')
 
 @skipIf(os.getenv('CONTINUOUS_INTEGRATION'), 'Running on Travis-CI')
+@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
 def test_openbios_and_netbsd(self):
 """
 :avocado: tags=arch:ppc
-- 
2.21.1




Re: [RFC 1/2] tpm: Let the TPM TIS device be usable on ARM

2020-02-11 Thread Auger Eric
Hi Peter,

On 2/11/20 11:56 AM, Peter Maydell wrote:
> On Tue, 11 Feb 2020 at 08:35, Auger Eric  wrote:
>>
>> Hi Philippe,
>>
>> On 2/11/20 9:25 AM, Philippe Mathieu-Daudé wrote:
>>> You don't need much to remove the RFC tag:
>>>
>>> - rename TYPE_TPM_TIS as TYPE_TPM_TIS_ISA
>>> - rename TPMState as TPMCommonState, add an abstract TYPE_TPM_TIS
>>> parent, let TYPE_TPM_TIS_ISA be a child
>>> - add TYPE_TPM_TIS_SYSBUS also child.
>> Yes I tried my luck without too much hopes ;-)
> 
> There should be a few existing examples in the tree
> of devices that we provide in a sysbus and also
> an isa or pci flavour, that you can use as templates
> for how to structure the device.
Yes I found some. Thank you.

Eric

> 
> -- PMM
> 




Re: [PATCH] tests/acceptance/ppc_prep_40p: Do not run NetBSD test by default

2020-02-11 Thread Kamil Rytarowski
Please use cdn.netbsd.org always.

On 11.02.2020 14:19, Philippe Mathieu-Daudé wrote:
> The ftp.netbsd.org server is slow and downloading the NetBSD ISO
> takes too long. Do not include this test in the default suite.
> 
> Similarly to commit 471c97a69b:
> 
>   Currently the Avocado framework does not distinct the time spent
>   downloading assets vs. the time spent running a test. With big
>   assets (like a full VM image) the tests likely fail.
> 
>   This is a limitation known by the Avocado team.
>   Until this issue get fixed, do not run this tests automatically.
> 
>   Tests can still be run setting the AVOCADO_TIMEOUT_EXPECTED
>   environment variable.
> 
> Reported-by: Alex Bennée 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/acceptance/ppc_prep_40p.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/acceptance/ppc_prep_40p.py 
> b/tests/acceptance/ppc_prep_40p.py
> index b27572f212..efe06037ba 100644
> --- a/tests/acceptance/ppc_prep_40p.py
> +++ b/tests/acceptance/ppc_prep_40p.py
> @@ -61,6 +61,7 @@ def test_openbios_192m(self):
>  wait_for_console_pattern(self, '>> CPU type PowerPC,604')
>  
>  @skipIf(os.getenv('CONTINUOUS_INTEGRATION'), 'Running on Travis-CI')
> +@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
>  def test_openbios_and_netbsd(self):
>  """
>  :avocado: tags=arch:ppc
> 




signature.asc
Description: OpenPGP digital signature


Re: [PULL 0/6] 9p patches 2020-02-08

2020-02-11 Thread Christian Schoenebeck
On Dienstag, 11. Februar 2020 09:42:01 CET Greg Kurz wrote:
> On Tue, 11 Feb 2020 09:15:41 +0100
> 
> Christian Schoenebeck  wrote:
> > On Montag, 10. Februar 2020 18:08:38 CET Peter Maydell wrote:
> > > On Sat, 8 Feb 2020 at 10:45, Greg Kurz  wrote:
> > > > The following changes since commit
> > 
> > 42ccca1bd9456568f996d5646b2001faac96944b:
> > > >   Merge remote-tracking branch
> > > >   'remotes/berrange/tags/misc-fixes-pull-request' into staging
> > > >   (2020-02-07 15:01:23 +)>
> > > > 
> > > > are available in the Git repository at:
> > > >   https://github.com/gkurz/qemu.git tags/9p-next-2020-02-08
> > > > 
> > > > for you to fetch changes up to 
2822602cbe2be98229b882101dfdb9d3a738c611:
> > > >   MAINTAINERS: 9pfs: Add myself as reviewer (2020-02-08 09:29:04
> > > >   +0100)
> > > > 
> > > > 
> > > > 9p patches:
> > > > - some more protocol sanity checks
> > > > - qtest for readdir
> > > > - Christian Schoenebeck now official reviewer
> > > > 
> > > > 
> > > 
> > > Applied, thanks.
> > > 
> > > Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
> > > for any user-visible changes.
> > > 
> > > -- PMM
> > 
> > I.e. msize >= 4096 now being required. AFAICS I cannot update the wiki
> > myself.
> I've updated the wiki.

Thanks Greg!

BTW, I think "negociate" is the 18th century variant of "negotiate". :)

Best regards,
Christian Schoenebeck





Re: [PATCH v2] gitlab-ci: Move EDK2 YAML config to .gitlab-ci.d directory

2020-02-11 Thread Wainer dos Santos Moschetta



On 2/11/20 7:50 AM, Philippe Mathieu-Daudé wrote:

We plan to let maintainers managing their own GitLab CI jobs.
The .gitlab-ci.d directory will contain all the new GitLab files,
to keep the root directory cleaner.

The EDK2 job was introduced before .gitlab-ci.d was suggested as
a common directory. Move the YAML file, update its references.

Suggested-by: Wainer dos Santos Moschetta 
Reviewed-by: Laszlo Ersek 
Acked-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
Supersedes: <20200211065022.11134-1-phi...@redhat.com>
v2: Reworded subject/description (Thomas)
---
  .gitignore   | 1 +
  .gitlab-ci-edk2.yml => .gitlab-ci.d/edk2.yml | 2 +-
  .gitlab-ci.yml   | 2 +-
  MAINTAINERS  | 3 +--
  4 files changed, 4 insertions(+), 4 deletions(-)
  rename .gitlab-ci-edk2.yml => .gitlab-ci.d/edk2.yml (98%)


Reviewed-by: Wainer dos Santos Moschetta 


diff --git a/.gitignore b/.gitignore
index bc0a035f9c..18288eacd1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -95,6 +95,7 @@
  *.tp
  *.vr
  *.d
+!/.gitlab-ci.d
  !/scripts/qemu-guest-agent/fsfreeze-hook.d
  *.o
  .sdk
diff --git a/.gitlab-ci-edk2.yml b/.gitlab-ci.d/edk2.yml
similarity index 98%
rename from .gitlab-ci-edk2.yml
rename to .gitlab-ci.d/edk2.yml
index 088ba4b43a..a9990b7147 100644
--- a/.gitlab-ci-edk2.yml
+++ b/.gitlab-ci.d/edk2.yml
@@ -2,7 +2,7 @@ docker-edk2:
   stage: build
   rules: # Only run this job when the Dockerfile is modified
   - changes:
-   - .gitlab-ci-edk2.yml
+   - .gitlab-ci.d/edk2.yml
 - .gitlab-ci.d/edk2/Dockerfile
 when: always
   image: docker:19.03.1
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c15e394f09..dae6045d78 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,5 +1,5 @@
  include:
-  - local: '/.gitlab-ci-edk2.yml'
+  - local: '/.gitlab-ci.d/edk2.yml'
  
  before_script:

   - apt-get update -qq
diff --git a/MAINTAINERS b/MAINTAINERS
index c7717df720..fb00a55f41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2391,8 +2391,7 @@ F: roms/edk2
  F: roms/edk2-*
  F: tests/data/uefi-boot-images/
  F: tests/uefi-test-tools/
-F: .gitlab-ci-edk2.yml
-F: .gitlab-ci.d/edk2/
+F: .gitlab-ci.d/edk2*
  
  Usermode Emulation

  --





[PATCH] block: make BlockConf.*_size properties 32-bit

2020-02-11 Thread Roman Kagan
Devices (virtio-blk, scsi, etc.) and the block layer are happy to use
32-bit for logical_block_size, physical_block_size, and min_io_size.
However, the properties in BlockConf are defined as uint16_t limiting
the values to 32768.

This appears unnecessary tight, and we've seen bigger block sizes handy
at times.

Make them 32 bit instead and lift the limitation.

Signed-off-by: Roman Kagan 
---
 hw/core/qdev-properties.c| 21 -
 include/hw/block/block.h |  8 
 include/hw/qdev-properties.h |  2 +-
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 7f93bfeb88..5f84e4a3b8 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -716,30 +716,32 @@ const PropertyInfo qdev_prop_pci_devfn = {
 
 /* --- blocksize --- */
 
+#define MIN_BLOCK_SIZE 512
+#define MAX_BLOCK_SIZE 2147483648
+
 static void set_blocksize(Object *obj, Visitor *v, const char *name,
   void *opaque, Error **errp)
 {
 DeviceState *dev = DEVICE(obj);
 Property *prop = opaque;
-uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
+uint32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
 Error *local_err = NULL;
-const int64_t min = 512;
-const int64_t max = 32768;
 
 if (dev->realized) {
 qdev_prop_set_after_realize(dev, name, errp);
 return;
 }
 
-visit_type_uint16(v, name, &value, &local_err);
+visit_type_uint32(v, name, &value, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
 }
 /* value of 0 means "unset" */
-if (value && (value < min || value > max)) {
+if (value && (value < MIN_BLOCK_SIZE || value > MAX_BLOCK_SIZE)) {
 error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-   dev->id ? : "", name, (int64_t)value, min, max);
+   dev->id ? : "", name, (int64_t)value,
+   (int64_t)MIN_BLOCK_SIZE, (int64_t)MAX_BLOCK_SIZE);
 return;
 }
 
@@ -755,9 +757,10 @@ static void set_blocksize(Object *obj, Visitor *v, const 
char *name,
 }
 
 const PropertyInfo qdev_prop_blocksize = {
-.name  = "uint16",
-.description = "A power of two between 512 and 32768",
-.get   = get_uint16,
+.name  = "uint32",
+.description = "A power of two between " stringify(MIN_BLOCK_SIZE)
+   " and " stringify(MAX_BLOCK_SIZE),
+.get   = get_uint32,
 .set   = set_blocksize,
 .set_default_value = set_default_value_uint,
 };
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index d7246f3862..9dd6bba56a 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -18,9 +18,9 @@
 
 typedef struct BlockConf {
 BlockBackend *blk;
-uint16_t physical_block_size;
-uint16_t logical_block_size;
-uint16_t min_io_size;
+uint32_t physical_block_size;
+uint32_t logical_block_size;
+uint32_t min_io_size;
 uint32_t opt_io_size;
 int32_t bootindex;
 uint32_t discard_granularity;
@@ -51,7 +51,7 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
   _conf.logical_block_size),\
 DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,\
   _conf.physical_block_size),   \
-DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),\
+DEFINE_PROP_UINT32("min_io_size", _state, _conf.min_io_size, 0),\
 DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
 DEFINE_PROP_UINT32("discard_granularity", _state,   \
_conf.discard_granularity, -1),  \
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 906e697c58..ebdeeb4866 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -193,7 +193,7 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
 #define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
 DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
 #define DEFINE_PROP_BLOCKSIZE(_n, _s, _f) \
-DEFINE_PROP_UNSIGNED(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t)
+DEFINE_PROP_UNSIGNED(_n, _s, _f, 0, qdev_prop_blocksize, uint32_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
 DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
 #define DEFINE_PROP_OFF_AUTO_PCIBAR(_n, _s, _f, _d) \
-- 
2.24.1




Requesting review about optimizing large guest start up time

2020-02-11 Thread 陈蒙蒙
From c882b155466313fcd85ac330a45a573e608b0d74 Mon Sep 17 00:00:00 2001
From: bauerchen 
Date: Tue, 11 Feb 2020 17:10:35 +0800
Subject: [PATCH] Optimize: large guest start-up in mem-prealloc
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

[desc]:
    Large memory VM starts slowly when using -mem-prealloc, and
    there are some areas to optimize in current method;

    1、mmap will be used to alloc threads stack during create page
    clearing threads, and it will attempt mm->mmap_sem for write
    lock, but clearing threads have hold read lock, this competition
    will cause threads createion very slow;

    2、methods of calcuating pages for per threads is not well;if we use
    64 threads to split 160 hugepage,63 threads clear 2page,1 thread
    clear 34 page,so the entire speed is very slow;

    to solve the first problem,we add a mutex in thread function,and
    start all threads when all threads finished createion;
    and the second problem, we spread remainder to other threads,in
    situation that 160 hugepage and 64 threads, there are 32 threads
    clear 3 pages,and 32 threads clear 2 pages;
[test]:
    320G 84c VM start time can be reduced to 10s
    680G 84c VM start time can be reduced to 18s

Signed-off-by: bauerchen 
Reviewed-by:Pan Rui 
Reviewed-by:Ivan Ren 
---
 util/oslib-posix.c | 44 
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 5a291cc..e97369b 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -76,6 +76,10 @@ static MemsetThread *memset_thread;
 static int memset_num_threads;
 static bool memset_thread_failed;
 
+static QemuMutex page_mutex;
+static QemuCond page_cond;
+static volatile bool thread_create_flag;
+
 int qemu_get_thread_id(void)
 {
 #if defined(__linux__)
@@ -403,6 +407,14 @@ static void *do_touch_pages(void *arg)
     MemsetThread *memset_args = (MemsetThread *)arg;
     sigset_t set, oldset;
 
+    /*wait for all threads create finished */
+    qemu_mutex_lock(&page_mutex);
+    while(!thread_create_flag){
+        qemu_cond_wait(&page_cond, &page_mutex);
+    }
+    qemu_mutex_unlock(&page_mutex);
+
+
     /* unblock SIGBUS */
     sigemptyset(&set);
     sigaddset(&set, SIGBUS);
@@ -448,30 +460,46 @@ static inline int get_memset_num_threads(int smp_cpus)
     return ret;
 }
 
+static void calc_page_per_thread(size_t numpages, int memset_threads, size_t 
*pages_per_thread){
+    int avg = numpages / memset_threads + 1;
+    int i = 0;
+    int last = avg * memset_threads - numpages;
+    for (i = 0; i < memset_threads; i++)
+    {
+        if(memset_threads - i <= last){
+            pages_per_thread[i] = avg - 1;
+        }else
+            pages_per_thread[i] = avg;
+    }
+}
+
 static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
                             int smp_cpus)
 {
-    size_t numpages_per_thread;
-    size_t size_per_thread;
+    size_t *numpages_per_thread;
     char *addr = area;
     int i = 0;
 
     memset_thread_failed = false;
+    thread_create_flag = false;
     memset_num_threads = get_memset_num_threads(smp_cpus);
+    numpages_per_thread = g_new0(size_t, memset_num_threads);
     memset_thread = g_new0(MemsetThread, memset_num_threads);
-    numpages_per_thread = (numpages / memset_num_threads);
-    size_per_thread = (hpagesize * numpages_per_thread);
+    calc_page_per_thread(numpages, memset_num_threads, numpages_per_thread);
+
     for (i = 0; i < memset_num_threads; i++) {
         memset_thread[i].addr = addr;
-        memset_thread[i].numpages = (i == (memset_num_threads - 1)) ?
-                                    numpages : numpages_per_thread;
+        memset_thread[i].numpages = numpages_per_thread[i];
         memset_thread[i].hpagesize = hpagesize;
         qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
                            do_touch_pages, &memset_thread[i],
                            QEMU_THREAD_JOINABLE);
-        addr += size_per_thread;
-        numpages -= numpages_per_thread;
+        addr += numpages_per_thread[i] * hpagesize;
+        numpages -= numpages_per_thread[i];
     }
+    thread_create_flag = true;
+    qemu_cond_broadcast(&page_cond);
+
     for (i = 0; i < memset_num_threads; i++) {
         qemu_thread_join(&memset_thread[i].pgthread);
     }
-- 
1.8.3.1

[PATCH] tests/acceptance/ppc_prep_40p: Use cdn.netbsd.org hostname

2020-02-11 Thread Philippe Mathieu-Daudé
Use NetBSD content delivery network to get faster downloads.

Suggested-by: Kamil Rytarowski 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/acceptance/ppc_prep_40p.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
index efe06037ba..6729d96f5e 100644
--- a/tests/acceptance/ppc_prep_40p.py
+++ b/tests/acceptance/ppc_prep_40p.py
@@ -34,7 +34,7 @@ def test_factory_firmware_and_netbsd(self):
 '7020-40p/P12H0456.IMG')
 bios_hash = '1775face4e6dc27f3a6ed955ef6eb331bf817f03'
 bios_path = self.fetch_asset(bios_url, asset_hash=bios_hash)
-drive_url = ('https://ftp.netbsd.org/pub/NetBSD/NetBSD-archive/'
+drive_url = ('https://cdn.netbsd.org/pub/NetBSD/NetBSD-archive/'
  'NetBSD-4.0/prep/installation/floppy/generic_com0.fs')
 drive_hash = 'dbcfc09912e71bd5f0d82c7c1ee43082fb596ceb'
 drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash)
@@ -67,7 +67,7 @@ def test_openbios_and_netbsd(self):
 :avocado: tags=arch:ppc
 :avocado: tags=machine:40p
 """
-drive_url = ('https://ftp.netbsd.org/pub/NetBSD/iso/7.1.2/'
+drive_url = ('https://cdn.netbsd.org/pub/NetBSD/iso/7.1.2/'
  'NetBSD-7.1.2-prep.iso')
 drive_hash = 'ac6fa2707d888b36d6fa64de6e7fe48e'
 drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash,
-- 
2.21.1




Re: [PATCH] migration/rdma: rdma_accept_incoming_migration fix error handling

2020-02-11 Thread Peter Xu
On Tue, Feb 11, 2020 at 09:22:00AM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Mon, Feb 10, 2020 at 07:44:59PM +, Dr. David Alan Gilbert (git) 
> > wrote:
> > > From: "Dr. David Alan Gilbert" 
> > > 
> > > rdma_accept_incoming_migration is called from an fd handler and
> > > can't return an Error * anywhere.
> > > Currently it's leaking Error's in errp/local_err - there's
> > > no point putting them in there unless we can report them.
> > > 
> > > Turn most into fprintf's, and the last into an error_reportf_err
> > > where it's coming up from another function.
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert 
> > > ---
> > >  migration/rdma.c | 11 +++
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > index 2379b8345b..f67161c98f 100644
> > > --- a/migration/rdma.c
> > > +++ b/migration/rdma.c
> > > @@ -3980,13 +3980,13 @@ static void rdma_accept_incoming_migration(void 
> > > *opaque)
> > >  RDMAContext *rdma = opaque;
> > >  int ret;
> > >  QEMUFile *f;
> > > -Error *local_err = NULL, **errp = &local_err;
> > > +Error *local_err = NULL;
> > >  
> > >  trace_qemu_rdma_accept_incoming_migration();
> > >  ret = qemu_rdma_accept(rdma);
> > >  
> > >  if (ret) {
> > > -ERROR(errp, "RDMA Migration initialization failed!");
> > > +fprintf(stderr, "RDMA ERROR: Migration initialization failed");
> > 
> > Is there any reason to explictly use stderr instead of the
> > error_reportf_err() below (then we simply jump to that for error
> > paths)?  The only difference of error_reportf_err() and stderr should
> > be when there's one HMP, while shall we always suggest to use
> > error_reportf_err() rather than stderr?
> 
> Because the error_reportf_err is taking an Error* (from an error
> reported by migration_fd_process_incoming) where as we don't have an
> Error* at the earlier points.

The ERROR() macro in rdma.c created them?  Though it also prints to
stderr so if we also use the same error_reportf_err() then we can
remove that prints to stderr too.

-- 
Peter Xu




Re: [PATCH v15 0/9] VIRTIO-IOMMU device

2020-02-11 Thread Michael S. Tsirkin
On Sat, Feb 08, 2020 at 01:00:13PM +0100, Eric Auger wrote:
> This series implements the QEMU virtio-iommu device.
> 
> This matches the v0.12 spec (voted) and the corresponding
> virtio-iommu driver upstreamed in 5.3. All kernel dependencies
> are resolved for DT integration. The virtio-iommu can be
> instantiated in ARM virt using:
> 
> "-device virtio-iommu-pci".
> 
> Non DT mode is not yet supported as it has non resolved kernel
> dependencies [1].
> 
> This feature targets 5.0.
> 
> Integration with vhost devices and vfio devices is not part
> of this series. Please follow Bharat's respins [2].
> 
> Best Regards
> 
> Eric


Looks good.
Reviewed-by: Michael S. Tsirkin 

I can see this merged through ARM tree, or through my tree with
Peters's ack for the ARM bits.


> This series can be found at:
> https://github.com/eauger/qemu/tree/v4.2-virtio-iommu-v15
> 
> References:
> [1] [RFC 00/13] virtio-iommu on non-devicetree platforms
> [2] [PATCH RFC v5 0/5] virtio-iommu: VFIO integration
> 
> Testing:
> - tested with guest using virtio-net-pci
>   (,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on)
>   and virtio-blk-pci
> - migration
> 
> History:
> 
> v14 -> v15:
> - removed x-dt-binding and just kept check on hotplug_handler
> - removed "tests: Add virtio-iommu test" as the check on
>   hotplug_handler fails on PC machine
> - destroy mappings in put_domain and remove
>   g_tree_destroy(domain->mappings) in virtio_iommu_detach
> 
> v13 -> v14:
> - added "virtio-iommu-pci: Introduce the x-dt-binding option"
> - Removed the mappings gtree ref counting and simply delete
>   the gtree when the last EP is detached from the domain
> - call virtio_iommu_detach_endpoint_from_domain from
>   virtio_iommu_put_endpoint
> 
> v12 -> v13:
> - Take into account Peter's comments
> - fix qtest error and accomodate for directory changes in
>   test
> - remove "[PATCH v12 01/13] migration: Support QLIST migration"
>   which is now upstream
> - fix iommu_find_iommu_pcibus()
> - squash commits as requested by Peter
> - remove spurious guest log
> 
> ../..
> 
> Eric Auger (9):
>   virtio-iommu: Add skeleton
>   virtio-iommu: Decode the command payload
>   virtio-iommu: Implement attach/detach command
>   virtio-iommu: Implement map/unmap
>   virtio-iommu: Implement translate
>   virtio-iommu: Implement fault reporting
>   virtio-iommu-pci: Add virtio iommu pci support
>   hw/arm/virt: Add the virtio-iommu device tree mappings
>   virtio-iommu: Support migration
> 
>  hw/arm/virt.c|  57 +-
>  hw/virtio/Kconfig|   5 +
>  hw/virtio/Makefile.objs  |   2 +
>  hw/virtio/trace-events   |  20 +
>  hw/virtio/virtio-iommu-pci.c | 103 
>  hw/virtio/virtio-iommu.c | 890 +++
>  include/hw/arm/virt.h|   2 +
>  include/hw/pci/pci.h |   1 +
>  include/hw/virtio/virtio-iommu.h |  61 +++
>  qdev-monitor.c   |   1 +
>  10 files changed, 1135 insertions(+), 7 deletions(-)
>  create mode 100644 hw/virtio/virtio-iommu-pci.c
>  create mode 100644 hw/virtio/virtio-iommu.c
>  create mode 100644 include/hw/virtio/virtio-iommu.h
> 
> -- 
> 2.20.1




Re: [virtio-dev] Re: [PATCH v2 4/5] virtio-mmio: add MSI interrupt feature support

2020-02-11 Thread Michael S. Tsirkin
On Tue, Feb 11, 2020 at 08:18:54PM +0800, Jason Wang wrote:
> 
> On 2020/2/11 下午8:08, Michael S. Tsirkin wrote:
> > On Tue, Feb 11, 2020 at 08:04:24PM +0800, Jason Wang wrote:
> > > On 2020/2/11 下午7:58, Michael S. Tsirkin wrote:
> > > > On Tue, Feb 11, 2020 at 03:40:23PM +0800, Jason Wang wrote:
> > > > > On 2020/2/11 下午2:02, Liu, Jing2 wrote:
> > > > > > On 2/11/2020 12:02 PM, Jason Wang wrote:
> > > > > > > On 2020/2/11 上午11:35, Liu, Jing2 wrote:
> > > > > > > > On 2/11/2020 11:17 AM, Jason Wang wrote:
> > > > > > > > > On 2020/2/10 下午5:05, Zha Bin wrote:
> > > > > > > > > > From: Liu Jiang
> > > > > > > > > > 
> > > > > > > > > > Userspace VMMs (e.g. Qemu microvm, Firecracker) take
> > > > > > > > > > advantage of using
> > > > > > > > > > virtio over mmio devices as a lightweight machine model for 
> > > > > > > > > > modern
> > > > > > > > > > cloud. The standard virtio over MMIO transport layer
> > > > > > > > > > only supports one
> > > > > > > > > > legacy interrupt, which is much heavier than virtio over
> > > > > > > > > > PCI transport
> > > > > > > > > > layer using MSI. Legacy interrupt has long work path and
> > > > > > > > > > causes specific
> > > > > > > > > > VMExits in following cases, which would considerably slow 
> > > > > > > > > > down the
> > > > > > > > > > performance:
> > > > > > > > > > 
> > > > > > > > > > 1) read interrupt status register
> > > > > > > > > > 2) update interrupt status register
> > > > > > > > > > 3) write IOAPIC EOI register
> > > > > > > > > > 
> > > > > > > > > > We proposed to add MSI support for virtio over MMIO via new 
> > > > > > > > > > feature
> > > > > > > > > > bit VIRTIO_F_MMIO_MSI[1] which increases the interrupt 
> > > > > > > > > > performance.
> > > > > > > > > > 
> > > > > > > > > > With the VIRTIO_F_MMIO_MSI feature bit supported, the 
> > > > > > > > > > virtio-mmio MSI
> > > > > > > > > > uses msi_sharing[1] to indicate the event and vector 
> > > > > > > > > > mapping.
> > > > > > > > > > Bit 1 is 0: device uses non-sharing and fixed vector per
> > > > > > > > > > event mapping.
> > > > > > > > > > Bit 1 is 1: device uses sharing mode and dynamic mapping.
> > > > > > > > > I believe dynamic mapping should cover the case of fixed 
> > > > > > > > > vector?
> > > > > > > > > 
> > > > > > > > Actually this bit*aims*  for msi sharing or msi non-sharing.
> > > > > > > > 
> > > > > > > > It means, when msi sharing bit is 1, device doesn't want vector
> > > > > > > > per queue
> > > > > > > > 
> > > > > > > > (it wants msi vector sharing as name) and doesn't want a high
> > > > > > > > interrupt rate.
> > > > > > > > 
> > > > > > > > So driver turns to !per_vq_vectors and has to do dynamical 
> > > > > > > > mapping.
> > > > > > > > 
> > > > > > > > So they are opposite not superset.
> > > > > > > > 
> > > > > > > > Thanks!
> > > > > > > > 
> > > > > > > > Jing
> > > > > > > I think you need add more comments on the command.
> > > > > > > 
> > > > > > > E.g if I want to map vector 0 to queue 1, how do I need to do?
> > > > > > > 
> > > > > > > write(1, queue_sel);
> > > > > > > write(0, vector_sel);
> > > > > > That's true. Besides, two commands are used for msi sharing mode,
> > > > > > 
> > > > > > VIRTIO_MMIO_MSI_CMD_MAP_CONFIG and VIRTIO_MMIO_MSI_CMD_MAP_QUEUE.
> > > > > > 
> > > > > > "To set up the event and vector mapping for MSI sharing mode, driver
> > > > > > SHOULD write a valid MsiVecSel followed by
> > > > > > VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE 
> > > > > > command to
> > > > > > map the configuration change/selected queue events respectively.  " 
> > > > > > (See
> > > > > > spec patch 5/5)
> > > > > > 
> > > > > > So if driver detects the msi sharing mode, when it does setup vq, 
> > > > > > writes
> > > > > > the queue_sel (this already exists in setup vq), vector sel and then
> > > > > > MAP_QUEUE command to do the queue event mapping.
> > > > > > 
> > > > > So actually the per vq msix could be done through this. I don't get 
> > > > > why you
> > > > > need to introduce MSI_SHARING_MASK which is the charge of driver 
> > > > > instead of
> > > > > device. The interrupt rate should have no direct relationship with 
> > > > > whether
> > > > > it has been shared or not.
> > > > > 
> > > > > Btw, you introduce mask/unmask without pending, how to deal with the 
> > > > > lost
> > > > > interrupt during the masking then?
> > > > pending can be an internal device register. as long as device
> > > > does not lose interrupts while masked, all's well.
> > > 
> > > You meant raise the interrupt during unmask automatically?
> > > 
> > 
> > yes - that's also what pci does.
> > 
> > the guest visible pending bit is partially implemented in qemu
> > as per pci spec but it's unused.
> 
> 
> Ok.
> 
> 
> > 
> > > > There's value is being able to say "this queue sends no
> > > > interrupts do not bother checking used notification area".
> > > > so we need way to say that. So I guess an enable interrupts
> > > > register

[PATCH] pc: remove erroneous seg_max_adjust setting for vhost-blk-device

2020-02-11 Thread Denis Plotnikov
vhost-blk-device isn't a part of qemu.git

Signed-off-by: Denis Plotnikov 
---
 hw/core/machine.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d8e30e4895..2501b540ec 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -31,7 +31,6 @@ GlobalProperty hw_compat_4_2[] = {
 { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
 { "virtio-blk-device", "seg-max-adjust", "off"},
 { "virtio-scsi-device", "seg_max_adjust", "off"},
-{ "vhost-blk-device", "seg_max_adjust", "off"},
 { "usb-host", "suppress-remote-wake", "off" },
 { "usb-redir", "suppress-remote-wake", "off" },
 };
-- 
2.17.0




[PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk

2020-02-11 Thread Denis Plotnikov
The goal is to reduce the amount of requests issued by a guest on
1M reads/writes. This rises the performance up to 4% on that kind of
disk access pattern.

The maximum chunk size to be used for the guest disk accessing is
limited with seg_max parameter, which represents the max amount of
pices in the scatter-geather list in one guest disk request.

Since seg_max is virqueue_size dependent, increasing the virtqueue
size increases seg_max, which, in turn, increases the maximum size
of data to be read/write from a guest disk.

More details in the original problem statment:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html

Suggested-by: Denis V. Lunev 
Signed-off-by: Denis Plotnikov 
---
 hw/block/virtio-blk.c | 4 ++--
 hw/core/machine.c | 2 ++
 hw/scsi/virtio-scsi.c | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..6df3a7a6df 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, 
uint8_t *config)
 memset(&blkcfg, 0, sizeof(blkcfg));
 virtio_stq_p(vdev, &blkcfg.capacity, capacity);
 virtio_stl_p(vdev, &blkcfg.seg_max,
- s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
+ s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 256 - 2);
 virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
 virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
 virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
@@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
 DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
 true),
 DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
-DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
 DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
 DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
  IOThread *),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2501b540ec..3427d6cf4c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@
 #include "hw/mem/nvdimm.h"
 
 GlobalProperty hw_compat_4_2[] = {
+{ "virtio-blk-device", "queue-size", "128"},
+{ "virtio-scsi-device", "virtqueue_size", "128"},
 { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
 { "virtio-blk-device", "seg-max-adjust", "off"},
 { "virtio-scsi-device", "seg_max_adjust", "off"},
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..b38f50a429 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -660,7 +660,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
 
 virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
 virtio_stl_p(vdev, &scsiconf->seg_max,
- s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 
2);
+ s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 256 - 
2);
 virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
 virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
 virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
@@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, 
Error **errp)
 static Property virtio_scsi_properties[] = {
 DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 
1),
 DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
- parent_obj.conf.virtqueue_size, 128),
+ parent_obj.conf.virtqueue_size, 256),
 DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
   parent_obj.conf.seg_max_adjust, true),
 DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
-- 
2.17.0




[Bug 1663079] Re: socket network not working

2020-02-11 Thread Thomas Huth
Works for me with the current QEMU master branch... can you still
reproduce the issue with the latest version of QEMU (either v4.2 or the
master branch)?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  socket network not working

Status in QEMU:
  Incomplete

Bug description:
  The socket network type is no longer working in 2.8.0.

  When trying to establish a network between 2 qemu instances:

  The listening host:
  qemu-system-x86_64 -netdev socket,id=in0,listen=127.0.0.1: -device 
virtio-net-pci,netdev=in0

  works fine, but for the second one:

  qemu-system-x86_64 -netdev socket,id=in0,connect=127.0.0.1:
  -device virtio-net-pci,netdev=in0

  It fails with:

  qemu-system-x86_64: -device virtio-net-pci,netdev=in0: Property
  'virtio-net-device.netdev' can't find value 'in0'

  netstat shows a new connection to port  in time_wait state every
  time.

  host: kernel 4.4.38, 64bits.

  It was working fine with previous version.

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



[Bug 1657538] Re: qemu 2.7.x 2.8 softmmu dont work on BE machine

2020-02-11 Thread Thomas Huth
Looking through old bug tickets ... can you still reproduce the issue
with the latest version of QEMU (v4.2) and a more recent Linux
distribution?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  qemu 2.7.x 2.8 softmmu dont work on BE machine

Status in QEMU:
  Incomplete

Bug description:
  Build on Be machine qemu 2.7.1 and 2.8 in pure softmmu (tgc) dont work on big 
endian hardware .
  tested with ppc-softmmu,i386-softmmu,arm-softmmu same result:

  with :
   ./qemu-system-i386 
  Gtk-Message: Failed to load module "overlay-scrollbar"
  qemu-system-i386: Trying to execute code outside RAM or ROM at 0x000a
  This usually means one of the following happened:

  (1) You told QEMU to execute a kernel for the wrong machine type, and it 
crashed on startup (eg trying to run a raspberry pi kernel on a versatilepb 
QEMU machine)
  (2) You didn't give QEMU a kernel or BIOS filename at all, and QEMU executed 
a ROM full of no-op instructions until it fell off the end
  (3) Your guest kernel has a bug and crashed by jumping off into nowhere

  This is almost always one of the first two, so check your command line and 
that you are using the right type of kernel for this machine.
  If you think option (3) is likely then you can try debugging your guest with 
the -d debug options; in particular -d guest_errors will cause the log to 
include a dump of the guest register state at this point.

  Execution cannot continue; stopping here.

  
  I try to add the -L option with ../pc-bios/bios.bin 
  and have the same result.

  note the ppc-softmmu and ppc64-softmmu work in kvm mode only emulated
  mode have issue.

  
  tested on my hardware a  Qriq P5040 and G5 4x970MP with Ubuntu Mate 16.10 
  thanks
  Luigi

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



[PATCH v1 2/2] tests/acceptance/virtio_check_params: prepare to check different params

2020-02-11 Thread Denis Plotnikov
Signed-off-by: Denis Plotnikov 
---
 tests/acceptance/virtio_check_params.py | 38 ++---
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/tests/acceptance/virtio_check_params.py 
b/tests/acceptance/virtio_check_params.py
index deec89bf86..e578952a97 100644
--- a/tests/acceptance/virtio_check_params.py
+++ b/tests/acceptance/virtio_check_params.py
@@ -43,7 +43,7 @@ VM_DEV_PARAMS = {'virtio-scsi-pci': ['-device', 
'virtio-scsi-pci,id=scsi0'],
 EXCLUDED_MACHINES = ['none', 'isapc', 'microvm']
 
 
-class VirtioMaxSegSettingsCheck(Test):
+class VirtioParamsCheck(Test):
 @staticmethod
 def make_pattern(props):
 pattern_items = ['{0} = \w+'.format(prop) for prop in props]
@@ -75,12 +75,12 @@ class VirtioMaxSegSettingsCheck(Test):
 props[p[0]] = p[1]
 return query_ok, props, error
 
-def check_mt(self, mt, dev_type_name):
-mt['device'] = dev_type_name # Only for the debug() call.
+def check_mt(self, mt, expected_vals, dev_type_name):
+msg = "mt: %s dev: %s" % (mt, dev_type_name) # For debug() call only.
 logger = logging.getLogger('machine')
-logger.debug(mt)
+logger.debug(msg)
 with QEMUMachine(self.qemu_bin) as vm:
-vm.set_machine(mt["name"])
+vm.set_machine(mt)
 vm.add_args('-nodefaults')
 for s in VM_DEV_PARAMS[dev_type_name]:
 vm.add_args(s)
@@ -92,11 +92,15 @@ class VirtioMaxSegSettingsCheck(Test):
 error = sys.exc_info()[0]
 
 if not query_ok:
-self.fail('machine type {0}: {1}'.format(mt['name'], error))
+self.fail('machine type {0}: {1}'.format(mt, error))
 
 for prop_name, prop_val in props.items():
-expected_val = mt[prop_name]
-self.assertEqual(expected_val, prop_val)
+expected_val = expected_vals[prop_name]
+msg = 'Property value mismatch for (MT: {0}, '\
+  'property name: {1}): expected value: "{2}" '\
+  'actual value: "{3}"'\
+  .format(mt, prop_name, expected_val, prop_val)
+self.assertEqual(expected_val, prop_val, msg)
 
 @staticmethod
 def seg_max_adjust_enabled(mt):
@@ -128,25 +132,27 @@ class VirtioMaxSegSettingsCheck(Test):
 
 @skip("break multi-arch CI")
 def test_machine_types(self):
-# collect all machine types except 'none', 'isapc', 'microvm'
+# collect all machine types
 with QEMUMachine(self.qemu_bin) as vm:
 vm.launch()
 machines = [m['name'] for m in vm.command('query-machines')]
 vm.shutdown()
 
+# ..and exclude non-relevant ones
 machines = self.filter_machines(machines)
 
 for dev_type in DEV_TYPES:
-# create the list of machine types and their parameters.
-mtypes = list()
+# define expected parameters for each machine type
+mt_expected_vals = dict()
 for m in machines:
 if self.seg_max_adjust_enabled(m):
 enabled = 'true'
 else:
 enabled = 'false'
-mtypes.append({'name': m,
-   DEV_TYPES[dev_type]['seg_max_adjust']: enabled})
 
-# test each machine type for a device type
-for mt in mtypes:
-self.check_mt(mt, dev_type)
+mt_expected_vals[m] = {
+DEV_TYPES[dev_type]['seg_max_adjust']: enabled }
+
+# test each machine type
+for mt in mt_expected_vals:
+self.check_mt(mt, mt_expected_vals[mt], dev_type)
-- 
2.17.0




[PATCH v1 1/2] tests/acceptance/virtio_check_params: remove excluded machine types carefully

2020-02-11 Thread Denis Plotnikov
Before, the test failed if an excluded machine type was absent in the machine
types lists.

Signed-off-by: Denis Plotnikov 
---
 tests/acceptance/virtio_check_params.py | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/acceptance/virtio_check_params.py 
b/tests/acceptance/virtio_check_params.py
index 87e6c839d1..deec89bf86 100644
--- a/tests/acceptance/virtio_check_params.py
+++ b/tests/acceptance/virtio_check_params.py
@@ -40,6 +40,8 @@ VM_DEV_PARAMS = {'virtio-scsi-pci': ['-device', 
'virtio-scsi-pci,id=scsi0'],
 '-drive',
 'driver=null-co,id=drive0,if=none']}
 
+EXCLUDED_MACHINES = ['none', 'isapc', 'microvm']
+
 
 class VirtioMaxSegSettingsCheck(Test):
 @staticmethod
@@ -117,6 +119,13 @@ class VirtioMaxSegSettingsCheck(Test):
 return True
 return False
 
+@staticmethod
+def filter_machines(machines):
+for mt in EXCLUDED_MACHINES:
+if mt in machines:
+machines.remove(mt)
+return machines
+
 @skip("break multi-arch CI")
 def test_machine_types(self):
 # collect all machine types except 'none', 'isapc', 'microvm'
@@ -124,9 +133,8 @@ class VirtioMaxSegSettingsCheck(Test):
 vm.launch()
 machines = [m['name'] for m in vm.command('query-machines')]
 vm.shutdown()
-machines.remove('none')
-machines.remove('isapc')
-machines.remove('microvm')
+
+machines = self.filter_machines(machines)
 
 for dev_type in DEV_TYPES:
 # create the list of machine types and their parameters.
-- 
2.17.0




[PATCH v1 0/2] Improve virtio_check_params test

2020-02-11 Thread Denis Plotnikov
* fixed failing on non-existed machine type removal
* the test refactored to add more parameters to check

Gereral questions left:
   How to restric test for using:
   1. on a set of target OS-es
   2. on a set target architectures
  

Denis Plotnikov (2):
  tests/acceptance/virtio_check_params: remove excluded machine types
carefully
  tests/acceptance/virtio_check_params: prepare to check different
params

 tests/acceptance/virtio_check_params.py | 52 -
 1 file changed, 33 insertions(+), 19 deletions(-)

-- 
2.17.0




Re: [RFC 1/2] tpm: Let the TPM TIS device be usable on ARM

2020-02-11 Thread Stefan Berger

On 2/11/20 5:56 AM, Peter Maydell wrote:

On Tue, 11 Feb 2020 at 08:35, Auger Eric  wrote:

Hi Philippe,

On 2/11/20 9:25 AM, Philippe Mathieu-Daudé wrote:

You don't need much to remove the RFC tag:

- rename TYPE_TPM_TIS as TYPE_TPM_TIS_ISA
- rename TPMState as TPMCommonState, add an abstract TYPE_TPM_TIS
parent, let TYPE_TPM_TIS_ISA be a child
- add TYPE_TPM_TIS_SYSBUS also child.

Yes I tried my luck without too much hopes ;-)

There should be a few existing examples in the tree
of devices that we provide in a sysbus and also
an isa or pci flavour, that you can use as templates
for how to structure the device.


block/fdc.c ?


   Stefan





Re: Cross-project NBD extension proposal: NBD_INFO_INIT_STATE

2020-02-11 Thread Eric Blake

On 2/10/20 4:52 PM, Richard W.M. Jones wrote:

On Mon, Feb 10, 2020 at 04:29:53PM -0600, Eric Blake wrote:

On 2/10/20 4:12 PM, Richard W.M. Jones wrote:

On Mon, Feb 10, 2020 at 03:37:20PM -0600, Eric Blake wrote:

For now, only 2 of those 16 bits are defined: NBD_INIT_SPARSE (the
image has at least one hole) and NBD_INIT_ZERO (the image reads
completely as zero); the two bits are orthogonal and can be set
independently, although it is easy enough to see completely sparse
files with both bits set.


I think I'm confused about the exact meaning of NBD_INIT_SPARSE.  Do
you really mean the whole image is sparse; or (as you seem to have
said above) that there exists a hole somewhere in the image but we're
not saying where it is and there can be non-sparse parts of the image?


As implemented:

NBD_INIT_SPARSE - there is at least one hole somewhere (allocation
would be required to write to that part of the file), but there may
b allocated data elsewhere in the image.  Most disk images will fit
this definition (for example, it is very common to have a hole
between the MBR or GPT and the first partition containing a file
system, or for file systems themselves to be sparse within the
larger block device).


I think I'm still confused about why this particular flag would be
useful for clients (I can completely understand why clients need
NBD_INIT_ZERO).

But anyway ... could a flag indicating that the whole image is sparse
be useful, either as well as NBD_INIT_SPARSE or instead of it?  You
could use it to avoid an initial disk trim, which is something that
mke2fs does:

   
https://github.com/tytso/e2fsprogs/blob/0670fc20df4a4bbbeb0edb30d82628ea30a80598/misc/mke2fs.c#L2768


I'm open to suggestions on how many initial bits should be provided.  In 
fact, if we wanted, we could have a pair mutually-exclusive bits, 
advertising:

00 - no information known
01 - image is completely sparse
10 - image is completely allocated
11 - error

The goal of providing a 16-bit answer (or we could mandate 32 or 64 
bits, if we think we will ever want to extend that far) was to make it 
easier to add in whatever other initial-state extensions that someone 
could find useful.  Until we're happy with the design, the size or any 
given bit assignment is not locked down; once we do start committing any 
of this series, we've locked in what interoperability will demand but 
still have spare bits as future extensions.


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




RE: [RFC PATCH 55/66] Hexagon HVX import instruction encodings

2020-02-11 Thread Taylor Simpson
Hi Philippe,

Thanks for all your feedback.  I' get your comments addressed soon.

I'm sorry you had trouble applying this patch.  The 
target/hexagon/imported/encode.def should have been created by patch 17.  Did 
you apply that one?

Taylor



> -Original Message-
> From: Philippe Mathieu-Daudé 
> Sent: Tuesday, February 11, 2020 1:02 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; laur...@vivier.eu; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com
> Subject: Re: [RFC PATCH 55/66] Hexagon HVX import instruction encodings
>
> > +#define EXTNAME mmvec
> > +#include "mmvec/encode_ext.def"
> > +#undef EXTNAME
> > diff --git a/target/hexagon/imported/encode.def
> b/target/hexagon/imported/encode.def
> > index 33c3396..d1b3510 100644
> > --- a/target/hexagon/imported/encode.def
> > +++ b/target/hexagon/imported/encode.def
> > @@ -71,6 +71,7 @@
> >
> >   #include "encode_pp.def"
> >   #include "encode_subinsn.def"
> > +#include "allextenc.def"
> >
> >   #ifdef __SELF_DEF_FIELD32
> >   #undef __SELF_DEF_FIELD32
>
> I had a problem applying this one:
>
> Applying: Hexagon HVX import instruction encodings
> error: target/hexagon/imported/encode.def: does not exist in index
> Patch failed at 0054 Hexagon HVX import instruction encodings
> hint: Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> Resolved by doing:
>
> (review/hexagon|AM 54/65)$ touch target/hexagon/imported/encode.def
> (review/hexagon|AM 54/65)$ git add target/hexagon/imported/encode.def
> (review/hexagon|AM 54/65)$ git am --continue
> Applying: Hexagon HVX import instruction encodings
> Applying: Hexagon HVX import semantics
> ...



Re: [PATCH v1 0/2] Improve virtio_check_params test

2020-02-11 Thread Philippe Mathieu-Daudé

Hi Denis,

On 2/11/20 3:25 PM, Denis Plotnikov wrote:

* fixed failing on non-existed machine type removal
* the test refactored to add more parameters to check

Gereral questions left:
How to restric test for using:
1. on a set of target OS-es
2. on a set target architectures
   


Denis Plotnikov (2):
   tests/acceptance/virtio_check_params: remove excluded machine types
 carefully
   tests/acceptance/virtio_check_params: prepare to check different
 params

  tests/acceptance/virtio_check_params.py | 52 -
  1 file changed, 33 insertions(+), 19 deletions(-)



Have you noticed my other series suggested by Cornelia?

It runs your test on S390X and PPC:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675092.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675095.html




Re: [RFC PATCH 55/66] Hexagon HVX import instruction encodings

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/11/20 3:35 PM, Taylor Simpson wrote:

Hi Philippe,

Thanks for all your feedback.  I' get your comments addressed soon.

I'm sorry you had trouble applying this patch.


No problem.


The target/hexagon/imported/encode.def should have been created by patch 17.  
Did you apply that one?


Ah, I haven't received patch #17, and can't find it on the list.



Taylor




-Original Message-
From: Philippe Mathieu-Daudé 
Sent: Tuesday, February 11, 2020 1:02 AM
To: Taylor Simpson ; qemu-devel@nongnu.org
Cc: richard.hender...@linaro.org; laur...@vivier.eu; riku.voi...@iki.fi;
aleksandar.m.m...@gmail.com
Subject: Re: [RFC PATCH 55/66] Hexagon HVX import instruction encodings


+#define EXTNAME mmvec
+#include "mmvec/encode_ext.def"
+#undef EXTNAME
diff --git a/target/hexagon/imported/encode.def

b/target/hexagon/imported/encode.def

index 33c3396..d1b3510 100644
--- a/target/hexagon/imported/encode.def
+++ b/target/hexagon/imported/encode.def
@@ -71,6 +71,7 @@

   #include "encode_pp.def"
   #include "encode_subinsn.def"
+#include "allextenc.def"

   #ifdef __SELF_DEF_FIELD32
   #undef __SELF_DEF_FIELD32


I had a problem applying this one:

Applying: Hexagon HVX import instruction encodings
error: target/hexagon/imported/encode.def: does not exist in index
Patch failed at 0054 Hexagon HVX import instruction encodings
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Resolved by doing:

(review/hexagon|AM 54/65)$ touch target/hexagon/imported/encode.def
(review/hexagon|AM 54/65)$ git add target/hexagon/imported/encode.def
(review/hexagon|AM 54/65)$ git am --continue
Applying: Hexagon HVX import instruction encodings
Applying: Hexagon HVX import semantics
...







[Bug 1661815] Re: Stack address is returned from function translate_one

2020-02-11 Thread Thomas Huth
Fixed here:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=344a7f656e8d211cdd6e

** Changed in: qemu
   Status: New => Fix Committed

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

Title:
  Stack address is returned from function translate_one

Status in QEMU:
  Fix Committed

Bug description:
  The vulnerable version is qemu-2.8.0, and the vulnerable function is
  in "target-s390x/translate.c".

  The code snippet is as following.

  static ExitStatus translate_one(CPUS390XState *env, DisasContext *s)
  {
  const DisasInsn *insn;
  ExitStatus ret = NO_EXIT;
  DisasFields f;
  ...
  s->fields = &f;
  ...
  s->pc = s->next_pc;
  return ret;
  }

  A stack address, i.e. the address of local variable "f" is returned
  from current function through the output parameter "s->fields" as a
  side effect.

  This issue is one kind of undefined behaviors, according the C
  Standard, 6.2.4 [ISO/IEC 9899:2011]
  
(https://www.securecoding.cert.org/confluence/display/c/DCL30-C.+Declare+objects+with+appropriate+storage+durations)

  This dangerous defect may lead to an exploitable vulnerability.
  We suggest sanitizing "s->fields" as null before return.

  Note that this issue is reported by shqking and Zhenwei Zou together.

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



Re: [RFC PATCH 55/66] Hexagon HVX import instruction encodings

2020-02-11 Thread Philippe Mathieu-Daudé
On Tue, Feb 11, 2020 at 3:40 PM Philippe Mathieu-Daudé
 wrote:
>
> On 2/11/20 3:35 PM, Taylor Simpson wrote:
> > Hi Philippe,
> >
> > Thanks for all your feedback.  I' get your comments addressed soon.
> >
> > I'm sorry you had trouble applying this patch.
>
> No problem.
>
> > The target/hexagon/imported/encode.def should have been created by patch 
> > 17.  Did you apply that one?
>
> Ah, I haven't received patch #17, and can't find it on the list.

Oops I was looking at incorrect list =)

Here it is:
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg02526.html

And patchew received it...
https://patchew.org/QEMU/1581381644-13678-1-git-send-email-tsimp...@quicinc.com/

>
> >
> > Taylor
> >
> >
> >
> >> -Original Message-
> >> From: Philippe Mathieu-Daudé 
> >> Sent: Tuesday, February 11, 2020 1:02 AM
> >> To: Taylor Simpson ; qemu-devel@nongnu.org
> >> Cc: richard.hender...@linaro.org; laur...@vivier.eu; riku.voi...@iki.fi;
> >> aleksandar.m.m...@gmail.com
> >> Subject: Re: [RFC PATCH 55/66] Hexagon HVX import instruction encodings
> >>
> >>> +#define EXTNAME mmvec
> >>> +#include "mmvec/encode_ext.def"
> >>> +#undef EXTNAME
> >>> diff --git a/target/hexagon/imported/encode.def
> >> b/target/hexagon/imported/encode.def
> >>> index 33c3396..d1b3510 100644
> >>> --- a/target/hexagon/imported/encode.def
> >>> +++ b/target/hexagon/imported/encode.def
> >>> @@ -71,6 +71,7 @@
> >>>
> >>>#include "encode_pp.def"
> >>>#include "encode_subinsn.def"
> >>> +#include "allextenc.def"
> >>>
> >>>#ifdef __SELF_DEF_FIELD32
> >>>#undef __SELF_DEF_FIELD32
> >>
> >> I had a problem applying this one:
> >>
> >> Applying: Hexagon HVX import instruction encodings
> >> error: target/hexagon/imported/encode.def: does not exist in index
> >> Patch failed at 0054 Hexagon HVX import instruction encodings
> >> hint: Use 'git am --show-current-patch' to see the failed patch
> >> When you have resolved this problem, run "git am --continue".
> >> If you prefer to skip this patch, run "git am --skip" instead.
> >> To restore the original branch and stop patching, run "git am --abort".
> >>
> >> Resolved by doing:
> >>
> >> (review/hexagon|AM 54/65)$ touch target/hexagon/imported/encode.def
> >> (review/hexagon|AM 54/65)$ git add target/hexagon/imported/encode.def
> >> (review/hexagon|AM 54/65)$ git am --continue
> >> Applying: Hexagon HVX import instruction encodings
> >> Applying: Hexagon HVX import semantics
> >> ...
> >




Re: [PATCH v1 0/2] Improve virtio_check_params test

2020-02-11 Thread Denis Plotnikov




On 11.02.2020 17:37, Philippe Mathieu-Daudé wrote:

Hi Denis,

On 2/11/20 3:25 PM, Denis Plotnikov wrote:

* fixed failing on non-existed machine type removal
* the test refactored to add more parameters to check

Gereral questions left:
    How to restric test for using:
    1. on a set of target OS-es
    2. on a set target architectures

Denis Plotnikov (2):
   tests/acceptance/virtio_check_params: remove excluded machine types
 carefully
   tests/acceptance/virtio_check_params: prepare to check different
 params

  tests/acceptance/virtio_check_params.py | 52 -
  1 file changed, 33 insertions(+), 19 deletions(-)



Have you noticed my other series suggested by Cornelia?

It runs your test on S390X and PPC:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675092.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675095.html

Hi, Philippe

Seems that I've missed them. I just made patches upon the fresh master.
Can I get a git tree which has those patches applied? Or should I wait 
while the patches landed to qemu master and the rebase on them?


Denis



[Bug 1661386] Re: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed

2020-02-11 Thread Thomas Huth
There was a fix for this assertion message wrt PMU registers in December 2017 
already:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=0b368a10c71af96f6cf
Thus, can you still reproduce this issue with the latest version of QEMU, or is 
the problem gone now?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed

Status in QEMU:
  Incomplete

Bug description:
  Hello,

  
  I see the following when try to run qemu from master as the following:

  # ./x86_64-softmmu/qemu-system-x86_64 --version
  QEMU emulator version 2.8.50 (v2.8.0-1006-g4e9f524)
  Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers
  # ./x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -nodefaults
  -no-reboot -nographic -cpu host -vga none  -kernel .build.kernel.kvm
  -initrd .build.initrd.kvm -append 'panic=1 no-kvmclock console=ttyS0
  loglevel=7' -m 1024 -serial stdio
  qemu-system-x86_64: /home/matwey/lab/qemu/target/i386/kvm.c:1849:
  kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.

  First broken commit has been bisected:

  commit 48e1a45c3166d659f781171a47dabf4a187ed7a5
  Author: Paolo Bonzini 
  Date:   Wed Mar 30 22:55:29 2016 +0200

  target-i386: assert that KVM_GET/SET_MSRS can set all requested MSRs
  
  This would have caught the bug in the previous patch.
  
  Signed-off-by: Paolo Bonzini 

  My cpuinfo is the following:

  processor   : 0
  vendor_id   : GenuineIntel
  cpu family  : 6
  model   : 44
  model name  : Intel(R) Xeon(R) CPU   X5675  @ 3.07GHz
  stepping: 2
  microcode   : 0x14
  cpu MHz : 3066.775
  cache size  : 12288 KB
  physical id : 0
  siblings: 2
  core id : 0
  cpu cores   : 2
  apicid  : 0
  initial apicid  : 0
  fpu : yes
  fpu_exception   : yes
  cpuid level : 11
  wp  : yes
  flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts mmx fxsr sse sse2 ss ht syscall nx rdtscp lm 
constant_tsc arch_perfmon pebs bts nopl xtopology tsc_reliable nonstop_tsc 
aperfmperf pni pclmulqdq vmx ssse3 cx16 sse4_1 sse4_2 popcnt aes hypervisor 
lahf_lm ida arat epb dtherm tpr_shadow vnmi ept vpid
  bugs:
  bogomips: 6133.55
  clflush size: 64
  cache_alignment : 64
  address sizes   : 40 bits physical, 48 bits virtual
  power management:

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



[Bug 1665389] Re: Nested kvm guest fails to start on a emulated Westmere CPU guest under a Broadwell CPU host

2020-02-11 Thread Thomas Huth
Can you still reproduce this issue with the latest version of QEMU
(v4.2)?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Nested kvm guest fails to start on a emulated Westmere CPU guest under
  a Broadwell CPU host

Status in QEMU:
  Incomplete

Bug description:
  Using latest master(5dae13), qemu fails to start any nested guest in a
  Westmere emulated guest(layer 1), under a Broadwell host(layer 0),
  with the error:

  qemu-custom: /root/qemu/target/i386/kvm.c:1849: kvm_put_msrs:
  Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.

  The qemu command used(though other CPUs didn't work either):
  /usr/bin/qemu-custom -name guest=12ed9230-vm-el73,debug-threads=on -S -object 
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-5-12ed9230-vm-el73/master-key.aes
 -machine pc-i440fx-2.9,accel=kvm,usb=off -cpu Westmere,+vmx -m 512 -realtime 
mlock=off -smp 2,sockets=2,cores=1,threads=1 -object iothread,id=iothread1 
-uuid f4ce4eba-985f-42a3-94c4-6e4a8a530347 -nographic -no-user-config 
-nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-5-12ed9230-vm-el73/monitor.sock,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-boot menu=off,strict=on -device 
virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -drive 
file=/root/lago/.lago/default/images/vm-el73_root.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,serial=1,discard=unmap
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -netdev tap,fd=26,id=hostnet0,vhost=on,vhostfd=28 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=54:52:c0:a7:c8:02,bus=pci.0,addr=0x2 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-chardev 
socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-5-12ed9230-vm-el73/org.qemu.guest_agent.0,server,nowait
 -device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
 -object rng-random,id=objrng0,filename=/dev/random -device 
virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0x9 -msg timestamp=on
  2017-02-16T15:14:45.840412Z qemu-custom: -chardev pty,id=charserial0: char 
device redirected to /dev/pts/2 (label charserial0)
  qemu-custom: /root/qemu/target/i386/kvm.c:1849: kvm_put_msrs: Assertion `ret 
== cpu->kvm_msr_buf->nmsrs' failed.

  
  The CPU flags in the Westmere guest:
  flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush mmx fxsr sse sse2 syscall nx lm constant_tsc rep_good nopl 
pni pclmulqdq vmx ssse3 cx16 sse4_1 sse4_2 x2apic popcnt aes hypervisor lahf_lm 
arat tpr_shadow vnmi flexpriority ept vpid

  The guest kernel is 3.10.0-514.2.2.el7.x86_64.

  The CPU flags of the host(Broadwell): 
  flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb 
rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology 
nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est 
tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt 
tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch epb 
intel_pt tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle 
avx2 smep bmi2 erms invpcid rtm mpx rdseed adx smap clflushopt xsaveopt xsavec 
xgetbv1 xsaves dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp

  qemu command on the host - Broadwell(which works):
  /usr/bin/qemu-kvm -name 4ffcd448-vm-el73,debug-threads=on -S -machine 
pc-i440fx-2.6,accel=kvm,usb=off -cpu Westmere,+x2apic,+vmx,+vme -m 4096 
-realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -object 
iothread,id=iothread1 -uuid 8cc0a2cf-d25a-4014-acdb-f159c376a532 -nographic 
-no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-4-4ffcd448-vm-el73/monitor.sock,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-boot menu=off,strict=on -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -drive 
file=/home/ngoldin/src/nvgoldin.github.com/lago-init-files/.lago/flags-tests/default/images/vm-el73_root.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,serial=1,discard=unmap
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -drive 
file=/home/ngoldin/src/nvgoldin.github.com/lago-init-files/.lago/flags-tests/default/images/vm-el73_additonal.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0,serial=2,discard=unmap
 -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2
 -netdev tap,fd=29,id=hostnet0,vhost

[Bug 1649040] Re: Ubuntu 16.04.1 Grub Splash Doesn't Appear

2020-02-11 Thread Thomas Huth
Does this still happen with a newer version of Ubuntu and the latest
version of QEMU?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Ubuntu 16.04.1 Grub Splash Doesn't Appear

Status in QEMU:
  Incomplete

Bug description:
  My Specs:

  Slackware 14.2 x86_64 > Host
  QEMU 2.7.0

  Ubuntu 16.04.1 x86_64 > Guest

  Start options for Ubuntu:

  qemu-system-x86_64 -drive format=raw,file=ubuntu.img \
  -cpu host \
  --enable-kvm \
  -smp 2 \
  -m 4096 \
  -vga vmware \
  -soundhw ac97 \
  -usbdevice tablet \
  -rtc base=localtime \
  -usbdevice host:0781:5575

  I've started Ubuntu around 6-8 times, and I have only see the Grub
  Boot Splash appear twice, so pretty much without fail it typically
  boots past the grub splash and automatically boots...

  These are the /etc/default/grub settings; (I only changed these
  options GRUB_TIMEOUT=15 and GRUB_GFXMODE=1440x900)

  # If you change this file, run 'update-grub' afterwards to update
  # /boot/grub/grub.cfg.
  # For full documentation of the options in this file, see:
  #   info -f grub -n 'Simple configuration'

  GRUB_DEFAULT=0
  GRUB_HIDDEN_TIMEOUT=0
  GRUB_HIDDEN_TIMEOUT_QUIET=true
  GRUB_TIMEOUT=15
  GRUB_DISTRIBUTOR=`lsb_release -i -s 2> /dev/null || echo Debian`
  GRUB_CMDLINE_LINUX_DEFAULT="quiet splash"
  GRUB_CMDLINE_LINUX=""

  # Uncomment to enable BadRAM filtering, modify to suit your needs
  # This works with Linux (no patch required) and with any kernel that obtains
  # the memory map information from GRUB (GNU Mach, kernel of FreeBSD ...)
  #GRUB_BADRAM="0x01234567,0xfefefefe,0x89abcdef,0xefefefef"

  # Uncomment to disable graphical terminal (grub-pc only)
  #GRUB_TERMINAL=console

  # The resolution used on graphical terminal
  # note that you can use only modes which your graphic card supports via VBE
  # you can see them in real GRUB with the command `vbeinfo'
  GRUB_GFXMODE=1440x900

  # Uncomment if you don't want GRUB to pass "root=UUID=xxx" parameter to Linux
  #GRUB_DISABLE_LINUX_UUID=true

  # Uncomment to disable generation of recovery mode menu entries
  #GRUB_DISABLE_RECOVERY="true"

  # Uncomment to get a beep at grub start
  #GRUB_INIT_TUNE="480 440 1"

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



Re: [PATCH v15 8/9] hw/arm/virt: Add the virtio-iommu device tree mappings

2020-02-11 Thread Peter Maydell
On Sat, 8 Feb 2020 at 12:01, Eric Auger  wrote:
>
> Adds the "virtio,pci-iommu" node in the host bridge node and
> the RID mapping, excluding the IOMMU RID.
>
> This is done in the virtio-iommu-pci hotplug handler which
> gets called only if no firmware is loaded or if -no-acpi is
> passed on the command line. As non DT integration is
> not yet supported by the kernel we must make sure we
> are in DT mode. This limitation will be removed as soon
> as the topology description feature gets supported.
>
> Signed-off-by: Eric Auger 
>
> +static void create_virtio_iommu(VirtMachineState *vms, Error **errp)
> +{
> +const char compat[] = "virtio,pci-iommu";
> +uint16_t bdf = vms->virtio_iommu_bdf;
> +char *node;
> +
> +vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
> +
> +node = g_strdup_printf("%s/virtio_iommu@%d", vms->pciehb_nodename, bdf);
> +qemu_fdt_add_subnode(vms->fdt, node);
> +qemu_fdt_setprop(vms->fdt, node, "compatible", compat, sizeof(compat));
> +qemu_fdt_setprop_sized_cells(vms->fdt, node, "reg",
> + 1, bdf << 8, 1, 0, 1, 0,
> + 1, 0, 1, 0);
> +
> +qemu_fdt_setprop_cell(vms->fdt, node, "#iommu-cells", 1);
> +qemu_fdt_setprop_cell(vms->fdt, node, "phandle", vms->iommu_phandle);
> +g_free(node);
> +
> +qemu_fdt_setprop_cells(vms->fdt, vms->pciehb_nodename, "iommu-map",
> +   0x0, vms->iommu_phandle, 0x0, bdf,
> +   bdf + 1, vms->iommu_phandle, bdf + 1, 0x - 
> bdf);
> +}

This function name implies that we're creating the IOMMU device
here (which would be a weird thing to do in a hotplug callback
for some other device), but it looks like we're only adding
device tree nodes ?

Given that we write the FDT blob into the guest RAM on bootup,
how does making changes to it here on hotplug (which I assume
to be 'after boot, whenever the user hot-plugs something') work?

thanks
-- PMM



Re: [PATCH v15 0/9] VIRTIO-IOMMU device

2020-02-11 Thread Peter Maydell
On Tue, 11 Feb 2020 at 13:56, Michael S. Tsirkin  wrote:
>
> On Sat, Feb 08, 2020 at 01:00:13PM +0100, Eric Auger wrote:
> > This series implements the QEMU virtio-iommu device.
> >
> > This matches the v0.12 spec (voted) and the corresponding
> > virtio-iommu driver upstreamed in 5.3. All kernel dependencies
> > are resolved for DT integration. The virtio-iommu can be
> > instantiated in ARM virt using:
> >
> > "-device virtio-iommu-pci".
> >
> > Non DT mode is not yet supported as it has non resolved kernel
> > dependencies [1].
> >
> > This feature targets 5.0.
> >
> > Integration with vhost devices and vfio devices is not part
> > of this series. Please follow Bharat's respins [2].
> >
> > Best Regards
> >
> > Eric
>
>
> Looks good.
> Reviewed-by: Michael S. Tsirkin 
>
> I can see this merged through ARM tree, or through my tree with
> Peters's ack for the ARM bits.

Either way would work for me. I left some review comments
on patch 8 which I think is the only arm-specific one.

Can you use the virtio-iommu on x86 ? Would you want to?

If I'm not misreading the MAINTAINERS file the new
files in hw/virtio aren't covered by any existing
entry there, so we should probably have a new one.

thanks
-- PMM



Re: [PATCH 1/7] migration/block-dirty-bitmap: refactor incoming state to be one struct

2020-02-11 Thread Vladimir Sementsov-Ogievskiy

24.01.2020 13:56, Juan Quintela wrote:

Vladimir Sementsov-Ogievskiy  wrote:

Move enabled_bitmaps and finish_lock, which are part of incoming state
to DirtyBitmapLoadState, and make static global variable to store state
instead of static local one.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  migration/block-dirty-bitmap.c | 77 +++---
  1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 7eafface61..281d20f41d 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -125,6 +125,13 @@ typedef struct DirtyBitmapMigState {
  BlockDriverState *prev_bs;
  BdrvDirtyBitmap *prev_bitmap;
  } DirtyBitmapMigState;
+static DirtyBitmapMigState dirty_bitmap_mig_state;


Missing new line.


+
+typedef struct DirtyBitmapLoadBitmapState {
+BlockDriverState *bs;
+BdrvDirtyBitmap *bitmap;
+bool migrated;
+} DirtyBitmapLoadBitmapState;
  
  typedef struct DirtyBitmapLoadState {

  uint32_t flags;
@@ -132,21 +139,15 @@ typedef struct DirtyBitmapLoadState {
  char bitmap_name[256];
  BlockDriverState *bs;
  BdrvDirtyBitmap *bitmap;
-} DirtyBitmapLoadState;
  
-static DirtyBitmapMigState dirty_bitmap_mig_state;

-
-typedef struct DirtyBitmapLoadBitmapState {
-BlockDriverState *bs;
-BdrvDirtyBitmap *bitmap;
-bool migrated;
-} DirtyBitmapLoadBitmapState;
-static GSList *enabled_bitmaps;
-QemuMutex finish_lock;
+GSList *enabled_bitmaps;
+QemuMutex finish_lock;
+} DirtyBitmapLoadState;
+static DirtyBitmapLoadState dbm_load_state;


You move two global variables to an struct (good)
But you create a even bigger global variable (i.e. state that was not
shared before.)


  /* First occurrence of this bitmap. It should be created if doesn't exist */
-static int dirty_bitmap_load_start(QEMUFile *f, DirtyBitmapLoadState *s)
+static int dirty_bitmap_load_start(QEMUFile *f)
  {
+DirtyBitmapLoadState *s = &dbm_load_state;


You create a local alias.


  Error *local_err = NULL;
  uint32_t granularity = qemu_get_be32(f);
  uint8_t flags = qemu_get_byte(f);
@@ -482,7 +484,8 @@ static int dirty_bitmap_load_start(QEMUFile *f, 
DirtyBitmapLoadState *s)
  b->bs = s->bs;
  b->bitmap = s->bitmap;
  b->migrated = false;
-enabled_bitmaps = g_slist_prepend(enabled_bitmaps, b);
+dbm_load_state.enabled_bitmaps =
+g_slist_prepend(dbm_load_state.enabled_bitmaps, b);


And then you access it using the global variable?


-static void dirty_bitmap_load_complete(QEMUFile *f, DirtyBitmapLoadState *s)
+static void dirty_bitmap_load_complete(QEMUFile *f)
  {
+DirtyBitmapLoadState *s = &dbm_load_state;


Exactly the same on this function.

I still don't understand why you are removing the pass as parameters of
this function.



-static DirtyBitmapLoadState s;


Aha, this is why you are moving it as a global.

But, why can't you put this inside dirty_bitmap_mig_state?  Then you:
a- don't have to have "yet" another global variable
b- you can clean it up on save_cleanup handler.


Because dirty_bitmap_mig_state is source state, and new dbm_load_state is
destination state. So, at least [b] will not work...

Do you think it's good to combine both source and destination states into one
struct, and use opaque everywhere?

It will look like

DirtyBitmapMigSourceState *s = ((DirtyBitmapMigState *)opaque)->source_state;

in save functions

and
DirtyBitmapIncomingState *s = ((DirtyBitmapMigState *)opaque)->incoming_state;

in load function



not related to this patch, but to the file in general:

static void dirty_bitmap_save_cleanup(void *opaque)
{
 dirty_bitmap_mig_cleanup();
}

We have opaque here, that we can do:

DirtyBitmapMigBitmapState *dbms = opaque;

And then pass dbms to dirty_bitmap_mig_cleanup().

/* Called with iothread lock taken.  */
static void dirty_bitmap_mig_cleanup(void)
{
 DirtyBitmapMigBitmapState *dbms;

 while ((dbms = QSIMPLEQ_FIRST(&dirty_bitmap_mig_state.dbms_list)) != NULL) 
{
 QSIMPLEQ_REMOVE_HEAD(&dirty_bitmap_mig_state.dbms_list, entry);
 bdrv_dirty_bitmap_set_busy(dbms->bitmap, false);
 bdrv_unref(dbms->bs);
 g_free(dbms);
 }
}


Because here we just use the global variable.

Later, Juan.




--
Best regards,
Vladimir



[Bug 1853826] Re: ELF loader fails to load shared object on ThunderX2 running RHEL7

2020-02-11 Thread Thomas Huth
** Bug watch removed: github.com/DynamoRIO/dynamorio/issues #3385
   https://github.com/DynamoRIO/dynamorio/issues/3385

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

Title:
  ELF loader fails to load shared object on ThunderX2 running RHEL7

Status in QEMU:
  Incomplete

Bug description:
  Simple test:
  hello.c

  include 

  int main(int argc, char* argv[])
  {
{
  printf("Hello World... \n");
}
return 0;
  }

  when compiled with :
  *Compiler 
  
https://developer.arm.com/tools-and-software/server-and-hpc/arm-architecture-tools/arm-allinea-studio/download
  Arm-Compiler-for-HPC_19.3_RHEL_7_aarch64.tar   

  *Running:
  1) with -armpl
   armclang -armpl hello.c
   ./qemu/build/aarch64-linux-user/qemu-aarch64 a.out
  2) without flag
  armclang hello.c
   ./qemu/build/aarch64-linux-user/qemu-aarch64 a.out

  •With Docker image:
 CentOS Linux release 7.7.1908 (AltArch)

  *Two different machines:
 AArch64, Taishan. tsv110, Kunpeng 920, ARMv8.2-A
 AArch64, Taishan 2280, Cortex-A72, ARMv8-A

  *QEMU 4.0
   qemu-aarch64 version 4.1.91 (v4.2.0-rc1)

  
  Results:

  
   Taishan 2280 Cortex-A72 
Running 
  1)with -armpl flag with and without the docker
WORKS-> Hello World...
 -> ldd a.out
  ldd a.out 
  linux-vdso.so.1 =>  (0xbc6a2000) 
  libamath_generic.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/clang/9.0.1/armpl_links/lib/libamath_generic.so
 (0xbc544000) 
  libm.so.6 => /lib64/libm.so.6 (0xbc493000) 
  libastring_generic.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/clang/9.0.1/armpl_links/lib/libastring_generic.so
 (0xbc472000) libarmflang.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/libarmflang.so
 (0xbbfd3000) 
  libomp.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/libomp.so
 (0xbbef5000) 
  librt.so.1 => /lib64/librt.so.1 (0xbbed4000) 
  libpthread.so.0 => /lib64/libpthread.so.0 (0xbbe9f000) 
  libarmpl_lp64_generic.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/clang/9.0.1/armpl_links/lib/libarmpl_lp64_generic.so
 (0xb3306000) 
  libc.so.6 => /lib64/libc.so.6 (0xb318) 
  libstdc++.so.6 => 
/scratch/gcc-9.2.0_Generic-AArch64_RHEL-8_aarch64-linux/lib64/libstdc++.so.6 
(0xb2f3) 
  libgcc_s.so.1 => 
/scratch/gcc-9.2.0_Generic-AArch64_RHEL-8_aarch64-linux/lib64/libgcc_s.so.1 
(0xb2eff000) 
  libdl.so.2 => /lib64/libdl.so.2 (0xb2ede000) 
  /lib/ld-linux-aarch64.so.1 (0xbc674000)
 

  Running 
  2) without -armpl flag with and without the docker
 WORKS -> Hello World...
   -> ldd a.out
  ldd a.out
   linux-vdso.so.1 =>  (0xa6895000) 
  libastring_generic.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/clang/9.0.1/armpl_links/lib/libastring_generic.so
 (0xa6846000) 
  libc.so.6 => /lib64/libc.so.6 (0xa66c) 
  /lib/ld-linux-aarch64.so.1 (0xa6867000)
  

  Taishan - tsv110  Kunpeng 920
 For Running 

  1)with -armpl flag with and without the docker
 DOES NOT WORK -> with and without Docker
   -> It shows : qemu:handle_cpu_signal received signal 
outside vCPU
   context @ pc=0xaaa8844a
   -> ldd a.out 
  ldd a.out 
  linux-vdso.so.1 =>  (0xad4b)
  libamath_generic.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/clang/9.0.1/armpl_links/lib/libamath_generic.so
 (0xad37) 
  libm.so.6 => /lib64/libm.so.6 (0xad2a) 
  libastring_generic.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/clang/9.0.1/armpl_links/lib/libastring_generic.so
 (0xad27) libarmflang.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/libarmflang.so
 (0xacdd) 
  libomp.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/libomp.so
 (0xaccf) 
  librt.so.1 => /lib64/librt.so.1 (0xaccc) 
  libpthread.so.0 => /lib64/libpthread.so.0 (0xacc8) 
  libarmpl_lp64_generic.so => 
/scratch/arm-linux-compiler-19.3_Generic-AArch64_RHEL-8_aarch64-linux/lib/clang/9.0.1/armpl_links/lib/libarmpl_lp64_generic.so
 (0xa40e) 
  libc.so.6 => /lib64/libc.so.6 (0xa3f5) 
  libstdc++.so.6 => 
/scratch/gcc-9.2.0_Generic-AArch64_RHEL-8_aarch64-linux/lib64/libstdc++.so.6 
(0xa3d0) 
  libgcc_s.so.1 => 
/scratch/gcc-9.2.0_Generic-AArch64_RHEL-8_aarch64-linux/lib64/libgcc_s.so.1 
(0xa3cc)
  libdl.so.2 => /lib64/libdl.so.2 (0xf

Re: [PATCH 4/7] migration/block-dirty-bitmap: keep bitmap state for all bitmaps

2020-02-11 Thread Vladimir Sementsov-Ogievskiy

24.01.2020 14:01, Juan Quintela wrote:

Vladimir Sementsov-Ogievskiy  wrote:

Keep bitmap state for disabled bitmaps too. Keep the state until the
end of the process. It's needed for the following commit to implement
bitmap postcopy canceling.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
-
-b = g_new(DirtyBitmapLoadBitmapState, 1);
-b->bs = s->bs;
-b->bitmap = s->bitmap;
-b->migrated = false;
-dbm_load_state.enabled_bitmaps =
-g_slist_prepend(dbm_load_state.enabled_bitmaps, b);
  }
  
+b = g_new(DirtyBitmapLoadBitmapState, 1);

+*b = (DirtyBitmapLoadBitmapState) {
+.bs = s->bs,
+.bitmap = s->bitmap,
+.migrated = false,
+.enabled = flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED,
+};


What is wrong with:
  b->bs = s->bs;
  b->bitmap = s->bitmap;
  b->migrated = false;
  b->enabled = flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED;

???


Nothing wrong. Compound literal is a bit better, as it will initialize to zero 
all skipped fields.
Still nothing missed here. The change is actually unrelated to the patch, I can 
drop it.


--
Best regards,
Vladimir



Re: [PATCH 4/7] migration/block-dirty-bitmap: keep bitmap state for all bitmaps

2020-02-11 Thread Vladimir Sementsov-Ogievskiy

22.01.2020 16:23, Vladimir Sementsov-Ogievskiy wrote:

Keep bitmap state for disabled bitmaps too. Keep the state until the
end of the process. It's needed for the following commit to implement
bitmap postcopy canceling.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  migration/block-dirty-bitmap.c | 59 ++
  1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index eeaab2174e..f96458113c 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -131,6 +131,7 @@ typedef struct DirtyBitmapLoadBitmapState {
  BlockDriverState *bs;
  BdrvDirtyBitmap *bitmap;
  bool migrated;
+bool enabled;
  } DirtyBitmapLoadBitmapState;
  
  typedef struct DirtyBitmapLoadState {

@@ -140,8 +141,11 @@ typedef struct DirtyBitmapLoadState {
  BlockDriverState *bs;
  BdrvDirtyBitmap *bitmap;
  
-GSList *enabled_bitmaps;

-QemuMutex lock; /* protect enabled_bitmaps */
+bool bitmaps_enabled; /* set in dirty_bitmap_mig_before_vm_start */
+bool stream_ended; /* set when all migrated data handled */
+
+GSList *bitmaps;
+QemuMutex lock; /* protect bitmaps */
  } DirtyBitmapLoadState;
  static DirtyBitmapLoadState dbm_load_state;
  
@@ -446,6 +450,7 @@ static int dirty_bitmap_load_start(QEMUFile *f)

  Error *local_err = NULL;
  uint32_t granularity = qemu_get_be32(f);
  uint8_t flags = qemu_get_byte(f);
+DirtyBitmapLoadBitmapState *b;
  
  if (s->bitmap) {

  error_report("Bitmap with the same name ('%s') already exists on "
@@ -472,22 +477,23 @@ static int dirty_bitmap_load_start(QEMUFile *f)
  
  bdrv_disable_dirty_bitmap(s->bitmap);

  if (flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED) {
-DirtyBitmapLoadBitmapState *b;
-
  bdrv_dirty_bitmap_create_successor(s->bitmap, &local_err);
  if (local_err) {
  error_report_err(local_err);
  return -EINVAL;
  }
-
-b = g_new(DirtyBitmapLoadBitmapState, 1);
-b->bs = s->bs;
-b->bitmap = s->bitmap;
-b->migrated = false;
-dbm_load_state.enabled_bitmaps =
-g_slist_prepend(dbm_load_state.enabled_bitmaps, b);
  }
  
+b = g_new(DirtyBitmapLoadBitmapState, 1);

+*b = (DirtyBitmapLoadBitmapState) {
+.bs = s->bs,
+.bitmap = s->bitmap,
+.migrated = false,
+.enabled = flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED,
+};
+
+dbm_load_state.bitmaps = g_slist_prepend(dbm_load_state.bitmaps, b);
+
  return 0;
  }
  
@@ -497,22 +503,25 @@ void dirty_bitmap_mig_before_vm_start(void)
  
  qemu_mutex_lock(&dbm_load_state.lock);
  
-for (item = dbm_load_state.enabled_bitmaps; item;

- item = g_slist_next(item))
-{
+for (item = dbm_load_state.bitmaps; item; item = g_slist_next(item)) {
  DirtyBitmapLoadBitmapState *b = item->data;
  
+if (!b->enabled) {

+continue;
+}
+
  if (b->migrated) {
  bdrv_enable_dirty_bitmap_locked(b->bitmap);
  } else {
  bdrv_dirty_bitmap_enable_successor(b->bitmap);
  }
-
-g_free(b);
  }
  
-g_slist_free(dbm_load_state.enabled_bitmaps);

-dbm_load_state.enabled_bitmaps = NULL;
+dbm_load_state.bitmaps_enabled = true;
+if (dbm_load_state.stream_ended) {
+g_slist_free_full(dbm_load_state.bitmaps, g_free);
+dbm_load_state.bitmaps = NULL;
+}
  
  qemu_mutex_unlock(&dbm_load_state.lock);

  }
@@ -530,9 +539,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f)
  bdrv_reclaim_dirty_bitmap(s->bitmap, &error_abort);
  }
  
-for (item = dbm_load_state.enabled_bitmaps; item;

- item = g_slist_next(item))
-{
+for (item = dbm_load_state.bitmaps; item; item = g_slist_next(item)) {
  DirtyBitmapLoadBitmapState *b = item->data;
  
  if (b->bitmap == s->bitmap) {

@@ -671,6 +678,16 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, 
int version_id)
  }
  } while (!(dbm_load_state.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
  
+qemu_mutex_lock(&dbm_load_state.lock);

+
+dbm_load_state.stream_ended = true;


This is wrong. EOS doesn't mean that bitmap migration finished: only one set of 
sequential
bitmap chunks finished. EOS may be sent several times during migration and it 
is needed to
switch to other things migration.


+if (dbm_load_state.bitmaps_enabled) {
+g_slist_free_full(dbm_load_state.bitmaps, g_free);
+dbm_load_state.bitmaps = NULL;
+}
+
+qemu_mutex_unlock(&dbm_load_state.lock);
+
  trace_dirty_bitmap_load_success();
  return 0;
  }




--
Best regards,
Vladimir



Re: [RFC PATCH 33/66] Hexagon TCG generation helpers - step 1

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/11/20 1:40 AM, Taylor Simpson wrote:

Helpers for reading and writing registers
Helpers for getting and setting parts of values (e.g., set bit)

Signed-off-by: Taylor Simpson 
---
  target/hexagon/genptr_helpers.h | 323 
  1 file changed, 323 insertions(+)
  create mode 100644 target/hexagon/genptr_helpers.h

diff --git a/target/hexagon/genptr_helpers.h b/target/hexagon/genptr_helpers.h
new file mode 100644
index 000..2b91fdb
--- /dev/null
+++ b/target/hexagon/genptr_helpers.h
@@ -0,0 +1,323 @@
+/*
+ *  Copyright (c) 2019 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  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 .
+ */
+
+#ifndef GENPTR_HELPERS_H
+#define GENPTR_HELPERS_H
+


#include "tcg/tcg.h"?


+static inline TCGv gen_read_reg(TCGv result, int num)
+{
+tcg_gen_mov_tl(result, hex_gpr[num]);
+return result;
+}
+
+static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
+{
+tcg_gen_mov_tl(pred, hex_pred[num]);
+return pred;
+}
+
+static inline TCGv gen_newreg_st(TCGv result, TCGv_env cpu_env, TCGv rnum)
+{
+gen_helper_new_value(result, cpu_env, rnum);
+return result;
+}
+
+static inline bool is_preloaded(DisasContext *ctx, int num)
+{
+int i;
+for (i = 0; i < ctx->ctx_reg_log_idx; i++) {
+if (ctx->ctx_reg_log[i] == num) {
+return true;
+}
+}
+return false;
+}
+
+static inline void gen_log_reg_write(int rnum, TCGv val, int slot,
+ int is_predicated)
+{
+if (is_predicated) {
+TCGv one = tcg_const_tl(1);
+TCGv zero = tcg_const_tl(0);
+TCGv slot_mask = tcg_temp_new();
+
+tcg_gen_andi_tl(slot_mask, hex_slot_cancelled, 1 << slot);
+tcg_gen_movcond_tl(TCG_COND_EQ, hex_new_value[rnum], slot_mask, zero,
+   val, hex_new_value[rnum]);
+
+tcg_temp_free(one);
+tcg_temp_free(zero);
+tcg_temp_free(slot_mask);
+} else {
+tcg_gen_mov_tl(hex_new_value[rnum], val);
+}
+}

[...]




docs: Update vhost-user spec regarding backend program conventions

2020-02-11 Thread Boeuf, Sebastien
From c073d528b8cd7082832fd1825dc33dd65b305aa2 Mon Sep 17 00:00:00 2001
From: Sebastien Boeuf 
Date: Tue, 11 Feb 2020 16:01:22 +0100
Subject: [PATCH] docs: Update vhost-user spec regarding backend program
 conventions

The vhost-user specification is not clearly stating the expected
behavior from a backend program whenever the client disconnects.

This patch addresses the issue by defining the default behavior and
proposing an alternative through a command line option.

By default, a backend program will have to keep listening even if the
client disconnects, unless told otherwise through the newly introduced
option --exit-on-disconnect.

Signed-off-by: Sebastien Boeuf 
Signed-off-by: Stefan Hajnoczi 
---
 docs/interop/vhost-user.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 5f8b3a456b..da9a1ebc4c 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -1323,6 +1323,10 @@ The backend program must end (as quickly and cleanly as 
possible) when
 the SIGTERM signal is received. Eventually, it may receive SIGKILL by
 the management layer after a few seconds.
 
+By default, the backend program continues running after the client
+disconnects. It accepts only 1 connection at a time on each UNIX domain
+socket.
+
 The following command line options have an expected behaviour. They
 are mandatory, unless explicitly said differently:
 
@@ -1337,6 +1341,12 @@ are mandatory, unless explicitly said differently:
   vhost-user socket as file descriptor FDNUM. It is incompatible with
   --socket-path.
 
+--exit-on-disconnect
+
+  When this option is provided, the backend program must terminate when
+  the client disconnects. This can be used to keep the backend program's
+  lifetime synchronized with its client process.
+
 --print-capabilities
 
   Output to stdout the backend capabilities in JSON format, and then
-- 
2.20.1

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


Re: [PATCH v2 01/33] block: Add BlockDriver.is_format

2020-02-11 Thread Alberto Garcia
On Tue 04 Feb 2020 06:08:16 PM CET, Max Reitz wrote:
> We want to unify child_format and child_file at some point.  One of the
> important things that set format drivers apart from other drivers is
> that they do not expect other format nodes under them (except in the
> backing chain).  That means we need something on which to distinguish
> format drivers from others, and hence this flag.
>
> Signed-off-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto



Re: [RFC PATCH 10/66] Hexagon register fields

2020-02-11 Thread Philippe Mathieu-Daudé

On 2/11/20 1:39 AM, Taylor Simpson wrote:

Declare bitfields within registers such as user status register (USR)

Signed-off-by: Taylor Simpson 
---
  target/hexagon/reg_fields.c |  28 +++
  target/hexagon/reg_fields.h |  40 +++
  target/hexagon/reg_fields_def.h | 109 
  3 files changed, 177 insertions(+)
  create mode 100644 target/hexagon/reg_fields.c
  create mode 100644 target/hexagon/reg_fields.h
  create mode 100644 target/hexagon/reg_fields_def.h

diff --git a/target/hexagon/reg_fields.c b/target/hexagon/reg_fields.c
new file mode 100644
index 000..983655e
--- /dev/null
+++ b/target/hexagon/reg_fields.c
@@ -0,0 +1,28 @@
+/*
+ *  Copyright (c) 2019 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  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 


Replace by:

  #include "qemu/osdep.h"


+#include "reg_fields.h"
+
+reg_field_t reg_field_info[] = {
+#define DEF_REG_FIELD(TAG, NAME, START, WIDTH, DESCRIPTION)\
+  {NAME, START, WIDTH, DESCRIPTION},
+#include "reg_fields_def.h"
+  {NULL, 0, 0}
+#undef DEF_REG_FIELD
+};
+
diff --git a/target/hexagon/reg_fields.h b/target/hexagon/reg_fields.h
new file mode 100644
index 000..79857c5
--- /dev/null
+++ b/target/hexagon/reg_fields.h
@@ -0,0 +1,40 @@
+/*
+ *  Copyright (c) 2019 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  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 .
+ */
+
+#ifndef REGS_H
+#define REGS_H


Maybe HEXAGON_REG_FIELDS?


+
+#define NUM_GEN_REGS 32
+
+typedef struct {
+const char *name;
+int offset;
+int width;
+const char *description;
+} reg_field_t;
+
+extern reg_field_t reg_field_info[];
+
+enum reg_fields_enum {
+#define DEF_REG_FIELD(TAG, NAME, START, WIDTH, DESCRIPTION) \
+TAG,
+#include "reg_fields_def.h"
+NUM_REG_FIELDS
+#undef DEF_REG_FIELD
+};
+
+#endif
diff --git a/target/hexagon/reg_fields_def.h b/target/hexagon/reg_fields_def.h
new file mode 100644
index 000..095a776
--- /dev/null
+++ b/target/hexagon/reg_fields_def.h
@@ -0,0 +1,109 @@
+/*
+ *  Copyright (c) 2019 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  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 .
+ */
+
+/*
+ * For registers that have individual fields, explain them here
+ *   DEF_REG_FIELD(tag,
+ * name,
+ * bit start offset,
+ * width,
+ * description
+ */
+
+/* USR fields */
+DEF_REG_FIELD(USR_OVF,
+"ovf", 0, 1,
+"Sticky Saturation Overflow - "
+"Set when saturation occurs while executing instruction that specifies "
+"optional saturation, remains set until explicitly cleared by a USR=Rs "
+"instruction.")
+DEF_REG_FIELD(USR_FPINVF,
+"fpinvf", 1, 1,
+"Floating-point IEEE Invalid Sticky Flag.")
+DEF_REG_FIELD(USR_FPDBZF,
+"fpdbzf", 2, 1,
+"Floating-point IEEE Divide-By-Zero Sticky Flag.")
+DEF_REG_FIELD(USR_FPOVFF,
+"fpovff", 3, 1,
+"Floating-point IEEE Overflow Sticky Flag.")
+DEF_REG_FIELD(USR_FPUNFF,
+"fpunff", 4, 1,
+"Floating-point IEEE Underflow Sticky Flag.")
+DEF_RE

Re: [PATCH v2 02/33] block: Rename BdrvChildRole to BdrvChildClass

2020-02-11 Thread Alberto Garcia
On Tue 04 Feb 2020 06:08:17 PM CET, Max Reitz wrote:
> This structure nearly only contains parent callbacks for child state
> changes.  It cannot really reflect a child's role, because different
> roles may overlap (as we will see when real roles are introduced), and
> because parents can have custom callbacks even when the child fulfills a
> standard role.
>
> Signed-off-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto



Re: [RFC PATCH 00/66] Hexagon patch series

2020-02-11 Thread Philippe Mathieu-Daudé

Hi Taylor,

On 2/11/20 1:39 AM, Taylor Simpson wrote:

This series adds support for the Hexagon processor with Linux user support


[...]>   MAINTAINERS  |8 +

  configure|9 +
  default-configs/hexagon-linux-user.mak   |1 +
  disas/Makefile.objs  |1 +
  disas/hexagon.c  |   56 +
  include/disas/dis-asm.h  |1 +
  include/elf.h|2 +
  linux-user/elfload.c |   16 +
  linux-user/hexagon/cpu_loop.c|  173 ++
  linux-user/hexagon/signal.c  |  276 ++
  linux-user/hexagon/sockbits.h|   18 +
  linux-user/hexagon/syscall_nr.h  |  346 +++
  linux-user/hexagon/target_cpu.h  |   44 +
  linux-user/hexagon/target_elf.h  |   38 +
  linux-user/hexagon/target_fcntl.h|   18 +
  linux-user/hexagon/target_signal.h   |   34 +
  linux-user/hexagon/target_structs.h  |   46 +
  linux-user/hexagon/target_syscall.h  |   32 +
  linux-user/hexagon/termbits.h|   18 +
  linux-user/syscall.c |2 +
  linux-user/syscall_defs.h|   33 +
  scripts/qemu-binfmt-conf.sh  |6 +-
  target/hexagon/Makefile.objs |  109 +
  target/hexagon/arch.c|  664 +
  target/hexagon/arch.h|   62 +
  target/hexagon/attribs.h |   32 +
  target/hexagon/attribs_def.h |  404 +++
  target/hexagon/conv_emu.c|  370 +++
  target/hexagon/conv_emu.h|   50 +
  target/hexagon/cpu-param.h   |   26 +
  target/hexagon/cpu.c |  356 +++
  target/hexagon/cpu.h |  207 ++
  target/hexagon/cpu_bits.h|   37 +
  target/hexagon/decode.c  |  792 +
  target/hexagon/decode.h  |   39 +
  target/hexagon/dectree.py|  354 +++
  target/hexagon/do_qemu.py| 1198 
  target/hexagon/fma_emu.c |  918 ++
  target/hexagon/fma_emu.h |   30 +
  target/hexagon/gdbstub.c |  111 +
  target/hexagon/gen_dectree_import.c  |  205 ++
  target/hexagon/gen_semantics.c   |  101 +
  target/hexagon/genptr.c  |   62 +
  target/hexagon/genptr.h  |   25 +
  target/hexagon/genptr_helpers.h  | 1022 +++
  target/hexagon/helper.h  |   38 +
  target/hexagon/helper_overrides.h| 1850 
  target/hexagon/hex_arch_types.h  |   42 +
  target/hexagon/hex_regs.h|   97 +
  target/hexagon/iclass.c  |  109 +
  target/hexagon/iclass.h  |   46 +
  target/hexagon/imported/allext.idef  |   25 +
  target/hexagon/imported/allext_macros.def|   25 +
  target/hexagon/imported/allextenc.def|   20 +
  target/hexagon/imported/allidefs.def |   92 +
  target/hexagon/imported/alu.idef | 1335 +
  target/hexagon/imported/branch.idef  |  344 +++
  target/hexagon/imported/compare.idef |  639 +
  target/hexagon/imported/encode.def   |  126 +
  target/hexagon/imported/encode_pp.def| 2283 +++
  target/hexagon/imported/encode_subinsn.def   |  150 +
  target/hexagon/imported/float.idef   |  498 
  target/hexagon/imported/iclass.def   |   52 +
  target/hexagon/imported/ldst.idef|  421 +++
  target/hexagon/imported/macros.def   | 3970 ++
  target/hexagon/imported/mmvec/encode_ext.def |  830 ++
  target/hexagon/imported/mmvec/ext.idef   | 2809 ++
  target/hexagon/imported/mmvec/macros.def | 1110 +++
  target/hexagon/imported/mpy.idef | 1269 
  target/hexagon/imported/shift.idef   | 1211 
  target/hexagon/imported/subinsns.idef|  152 +
  target/hexagon/imported/system.idef  |  302 ++
  target/hexagon/insn.h|  149 +
  target/hexagon/internal.h|   54 +
  target/hexagon/macros.h  | 1499 ++
  target/hexagon/mmvec/decode_ext_mmvec.c  |  673 +
  target/hexagon/mmvec/decode_ext_mmvec.h  |   24 +
  target/hexagon/mmvec/macros.h|  668 +
  target/hexagon/mmvec/mmvec.h |   87 +
  target/hexagon/mmvec/system_ext_mmvec.c  |  265 ++
  target/hexagon/mmvec/system_ext_mmvec.h  |   38 +
  target/hexagon/op_helper.c   |  507 
  target/hexagon/opcodes.c |  223 ++
  target/hexagon/opcodes.h 

Re: [kvm-unit-tests PATCH v2 4/9] arm: pmu: Check Required Event Support

2020-02-11 Thread Peter Maydell
On Thu, 30 Jan 2020 at 11:25, Eric Auger  wrote:
>
> If event counters are implemented check the common events
> required by the PMUv3 are implemented.
>
> Some are unconditionally required (SW_INCR, CPU_CYCLES,
> either INST_RETIRED or INST_SPEC). Some others only are
> required if the implementation implements some other features.
>
> Check those wich are unconditionally required.
>
> This test currently fails on TCG as neither INST_RETIRED
> or INST_SPEC are supported.
>
> Signed-off-by: Eric Auger 
>
> ---
>

> +static bool is_event_supported(uint32_t n, bool warn)
> +{
> +   uint64_t pmceid0 = read_sysreg(pmceid0_el0);
> +   uint64_t pmceid1 = read_sysreg_s(PMCEID1_EL0);
> +   bool supported;
> +   uint64_t reg;
> +
> +   /*
> +* The low 32-bits of PMCEID0/1 respectly describe

"respectively"

> +* event support for events 0-31/32-63. Their High
> +* 32-bits describe support for extended events
> +* starting at 0x4000, using the same split.
> +*/
> +   if (n >= 0x0  && n <= 0x3F)
> +   reg = (pmceid0 & 0x) | ((pmceid1 & 0x) << 32);
> +   else if  (n >= 0x4000 && n <= 0x403F)
> +   reg = (pmceid0 >> 32) | ((pmceid1 >> 32) << 32);
> +   else
> +   abort();
> +
> +   supported =  reg & (1UL << (n & 0x3F));
> +
> +   if (!supported && warn)
> +   report_info("event %d is not supported", n);
> +   return supported;
> +}
> +
> +static void test_event_introspection(void)
> +{
> +   bool required_events;
> +
> +   if (!pmu.nb_implemented_counters) {
> +   report_skip("No event counter, skip ...");
> +   return;
> +   }
> +
> +   /* PMUv3 requires an implementation includes some common events */
> +   required_events = is_event_supported(0x0, true) /* SW_INCR */ &&
> + is_event_supported(0x11, true) /* CPU_CYCLES */ &&
> + (is_event_supported(0x8, true) /* INST_RETIRED */ ||
> +  is_event_supported(0x1B, true) /* INST_PREC */);
> +
> +   if (pmu.version == 0x4) {

This condition will only test for v8.1-required events if the PMU
is exactly 8.1, so you lose coverage if the implementation happens
to support ARMv8.4-PMU. Hopefully you have already bailed out
for "ID_AA64DFR0_EL1.PMUVer == 0xf" which means "non-standard IMPDEF
PMU", in which case you can just check >= 0x4.

> +   /* ARMv8.1 PMU: STALL_FRONTEND and STALL_BACKEND are required 
> */
> +   required_events = required_events &&
> + is_event_supported(0x23, true) &&
> + is_event_supported(0x24, true);
> +   }
> +
> +   report(required_events, "Check required events are implemented");
> +}
> +
>  #endif

thanks
-- PMM



Re: [PATCH v2 03/33] block: Add BdrvChildRole

2020-02-11 Thread Eric Blake

On 2/6/20 4:49 AM, Max Reitz wrote:


+
+    /* Filtered child */
+    BDRV_CHILD_FILTERED = (1 << 2),


I'm not sure this comment does justice for what the flag represents, but
am not sure of what longer comment to put in its place.


You’re right.  I thought I could just rely on our .is_filter
documentation (at least after
https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg01721.html),
but that doesn’t really apply here.

For example, this series makes raw (without further parameters) have a
CHILD_FILTERED child, without raw being a filter itself.

So there should indeed be some definition here.

Maybe:

A child to which the parent forwards all reads and writes.  Therefore,
this child presents exactly the same visible data as the parent.


On second thought, the “therefore” is wrong, because the first sentence
applies to quorum, but the logical conclusion does not.

So maybe rather:

A child to which the parent forwards all reads and writes.  It must
present exactly the same visible data as the parent.
Any node may have at most one filtered child at a time.


Yes, this works for me.

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




Re: [PATCH v2 04/33] block: Add BdrvChildRole to BdrvChild

2020-02-11 Thread Eric Blake

On 2/6/20 4:53 AM, Max Reitz wrote:


+++ b/block.c
@@ -2381,6 +2381,7 @@ static void bdrv_replace_child(BdrvChild *child,
BlockDriverState *new_bs)
   BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
     const char *child_name,
     const BdrvChildClass *child_class,
+  BdrvChildRole child_role,


Hmm - C is loose enough to allow the declaration of a parameter as an
enum type even when its intended usage is to receive a bitwise-or
derived from that enum but not declared in the enum.  For example, if I
understand intent correctly, a caller might pass in 0x3
(BDRV_CHILD_DATA|BDRV_CHILD_METADATA) which does NOT appear in
BdrvChildRole.  It feels like it might be cleaner to document the
interface as taking an unsigned int (although then you've lost the
documentation that the int is derived from BdrvChildRole), than to abuse
the typesystem to pass values that are not BdrvChildRole through the
parameter.


I don’t necessarily disagree, but we have pre-existing examples of such
abuse, like BdrvRequestFlags.

The advantage of using BdrvChildRole as a type here is to show that we
expect values from that enum.  I personally prefer that.


Yeah, the self-documenting aspect is nice.



I mean, we could do something else entirely and name the enum
“BdrvChildRoleBits” and add a “typedef unsigned int BdrvChildRole;”.  I
don’t think we’ve ever done that before but maybe it’d be the cleanest way?


You're right that we haven't done it, but it is also the slickest 
solution that preserves documentation intent.  In C, such a typedef 
serves only as documentation (and the compiler doesn't enforce it, 
compared to what a strongly-typed language would do), but I do like the 
idea.


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




Re: [PATCH v2 15/33] block: Pull out bdrv_default_perms_for_backing()

2020-02-11 Thread Eric Blake

On 2/6/20 5:19 AM, Max Reitz wrote:

On 05.02.20 21:56, Eric Blake wrote:

On 2/4/20 11:08 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 


Rather light on the commit message. But looks like straightforward
refactoring (with the previous patch making it easier to follow).


Would this work:

Right now, bdrv_format_default_perms() is used by format parents
(generally). We want to switch to a model where most parents use a
single BdrvChildClass, which then decides the permissions based on the
child role. To do so, we have to split bdrv_format_default_perms() into
separate functions for each such role.

?


Yes.

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




Re: [PATCH v2 22/33] block: Make backing files child_of_bds children

2020-02-11 Thread Eric Blake

On 2/6/20 5:33 AM, Max Reitz wrote:

On 05.02.20 23:45, Eric Blake wrote:

On 2/4/20 11:08 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 


Another sparse commit message (a recurring theme of this series). The
subject line says 'what', and the patch appears to be faithful to that,
but if a future bisection lands here, even a one-sentence 'why' would be
handy; maybe:

This is part of a larger series of unifying block device relationships
via child_of_bds.


Sure, works for me.  Or maybe:

Make all parents of backing files pass the appropriate BdrvChildRole.
By doing so, we can switch their BdrvChildClass over to the generic
child_of_bds, which will do the right thing when given a correct
BdrvChildRole.


Sounds good.



(Because actually the point of this series is not child_of_bds, but the
BdrvChildRole, which allows the “Deal with filters” series to implement
the child access functions in a more obvious way.  I hope.)


And hopefully I finish my review of the rest of the series today, to see 
if we met that goal.


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




Re: [PATCH v2 0/4] linux-user: fix use of SIGRTMIN

2020-02-11 Thread Laurent Vivier
Peter,

do you have any comment on this way to fix the SIGRTMIN problem we have
now for years?

Thanks,
Laurent

Le 04/02/2020 à 18:10, Laurent Vivier a écrit :
> This series fixes the problem of the first real-time signals already
> in use by the glibc that are not available for the target glibc.
> 
> Instead of reverting the first and last real-time signals we rely on
> the value provided by the glibc (SIGRTMIN) to know the first available
> signal and we map all the signals from this value to SIGRTMAX on top
> of TARGET_SIGRTMIN. So the consequence is we have less available signals
> in the target (generally 2) but all seems fine as at least 30 signals are
> still available.
> 
> This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64
> fedora 31. We can avoid the failure in this case allowing the unsupported
> signals when we don't provide the "act" parameters to sigaction, only the
> "oldact" one. I have also run the LTP suite with several target and debian
> based distros.
> 
> v2: tested with golang 1.12.10 linux/arm64, eoan)
> Ignore unsupported signals rather than returning an error
> replace i, j by target_sig, host_sig
> 
> Laurent Vivier (4):
>   linux-user: add missing TARGET_SIGRTMIN for hppa
>   linux-user: cleanup signal.c
>   linux-user: fix TARGET_NSIG and _NSIG uses
>   linux-user: fix use of SIGRTMIN
> 
>  linux-user/hppa/target_signal.h |   1 +
>  linux-user/signal.c | 117 +++-
>  linux-user/trace-events |   3 +
>  3 files changed, 89 insertions(+), 32 deletions(-)
> 




Re: [PATCH v2 03/33] block: Add BdrvChildRole

2020-02-11 Thread Alberto Garcia
On Tue 04 Feb 2020 06:08:18 PM CET, Max Reitz wrote:
> +/* Child to COW from (backing child) */
> +BDRV_CHILD_COW  = (1 << 3),

Without the comment in brackets I'm not sure that I would have
understood that this is meant for backing files.

This is the "child that contains the data that is not allocated in the
parent", or something like that, right?

Berto



Re: [PATCH v2 27/33] tests: Use child_of_bds instead of child_file

2020-02-11 Thread Eric Blake

On 2/4/20 11:08 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---


Another sparse commit message.


  tests/test-bdrv-drain.c | 29 +
  tests/test-bdrv-graph-mod.c |  6 --
  2 files changed, 21 insertions(+), 14 deletions(-)



Just touches the tests - so if the testsuite passes, the patch must be 
correct ;)


Reviewed-by: Eric Blake 

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




  1   2   3   >