[PATCH v3 1/3] qcow2: introduce Qcow2Metadata structure

2020-01-14 Thread Andrey Shinkevich
The preliminary patch to provide an extendable structure for dumping
QCOW2 metadata allocations in image. The command line optional key is
introduced in the patch that follows.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 qapi/block-core.json | 209 ++-
 1 file changed, 208 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ff5e5e..fab7435 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -176,6 +176,209 @@
'*backing-image': 'ImageInfo',
'*format-specific': 'ImageInfoSpecific' } }
 
+
+##
+# @Qcow2Metadata:
+#
+# Encapsulates QCOW2 metadata information
+#
+# @qcow2-header: QCOW2 header info
+#
+# @active-l1: L1 active table info
+#
+# @refcount-table: refcount table info
+#
+# @crypt-header: encryption header info
+#
+# @bitmaps: bitmap tables info
+#
+# @snapshot-table: snapshot tables info
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2Metadata',
+  'data': { 'qcow2-header': 'Qcow2Header',
+'refcount-table': 'Qcow2RefcountTable',
+'active-l1': 'Qcow2L1Table',
+'*crypt-header': 'Qcow2EncryptionHeader',
+'*bitmaps': 'Qcow2Bitmaps',
+'*snapshot-table': 'Qcow2SnapshotsTable' } }
+
+##
+# @Qcow2Header:
+#
+# QCOW2 header information
+#
+# @version: version number
+#
+# @location: header offset and size in image
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2Header',
+  'data': {'version': 'int',
+   'location': 'Qcow2Allocation' } }
+
+##
+# @Qcow2EncryptionHeader:
+#
+# QCOW2 encryption header information
+#
+# @location: header offset and size in image
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2EncryptionHeader',
+  'data': {'location': 'Qcow2Allocation' } }
+
+##
+# @Qcow2RefcountTable:
+#
+# QCOW2 refcount table information
+#
+# @location: refcount table offset and size in image
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2RefcountTable',
+  'data': {'location': 'Qcow2Allocation' } }
+
+##
+# @Qcow2L1Table:
+#
+# L1 table information
+#
+# @l2-list: list of L2 table locations
+#
+# @location: L1 table offset and size in image
+#
+# @name: entity name the table relates to
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2L1Table',
+  'data': {'l2-list': ['Qcow2Allocation'],
+   'location': 'Qcow2Allocation',
+   'name': 'str'} }
+
+##
+# @Qcow2Allocation:
+#
+# QCOW2 data location in image
+#
+# @offset: data offset
+#
+# @size: data size
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2Allocation',
+  'data': {'offset': 'uint64', 'size': 'uint64' } }
+
+##
+# @Qcow2Bitmaps:
+#
+# QCOW2 bitmaps information
+#
+# @nb-bitmaps: the number of bitmaps contained in the image
+#
+# @bitmap-dir: bitmap directory information
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2Bitmaps',
+  'data': {'nb-bitmaps': 'int',
+   'bitmap-dir': 'Qcow2BitmapDir' } }
+
+##
+# @Qcow2BitmapDir:
+#
+# QCOW2 bitmap directory information
+#
+# @dir-entries: list of bitmap directory entries
+#
+# @location: bitmap directory offset and size in image
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2BitmapDir',
+  'data': {'dir-entries': ['Qcow2BitmapDirectoryEntry'],
+   'location': 'Qcow2Allocation' } }
+
+##
+# @Qcow2BitmapDirectoryEntry:
+#
+# QCOW2 bitmap directory entry information
+#
+# @bitmap-table: bitmap table offset and size in image
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2BitmapDirectoryEntry',
+  'data': {'bitmap-table': 'Qcow2BitmapTableInfo',
+   'bitmap-name': 'str' } }
+
+##
+# @Qcow2BitmapTableInfo:
+#
+# QCOW2 bitmap table information
+#
+# @table-entries: list of bitmap table entries
+#
+# @location: bitmap table offset and size in image
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2BitmapTableInfo',
+  'data': {'table-entries': ['Qcow2BitmapTableInfoEntry'],
+   'location': 'Qcow2Allocation' } }
+
+##
+# @Qcow2BitmapTableInfoEntry:
+#
+# QCOW2 bitmap table entry information
+#
+# @type: bitmap table entry type
+#
+# @cluster: bitmap table entry offset and size in image
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2BitmapTableInfoEntry',
+  'data': {'type': 'Qcow2BitmapTableInfoEntryType',
+   '*cluster': 'Qcow2Allocation' } }
+
+##
+# @Qcow2BitmapTableInfoEntryType:
+#
+# An enumeration of cluster types in bitmap table
+#
+# @all-zeroes: cluster should be read as all zeroes
+#
+# @all-ones: cluster should be read as all ones
+#
+# @serialized: cluster data are written on disk
+#
+# Since: 5.0
+##
+{ 'enum': 'Qcow2BitmapTableInfoEntryType',
+  'data': ['all-zeroes', 'all-ones', 'serialized'] }
+
+##
+# @Qcow2SnapshotsTable:
+#
+# Snapshots table location in image file.
+#
+# @location: offset and size of snapshot table
+#
+# @l1-list: list of snapshots L1 tables
+#
+# Since: 5.0
+##
+{ 'struct': 'Qcow2SnapshotsTable',
+  'data': {'location': 'Qcow2Allocation',
+   'l1-list': ['Qcow2L1Table'] } }
+
 ##
 # @ImageCheck:
 #
@@ -215,6 +418,9 @@
 #   field is present if the driv

[PATCH v3 0/3] Dump QCOW2 metadata

2020-01-14 Thread Andrey Shinkevich
The information about QCOW2 metadata allocations in an image ELF-file is
helpful for finding issues with the image data integrity.

v3: Descriptions of the new key option were added. The names of identifiers
were amended. The QEMU-IMG key options were put in the sorted order in
the code. All suggested by Eric. Discussed in the email thread with ID
<1577447039-400109-1-git-send-email-andrey.shinkev...@virtuozzo.com>

Snapshots dump example:

$ sudo ./qemu-img check /.../.../harddisk.hdd -M --output=json
{
"image-end-offset": 24820842496,
"total-clusters": 153600,
"check-errors": 0,
"viscera": {
"refcount-table": {
"location": {
"offset": 3845128192,
"size": 1048576
}
},
"active-l1": {
"name": "L1 active table",
"location": {
"offset": 4194304,
"size": 16
},
"l2-list": [
{
"offset": 619708416,
"size": 1048576
},
{
"offset": 1156579328,
"size": 1048576
}
]
},
"qcow2-header": {
"location": {
"offset": 0,
"size": 1048576
},
"version": 3
},
"snapshot-table": {
"location": {
"offset": 648019968,
"size": 191
},
"l1-list": [
{
"name": "{3036f6c5-3a1f-44cb-af1f-653cc87fba04}",
"location": {
"offset": 14680064,
"size": 16
},
"l2-list": [
{
"offset": 3957325824,
"size": 1048576
},
{
"offset": 7025459200,
"size": 1048576
}
]
},
{
"name": "{0aa1a7d6-16ee-4b44-a515-b5ecc571c959}",
"location": {
"offset": 638582784,
"size": 16
},
"l2-list": [
{
"offset": 3957325824,
"size": 1048576
},
{
"offset": 7025459200,
"size": 1048576
}
]
}
]
}
},
"allocated-clusters": 22485,
"filename": "/.../.../harddisk.hdd",
"format": "qcow2",
"fragmented-clusters": 3549
}

Bitmaps dump example:

$ ./qemu-img check /home/disk -M --output=json
{
"image-end-offset": 1441792,
"total-clusters": 16,
"check-errors": 0,
"viscera": {
"refcount-table": {
"location": {
"offset": 65536,
"size": 65536
}
},
"active-l1": {
"name": "L1 active table",
"location": {
"offset": 196608,
"size": 8
},
"l2-list": [
{
"offset": 262144,
"size": 65536
}
]
},
"bitmaps": {
"bitmap-dir": {
"location": {
"offset": 1048576,
"size": 64
},
"dir-entries": [
{
"bitmap-table": {
"location": {
"offset": 589824,
"size": 8
},
"table-entries": [
{
"type": "all-zeros"
}
]
},
"bitmap-name": "bitmap-1"
},
{
"bitmap-table": {
"location": {
"offset": 983040,
"size": 8
},
"table-entries": [
{
"cluster": {
"offset": 655360,
"size": 65536
},
"type": "serialized"
}
]
},
"bitmap-name": "bitmap-2"
}
   

[PATCH v3 2/3] qemu-img: sort key options alphabetically

2020-01-14 Thread Andrey Shinkevich
Put qemu-img key options in the alphabetical order in the code
for a faster finding.

Suggested-by: Eric Blake 
Signed-off-by: Andrey Shinkevich 
---
 qemu-img.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 6233b8c..c86f760 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -703,7 +703,7 @@ static int img_check(int argc, char **argv)
 {"force-share", no_argument, 0, 'U'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hf:r:T:qU",
+c = getopt_long(argc, argv, ":f:hqr:T:U",
 long_options, &option_index);
 if (c == -1) {
 break;
@@ -715,11 +715,14 @@ static int img_check(int argc, char **argv)
 case '?':
 unrecognized_option(argv[optind - 1]);
 break;
+case 'f':
+fmt = optarg;
+break;
 case 'h':
 help();
 break;
-case 'f':
-fmt = optarg;
+case 'q':
+quiet = true;
 break;
 case 'r':
 flags |= BDRV_O_RDWR;
@@ -733,18 +736,15 @@ static int img_check(int argc, char **argv)
"(expecting 'leaks' or 'all'): %s", optarg);
 }
 break;
-case OPTION_OUTPUT:
-output = optarg;
-break;
 case 'T':
 cache = optarg;
 break;
-case 'q':
-quiet = true;
-break;
 case 'U':
 force_share = true;
 break;
+case OPTION_IMAGE_OPTS:
+image_opts = true;
+break;
 case OPTION_OBJECT: {
 QemuOpts *opts;
 opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -753,8 +753,8 @@ static int img_check(int argc, char **argv)
 return 1;
 }
 }   break;
-case OPTION_IMAGE_OPTS:
-image_opts = true;
+case OPTION_OUTPUT:
+output = optarg;
 break;
 }
 }
-- 
1.8.3.1




[PATCH v3 3/3] qcow2: dump QCOW2 metadata

2020-01-14 Thread Andrey Shinkevich
Let QEMU-IMG CHECK command show QCOW2 structure to inform a user about
metadata allocations on disk. Introduce '-M'('--dump-meta') key option.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 block/qcow2-bitmap.c   | 54 +---
 block/qcow2-refcount.c | 84 +-
 block/qcow2.c  | 30 ++
 block/qcow2.h  |  6 ++--
 include/block/block.h  |  3 +-
 qemu-img.c | 30 +-
 qemu-img.texi  |  7 -
 7 files changed, 190 insertions(+), 24 deletions(-)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index d41f5d0..15e035a 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -659,20 +659,29 @@ fail:
 
 int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
   void **refcount_table,
-  int64_t *refcount_table_size)
+  int64_t *refcount_table_size,
+  Qcow2Bitmaps *bitmaps)
 {
 int ret;
 BDRVQcow2State *s = bs->opaque;
 Qcow2BitmapList *bm_list;
 Qcow2Bitmap *bm;
+Qcow2BitmapDirectoryEntryList **pp_dir =
+bitmaps ? &bitmaps->bitmap_dir->dir_entries : NULL;
 
 if (s->nb_bitmaps == 0) {
 return 0;
 }
 
+if (bitmaps) {
+bitmaps->nb_bitmaps = s->nb_bitmaps;
+}
+
 ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, 
refcount_table_size,
s->bitmap_directory_offset,
-   s->bitmap_directory_size);
+   s->bitmap_directory_size,
+   bitmaps ? bitmaps->bitmap_dir->location
+   : NULL);
 if (ret < 0) {
 return ret;
 }
@@ -686,12 +695,28 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 
 QSIMPLEQ_FOREACH(bm, bm_list, entry) {
 uint64_t *bitmap_table = NULL;
+Qcow2BitmapTableInfoEntryList **pp_table;
 int i;
 
+Qcow2BitmapDirectoryEntry *bmde = NULL;
+if (bitmaps) {
+bmde = g_new0(Qcow2BitmapDirectoryEntry, 1);
+bmde->bitmap_name = g_strdup(bm->name);
+bmde->bitmap_table = g_new0(Qcow2BitmapTableInfo, 1);
+bmde->bitmap_table->location = g_new0(Qcow2Allocation, 1);
+Qcow2BitmapDirectoryEntryList *obj =
+g_new0(Qcow2BitmapDirectoryEntryList, 1);
+obj->value = bmde;
+*pp_dir = obj;
+pp_dir = &obj->next;
+}
+
 ret = qcow2_inc_refcounts_imrt(bs, res,
refcount_table, refcount_table_size,
bm->table.offset,
-   bm->table.size * sizeof(uint64_t));
+   bm->table.size * sizeof(uint64_t),
+   bmde ? bmde->bitmap_table->location
+   : NULL);
 if (ret < 0) {
 goto out;
 }
@@ -702,6 +727,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 goto out;
 }
 
+pp_table = bmde ? &bmde->bitmap_table->table_entries : NULL;
+
 for (i = 0; i < bm->table.size; ++i) {
 uint64_t entry = bitmap_table[i];
 uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK;
@@ -711,13 +738,32 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 continue;
 }
 
+Qcow2BitmapTableInfoEntry *bmte = NULL;
+if (bmde) {
+bmte = g_new0(Qcow2BitmapTableInfoEntry, 1);
+bmte->type = offset ?
+QCOW2_BITMAP_TABLE_INFO_ENTRY_TYPE_SERIALIZED :
+entry & BME_TABLE_ENTRY_FLAG_ALL_ONES;
+if (offset) {
+bmte->cluster = g_new0(Qcow2Allocation, 1);
+}
+bmte->has_cluster = !!(bmte->cluster);
+Qcow2BitmapTableInfoEntryList *elem =
+g_new0(Qcow2BitmapTableInfoEntryList, 1);
+elem->value = bmte;
+*pp_table = elem;
+pp_table = &elem->next;
+}
+
 if (offset == 0) {
 continue;
 }
 
 ret = qcow2_inc_refcounts_imrt(bs, res,
refcount_table, refcount_table_size,
-   offset, s->cluster_size);
+   offset, s->cluster_size,
+   bmte && bmte->cluster ? 
bmte->cluster
+   : NULL);
 if (ret < 0) {
 g_free(b

Re: [PATCH 1/4] linux-user: Use `qemu_log' for non-strace logging

2020-01-14 Thread Laurent Vivier
Le 14/01/2020 à 04:01, Josh Kunz a écrit :
> This change introduces a new logging mask "LOG_USER", which is used for
> masking general user-mode logging. This change also switches all non-strace
> uses of `gemu_log' in linux-user/ to use `qemu_log_mask(LOG_USER, ...)'
> instead. This allows the user to easily log to a file, and to mask out
> these log messages if they desire.
> 
> Signed-off-by: Josh Kunz 
> ---
>  include/qemu/log.h|  2 ++
>  linux-user/arm/cpu_loop.c |  5 ++--
>  linux-user/fd-trans.c | 55 +--
>  linux-user/main.c | 24 +
>  linux-user/syscall.c  | 30 -
>  linux-user/vm86.c |  3 ++-
>  util/log.c|  3 +++
>  7 files changed, 86 insertions(+), 36 deletions(-)

I think most of these log messages should use LOG_UNIMP, so no need for
a new flag.

Thanks,
Laurent




Re: [PATCH 066/104] virtiofsd: passthrough_ll: add renameat2 support

2020-01-14 Thread Daniel P . Berrangé
On Mon, Jan 13, 2020 at 08:06:24PM +, Dr. David Alan Gilbert wrote:
> * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > On Thu, Dec 12, 2019 at 04:38:26PM +, Dr. David Alan Gilbert (git) 
> > > wrote:
> > > > From: Miklos Szeredi 
> > > > 
> > > > No glibc support yet, so use syscall().
> > > 
> > > It exists in glibc in my Fedora 31 install.
> > > 
> > > Presumably this is related to an older version
> > > 
> > > > Signed-off-by: Miklos Szeredi 
> > > > ---
> > > >  tools/virtiofsd/passthrough_ll.c | 10 ++
> > > >  1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git a/tools/virtiofsd/passthrough_ll.c 
> > > > b/tools/virtiofsd/passthrough_ll.c
> > > > index 91d3120033..bed2270141 100644
> > > > --- a/tools/virtiofsd/passthrough_ll.c
> > > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > > @@ -1083,7 +1083,17 @@ static void lo_rename(fuse_req_t req, fuse_ino_t 
> > > > parent, const char *name,
> > > >  }
> > > >  
> > > >  if (flags) {
> > > > +#ifndef SYS_renameat2
> > > >  fuse_reply_err(req, EINVAL);
> > > > +#else
> > > > +res = syscall(SYS_renameat2, lo_fd(req, parent), name,
> > > > +  lo_fd(req, newparent), newname, flags);
> > > > +if (res == -1 && errno == ENOSYS) {
> > > > +fuse_reply_err(req, EINVAL);
> > > > +} else {
> > > > +fuse_reply_err(req, res == -1 ? errno : 0);
> > > > +}
> > > > +#endif
> > > 
> > > We should use the formal API if available as first choice
> > 
> > OK, done - I've kept the 'ifndef SYS_renameat2' that drops back to an
> > error for truly ancient cases; although I doubt everything else will
> > build on something that old.
> 
> Hmm, and this breaks on middle age distros;  older distros don't have it
> at all, new ones have both the syscall and the wrapper; but for the
> middle age ones they have the syscall but not the wrapper.
>
> Dan: What's your preference here; should I add a config fragment to
> detect the wrapper - it seems overkill rather than just reverting it
> until it becomes common.

What specific middle age distro in particular is affected ? My general
thought would be to /not/ support such distros. Focus on modern distros
since this is a brand new feature in QEMU, where we should try to
minimize support for legacy stuff at the start. But depending on the
distro impacted, the might be a reason to stay with SYS_..

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




Re: [PATCH 1/4] linux-user: Use `qemu_log' for non-strace logging

2020-01-14 Thread Laurent Vivier
Le 14/01/2020 à 04:01, Josh Kunz a écrit :
> This change introduces a new logging mask "LOG_USER", which is used for
> masking general user-mode logging. This change also switches all non-strace
> uses of `gemu_log' in linux-user/ to use `qemu_log_mask(LOG_USER, ...)'
> instead. This allows the user to easily log to a file, and to mask out
> these log messages if they desire.
> 
> Signed-off-by: Josh Kunz 
> ---
>  include/qemu/log.h|  2 ++
>  linux-user/arm/cpu_loop.c |  5 ++--
>  linux-user/fd-trans.c | 55 +--
>  linux-user/main.c | 24 +
>  linux-user/syscall.c  | 30 -
>  linux-user/vm86.c |  3 ++-
>  util/log.c|  3 +++
>  7 files changed, 86 insertions(+), 36 deletions(-)
> 

I think most of these log messages should use LOG_UNIMP, so no need for
a new flag.

Thanks,
Laurent




Re: [PATCH v12 04/13] virtio-iommu: Add the iommu regions

2020-01-14 Thread Auger Eric
Hi Peter,

On 1/13/20 8:53 PM, Peter Xu wrote:
> On Thu, Jan 09, 2020 at 03:43:10PM +0100, Eric Auger wrote:
>> Implement a callback called on PCI bus enumeration that
>> initializes for a given device on the bus hierarchy
>> an IOMMU memory region. The PCI bus hierarchy is stored
>> locally in IOMMUPciBus and IOMMUDevice objects.
>>
>> At the time of the enumeration, the bus number may not be
>> computed yet.
>>
>> So operations that will need to retrieve the IOMMUdevice
>> and its IOMMU memory region from the bus number and devfn,
>> once the bus number is garanteed to be frozen,
>> use an array of IOMMUPciBus, lazily populated.
>>
>> virtio_iommu_mr() is the top helper that allows to retrieve
>> the IOMMU memory region from the requester ID.
>>
>> Signed-off-by: Eric Auger 
>>
>> ---
>> v11 -> v12:
>> - add the iommu_find_iommu_pcibus() mechanics. Without it,
>>   when attaching t device to a domain we could not check
>>   the device is effectively protected by this IOMMU
> 
> Sorry I probably lost the context again after read the previous
> version...  Could you hint me what does this used for?
In v11 Jean pointed out that as_by_bus_num was not used in my series. I
first planned to remove it and then noticed that it could be useful to
test on "attach" whether the RID of the device effectively corresponds
to a device protected by the IOMMU and in the negative, return an error.

In https://patchwork.kernel.org/patch/11258269/#23067995

This is the same mechanics used in intel_iommu/smmu.


> 
> In all cases, I see that virtio_iommu_mr() is introduced but not used.
> Would be good to put it into the patch where it's firstly used.
OK fair enough, I will put the helper in the same patch as the user as
you have requested that since the beginning ;-) The resulting patch may
be huge. Just hope nobody will request me to split it back ;-)

Thanks

Eric
> 
> Thanks,
> 




Re: [PATCH v12 04/13] virtio-iommu: Add the iommu regions

2020-01-14 Thread Auger Eric
Hi Peter,

On 1/13/20 9:06 PM, Peter Xu wrote:
> On Thu, Jan 09, 2020 at 03:43:10PM +0100, Eric Auger wrote:
>> +/**
>> + * The bus number is used for lookup when SID based operations occur.
>> + * In that case we lazily populate the IOMMUPciBus array from the bus hash
>> + * table. At the time the IOMMUPciBus is created (iommu_find_add_as), the 
>> bus
>> + * numbers may not be always initialized yet.
>> + */
>> +static IOMMUPciBus *iommu_find_iommu_pcibus(VirtIOIOMMU *s, uint8_t bus_num)
>> +{
>> +IOMMUPciBus *iommu_pci_bus = s->iommu_pcibus_by_bus_num[bus_num];
>> +
>> +if (!iommu_pci_bus) {
>> +GHashTableIter iter;
>> +
>> +g_hash_table_iter_init(&iter, s->as_by_busptr);
>> +while (g_hash_table_iter_next(&iter, NULL, (void 
>> **)&iommu_pci_bus)) {
>> +if (pci_bus_num(iommu_pci_bus->bus) == bus_num) {
>> +s->iommu_pcibus_by_bus_num[bus_num] = iommu_pci_bus;
>> +return iommu_pci_bus;
>> +}
>> +}
> 
> Btw, we may need to:
> 
>return NULL;
Yes. By the way Yi's patch "intel_iommu: a fix to
vtd_find_as_from_bus_num()" also applies to SMMU code. I will send a patch.

Thanks

Eric
> 
> here.
> 
>> +}
>> +return iommu_pci_bus;
>> +}
> 




Re: [PATCH v12 05/13] virtio-iommu: Endpoint and domains structs and helpers

2020-01-14 Thread Auger Eric
Hi Peter,

On 1/13/20 9:23 PM, Peter Xu wrote:
> On Thu, Jan 09, 2020 at 03:43:11PM +0100, Eric Auger wrote:
> 
> [...]
> 
>> +VirtIOIOMMUEndpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t 
>> ep_id);
>> +VirtIOIOMMUEndpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s, uint32_t 
>> ep_id)
> 
> Is the extra definition trying to workaround the compiler
> warning/error?

yes it does
> 
> I'm not sure whether it's only me who prefer this, but again I'd
> really perfer we move the function into the caller patch, add "static"
> as needed altogether, even if that patch can be big.

OK I will do that.
> 
>> +{
>> +VirtIOIOMMUEndpoint *ep;
>> +
>> +ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
>> +if (ep) {
>> +return ep;
>> +}
>> +if (!virtio_iommu_mr(s, ep_id)) {
> 
> Could I ask when this will trigger?

This can happen when a device is attached to a domain and its RID does
not correspond to one of the devices protected by the iommu.

Thanks

Eric
> 
>> +return NULL;
>> +}
>> +ep = g_malloc0(sizeof(*ep));
>> +ep->id = ep_id;
>> +trace_virtio_iommu_get_endpoint(ep_id);
>> +g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
>> +return ep;
>> +}
> 
> Thanks,
> 




Re: Priority of -accel

2020-01-14 Thread Markus Armbruster
Paolo Bonzini  writes:

> On 13/01/20 17:17, Markus Armbruster wrote:
>> Perfect opportunity to change the default to something more useful.
>
> I am not sure acutally if it's that more useful, now that we have
> sanctioned qemu-kvm as the fast alternative.

If there is a fast alternative, why ship the slow one?

> Particularly it would be confusing for qemu-system-x86_64 to use
> hardware virtualization on Linux, but not on other operating systems
> where the accelerators are not stable enough.

Hardly more confusing than qemu-kvm not using hardware virtualization
when /dev/kvm is unavailable.

No matter what we do, somebody is going to be confused.  How to resolve
such a conundrum?  Utilitarian philosophy teaches us to pursue the
greatest confusion of the greatest numbers.  I think not using x86
hardware virtualization by default has been admirably successful there.

;-P




Re: [RFC PATCH qemu] spapr: Kill SLOF

2020-01-14 Thread Michal Suchánek
On Tue, Jan 07, 2020 at 12:23:13PM +0100, Thomas Huth wrote:
> On 07/01/2020 10.39, Andrea Bolognani wrote:
> > On Tue, 2020-01-07 at 12:55 +1100, Alexey Kardashevskiy wrote:
> >> On 07/01/2020 01:15, Daniel Henrique Barboza wrote:
> >>> Question: does Petitboot already replaces SLOF in every possible
> >>> scenario for all
> >>> the spapr machine features?
> >>
> >> Petitboot kernel+initramdisk almost replaces SLOF + GRUB.
> > 
> > Is this necessarily a good thing? Personally I quite like the fact
> > that I can use the same bootloader across x86, ppc64 and aarch64.
> 
> AFAIK petitboot can use the grub config files ... and it is already used
> on bare metal POWER8 and POWER9 systems, so it should not be such a big
> change to use it for POWER KVM guests, too?

Except it is not really pSeries then anymore.

Thanks

Michal



Re: [PATCH v7] migration: Support QLIST migration

2020-01-14 Thread Juan Quintela
Eric Auger  wrote:
> Support QLIST migration using the same principle as QTAILQ:
> 94869d5c52 ("migration: migrate QTAILQ").
>
> The VMSTATE_QLIST_V macro has the same proto as VMSTATE_QTAILQ_V.
> The change mainly resides in QLIST RAW macros: QLIST_RAW_INSERT_HEAD
> and QLIST_RAW_REVERSE.
>
> Tests also are provided.
>
> Signed-off-by: Eric Auger 
> Reviewed-by: Peter Xu 
> Reviewed-by: Juan Quintela 
> Signed-off-by: Juan Quintela 

Reviewed-by: Juan Quintela 

(again)

queued.




Re: [PATCH 2/4] linux-user: Use `qemu_log' for strace

2020-01-14 Thread Laurent Vivier
Le 14/01/2020 à 04:01, Josh Kunz a écrit :
> This change switches linux-user strace logging to use the newer `qemu_log`
> logging subsystem rather than the older `gemu_log` (notice the "g")
> logger. `qemu_log` has several advantages, namely that it allows logging
> to a file, and provides a more unified interface for configuration
> of logging (via the QEMU_LOG environment variable or options).
> 
> Signed-off-by: Josh Kunz 
> ---
>  include/qemu/log.h   |  13 ++
>  linux-user/main.c|  17 +-
>  linux-user/qemu.h|   1 -
>  linux-user/signal.c  |   3 +-
>  linux-user/strace.c  | 479 ++-
>  linux-user/syscall.c |  13 +-
>  util/log.c   |   2 +
>  7 files changed, 282 insertions(+), 246 deletions(-)
> 
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index 503e4f88d5..8f044c1716 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -64,6 +64,7 @@ static inline bool qemu_log_separate(void)
>  #define CPU_LOG_PLUGIN (1 << 18)
>  /* LOG_USER is used for some informational user-mode logging. */
>  #define LOG_USER   (1 << 19)
> +#define LOG_STRACE (1 << 20)

Perhaps we can use flag LOG_TRACE? (cc Paolo)

>  
>  /* Lock output for a series of related logs.  Since this is not needed
>   * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we
> @@ -154,6 +155,18 @@ void qemu_set_dfilter_ranges(const char *ranges, Error 
> **errp);
>  bool qemu_log_in_addr_range(uint64_t addr);
>  int qemu_str_to_log_mask(const char *str);
>  
> +/* Add (union) the given "log_flags" to the current log mask. */
> +static inline void qemu_add_log(int log_flags)
> +{
> +qemu_set_log(qemu_loglevel | log_flags);
> +}

This is really a special case as the flags are all given at the same
time, could you use directly qemu_set_log() in main()?

> +/* Remove (subtract) the given log flags from the current log mask. */
> +static inline void qemu_del_log(int log_flags)
> +{
> +qemu_set_log(qemu_loglevel & ~(log_flags));
> +}

Unused and unneeded.

Except that, the patch seems good.

Thanks,
Laurent



[PATCH v2] scsi-disk: define props in scsi_block_disk to avoid memleaks

2020-01-14 Thread pannengyuan
From: Pan Nengyuan 

scsi_block_realize() use scsi_realize() to init some props, but
these props is not defined in scsi_block_disk_properties, so they will
not be freed.

This patch defines these prop in scsi_block_disk_properties and aslo
calls scsi_unrealize to avoid memleaks, the leak stack as
follow(it's easy to reproduce by attaching/detaching scsi-block-disks):

=
==qemu-system-x86_64==32195==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 57 byte(s) in 3 object(s) allocated from:
  #0 0x7f19f8bed768 (/lib64/libasan.so.5+0xef768)  ??:?
  #1 0x7f19f64d9445 (/lib64/libglib-2.0.so.0+0x52445)  ??:?
  #2 0x7f19f64f2d92 (/lib64/libglib-2.0.so.0+0x6bd92)  ??:?
  #3 0x55975366e596 (qemu-system-x86_64+0x35c0596)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2399
  #4 0x559753671201 (emu-system-x86_64+0x35c3201)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2681
  #5 0x559753687e3e (qemu-system-x86_64+0x35d9e3e)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:58
  #6 0x55975368ac44 (qemu-system-x86_64+0x35dcc44)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:216
  #7 0x5597532a7840 (qemu-system-x86_64+0x31f9840)  
/mnt/sdb/qemu/hw/core/qdev.c:876

Direct leak of 15 byte(s) in 3 object(s) allocated from:
  #0 0x7f19f8bed768 (/lib64/libasan.so.5+0xef768)  ??:?
  #1 0x7f19f64d9445 (/lib64/libglib-2.0.so.0+0x52445)  ??:?
  #2 0x7f19f64f2d92 (/lib64/libglib-2.0.so.0+0x6bd92)  ??:?
  #3 0x55975366e06f (qemu-system-x86_64+0x35c006f)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2388
  #4 0x559753671201 (qemu-system-x86_64+0x35c3201)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2681
  #5 0x559753687e3e (qemu-system-x86_64+0x35d9e3e)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:58
  #6 0x55975368ac44 (qemu-system-x86_64+0x35dcc44)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:216

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- move 'drive' to the front to keep the original props's order.
---
 hw/scsi/scsi-disk.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e44c61eeb4..a1194b9fa7 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2981,7 +2981,6 @@ static const TypeInfo scsi_disk_base_info = {
 };
 
 #define DEFINE_SCSI_DISK_PROPERTIES()   \
-DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),  \
 DEFINE_BLOCK_PROPERTIES_BASE(SCSIDiskState, qdev.conf), \
 DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf),\
 DEFINE_PROP_STRING("ver", SCSIDiskState, version),  \
@@ -2992,6 +2991,7 @@ static const TypeInfo scsi_disk_base_info = {
 
 
 static Property scsi_hd_properties[] = {
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_BIT("removable", SCSIDiskState, features,
 SCSI_DISK_F_REMOVABLE, false),
@@ -3047,6 +3047,7 @@ static const TypeInfo scsi_hd_info = {
 };
 
 static Property scsi_cd_properties[] = {
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_UINT64("wwn", SCSIDiskState, qdev.wwn, 0),
 DEFINE_PROP_UINT64("port_wwn", SCSIDiskState, qdev.port_wwn, 0),
@@ -3079,9 +3080,8 @@ static const TypeInfo scsi_cd_info = {
 
 #ifdef __linux__
 static Property scsi_block_properties[] = {
-DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
+DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk),
-DEFINE_PROP_BOOL("share-rw", SCSIDiskState, qdev.conf.share_rw, false),
 DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
 DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size,
DEFAULT_MAX_UNMAP_SIZE),
@@ -3099,6 +3099,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, 
void *data)
 SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
 
 sc->realize  = scsi_block_realize;
+sc->unrealize= scsi_unrealize;
 sc->alloc_req= scsi_block_new_request;
 sc->parse_cdb= scsi_block_parse_cdb;
 sdc->dma_readv   = scsi_block_dma_readv;
@@ -3118,6 +3119,7 @@ static const TypeInfo scsi_block_info = {
 #endif
 
 static Property scsi_disk_properties[] = {
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_BIT("removable", SCSIDiskState, features,
 SCSI_DISK_F_REMOVABLE, false),
-- 
2.21.0.windows.1





Re: [PATCH] vhost-vsock: delete vqs in vhost_vsock_unrealize to avoid memleaks

2020-01-14 Thread Stefano Garzarella
On Tue, Jan 14, 2020 at 03:52:29PM +0800, pannengy...@huawei.com wrote:
> From: Pan Nengyuan 
> 
> Receive/transmit/event vqs forgot to cleanup in vhost_vsock_unrealize. This
> patch save receive/transmit vq pointer in realize() and cleanup vqs
> through those vq pointers in unrealize(). The leak stack is as follow:
> 
> Direct leak of 21504 byte(s) in 3 object(s) allocated from:
>   #0 0x7f86a1356970 (/lib64/libasan.so.5+0xef970)  ??:?
>   #1 0x7f86a09aa49d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
>   #2 0x5604852f85ca (./x86_64-softmmu/qemu-system-x86_64+0x2c3e5ca)  
> /mnt/sdb/qemu/hw/virtio/virtio.c:2333
>   #3 0x560485356208 (./x86_64-softmmu/qemu-system-x86_64+0x2c9c208)  
> /mnt/sdb/qemu/hw/virtio/vhost-vsock.c:339
>   #4 0x560485305a17 (./x86_64-softmmu/qemu-system-x86_64+0x2c4ba17)  
> /mnt/sdb/qemu/hw/virtio/virtio.c:3531
>   #5 0x5604858e6b65 (./x86_64-softmmu/qemu-system-x86_64+0x322cb65)  
> /mnt/sdb/qemu/hw/core/qdev.c:865
>   #6 0x5604861e6c41 (./x86_64-softmmu/qemu-system-x86_64+0x3b2cc41)  
> /mnt/sdb/qemu/qom/object.c:2102
> 
> Reported-by: Euler Robot 
> Signed-off-by: Pan Nengyuan 
> ---
>  hw/virtio/vhost-vsock.c | 9 +++--
>  include/hw/virtio/vhost-vsock.h | 2 ++
>  2 files changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Stefano Garzarella 

> 
> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
> index f5744363a8..896c0174c1 100644
> --- a/hw/virtio/vhost-vsock.c
> +++ b/hw/virtio/vhost-vsock.c
> @@ -335,8 +335,10 @@ static void vhost_vsock_device_realize(DeviceState *dev, 
> Error **errp)
>  sizeof(struct virtio_vsock_config));
>  
>  /* Receive and transmit queues belong to vhost */
> -virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE, 
> vhost_vsock_handle_output);
> -virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE, 
> vhost_vsock_handle_output);
> +vsock->recv_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> +  vhost_vsock_handle_output);
> +vsock->trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> +   vhost_vsock_handle_output);
>  
>  /* The event queue belongs to QEMU */
>  vsock->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> @@ -378,6 +380,9 @@ static void vhost_vsock_device_unrealize(DeviceState 
> *dev, Error **errp)
>  /* This will stop vhost backend if appropriate. */
>  vhost_vsock_set_status(vdev, 0);
>  
> +virtio_delete_queue(vsock->recv_vq);
> +virtio_delete_queue(vsock->trans_vq);
> +virtio_delete_queue(vsock->event_vq);
>  vhost_dev_cleanup(&vsock->vhost_dev);
>  virtio_cleanup(vdev);
>  }
> diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
> index d509d67c4a..bc5a988ee5 100644
> --- a/include/hw/virtio/vhost-vsock.h
> +++ b/include/hw/virtio/vhost-vsock.h
> @@ -33,6 +33,8 @@ typedef struct {
>  struct vhost_virtqueue vhost_vqs[2];
>  struct vhost_dev vhost_dev;
>  VirtQueue *event_vq;
> +VirtQueue *recv_vq;
> +VirtQueue *trans_vq;
>  QEMUTimer *post_load_timer;
>  
>  /*< public >*/
> -- 
> 2.21.0.windows.1
> 
> 
> 

-- 




Re: [PATCH] target/riscv: Set mstatus.DS & FS correctly

2020-01-14 Thread ShihPo Hung
On Tue, Jan 14, 2020 at 10:32 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 1/9/20 11:05 PM, shihpo.h...@sifive.com wrote:
> > Because ctx->mstatus_fs changes dynamically during runtime, we should
> > remove the mstatus_fs check at the translation stage.
>
> This change is incorrect.
>
> The actual bug is in cpu_get_tb_cpu_state(), as I just noticed during
> review:
>
> "[PATCH 2/3] RISC-V: use FIELD macro to define tb flags"
> https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg02455.html


I didn't understand the purpose of mstatus_fs, but now I get it.
Thanks for pointing me to.
I will resend my patches.


[PULL 00/29] Migration pull patches (second try)

2020-01-14 Thread Juan Quintela
The following changes since commit 3c8a6575985b1652b45bfa670b5e1907d642cfa0:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20200113-pull-request' 
into staging (2020-01-13 14:19:57 +)

are available in the Git repository at:

  https://github.com/juanquintela/qemu.git tags/migration-pull-pull-request

for you to fetch changes up to 4eafab585c091050b5ae63130f46fe46ac919c3a:

  migration: Support QLIST migration (2020-01-14 10:17:12 +0100)


Migration pull request
- updated QList patch
- initialize local msg variable



Alexey Romko (1):
  Bug #1829242 correction.

Daniel Henrique Barboza (1):
  ram.c: remove unneeded labels

Dr. David Alan Gilbert (1):
  migration: Rate limit inside host pages

Eric Auger (1):
  migration: Support QLIST migration

Fangrui Song (1):
  migration: Fix incorrect integer->float conversion caught by clang

Jiahui Cen (2):
  migration/multifd: fix nullptr access in terminating multifd threads
  migration/multifd: fix destroyed mutex access in terminating multifd
threads

Juan Quintela (4):
  multifd: Initialize local variable
  migration-test: Add migration multifd test
  migration: Make sure that we don't call write() in case of error
  migration-test: introduce functions to handle string parameters

Laurent Vivier (2):
  migration-test: ppc64: fix FORTH test program
  runstate: ignore finishmigrate -> prelaunch transition

Marc-André Lureau (1):
  misc: use QEMU_IS_ALIGNED

Peter Xu (3):
  migration: Define VMSTATE_INSTANCE_ID_ANY
  migration: Change SaveStateEntry.instance_id into uint32_t
  apic: Use 32bit APIC ID for migration instance ID

Scott Cheloha (2):
  migration: add savevm_state_handler_remove()
  migration: savevm_state_handler_insert: constant-time element
insertion

Wei Yang (8):
  migration/postcopy: reduce memset when it is zero page and
matches_target_page_size
  migration/postcopy: wait for decompress thread in precopy
  migration/postcopy: count target page number to decide the
place_needed
  migration/postcopy: set all_zero to true on the first target page
  migration/postcopy: enable random order target page arrival
  migration/postcopy: enable compress during postcopy
  migration/multifd: clean pages after filling packet
  migration/multifd: not use multifd during postcopy

Yury Kotov (2):
  migration: Fix the re-run check of the migrate-incoming command
  migration/ram: Yield periodically to the main loop

 backends/dbus-vmstate.c  |   3 +-
 exec.c   |   4 +-
 hw/arm/stellaris.c   |   2 +-
 hw/core/qdev.c   |   3 +-
 hw/display/ads7846.c |   2 +-
 hw/i2c/core.c|   2 +-
 hw/input/stellaris_input.c   |   3 +-
 hw/intc/apic_common.c|   7 +-
 hw/misc/max111x.c|   3 +-
 hw/net/eepro100.c|   3 +-
 hw/pci/pci.c |   2 +-
 hw/ppc/spapr.c   |   2 +-
 hw/timer/arm_timer.c |   2 +-
 hw/tpm/tpm_emulator.c|   3 +-
 include/migration/register.h |   2 +-
 include/migration/vmstate.h  |  25 -
 include/qemu/queue.h |  39 
 migration/migration.c|  72 +++---
 migration/migration.h|   1 +
 migration/ram.c  | 183 ++-
 migration/savevm.c   |  61 
 migration/trace-events   |   9 +-
 migration/vmstate-types.c|  70 ++
 stubs/vmstate.c  |   2 +-
 tests/qtest/migration-test.c |  97 ++-
 tests/test-vmstate.c | 170 
 vl.c |  10 +-
 27 files changed, 653 insertions(+), 129 deletions(-)

-- 
2.24.1




[PULL 02/29] migration-test: Add migration multifd test

2020-01-14 Thread Juan Quintela
We set multifd-channels.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Thomas Huth 
Tested-by: Wei Yang 
---
 tests/qtest/migration-test.c | 56 
 1 file changed, 56 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 53afec4395..fb70214f44 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1202,6 +1202,61 @@ static void test_migrate_auto_converge(void)
 test_migrate_end(from, to, true);
 }
 
+static void test_multifd_tcp(void)
+{
+MigrateStart *args = migrate_start_new();
+QTestState *from, *to;
+QDict *rsp;
+char *uri;
+
+if (test_migrate_start(&from, &to, "defer", args)) {
+return;
+}
+
+/*
+ * We want to pick a speed slow enough that the test completes
+ * quickly, but that it doesn't complete precopy even on a slow
+ * machine, so also set the downtime.
+ */
+/* 1 ms should make it not converge*/
+migrate_set_parameter_int(from, "downtime-limit", 1);
+/* 1GB/s */
+migrate_set_parameter_int(from, "max-bandwidth", 10);
+
+migrate_set_parameter_int(from, "multifd-channels", 16);
+migrate_set_parameter_int(to, "multifd-channels", 16);
+
+migrate_set_capability(from, "multifd", "true");
+migrate_set_capability(to, "multifd", "true");
+
+/* Start incoming migration from the 1st socket */
+rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
+   "  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
+qobject_unref(rsp);
+
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+uri = migrate_get_socket_address(to, "socket-address");
+
+migrate_qmp(from, uri, "{}");
+
+wait_for_migration_pass(from);
+
+/* 300ms it should converge */
+migrate_set_parameter_int(from, "downtime-limit", 300);
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+qtest_qmp_eventwait(to, "RESUME");
+
+wait_for_serial("dest_serial");
+wait_for_migration_complete(from);
+test_migrate_end(from, to, true);
+free(uri);
+}
+
 int main(int argc, char **argv)
 {
 char template[] = "/tmp/migration-test-XX";
@@ -1266,6 +1321,7 @@ int main(int argc, char **argv)
test_validate_uuid_dst_not_set);
 
 qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
+qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
 
 ret = g_test_run();
 
-- 
2.24.1




[PULL 04/29] migration-test: introduce functions to handle string parameters

2020-01-14 Thread Juan Quintela
Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 tests/qtest/migration-test.c | 37 
 1 file changed, 37 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index fb70214f44..b0580dd541 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -356,6 +356,43 @@ static void migrate_set_parameter_int(QTestState *who, 
const char *parameter,
 migrate_check_parameter_int(who, parameter, value);
 }
 
+static char *migrate_get_parameter_str(QTestState *who,
+   const char *parameter)
+{
+QDict *rsp;
+char *result;
+
+rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
+result = g_strdup(qdict_get_str(rsp, parameter));
+qobject_unref(rsp);
+return result;
+}
+
+static void migrate_check_parameter_str(QTestState *who, const char *parameter,
+const char *value)
+{
+char *result;
+
+result = migrate_get_parameter_str(who, parameter);
+g_assert_cmpstr(result, ==, value);
+g_free(result);
+}
+
+__attribute__((unused))
+static void migrate_set_parameter_str(QTestState *who, const char *parameter,
+  const char *value)
+{
+QDict *rsp;
+
+rsp = qtest_qmp(who,
+"{ 'execute': 'migrate-set-parameters',"
+"'arguments': { %s: %s } }",
+parameter, value);
+g_assert(qdict_haskey(rsp, "return"));
+qobject_unref(rsp);
+migrate_check_parameter_str(who, parameter, value);
+}
+
 static void migrate_pause(QTestState *who)
 {
 QDict *rsp;
-- 
2.24.1




[PULL 06/29] runstate: ignore finishmigrate -> prelaunch transition

2020-01-14 Thread Juan Quintela
From: Laurent Vivier 

Commit 1bd71dce4bf2 tries to prevent a finishmigrate -> prelaunch
transition by exiting at the beginning of the main_loop_should_exit()
function if the state is already finishmigrate.

As the finishmigrate state is set in the migration thread it can
happen concurrently to the function. The migration thread and the
function are normally protected by the iothread mutex and thus the
state should no evolve between the start of the function and its end.

Unfortunately during the function life the lock is released by
pause_all_vcpus() just before the point we need to be sure we are
not in finishmigrate state and if the migration thread is waiting
for the lock it will take the opportunity to change the state
to finishmigrate.

The only way to be sure we are not in the finishmigrate state when
we need is to check the state after the pause_all_vcpus() function.

Fixes: 1bd71dce4bf2 ("runstate: ignore exit request in finish migrate state")
Signed-off-by: Laurent Vivier 
Signed-off-by: Juan Quintela 
---
 vl.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 158a05ed32..ba3e176094 100644
--- a/vl.c
+++ b/vl.c
@@ -1604,9 +1604,6 @@ static bool main_loop_should_exit(void)
 RunState r;
 ShutdownCause request;
 
-if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
-return false;
-}
 if (preconfig_exit_requested) {
 if (runstate_check(RUN_STATE_PRECONFIG)) {
 runstate_set(RUN_STATE_PRELAUNCH);
@@ -1635,8 +1632,13 @@ static bool main_loop_should_exit(void)
 pause_all_vcpus();
 qemu_system_reset(request);
 resume_all_vcpus();
+/*
+ * runstate can change in pause_all_vcpus()
+ * as iothread mutex is unlocked
+ */
 if (!runstate_check(RUN_STATE_RUNNING) &&
-!runstate_check(RUN_STATE_INMIGRATE)) {
+!runstate_check(RUN_STATE_INMIGRATE) &&
+!runstate_check(RUN_STATE_FINISH_MIGRATE)) {
 runstate_set(RUN_STATE_PRELAUNCH);
 }
 }
-- 
2.24.1




[PULL 01/29] multifd: Initialize local variable

2020-01-14 Thread Juan Quintela
Fill everything with zero, so the padding fields are also initialized.
---
 migration/ram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 96feb4062c..b9147bcca3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -703,7 +703,7 @@ typedef struct {
 
 static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
 {
-MultiFDInit_t msg;
+MultiFDInit_t msg = {};
 int ret;
 
 msg.magic = cpu_to_be32(MULTIFD_MAGIC);
-- 
2.24.1




[PULL 08/29] migration: Rate limit inside host pages

2020-01-14 Thread Juan Quintela
From: "Dr. David Alan Gilbert" 

When using hugepages, rate limiting is necessary within each huge
page, since a 1G huge page can take a significant time to send, so
you end up with bursty behaviour.

Fixes: 4c011c37ecb3 ("postcopy: Send whole huge pages")
Reported-by: Lin Ma 
Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Reviewed-by: Peter Xu 
Signed-off-by: Juan Quintela 
---
 migration/migration.c  | 57 --
 migration/migration.h  |  1 +
 migration/ram.c|  2 ++
 migration/trace-events |  4 +--
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 354ad072fa..27500d09a9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3224,6 +3224,37 @@ void migration_consume_urgent_request(void)
 qemu_sem_wait(&migrate_get_current()->rate_limit_sem);
 }
 
+/* Returns true if the rate limiting was broken by an urgent request */
+bool migration_rate_limit(void)
+{
+int64_t now = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+MigrationState *s = migrate_get_current();
+
+bool urgent = false;
+migration_update_counters(s, now);
+if (qemu_file_rate_limit(s->to_dst_file)) {
+/*
+ * Wait for a delay to do rate limiting OR
+ * something urgent to post the semaphore.
+ */
+int ms = s->iteration_start_time + BUFFER_DELAY - now;
+trace_migration_rate_limit_pre(ms);
+if (qemu_sem_timedwait(&s->rate_limit_sem, ms) == 0) {
+/*
+ * We were woken by one or more urgent things but
+ * the timedwait will have consumed one of them.
+ * The service routine for the urgent wake will dec
+ * the semaphore itself for each item it consumes,
+ * so add this one we just eat back.
+ */
+qemu_sem_post(&s->rate_limit_sem);
+urgent = true;
+}
+trace_migration_rate_limit_post(urgent);
+}
+return urgent;
+}
+
 /*
  * Master migration thread on the source VM.
  * It drives the migration and pumps the data down the outgoing channel.
@@ -3290,8 +3321,6 @@ static void *migration_thread(void *opaque)
 trace_migration_thread_setup_complete();
 
 while (migration_is_active(s)) {
-int64_t current_time;
-
 if (urgent || !qemu_file_rate_limit(s->to_dst_file)) {
 MigIterateState iter_state = migration_iteration_run(s);
 if (iter_state == MIG_ITERATE_SKIP) {
@@ -3318,29 +3347,7 @@ static void *migration_thread(void *opaque)
 update_iteration_initial_status(s);
 }
 
-current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
-
-migration_update_counters(s, current_time);
-
-urgent = false;
-if (qemu_file_rate_limit(s->to_dst_file)) {
-/* Wait for a delay to do rate limiting OR
- * something urgent to post the semaphore.
- */
-int ms = s->iteration_start_time + BUFFER_DELAY - current_time;
-trace_migration_thread_ratelimit_pre(ms);
-if (qemu_sem_timedwait(&s->rate_limit_sem, ms) == 0) {
-/* We were worken by one or more urgent things but
- * the timedwait will have consumed one of them.
- * The service routine for the urgent wake will dec
- * the semaphore itself for each item it consumes,
- * so add this one we just eat back.
- */
-qemu_sem_post(&s->rate_limit_sem);
-urgent = true;
-}
-trace_migration_thread_ratelimit_post(urgent);
-}
+urgent = migration_rate_limit();
 }
 
 trace_migration_thread_after_loop();
diff --git a/migration/migration.h b/migration/migration.h
index 79b3dda146..aa9ff6f27b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -341,5 +341,6 @@ int foreach_not_ignored_block(RAMBlockIterFunc func, void 
*opaque);
 
 void migration_make_urgent_request(void);
 void migration_consume_urgent_request(void);
+bool migration_rate_limit(void);
 
 #endif
diff --git a/migration/ram.c b/migration/ram.c
index 17cd8d524b..1ec5c10561 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2639,6 +2639,8 @@ static int ram_save_host_page(RAMState *rs, 
PageSearchStatus *pss,
 
 pages += tmppages;
 pss->page++;
+/* Allow rate limiting to happen in the middle of huge pages */
+migration_rate_limit();
 } while ((pss->page & (pagesize_bits - 1)) &&
  offset_in_ramblock(pss->block, pss->page << TARGET_PAGE_BITS));
 
diff --git a/migration/trace-events b/migration/trace-events
index 6dee7b5389..2f9129e213 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -138,12 +138,12 @@ migrate_send_rp_recv_bitmap(char *name, int64_t size) 
"block '%s' size 0x%"PRIi6
 migration_completion_file_err(void) ""
 migration_c

[PULL 03/29] migration: Make sure that we don't call write() in case of error

2020-01-14 Thread Juan Quintela
If we are exiting due to an error/finish/ Just don't try to even
touch the channel with one IO operation.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index b9147bcca3..f946282adb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -900,6 +900,12 @@ struct {
 uint64_t packet_num;
 /* send channels ready */
 QemuSemaphore channels_ready;
+/*
+ * Have we already run terminate threads.  There is a race when it
+ * happens that we got one error while we are exiting.
+ * We will use atomic operations.  Only valid values are 0 and 1.
+ */
+int exiting;
 } *multifd_send_state;
 
 /*
@@ -928,6 +934,10 @@ static int multifd_send_pages(RAMState *rs)
 MultiFDPages_t *pages = multifd_send_state->pages;
 uint64_t transferred;
 
+if (atomic_read(&multifd_send_state->exiting)) {
+return -1;
+}
+
 qemu_sem_wait(&multifd_send_state->channels_ready);
 for (i = next_channel;; i = (i + 1) % migrate_multifd_channels()) {
 p = &multifd_send_state->params[i];
@@ -1009,6 +1019,16 @@ static void multifd_send_terminate_threads(Error *err)
 }
 }
 
+/*
+ * We don't want to exit each threads twice.  Depending on where
+ * we get the error, or if there are two independent errors in two
+ * threads at the same time, we can end calling this function
+ * twice.
+ */
+if (atomic_xchg(&multifd_send_state->exiting, 1)) {
+return;
+}
+
 for (i = 0; i < migrate_multifd_channels(); i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
 
@@ -1118,6 +1138,10 @@ static void *multifd_send_thread(void *opaque)
 
 while (true) {
 qemu_sem_wait(&p->sem);
+
+if (atomic_read(&multifd_send_state->exiting)) {
+break;
+}
 qemu_mutex_lock(&p->mutex);
 
 if (p->pending_job) {
@@ -1224,6 +1248,7 @@ int multifd_save_setup(void)
 multifd_send_state->params = g_new0(MultiFDSendParams, thread_count);
 multifd_send_state->pages = multifd_pages_init(page_count);
 qemu_sem_init(&multifd_send_state->channels_ready, 0);
+atomic_set(&multifd_send_state->exiting, 0);
 
 for (i = 0; i < thread_count; i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
-- 
2.24.1




[PULL 13/29] migration: savevm_state_handler_insert: constant-time element insertion

2020-01-14 Thread Juan Quintela
From: Scott Cheloha 

savevm_state's SaveStateEntry TAILQ is a priority queue.  Priority
sorting is maintained by searching from head to tail for a suitable
insertion spot.  Insertion is thus an O(n) operation.

If we instead keep track of the head of each priority's subqueue
within that larger queue we can reduce this operation to O(1) time.

savevm_state_handler_remove() becomes slightly more complex to
accomodate these gains: we need to replace the head of a priority's
subqueue when removing it.

With O(1) insertion, booting VMs with many SaveStateEntry objects is
more plausible.  For example, a ppc64 VM with maxmem=8T has 4 such
objects to insert.

Signed-off-by: Scott Cheloha 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/savevm.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 30d980caa2..e57686bca7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -250,6 +250,7 @@ typedef struct SaveStateEntry {
 
 typedef struct SaveState {
 QTAILQ_HEAD(, SaveStateEntry) handlers;
+SaveStateEntry *handler_pri_head[MIG_PRI_MAX + 1];
 int global_section_id;
 uint32_t len;
 const char *name;
@@ -261,6 +262,7 @@ typedef struct SaveState {
 
 static SaveState savevm_state = {
 .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
+.handler_pri_head = { [MIG_PRI_DEFAULT ... MIG_PRI_MAX] = NULL },
 .global_section_id = 0,
 };
 
@@ -709,24 +711,42 @@ static void savevm_state_handler_insert(SaveStateEntry 
*nse)
 {
 MigrationPriority priority = save_state_priority(nse);
 SaveStateEntry *se;
+int i;
 
 assert(priority <= MIG_PRI_MAX);
 
-QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
-if (save_state_priority(se) < priority) {
+for (i = priority - 1; i >= 0; i--) {
+se = savevm_state.handler_pri_head[i];
+if (se != NULL) {
+assert(save_state_priority(se) < priority);
 break;
 }
 }
 
-if (se) {
+if (i >= 0) {
 QTAILQ_INSERT_BEFORE(se, nse, entry);
 } else {
 QTAILQ_INSERT_TAIL(&savevm_state.handlers, nse, entry);
 }
+
+if (savevm_state.handler_pri_head[priority] == NULL) {
+savevm_state.handler_pri_head[priority] = nse;
+}
 }
 
 static void savevm_state_handler_remove(SaveStateEntry *se)
 {
+SaveStateEntry *next;
+MigrationPriority priority = save_state_priority(se);
+
+if (se == savevm_state.handler_pri_head[priority]) {
+next = QTAILQ_NEXT(se, entry);
+if (next != NULL && save_state_priority(next) == priority) {
+savevm_state.handler_pri_head[priority] = next;
+} else {
+savevm_state.handler_pri_head[priority] = NULL;
+}
+}
 QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
 }
 
-- 
2.24.1




[PULL 09/29] migration: Fix incorrect integer->float conversion caught by clang

2020-01-14 Thread Juan Quintela
From: Fangrui Song 

Clang does not like qmp_migrate_set_downtime()'s code to clamp double
@value to 0..INT64_MAX:

qemu/migration/migration.c:2038:24: error: implicit conversion from 'long' 
to 'double' changes value from 9223372036854775807 to 9223372036854775808 
[-Werror,-Wimplicit-int-float-conversion]

The warning will be enabled by default in clang 10. It is not
available for clang <= 9.

The clamp is actually useless; @value is checked to be within
0..MAX_MIGRATE_DOWNTIME_SECONDS immediately before.  Delete it.

While there, make the conversion from double to int64_t explicit.

Signed-off-by: Fangrui Song 
Reviewed-by: Markus Armbruster 
Reviewed-by: Juan Quintela 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
[Patch split, commit message improved]
Signed-off-by: Markus Armbruster 
Signed-off-by: Juan Quintela 
---
 migration/migration.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 27500d09a9..f79d0bf89a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2035,11 +2035,10 @@ void qmp_migrate_set_downtime(double value, Error 
**errp)
 }
 
 value *= 1000; /* Convert to milliseconds */
-value = MAX(0, MIN(INT64_MAX, value));
 
 MigrateSetParameters p = {
 .has_downtime_limit = true,
-.downtime_limit = value,
+.downtime_limit = (int64_t)value,
 };
 
 qmp_migrate_set_parameters(&p, errp);
-- 
2.24.1




[PULL 05/29] migration-test: ppc64: fix FORTH test program

2020-01-14 Thread Juan Quintela
From: Laurent Vivier 

Commit e51e711b1bef has moved the initialization of start_address and
end_address after the definition of the command line argument,
where the nvramrc is initialized, and thus the loop is between 0 and 0
rather than 1 MiB and 100 MiB.

It doesn't affect the result of the test if all the tests are run in
sequence because the two first tests don't run the loop, so the
values are correctly initialized when we actually need them.

But it hangs when we ask to run only one test, for instance:

QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 \
tests/migration-test -m=quick -p /ppc64/migration/validate_uuid_error

Fixes: e51e711b1bef ("tests/migration: Add migration-test header file")
Cc: w...@redhat.com
Signed-off-by: Laurent Vivier 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Reviewed-by: David Gibson 
Signed-off-by: Juan Quintela 
---
 tests/qtest/migration-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b0580dd541..26e2e77289 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -517,14 +517,14 @@ static int test_migrate_start(QTestState **from, 
QTestState **to,
 } else if (strcmp(arch, "ppc64") == 0) {
 machine_opts = "vsmt=8";
 memory_size = "256M";
+start_address = PPC_TEST_MEM_START;
+end_address = PPC_TEST_MEM_END;
 arch_source = g_strdup_printf("-nodefaults "
   "-prom-env 'use-nvramrc?=true' -prom-env 
"
   "'nvramrc=hex .\" _\" begin %x %x "
   "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
   "until'", end_address, start_address);
 arch_target = g_strdup("");
-start_address = PPC_TEST_MEM_START;
-end_address = PPC_TEST_MEM_END;
 } else if (strcmp(arch, "aarch64") == 0) {
 init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel));
 machine_opts = "virt,gic-version=max";
-- 
2.24.1




[PULL 25/29] Bug #1829242 correction.

2020-01-14 Thread Juan Quintela
From: Alexey Romko 

Added type conversions to ram_addr_t before all left shifts of page
indexes to TARGET_PAGE_BITS, to correct overflows when the page
address was 4Gb and more.

Signed-off-by: Alexey Romko 
Reviewed-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 7221f54139..d0940387d1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1768,7 +1768,7 @@ static inline bool migration_bitmap_clear_dirty(RAMState 
*rs,
 if (rb->clear_bmap && clear_bmap_test_and_clear(rb, page)) {
 uint8_t shift = rb->clear_bmap_shift;
 hwaddr size = 1ULL << (TARGET_PAGE_BITS + shift);
-hwaddr start = (page << TARGET_PAGE_BITS) & (-size);
+hwaddr start = (((ram_addr_t)page) << TARGET_PAGE_BITS) & (-size);
 
 /*
  * CLEAR_BITMAP_SHIFT_MIN should always guarantee this... this
@@ -2005,7 +2005,7 @@ static void ram_release_pages(const char *rbname, 
uint64_t offset, int pages)
 return;
 }
 
-ram_discard_range(rbname, offset, pages << TARGET_PAGE_BITS);
+ram_discard_range(rbname, offset, ((ram_addr_t)pages) << TARGET_PAGE_BITS);
 }
 
 /*
@@ -2093,7 +2093,7 @@ static int ram_save_page(RAMState *rs, PageSearchStatus 
*pss, bool last_stage)
 uint8_t *p;
 bool send_async = true;
 RAMBlock *block = pss->block;
-ram_addr_t offset = pss->page << TARGET_PAGE_BITS;
+ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
 ram_addr_t current_addr = block->offset + offset;
 
 p = block->host + offset;
@@ -2280,7 +2280,8 @@ static bool find_dirty_block(RAMState *rs, 
PageSearchStatus *pss, bool *again)
 *again = false;
 return false;
 }
-if ((pss->page << TARGET_PAGE_BITS) >= pss->block->used_length) {
+if ram_addr_t)pss->page) << TARGET_PAGE_BITS)
+>= pss->block->used_length) {
 /* Didn't find anything in this RAM Block */
 pss->page = 0;
 pss->block = QLIST_NEXT_RCU(pss->block, next);
@@ -2571,7 +2572,7 @@ static int ram_save_target_page(RAMState *rs, 
PageSearchStatus *pss,
 bool last_stage)
 {
 RAMBlock *block = pss->block;
-ram_addr_t offset = pss->page << TARGET_PAGE_BITS;
+ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
 int res;
 
 if (control_save_page(rs, block, offset, &res)) {
@@ -2657,7 +2658,8 @@ static int ram_save_host_page(RAMState *rs, 
PageSearchStatus *pss,
 /* Allow rate limiting to happen in the middle of huge pages */
 migration_rate_limit();
 } while ((pss->page & (pagesize_bits - 1)) &&
- offset_in_ramblock(pss->block, pss->page << TARGET_PAGE_BITS));
+ offset_in_ramblock(pss->block,
+((ram_addr_t)pss->page) << TARGET_PAGE_BITS));
 
 /* The offset we leave with is the last one we looked at */
 pss->page--;
@@ -2874,8 +2876,10 @@ void ram_postcopy_migrated_memory_release(MigrationState 
*ms)
 
 while (run_start < range) {
 unsigned long run_end = find_next_bit(bitmap, range, run_start + 
1);
-ram_discard_range(block->idstr, run_start << TARGET_PAGE_BITS,
-  (run_end - run_start) << TARGET_PAGE_BITS);
+ram_discard_range(block->idstr,
+  ((ram_addr_t)run_start) << TARGET_PAGE_BITS,
+  ((ram_addr_t)(run_end - run_start))
+<< TARGET_PAGE_BITS);
 run_start = find_next_zero_bit(bitmap, range, run_end + 1);
 }
 }
@@ -4273,13 +4277,16 @@ static void colo_flush_ram_cache(void)
 while (block) {
 offset = migration_bitmap_find_dirty(ram_state, block, offset);
 
-if (offset << TARGET_PAGE_BITS >= block->used_length) {
+if (((ram_addr_t)offset) << TARGET_PAGE_BITS
+>= block->used_length) {
 offset = 0;
 block = QLIST_NEXT_RCU(block, next);
 } else {
 migration_bitmap_clear_dirty(ram_state, block, offset);
-dst_host = block->host + (offset << TARGET_PAGE_BITS);
-src_host = block->colo_cache + (offset << TARGET_PAGE_BITS);
+dst_host = block->host
+ + (((ram_addr_t)offset) << TARGET_PAGE_BITS);
+src_host = block->colo_cache
+ + (((ram_addr_t)offset) << TARGET_PAGE_BITS);
 memcpy(dst_host, src_host, TARGET_PAGE_SIZE);
 }
 }
-- 
2.24.1




[PULL 07/29] ram.c: remove unneeded labels

2020-01-14 Thread Juan Quintela
From: Daniel Henrique Barboza 

ram_save_queue_pages() has an 'err' label that can be replaced by
'return -1' instead.

Same thing with ram_discard_range(), and in this case we can also
get rid of the 'ret' variable and return either '-1' on error
or the result of ram_block_discard_range().

CC: Juan Quintela 
CC: Dr. David Alan Gilbert 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index f946282adb..17cd8d524b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2459,7 +2459,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
  * it's the 1st request.
  */
 error_report("ram_save_queue_pages no previous block");
-goto err;
+return -1;
 }
 } else {
 ramblock = qemu_ram_block_by_name(rbname);
@@ -2467,7 +2467,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
 if (!ramblock) {
 /* We shouldn't be asked for a non-existent RAMBlock */
 error_report("ram_save_queue_pages no block '%s'", rbname);
-goto err;
+return -1;
 }
 rs->last_req_rb = ramblock;
 }
@@ -2476,7 +2476,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
 error_report("%s request overrun start=" RAM_ADDR_FMT " len="
  RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
  __func__, start, len, ramblock->used_length);
-goto err;
+return -1;
 }
 
 struct RAMSrcPageRequest *new_entry =
@@ -2492,9 +2492,6 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
 qemu_mutex_unlock(&rs->src_page_req_mutex);
 
 return 0;
-
-err:
-return -1;
 }
 
 static bool save_page_use_compression(RAMState *rs)
@@ -3097,8 +3094,6 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms)
  */
 int ram_discard_range(const char *rbname, uint64_t start, size_t length)
 {
-int ret = -1;
-
 trace_ram_discard_range(rbname, start, length);
 
 RCU_READ_LOCK_GUARD();
@@ -3106,7 +3101,7 @@ int ram_discard_range(const char *rbname, uint64_t start, 
size_t length)
 
 if (!rb) {
 error_report("ram_discard_range: Failed to find block '%s'", rbname);
-goto err;
+return -1;
 }
 
 /*
@@ -3118,10 +3113,7 @@ int ram_discard_range(const char *rbname, uint64_t 
start, size_t length)
  length >> qemu_target_page_bits());
 }
 
-ret = ram_block_discard_range(rb, start, length);
-
-err:
-return ret;
+return ram_block_discard_range(rb, start, length);
 }
 
 /*
-- 
2.24.1




[PULL 10/29] migration: Fix the re-run check of the migrate-incoming command

2020-01-14 Thread Juan Quintela
From: Yury Kotov 

The current check sets an error but doesn't fail the command.
This may cause a problem if new connection attempt by the same URI
affects the first connection.

Signed-off-by: Yury Kotov 
Reviewed-by: Juan Quintela 
Reviewed-by: Darren Kenny 
Signed-off-by: Juan Quintela 
---
 migration/migration.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/migration.c b/migration/migration.c
index f79d0bf89a..e55edee606 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1784,6 +1784,7 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
 }
 if (!once) {
 error_setg(errp, "The incoming migration has already been started");
+return;
 }
 
 qemu_start_incoming_migration(uri, &local_err);
-- 
2.24.1




[PULL 14/29] migration/ram: Yield periodically to the main loop

2020-01-14 Thread Juan Quintela
From: Yury Kotov 

Usually, incoming migration coroutine yields to the main loop
while its IO-channel is waiting for data to receive. But there is a case
when RAM migration and data receive have the same speed: VM with huge
zeroed RAM. In this case, IO-channel won't read and thus the main loop
is stuck and for instance, it doesn't respond to QMP commands.

For this case, yield periodically, but not too often, so as not to
affect the speed of migration.

Signed-off-by: Yury Kotov 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 1ec5c10561..5cd066467c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4246,7 +4246,7 @@ static void colo_flush_ram_cache(void)
  */
 static int ram_load_precopy(QEMUFile *f)
 {
-int flags = 0, ret = 0, invalid_flags = 0, len = 0;
+int flags = 0, ret = 0, invalid_flags = 0, len = 0, i = 0;
 /* ADVISE is earlier, it shows the source has the postcopy capability on */
 bool postcopy_advised = postcopy_is_advised();
 if (!migrate_use_compression()) {
@@ -4258,6 +4258,17 @@ static int ram_load_precopy(QEMUFile *f)
 void *host = NULL;
 uint8_t ch;
 
+/*
+ * Yield periodically to let main loop run, but an iteration of
+ * the main loop is expensive, so do it each some iterations
+ */
+if ((i & 32767) == 0 && qemu_in_coroutine()) {
+aio_co_schedule(qemu_get_current_aio_context(),
+qemu_coroutine_self());
+qemu_coroutine_yield();
+}
+i++;
+
 addr = qemu_get_be64(f);
 flags = addr & ~TARGET_PAGE_MASK;
 addr &= TARGET_PAGE_MASK;
-- 
2.24.1




[PULL 28/29] apic: Use 32bit APIC ID for migration instance ID

2020-01-14 Thread Juan Quintela
From: Peter Xu 

Migration is silently broken now with x2apic config like this:

 -smp 200,maxcpus=288,sockets=2,cores=72,threads=2 \
 -device intel-iommu,intremap=on,eim=on

After migration, the guest kernel could hang at anything, due to
x2apic bit not migrated correctly in IA32_APIC_BASE on some vcpus, so
any operations related to x2apic could be broken then (e.g., RDMSR on
x2apic MSRs could fail because KVM would think that the vcpu hasn't
enabled x2apic at all).

The issue is that the x2apic bit was never applied correctly for vcpus
whose ID > 255 when migrate completes, and that's because when we
migrate APIC we use the APICCommonState.id as instance ID of the
migration stream, while that's too short for x2apic.

Let's use the newly introduced initial_apic_id for that.

Signed-off-by: Peter Xu 
Reviewed-by: Juan Quintela 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Juan Quintela 
---
 hw/intc/apic_common.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 54b8731fca..b5dbeb6206 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -268,7 +268,10 @@ static void apic_common_realize(DeviceState *dev, Error 
**errp)
 APICCommonState *s = APIC_COMMON(dev);
 APICCommonClass *info;
 static DeviceState *vapic;
-uint32_t instance_id = s->id;
+uint32_t instance_id = s->initial_apic_id;
+
+/* Normally initial APIC ID should be no more than hundreds */
+assert(instance_id != VMSTATE_INSTANCE_ID_ANY);
 
 info = APIC_COMMON_GET_CLASS(s);
 info->realize(dev, errp);
-- 
2.24.1




[PULL 11/29] misc: use QEMU_IS_ALIGNED

2020-01-14 Thread Juan Quintela
From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Reviewed-by: Juan Quintela 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Berger 
Reviewed-by: Paolo Bonzini 
Signed-off-by: Juan Quintela 
---
 exec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index d4b769d0d4..1feda49ca1 100644
--- a/exec.c
+++ b/exec.c
@@ -3895,7 +3895,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, 
size_t length)
 
 uint8_t *host_startaddr = rb->host + start;
 
-if ((uintptr_t)host_startaddr & (rb->page_size - 1)) {
+if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
 error_report("ram_block_discard_range: Unaligned start address: %p",
  host_startaddr);
 goto err;
@@ -3903,7 +3903,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, 
size_t length)
 
 if ((start + length) <= rb->used_length) {
 bool need_madvise, need_fallocate;
-if (length & (rb->page_size - 1)) {
+if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
 error_report("ram_block_discard_range: Unaligned length: %zx",
  length);
 goto err;
-- 
2.24.1




[PULL 12/29] migration: add savevm_state_handler_remove()

2020-01-14 Thread Juan Quintela
From: Scott Cheloha 

Create a function to abstract common logic needed when removing a
SaveStateEntry element from the savevm_state.handlers queue.

For now we just remove the element.  Soon it will involve additional
cleanup.

Signed-off-by: Scott Cheloha 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/savevm.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 59efc1981d..30d980caa2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -725,6 +725,11 @@ static void savevm_state_handler_insert(SaveStateEntry 
*nse)
 }
 }
 
+static void savevm_state_handler_remove(SaveStateEntry *se)
+{
+QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
+}
+
 /* TODO: Individual devices generally have very little idea about the rest
of the system, so instance_id should be removed/replaced.
Meanwhile pass -1 as instance_id if you do not already have a clearly
@@ -777,7 +782,7 @@ void unregister_savevm(VMStateIf *obj, const char *idstr, 
void *opaque)
 
 QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
 if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
-QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
+savevm_state_handler_remove(se);
 g_free(se->compat);
 g_free(se);
 }
@@ -841,7 +846,7 @@ void vmstate_unregister(VMStateIf *obj, const 
VMStateDescription *vmsd,
 
 QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
 if (se->vmsd == vmsd && se->opaque == opaque) {
-QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
+savevm_state_handler_remove(se);
 g_free(se->compat);
 g_free(se);
 }
-- 
2.24.1




[PULL 17/29] migration/postcopy: count target page number to decide the place_needed

2020-01-14 Thread Juan Quintela
From: Wei Yang 

In postcopy, it requires to place whole host page instead of target
page.

Currently, it relies on the page offset to decide whether this is the
last target page. We also can count the target page number during the
iteration. When the number of target page equals
(host page size / target page size), this means it is the last target
page in the host page.

This is a preparation for non-ordered target page transmission.

Signed-off-by: Wei Yang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index c13b44b4d9..8ebaea255e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4052,6 +4052,7 @@ static int ram_load_postcopy(QEMUFile *f)
 void *postcopy_host_page = mis->postcopy_tmp_page;
 void *last_host = NULL;
 bool all_zero = false;
+int target_pages = 0;
 
 while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
 ram_addr_t addr;
@@ -4086,6 +4087,7 @@ static int ram_load_postcopy(QEMUFile *f)
 ret = -EINVAL;
 break;
 }
+target_pages++;
 matches_target_page_size = block->page_size == TARGET_PAGE_SIZE;
 /*
  * Postcopy requires that we place whole host pages atomically;
@@ -4117,8 +4119,10 @@ static int ram_load_postcopy(QEMUFile *f)
  * If it's the last part of a host page then we place the host
  * page
  */
-place_needed = (((uintptr_t)host + TARGET_PAGE_SIZE) &
- (block->page_size - 1)) == 0;
+if (target_pages == (block->page_size / TARGET_PAGE_SIZE)) {
+place_needed = true;
+target_pages = 0;
+}
 place_source = postcopy_host_page;
 }
 last_host = host;
-- 
2.24.1




[PULL 1/1] linux-aio: increasing MAX_EVENTS to a larger hardcoded value

2020-01-14 Thread Stefan Hajnoczi
From: Wangyong 

Since commit 6040aedddb5f474a9c2304b6a432a652d82b3d3c "virtio-blk:
make queue size configurable",if the user set the queue size to
more than 128 ,it will not take effect. That's because linux aio's
maximum outstanding requests at a time is always less than or equal
to 128.

This patch simply increase MAX_EVENTS to a larger hardcoded value of
1024 as a shortterm fix.

Signed-off-by: wangyong 
Message-id: faa5781afd354a96a0be152b288f6...@h3c.com
Message-Id: 
Signed-off-by: Stefan Hajnoczi 
---
 block/linux-aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/linux-aio.c b/block/linux-aio.c
index c7eca9a256..91204a25a2 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -26,7 +26,7 @@
  *  than this we will get EAGAIN from io_submit which is communicated to
  *  the guest as an I/O error.
  */
-#define MAX_EVENTS 128
+#define MAX_EVENTS 1024
 
 struct qemu_laiocb {
 Coroutine *co;
-- 
2.24.1




[PULL 16/29] migration/postcopy: wait for decompress thread in precopy

2020-01-14 Thread Juan Quintela
From: Wei Yang 

Compress is not supported with postcopy, it is safe to wait for
decompress thread just in precopy.

This is a preparation for later patch.

Signed-off-by: Wei Yang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index bdb0316892..c13b44b4d9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4421,6 +4421,7 @@ static int ram_load_precopy(QEMUFile *f)
 }
 }
 
+ret |= wait_for_decompress_done();
 return ret;
 }
 
@@ -4452,8 +4453,6 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 } else {
 ret = ram_load_precopy(f);
 }
-
-ret |= wait_for_decompress_done();
 }
 trace_ram_load_complete(ret, seq_iter);
 
-- 
2.24.1




[PULL 18/29] migration/postcopy: set all_zero to true on the first target page

2020-01-14 Thread Juan Quintela
From: Wei Yang 

For the first target page, all_zero is set to true for this round check.

After target_pages introduced, we could leverage this variable instead
of checking the address offset.

Signed-off-by: Wei Yang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 8ebaea255e..460abfa2c3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4102,7 +4102,7 @@ static int ram_load_postcopy(QEMUFile *f)
 page_buffer = postcopy_host_page +
   ((uintptr_t)host & (block->page_size - 1));
 /* If all TP are zero then we can optimise the place */
-if (!((uintptr_t)host & (block->page_size - 1))) {
+if (target_pages == 1) {
 all_zero = true;
 } else {
 /* not the 1st TP within the HP */
-- 
2.24.1




Re: [PULL 0/3] capstone update

2020-01-14 Thread Peter Maydell
On Mon, 13 Jan 2020 at 19:53, Richard Henderson
 wrote:
>
> On 1/10/20 6:07 AM, Peter Maydell wrote:
> > On Wed, 8 Jan 2020 at 04:23, Richard Henderson
> >  wrote:
> >>
> >> The following changes since commit 
> >> 035eed4c0d257c905a556fa0f4865a0c077b4e7f:
> >>
> >>   Merge remote-tracking branch 
> >> 'remotes/vivier/tags/q800-for-5.0-pull-request' into staging (2020-01-07 
> >> 17:08:21 +)
> >>
> >> are available in the Git repository at:
> >>
> >>   https://github.com/rth7680/qemu.git tags/pull-cap-20200108
> >>
> >> for you to fetch changes up to 7cc3836eac04a3e358b2496fbca704b3ee5197ae:
> >>
> >>   capstone: Add skipdata hook for s390x (2020-01-08 14:53:54 +1100)
> >>
> >> 
> >> Update capstone to next
> >>
> >> 
> >> Richard Henderson (3):
> >>   capstone: Update to next
> >>   capstone: Enable disassembly for s390x
> >>   capstone: Add skipdata hook for s390x
> >
> > Build failures:
> >
> >   CC  aarch64-linux-user/disas.o
> > In file included from
> > /home/ubuntu/qemu/capstone/include/capstone/capstone.h:302:0,
> >  from /home/ubuntu/qemu/include/disas/capstone.h:6,
> >  from /home/ubuntu/qemu/disas.c:9:
> > /home/ubuntu/qemu/capstone/include/capstone/riscv.h:16:10: fatal
> > error: capstone/platform.h: No such file or directory
> >  #include "capstone/platform.h"
> >   ^
> > compilation terminated.
> >
> > (same on most hosts)
>
> This comes from not re-running configure, which changes the CFLAGS for the
> build of capstone from git.  Given that the source tree for capstone got
> rearranged between 3.x and 4.0, I don't see how I can avoid this.

Hmm, shouldn't the update to 'configure' in this merge cause Make
to rerun configure, though ?

thanks
-- PMM



RE: [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots

2020-01-14 Thread Pavel Dovgalyuk
> From: Alex Bennée [mailto:alex.ben...@linaro.org]
> Kevin Wolf  writes:
> 
> > Am 23.12.2019 um 10:47 hat Pavel Dovgalyuk geschrieben:
> >> From: Pavel Dovgalyuk 
> >>
> >> Saving icount as a parameters of the snapshot allows navigation between
> >> them in the execution replay scenario.
> >> This information can be used for finding a specific snapshot for proceeding
> >> the recorded execution to the specific moment of the time.
> >> E.g., 'reverse step' action (introduced in one of the following patches)
> >> needs to load the nearest snapshot which is prior to the current moment
> >> of time.
> >>
> >> Signed-off-by: Pavel Dovgalyuk 
> >> Acked-by: Markus Armbruster 
> >
> > Acked-by: Kevin Wolf 
> 
> Apologies my mailer ignored my replay-all:
> 
> This commit breaks when of the iotests for me:
> 
>  git bisect run /bin/sh -c "cd builds/all && make -j4 \
>  && cd tests/qemu-iotests && ./check -qcow2 267"
> 
> 
>List of snapshots present on all disks:
>   -IDTAG VM SIZEDATE   VM CLOCK
>   +IDTAG   VM SIZEDATE VM CLOCK 
> ICOUNT
> 
> But I've also seen:
> 
>   ERROR:/home/.../qemu.git/replay/replay-events.c:80:replay_flush_events:
>  assertion failed: (replay_mutex_locked())

Thank you, I've updated the code.
I also added a patch for fixing the test output.

Pavel Dovgalyuk




[PULL 15/29] migration/postcopy: reduce memset when it is zero page and matches_target_page_size

2020-01-14 Thread Juan Quintela
From: Wei Yang 

In this case, page_buffer content would not be used.

Skip this to save some time.

Signed-off-by: Wei Yang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 5cd066467c..bdb0316892 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4126,7 +4126,13 @@ static int ram_load_postcopy(QEMUFile *f)
 switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
 case RAM_SAVE_FLAG_ZERO:
 ch = qemu_get_byte(f);
-memset(page_buffer, ch, TARGET_PAGE_SIZE);
+/*
+ * Can skip to set page_buffer when
+ * this is a zero page and (block->page_size == TARGET_PAGE_SIZE).
+ */
+if (ch || !matches_target_page_size) {
+memset(page_buffer, ch, TARGET_PAGE_SIZE);
+}
 if (ch) {
 all_zero = false;
 }
-- 
2.24.1




[PULL 22/29] migration/multifd: not use multifd during postcopy

2020-01-14 Thread Juan Quintela
From: Wei Yang 

We don't support multifd during postcopy, but user still could enable
both multifd and postcopy. This leads to migration failure.

Skip multifd during postcopy.

Signed-off-by: Wei Yang 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index a05448c0c9..d4f33a4f2f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2587,10 +2587,13 @@ static int ram_save_target_page(RAMState *rs, 
PageSearchStatus *pss,
 }
 
 /*
- * do not use multifd for compression as the first page in the new
- * block should be posted out before sending the compressed page
+ * Do not use multifd for:
+ * 1. Compression as the first page in the new block should be posted out
+ *before sending the compressed page
+ * 2. In postcopy as one whole host page should be placed
  */
-if (!save_page_use_compression(rs) && migrate_use_multifd()) {
+if (!save_page_use_compression(rs) && migrate_use_multifd()
+&& !migration_in_postcopy()) {
 return ram_save_multifd_page(rs, block, offset);
 }
 
-- 
2.24.1




[PULL 20/29] migration/postcopy: enable compress during postcopy

2020-01-14 Thread Juan Quintela
From: Wei Yang 

postcopy requires to place a whole host page, while migration thread
migrate memory in target page size. This makes postcopy need to collect
all target pages in one host page before placing via userfaultfd.

To enable compress during postcopy, there are two problems to solve:

1. Random order for target page arrival
2. Target pages in one host page arrives without interrupt by target
   page from other host page

The first one is handled by previous cleanup patch.

This patch handles the second one by:

1. Flush compress thread for each host page
2. Wait for decompress thread for before placing host page

Signed-off-by: Wei Yang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/migration.c | 11 ---
 migration/ram.c   | 28 +++-
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index e55edee606..990bff00c0 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1005,17 +1005,6 @@ static bool migrate_caps_check(bool *cap_list,
 #endif
 
 if (cap_list[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
-if (cap_list[MIGRATION_CAPABILITY_COMPRESS]) {
-/* The decompression threads asynchronously write into RAM
- * rather than use the atomic copies needed to avoid
- * userfaulting.  It should be possible to fix the decompression
- * threads for compatibility in future.
- */
-error_setg(errp, "Postcopy is not currently compatible "
-   "with compression");
-return false;
-}
-
 /* This check is reasonably expensive, so only when it's being
  * set the first time, also it's only the destination that needs
  * special support.
diff --git a/migration/ram.c b/migration/ram.c
index a7414170e5..5f20c3d15d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3469,6 +3469,14 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 
 rs->target_page_count += pages;
 
+/*
+ * During postcopy, it is necessary to make sure one whole host
+ * page is sent in one chunk.
+ */
+if (migrate_postcopy_ram()) {
+flush_compressed_data(rs);
+}
+
 /*
  * we want to check in the 1st loop, just in case it was the 1st
  * time and we had to sync the dirty bitmap.
@@ -4061,6 +4069,7 @@ static int ram_load_postcopy(QEMUFile *f)
 void *place_source = NULL;
 RAMBlock *block = NULL;
 uint8_t ch;
+int len;
 
 addr = qemu_get_be64(f);
 
@@ -4078,7 +4087,8 @@ static int ram_load_postcopy(QEMUFile *f)
 
 trace_ram_load_postcopy_loop((uint64_t)addr, flags);
 place_needed = false;
-if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE)) {
+if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
+ RAM_SAVE_FLAG_COMPRESS_PAGE)) {
 block = ram_block_from_stream(f, flags);
 
 host = host_from_ram_block_offset(block, addr);
@@ -4161,6 +4171,17 @@ static int ram_load_postcopy(QEMUFile *f)
  TARGET_PAGE_SIZE);
 }
 break;
+case RAM_SAVE_FLAG_COMPRESS_PAGE:
+all_zero = false;
+len = qemu_get_be32(f);
+if (len < 0 || len > compressBound(TARGET_PAGE_SIZE)) {
+error_report("Invalid compressed data length: %d", len);
+ret = -EINVAL;
+break;
+}
+decompress_data_with_multi_threads(f, page_buffer, len);
+break;
+
 case RAM_SAVE_FLAG_EOS:
 /* normal exit */
 multifd_recv_sync_main();
@@ -4172,6 +4193,11 @@ static int ram_load_postcopy(QEMUFile *f)
 break;
 }
 
+/* Got the whole host page, wait for decompress before placing. */
+if (place_needed) {
+ret |= wait_for_decompress_done();
+}
+
 /* Detect for any possible file errors */
 if (!ret && qemu_file_get_error(f)) {
 ret = qemu_file_get_error(f);
-- 
2.24.1




Re: [PATCH] qemu-nbd: Convert invocation documentation to rST

2020-01-14 Thread Peter Maydell
On Mon, 13 Jan 2020 at 19:58, Eric Blake  wrote:
>
> On 1/13/20 12:08 PM, Peter Maydell wrote:
> > The qemu-nbd documentation is currently in qemu-nbd.texi in Texinfo
> > format, which we present to the user as:
> >   * a qemu-nbd manpage
> >   * a section of the main qemu-doc HTML documentation
> >
> > Convert the documentation to rST format, and present it to the user as:
> >   * a qemu-nbd manpage
> >   * part of the interop/ Sphinx manual
> >
> > This follows the same pattern as commit 27a296fce982 did for the
> > qemu-ga manpage.
> >
>
> I'm not an rST expert, but trust that you compared the resulting
> renderings.  Is there a quick recipe for doing the build locally so I
> can easily inspect local artifacts myself?

Yep -- assuming you have the prerequisites installed to
build the docs (should be just makeinfo, pod2man, sphinx),
apply the patch and then 'make' will build the docs -- the new
qemu-nbd.8 should be in docs/interop in the build tree,
assuming you do builds not in the source tree. If you do
builds in the source tree then the built artefacts
are created under docs/built. The html that forms part of
the interop manpage set is in docs/interop/qemu-nbd.html,
also in the build tree.

> > +++ b/Makefile
> > @@ -338,7 +338,7 @@ MANUAL_BUILDDIR := docs
> >   endif
> >
> >   ifdef BUILD_DOCS
> > -DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 
> > $(MANUAL_BUILDDIR)/interop/qemu-ga.8
> > +DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 
> > $(MANUAL_BUILDDIR)/interop/qemu-nbd.8 $(MANUAL_BUILDDIR)/interop/qemu-ga.8
>
> Worth splitting this long line, either with \-newline, or
>
> >   DOCS+=docs/interop/qemu-qmp-ref.html docs/interop/qemu-qmp-ref.txt 
> > docs/interop/qemu-qmp-ref.7
> >   DOCS+=docs/interop/qemu-ga-ref.html docs/interop/qemu-ga-ref.txt 
> > docs/interop/qemu-ga-ref.7
>
> with additional DOCS+= lines?

I think I prefer the latter.

> > +++ b/MAINTAINERS
> > @@ -2503,6 +2503,7 @@ F: include/block/nbd*
> >   F: qemu-nbd.*
> >   F: blockdev-nbd.c
> >   F: docs/interop/nbd.txt
> > +F: docs/interop/qemu-nbd.rst
> >   T: git https://repo.or.cz/qemu/ericb.git nbd
>
> Would I be taking this patch through my NBD tree, or would you be
> bundling it with other doc patches?

Either way would work for me, depends a bit whether I
get round to trying to do another doc conversion
and whether you had any other pending updates to the
old texinfo doc.

> > +++ b/docs/interop/qemu-nbd.rst
> > @@ -0,0 +1,263 @@
>
> > +.. option:: -s, --snapshot
> > +
> > +  Use *filename* as an external snapshot, create a temporary
> > +  file with ``backing_file=``\ *filename*, redirect the write to
> > +  the temporary one.
>
> Pre-existing poor grammar. Better might be:
>
> Use *filename* as an external snapshot, by creating a temporary file
> with ``backing_file=``\ *filename*, and redirecting writes to the
> temporary file.
>
> But that could also be done as a separate patch, to keep this one as
> mechanical as possible on the conversion.

Yes. I noticed a few things I would consider docs nits
which I deliberately didn't fix up in this conversion patch;
I'd prefer those to be done separately afterwards.

thanks
-- PMM



[PULL 19/29] migration/postcopy: enable random order target page arrival

2020-01-14 Thread Juan Quintela
From: Wei Yang 

After using number of target page received to track one host page, we
could have the capability to handle random order target page arrival in
one host page.

This is a preparation for enabling compress during postcopy.

Signed-off-by: Wei Yang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 460abfa2c3..a7414170e5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4050,7 +4050,7 @@ static int ram_load_postcopy(QEMUFile *f)
 MigrationIncomingState *mis = migration_incoming_get_current();
 /* Temporary page that is later 'placed' */
 void *postcopy_host_page = mis->postcopy_tmp_page;
-void *last_host = NULL;
+void *this_host = NULL;
 bool all_zero = false;
 int target_pages = 0;
 
@@ -4097,24 +4097,26 @@ static int ram_load_postcopy(QEMUFile *f)
  * that's moved into place later.
  * The migration protocol uses,  possibly smaller, target-pages
  * however the source ensures it always sends all the components
- * of a host page in order.
+ * of a host page in one chunk.
  */
 page_buffer = postcopy_host_page +
   ((uintptr_t)host & (block->page_size - 1));
 /* If all TP are zero then we can optimise the place */
 if (target_pages == 1) {
 all_zero = true;
+this_host = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
+block->page_size);
 } else {
 /* not the 1st TP within the HP */
-if (host != (last_host + TARGET_PAGE_SIZE)) {
-error_report("Non-sequential target page %p/%p",
-  host, last_host);
+if (QEMU_ALIGN_DOWN((uintptr_t)host, block->page_size) !=
+(uintptr_t)this_host) {
+error_report("Non-same host page %p/%p",
+  host, this_host);
 ret = -EINVAL;
 break;
 }
 }
 
-
 /*
  * If it's the last part of a host page then we place the host
  * page
@@ -4125,7 +4127,6 @@ static int ram_load_postcopy(QEMUFile *f)
 }
 place_source = postcopy_host_page;
 }
-last_host = host;
 
 switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
 case RAM_SAVE_FLAG_ZERO:
@@ -4178,7 +4179,8 @@ static int ram_load_postcopy(QEMUFile *f)
 
 if (!ret && place_needed) {
 /* This gets called at the last target page in the host page */
-void *place_dest = host + TARGET_PAGE_SIZE - block->page_size;
+void *place_dest = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
+   block->page_size);
 
 if (all_zero) {
 ret = postcopy_place_page_zero(mis, place_dest,
-- 
2.24.1




[PULL 27/29] migration: Change SaveStateEntry.instance_id into uint32_t

2020-01-14 Thread Juan Quintela
From: Peter Xu 

It was always used as 32bit, so define it as used to be clear.
Instead of using -1 as the auto-gen magic value, we switch to
UINT32_MAX.  We also make sure that we don't auto-gen this value to
avoid overflowed instance IDs without being noticed.

Suggested-by: Juan Quintela 
Signed-off-by: Peter Xu 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 hw/intc/apic_common.c|  2 +-
 include/migration/register.h |  2 +-
 include/migration/vmstate.h  |  2 +-
 migration/savevm.c   | 18 ++
 stubs/vmstate.c  |  2 +-
 5 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index f2c3a7f309..54b8731fca 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -268,7 +268,7 @@ static void apic_common_realize(DeviceState *dev, Error 
**errp)
 APICCommonState *s = APIC_COMMON(dev);
 APICCommonClass *info;
 static DeviceState *vapic;
-int instance_id = s->id;
+uint32_t instance_id = s->id;
 
 info = APIC_COMMON_GET_CLASS(s);
 info->realize(dev, errp);
diff --git a/include/migration/register.h b/include/migration/register.h
index 00c38ebe9f..c1dcff0f90 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -71,7 +71,7 @@ typedef struct SaveVMHandlers {
 } SaveVMHandlers;
 
 int register_savevm_live(const char *idstr,
- int instance_id,
+ uint32_t instance_id,
  int version_id,
  const SaveVMHandlers *ops,
  void *opaque);
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index ed74dd5624..01790b8d9b 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1160,7 +1160,7 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, 
void *opaque);
 #define  VMSTATE_INSTANCE_ID_ANY  -1
 
 /* Returns: 0 on success, -1 on failure */
-int vmstate_register_with_alias_id(VMStateIf *obj, int instance_id,
+int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
const VMStateDescription *vmsd,
void *base, int alias_id,
int required_for_version,
diff --git a/migration/savevm.c b/migration/savevm.c
index 8dab99efc4..adfdca26ac 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -233,7 +233,7 @@ typedef struct CompatEntry {
 typedef struct SaveStateEntry {
 QTAILQ_ENTRY(SaveStateEntry) entry;
 char idstr[256];
-int instance_id;
+uint32_t instance_id;
 int alias_id;
 int version_id;
 /* version id read from the stream */
@@ -667,10 +667,10 @@ void dump_vmstate_json_to_file(FILE *out_file)
 fclose(out_file);
 }
 
-static int calculate_new_instance_id(const char *idstr)
+static uint32_t calculate_new_instance_id(const char *idstr)
 {
 SaveStateEntry *se;
-int instance_id = 0;
+uint32_t instance_id = 0;
 
 QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
 if (strcmp(idstr, se->idstr) == 0
@@ -678,6 +678,8 @@ static int calculate_new_instance_id(const char *idstr)
 instance_id = se->instance_id + 1;
 }
 }
+/* Make sure we never loop over without being noticed */
+assert(instance_id != VMSTATE_INSTANCE_ID_ANY);
 return instance_id;
 }
 
@@ -755,7 +757,7 @@ static void savevm_state_handler_remove(SaveStateEntry *se)
Meanwhile pass -1 as instance_id if you do not already have a clearly
distinguishing id for all instances of your device class. */
 int register_savevm_live(const char *idstr,
- int instance_id,
+ uint32_t instance_id,
  int version_id,
  const SaveVMHandlers *ops,
  void *opaque)
@@ -809,7 +811,7 @@ void unregister_savevm(VMStateIf *obj, const char *idstr, 
void *opaque)
 }
 }
 
-int vmstate_register_with_alias_id(VMStateIf *obj, int instance_id,
+int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
const VMStateDescription *vmsd,
void *opaque, int alias_id,
int required_for_version,
@@ -1625,7 +1627,7 @@ int qemu_save_device_state(QEMUFile *f)
 return qemu_file_get_error(f);
 }
 
-static SaveStateEntry *find_se(const char *idstr, int instance_id)
+static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id)
 {
 SaveStateEntry *se;
 
@@ -2292,7 +2294,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, 
MigrationIncomingState *mis)
 /* Find savevm section */
 se = find_se(idstr, instance_id);
 if (se == NULL) {
-error_report("Unknown savevm section or instance '%s' %d. "
+error_report("Unknown savevm section or instance '%s' %"PRIu32". "
  "Make

[PULL 23/29] migration/multifd: fix nullptr access in terminating multifd threads

2020-01-14 Thread Juan Quintela
From: Jiahui Cen 

One multifd channel will shutdown all the other multifd's IOChannel when it
fails to receive an IOChannel. In this senario, if some multifds had not
received its IOChannel yet, it would try to shutdown its IOChannel which could
cause nullptr access at qio_channel_shutdown.

Here is the coredump stack:
#0  object_get_class (obj=obj@entry=0x0) at qom/object.c:908
#1  0x5563fdbb8f4a in qio_channel_shutdown (ioc=0x0, 
how=QIO_CHANNEL_SHUTDOWN_BOTH, errp=0x0) at io/channel.c:355
#2  0x5563fd7b4c5f in multifd_recv_terminate_threads (err=) at migration/ram.c:1280
#3  0x5563fd7bc019 in multifd_recv_new_channel 
(ioc=ioc@entry=0x556400255610, errp=errp@entry=0x7ffec07dce00) at 
migration/ram.c:1478
#4  0x5563fda82177 in migration_ioc_process_incoming 
(ioc=ioc@entry=0x556400255610, errp=errp@entry=0x7ffec07dce30) at 
migration/migration.c:605
#5  0x5563fda8567d in migration_channel_process_incoming 
(ioc=0x556400255610) at migration/channel.c:44
#6  0x5563fda83ee0 in socket_accept_incoming_migration 
(listener=0x5563fff6b920, cioc=0x556400255610, opaque=) at 
migration/socket.c:166
#7  0x5563fdbc25cd in qio_net_listener_channel_func (ioc=, condition=, opaque=) at io/net-listener.c:54
#8  0x7f895b6fe9a9 in g_main_context_dispatch () from 
/usr/lib64/libglib-2.0.so.0
#9  0x5563fdc18136 in glib_pollfds_poll () at util/main-loop.c:218
#10 0x5563fdc181b5 in os_host_main_loop_wait (timeout=10) at 
util/main-loop.c:241
#11 0x5563fdc183a2 in main_loop_wait (nonblocking=nonblocking@entry=0) 
at util/main-loop.c:517
#12 0x5563fd8edb37 in main_loop () at vl.c:1791
#13 0x5563fd74fd45 in main (argc=, argv=, 
envp=) at vl.c:4473

To fix it up, let's check p->c before calling qio_channel_shutdown.

Signed-off-by: Jiahui Cen 
Signed-off-by: Ying Fang 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index d4f33a4f2f..278b2ff87a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1308,7 +1308,9 @@ static void multifd_recv_terminate_threads(Error *err)
- normal quit, i.e. everything went fine, just finished
- error quit: We close the channels so the channel threads
  finish the qio_channel_read_all_eof() */
-qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+if (p->c) {
+qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
 qemu_mutex_unlock(&p->mutex);
 }
 }
-- 
2.24.1




[PULL 24/29] migration/multifd: fix destroyed mutex access in terminating multifd threads

2020-01-14 Thread Juan Quintela
From: Jiahui Cen 

One multifd will lock all the other multifds' IOChannel mutex to inform them
to quit by setting p->quit or shutting down p->c. In this senario, if some
multifds had already been terminated and 
multifd_load_cleanup/multifd_save_cleanup
had destroyed their mutex, it could cause destroyed mutex access when trying
lock their mutex.

Here is the coredump stack:
#0  0x7f81a2794437 in raise () from /usr/lib64/libc.so.6
#1  0x7f81a2795b28 in abort () from /usr/lib64/libc.so.6
#2  0x7f81a278d1b6 in __assert_fail_base () from /usr/lib64/libc.so.6
#3  0x7f81a278d262 in __assert_fail () from /usr/lib64/libc.so.6
#4  0x55eb1bfadbd3 in qemu_mutex_lock_impl (mutex=0x55eb1e2d1988, 
file=, line=) at util/qemu-thread-posix.c:64
#5  0x55eb1bb4564a in multifd_send_terminate_threads (err=) at migration/ram.c:1015
#6  0x55eb1bb4bb7f in multifd_send_thread (opaque=0x55eb1e2d19f8) at 
migration/ram.c:1171
#7  0x55eb1bfad628 in qemu_thread_start (args=0x55eb1e170450) at 
util/qemu-thread-posix.c:502
#8  0x7f81a2b36df5 in start_thread () from /usr/lib64/libpthread.so.0
#9  0x7f81a286048d in clone () from /usr/lib64/libc.so.6

To fix it up, let's destroy the mutex after all the other multifd threads had
been terminated.

Signed-off-by: Jiahui Cen 
Signed-off-by: Ying Fang 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 278b2ff87a..7221f54139 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1053,6 +1053,10 @@ void multifd_save_cleanup(void)
 if (p->running) {
 qemu_thread_join(&p->thread);
 }
+}
+for (i = 0; i < migrate_multifd_channels(); i++) {
+MultiFDSendParams *p = &multifd_send_state->params[i];
+
 socket_send_channel_destroy(p->c);
 p->c = NULL;
 qemu_mutex_destroy(&p->mutex);
@@ -1336,6 +1340,10 @@ int multifd_load_cleanup(Error **errp)
 qemu_sem_post(&p->sem_sync);
 qemu_thread_join(&p->thread);
 }
+}
+for (i = 0; i < migrate_multifd_channels(); i++) {
+MultiFDRecvParams *p = &multifd_recv_state->params[i];
+
 object_unref(OBJECT(p->c));
 p->c = NULL;
 qemu_mutex_destroy(&p->mutex);
-- 
2.24.1




[PULL 21/29] migration/multifd: clean pages after filling packet

2020-01-14 Thread Juan Quintela
From: Wei Yang 

This is a preparation for the next patch:

not use multifd during postcopy.

Without enabling postcopy, everything looks good. While after enabling
postcopy, migration may fail even not use multifd during postcopy. The
reason is the pages is not properly cleared and *old* target page will
continue to be transferred.

After clean pages, migration succeeds.

Signed-off-by: Wei Yang 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 5f20c3d15d..a05448c0c9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -955,10 +955,10 @@ static int multifd_send_pages(RAMState *rs)
 }
 qemu_mutex_unlock(&p->mutex);
 }
-p->pages->used = 0;
+assert(!p->pages->used);
+assert(!p->pages->block);
 
 p->packet_num = multifd_send_state->packet_num++;
-p->pages->block = NULL;
 multifd_send_state->pages = p->pages;
 p->pages = pages;
 transferred = ((uint64_t) pages->used) * TARGET_PAGE_SIZE + p->packet_len;
@@ -1154,6 +1154,8 @@ static void *multifd_send_thread(void *opaque)
 p->flags = 0;
 p->num_packets++;
 p->num_pages += used;
+p->pages->used = 0;
+p->pages->block = NULL;
 qemu_mutex_unlock(&p->mutex);
 
 trace_multifd_send(p->id, packet_num, used, flags,
-- 
2.24.1




[PULL 29/29] migration: Support QLIST migration

2020-01-14 Thread Juan Quintela
From: Eric Auger 

Support QLIST migration using the same principle as QTAILQ:
94869d5c52 ("migration: migrate QTAILQ").

The VMSTATE_QLIST_V macro has the same proto as VMSTATE_QTAILQ_V.
The change mainly resides in QLIST RAW macros: QLIST_RAW_INSERT_HEAD
and QLIST_RAW_REVERSE.

Tests also are provided.

Signed-off-by: Eric Auger 
Reviewed-by: Peter Xu 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 include/migration/vmstate.h |  21 +
 include/qemu/queue.h|  39 +
 migration/trace-events  |   5 ++
 migration/vmstate-types.c   |  70 +++
 tests/test-vmstate.c| 170 
 5 files changed, 305 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 01790b8d9b..30667631bc 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -229,6 +229,7 @@ extern const VMStateInfo vmstate_info_tmp;
 extern const VMStateInfo vmstate_info_bitmap;
 extern const VMStateInfo vmstate_info_qtailq;
 extern const VMStateInfo vmstate_info_gtree;
+extern const VMStateInfo vmstate_info_qlist;
 
 #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
 /*
@@ -798,6 +799,26 @@ extern const VMStateInfo vmstate_info_gtree;
 .offset   = offsetof(_state, _field),  
\
 }
 
+/*
+ * For migrating a QLIST
+ * Target QLIST needs be properly initialized.
+ * _type: type of QLIST element
+ * _next: name of QLIST_ENTRY entry field in QLIST element
+ * _vmsd: VMSD for QLIST element
+ * size: size of QLIST element
+ * start: offset of QLIST_ENTRY in QTAILQ element
+ */
+#define VMSTATE_QLIST_V(_field, _state, _version, _vmsd, _type, _next)  \
+{\
+.name = (stringify(_field)), \
+.version_id   = (_version),  \
+.vmsd = &(_vmsd),\
+.size = sizeof(_type),   \
+.info = &vmstate_info_qlist, \
+.offset   = offsetof(_state, _field),\
+.start= offsetof(_type, _next),  \
+}
+
 /* _f : field name
_f_n : num of elements field_name
_n : num of elements
diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index 4764d93ea3..4d4554a7ce 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -501,4 +501,43 @@ union {
 \
 QTAILQ_RAW_TQH_CIRC(head)->tql_prev = QTAILQ_RAW_TQE_CIRC(elm, entry); 
 \
 } while (/*CONSTCOND*/0)
 
+#define QLIST_RAW_FIRST(head)  
\
+field_at_offset(head, 0, void *)
+
+#define QLIST_RAW_NEXT(elm, entry) 
\
+field_at_offset(elm, entry, void *)
+
+#define QLIST_RAW_PREVIOUS(elm, entry) 
\
+field_at_offset(elm, entry + sizeof(void *), void *)
+
+#define QLIST_RAW_FOREACH(elm, head, entry)
\
+for ((elm) = *QLIST_RAW_FIRST(head);   
\
+ (elm);
\
+ (elm) = *QLIST_RAW_NEXT(elm, entry))
+
+#define QLIST_RAW_INSERT_HEAD(head, elm, entry) do {   
\
+void *first = *QLIST_RAW_FIRST(head);  
\
+*QLIST_RAW_FIRST(head) = elm;  
\
+*QLIST_RAW_PREVIOUS(elm, entry) = QLIST_RAW_FIRST(head);   
\
+if (first) {   
\
+*QLIST_RAW_NEXT(elm, entry) = first;   
\
+*QLIST_RAW_PREVIOUS(first, entry) = QLIST_RAW_NEXT(elm, entry);
\
+} else {   
\
+*QLIST_RAW_NEXT(elm, entry) = NULL;
\
+}  
\
+} while (0)
+
+#define QLIST_RAW_REVERSE(head, elm, entry) do {   
\
+void *iter = *QLIST_RAW_FIRST(head), *prev = NULL, *next;  
\
+while (iter) { 
\
+next = *QLIST_RAW_NEXT(iter, entry);   
\
+*QLIST_RAW_PREVIOUS(iter, entry) = QLIST_RAW_NEXT(next, entry);
\
+*QLIST_RAW_NEXT(iter, entry) = prev;   
\
+prev = iter;   
\
+iter = next;   
\
+}   

[PULL 1/1] trace: update qemu-trace-stap to Python 3

2020-01-14 Thread Stefan Hajnoczi
qemu-trace-stap does not support Python 3 yet:

  $ scripts/qemu-trace-stap list path/to/qemu-system-x86_64
  Traceback (most recent call last):
File "scripts/qemu-trace-stap", line 175, in 
  main()
File "scripts/qemu-trace-stap", line 171, in main
  args.func(args)
File "scripts/qemu-trace-stap", line 118, in cmd_list
  print_probes(args.verbose, "*")
File "scripts/qemu-trace-stap", line 114, in print_probes
  if line.startswith(prefix):
  TypeError: startswith first arg must be bytes or a tuple of bytes, not str

Now that QEMU requires Python 3.5 or later we can switch to pure Python
3.  Use Popen()'s universal_newlines=True argument to treat stdout as
text instead of binary.

Fixes: 62dd1048c0bd ("trace: add ability to do simple printf logging via 
systemtap")
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1787395
Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Daniel P. Berrangé 
Message-id: 20200107112438.383958-1-stefa...@redhat.com
Message-Id: <20200107112438.383958-1-stefa...@redhat.com>
Signed-off-by: Stefan Hajnoczi 
---
 scripts/qemu-trace-stap | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qemu-trace-stap b/scripts/qemu-trace-stap
index 91d1051cdc..90527eb974 100755
--- a/scripts/qemu-trace-stap
+++ b/scripts/qemu-trace-stap
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
 # -*- python -*-
 #
 # Copyright (C) 2019 Red Hat, Inc
@@ -18,8 +18,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see .
 
-from __future__ import print_function
-
 import argparse
 import copy
 import os.path
@@ -104,7 +102,9 @@ def cmd_list(args):
 if verbose:
 print("Listing probes with name '%s'" % script)
 proc = subprocess.Popen(["stap", "-l", script],
-stdout=subprocess.PIPE, 
env=tapset_env(tapsets))
+stdout=subprocess.PIPE,
+universal_newlines=True,
+env=tapset_env(tapsets))
 out, err = proc.communicate()
 if proc.returncode != 0:
 print("No probes found, are the tapsets installed in %s" % 
tapset_dir(args.binary))
-- 
2.24.1




[PULL 0/1] Tracing patches

2020-01-14 Thread Stefan Hajnoczi
The following changes since commit dc65a5bdc9fa543690a775b50d4ffbeb22c56d6d:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.0-20200108' into 
staging (2020-01-10 16:15:04 +)

are available in the Git repository at:

  https://github.com/stefanha/qemu.git tags/tracing-pull-request

for you to fetch changes up to 3f0097169bb60268cc5dda0c5ea47c31ab57b22f:

  trace: update qemu-trace-stap to Python 3 (2020-01-13 16:42:20 +)


Pull request



Stefan Hajnoczi (1):
  trace: update qemu-trace-stap to Python 3

 scripts/qemu-trace-stap | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.24.1




[PULL 26/29] migration: Define VMSTATE_INSTANCE_ID_ANY

2020-01-14 Thread Juan Quintela
From: Peter Xu 

Define the new macro VMSTATE_INSTANCE_ID_ANY for callers who wants to
auto-generate the vmstate instance ID.  Previously it was hard coded
as -1 instead of this macro.  It helps to change this default value in
the follow up patches.  No functional change.

Signed-off-by: Peter Xu 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 backends/dbus-vmstate.c | 3 ++-
 hw/arm/stellaris.c  | 2 +-
 hw/core/qdev.c  | 3 ++-
 hw/display/ads7846.c| 2 +-
 hw/i2c/core.c   | 2 +-
 hw/input/stellaris_input.c  | 3 ++-
 hw/intc/apic_common.c   | 2 +-
 hw/misc/max111x.c   | 3 ++-
 hw/net/eepro100.c   | 3 ++-
 hw/pci/pci.c| 2 +-
 hw/ppc/spapr.c  | 2 +-
 hw/timer/arm_timer.c| 2 +-
 hw/tpm/tpm_emulator.c   | 3 ++-
 include/migration/vmstate.h | 2 ++
 migration/savevm.c  | 8 
 15 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
index 56b482a7d6..cc594a722e 100644
--- a/backends/dbus-vmstate.c
+++ b/backends/dbus-vmstate.c
@@ -412,7 +412,8 @@ dbus_vmstate_complete(UserCreatable *uc, Error **errp)
 return;
 }
 
-if (vmstate_register(VMSTATE_IF(self), -1, &dbus_vmstate, self) < 0) {
+if (vmstate_register(VMSTATE_IF(self), VMSTATE_INSTANCE_ID_ANY,
+ &dbus_vmstate, self) < 0) {
 error_setg(errp, "Failed to register vmstate");
 }
 }
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index b198066b54..bb025e0bd0 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -708,7 +708,7 @@ static int stellaris_sys_init(uint32_t base, qemu_irq irq,
 memory_region_init_io(&s->iomem, NULL, &ssys_ops, s, "ssys", 0x1000);
 memory_region_add_subregion(get_system_memory(), base, &s->iomem);
 ssys_reset(s);
-vmstate_register(NULL, -1, &vmstate_stellaris_sys, s);
+vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, &vmstate_stellaris_sys, s);
 return 0;
 }
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9f1753f5cf..58e87d336d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -879,7 +879,8 @@ static void device_set_realized(Object *obj, bool value, 
Error **errp)
 
 if (qdev_get_vmsd(dev)) {
 if (vmstate_register_with_alias_id(VMSTATE_IF(dev),
-   -1, qdev_get_vmsd(dev), dev,
+   VMSTATE_INSTANCE_ID_ANY,
+   qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
dev->alias_required_for_version,
&local_err) < 0) {
diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c
index c12272ae72..9228b40b1a 100644
--- a/hw/display/ads7846.c
+++ b/hw/display/ads7846.c
@@ -154,7 +154,7 @@ static void ads7846_realize(SSISlave *d, Error **errp)
 
 ads7846_int_update(s);
 
-vmstate_register(NULL, -1, &vmstate_ads7846, s);
+vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, &vmstate_ads7846, s);
 }
 
 static void ads7846_class_init(ObjectClass *klass, void *data)
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 92cd489069..d770035ba0 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -61,7 +61,7 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char *name)
 
 bus = I2C_BUS(qbus_create(TYPE_I2C_BUS, parent, name));
 QLIST_INIT(&bus->current_devs);
-vmstate_register(NULL, -1, &vmstate_i2c_bus, bus);
+vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, &vmstate_i2c_bus, bus);
 return bus;
 }
 
diff --git a/hw/input/stellaris_input.c b/hw/input/stellaris_input.c
index 59892b07fc..e6ee5e11f1 100644
--- a/hw/input/stellaris_input.c
+++ b/hw/input/stellaris_input.c
@@ -88,5 +88,6 @@ void stellaris_gamepad_init(int n, qemu_irq *irq, const int 
*keycode)
 }
 s->num_buttons = n;
 qemu_add_kbd_event_handler(stellaris_gamepad_put_key, s);
-vmstate_register(NULL, -1, &vmstate_stellaris_gamepad, s);
+vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY,
+ &vmstate_stellaris_gamepad, s);
 }
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 375cb6abe9..f2c3a7f309 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -284,7 +284,7 @@ static void apic_common_realize(DeviceState *dev, Error 
**errp)
 }
 
 if (s->legacy_instance_id) {
-instance_id = -1;
+instance_id = VMSTATE_INSTANCE_ID_ANY;
 }
 vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
s, -1, 0, NULL);
diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
index 211008ce02..2b87bdee5b 100644
--- a/hw/misc/max111x.c
+++ b/hw/misc/max111x.c
@@ -146,7 +146,8 @@ static int max111x_init(SSISlave *d, int inputs)
 s->input[7] = 0x80;
 s->com = 0;
 
-vmstate_register(VM

[PULL 0/1] Block patches

2020-01-14 Thread Stefan Hajnoczi
The following changes since commit dc65a5bdc9fa543690a775b50d4ffbeb22c56d6d:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.0-20200108' into 
staging (2020-01-10 16:15:04 +)

are available in the Git repository at:

  https://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 2558cb8dd4150512bc8ae6d505cdcd10d0cc46bb:

  linux-aio: increasing MAX_EVENTS to a larger hardcoded value (2020-01-13 
16:41:45 +)


Pull request



Wangyong (1):
  linux-aio: increasing MAX_EVENTS to a larger hardcoded value

 block/linux-aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.24.1




Re: Semihosting, arm, riscv, ppc and common code

2020-01-14 Thread Alex Bennée


Benjamin Herrenschmidt  writes:

> On Tue, 2020-01-14 at 09:32 +0200, Liviu Ionescu wrote:
>> > On 14 Jan 2020, at 08:25, Benjamin Herrenschmidt <
>> > b...@kernel.crashing.org> wrote:
>> > 
>> > I noticed that the bulk of arm-semi.c (or riscv-semi.c) is
>> > trivially
>> > made completely generic by doing a couple of changes:
>> 
>> Last year I did a similar exercise in OpenOCD, where I took the Arm
>> semihosting code from the Arm specific location, extracted the common
>> part, updated it for latest Arm 64-bit specs, and then used the
>> common code for both Arm and RISC-V.
>> 
>> If you think useful, you might take a look there too.
>
> Well, one of the LCA talks wasn't that interesting so I started doing
> it and am almost done :-)
>
> I'll look at doing something for arm, riscv and ppc and send patches
> once I get it working.

Cool. Are you considering linux-user as well or are these only things
that make sense for system emulation?

>
> Cheers,
> Ben.
>
>> 
>> Regards,
>> 
>> Liviu


-- 
Alex Bennée



[PATCH] migration: Maybe VM is paused when migration is cancelled

2020-01-14 Thread Zhimin Feng
If the migration is cancelled when it is in the completion phase,
the migration state is set to MIGRATION_STATUS_CANCELLING.
The VM maybe wait for the 'pause_sem' semaphore in migration_maybe_pause
function, so that VM always is paused.

Reported-by: Euler Robot 
Signed-off-by: Zhimin Feng 
---
 migration/migration.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 354ad07..82ee981 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2765,14 +2765,22 @@ static int migration_maybe_pause(MigrationState *s,
 /* This block intentionally left blank */
 }
 
-qemu_mutex_unlock_iothread();
-migrate_set_state(&s->state, *current_active_state,
-  MIGRATION_STATUS_PRE_SWITCHOVER);
-qemu_sem_wait(&s->pause_sem);
-migrate_set_state(&s->state, MIGRATION_STATUS_PRE_SWITCHOVER,
-  new_state);
-*current_active_state = new_state;
-qemu_mutex_lock_iothread();
+/*
+ * If the migration is cancelled when it is in the completion phase,
+ * the migration state is set to MIGRATION_STATUS_CANCELLING.
+ * So we don't need to wait a semaphore, otherwise we would always
+ * wait for the 'pause_sem' semaphore.
+ */
+if (s->state != MIGRATION_STATUS_CANCELLING) {
+qemu_mutex_unlock_iothread();
+migrate_set_state(&s->state, *current_active_state,
+  MIGRATION_STATUS_PRE_SWITCHOVER);
+qemu_sem_wait(&s->pause_sem);
+migrate_set_state(&s->state, MIGRATION_STATUS_PRE_SWITCHOVER,
+  new_state);
+*current_active_state = new_state;
+qemu_mutex_lock_iothread();
+}
 
 return s->state == new_state ? 0 : -EINVAL;
 }
-- 
1.8.3.1





[PULL 1/2] ui: Print available display backends with '-display help'

2020-01-14 Thread Gerd Hoffmann
From: Thomas Huth 

We already print availabled devices with "-device help", or available
backends with "-netdev help" or "-chardev help". Let's provide a way
for the users to query the available display backends, too.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
Message-id: 20200108144702.29969-1-th...@redhat.com
Signed-off-by: Gerd Hoffmann 
---
 include/ui/console.h |  1 +
 ui/console.c | 15 +++
 vl.c |  5 +
 qemu-options.hx  |  3 ++-
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 281f9c145b58..f35b4fc082b4 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -442,6 +442,7 @@ void qemu_display_register(QemuDisplay *ui);
 bool qemu_display_find_default(DisplayOptions *opts);
 void qemu_display_early_init(DisplayOptions *opts);
 void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
+void qemu_display_help(void);
 
 /* vnc.c */
 void vnc_display_init(const char *id, Error **errp);
diff --git a/ui/console.c b/ui/console.c
index ac79d679f576..69339b028bb2 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2333,6 +2333,21 @@ void qemu_display_init(DisplayState *ds, DisplayOptions 
*opts)
 dpys[opts->type]->init(ds, opts);
 }
 
+void qemu_display_help(void)
+{
+int idx;
+
+printf("Available display backend types:\n");
+for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
+if (!dpys[idx]) {
+ui_module_load_one(DisplayType_str(idx));
+}
+if (dpys[idx]) {
+printf("%s\n",  DisplayType_str(dpys[idx]->type));
+}
+}
+}
+
 void qemu_chr_parse_vc(QemuOpts *opts, ChardevBackend *backend, Error **errp)
 {
 int val;
diff --git a/vl.c b/vl.c
index 158a05ed321c..751401214c80 100644
--- a/vl.c
+++ b/vl.c
@@ -1869,6 +1869,11 @@ static void parse_display(const char *p)
 {
 const char *opts;
 
+if (is_help_option(p)) {
+qemu_display_help();
+exit(0);
+}
+
 if (strstart(p, "sdl", &opts)) {
 /*
  * sdl DisplayType needs hand-crafted parser instead of
diff --git a/qemu-options.hx b/qemu-options.hx
index d4b73ef60c1d..709162c159ad 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1669,7 +1669,8 @@ STEXI
 @item -display @var{type}
 @findex -display
 Select type of display to use. This option is a replacement for the
-old style -sdl/-curses/... options. Valid values for @var{type} are
+old style -sdl/-curses/... options. Use @code{-display help} to list
+the available display types. Valid values for @var{type} are
 @table @option
 @item sdl
 Display video output via SDL (usually in a separate graphics
-- 
2.18.1




Re: Semihosting, arm, riscv, ppc and common code

2020-01-14 Thread Peter Maydell
On Tue, 14 Jan 2020 at 06:29, Benjamin Herrenschmidt
 wrote:
>
> Hi Folks !
>
> So I started "porting" over (read: copying) the arm semihosting code to
> ppc to mimmic what Keith did for risv (mostly for picolibc support).
>
> I noticed that the bulk of arm-semi.c (or riscv-semi.c) is trivially
> made completely generic by doing a couple of changes:

Note that semihosting is not a "here's a handy QEMU feature"
thing. It's an architecture-specific API and ABI, which should
be defined somewhere in a standard external to QEMU.
You need to start by having a definition for PPC of what
semihosting is. If you're starting from scratch there, there
are some important things you should do differently to Arm --
there is no benefit to repeating the mistakes of API definition
that we made! Most notably, you want to specify and require
that any unrecognized semihosting call function fails in a
clean and detectable way; you also should have a semihosting
function for "ask for a feature bit mask" so you don't need
the silly magic-filename approach Arm had to go for. You
also want to standardize what the errno values are, which Arm
forgot to do and which makes the errno handling in the spec
pretty useless.

TLDR: don't start by writing code, start by writing the *API/ABI spec*.
I tried to push the RISCV folks in this direction as well.

thanks
-- PMM



Re: [PATCH v3 1/2] migration: Fix incorrect integer->float conversion caught by clang

2020-01-14 Thread Markus Armbruster
Markus Armbruster  writes:

> Juan Quintela  writes:
>
>> Markus Armbruster  wrote:
>>> From: Fangrui Song 
>>>
>>> Clang does not like qmp_migrate_set_downtime()'s code to clamp double
>>> @value to 0..INT64_MAX:
>>>
>>> qemu/migration/migration.c:2038:24: error: implicit conversion from 
>>> 'long' to 'double' changes value from 9223372036854775807 to 
>>> 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion]
>>>
>>> The warning will be enabled by default in clang 10. It is not
>>> available for clang <= 9.
>>>
>>> The clamp is actually useless; @value is checked to be within
>>> 0..MAX_MIGRATE_DOWNTIME_SECONDS immediately before.  Delete it.
>>>
>>> While there, make the conversion from double to int64_t explicit.
>>>
>>> Signed-off-by: Fangrui Song 
>>> Reviewed-by: Markus Armbruster 
>>> [Patch split, commit message improved]
>>> Signed-off-by: Markus Armbruster 
>>
>> Reviewed-by: Juan Quintela 
>>
>> Should I get this through migration tree, or are you going to pull it?
>
> Plase take this patch through the migration tree.

Ping...




Call for sponsors for Outreachy 2020 May-August

2020-01-14 Thread Stefan Hajnoczi
QEMU has participated in the Outreachy open source internship program
for the past years thanks to the generous sponsorship of companies
supporting our community.  This year QEMU will apply as an Outreachy
organization again so we can provide internships to anyone who faces
under-representation, systemic bias, or discrimination in the
technology industry.

Internships are 12 weeks long and consist of paid full-time remote
work.  An internship is paid for by $6,500 of donations.

QEMU relies on sponsors to fund these internships.  If you or your
organization would like to become a sponsor, please email
organiz...@outreachy.org and CC me.  You can find additional
information about sponsorship here:
https://www.outreachy.org/sponsor/

Thank you,
Stefan



[PULL 0/2] Ui 20200114 patches

2020-01-14 Thread Gerd Hoffmann
The following changes since commit 3c8a6575985b1652b45bfa670b5e1907d642cfa0:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20200113-pull-request' 
into staging (2020-01-13 14:19:57 +)

are available in the Git repository at:

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

for you to fetch changes up to c4c00922cc948bb5e879bfae60764eba1f8745f3:

  display/gtk: get proper refreshrate (2020-01-14 07:26:36 +0100)


ui: add "-display help", gtk refresh rate.



Nikola Pavlica (1):
  display/gtk: get proper refreshrate

Thomas Huth (1):
  ui: Print available display backends with '-display help'

 include/ui/console.h |  1 +
 include/ui/gtk.h |  2 ++
 ui/console.c | 15 +++
 ui/gtk.c | 11 +++
 vl.c |  5 +
 qemu-options.hx  |  3 ++-
 6 files changed, 36 insertions(+), 1 deletion(-)

-- 
2.18.1




Re: [Xen-devel] [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE

2020-01-14 Thread Roger Pau Monné
On Mon, Jan 13, 2020 at 02:01:47PM -0500, Jason Andryuk wrote:
> On Fri, Mar 22, 2019 at 3:43 PM Jason Andryuk  wrote:
> >
> > On Thu, Mar 21, 2019 at 11:09 PM Roger Pau Monné  
> > wrote:
> > >
> > > On Wed, Mar 20, 2019 at 01:28:47PM -0400, Jason Andryuk wrote:
> > > > On Fri, Mar 15, 2019 at 12:28 PM Andrew Cooper
> > > >  wrote:
> > > > >
> > > > > On 15/03/2019 09:17, Paul Durrant wrote:
> > > > > >> -Original Message-
> > > > > >> From: Jason Andryuk [mailto:jandr...@gmail.com]
> > > > > >> Sent: 14 March 2019 18:16
> > > > > >> To: Paul Durrant 
> > > > > >> Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; 
> > > > > >> marma...@invisiblethingslab.com; Simon
> > > > > >> Gaiser ; Stefano Stabellini 
> > > > > >> ; Anthony Perard
> > > > > >> 
> > > > > >> Subject: Re: [PATCH 6/6] xen-pt: Round pci regions sizes to 
> > > > > >> XEN_PAGE_SIZE
> > > > > >>
> > > > > >> On Wed, Mar 13, 2019 at 11:09 AM Paul Durrant 
> > > > > >>  wrote:
> > > > >  -Original Message-
> > > > >  From: Jason Andryuk [mailto:jandr...@gmail.com]
> > > > >  Sent: 11 March 2019 18:02
> > > > >  To: qemu-devel@nongnu.org
> > > > >  Cc: xen-de...@lists.xenproject.org; 
> > > > >  marma...@invisiblethingslab.com; Simon Gaiser
> > > > >  ; Jason Andryuk 
> > > > >  ; Stefano Stabellini
> > > > >  ; Anthony Perard 
> > > > >  ; Paul Durrant
> > > > >  
> > > > >  Subject: [PATCH 6/6] xen-pt: Round pci regions sizes to 
> > > > >  XEN_PAGE_SIZE
> > > > > 
> > > > >  From: Simon Gaiser 
> > > > > 
> > > > >  If a pci memory region has a size < XEN_PAGE_SIZE it can get 
> > > > >  located at
> > > > >  an address which is not page aligned.
> > > > > >>> IIRC the PCI spec says that the minimum memory region size should 
> > > > > >>> be at least 4k. Should we even be
> > > > > >> tolerating BARs smaller than that?
> > > > > >>>   Paul
> > > > > >>>
> > > > > >> Hi, Paul.
> > > > > >>
> > > > > >> Simon found this, so it affects a real device.  Simon, do you 
> > > > > >> recall
> > > > > >> which device was affected?
> > > > > >>
> > > > > >> I think BARs only need to be power-of-two size and aligned, and 4k 
> > > > > >> is
> > > > > >> not a minimum.  16bytes may be a minimum, but I don't know what the
> > > > > >> spec says.
> > > > > >>
> > > > > >> On an Ivy Bridge system, here are some of the devices with BARs 
> > > > > >> smaller than 4K:
> > > > > >> 00:16.0 Communication controller: Intel Corporation 7 Series/C210
> > > > > >> Series Chipset Family MEI Controller #1 (rev 04)
> > > > > >>Memory at d0735000 (64-bit, non-prefetchable) [disabled] 
> > > > > >> [size=16]
> > > > > >> 00:1d.0 USB controller: Intel Corporation 7 Series/C210 Series 
> > > > > >> Chipset
> > > > > >> Family USB Enhanced Host Controller #1 (rev 04) (prog-if 20 [EHCI])
> > > > > >>Memory at d0739000 (32-bit, non-prefetchable) [disabled] 
> > > > > >> [size=1K]
> > > > > >> 00:1f.3 SMBus: Intel Corporation 7 Series/C210 Series Chipset 
> > > > > >> Family
> > > > > >> SMBus Controller (rev 04)
> > > > > >>Memory at d0734000 (64-bit, non-prefetchable) [disabled] 
> > > > > >> [size=256]
> > > > > >> 02:00.0 System peripheral: JMicron Technology Corp. SD/MMC Host
> > > > > >> Controller (rev 30)
> > > > > >>Memory at d0503000 (32-bit, non-prefetchable) [disabled] 
> > > > > >> [size=256]
> > > > > >>
> > > > > >> These examples are all 4K aligned, so this is not an issue on this 
> > > > > >> machine.
> > > > > >>
> > > > > >> Reviewing the code, I'm now wondering if the following in
> > > > > >> hw/xen/xen_pt.c:xen_pt_region_update is wrong:rc =
> > > > > >> xc_domain_memory_mapping(xen_xc, xen_domid,
> > > > > >>  XEN_PFN(guest_addr + 
> > > > > >> XC_PAGE_SIZE - 1),
> > > > > >>  XEN_PFN(machine_addr + 
> > > > > >> XC_PAGE_SIZE - 1),
> > > > > >>  XEN_PFN(size + XC_PAGE_SIZE - 
> > > > > >> 1),
> > > > > >>  op);
> > > > > >>
> > > > > >> If a bar of size 0x100 is at 0xd0500800, then the machine_addr 
> > > > > >> passed
> > > > > >> in would be 0xd0501000 which is past the actual location.  Should 
> > > > > >> the
> > > > > >> call arguments just be XEN_PFN(guest_addr) & XEN_PFN(machine_addr)?
> > > > > >>
> > > > > >> BARs smaller than a page would also be a problem if BARs for 
> > > > > >> different
> > > > > >> devices shared the same page.
> > > > > > Exactly. We cannot pass them through with any degree of safety (not 
> > > > > > that passthrough of an arbitrary device is a particularly safe 
> > > > > > thing to do anyway). The xen-pt code would instead need to trap 
> > > > > > those BARs and perform the accesses to the real BAR itself. 
> > > > > > Ultimately though I think we should be retiring the xen-pt code in 
> > > > > > favour of a standalone emulator.
> > > > >
> > > 

[PULL 2/2] display/gtk: get proper refreshrate

2020-01-14 Thread Gerd Hoffmann
From: Nikola Pavlica 

Because some VMs in QEMU can get GPU virtualization (using technologies
such as iGVT-g, as mentioned previously), they could produce a video
output that had a higher display refresh rate than of what the GTK
display was displaying. (fxp. Playing a video game inside of a Windows
VM at 60 Hz, while the output stood locked at 33 Hz because of defaults
set in include/ui/console.h)

Since QEMU does indeed have internal systems for determining frame
times as defined in ui/console.c.
The code checks for a variable called update_interval that it later
uses for time calculation. This variable, however, isn't defined
anywhere in ui/gtk.c and instead ui/console.c just sets it to
GUI_REFRESH_INTERVAL_DEFAULT which is 30

update_interval represents the number of milliseconds per display
refresh, and by doing some math we get that 1000/30 = 33.33... Hz

This creates the mentioned problem and what this patch does is that it
checks for the display refresh rate reported by GTK itself (we can take
this as a safe value) and just converts it back to a number of
milliseconds per display refresh.

Signed-off-by: Nikola Pavlica 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20200108121342.29597-1-pavlica.nik...@gmail.com

[ kraxel: style tweak: add blank line between vars and code ]

Signed-off-by: Gerd Hoffmann 
---
 include/ui/gtk.h |  2 ++
 ui/gtk.c | 11 +++
 2 files changed, 13 insertions(+)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index d9eedad976ef..d1b230848a7b 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -28,6 +28,8 @@
 #include "ui/egl-context.h"
 #endif
 
+#define MILLISEC_PER_SEC 100
+
 typedef struct GtkDisplayState GtkDisplayState;
 
 typedef struct VirtualGfxConsole {
diff --git a/ui/gtk.c b/ui/gtk.c
index 692ccc7bbb90..7355d34fcffd 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1966,6 +1966,11 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, 
VirtualConsole *vc,
   GSList *group, GtkWidget *view_menu)
 {
 bool zoom_to_fit = false;
+int refresh_rate_millihz;
+
+GdkDisplay *dpy = gtk_widget_get_display(s->window);
+GdkWindow *win = gtk_widget_get_window(s->window);
+GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
 
 vc->label = qemu_console_get_label(con);
 vc->s = s;
@@ -2026,6 +2031,12 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, 
VirtualConsole *vc,
 
 vc->gfx.kbd = qkbd_state_init(con);
 vc->gfx.dcl.con = con;
+
+refresh_rate_millihz = gdk_monitor_get_refresh_rate(monitor);
+if (refresh_rate_millihz) {
+vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
+}
+
 register_displaychangelistener(&vc->gfx.dcl);
 
 gd_connect_vc_gfx_signals(vc);
-- 
2.18.1




Re: [PATCH 066/104] virtiofsd: passthrough_ll: add renameat2 support

2020-01-14 Thread Dr. David Alan Gilbert
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Mon, Jan 13, 2020 at 08:06:24PM +, Dr. David Alan Gilbert wrote:
> > * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> > > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > > On Thu, Dec 12, 2019 at 04:38:26PM +, Dr. David Alan Gilbert (git) 
> > > > wrote:
> > > > > From: Miklos Szeredi 
> > > > > 
> > > > > No glibc support yet, so use syscall().
> > > > 
> > > > It exists in glibc in my Fedora 31 install.
> > > > 
> > > > Presumably this is related to an older version
> > > > 
> > > > > Signed-off-by: Miklos Szeredi 
> > > > > ---
> > > > >  tools/virtiofsd/passthrough_ll.c | 10 ++
> > > > >  1 file changed, 10 insertions(+)
> > > > > 
> > > > > diff --git a/tools/virtiofsd/passthrough_ll.c 
> > > > > b/tools/virtiofsd/passthrough_ll.c
> > > > > index 91d3120033..bed2270141 100644
> > > > > --- a/tools/virtiofsd/passthrough_ll.c
> > > > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > > > @@ -1083,7 +1083,17 @@ static void lo_rename(fuse_req_t req, 
> > > > > fuse_ino_t parent, const char *name,
> > > > >  }
> > > > >  
> > > > >  if (flags) {
> > > > > +#ifndef SYS_renameat2
> > > > >  fuse_reply_err(req, EINVAL);
> > > > > +#else
> > > > > +res = syscall(SYS_renameat2, lo_fd(req, parent), name,
> > > > > +  lo_fd(req, newparent), newname, flags);
> > > > > +if (res == -1 && errno == ENOSYS) {
> > > > > +fuse_reply_err(req, EINVAL);
> > > > > +} else {
> > > > > +fuse_reply_err(req, res == -1 ? errno : 0);
> > > > > +}
> > > > > +#endif
> > > > 
> > > > We should use the formal API if available as first choice
> > > 
> > > OK, done - I've kept the 'ifndef SYS_renameat2' that drops back to an
> > > error for truly ancient cases; although I doubt everything else will
> > > build on something that old.
> > 
> > Hmm, and this breaks on middle age distros;  older distros don't have it
> > at all, new ones have both the syscall and the wrapper; but for the
> > middle age ones they have the syscall but not the wrapper.
> >
> > Dan: What's your preference here; should I add a config fragment to
> > detect the wrapper - it seems overkill rather than just reverting it
> > until it becomes common.
> 
> What specific middle age distro in particular is affected ? My general
> thought would be to /not/ support such distros. Focus on modern distros
> since this is a brand new feature in QEMU, where we should try to
> minimize support for legacy stuff at the start. But depending on the
> distro impacted, the might be a reason to stay with SYS_..

The report came from Ubuntu 18.04 (which Intel uses on CI); that's not
that old, so I think it sohuld be supported.   I don't really see the
justification for insisting on using the wrapper.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




Re: [PATCH] qemu-deprecated: Remove text about Python 2

2020-01-14 Thread Thomas Huth
On 13/01/2020 23.36, John Snow wrote:
> 
> 
> On 1/9/20 4:51 AM, Thomas Huth wrote:
>> Python 2 support has been removed, so we should now also remove
>> the announcement text for the deprecation.
>>
>> Signed-off-by: Thomas Huth 
> 
> Reviewed-by: John Snow 
> 
>> ---
>>  qemu-deprecated.texi | 8 
>>  1 file changed, 8 deletions(-)
>>
>> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
>> index 7033e531de..8b23e98474 100644
>> --- a/qemu-deprecated.texi
>> +++ b/qemu-deprecated.texi
>> @@ -341,14 +341,6 @@ they have no effect when used with @option{-n} to skip 
>> image creation.
>>  Silently ignored options can be confusing, so this combination of
>>  options will be made an error in future versions.
>>  
>> -@section Build system
>> -
>> -@subsection Python 2 support (since 4.1.0)
>> -
>> -In the future, QEMU will require Python 3 to be available at
>> -build time.  Support for Python 2 in scripts shipped with QEMU
>> -is deprecated.
>> -
>>  @section Backwards compatibility
>>  
>>  @subsection Runnability guarantee of CPU models (since 4.1.0)
>>
> 
> Genuine question, I'm sorry:
> 
> Is it worth documenting things we recently removed?

Basically yes. In case of Python 2, it's not a QEMU feature that we
remove here, but a build requirement, and we tell the users that we need
at least Python 3.5 when they run "configure", so I'm not sure whether
that needs to be explicitely mentioned again the docs beside our ChangeLog?

> Right now, we don't
> really have these docs hosted in a searchable way online in a
> per-version format. Once the notice is gone, it's gone from the mirror.
> 
> I removed some bitmap functionality not too long ago and I created a
> "Recently Removed" section as a bit of a troubleshooting guide should it
> be needed.
> 
> - Do we want this section?
> - Should I remove it?
> - Can we add historical docs to the website to see previous deprecated
> docs in a searchable manner?

I also once started a page in the Wiki here:

 https://wiki.qemu.org/Features/RemovedFeatures

... but apparently, it did not get enough attention yet, otherwise you
would have noticed it before introducing the new chapter into the
qemu-doc ...

We definitely need one spot where we can document removed features. I
don't mind which way we do it, either the qemu-doc or the wiki, but we
should unify on one of the two. I guess the qemu-doc is the better place
since we are tracking the deprecated features there already and one more
or less just has to move the text to the other chapter when things get
finally removed?

 Thomas




Re: [PATCH] virtio-9p-device: fix memleak in virtio_9p_device_unrealize

2020-01-14 Thread Christian Schoenebeck
On Dienstag, 14. Januar 2020 08:40:20 CET pannengy...@huawei.com wrote:
> From: Pan Nengyuan 
> 
> v->vq forgot to cleanup in virtio_9p_device_unrealize, the memory leak
> stack is as follow:
> 
> Direct leak of 14336 byte(s) in 2 object(s) allocated from:
>   #0 0x7f819ae43970 (/lib64/libasan.so.5+0xef970)  ??:?
>   #1 0x7f819872f49d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
>   #2 0x55a3a58da624 (./x86_64-softmmu/qemu-system-x86_64+0x2c14624) 
> /mnt/sdb/qemu/hw/virtio/virtio.c:2327 #3 0x55a3a571bac7
> (./x86_64-softmmu/qemu-system-x86_64+0x2a55ac7) 
> /mnt/sdb/qemu/hw/9pfs/virtio-9p-device.c:209 #4 0x55a3a58e7bc6
> (./x86_64-softmmu/qemu-system-x86_64+0x2c21bc6) 
> /mnt/sdb/qemu/hw/virtio/virtio.c:3504 #5 0x55a3a5ebfb37
> (./x86_64-softmmu/qemu-system-x86_64+0x31f9b37) 
> /mnt/sdb/qemu/hw/core/qdev.c:876
> 
> Reported-by: Euler Robot 
> Signed-off-by: Pan Nengyuan 
> ---
>  hw/9pfs/virtio-9p-device.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index b5a7c03f26..b146387ae2 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -215,6 +215,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev,
> Error **errp) V9fsVirtioState *v = VIRTIO_9P(dev);
>  V9fsState *s = &v->state;
> 
> +virtio_delete_queue(v->vq);
>  virtio_cleanup(vdev);
>  v9fs_device_unrealize_common(s, errp);
>  }

Looks like you are using an old interface. The new one is

void virtio_del_queue(VirtIODevice *vdev, int n);

Best regards,
Christian Schoenebeck





Re: [PATCH 066/104] virtiofsd: passthrough_ll: add renameat2 support

2020-01-14 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 10:07:03AM +, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > On Mon, Jan 13, 2020 at 08:06:24PM +, Dr. David Alan Gilbert wrote:
> > > * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> > > > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > > > On Thu, Dec 12, 2019 at 04:38:26PM +, Dr. David Alan Gilbert 
> > > > > (git) wrote:
> > > > > > From: Miklos Szeredi 
> > > > > > 
> > > > > > No glibc support yet, so use syscall().
> > > > > 
> > > > > It exists in glibc in my Fedora 31 install.
> > > > > 
> > > > > Presumably this is related to an older version
> > > > > 
> > > > > > Signed-off-by: Miklos Szeredi 
> > > > > > ---
> > > > > >  tools/virtiofsd/passthrough_ll.c | 10 ++
> > > > > >  1 file changed, 10 insertions(+)
> > > > > > 
> > > > > > diff --git a/tools/virtiofsd/passthrough_ll.c 
> > > > > > b/tools/virtiofsd/passthrough_ll.c
> > > > > > index 91d3120033..bed2270141 100644
> > > > > > --- a/tools/virtiofsd/passthrough_ll.c
> > > > > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > > > > @@ -1083,7 +1083,17 @@ static void lo_rename(fuse_req_t req, 
> > > > > > fuse_ino_t parent, const char *name,
> > > > > >  }
> > > > > >  
> > > > > >  if (flags) {
> > > > > > +#ifndef SYS_renameat2
> > > > > >  fuse_reply_err(req, EINVAL);
> > > > > > +#else
> > > > > > +res = syscall(SYS_renameat2, lo_fd(req, parent), name,
> > > > > > +  lo_fd(req, newparent), newname, flags);
> > > > > > +if (res == -1 && errno == ENOSYS) {
> > > > > > +fuse_reply_err(req, EINVAL);
> > > > > > +} else {
> > > > > > +fuse_reply_err(req, res == -1 ? errno : 0);
> > > > > > +}
> > > > > > +#endif
> > > > > 
> > > > > We should use the formal API if available as first choice
> > > > 
> > > > OK, done - I've kept the 'ifndef SYS_renameat2' that drops back to an
> > > > error for truly ancient cases; although I doubt everything else will
> > > > build on something that old.
> > > 
> > > Hmm, and this breaks on middle age distros;  older distros don't have it
> > > at all, new ones have both the syscall and the wrapper; but for the
> > > middle age ones they have the syscall but not the wrapper.
> > >
> > > Dan: What's your preference here; should I add a config fragment to
> > > detect the wrapper - it seems overkill rather than just reverting it
> > > until it becomes common.
> > 
> > What specific middle age distro in particular is affected ? My general
> > thought would be to /not/ support such distros. Focus on modern distros
> > since this is a brand new feature in QEMU, where we should try to
> > minimize support for legacy stuff at the start. But depending on the
> > distro impacted, the might be a reason to stay with SYS_..
> 
> The report came from Ubuntu 18.04 (which Intel uses on CI); that's not
> that old, so I think it sohuld be supported.   I don't really see the
> justification for insisting on using the wrapper.

Yeah, ok that's pretty new & will be around for a while yet.


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




[PATCH v2 1/3] target/riscv: Fix tb->flags FS status

2020-01-14 Thread shihpo . hung
It was found that running libquantum on riscv-linux qemu produced an
incorrect result. After investigation, FP registers are not saved
during context switch due to incorrect mstatus.FS.

In current implementation tb->flags merges all non-disabled state to
dirty. This means the code in mark_fs_dirty in translate.c that
handles initial and clean states is unreachable.

This patch fixes it and is successfully tested with:
  libquantum

Thanks to Richard for pointing out the actual bug.

Suggested-by: Richard Henderson 
Signed-off-by: ShihPo Hung 
---
 target/riscv/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e59343e..f0ff57e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -295,7 +295,7 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState *env, 
target_ulong *pc,
 #else
 *flags = cpu_mmu_index(env, 0);
 if (riscv_cpu_fp_enabled(env)) {
-*flags |= TB_FLAGS_MSTATUS_FS;
+*flags |= env->mstatus & MSTATUS_FS;
 }
 #endif
 }
-- 
2.7.4




[PATCH v2 2/3] target/riscv: fsd/fsw doesn't dirty FP state

2020-01-14 Thread shihpo . hung
Signed-off-by: ShihPo Hung 
---
 target/riscv/insn_trans/trans_rvd.inc.c | 1 -
 target/riscv/insn_trans/trans_rvf.inc.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvd.inc.c 
b/target/riscv/insn_trans/trans_rvd.inc.c
index 393fa02..ea1044f 100644
--- a/target/riscv/insn_trans/trans_rvd.inc.c
+++ b/target/riscv/insn_trans/trans_rvd.inc.c
@@ -43,7 +43,6 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
 
 tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], t0, ctx->mem_idx, MO_TEQ);
 
-mark_fs_dirty(ctx);
 tcg_temp_free(t0);
 return true;
 }
diff --git a/target/riscv/insn_trans/trans_rvf.inc.c 
b/target/riscv/insn_trans/trans_rvf.inc.c
index 172dbfa..e23cd63 100644
--- a/target/riscv/insn_trans/trans_rvf.inc.c
+++ b/target/riscv/insn_trans/trans_rvf.inc.c
@@ -52,7 +52,6 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
 tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], t0, ctx->mem_idx, MO_TEUL);
 
 tcg_temp_free(t0);
-mark_fs_dirty(ctx);
 return true;
 }
 
-- 
2.7.4




[PATCH v2 3/3] target/riscv: update mstatus.SD when FS is set dirty

2020-01-14 Thread shihpo . hung
remove the check becuase SD bit should summarize FS and XS fields
unconditionally.

Signed-off-by: ShihPo Hung 
---
 target/riscv/csr.c   | 3 +--
 target/riscv/translate.c | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index da02f9f..0e34c29 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -341,8 +341,7 @@ static int write_mstatus(CPURISCVState *env, int csrno, 
target_ulong val)
 
 mstatus = (mstatus & ~mask) | (val & mask);
 
-dirty = (riscv_cpu_fp_enabled(env) &&
- ((mstatus & MSTATUS_FS) == MSTATUS_FS)) |
+dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
 ((mstatus & MSTATUS_XS) == MSTATUS_XS);
 mstatus = set_field(mstatus, MSTATUS_SD, dirty);
 env->mstatus = mstatus;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index ab6a891..e825ee6 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -395,6 +395,7 @@ static void mark_fs_dirty(DisasContext *ctx)
 tmp = tcg_temp_new();
 tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
 tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS);
+tcg_gen_ori_tl(tmp, tmp, MSTATUS_SD);
 tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
 tcg_temp_free(tmp);
 }
-- 
2.7.4




[PULL 1/6] qapi: Tweak "command returns a nice type" check for clarity

2020-01-14 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-Id: <20191120182551.23795-2-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 scripts/qapi/schema.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cf0045f34e..cfb574c85d 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -711,10 +711,11 @@ class QAPISchemaCommand(QAPISchemaEntity):
 self.ret_type = schema.resolve_type(
 self._ret_type_name, self.info, "command's 'returns'")
 if self.name not in self.info.pragma.returns_whitelist:
-if not (isinstance(self.ret_type, QAPISchemaObjectType)
-or (isinstance(self.ret_type, QAPISchemaArrayType)
-and isinstance(self.ret_type.element_type,
-   QAPISchemaObjectType))):
+typ = self.ret_type
+if isinstance(typ, QAPISchemaArrayType):
+typ = self.ret_type.element_type
+assert typ
+if not isinstance(typ, QAPISchemaObjectType):
 raise QAPISemError(
 self.info,
 "command's 'returns' cannot take %s"
-- 
2.21.1




Re: [PATCH/RFC 0/1] Vhost User Cross Cable: Intro

2020-01-14 Thread Stefan Hajnoczi
On Fri, Jan 10, 2020 at 10:34 AM Marc-André Lureau
 wrote:
> On Wed, Jan 8, 2020 at 5:57 AM V.  wrote:

Hi V.,
I think I remember you from Etherboot/gPXE days :).

> > 3.
> > Now if Cross Cable is actually a new and (after a code-rewrite of 10) a 
> > viable way to connect 2 QEMU's together, could I actually
> > suggest a better way?
> > I am thinking of a '-netdev vhost-user-slave' option to connect (as client 
> > or server) to a master QEMU running '-netdev vhost-user'.
> > This way there is no need for any external program at all, the master can 
> > have it's memory unshared and everything would just work
> > and be fast.
> > Also the whole thing can fall back to normal virtio if memory is not shared 
> > and it would even work in pure usermode without any
> > context switch.
> >
> > Building a patch for this idea I could maybe get around to, don't clearly 
> > have an idea how much work this would be but I've done
> > crazier things.
> > But is this is something that someone might be able to whip up in an hour 
> > or two? Someone who actually does have a clue about vhost
> > and virtio maybe? ;-)
>
> I believe https://wiki.qemu.org/Features/VirtioVhostUser is what you
> are after. It's still being discussed and non-trivial, but not very
> active lately afaik.

virtio-vhost-user is being experimented with in the SPDK community but
there has been no activity on VIRTIO standardization or the QEMU
patches for some time.  More info here:
https://ndragazis.github.io/spdk.html

I think the new ivshmem v2 feature may provide the functionality you
are looking for, but I haven't looked at them yet.  Here is the link:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg668749.html

And here is Jan's KVM Forum presentation on ivshmem v2:
https://www.youtube.com/watch?v=TiZrngLUFMA

Stefan



Re: [PATCH] qemu-deprecated: Remove text about Python 2

2020-01-14 Thread Daniel P . Berrangé
On Tue, Jan 14, 2020 at 11:08:16AM +0100, Thomas Huth wrote:
> On 13/01/2020 23.36, John Snow wrote:
> > 
> > 
> > On 1/9/20 4:51 AM, Thomas Huth wrote:
> >> Python 2 support has been removed, so we should now also remove
> >> the announcement text for the deprecation.
> >>
> >> Signed-off-by: Thomas Huth 
> > 
> > Reviewed-by: John Snow 
> > 
> >> ---
> >>  qemu-deprecated.texi | 8 
> >>  1 file changed, 8 deletions(-)
> >>
> >> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> >> index 7033e531de..8b23e98474 100644
> >> --- a/qemu-deprecated.texi
> >> +++ b/qemu-deprecated.texi
> >> @@ -341,14 +341,6 @@ they have no effect when used with @option{-n} to 
> >> skip image creation.
> >>  Silently ignored options can be confusing, so this combination of
> >>  options will be made an error in future versions.
> >>  
> >> -@section Build system
> >> -
> >> -@subsection Python 2 support (since 4.1.0)
> >> -
> >> -In the future, QEMU will require Python 3 to be available at
> >> -build time.  Support for Python 2 in scripts shipped with QEMU
> >> -is deprecated.
> >> -
> >>  @section Backwards compatibility
> >>  
> >>  @subsection Runnability guarantee of CPU models (since 4.1.0)
> >>
> > 
> > Genuine question, I'm sorry:
> > 
> > Is it worth documenting things we recently removed?
> 
> Basically yes. In case of Python 2, it's not a QEMU feature that we
> remove here, but a build requirement, and we tell the users that we need
> at least Python 3.5 when they run "configure", so I'm not sure whether
> that needs to be explicitely mentioned again the docs beside our ChangeLog?

In general changed build pre-requisites such as new minimum software
versions are documented in the release notes:

   https://wiki.qemu.org/ChangeLog/5.0#Build_Information

We normally would not list build pre-requisites in the deprecation notes
at all, since they don't follow the deprecation process normally. We
just update minimum versions immediately that our supported OS build
platforms change due to an OS going end of life. So for example we
have in the past bumped gnutls, glib, nettle, gcc, etc min versions
with no warning.  So the fact that Python 2 was mentioned in the
deprecations at all was slightly unusual. This is mostly just to be
nice to users since the OS platforms here aren't going EOL and still
ship Python 2, we simply don't wish to support it any more, since
the distros also all have Py 3.


> 
> > Right now, we don't
> > really have these docs hosted in a searchable way online in a
> > per-version format. Once the notice is gone, it's gone from the mirror.
> > 
> > I removed some bitmap functionality not too long ago and I created a
> > "Recently Removed" section as a bit of a troubleshooting guide should it
> > be needed.
> > 
> > - Do we want this section?
> > - Should I remove it?
> > - Can we add historical docs to the website to see previous deprecated
> > docs in a searchable manner?
> 
> I also once started a page in the Wiki here:
> 
>  https://wiki.qemu.org/Features/RemovedFeatures
> 
> ... but apparently, it did not get enough attention yet, otherwise you
> would have noticed it before introducing the new chapter into the
> qemu-doc ...
> 
> We definitely need one spot where we can document removed features. I
> don't mind which way we do it, either the qemu-doc or the wiki, but we
> should unify on one of the two. I guess the qemu-doc is the better place
> since we are tracking the deprecated features there already and one more
> or less just has to move the text to the other chapter when things get
> finally removed?

Yeah, I've said in the past that we should not be deleting deprecations
from the docs entirely.

If you look at GTK docs for example, you'll see they keep a record of
all incompatible or noteworth changes between release:

  https://developer.gnome.org/gtk3/stable/gtk-migrating-3-x-to-y.html

IMHO, we should follow this and have an appendix of removed features,
with sub-sections per QEMU release listing each removed feature. Thus
deprecation docs just get moved to this appendix at the right time.

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




[PULL 0/6] QAPI patches for 2020-01-14

2020-01-14 Thread Markus Armbruster
My previous pull request failed tests with Python 2, which is now
gone.  Try again.

The following changes since commit dc65a5bdc9fa543690a775b50d4ffbeb22c56d6d:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.0-20200108' into 
staging (2020-01-10 16:15:04 +)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2020-01-14

for you to fetch changes up to 3bef3aaec91815b75a78a4c12ca92ac3cec53faf:

  qapi: Simplify QAPISchemaModularCVisitor (2020-01-14 11:01:58 +0100)


QAPI patches for 2020-01-14


Markus Armbruster (6):
  qapi: Tweak "command returns a nice type" check for clarity
  tests/Makefile.include: Fix missing test-qapi-emit-events.[ch]
  qapi: Generate command registration stuff into separate files
  qapi: Proper intermediate representation for modules
  qapi: Fix code generation for empty modules
  qapi: Simplify QAPISchemaModularCVisitor

 docs/devel/qapi-code-gen.txt | 19 +--
 Makefile |  4 +-
 monitor/misc.c   |  7 ++-
 qga/main.c   |  2 +-
 tests/test-qmp-cmds.c|  1 +
 .gitignore   |  1 +
 qapi/Makefile.objs   |  1 +
 qga/Makefile.objs|  1 +
 scripts/qapi/commands.py | 17 --
 scripts/qapi/events.py   |  2 +-
 scripts/qapi/gen.py  | 28 +-
 scripts/qapi/schema.py   | 92 
 scripts/qapi/types.py|  5 +-
 scripts/qapi/visit.py|  8 +--
 tests/.gitignore |  1 +
 tests/Makefile.include   |  9 +++-
 tests/qapi-schema/empty.out  |  1 +
 tests/qapi-schema/include-repetition.out |  6 +--
 tests/qapi-schema/qapi-schema-test.out   | 24 -
 19 files changed, 144 insertions(+), 85 deletions(-)

-- 
2.21.1




[PULL 6/6] qapi: Simplify QAPISchemaModularCVisitor

2020-01-14 Thread Markus Armbruster
Since the previous commit, QAPISchemaVisitor.visit_module() is called
just once.  Simplify QAPISchemaModularCVisitor accordingly.

Signed-off-by: Markus Armbruster 
Message-Id: <20191120182551.23795-7-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 scripts/qapi/commands.py |  2 +-
 scripts/qapi/events.py   |  2 +-
 scripts/qapi/gen.py  | 28 ++--
 scripts/qapi/types.py|  5 +++--
 scripts/qapi/visit.py|  8 
 5 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 47f4a18cfe..afa55b055c 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -239,7 +239,7 @@ class 
QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
 def __init__(self, prefix):
 QAPISchemaModularCVisitor.__init__(
 self, prefix, 'qapi-commands',
-' * Schema-defined QAPI/QMP commands', __doc__)
+' * Schema-defined QAPI/QMP commands', None, __doc__)
 self._regy = QAPIGenCCode(None)
 self._visited_ret_types = {}
 
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 10fc509fa9..2bde3e6128 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -140,7 +140,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
 def __init__(self, prefix):
 QAPISchemaModularCVisitor.__init__(
 self, prefix, 'qapi-events',
-' * Schema-defined QAPI/QMP events', __doc__)
+' * Schema-defined QAPI/QMP events', None, __doc__)
 self._event_enum_name = c_name(prefix + 'QAPIEvent', protect=False)
 self._event_enum_members = []
 self._event_emit_name = c_name(prefix + 'qapi_event_emit')
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 112b6d94c5..95afae0615 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -201,10 +201,11 @@ class QAPISchemaMonolithicCVisitor(QAPISchemaVisitor):
 
 class QAPISchemaModularCVisitor(QAPISchemaVisitor):
 
-def __init__(self, prefix, what, blurb, pydoc):
+def __init__(self, prefix, what, user_blurb, builtin_blurb, pydoc):
 self._prefix = prefix
 self._what = what
-self._blurb = blurb
+self._user_blurb = user_blurb
+self._builtin_blurb = builtin_blurb
 self._pydoc = pydoc
 self._genc = None
 self._genh = None
@@ -245,7 +246,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
 genc = QAPIGenC(basename + '.c', blurb, self._pydoc)
 genh = QAPIGenH(basename + '.h', blurb, self._pydoc)
 self._module[name] = (genc, genh)
-self._set_module(name)
+self._genc, self._genh = self._module[name]
 
 def _add_user_module(self, name, blurb):
 assert self._is_user_module(name)
@@ -256,9 +257,6 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
 def _add_system_module(self, name, blurb):
 self._add_module(name and './' + name, blurb)
 
-def _set_module(self, name):
-self._genc, self._genh = self._module[name]
-
 def write(self, output_dir, opt_builtins=False):
 for name in self._module:
 if self._is_builtin_module(name) and not opt_builtins:
@@ -271,15 +269,17 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
 pass
 
 def visit_module(self, name):
-if name in self._module:
-self._set_module(name)
-elif self._is_builtin_module(name):
-# The built-in module has not been created.  No code may
-# be generated.
-self._genc = None
-self._genh = None
+if name is None:
+if self._builtin_blurb:
+self._add_system_module(None, self._builtin_blurb)
+self._begin_system_module(name)
+else:
+# The built-in module has not been created.  No code may
+# be generated.
+self._genc = None
+self._genh = None
 else:
-self._add_user_module(name, self._blurb)
+self._add_user_module(name, self._user_blurb)
 self._begin_user_module(name)
 
 def visit_include(self, name, info):
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index d8751daa04..99dcaf7074 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -243,8 +243,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
 def __init__(self, prefix):
 QAPISchemaModularCVisitor.__init__(
 self, prefix, 'qapi-types', ' * Schema-defined QAPI types',
-__doc__)
-self._add_system_module(None, ' * Built-in QAPI types')
+' * Built-in QAPI types', __doc__)
+
+def _begin_system_module(self, name):
 self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
 #include "qapi/dealloc-visitor.h"
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index c72f2bc5c0..4efce62b0c 100644
--- a/

[PULL 2/6] tests/Makefile.include: Fix missing test-qapi-emit-events.[ch]

2020-01-14 Thread Markus Armbruster
Commit 5d75648b56 "qapi: Generate QAPIEvent stuff into separate files"
added tests/test-qapi-emit-events.[ch] to the set of generated files,
but neglected to update tests/.gitignore and tests/Makefile.include.
Commit a0af8cee3c "tests/.gitignore: ignore test-qapi-emit-events.[ch]
for in-tree builds" fixed the former.  Now fix the latter.

Signed-off-by: Markus Armbruster 
Message-Id: <20191120182551.23795-3-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 tests/Makefile.include | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7a767bf114..cd5e13f42f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -506,6 +506,7 @@ generated-files-y += tests/test-qapi-visit-sub-sub-module.h
 generated-files-y += tests/test-qapi-commands.h
 generated-files-y += tests/include/test-qapi-commands-sub-module.h
 generated-files-y += tests/test-qapi-commands-sub-sub-module.h
+generated-files-y += tests/test-qapi-emit-events.h
 generated-files-y += tests/test-qapi-events.h
 generated-files-y += tests/include/test-qapi-events-sub-module.h
 generated-files-y += tests/test-qapi-events-sub-sub-module.h
@@ -614,6 +615,7 @@ tests/include/test-qapi-commands-sub-module.h \
 tests/include/test-qapi-commands-sub-module.c \
 tests/test-qapi-commands-sub-sub-module.h \
 tests/test-qapi-commands-sub-sub-module.c \
+tests/test-qapi-emit-events.c tests/test-qapi-emit-events.h \
 tests/test-qapi-events.c tests/test-qapi-events.h \
 tests/include/test-qapi-events-sub-module.c \
 tests/include/test-qapi-events-sub-module.h \
@@ -654,7 +656,7 @@ tests/dbus-vmstate-test.o: tests/dbus-vmstate1.h
 
 tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o 
$(test-qapi-obj-y)
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o 
$(test-qapi-obj-y)
-tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) 
tests/test-qapi-events.o
+tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) 
tests/test-qapi-emit-events.o tests/test-qapi-events.o
 tests/test-qobject-output-visitor$(EXESUF): 
tests/test-qobject-output-visitor.o $(test-qapi-obj-y)
 tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o 
$(test-qapi-obj-y)
 tests/test-qobject-input-visitor$(EXESUF): tests/test-qobject-input-visitor.o 
$(test-qapi-obj-y)
-- 
2.21.1




[PULL 5/6] qapi: Fix code generation for empty modules

2020-01-14 Thread Markus Armbruster
When a sub-module doesn't contain any definitions, we don't generate
code for it, but we do generate the #include.

We generate code only for modules that get visited.
QAPISchema.visit() visits only modules that have definitions.  It can
visit modules multiple times.

Clean this up as follows.  Collect entities in their QAPISchemaModule.
Have QAPISchema.visit() call QAPISchemaModule.visit() for each module.
Have QAPISchemaModule.visit() call .visit_module() for itself, and
QAPISchemaEntity.visit() for each of its entities.  This way, we visit
each module exactly once.

Signed-off-by: Markus Armbruster 
Message-Id: <20191120182551.23795-6-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 scripts/qapi/schema.py   | 24 +---
 tests/qapi-schema/empty.out  |  1 +
 tests/qapi-schema/include-repetition.out |  6 ++
 tests/qapi-schema/qapi-schema-test.out   | 24 ++--
 4 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 0f2e0dcfce..0bfc5256fb 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -68,6 +68,7 @@ class QAPISchemaEntity(object):
 def _set_module(self, schema, info):
 assert self._checked
 self._module = schema.module_by_fname(info and info.fname)
+self._module.add_entity(self)
 
 def set_module(self, schema):
 self._set_module(schema, self.info)
@@ -77,11 +78,6 @@ class QAPISchemaEntity(object):
 assert self._checked
 return self._ifcond
 
-@property
-def module(self):
-assert self._module or not self.info
-return self._module
-
 def is_implicit(self):
 return not self.info
 
@@ -142,6 +138,16 @@ class QAPISchemaVisitor(object):
 class QAPISchemaModule(object):
 def __init__(self, name):
 self.name = name
+self._entity_list = []
+
+def add_entity(self, ent):
+self._entity_list.append(ent)
+
+def visit(self, visitor):
+visitor.visit_module(self.name)
+for entity in self._entity_list:
+if visitor.visit_needed(entity):
+entity.visit(visitor)
 
 
 class QAPISchemaInclude(QAPISchemaEntity):
@@ -1093,10 +1099,6 @@ class QAPISchema(object):
 def visit(self, visitor):
 visitor.visit_begin(self)
 module = None
-for entity in self._entity_list:
-if visitor.visit_needed(entity):
-if entity.module != module:
-module = entity.module
-visitor.visit_module(module.name)
-entity.visit(visitor)
+for mod in self._module_dict.values():
+mod.visit(visitor)
 visitor.visit_end()
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
index 5b53d00702..69666c39ad 100644
--- a/tests/qapi-schema/empty.out
+++ b/tests/qapi-schema/empty.out
@@ -9,3 +9,4 @@ enum QType
 member qdict
 member qlist
 member qbool
+module empty.json
diff --git a/tests/qapi-schema/include-repetition.out 
b/tests/qapi-schema/include-repetition.out
index 5423983239..0b654ddebb 100644
--- a/tests/qapi-schema/include-repetition.out
+++ b/tests/qapi-schema/include-repetition.out
@@ -11,15 +11,13 @@ enum QType
 member qbool
 module include-repetition.json
 include comments.json
+include include-repetition-sub.json
+include comments.json
 module comments.json
 enum Status
 member good
 member bad
 member ugly
-module include-repetition.json
-include include-repetition-sub.json
 module include-repetition-sub.json
 include comments.json
 include comments.json
-module include-repetition.json
-include comments.json
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 3660e75a48..9bd3c4a490 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -153,9 +153,6 @@ object q_obj_sizeList-wrapper
 member data: sizeList optional=False
 object q_obj_anyList-wrapper
 member data: anyList optional=False
-module sub-sub-module.json
-array StatusList Status
-module qapi-schema-test.json
 object q_obj_StatusList-wrapper
 member data: StatusList optional=False
 enum UserDefListUnionKind
@@ -193,17 +190,6 @@ object UserDefListUnion
 case any: q_obj_anyList-wrapper
 case user: q_obj_StatusList-wrapper
 include include/sub-module.json
-module include/sub-module.json
-include sub-sub-module.json
-module sub-sub-module.json
-enum Status
-member good
-member bad
-member ugly
-module include/sub-module.json
-object SecondArrayRef
-member s: StatusList optional=False
-module qapi-schema-test.json
 command user_def_cmd None -> None
 gen=True success_response=True boxed=False oob=False preconfig=False
 object q_obj_user_def_cmd1-arg
@@ -435,3 +421,13 @@ command test-command-cond-features3 None -> None
 gen=True success_response=True boxed=False oob=False preco

Re: [PATCH v2 03/10] target/ppc: Correct handling of real mode accesses with vhyp on hash MMU

2020-01-14 Thread Cédric Le Goater
On 1/7/20 5:48 AM, David Gibson wrote:
> On ppc we have the concept of virtual hypervisor ("vhyp") mode, where we
> only model the non-hypervisor-privileged parts of the cpu.  Essentially we
> model the hypervisor's behaviour from the point of view of a guest OS, but
> we don't model the hypervisor's execution.
> 
> In particular, in this mode, qemu's notion of target physical address is
> a guest physical address from the vcpu's point of view.  So accesses in
> guest real mode don't require translation.  If we were modelling the
> hypervisor mode, we'd need to translate the guest physical address into
> a host physical address.
> 
> Currently, we handle this sloppily: we rely on setting up the virtual LPCR
> and RMOR registers so that GPAs are simply HPAs plus an offset, which we
> set to zero.  This is already conceptually dubious, since the LPCR and RMOR
> registers don't exist in the non-hypervisor portion of the CPU.  It gets
> worse with POWER9, where RMOR and LPCR[VPM0] no longer exist at all.
> 
> Clean this up by explicitly handling the vhyp case.  While we're there,
> remove some unnecessary nesting of if statements that made the logic to
> select the correct real mode behaviour a bit less clear than it could be.
> 
> Signed-off-by: David Gibson 

I went through the changes and they look correct to me.

Reviewed-by: Cédric Le Goater 

C.

> ---
>  target/ppc/mmu-hash64.c | 60 -
>  1 file changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index a881876647..5fabd93c92 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -789,27 +789,30 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr 
> eaddr,
>   */
>  raddr = eaddr & 0x0FFFULL;
>  
> -/* In HV mode, add HRMOR if top EA bit is clear */
> -if (msr_hv || !env->has_hv_mode) {
> +if (cpu->vhyp) {
> +/*
> + * In virtual hypervisor mode, there's nothing to do:
> + *   EA == GPA == qemu guest address
> + */
> +} else if (msr_hv || !env->has_hv_mode) {
> +/* In HV mode, add HRMOR if top EA bit is clear */
>  if (!(eaddr >> 63)) {
>  raddr |= env->spr[SPR_HRMOR];
>  }
> -} else {
> -/* Otherwise, check VPM for RMA vs VRMA */
> -if (env->spr[SPR_LPCR] & LPCR_VPM0) {
> -slb = &env->vrma_slb;
> -if (slb->sps) {
> -goto skip_slb_search;
> -}
> -/* Not much else to do here */
> +} else if (env->spr[SPR_LPCR] & LPCR_VPM0) {
> +/* Emulated VRMA mode */
> +slb = &env->vrma_slb;
> +if (!slb->sps) {
> +/* Invalid VRMA setup, machine check */
>  cs->exception_index = POWERPC_EXCP_MCHECK;
>  env->error_code = 0;
>  return 1;
> -} else if (raddr < env->rmls) {
> -/* RMA. Check bounds in RMLS */
> -raddr |= env->spr[SPR_RMOR];
> -} else {
> -/* The access failed, generate the approriate interrupt */
> +}
> +
> +goto skip_slb_search;
> +} else {
> +/* Emulated old-style RMO mode, bounds check against RMLS */
> +if (raddr >= env->rmls) {
>  if (rwx == 2) {
>  ppc_hash64_set_isi(cs, SRR1_PROTFAULT);
>  } else {
> @@ -821,6 +824,8 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr 
> eaddr,
>  }
>  return 1;
>  }
> +
> +raddr |= env->spr[SPR_RMOR];
>  }
>  tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
>   PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
> @@ -953,22 +958,27 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, 
> target_ulong addr)
>  /* In real mode the top 4 effective address bits are ignored */
>  raddr = addr & 0x0FFFULL;
>  
> -/* In HV mode, add HRMOR if top EA bit is clear */
> -if ((msr_hv || !env->has_hv_mode) && !(addr >> 63)) {
> +if (cpu->vhyp) {
> +/*
> + * In virtual hypervisor mode, there's nothing to do:
> + *   EA == GPA == qemu guest address
> + */
> +return raddr;
> +} else if ((msr_hv || !env->has_hv_mode) && !(addr >> 63)) {
> +/* In HV mode, add HRMOR if top EA bit is clear */
>  return raddr | env->spr[SPR_HRMOR];
> -}
> -
> -/* Otherwise, check VPM for RMA vs VRMA */
> -if (env->spr[SPR_LPCR] & LPCR_VPM0) {
> +} else if (env->spr[SPR_LPCR] & LPCR_VPM0) {
> +/* Emulated VRMA mode */
>  slb = &env->vrma_slb;
>  if (!slb->sp

[PULL 3/6] qapi: Generate command registration stuff into separate files

2020-01-14 Thread Markus Armbruster
Having to include qapi-commands.h just for qmp_init_marshal() is
suboptimal.  Generate it into separate files.  This lets
monitor/misc.c, qga/main.c, and the generated qapi-commands-FOO.h
include less.

Signed-off-by: Markus Armbruster 
Message-Id: <20191120182551.23795-4-arm...@redhat.com>
[Typos in docs/devel/qapi-code-gen.txt fixed]
Reviewed-by: Eric Blake 
---
 docs/devel/qapi-code-gen.txt | 19 ---
 Makefile |  4 +++-
 monitor/misc.c   |  7 ++-
 qga/main.c   |  2 +-
 tests/test-qmp-cmds.c|  1 +
 .gitignore   |  1 +
 qapi/Makefile.objs   |  1 +
 qga/Makefile.objs|  1 +
 scripts/qapi/commands.py | 15 +++
 tests/.gitignore |  1 +
 tests/Makefile.include   |  5 -
 11 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 45c93a43cc..59d6973e1e 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1493,6 +1493,10 @@ $(prefix)qapi-commands.c: Command marshal/dispatch 
functions for each
 $(prefix)qapi-commands.h: Function prototypes for the QMP commands
   specified in the schema
 
+$(prefix)qapi-init-commands.h - Command initialization prototype
+
+$(prefix)qapi-init-commands.c - Command initialization code
+
 Example:
 
 $ cat qapi-generated/example-qapi-commands.h
@@ -1502,11 +1506,9 @@ Example:
 #define EXAMPLE_QAPI_COMMANDS_H
 
 #include "example-qapi-types.h"
-#include "qapi/qmp/dispatch.h"
 
 UserDefOne *qmp_my_command(UserDefOneList *arg1, Error **errp);
 void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp);
-void example_qmp_init_marshal(QmpCommandList *cmds);
 
 #endif /* EXAMPLE_QAPI_COMMANDS_H */
 $ cat qapi-generated/example-qapi-commands.c
@@ -1566,7 +1568,19 @@ Example:
 visit_end_struct(v, NULL);
 visit_free(v);
 }
+[Uninteresting stuff omitted...]
+$ cat qapi-generated/example-qapi-init-commands.h
+[Uninteresting stuff omitted...]
+#ifndef EXAMPLE_QAPI_INIT_COMMANDS_H
+#define EXAMPLE_QAPI_INIT_COMMANDS_H
 
+#include "qapi/qmp/dispatch.h"
+
+void example_qmp_init_marshal(QmpCommandList *cmds);
+
+#endif /* EXAMPLE_QAPI_INIT_COMMANDS_H */
+$ cat qapi-generated/example-qapi-init-commands.c
+[Uninteresting stuff omitted...]
 void example_qmp_init_marshal(QmpCommandList *cmds)
 {
 QTAILQ_INIT(cmds);
@@ -1574,7 +1588,6 @@ Example:
 qmp_register_command(cmds, "my-command",
  qmp_marshal_my_command, QCO_NO_OPTIONS);
 }
-
 [Uninteresting stuff omitted...]
 
 For a modular QAPI schema (see section Include directives), code for
diff --git a/Makefile b/Makefile
index 6b5ad1121b..56ff731a53 100644
--- a/Makefile
+++ b/Makefile
@@ -117,6 +117,7 @@ GENERATED_QAPI_FILES += qapi/qapi-builtin-visit.h 
qapi/qapi-builtin-visit.c
 GENERATED_QAPI_FILES += qapi/qapi-visit.h qapi/qapi-visit.c
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-visit-%.h)
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-visit-%.c)
+GENERATED_QAPI_FILES += qapi/qapi-init-commands.h qapi/qapi-init-commands.c
 GENERATED_QAPI_FILES += qapi/qapi-commands.h qapi/qapi-commands.c
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-commands-%.h)
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-commands-%.c)
@@ -601,6 +602,7 @@ $(SRC_PATH)/scripts/qapi-gen.py
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
 qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h \
 qga/qapi-generated/qga-qapi-commands.h qga/qapi-generated/qga-qapi-commands.c \
+qga/qapi-generated/qga-qapi-init-commands.h 
qga/qapi-generated/qga-qapi-init-commands.c \
 qga/qapi-generated/qga-qapi-doc.texi: \
 qga/qapi-generated/qapi-gen-timestamp ;
 qga/qapi-generated/qapi-gen-timestamp: $(SRC_PATH)/qga/qapi-schema.json 
$(qapi-py)
@@ -619,7 +621,7 @@ qapi-gen-timestamp: $(qapi-modules) $(qapi-py)
"GEN","$(@:%-timestamp=%)")
@>$@
 
-QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h 
qga-qapi-commands.h)
+QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h 
qga-qapi-commands.h qga-qapi-init-commands.h)
 $(qga-obj-y): $(QGALIB_GEN)
 
 qemu-ga$(EXESUF): $(qga-obj-y) $(COMMON_LDADDS)
diff --git a/monitor/misc.c b/monitor/misc.c
index a04d7edde0..de1ca4d114 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -66,8 +66,13 @@
 #include "qemu/option.h"
 #include "qemu/thread.h"
 #include "block/qapi.h"
-#include "qapi/qapi-commands.h"
+#include "qapi/qapi-commands-char.h"
+#include "qapi/qapi-commands-migration.h"
+#include "qapi/qapi-commands-misc.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-commands-trace.h"
 #include "qapi/qapi-emit-events.h"
+#include "qapi/qapi-init-commands.h"
 #include "qapi/error.h"
 #include "qap

[PULL 4/6] qapi: Proper intermediate representation for modules

2020-01-14 Thread Markus Armbruster
Modules are represented only by their names so far.  Introduce class
QAPISchemaModule.  So far, it merely wraps the name.  The next patch
will put it to more interesting use.

Once again, arrays spice up the patch a bit.  For any other type,
@info points to the definition, which lets us map from @info to
module.  For arrays, there is no definition, and @info points to the
first use instead.  We have to use the element type's module instead,
which is only available after .check().

Signed-off-by: Markus Armbruster 
Message-Id: <20191120182551.23795-5-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 scripts/qapi/schema.py | 63 --
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cfb574c85d..0f2e0dcfce 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -50,9 +50,6 @@ class QAPISchemaEntity(object):
 
 def check(self, schema):
 assert not self._checked
-if self.info:
-self._module = os.path.relpath(self.info.fname,
-   os.path.dirname(schema.fname))
 seen = {}
 for f in self.features:
 f.check_clash(self.info, seen)
@@ -68,6 +65,13 @@ class QAPISchemaEntity(object):
 if self.doc:
 self.doc.check()
 
+def _set_module(self, schema, info):
+assert self._checked
+self._module = schema.module_by_fname(info and info.fname)
+
+def set_module(self, schema):
+self._set_module(schema, self.info)
+
 @property
 def ifcond(self):
 assert self._checked
@@ -75,7 +79,7 @@ class QAPISchemaEntity(object):
 
 @property
 def module(self):
-assert self._checked
+assert self._module or not self.info
 return self._module
 
 def is_implicit(self):
@@ -135,15 +139,19 @@ class QAPISchemaVisitor(object):
 pass
 
 
+class QAPISchemaModule(object):
+def __init__(self, name):
+self.name = name
+
+
 class QAPISchemaInclude(QAPISchemaEntity):
-
-def __init__(self, fname, info):
+def __init__(self, sub_module, info):
 QAPISchemaEntity.__init__(self, None, info, None)
-self.fname = fname
+self._sub_module = sub_module
 
 def visit(self, visitor):
 QAPISchemaEntity.visit(self, visitor)
-visitor.visit_include(self.fname, self.info)
+visitor.visit_include(self._sub_module.name, self.info)
 
 
 class QAPISchemaType(QAPISchemaEntity):
@@ -276,16 +284,14 @@ class QAPISchemaArrayType(QAPISchemaType):
 self.info and self.info.defn_meta)
 assert not isinstance(self.element_type, QAPISchemaArrayType)
 
+def set_module(self, schema):
+self._set_module(schema, self.element_type.info)
+
 @property
 def ifcond(self):
 assert self._checked
 return self.element_type.ifcond
 
-@property
-def module(self):
-assert self._checked
-return self.element_type.module
-
 def is_implicit(self):
 return True
 
@@ -783,6 +789,10 @@ class QAPISchema(object):
 self.docs = parser.docs
 self._entity_list = []
 self._entity_dict = {}
+self._module_dict = {}
+self._schema_dir = os.path.dirname(fname)
+self._make_module(None) # built-ins
+self._make_module(fname)
 self._predefining = True
 self._def_predefineds()
 self._predefining = False
@@ -826,14 +836,26 @@ class QAPISchema(object):
 info, "%s uses unknown type '%s'" % (what, name))
 return typ
 
+def _module_name(self, fname):
+if fname is None:
+return None
+return os.path.relpath(fname, self._schema_dir)
+
+def _make_module(self, fname):
+name = self._module_name(fname)
+if not name in self._module_dict:
+self._module_dict[name] = QAPISchemaModule(name)
+return self._module_dict[name]
+
+def module_by_fname(self, fname):
+name = self._module_name(fname)
+assert name in self._module_dict
+return self._module_dict[name]
+
 def _def_include(self, expr, info, doc):
 include = expr['include']
 assert doc is None
-main_info = info
-while main_info.parent:
-main_info = main_info.parent
-fname = os.path.relpath(include, os.path.dirname(main_info.fname))
-self._def_entity(QAPISchemaInclude(fname, info))
+self._def_entity(QAPISchemaInclude(self._make_module(include), info))
 
 def _def_builtin_type(self, name, json_type, c_type):
 self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
@@ -1065,15 +1087,16 @@ class QAPISchema(object):
 ent.check(self)
 ent.connect_doc()
 ent.check_doc()
+for ent in self._entity_list:
+ent.set_module(self)
 
 def visit(self, visitor):
 visitor.visit

[PATCH v3 0/5] aspeed: extensions and fixes

2020-01-14 Thread Cédric Le Goater
Hi,

Here is a short series adding :

 - a new eMMC controller model for the AST2600 SoC (Andrew)
 - accessors to control the led state of the pca9552 device (Joel)
 - a 'execute-in-place' property to boot directly from CE0

Thanks,

C.

Changes since v2:

  - wrote a better commit log for the ftgmac100 fixes 
  - renamed sdhci object names

Changes since v1:

  - removed ternary operator from sdhci_attach_drive()
  - changed object name to "emmc"


Andrew Jeffery (2):
  hw/sd: Configure number of slots exposed by the ASPEED SDHCI model
  hw/arm: ast2600: Wire up the eMMC controller

Cédric Le Goater (2):
  ftgmac100: check RX and TX buffer alignment
  hw/arm/aspeed: add a 'execute-in-place' property to boot directly from
CE0

Joel Stanley (1):
  misc/pca9552: Add qom set and get

 include/hw/arm/aspeed.h  |  2 +
 include/hw/arm/aspeed_soc.h  |  2 +
 include/hw/sd/aspeed_sdhci.h |  1 +
 hw/arm/aspeed.c  | 72 +++--
 hw/arm/aspeed_ast2600.c  | 31 +++--
 hw/arm/aspeed_soc.c  |  2 +
 hw/misc/pca9552.c| 90 
 hw/net/ftgmac100.c   | 13 ++
 hw/sd/aspeed_sdhci.c | 11 -
 9 files changed, 204 insertions(+), 20 deletions(-)

-- 
2.21.1




[PATCH v3 1/5] hw/sd: Configure number of slots exposed by the ASPEED SDHCI model

2020-01-14 Thread Cédric Le Goater
From: Andrew Jeffery 

The AST2600 includes a second cut-down version of the SD/MMC controller
found in the AST2500, named the eMMC controller. It's cut down in the
sense that it only supports one slot rather than two, but it brings the
total number of slots supported by the AST2600 to three.

The existing code assumed that the SD controller always provided two
slots. Rework the SDHCI object to expose the number of slots as a
property to be set by the SoC configuration.

Signed-off-by: Andrew Jeffery 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
Signed-off-by: Cédric Le Goater 
---
 include/hw/sd/aspeed_sdhci.h |  1 +
 hw/arm/aspeed.c  |  2 +-
 hw/arm/aspeed_ast2600.c  |  2 ++
 hw/arm/aspeed_soc.c  |  2 ++
 hw/sd/aspeed_sdhci.c | 11 +--
 5 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/hw/sd/aspeed_sdhci.h b/include/hw/sd/aspeed_sdhci.h
index dfdab4379021..dffbb46946b9 100644
--- a/include/hw/sd/aspeed_sdhci.h
+++ b/include/hw/sd/aspeed_sdhci.h
@@ -24,6 +24,7 @@ typedef struct AspeedSDHCIState {
 SysBusDevice parent;
 
 SDHCIState slots[ASPEED_SDHCI_NUM_SLOTS];
+uint8_t num_slots;
 
 MemoryRegion iomem;
 qemu_irq irq;
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index cc06af4fbb3e..4174e313cae5 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -263,7 +263,7 @@ static void aspeed_machine_init(MachineState *machine)
 amc->i2c_init(bmc);
 }
 
-for (i = 0; i < ARRAY_SIZE(bmc->soc.sdhci.slots); i++) {
+for (i = 0; i < bmc->soc.sdhci.num_slots; i++) {
 SDHCIState *sdhci = &bmc->soc.sdhci.slots[i];
 DriveInfo *dinfo = drive_get_next(IF_SD);
 BlockBackend *blk;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 89e4b0095041..fb73c4043ea3 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -199,6 +199,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
 sysbus_init_child_obj(obj, "sdc", OBJECT(&s->sdhci), sizeof(s->sdhci),
   TYPE_ASPEED_SDHCI);
 
+object_property_set_int(OBJECT(&s->sdhci), 2, "num-slots", &error_abort);
+
 /* Init sd card slot class here so that they're under the correct parent */
 for (i = 0; i < ASPEED_SDHCI_NUM_SLOTS; ++i) {
 sysbus_init_child_obj(obj, "sdhci[*]", OBJECT(&s->sdhci.slots[i]),
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index a6237e594017..c15ceb683950 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -209,6 +209,8 @@ static void aspeed_soc_init(Object *obj)
 sysbus_init_child_obj(obj, "sdc", OBJECT(&s->sdhci), sizeof(s->sdhci),
   TYPE_ASPEED_SDHCI);
 
+object_property_set_int(OBJECT(&s->sdhci), 2, "num-slots", &error_abort);
+
 /* Init sd card slot class here so that they're under the correct parent */
 for (i = 0; i < ASPEED_SDHCI_NUM_SLOTS; ++i) {
 sysbus_init_child_obj(obj, "sdhci[*]", OBJECT(&s->sdhci.slots[i]),
diff --git a/hw/sd/aspeed_sdhci.c b/hw/sd/aspeed_sdhci.c
index cff3eb7dd21e..939d1510dedb 100644
--- a/hw/sd/aspeed_sdhci.c
+++ b/hw/sd/aspeed_sdhci.c
@@ -13,6 +13,7 @@
 #include "qapi/error.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
 
 #define ASPEED_SDHCI_INFO0x00
 #define  ASPEED_SDHCI_INFO_RESET 0x0003
@@ -120,14 +121,14 @@ static void aspeed_sdhci_realize(DeviceState *dev, Error 
**errp)
 
 /* Create input irqs for the slots */
 qdev_init_gpio_in_named_with_opaque(DEVICE(sbd), aspeed_sdhci_set_irq,
-sdhci, NULL, ASPEED_SDHCI_NUM_SLOTS);
+sdhci, NULL, sdhci->num_slots);
 
 sysbus_init_irq(sbd, &sdhci->irq);
 memory_region_init_io(&sdhci->iomem, OBJECT(sdhci), &aspeed_sdhci_ops,
   sdhci, TYPE_ASPEED_SDHCI, 0x1000);
 sysbus_init_mmio(sbd, &sdhci->iomem);
 
-for (int i = 0; i < ASPEED_SDHCI_NUM_SLOTS; ++i) {
+for (int i = 0; i < sdhci->num_slots; ++i) {
 Object *sdhci_slot = OBJECT(&sdhci->slots[i]);
 SysBusDevice *sbd_slot = SYS_BUS_DEVICE(&sdhci->slots[i]);
 
@@ -174,6 +175,11 @@ static const VMStateDescription vmstate_aspeed_sdhci = {
 },
 };
 
+static Property aspeed_sdhci_properties[] = {
+DEFINE_PROP_UINT8("num-slots", AspeedSDHCIState, num_slots, 0),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void aspeed_sdhci_class_init(ObjectClass *classp, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(classp);
@@ -181,6 +187,7 @@ static void aspeed_sdhci_class_init(ObjectClass *classp, 
void *data)
 dc->realize = aspeed_sdhci_realize;
 dc->reset = aspeed_sdhci_reset;
 dc->vmsd = &vmstate_aspeed_sdhci;
+dc->props = aspeed_sdhci_properties;
 }
 
 static TypeInfo aspeed_sdhci_info = {
-- 
2.21.1




[PATCH v3 2/5] hw/arm: ast2600: Wire up the eMMC controller

2020-01-14 Thread Cédric Le Goater
From: Andrew Jeffery 

Initialise another SDHCI model instance for the AST2600's eMMC
controller and use the SDHCI's num_slots value introduced previously to
determine whether we should create an SD card instance for the new slot.

Signed-off-by: Andrew Jeffery 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Philippe Mathieu-Daudé 
[ clg : - removed ternary operator from sdhci_attach_drive()
- renamed SDHCI objects with a '-controller' prefix ]
Signed-off-by: Cédric Le Goater 
---
 include/hw/arm/aspeed_soc.h |  2 ++
 hw/arm/aspeed.c | 26 +-
 hw/arm/aspeed_ast2600.c | 29 ++---
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index e84380984f7b..90ac7f7ffa3b 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -57,6 +57,7 @@ typedef struct AspeedSoCState {
 AspeedGPIOState gpio;
 AspeedGPIOState gpio_1_8v;
 AspeedSDHCIState sdhci;
+AspeedSDHCIState emmc;
 } AspeedSoCState;
 
 #define TYPE_ASPEED_SOC "aspeed-soc"
@@ -126,6 +127,7 @@ enum {
 ASPEED_MII4,
 ASPEED_SDRAM,
 ASPEED_XDMA,
+ASPEED_EMMC,
 };
 
 #endif /* ASPEED_SOC_H */
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 4174e313cae5..8702256af1b2 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -171,6 +171,19 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, 
const char *flashtype,
 }
 }
 
+static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
+{
+DeviceState *card;
+
+card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"),
+   TYPE_SD_CARD);
+if (dinfo) {
+qdev_prop_set_drive(card, "drive", blk_by_legacy_dinfo(dinfo),
+&error_fatal);
+}
+object_property_set_bool(OBJECT(card), true, "realized", &error_fatal);
+}
+
 static void aspeed_machine_init(MachineState *machine)
 {
 AspeedBoardState *bmc;
@@ -264,16 +277,11 @@ static void aspeed_machine_init(MachineState *machine)
 }
 
 for (i = 0; i < bmc->soc.sdhci.num_slots; i++) {
-SDHCIState *sdhci = &bmc->soc.sdhci.slots[i];
-DriveInfo *dinfo = drive_get_next(IF_SD);
-BlockBackend *blk;
-DeviceState *card;
+sdhci_attach_drive(&bmc->soc.sdhci.slots[i], drive_get_next(IF_SD));
+}
 
-blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
-card = qdev_create(qdev_get_child_bus(DEVICE(sdhci), "sd-bus"),
-   TYPE_SD_CARD);
-qdev_prop_set_drive(card, "drive", blk, &error_fatal);
-object_property_set_bool(OBJECT(card), true, "realized", &error_fatal);
+if (bmc->soc.emmc.num_slots) {
+sdhci_attach_drive(&bmc->soc.emmc.slots[0], drive_get_next(IF_SD));
 }
 
 arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index fb73c4043ea3..90cf1c755d3d 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -46,6 +46,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 [ASPEED_ADC]   = 0x1E6E9000,
 [ASPEED_VIDEO] = 0x1E70,
 [ASPEED_SDHCI] = 0x1E74,
+[ASPEED_EMMC]  = 0x1E75,
 [ASPEED_GPIO]  = 0x1E78,
 [ASPEED_GPIO_1_8V] = 0x1E780800,
 [ASPEED_RTC]   = 0x1E781000,
@@ -64,6 +65,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
 
 #define ASPEED_SOC_AST2600_MAX_IRQ 128
 
+/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
 static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_UART1] = 47,
 [ASPEED_UART2] = 48,
@@ -77,6 +79,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
 [ASPEED_ADC]   = 78,
 [ASPEED_XDMA]  = 6,
 [ASPEED_SDHCI] = 43,
+[ASPEED_EMMC]  = 15,
 [ASPEED_GPIO]  = 40,
 [ASPEED_GPIO_1_8V] = 11,
 [ASPEED_RTC]   = 13,
@@ -196,16 +199,26 @@ static void aspeed_soc_ast2600_init(Object *obj)
 sysbus_init_child_obj(obj, "gpio_1_8v", OBJECT(&s->gpio_1_8v),
   sizeof(s->gpio_1_8v), typename);
 
-sysbus_init_child_obj(obj, "sdc", OBJECT(&s->sdhci), sizeof(s->sdhci),
-  TYPE_ASPEED_SDHCI);
+sysbus_init_child_obj(obj, "sd-controller", OBJECT(&s->sdhci),
+  sizeof(s->sdhci), TYPE_ASPEED_SDHCI);
 
 object_property_set_int(OBJECT(&s->sdhci), 2, "num-slots", &error_abort);
 
 /* Init sd card slot class here so that they're under the correct parent */
 for (i = 0; i < ASPEED_SDHCI_NUM_SLOTS; ++i) {
-sysbus_init_child_obj(obj, "sdhci[*]", OBJECT(&s->sdhci.slots[i]),
+sysbus_init_child_obj(obj, "sd-controller.sdhci[*]",
+  OBJECT(&s->sdhci.slots[i]),
   sizeof(s->sdhci.slots[i]), TYPE_SYSBUS_SDHCI);
 }
+
+sysbus_init_child_o

[PATCH v3 4/5] hw/arm/aspeed: add a 'execute-in-place' property to boot directly from CE0

2020-01-14 Thread Cédric Le Goater
The overhead for the OpenBMC firmware images using the a custom U-Boot
is around 2 seconds, which is fine, but with a U-Boot from mainline,
it takes an extra 50 seconds or so to reach Linux. A quick survey on
the number of reads performed on the flash memory region gives the
following figures :

  OpenBMC U-Boot  922478 (~ 3.5 MBytes)
  Mainline U-Boot   20569977 (~ 80  MBytes)

QEMU must be trashing the TCG TBs and reloading text very often. Some
addresses are read more than 250.000 times. Until we find a solution
to improve boot time, execution from MMIO is not activated by default.

Setting this option also breaks migration compatibility.

Signed-off-by: Cédric Le Goater 
Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
---
 include/hw/arm/aspeed.h |  2 ++
 hw/arm/aspeed.c | 44 -
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 4423cd0cda71..18521484b90e 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -19,6 +19,8 @@ typedef struct AspeedBoardState AspeedBoardState;
 
 typedef struct AspeedMachine {
 MachineState parent_obj;
+
+bool mmio_exec;
 } AspeedMachine;
 
 #define ASPEED_MACHINE_CLASS(klass) \
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 8702256af1b2..a17843f0d3bf 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -261,11 +261,18 @@ static void aspeed_machine_init(MachineState *machine)
  * SoC and 128MB for the AST2500 SoC, which is twice as big as
  * needed by the flash modules of the Aspeed machines.
  */
-memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
-   fl->size, &error_abort);
-memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
-boot_rom);
-write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
+if (ASPEED_MACHINE(machine)->mmio_exec) {
+memory_region_init_alias(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
+ &fl->mmio, 0, fl->size);
+memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
+boot_rom);
+} else {
+memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
+   fl->size, &error_abort);
+memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
+boot_rom);
+write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
+}
 }
 
 aspeed_board_binfo.ram_size = ram_size;
@@ -399,6 +406,30 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
 /* Bus 11: TODO ucd90160@64 */
 }
 
+static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
+{
+return ASPEED_MACHINE(obj)->mmio_exec;
+}
+
+static void aspeed_set_mmio_exec(Object *obj, bool value, Error **errp)
+{
+ASPEED_MACHINE(obj)->mmio_exec = value;
+}
+
+static void aspeed_machine_instance_init(Object *obj)
+{
+ASPEED_MACHINE(obj)->mmio_exec = false;
+}
+
+static void aspeed_machine_class_props_init(ObjectClass *oc)
+{
+object_class_property_add_bool(oc, "execute-in-place",
+   aspeed_get_mmio_exec,
+   aspeed_set_mmio_exec, &error_abort);
+object_class_property_set_description(oc, "execute-in-place",
+   "boot directly from CE0 flash device", 
&error_abort);
+}
+
 static void aspeed_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -408,6 +439,8 @@ static void aspeed_machine_class_init(ObjectClass *oc, void 
*data)
 mc->no_floppy = 1;
 mc->no_cdrom = 1;
 mc->no_parallel = 1;
+
+aspeed_machine_class_props_init(oc);
 }
 
 static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
@@ -550,6 +583,7 @@ static const TypeInfo aspeed_machine_types[] = {
 .name  = TYPE_ASPEED_MACHINE,
 .parent= TYPE_MACHINE,
 .instance_size = sizeof(AspeedMachine),
+.instance_init = aspeed_machine_instance_init,
 .class_size= sizeof(AspeedMachineClass),
 .class_init= aspeed_machine_class_init,
 .abstract  = true,
-- 
2.21.1




Re: [PATCH v3 08/15] block/file-posix.c: extend to use io_uring

2020-01-14 Thread Stefan Hajnoczi
On Mon, Jan 13, 2020 at 12:49:27PM +0100, Stefano Garzarella wrote:
> On Wed, Dec 18, 2019 at 04:32:21PM +, Stefan Hajnoczi wrote:
> > @@ -503,9 +504,11 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> > *options,
> >  goto fail;
> >  }
> >  
> > -aio_default = (bdrv_flags & BDRV_O_NATIVE_AIO)
> > -  ? BLOCKDEV_AIO_OPTIONS_NATIVE
> > -  : BLOCKDEV_AIO_OPTIONS_THREADS;
> > +if (bdrv_flags & BDRV_O_NATIVE_AIO) {
> > +aio_default = BLOCKDEV_AIO_OPTIONS_NATIVE;
> > +} else {
> > +aio_default = BLOCKDEV_AIO_OPTIONS_THREADS;
> > +}
> 
> This is only a cosmetic change?
...
> > @@ -578,7 +585,7 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> > *options,
> >  s->shared_perm = BLK_PERM_ALL;
> >  
> >  #ifdef CONFIG_LINUX_AIO
> > - /* Currently Linux does AIO only for files opened with O_DIRECT */
> > +/* Currently Linux does AIO only for files opened with O_DIRECT */
> 
> Also this is a not related fix, if you respin maybe we should split in a
> new patch or say something in the commit message.

Thanks, I'll remove whitespace changes and unrelated reformatting from
this patch.

> > @@ -1877,21 +1900,25 @@ static int coroutine_fn raw_co_prw(BlockDriverState 
> > *bs, uint64_t offset,
> >  return -EIO;
> >  
> >  /*
> > - * Check if the underlying device requires requests to be aligned,
> > - * and if the request we are trying to submit is aligned or not.
> > - * If this is the case tell the low-level driver that it needs
> > - * to copy the buffer.
> > + * When using O_DIRECT, the request must be aligned to be able to use
> > + * either libaio or io_uring interface. If not fail back to regular 
> > thread
> > + * pool read/write code which emulates this for us if we
> > + * set QEMU_AIO_MISALIGNED.
> >   */
> > -if (s->needs_alignment) {
> > -if (!bdrv_qiov_is_aligned(bs, qiov)) {
> > -type |= QEMU_AIO_MISALIGNED;
> > +if (s->needs_alignment && !bdrv_qiov_is_aligned(bs, qiov)) {
> > +type |= QEMU_AIO_MISALIGNED;
> > +#ifdef CONFIG_LINUX_IO_URING
> > +} else if (s->use_linux_io_uring) {
> > +LuringState *aio = 
> > aio_get_linux_io_uring(bdrv_get_aio_context(bs));
> > +assert(qiov->size == bytes);
> > +return luring_co_submit(bs, aio, s->fd, offset, qiov, type);
> > +#endif
> >  #ifdef CONFIG_LINUX_AIO
> > -} else if (s->use_linux_aio) {
> > -LinuxAioState *aio = 
> > aio_get_linux_aio(bdrv_get_aio_context(bs));
> > -assert(qiov->size == bytes);
> > -return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
> > +} else if (s->use_linux_aio) {
> 
> This code block was executed if "s->needs_alignment" was true, now we don't
> check it. Could this be a problem?

From raw_open_common():

  /* Currently Linux does AIO only for files opened with O_DIRECT */
  if (s->use_linux_aio) {
  if (!(s->open_flags & O_DIRECT)) {
  error_setg(errp, "aio=native was specified, but it requires "
   "cache.direct=on, which was not specified.");
  ret = -EINVAL;
  goto fail;

There is no change in behavior since use_linux_aio is only true when
needs_alignment is set.


signature.asc
Description: PGP signature


[PATCH v3 3/5] ftgmac100: check RX and TX buffer alignment

2020-01-14 Thread Cédric Le Goater
These buffers should be aligned on 16 bytes.

Ignore invalid RX and TX buffer addresses and log an error. All
incoming and outgoing traffic will be dropped because no valid RX or
TX descriptors will be available.

Signed-off-by: Cédric Le Goater 
---
 hw/net/ftgmac100.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 86ac25894a89..051f7b7af2d6 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -198,6 +198,8 @@ typedef struct {
 uint32_tdes3;
 } FTGMAC100Desc;
 
+#define FTGMAC100_DESC_ALIGNMENT 16
+
 /*
  * Specific RTL8211E MII Registers
  */
@@ -722,6 +724,12 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
 s->itc = value;
 break;
 case FTGMAC100_RXR_BADR: /* Ring buffer address */
+if (!QEMU_IS_ALIGNED(value, FTGMAC100_DESC_ALIGNMENT)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad RX buffer alignment 0x%"
+  HWADDR_PRIx "\n", __func__, value);
+return;
+}
+
 s->rx_ring = value;
 s->rx_descriptor = s->rx_ring;
 break;
@@ -731,6 +739,11 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
 break;
 
 case FTGMAC100_NPTXR_BADR: /* Transmit buffer address */
+if (!QEMU_IS_ALIGNED(value, FTGMAC100_DESC_ALIGNMENT)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad TX buffer alignment 0x%"
+  HWADDR_PRIx "\n", __func__, value);
+return;
+}
 s->tx_ring = value;
 s->tx_descriptor = s->tx_ring;
 break;
-- 
2.21.1




[PATCH v3 5/5] misc/pca9552: Add qom set and get

2020-01-14 Thread Cédric Le Goater
From: Joel Stanley 

Following the pattern of the work recently done with the ASPEED GPIO
model, this adds support for inspecting and modifying the PCA9552 LEDs
from the monitor.

 (qemu) qom-set  /machine/unattached/device[17] led0 on
 (qemu) qom-set  /machine/unattached/device[17] led0 off
 (qemu) qom-set  /machine/unattached/device[17] led0 pwm0
 (qemu) qom-set  /machine/unattached/device[17] led0 pwm1

Signed-off-by: Joel Stanley 
[clg: - removed the "qom-get" examples from the commit log
  - merged memory leak fixes from Joel ]
Signed-off-by: Cédric Le Goater 
---
 hw/misc/pca9552.c | 90 +++
 1 file changed, 90 insertions(+)

diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 73be28d9369c..efd961e04148 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -15,12 +15,16 @@
 #include "hw/misc/pca9552.h"
 #include "hw/misc/pca9552_regs.h"
 #include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
 
 #define PCA9552_LED_ON   0x0
 #define PCA9552_LED_OFF  0x1
 #define PCA9552_LED_PWM0 0x2
 #define PCA9552_LED_PWM1 0x3
 
+static const char *led_state[] = {"on", "off", "pwm0", "pwm1"};
+
 static uint8_t pca9552_pin_get_config(PCA9552State *s, int pin)
 {
 uint8_t reg   = PCA9552_LS0 + (pin / 4);
@@ -169,6 +173,82 @@ static int pca9552_event(I2CSlave *i2c, enum i2c_event 
event)
 return 0;
 }
 
+static void pca9552_get_led(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+PCA9552State *s = PCA9552(obj);
+int led, rc, reg;
+uint8_t state;
+
+rc = sscanf(name, "led%2d", &led);
+if (rc != 1) {
+error_setg(errp, "%s: error reading %s", __func__, name);
+return;
+}
+if (led < 0 || led > s->nr_leds) {
+error_setg(errp, "%s invalid led %s", __func__, name);
+return;
+}
+/*
+ * Get the LSx register as the qom interface should expose the device
+ * state, not the modeled 'input line' behaviour which would come from
+ * reading the INPUTx reg
+ */
+reg = PCA9552_LS0 + led / 4;
+state = (pca9552_read(s, reg) >> (led % 8)) & 0x3;
+visit_type_str(v, name, (char **)&led_state[state], errp);
+}
+
+/*
+ * Return an LED selector register value based on an existing one, with
+ * the appropriate 2-bit state value set for the given LED number (0-3).
+ */
+static inline uint8_t pca955x_ledsel(uint8_t oldval, int led_num, int state)
+{
+return (oldval & (~(0x3 << (led_num << 1 |
+((state & 0x3) << (led_num << 1));
+}
+
+static void pca9552_set_led(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+PCA9552State *s = PCA9552(obj);
+Error *local_err = NULL;
+int led, rc, reg, val;
+uint8_t state;
+char *state_str;
+
+visit_type_str(v, name, &state_str, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+rc = sscanf(name, "led%2d", &led);
+if (rc != 1) {
+error_setg(errp, "%s: error reading %s", __func__, name);
+return;
+}
+if (led < 0 || led > s->nr_leds) {
+error_setg(errp, "%s invalid led %s", __func__, name);
+return;
+}
+
+for (state = 0; state < ARRAY_SIZE(led_state); state++) {
+if (!strcmp(state_str, led_state[state])) {
+break;
+}
+}
+if (state >= ARRAY_SIZE(led_state)) {
+error_setg(errp, "%s invalid led state %s", __func__, state_str);
+return;
+}
+
+reg = PCA9552_LS0 + led / 4;
+val = pca9552_read(s, reg);
+val = pca955x_ledsel(val, led % 4, state);
+pca9552_write(s, reg, val);
+}
+
 static const VMStateDescription pca9552_vmstate = {
 .name = "PCA9552",
 .version_id = 0,
@@ -204,6 +284,7 @@ static void pca9552_reset(DeviceState *dev)
 static void pca9552_initfn(Object *obj)
 {
 PCA9552State *s = PCA9552(obj);
+int led;
 
 /* If support for the other PCA955X devices are implemented, these
  * constant values might be part of class structure describing the
@@ -211,6 +292,15 @@ static void pca9552_initfn(Object *obj)
  */
 s->max_reg = PCA9552_LS3;
 s->nr_leds = 16;
+
+for (led = 0; led < s->nr_leds; led++) {
+char *name;
+
+name = g_strdup_printf("led%d", led);
+object_property_add(obj, name, "bool", pca9552_get_led, 
pca9552_set_led,
+NULL, NULL, NULL);
+g_free(name);
+}
 }
 
 static void pca9552_class_init(ObjectClass *klass, void *data)
-- 
2.21.1




Re: [PATCH v3 04/15] block/io_uring: implements interfaces for io_uring

2020-01-14 Thread Stefan Hajnoczi
On Mon, Jan 13, 2020 at 12:24:07PM +0100, Stefano Garzarella wrote:
> On Wed, Dec 18, 2019 at 04:32:17PM +, Stefan Hajnoczi wrote:
> > From: Aarushi Mehta 
> > 
> > Aborts when sqe fails to be set as sqes cannot be returned to the
> > ring. Adds slow path for short reads for older kernels
> > 
> > Signed-off-by: Aarushi Mehta 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> > v11:
> >  * Fix git bisect compilation breakage: move trace_luring_init_state()
> >into the tracing commit.
> > v10:
> >  * Update MAINTAINERS file [Julia]
> >  * Rename MAX_EVENTS to MAX_ENTRIES [Julia]
> >  * Define ioq_submit() before callers so the prototype isn't necessary 
> > [Julia]
> >  * Declare variables at the beginning of the block in luring_init() [Julia]
> > ---
> >  MAINTAINERS |   8 +
> >  block/Makefile.objs |   3 +
> >  block/io_uring.c| 401 
> >  include/block/aio.h |  16 +-
> >  include/block/raw-aio.h |  12 ++
> >  5 files changed, 439 insertions(+), 1 deletion(-)
> >  create mode 100644 block/io_uring.c
> 
> There are few double spaces in several comment blocks, so if you need to
> respin maybe you can clean these, otherwise we can do later.

Double spaces are used throughout include/block/aio.h and generally in
QEMU.  Both a single space and double space are fine.


signature.asc
Description: PGP signature


Re: [PATCH 1/4] linux-user: Use `qemu_log' for non-strace logging

2020-01-14 Thread Alex Bennée


Josh Kunz  writes:

> This change introduces a new logging mask "LOG_USER", which is used for
> masking general user-mode logging. This change also switches all non-strace
> uses of `gemu_log' in linux-user/ to use `qemu_log_mask(LOG_USER, ...)'
> instead. This allows the user to easily log to a file, and to mask out
> these log messages if they desire.
>
> Signed-off-by: Josh Kunz 
> ---
>  include/qemu/log.h|  2 ++
>  linux-user/arm/cpu_loop.c |  5 ++--
>  linux-user/fd-trans.c | 55 +--
>  linux-user/main.c | 24 +
>  linux-user/syscall.c  | 30 -
>  linux-user/vm86.c |  3 ++-
>  util/log.c|  3 +++
>  7 files changed, 86 insertions(+), 36 deletions(-)
>
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index e0f4e40628..503e4f88d5 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -62,6 +62,8 @@ static inline bool qemu_log_separate(void)
>  #define CPU_LOG_TB_OP_IND  (1 << 16)
>  #define CPU_LOG_TB_FPU (1 << 17)
>  #define CPU_LOG_PLUGIN (1 << 18)
> +/* LOG_USER is used for some informational user-mode logging. */
> +#define LOG_USER   (1 << 19)

As Laurent said I think LOG_UNIMP is perfectly fine for stuff we haven't
done. I don't think any of the cases warrant LOG_GUEST_ERROR.
>  
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 8718d03ee2..c4f3de77db 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -60,6 +60,9 @@ unsigned long mmap_min_addr;
>  unsigned long guest_base;
>  int have_guest_base;
>  
> +/* Used to implement backwards-compatibility for user-mode logging. */
> +static bool force_user_mode_logging = true;
> +

I'm not sure want to bother with this. I know we like to avoid
regression but isn't this all debug log stuff? If we must keep it can we
invert the variable to save the initialisation.

>  /*
>   * When running 32-on-64 we should make sure we can fit all of the possible
>   * guest address space into a contiguous chunk of virtual host memory.
> @@ -399,6 +402,11 @@ static void handle_arg_abi_call0(const char *arg)
>  }
>  #endif
>  
> +static void handle_arg_no_force_user_mode_logging(const char *arg)
> +{
> +force_user_mode_logging = false;
> +}
> +
>  static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
>  
>  #ifdef CONFIG_PLUGIN
> @@ -469,6 +477,10 @@ static const struct qemu_argument arg_table[] = {
>  {"xtensa-abi-call0", "QEMU_XTENSA_ABI_CALL0", false, 
> handle_arg_abi_call0,
>   "",   "assume CALL0 Xtensa ABI"},
>  #endif
> +{"no-force-user-mode-logging", "", false,
> +  handle_arg_no_force_user_mode_logging,
> +  "",  "disable forced user-mode logging, other logging options "
> +   "will be used exactly as provided" },
>  {NULL, NULL, false, NULL, NULL, NULL}
>  };
>  
> @@ -661,6 +673,18 @@ int main(int argc, char **argv, char **envp)
>  
>  optind = parse_args(argc, argv);
>  
> +if (force_user_mode_logging) {
> +/*
> + * Backwards Compatibility: gemu_log for non-strace messages was not
> + * configurable, and was always on. Unless the user explicitly 
> disables
> + * forced LOG_USER, force LOG_USER into the mask.
> + */
> +qemu_add_log(LOG_USER);
> +}
> +
> +qemu_log_mask(LOG_USER, "--> from user\n");
> +qemu_log_mask(LOG_STRACE, "--> from strace\n");
> +

I mean we jumped through hoops to maintain backwards compatibility and
then added new output? Also LOG_STRACE doesn't exist yet.

>  if (!trace_init_backends()) {
>  exit(1);
>  }
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 171c0caef3..7e23dd6327 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1555,7 +1555,7 @@ static inline abi_long target_to_host_cmsg(struct 
> msghdr *msgh,
>   * something more intelligent than "twice the size of the
>   * target buffer we're reading from".
>   */
> -gemu_log("Host cmsg overflow\n");
> +qemu_log_mask(LOG_USER, "Host cmsg overflow\n");

I'm not sure we shouldn't just be asserting this case above. The
comments imply it is a bug on our part. The rest look like good cases
for LOG_UNIMP.

-- 
Alex Bennée



Re: Priority of -accel

2020-01-14 Thread Paolo Bonzini
On 14/01/20 09:59, Markus Armbruster wrote:
> Paolo Bonzini  writes:
> 
>> On 13/01/20 17:17, Markus Armbruster wrote:
>>> Perfect opportunity to change the default to something more useful.
>>
>> I am not sure acutally if it's that more useful, now that we have
>> sanctioned qemu-kvm as the fast alternative.
> 
> If there is a fast alternative, why ship the slow one?

I find it more consistent that qemu-system-* is doing emulation (and can
usually be ignored) and qemu-kvm is doing virtualization.  It's more
intuitive to launch qemu-system-x86_64 than "qemu-kvm --no-kvm".

What we could do is automatically install a qemu-kvm binary for the
"most suitable" target that has KVM enabled (i.e. for
qemu-system-x86_64, not qemu-system-i386) instead of leaving it to distros.

Paolo

> No matter what we do, somebody is going to be confused.  How to resolve
> such a conundrum?  Utilitarian philosophy teaches us to pursue the
> greatest confusion of the greatest numbers.  I think not using x86
> hardware virtualization by default has been admirably successful there.




Re: [PATCH v2] hw/usb: Introduce Kconfig switches for the CCID card devices

2020-01-14 Thread Thomas Huth
On 13/01/2020 10.44, Gerd Hoffmann wrote:
> On Tue, Jan 07, 2020 at 08:36:07AM +0100, Gerd Hoffmann wrote:
>> On Wed, Dec 11, 2019 at 11:20:29AM +0100, Thomas Huth wrote:
>>> In our downstream distribution of QEMU, we need more fine-grained
>>> control on the set of CCID card devices that we want to include.
>>> So let's introduce some proper Kconfig switches that it is easier
>>> to disable them without modifying the corresponding Makefile.objs.
>>>
>>> Signed-off-by: Thomas Huth 
>>
>> Added to usb queue.
> 
> Oops, this patch breaks "make check" on openbsd (make
> TARGET_LIST=x86_64-softmmu vm-build-openbsd).  Unqueued.

Sorry, I missed that tests/qtest/usb-hcd-xhci-test.c uses the "usb-ccid"
device ... I'll fix my patch and send a v3.

 Thomas




Re: [PATCH 2/4] linux-user: Use `qemu_log' for strace

2020-01-14 Thread Alex Bennée


Josh Kunz  writes:

> This change switches linux-user strace logging to use the newer `qemu_log`
> logging subsystem rather than the older `gemu_log` (notice the "g")
> logger. `qemu_log` has several advantages, namely that it allows logging
> to a file, and provides a more unified interface for configuration
> of logging (via the QEMU_LOG environment variable or options).
>
> Signed-off-by: Josh Kunz 
> ---
>  include/qemu/log.h   |  13 ++
>  linux-user/main.c|  17 +-
>  linux-user/qemu.h|   1 -
>  linux-user/signal.c  |   3 +-
>  linux-user/strace.c  | 479 ++-
>  linux-user/syscall.c |  13 +-
>  util/log.c   |   2 +
>  7 files changed, 282 insertions(+), 246 deletions(-)
>
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index 503e4f88d5..8f044c1716 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -64,6 +64,7 @@ static inline bool qemu_log_separate(void)
>  #define CPU_LOG_PLUGIN (1 << 18)
>  /* LOG_USER is used for some informational user-mode logging. */
>  #define LOG_USER   (1 << 19)
> +#define LOG_STRACE (1 << 20)
>
>  /* Lock output for a series of related logs.  Since this is not needed
>   * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we
> @@ -154,6 +155,18 @@ void qemu_set_dfilter_ranges(const char *ranges, Error 
> **errp);
>  bool qemu_log_in_addr_range(uint64_t addr);
>  int qemu_str_to_log_mask(const char *str);
>
> +/* Add (union) the given "log_flags" to the current log mask. */
> +static inline void qemu_add_log(int log_flags)
> +{
> +qemu_set_log(qemu_loglevel | log_flags);
> +}
> +
> +/* Remove (subtract) the given log flags from the current log mask. */
> +static inline void qemu_del_log(int log_flags)
> +{
> +qemu_set_log(qemu_loglevel & ~(log_flags));
> +}
> +
>  /* Print a usage message listing all the valid logging categories
>   * to the specified FILE*.
>   */
> diff --git a/linux-user/main.c b/linux-user/main.c
> index c4f3de77db..0bf40c4d27 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -63,6 +63,12 @@ int have_guest_base;
>  /* Used to implement backwards-compatibility for user-mode logging. */
>  static bool force_user_mode_logging = true;
>
> +/*
> + * Used to implement backwards-compatibility for the `-strace`, and
> + * QEMU_STRACE options.
> + */
> +static bool enable_strace;
> +
>  /*
>   * When running 32-on-64 we should make sure we can fit all of the possible
>   * guest address space into a contiguous chunk of virtual host memory.
> @@ -378,7 +384,7 @@ static void handle_arg_singlestep(const char *arg)
>
>  static void handle_arg_strace(const char *arg)
>  {
> -do_strace = 1;
> +enable_strace = true;

Could we cut out the middle-man and just qemu_add_log(LOG_STRACE) here
and drop the enable_strace variable.

>  }
>
>  static void handle_arg_version(const char *arg)
> @@ -672,6 +678,15 @@ int main(int argc, char **argv, char **envp)
>  qemu_plugin_add_opts();
>
>  optind = parse_args(argc, argv);
> +/*
> + * Backwards Compatability: If handle_arg_strace just enabled strace
> + * logging directly, then it could be accidentally turned off by a
> + * QEMU_LOG/-d option. To make sure that strace logging is always enabled
> + * when QEMU_STRACE/-strace is set, re-enable LOG_STRACE here.
> + */
> +if (enable_strace) {
> +qemu_add_log(LOG_STRACE);
> +}
>
>  if (force_user_mode_logging) {
>  /*
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index f6f5fe5fbb..02c6890c8a 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -385,7 +385,6 @@ void print_syscall_ret(int num, abi_long arg1);
>   * --- SIGSEGV {si_signo=SIGSEGV, si_code=SI_KERNEL, si_addr=0} ---
>   */
>  void print_taken_signal(int target_signum, const target_siginfo_t *tinfo);
> -extern int do_strace;
>
>  /* signal.c */
>  void process_pending_signals(CPUArchState *cpu_env);
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 5ca6d62b15..2ff0065804 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -864,9 +864,8 @@ static void handle_pending_signal(CPUArchState *cpu_env, 
> int sig,
>  handler = sa->_sa_handler;
>  }
>
> -if (do_strace) {
> +if (unlikely(qemu_loglevel_mask(LOG_STRACE)))
>  print_taken_signal(sig, &k->info);
> -}

Please don't drop the brace - c.f. CODING_STYLE.rst

Side note, print_taken_signal might want to consider using the
qemu_log_lock() functionality to keep sequential qemu_log's together. I
suspect you might want to do the same for syscalls.

--
Alex Bennée



Re: [PATCH 3/4] linux-user: remove gemu_log from the linux-user tree

2020-01-14 Thread Alex Bennée


Josh Kunz  writes:

> Now that all uses have been migrated to `qemu_log' it is no longer
> needed.
>
> Signed-off-by: Josh Kunz 

Reviewed-by: Alex Bennée 

> ---
>  linux-user/main.c | 9 -
>  linux-user/qemu.h | 1 -
>  2 files changed, 10 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 0bf40c4d27..945b6adf3a 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -108,15 +108,6 @@ const char *qemu_uname_release;
> by remapping the process stack directly at the right place */
>  unsigned long guest_stack_size = 8 * 1024 * 1024UL;
>  
> -void gemu_log(const char *fmt, ...)
> -{
> -va_list ap;
> -
> -va_start(ap, fmt);
> -vfprintf(stderr, fmt, ap);
> -va_end(ap);
> -}
> -
>  #if defined(TARGET_I386)
>  int cpu_get_pic_interrupt(CPUX86State *env)
>  {
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 02c6890c8a..329b409e65 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -210,7 +210,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  abi_long arg2, abi_long arg3, abi_long arg4,
>  abi_long arg5, abi_long arg6, abi_long arg7,
>  abi_long arg8);
> -void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  extern __thread CPUState *thread_cpu;
>  void cpu_loop(CPUArchState *env);
>  const char *target_strerror(int err);


-- 
Alex Bennée



[PATCH v4 02/15] qapi/block-core: add option for io_uring

2020-01-14 Thread Stefan Hajnoczi
From: Aarushi Mehta 

Since io_uring is the actual name of the Linux API, we use it as enum
value even though the QAPI schema conventions would prefer io-uring.

Signed-off-by: Aarushi Mehta 
Acked-by: Markus Armbruster 
Acked-by: Stefano Garzarella 
Signed-off-by: Stefan Hajnoczi 
---
v12:
 * Reword commit description [Markus]
 * Update version from 4.2 to 5.0
---
 qapi/block-core.json | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ff5e5edaf..ef94a29686 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2851,11 +2851,13 @@
 #
 # @threads: Use qemu's thread pool
 # @native:  Use native AIO backend (only Linux and Windows)
+# @io_uring:Use linux io_uring (since 5.0)
 #
 # Since: 2.9
 ##
 { 'enum': 'BlockdevAioOptions',
-  'data': [ 'threads', 'native' ] }
+  'data': [ 'threads', 'native',
+{ 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' } ] }
 
 ##
 # @BlockdevCacheOptions:
-- 
2.24.1




[PATCH v4 00/15] io_uring: add Linux io_uring AIO engine

2020-01-14 Thread Stefan Hajnoczi
v13:
 * Drop unnecessary changes in Patch 8 [Stefano]

v12:
 * Reword BlockdevAioOptions QAPI schema commit description [Markus]
 * Increase QAPI "Since: 4.2" to "Since: 5.0"
 * Explain rationale for io_uring stubs in commit description [Kevin]
 * Tried to use file.aio=io_uring instead of BDRV_O_IO_URING but it's really
   hard to make qemu-iotests work.  Tests build blkdebug: and other graphs so
   the syntax for io_uring is dependent on the test case.  I scrapped this
   approach and went back to a global flag.

v11:
 * Drop fd registration because it breaks QEMU's file locking and will need to
   be resolved in a separate patch series
 * Drop line-wrapping changes that accidentally broke several qemu-iotests

v10:
 * Dropped kernel submission queue polling, it requires root and has additional
   limitations.  It should be benchmarked and considered for inclusion later,
   maybe even together with kernel side changes.
 * Add io_uring_register_files() return value to trace_luring_fd_register()
 * Fix indentation in luring_fd_unregister()
 * Set s->fd_reg.fd_array to NULL after g_free() to avoid dangling pointers
 * Simplify fd registration code
 * Add luring_fd_unregister() and call it from file-posix.c to prevent
   fd leaks
 * Add trace_luring_fd_unregister() trace event
 * Add missing space to qemu-img command-line documentation
 * Update MAINTAINERS file [Julia]
 * Rename MAX_EVENTS to MAX_ENTRIES [Julia]
 * Define ioq_submit() before callers so the prototype isn't necessary [Julia]
 * Declare variables at the beginning of the block in luring_init() [Julia]

This patch series is based on Aarushi Mehta's v9 patch series written for
Google Summer of Code 2019:

  https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg00179.html

It adds a new AIO engine that uses the new Linux io_uring API.  This is the
successor to Linux AIO with a number of improvements:
1. Both O_DIRECT and buffered I/O work
2. fdatasync(2) is supported (no need for a separate thread pool!)
3. True async behavior so the syscall doesn't block (Linux AIO got there to 
some degree...)
4. Advanced performance optimizations are available (file registration, memory
   buffer registration, completion polling, submission polling).

Since Aarushi has been busy, I have taken up this patch series.  Booting a
guest works with -drive aio=io_uring and -drive aio=io_uring,cache=none with a
raw file on XFS.

I currently recommend using -drive aio=io_uring only with host block devices
(like NVMe devices).  As of Linux v5.4-rc1 I still hit kernel bugs when using
image files on ext4 or XFS.

Aarushi Mehta (15):
  configure: permit use of io_uring
  qapi/block-core: add option for io_uring
  block/block: add BDRV flag for io_uring
  block/io_uring: implements interfaces for io_uring
  stubs: add stubs for io_uring interface
  util/async: add aio interfaces for io_uring
  blockdev: adds bdrv_parse_aio to use io_uring
  block/file-posix.c: extend to use io_uring
  block: add trace events for io_uring
  block/io_uring: adds userspace completion polling
  qemu-io: adds option to use aio engine
  qemu-img: adds option to use aio engine for benchmarking
  qemu-nbd: adds option for aio engines
  tests/qemu-iotests: enable testing with aio options
  tests/qemu-iotests: use AIOMODE with various tests

 MAINTAINERS   |   9 +
 block.c   |  22 ++
 block/Makefile.objs   |   3 +
 block/file-posix.c|  85 +--
 block/io_uring.c  | 433 ++
 block/trace-events|  12 +
 blockdev.c|  12 +-
 configure |  27 +++
 include/block/aio.h   |  16 +-
 include/block/block.h |   2 +
 include/block/raw-aio.h   |  12 +
 qapi/block-core.json  |   4 +-
 qemu-img-cmds.hx  |   4 +-
 qemu-img.c|  11 +-
 qemu-img.texi |   5 +-
 qemu-io.c |  25 +-
 qemu-nbd.c|  12 +-
 qemu-nbd.texi |   4 +-
 stubs/Makefile.objs   |   1 +
 stubs/io_uring.c  |  32 +++
 tests/qemu-iotests/028|   2 +-
 tests/qemu-iotests/058|   2 +-
 tests/qemu-iotests/089|   4 +-
 tests/qemu-iotests/091|   4 +-
 tests/qemu-iotests/109|   2 +-
 tests/qemu-iotests/147|   5 +-
 tests/qemu-iotests/181|   8 +-
 tests/qemu-iotests/183|   4 +-
 tests/qemu-iotests/185|  10 +-
 tests/qemu-iotests/200|   2 +-
 tests/qemu-iotests/201|   8 +-
 tests/qemu-iotests/check  |  15 +-
 tests/qemu-iotests/common.rc  |  14 ++
 tests/qemu-iotests/iotests.py |  12 +-
 util/async.c  |  36 +++
 35 files changed, 787 insertions(+), 72 deletions(-)
 create mode 100644 block/io_uring.c
 create mode 100644 stubs/io_uring.c

-- 
2.24.1




[PATCH v4 01/15] configure: permit use of io_uring

2020-01-14 Thread Stefan Hajnoczi
From: Aarushi Mehta 

Signed-off-by: Aarushi Mehta 
Reviewed-by: Maxim Levitsky 
Acked-by: Stefano Garzarella 
Signed-off-by: Stefan Hajnoczi 
---
 configure | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/configure b/configure
index 08c3a1c1f0..ca0af11db0 100755
--- a/configure
+++ b/configure
@@ -371,6 +371,7 @@ xen=""
 xen_ctrl_version=""
 xen_pci_passthrough=""
 linux_aio=""
+linux_io_uring=""
 cap_ng=""
 attr=""
 libattr=""
@@ -1253,6 +1254,10 @@ for opt do
   ;;
   --enable-linux-aio) linux_aio="yes"
   ;;
+  --disable-linux-io-uring) linux_io_uring="no"
+  ;;
+  --enable-linux-io-uring) linux_io_uring="yes"
+  ;;
   --disable-attr) attr="no"
   ;;
   --enable-attr) attr="yes"
@@ -1773,6 +1778,7 @@ disabled with --disable-FEATURE, default is enabled if 
available:
   vde support for vde network
   netmap  support for netmap network
   linux-aio   Linux AIO support
+  linux-io-uring  Linux io_uring support
   cap-ng  libcap-ng support
   attrattr and xattr support
   vhost-net   vhost-net kernel acceleration support
@@ -4004,6 +4010,21 @@ EOF
 linux_aio=no
   fi
 fi
+##
+# linux-io-uring probe
+
+if test "$linux_io_uring" != "no" ; then
+  if $pkg_config liburing; then
+linux_io_uring_cflags=$($pkg_config --cflags liburing)
+linux_io_uring_libs=$($pkg_config --libs liburing)
+linux_io_uring=yes
+  else
+if test "$linux_io_uring" = "yes" ; then
+  feature_not_found "linux io_uring" "Install liburing devel"
+fi
+linux_io_uring=no
+  fi
+fi
 
 ##
 # TPM emulation is only on POSIX
@@ -6492,6 +6513,7 @@ echo "PIE   $pie"
 echo "vde support   $vde"
 echo "netmap support$netmap"
 echo "Linux AIO support $linux_aio"
+echo "Linux io_uring support $linux_io_uring"
 echo "ATTR/XATTR support $attr"
 echo "Install blobs $blobs"
 echo "KVM support   $kvm"
@@ -6972,6 +6994,11 @@ fi
 if test "$linux_aio" = "yes" ; then
   echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
 fi
+if test "$linux_io_uring" = "yes" ; then
+  echo "CONFIG_LINUX_IO_URING=y" >> $config_host_mak
+  echo "LINUX_IO_URING_CFLAGS=$linux_io_uring_cflags" >> $config_host_mak
+  echo "LINUX_IO_URING_LIBS=$linux_io_uring_libs" >> $config_host_mak
+fi
 if test "$attr" = "yes" ; then
   echo "CONFIG_ATTR=y" >> $config_host_mak
 fi
-- 
2.24.1




[PATCH v4 03/15] block/block: add BDRV flag for io_uring

2020-01-14 Thread Stefan Hajnoczi
From: Aarushi Mehta 

Signed-off-by: Aarushi Mehta 
Reviewed-by: Maxim Levitsky 
Acked-by: Stefano Garzarella 
Signed-off-by: Stefan Hajnoczi 
---
 include/block/block.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/block/block.h b/include/block/block.h
index e9dcfef7fa..ea155da45a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -126,6 +126,7 @@ typedef struct HDGeometry {
   ignoring the format layer */
 #define BDRV_O_NO_IO   0x1 /* don't initialize for I/O */
 #define BDRV_O_AUTO_RDONLY 0x2 /* degrade to read-only if opening 
read-write fails */
+#define BDRV_O_IO_URING0x4 /* use io_uring instead of the thread pool 
*/
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
 
-- 
2.24.1




[PATCH v4 04/15] block/io_uring: implements interfaces for io_uring

2020-01-14 Thread Stefan Hajnoczi
From: Aarushi Mehta 

Aborts when sqe fails to be set as sqes cannot be returned to the
ring. Adds slow path for short reads for older kernels

Signed-off-by: Aarushi Mehta 
Acked-by: Stefano Garzarella 
Signed-off-by: Stefan Hajnoczi 
---
v11:
 * Fix git bisect compilation breakage: move trace_luring_init_state()
   into the tracing commit.
v10:
 * Update MAINTAINERS file [Julia]
 * Rename MAX_EVENTS to MAX_ENTRIES [Julia]
 * Define ioq_submit() before callers so the prototype isn't necessary [Julia]
 * Declare variables at the beginning of the block in luring_init() [Julia]
---
 MAINTAINERS |   8 +
 block/Makefile.objs |   3 +
 block/io_uring.c| 401 
 include/block/aio.h |  16 +-
 include/block/raw-aio.h |  12 ++
 5 files changed, 439 insertions(+), 1 deletion(-)
 create mode 100644 block/io_uring.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 483edfbc0b..dc9da76b84 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2606,6 +2606,14 @@ F: block/file-posix.c
 F: block/file-win32.c
 F: block/win32-aio.c
 
+Linux io_uring
+M: Aarushi Mehta 
+M: Julia Suvorova 
+M: Stefan Hajnoczi 
+L: qemu-bl...@nongnu.org
+S: Maintained
+F: block/io_uring.c
+
 qcow2
 M: Kevin Wolf 
 M: Max Reitz 
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 330529b0b7..3bcb35c81d 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -18,6 +18,7 @@ block-obj-y += block-backend.o snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += file-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+block-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
 block-obj-y += null.o mirror.o commit.o io.o create.o
 block-obj-y += throttle-groups.o
 block-obj-$(CONFIG_LINUX) += nvme.o
@@ -66,5 +67,7 @@ block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o
 dmg-lzfse.o-libs   := $(LZFSE_LIBS)
 qcow.o-libs:= -lz
 linux-aio.o-libs   := -laio
+io_uring.o-cflags  := $(LINUX_IO_URING_CFLAGS)
+io_uring.o-libs:= $(LINUX_IO_URING_LIBS)
 parallels.o-cflags := $(LIBXML2_CFLAGS)
 parallels.o-libs   := $(LIBXML2_LIBS)
diff --git a/block/io_uring.c b/block/io_uring.c
new file mode 100644
index 00..bb433a685b
--- /dev/null
+++ b/block/io_uring.c
@@ -0,0 +1,401 @@
+/*
+ * Linux io_uring support.
+ *
+ * Copyright (C) 2009 IBM, Corp.
+ * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2019 Aarushi Mehta
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include 
+#include "qemu-common.h"
+#include "block/aio.h"
+#include "qemu/queue.h"
+#include "block/block.h"
+#include "block/raw-aio.h"
+#include "qemu/coroutine.h"
+#include "qapi/error.h"
+
+/* io_uring ring size */
+#define MAX_ENTRIES 128
+
+typedef struct LuringAIOCB {
+Coroutine *co;
+struct io_uring_sqe sqeq;
+ssize_t ret;
+QEMUIOVector *qiov;
+bool is_read;
+QSIMPLEQ_ENTRY(LuringAIOCB) next;
+
+/*
+ * Buffered reads may require resubmission, see
+ * luring_resubmit_short_read().
+ */
+int total_read;
+QEMUIOVector resubmit_qiov;
+} LuringAIOCB;
+
+typedef struct LuringQueue {
+int plugged;
+unsigned int in_queue;
+unsigned int in_flight;
+bool blocked;
+QSIMPLEQ_HEAD(, LuringAIOCB) submit_queue;
+} LuringQueue;
+
+typedef struct LuringState {
+AioContext *aio_context;
+
+struct io_uring ring;
+
+/* io queue for submit at batch.  Protected by AioContext lock. */
+LuringQueue io_q;
+
+/* I/O completion processing.  Only runs in I/O thread.  */
+QEMUBH *completion_bh;
+} LuringState;
+
+/**
+ * luring_resubmit:
+ *
+ * Resubmit a request by appending it to submit_queue.  The caller must ensure
+ * that ioq_submit() is called later so that submit_queue requests are started.
+ */
+static void luring_resubmit(LuringState *s, LuringAIOCB *luringcb)
+{
+QSIMPLEQ_INSERT_TAIL(&s->io_q.submit_queue, luringcb, next);
+s->io_q.in_queue++;
+}
+
+/**
+ * luring_resubmit_short_read:
+ *
+ * Before Linux commit 9d93a3f5a0c ("io_uring: punt short reads to async
+ * context") a buffered I/O request with the start of the file range in the
+ * page cache could result in a short read.  Applications need to resubmit the
+ * remaining read request.
+ *
+ * This is a slow path but recent kernels never take it.
+ */
+static void luring_resubmit_short_read(LuringState *s, LuringAIOCB *luringcb,
+   int nread)
+{
+QEMUIOVector *resubmit_qiov;
+size_t remaining;
+
+/* Update read position */
+luringcb->total_read = nread;
+remaining = luringcb->qiov->size - luringcb->total_read;
+
+/* Shorten qiov */
+resubmit_qiov = &luringcb->resubmit_qiov;
+if (resubmit_qiov->iov == NULL) {
+qemu_iovec_init(resubmit_qiov, luringcb->qiov->niov);
+} else {
+qemu_iovec_reset(resubmit_qiov);
+}
+qemu_iovec_

[PATCH v4 05/15] stubs: add stubs for io_uring interface

2020-01-14 Thread Stefan Hajnoczi
From: Aarushi Mehta 

Follow linux-aio.o and stub out the block/io_uring.o APIs that will be
missing when a binary is linked with obj-util-y but without
block-util-y (e.g. vhost-user-gpu).

For example, the stubs are necessary so that a binary using util/async.o
from obj-util-y for qemu_bh_new() links successfully.  In this case
block/io_uring.o from block-util-y isn't needed and we can avoid
dragging in the block layer by linking the stubs instead.  The stub
functions never get called.

Signed-off-by: Aarushi Mehta 
Acked-by: Stefano Garzarella 
Signed-off-by: Stefan Hajnoczi 
---
 MAINTAINERS |  1 +
 stubs/Makefile.objs |  1 +
 stubs/io_uring.c| 32 
 3 files changed, 34 insertions(+)
 create mode 100644 stubs/io_uring.c

diff --git a/MAINTAINERS b/MAINTAINERS
index dc9da76b84..ab15b76912 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2613,6 +2613,7 @@ M: Stefan Hajnoczi 
 L: qemu-bl...@nongnu.org
 S: Maintained
 F: block/io_uring.c
+F: stubs/io_uring.c
 
 qcow2
 M: Kevin Wolf 
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8b0ff25508..7afbe5fb61 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -13,6 +13,7 @@ stub-obj-y += iothread.o
 stub-obj-y += iothread-lock.o
 stub-obj-y += is-daemonized.o
 stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
 stub-obj-y += machine-init-done.o
 stub-obj-y += migr-blocker.o
 stub-obj-y += change-state-handler.o
diff --git a/stubs/io_uring.c b/stubs/io_uring.c
new file mode 100644
index 00..622d1e4648
--- /dev/null
+++ b/stubs/io_uring.c
@@ -0,0 +1,32 @@
+/*
+ * Linux io_uring support.
+ *
+ * Copyright (C) 2009 IBM, Corp.
+ * Copyright (C) 2009 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "block/aio.h"
+#include "block/raw-aio.h"
+
+void luring_detach_aio_context(LuringState *s, AioContext *old_context)
+{
+abort();
+}
+
+void luring_attach_aio_context(LuringState *s, AioContext *new_context)
+{
+abort();
+}
+
+LuringState *luring_init(Error **errp)
+{
+abort();
+}
+
+void luring_cleanup(LuringState *s)
+{
+abort();
+}
-- 
2.24.1




[PATCH] target/arm: add PMU feature to cortex-r5 and cortex-r5f

2020-01-14 Thread Clement Deschamps
Signed-off-by: Clement Deschamps 
---
See cortex-r5 TRM - 1.3 Features

PMU is not optional on cortex-r5 and cortex-r5f
---
 target/arm/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index d62fd5fdc6..64cd0a7d73 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2121,6 +2121,7 @@ static void cortex_r5_initfn(Object *obj)
 set_feature(&cpu->env, ARM_FEATURE_V7);
 set_feature(&cpu->env, ARM_FEATURE_V7MP);
 set_feature(&cpu->env, ARM_FEATURE_PMSA);
+set_feature(&cpu->env, ARM_FEATURE_PMU);
 cpu->midr = 0x411fc153; /* r1p3 */
 cpu->id_pfr0 = 0x0131;
 cpu->id_pfr1 = 0x001;
-- 
2.24.1




  1   2   3   4   5   >