[Qemu-devel] [PATCH v2] Virt: ACPI: fix qemu assert due to re-assigned table data address

[Shannon Zhao]

From: Zhaoshenglong 

acpi_data_push uses g_array_set_size to resize the memory size. If there
is no enough contiguous memory, the address will be changed. If we use
the old value, it will assert.
qemu-kvm: hw/acpi/bios-linker-loader.c:214: bios_linker_loader_add_checksum:
Assertion `start_offset < file->blob->len' failed.`

This issue only happens in building SRAT table now but here we unify the
pattern for other tables as well to avoid possible issues in the future.

Signed-off-by: Zhaoshenglong 
---
v2: update the commit message according to Andrew's comments
---
 hw/arm/virt-acpi-build.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 3d78ff6..5901142 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -453,6 +453,7 @@ build_spcr(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 AcpiSerialPortConsoleRedirection *spcr;
 const MemMapEntry *uart_memmap = &vms->memmap[VIRT_UART];
 int irq = vms->irqmap[VIRT_UART] + ARM_SPI_BASE;
+int spcr_start = table_data->len;
 
 spcr = acpi_data_push(table_data, sizeof(*spcr));
 
@@ -476,8 +477,8 @@ build_spcr(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 spcr->pci_device_id = 0x;  /* PCI Device ID: not a PCI device */
 spcr->pci_vendor_id = 0x;  /* PCI Vendor ID: not a PCI device */
 
-build_header(linker, table_data, (void *)spcr, "SPCR", sizeof(*spcr), 2,
- NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + spcr_start),
+ "SPCR", table_data->len - spcr_start, 2, NULL, NULL);
 }
 
 static void
@@ -512,8 +513,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 mem_base += numa_info[i].node_mem;
 }
 
-build_header(linker, table_data, (void *)srat, "SRAT",
- table_data->len - srat_start, 3, NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + srat_start),
+ "SRAT", table_data->len - srat_start, 3, NULL, NULL);
 }
 
 static void
@@ -522,6 +523,7 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 AcpiTableMcfg *mcfg;
 const MemMapEntry *memmap = vms->memmap;
 int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
+int mcfg_start = table_data->len;
 
 mcfg = acpi_data_push(table_data, len);
 mcfg->allocation[0].address = cpu_to_le64(memmap[VIRT_PCIE_ECAM].base);
@@ -532,7 +534,8 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 mcfg->allocation[0].end_bus_number = (memmap[VIRT_PCIE_ECAM].size
   / PCIE_MMCFG_SIZE_MIN) - 1;
 
-build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
+ "MCFG", len, 1, NULL, NULL);
 }
 
 /* GTDT */
@@ -651,6 +654,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 static void build_fadt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms, unsigned dsdt_tbl_offset)
 {
+int fadt_start = table_data->len;
 AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
 unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
 uint16_t bootflags;
@@ -681,8 +685,8 @@ static void build_fadt(GArray *table_data, BIOSLinker 
*linker,
 ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
 ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-build_header(linker, table_data,
- (void *)fadt, "FACP", sizeof(*fadt), 5, NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + fadt_start),
+ "FACP", table_data->len - fadt_start, 5, NULL, NULL);
 }
 
 /* DSDT */
-- 
2.0.4

Re: [Qemu-devel] [PATCH v2] Virt: ACPI: fix qemu assert due to re-assigned table data address

[Andrew Jones]

On Wed, Dec 27, 2017 at 04:16:12PM +0800, Shannon Zhao wrote:
> From: Zhaoshenglong 
> 
> acpi_data_push uses g_array_set_size to resize the memory size. If there
> is no enough contiguous memory, the address will be changed. If we use
> the old value, it will assert.
> qemu-kvm: hw/acpi/bios-linker-loader.c:214: bios_linker_loader_add_checksum:
> Assertion `start_offset < file->blob->len' failed.`
> 
> This issue only happens in building SRAT table now but here we unify the
> pattern for other tables as well to avoid possible issues in the future.
> 
> Signed-off-by: Zhaoshenglong 
> ---
> v2: update the commit message according to Andrew's comments
> ---
>  hw/arm/virt-acpi-build.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 3d78ff6..5901142 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -453,6 +453,7 @@ build_spcr(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  AcpiSerialPortConsoleRedirection *spcr;
>  const MemMapEntry *uart_memmap = &vms->memmap[VIRT_UART];
>  int irq = vms->irqmap[VIRT_UART] + ARM_SPI_BASE;
> +int spcr_start = table_data->len;
>  
>  spcr = acpi_data_push(table_data, sizeof(*spcr));
>  
> @@ -476,8 +477,8 @@ build_spcr(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  spcr->pci_device_id = 0x;  /* PCI Device ID: not a PCI device */
>  spcr->pci_vendor_id = 0x;  /* PCI Vendor ID: not a PCI device */
>  
> -build_header(linker, table_data, (void *)spcr, "SPCR", sizeof(*spcr), 2,
> - NULL, NULL);
> +build_header(linker, table_data, (void *)(table_data->data + spcr_start),
> + "SPCR", table_data->len - spcr_start, 2, NULL, NULL);
>  }
>  
>  static void
> @@ -512,8 +513,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  mem_base += numa_info[i].node_mem;
>  }
>  
> -build_header(linker, table_data, (void *)srat, "SRAT",
> - table_data->len - srat_start, 3, NULL, NULL);
> +build_header(linker, table_data, (void *)(table_data->data + srat_start),
> + "SRAT", table_data->len - srat_start, 3, NULL, NULL);
>  }
>  
>  static void
> @@ -522,6 +523,7 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  AcpiTableMcfg *mcfg;
>  const MemMapEntry *memmap = vms->memmap;
>  int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
> +int mcfg_start = table_data->len;
>  
>  mcfg = acpi_data_push(table_data, len);
>  mcfg->allocation[0].address = cpu_to_le64(memmap[VIRT_PCIE_ECAM].base);
> @@ -532,7 +534,8 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  mcfg->allocation[0].end_bus_number = (memmap[VIRT_PCIE_ECAM].size
>/ PCIE_MMCFG_SIZE_MIN) - 1;
>  
> -build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, 
> NULL);
> +build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
> + "MCFG", len, 1, NULL, NULL);

If we're going to change all of them for the new pattern, then len should
be changed here to table_data->len - mcfg_start

>  }
>  
>  /* GTDT */
> @@ -651,6 +654,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  static void build_fadt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms, unsigned dsdt_tbl_offset)
>  {
> +int fadt_start = table_data->len;
>  AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, 
> sizeof(*fadt));
>  unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
>  uint16_t bootflags;
> @@ -681,8 +685,8 @@ static void build_fadt(GArray *table_data, BIOSLinker 
> *linker,
>  ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
>  ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
>  
> -build_header(linker, table_data,
> - (void *)fadt, "FACP", sizeof(*fadt), 5, NULL, NULL);
> +build_header(linker, table_data, (void *)(table_data->data + fadt_start),
> + "FACP", table_data->len - fadt_start, 5, NULL, NULL);
>  }
>  
>  /* DSDT */
> -- 
> 2.0.4
> 
> 
>

Thanks,
drew 

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Juan Quintela]

Peter Xu  wrote:
> On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
>> Peter Xu  wrote:
>> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
>> >> We now test the deprecated commands everytime that we test the new
>> >> commands.  This makes unnecesary to add tests for deprecated commands.
>> >> 
>> >> Signed-off-by: Juan Quintela 
>> >> ---
>> >>  tests/migration-test.c | 32 
>> >>  1 file changed, 28 insertions(+), 4 deletions(-)
>> >> 
>> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> >> index 799e24ebc6..51f49c74e9 100644
>> >> --- a/tests/migration-test.c
>> >> +++ b/tests/migration-test.c
>> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, 
>> >> const char *parameter,
>> >>  QDECREF(rsp);
>> >>  }
>> >>  
>> >> -static void migrate_set_downtime(QTestState *who, const double value)
>> >> +static void deprecated_set_downtime(QTestState *who, const double value)
>> >>  {
>> >>  QDict *rsp;
>> >>  gchar *cmd;
>> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, 
>> >> const double value)
>> >>  g_free(expected);
>> >>  }
>> >>  
>> >> -static void migrate_set_speed(QTestState *who, const char *value)
>> >> +static void deprecated_set_speed(QTestState *who, const char *value)
>> >>  {
>> >>  QDict *rsp;
>> >>  gchar *cmd;
>> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const 
>> >> char *value)
>> >>  migrate_check_parameter(who, "max-bandwidth", value);
>> >>  }
>> >>  
>> >> +static void migrate_set_parameter(QTestState *who, const char *parameter,
>> >> +  const char *value)
>> >> +{
>> >> +QDict *rsp;
>> >> +gchar *cmd;
>> >> +
>> >> +if (strcmp(parameter, "downtime-limit") == 0) {
>> >> +deprecated_set_downtime(who, 0.12345);
>> >> +}
>> >> +
>> >> +if (strcmp(parameter, "max-bandwidth") == 0) {
>> >> +deprecated_set_speed(who, "12345");
>> >> +}
>> >
>> > I'm fine with current approach, but I would really prefer to put them
>> > all into a standalone test, by just calling them one by one with some
>> > specific numbers and that's all.
>> 
>> That means another test (at least), and we have, also at least three
>> deprecated comands:
>> - migrate_set_speed
>> - migrate_set_downtime
>> - migrate_set_cache_size
>> 
>> And each test makes things slower.  So I *thought* it would we wiser to
>> just check _always_ use the deprecated an the not deprecated one.
>> 
>> > (luckily we only have two deprecated commands and limited tests,
>> >  otherwise extra commands will be M*N, say "number of deprecated
>> >  command" * "number of test mirations")
>> 
>> Each test takes time, so adding tests make everything much slower.
>> Notice that setting a new setting is fast.
>> 
>> This was the way that I understood Dave he wanted.
>
> Do you mean every test is slow, or just migration tests?

Each migration test adds around 2 seconds on my machine.  So I decided
that it was easier that each time that we check one command, we test the
deprecated and non-deprecated versions of the command.  Amount of time
added to the test is negigible, and we are sure that we always test both
functions.

If we ever remove the deprecated method, we can always remove only that
part of th etest.


> Here I mean
> to only test setting the parameters without doing real migration tests
> (then query to make sure the settings were taking effect).  I assume
> that should be fast too?  Thanks,

We could create a new test for that, but we need to start in on
source/destination, I thought it just made things more complicated.

If your preffer that way, please suggest how/what?

Thanks, Juan.

Re: [Qemu-devel] [PATCH v4] Add ability for user to specify mouse ungrab key

[no-reply]

Hi,

This series failed build test on s390x host. Please find the details below.

Type: series
Message-id: 20171227011428.40996-1-programmingk...@gmail.com
Subject: [Qemu-devel] [PATCH v4] Add ability for user to specify mouse ungrab 
key

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/20171227011428.40996-1-programmingk...@gmail.com -> 
patchew/20171227011428.40996-1-programmingk...@gmail.com
Switched to a new branch 'test'
8a4c11e9f8 Add ability for user to specify mouse ungrab key

=== OUTPUT BEGIN ===
=== ENV ===
LANG=en_US.UTF-8
XDG_SESSION_ID=25790
USER=fam
PWD=/var/tmp/patchew-tester-tmp-nlf9oq_v/src
HOME=/home/fam
SHELL=/bin/sh
SHLVL=2
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
PATH=/usr/bin:/bin
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
glibc-debuginfo-common-2.24-10.fc25.s390x
fedora-release-26-1.noarch
dejavu-sans-mono-fonts-2.35-4.fc26.noarch
xemacs-filesystem-21.5.34-22.20170124hgf412e9f093d4.fc26.noarch
bash-4.4.12-7.fc26.s390x
freetype-2.7.1-9.fc26.s390x
libSM-1.2.2-5.fc26.s390x
libmpc-1.0.2-6.fc26.s390x
libaio-0.3.110-7.fc26.s390x
libverto-0.2.6-7.fc26.s390x
perl-Scalar-List-Utils-1.48-1.fc26.s390x
iptables-libs-1.6.1-2.fc26.s390x
perl-threads-shared-1.57-1.fc26.s390x
p11-kit-trust-0.23.9-2.fc26.s390x
tcl-8.6.6-2.fc26.s390x
libxshmfence-1.2-4.fc26.s390x
expect-5.45-23.fc26.s390x
perl-Thread-Queue-3.12-1.fc26.noarch
perl-encoding-2.19-6.fc26.s390x
keyutils-1.5.10-1.fc26.s390x
gmp-devel-6.1.2-4.fc26.s390x
python2-setuptools-36.2.0-2.fc26.noarch
enchant-1.6.0-16.fc26.s390x
net-snmp-libs-5.7.3-17.fc26.s390x
python-gobject-base-3.24.1-1.fc26.s390x
glusterfs-cli-3.10.7-1.fc26.s390x
python3-distro-1.0.3-1.fc26.noarch
python3-enchant-1.6.10-1.fc26.noarch
python-lockfile-0.11.0-6.fc26.noarch
python2-pyparsing-2.1.10-3.fc26.noarch
python2-lxml-4.1.1-1.fc26.s390x
librados2-10.2.7-2.fc26.s390x
trousers-lib-0.3.13-7.fc26.s390x
libpaper-1.1.24-14.fc26.s390x
libdatrie-0.2.9-4.fc26.s390x
libsoup-2.58.2-1.fc26.s390x
passwd-0.79-9.fc26.s390x
bind99-libs-9.9.10-3.P3.fc26.s390x
python3-rpm-4.13.0.2-1.fc26.s390x
bodhi-client-2.12.2-2.fc26.noarch
mock-core-configs-27.4-1.fc26.noarch
systemd-233-7.fc26.s390x
virglrenderer-0.6.0-1.20170210git76b3da97b.fc26.s390x
s390utils-ziomon-1.36.1-3.fc26.s390x
s390utils-osasnmpd-1.36.1-3.fc26.s390x
libXrandr-1.5.1-2.fc26.s390x
libglvnd-glx-1.0.0-1.fc26.s390x
texlive-ifxetex-svn19685.0.5-33.fc26.2.noarch
texlive-psnfss-svn33946.9.2a-33.fc26.2.noarch
texlive-dvipdfmx-def-svn40328-33.fc26.2.noarch
texlive-natbib-svn20668.8.31b-33.fc26.2.noarch
texlive-xdvi-bin-svn40750-33.20160520.fc26.2.s390x
texlive-cm-svn32865.0-33.fc26.2.noarch
texlive-beton-svn15878.0-33.fc26.2.noarch
texlive-fpl-svn15878.1.002-33.fc26.2.noarch
texlive-mflogo-svn38628-33.fc26.2.noarch
texlive-texlive-docindex-svn41430-33.fc26.2.noarch
texlive-luaotfload-bin-svn34647.0-33.20160520.fc26.2.noarch
texlive-koma-script-svn41508-33.fc26.2.noarch
texlive-pst-tree-svn24142.1.12-33.fc26.2.noarch
texlive-breqn-svn38099.0.98d-33.fc26.2.noarch
texlive-xetex-svn41438-33.fc26.2.noarch
gstreamer1-plugins-bad-free-1.12.3-1.fc26.s390x
xorg-x11-font-utils-7.5-33.fc26.s390x
ghostscript-fonts-5.50-36.fc26.noarch
libXext-devel-1.3.3-5.fc26.s390x
libusbx-devel-1.0.21-2.fc26.s390x
libglvnd-devel-1.0.0-1.fc26.s390x
pcre2-devel-10.23-10.fc26.s390x
emacs-25.3-3.fc26.s390x
alsa-lib-devel-1.1.4.1-1.fc26.s390x
kbd-2.0.4-2.fc26.s390x
dhcp-client-4.3.5-9.fc26.s390x
dconf-0.26.0-2.fc26.s390x
ccache-3.3.4-1.fc26.s390x
glibc-static-2.25-12.fc26.s390x
mc-4.8.19-5.fc26.s390x
doxygen-1.8.13-9.fc26.s390x
dpkg-1.18.24-1.fc26.s390x
libtdb-1.3.13-1.fc26.s390x
python2-pynacl-1.1.1-1.fc26.s390x
nss-sysinit-3.34.0-1.0.fc26.s390x
gnutls-dane-3.5.16-3.fc26.s390x
kernel-4.13.16-202.fc26.s390x
perl-Filter-1.58-1.fc26.s390x
glibc-debuginfo-2.24-10.fc25.s390x
kernel-devel-4.13.13-100.fc25.s390x
fedora-repos-26-1.noarch
dejavu-fonts-common-2.35-4.fc26.noarch
bind99-license-9.9.10-3.P3.fc26.noarch
ncurses-libs-6.0-8.20170212.fc26.s390x
libpng-1.6.28-2.fc26.s390x
libICE-1.0.9-9.fc26.s390x
boost-thread-1.63.0-9.fc26.s390x
kmod-24-1.fc26.s390x
libseccomp-2.3.2-1.fc26.s390x
perl-Text-ParseWords-3.30-366.fc26.noarch
libtool-ltdl-2.4.6-17.fc26.s390x
perl-threads-2.16-1.fc26.s390x
ca-certificates-2017.2.16-1.

Re: [Qemu-devel] [PATCH v4] Add ability for user to specify mouse ungrab key

[no-reply]

Hi,

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

Type: series
Message-id: 20171227011428.40996-1-programmingk...@gmail.com
Subject: [Qemu-devel] [PATCH v4] Add ability for user to specify mouse ungrab 
key

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
# iotests is broken now, skip
# time make docker-test-block@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/20171227011428.40996-1-programmingk...@gmail.com -> 
patchew/20171227011428.40996-1-programmingk...@gmail.com
Switched to a new branch 'test'
8a4c11e9f8 Add ability for user to specify mouse ungrab key

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-d4gv2uuv/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-d4gv2uuv/src'
  GEN 
/var/tmp/patchew-tester-tmp-d4gv2uuv/src/docker-src.2017-12-26-20.53.14.3598/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-d4gv2uuv/src/docker-src.2017-12-26-20.53.14.3598/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-d4gv2uuv/src/docker-src.2017-12-26-20.53.14.3598/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-d4gv2uuv/src/docker-src.2017-12-26-20.53.14.3598/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'10739aa26051a5d49d88132604539d3ed085e72e'
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
gettext-0.17-18.el6.x86_64
git-1.7.1-9.el6_9.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.6-2.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc gettext git glib2-devel libepoxy-devel libfdt-devel
 librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel spice-glib-devel 
spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=f962933da5b9
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/tmp/qemu-test/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/install
BIOS directory/tmp/qemu-test/install/share/qemu
firmware path /tmp/qemu-test/install/share/qemu-firmware
binary directory  /tmp/qemu-test/install/bin
library directory /tmp/qemu-test/install/lib
module directory  /tmp/qemu-test/install/lib/qemu
libexec directory /tmp/qemu-test/install/libexec
include directory /tmp/qemu-test/install/include
config directory  /tmp/qemu-test/install/etc
local state directory   /tmp/qemu-test/install/var
Manual directory  /tmp/qemu-test/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
GIT binarygit
GIT submodules
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1   -I$(SRC_PATH)/dtc/libfdt -pthread 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -DNCURSES_WIDECHAR   
-fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -Wstr

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Peter Xu]

On Wed, Dec 27, 2017 at 10:41:02AM +0100, Juan Quintela wrote:
> Peter Xu  wrote:
> > On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
> >> Peter Xu  wrote:
> >> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
> >> >> We now test the deprecated commands everytime that we test the new
> >> >> commands.  This makes unnecesary to add tests for deprecated commands.
> >> >> 
> >> >> Signed-off-by: Juan Quintela 
> >> >> ---
> >> >>  tests/migration-test.c | 32 
> >> >>  1 file changed, 28 insertions(+), 4 deletions(-)
> >> >> 
> >> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> >> index 799e24ebc6..51f49c74e9 100644
> >> >> --- a/tests/migration-test.c
> >> >> +++ b/tests/migration-test.c
> >> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState 
> >> >> *who, const char *parameter,
> >> >>  QDECREF(rsp);
> >> >>  }
> >> >>  
> >> >> -static void migrate_set_downtime(QTestState *who, const double value)
> >> >> +static void deprecated_set_downtime(QTestState *who, const double 
> >> >> value)
> >> >>  {
> >> >>  QDict *rsp;
> >> >>  gchar *cmd;
> >> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, 
> >> >> const double value)
> >> >>  g_free(expected);
> >> >>  }
> >> >>  
> >> >> -static void migrate_set_speed(QTestState *who, const char *value)
> >> >> +static void deprecated_set_speed(QTestState *who, const char *value)
> >> >>  {
> >> >>  QDict *rsp;
> >> >>  gchar *cmd;
> >> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, 
> >> >> const char *value)
> >> >>  migrate_check_parameter(who, "max-bandwidth", value);
> >> >>  }
> >> >>  
> >> >> +static void migrate_set_parameter(QTestState *who, const char 
> >> >> *parameter,
> >> >> +  const char *value)
> >> >> +{
> >> >> +QDict *rsp;
> >> >> +gchar *cmd;
> >> >> +
> >> >> +if (strcmp(parameter, "downtime-limit") == 0) {
> >> >> +deprecated_set_downtime(who, 0.12345);
> >> >> +}
> >> >> +
> >> >> +if (strcmp(parameter, "max-bandwidth") == 0) {
> >> >> +deprecated_set_speed(who, "12345");
> >> >> +}
> >> >
> >> > I'm fine with current approach, but I would really prefer to put them
> >> > all into a standalone test, by just calling them one by one with some
> >> > specific numbers and that's all.
> >> 
> >> That means another test (at least), and we have, also at least three
> >> deprecated comands:
> >> - migrate_set_speed
> >> - migrate_set_downtime
> >> - migrate_set_cache_size
> >> 
> >> And each test makes things slower.  So I *thought* it would we wiser to
> >> just check _always_ use the deprecated an the not deprecated one.
> >> 
> >> > (luckily we only have two deprecated commands and limited tests,
> >> >  otherwise extra commands will be M*N, say "number of deprecated
> >> >  command" * "number of test mirations")
> >> 
> >> Each test takes time, so adding tests make everything much slower.
> >> Notice that setting a new setting is fast.
> >> 
> >> This was the way that I understood Dave he wanted.
> >
> > Do you mean every test is slow, or just migration tests?
> 
> Each migration test adds around 2 seconds on my machine.  So I decided
> that it was easier that each time that we check one command, we test the
> deprecated and non-deprecated versions of the command.  Amount of time
> added to the test is negigible, and we are sure that we always test both
> functions.
> 
> If we ever remove the deprecated method, we can always remove only that
> part of th etest.
> 
> 
> > Here I mean
> > to only test setting the parameters without doing real migration tests
> > (then query to make sure the settings were taking effect).  I assume
> > that should be fast too?  Thanks,
> 
> We could create a new test for that, but we need to start in on
> source/destination, I thought it just made things more complicated.
> 
> If your preffer that way, please suggest how/what?

I mean, an isolated test that only runs a single VM instance rather
than two (so no test_migrate_start() at all), but only: qtest_init(),
then qmp() to send the deprecated commands, then query to see whether
the values are set.  That's all.  Can that work for us?

-- 
Peter Xu

Re: [Qemu-devel] [RESEND PATCH for-2.12 2/2] hw/acpi-build: Make next_base easy to follow

[Igor Mammedov]

On Thu, 14 Dec 2017 12:08:55 +0800
Dou Liyang  wrote:

> It may be hard to read the assignment statement of "next_base", so
> 
> S/next_base += (1ULL << 32) - pcms->below_4g_mem_size;
>  /next_base = mem_base + mem_len;
> 
> ... for readability.
> 
> No functionality change.
> 
> Signed-off-by: Dou Liyang 
> ---
>  hw/i386/acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 73519ab3ac..2010108ae0 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2381,7 +2381,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> MachineState *machine)
>  }
>  mem_base = 1ULL << 32;
>  mem_len = next_base - pcms->below_4g_mem_size;
> -next_base += (1ULL << 32) - pcms->below_4g_mem_size;
> +next_base = mem_base + mem_len;
>  }
>  numamem = acpi_data_push(table_data, sizeof *numamem);
>  build_srat_memory(numamem, mem_base, mem_len, i - 1,

Reviewed-by: Igor Mammedov 

Re: [Qemu-devel] [RESEND PATCH for-2.12 1/2] ACPI/unit-test: Add a testcase for RAM allocation in numa node

[Igor Mammedov]

On Thu, 14 Dec 2017 12:08:54 +0800
Dou Liyang  wrote:

> As QEMU supports the memory-less node, it is possible that there is
> no RAM in the first numa node(also be called as node0). eg:
>   ... \
>   -m 128,slots=3,maxmem=1G \
>   -numa node -numa node,mem=128M \
> 
> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> table. Only fixing it is not enough.
> 
> Add a testcase for this situation to make sure the ACPI table is
> correct for guest.
> 
> Suggested-by: Eduardo Habkost 
> Signed-off-by: Dou Liyang 
> ---
>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 5150 bytes
>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 224 bytes
>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 7834 bytes
>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 224 bytes
>  tests/bios-tables-test.c  |  24 
>  5 files changed, 24 insertions(+)
>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
> 
> diff --git a/tests/acpi-test-data/pc/DSDT.numamem 
> b/tests/acpi-test-data/pc/DSDT.numamem
> new file mode 100644
> index 
> ..224cfdd9e983e02dac5f4bf7e210eaa64cb0dc78
> GIT binary patch
> literal 5150
> zcmb7I-EJG#5uUTVl$K{nX(_FLmDq$F*GSsf{PAB z7fuXB5rWbTpg6t@R2RMI1#R%!dW7~7@(AfGxM-}aurs?n6hmo&gaDUk&V2jL%sF%B
> zjAS`h zqwW3O^FIH^V=W)HUg zbx@ld6^rRrHN=$EvR!Hj5JM2E(T%d*s6qnZ`=2yW+^0Ip^Y= zX~y$us`aJEb+4EBZr_7_Pa6#S(3|;gzXgA5CE!2*i}j(;XVs=zcTy=nvlqvRksK6&
> z9ndddC2z=Gw|maMpUnrpCS+jfik;1y67Ye(6dQw?O2sKOLmVzF;jU*)iF+K~{mq}R
> z5(~WvP`(D!Yj&x|;5Nu+fd;Z!#2?+fcuf|DiOubPSZ|m}8ZMcJi$(sP<)>Dd4?gJ9
> zBCr+q7#@Q(wF7SV)@soj!DZQq2dgp)G zk1c{uz=FDLnWIgd9(uIE&#tgH@*5eH74}wsiwm{51Rp2?cXoNrE{M%uwkmUJ9e9kd
> z^9 zyALmwQOr6|uCOoUFJx6+>;(Rl6mz|r6^j~UVFn5s+K?!kL-|k!bx{v!mWd`eLBpjH
> z5AJ9rk8~&@kBU5cSv^Xkj%_*(ron5jVv3VsVh(Pk@nNOij#IjWM^SxE8Kse67Bi(g
> zs3_K|b*AZ|f&{Zz+o+~pR$Mbz!MJRjr8|;)iKM~6Z z<&^e@#hZ4$0sfrS@qB2#D#l;5c*p;;#U z4_Gg?-{Q(>+JWtt$&pk*P-_1Hmqs(i;fy?*F=5;PYG;e z%xl4uR^M} zM?gK3n&H~FQxFf5Z3mvy9FO6%($ikFG3GXn`!wx#*QPN{Oy=-FCa{1|c~Wt$oYZMP
> z|LqLK z6`CJfFHTs;{Qbq&vt!n?f%PoS-IboekzruWO%zQ2|pPMkBi!h%X
> znzts+;|1}HqtB0-&j;r76Xx>~=JRfMUBT#`N%Kl*UZfD+BDm$E>}tkTu-S$dt}(fa
> zQ?$GK`mON7Gx_FG(YNcRkqJH(Bv;b3H`3)tpZlLa`Ahoo$DciV^7z5WpMlFvdu?@C
> zev_Q9FgSn%mcSZ~NflI)1DSP(J{6w|C)SZd}7B%4lY
> zEsaR4&`2AJW~Ek9eV|FVTl{b{s8Z-l=wGs6+LcTun;{Rv`#ff(%*OJcq#oaI!=9PR
> zWF8vjDXd|IxO-{ynj$VXijjP$N;El*#(eO4=l3TSlJ@?8n&Dv_;GpaF)=+_xy
> zlT^rXBEM0dF&(p}Muow=R|Rd~!z&p}PtaBoF97ErJgxLlKPTzuvq^9<2G^Ionh
> zz4CFYU_Vc$;_`7Wgd^9 zs|uwRo-zDjhGJ@|eM&G)!fK(B(zpsL^ zy<(S|rA!X@ENHtZyJ@5Kb%j*HSsx~P<|Y^)AvO!NHlrpjyivy{Y_k?P|1*Sc8sYJw
> zyDDtqmN@_Cvm{8^(c))74{XT}6Nd?=;ylAv6F+ZJeV7=@NxTaI{`<`tfKX5O%wRzG
> z41FXBG@@xz2ZMk86l(tc zqWQ5(gpGTnz_l>0pc4IX@WTO9h~$o9m+gAEkUuBEr8uFg9p{4A@R}LeC%wOk=AC?V
> zZc1V3Gmft}=TrrIN9+nwx zZw6gZ^--`_MJz}dhUR24*m2UID{l7#Cp#2ycO)>7Eo))VNrRjiXN@kM8&
> zHvDr3Uy;F>3zCz%GBQAFg64>ppm?dEDTHEk8vzxpF#(N5LGywYual5 z3VL0zVzVBys*zB^8W+%b6!eB*TY_~)
> zKxZPMf)(4v(AU{0=xxEO3#c9m6|5-%O+`UJ60CCqIu{8Qtn&go9|bK4R?Pe`EF%&s
> zSWgM)sVHbsu$~ss(~(fYdPYFcL_tl#dR9QsMnVPaIRQNv1+@h0c>z5i2^Fjh0=f_d
> zEeY0l1oWLqs9=3pK;MmmE(_KR0(v15Dp)TH=*5$uMtQ-yS8GVU#BLjJ-aV>y4+B9*
> z0jpUkq)8B(B^0nCg_;ftq)G0!HN3X>94sqCNg`>aQ&7U z9ht*}I@FQ9Tcn%aZDV!CTV&6{11Ds>M^HY}<889%;L#C!Y%Nee((5GSArSg>ARp zNXB#)`c5Dp>4S(dF@+v4lRd{A(^J%#Vk14pZH?(Ea!i(yK27qNehFpV_L06rGU| X)TZG7kLVu(w1s%rZLs0M;`09i9gxjE
> 
> literal 0
> HcmV?d1
> 
> diff --git a/tests/acpi-test-data/pc/SRAT.numamem 
> b/tests/acpi-test-data/pc/SRAT.numamem
> new file mode 100644
> index 
> ..dbc595d9cb85d3fcb5a4243153f42bb431c9de8f
> GIT binary patch
> literal 224
> zcmWFzatwLEz`(%x#mV2jB1F=Cg2*ZH@DxXmUE`z~9*2k!U%mXRq
> Yf~!ZCL8t>-1O^}2VG2>z!9?-X08OU~0RR91
> 
> literal 0
> HcmV?d1
> 
> diff --git a/tests/acpi-test-data/q35/DSDT.numamem 
> b/tests/acpi-test-data/q35/DSDT.numamem
> new file mode 100644
> index 
> ..8c9fa445b0119b6f67533cb968855b41fb9925d9
> GIT binary patch
> literal 7834
> zcmb7JTW=f38J#7U)M`meOKN5LF2Q*SdPz*#X;QRkU~-qYN|Y#)juTJ-*KX3vae)xN
> z07;Dik^ zD{}l;3UzA=)e<1Z~r=*RZ!mZ#OL0`c@`kTfMyuZ{w@%_&*KH
> zH*3~kcA~GN=;3FPb$cN0eB=H3&h5b`pGS8DksjR{xGrsYYi_?&?)UZsw-q#sPQTs=
> zv!!oc$LR-hE9vx0VOu!;n~l)&*Jt{hoxpx_PXytu)!|0!U?;!edcEequ79_D@y6G;
> z9+sZ{>Yu-Ta?iCvxQ1T`zt;!!6+MpJS=9_<-K)bcbd
> zo2aYR!+T!Skz=hm^;V^;#%f`N7#rrJ-s(0*R)}xE&j`D>=Mi3btFjTUwAx{R#ecou
> z>6XNRTA~3p-Tp)5deJcZ z#nhv7VGHq*@NUe!O2eUP!}O&aol>8t3IkZiU|Nm#R>F zX0wzO7Jkb3S#Nd21hXm$cb`~`U{yp=s%%(68DHxye0evPt|nDQs@B_orQHosR35Vo
> zjuDOYth-pV24Wz*#1&!Hto!1=NTfd!z5O@V%Hc6Rne=CQK8`K2FcOilpn6xli{C2=
> zIPLAf+}yl*ESz763mrFgMR-Jf6JCyqw(!r&8K?0_!!&03P&~Dd3wmy@W__6aFzGl~
> zcDYc+#+ zpep=qAHUXMhmiNjcPu&tUhp5jTw3jJtFZQ9w~(=M_K5Y3wTh727PhORkfnAv0Z>yQ
> z2~Zb>oShwwa4}2X7?BFZIk}H@pejg8^v~P`E5RKjQYvafuo6^O0+JG=VQ!L@Y6Nut
> 

Re: [Qemu-devel] [PATCH v5 1/7] vhost: Build temporary section list and deref after commit

[Igor Mammedov]

On Mon, 18 Dec 2017 20:13:34 +
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> Igor spotted that there's a race, where a region that's unref'd
> in a _del callback might be free'd before the set_mem_table call in
> the _commit callback, and thus the vhost might end up using free memory.
> 
> Fix this by building a complete temporary sections list, ref'ing every
> section (during add and nop) and then unref'ing the whole list right
> at the end of commit.
> 
> Signed-off-by: Dr. David Alan Gilbert 

As Michael've suggested it should go into stable (CCed)

Reviewed-by: Igor Mammedov 

> ---
>  hw/virtio/vhost.c | 73 
> ++-
>  include/hw/virtio/vhost.h |  2 ++
>  2 files changed, 49 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index e4290ce93d..610bfe6945 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -621,6 +621,8 @@ static void vhost_begin(MemoryListener *listener)
>   memory_listener);
>  dev->mem_changed_end_addr = 0;
>  dev->mem_changed_start_addr = -1;
> +dev->tmp_sections = NULL;
> +dev->n_tmp_sections = 0;
>  }
>  
>  static void vhost_commit(MemoryListener *listener)
> @@ -629,17 +631,25 @@ static void vhost_commit(MemoryListener *listener)
>   memory_listener);
>  hwaddr start_addr = 0;
>  ram_addr_t size = 0;
> +MemoryRegionSection *old_sections;
> +int n_old_sections;
> +
>  uint64_t log_size;
>  int r;
>  
> +old_sections = dev->mem_sections;
> +n_old_sections = dev->n_mem_sections;
> +dev->mem_sections = dev->tmp_sections;
> +dev->n_mem_sections = dev->n_tmp_sections;
> +
>  if (!dev->memory_changed) {
> -return;
> +goto out;
>  }
>  if (!dev->started) {
> -return;
> +goto out;
>  }
>  if (dev->mem_changed_start_addr > dev->mem_changed_end_addr) {
> -return;
> +goto out;
>  }
>  
>  if (dev->started) {
> @@ -656,7 +666,7 @@ static void vhost_commit(MemoryListener *listener)
>  VHOST_OPS_DEBUG("vhost_set_mem_table failed");
>  }
>  dev->memory_changed = false;
> -return;
> +goto out;
>  }
>  log_size = vhost_get_log_size(dev);
>  /* We allocate an extra 4K bytes to log,
> @@ -675,6 +685,27 @@ static void vhost_commit(MemoryListener *listener)
>  vhost_dev_log_resize(dev, log_size);
>  }
>  dev->memory_changed = false;
> +
> +out:
> +/* Deref the old list of sections, this must happen _after_ the
> + * vhost_set_mem_table to ensure the client isn't still using the
> + * section we're about to unref.
> + */
> +while (n_old_sections--) {
> +memory_region_unref(old_sections[n_old_sections].mr);
> +}
> +g_free(old_sections);
> +return;
> +}
> +
> +static void vhost_add_section(struct vhost_dev *dev,
> +  MemoryRegionSection *section)
> +{
> +++dev->n_tmp_sections;
> +dev->tmp_sections = g_renew(MemoryRegionSection, dev->tmp_sections,
> +dev->n_tmp_sections);
> +dev->tmp_sections[dev->n_tmp_sections - 1] = *section;
> +memory_region_ref(section->mr);
>  }
>  
>  static void vhost_region_add(MemoryListener *listener,
> @@ -687,36 +718,31 @@ static void vhost_region_add(MemoryListener *listener,
>  return;
>  }
>  
> -++dev->n_mem_sections;
> -dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,
> -dev->n_mem_sections);
> -dev->mem_sections[dev->n_mem_sections - 1] = *section;
> -memory_region_ref(section->mr);
> +vhost_add_section(dev, section);
>  vhost_set_memory(listener, section, true);
>  }
>  
> -static void vhost_region_del(MemoryListener *listener,
> +static void vhost_region_nop(MemoryListener *listener,
>   MemoryRegionSection *section)
>  {
>  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
>   memory_listener);
> -int i;
>  
>  if (!vhost_section(section)) {
>  return;
>  }
>  
> -vhost_set_memory(listener, section, false);
> -memory_region_unref(section->mr);
> -for (i = 0; i < dev->n_mem_sections; ++i) {
> -if (dev->mem_sections[i].offset_within_address_space
> -== section->offset_within_address_space) {
> ---dev->n_mem_sections;
> -memmove(&dev->mem_sections[i], &dev->mem_sections[i+1],
> -(dev->n_mem_sections - i) * sizeof(*dev->mem_sections));
> -break;
> -}
> +vhost_add_section(dev, section);
> +}
> +
> +static void vhost_region_del(MemoryListener *listener,
> + MemoryRegionSection *section)
> +{
> +if (!vhost_section(section)) {

Re: [Qemu-devel] [PATCH v4 1/6] vhost: Move log_dirty check

[Igor Mammedov]

On Thu, 14 Dec 2017 15:20:10 +
"Dr. David Alan Gilbert"  wrote:

> * Igor Mammedov (imamm...@redhat.com) wrote:
> > On Wed, 13 Dec 2017 18:08:02 +
> > "Dr. David Alan Gilbert (git)"  wrote:
> > 
> > > From: "Dr. David Alan Gilbert" 
> > > 
> > > Move the log_dirty check into vhost_section.
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert 
> > > ---
> > >  hw/virtio/trace-events |  3 +++
> > >  hw/virtio/vhost.c  | 20 +---
> > >  2 files changed, 16 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > > index 775461ae98..4a493bcd46 100644
> > > --- a/hw/virtio/trace-events
> > > +++ b/hw/virtio/trace-events
> > > @@ -1,5 +1,8 @@
> > >  # See docs/devel/tracing.txt for syntax documentation.
> > >  
> > > +# hw/virtio/vhost.c
> > > +vhost_section(const char *name, int r) "%s:%d"
> > > +
> > >  # hw/virtio/virtio.c
> > >  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned 
> > > out_num) "elem %p size %zd in_num %u out_num %u"
> > >  virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned 
> > > int idx) "vq %p elem %p len %u idx %u"
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > index e4290ce93d..e923219e63 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -27,6 +27,7 @@
> > >  #include "hw/virtio/virtio-access.h"
> > >  #include "migration/blocker.h"
> > >  #include "sysemu/dma.h"
> > > +#include "trace.h"
> > >  
> > >  /* enabled until disconnected backend stabilizes */
> > >  #define _VHOST_DEBUG 1
> > > @@ -567,18 +568,12 @@ static void vhost_set_memory(MemoryListener 
> > > *listener,
> > >   memory_listener);
> > >  hwaddr start_addr = section->offset_within_address_space;
> > >  ram_addr_t size = int128_get64(section->size);
> > > -bool log_dirty =
> > > -memory_region_get_dirty_log_mask(section->mr) & ~(1 << 
> > > DIRTY_MEMORY_MIGRATION);
> > >  int s = offsetof(struct vhost_memory, regions) +
> > >  (dev->mem->nregions + 1) * sizeof dev->mem->regions[0];
> > >  void *ram;
> > >  
> > >  dev->mem = g_realloc(dev->mem, s);
> > >  
> > > -if (log_dirty) {
> > > -add = false;
> > > -}
> > > -
> > >  assert(size);
> > >  
> > >  /* Optimize no-change case. At least cirrus_vga does this a lot at 
> > > this time. */
> > > @@ -611,8 +606,19 @@ static void vhost_set_memory(MemoryListener 
> > > *listener,
> > >  
> > >  static bool vhost_section(MemoryRegionSection *section)
> > >  {
> > > -return memory_region_is_ram(section->mr) &&
> > > +bool result;
> > > +bool log_dirty = memory_region_get_dirty_log_mask(section->mr) &
> > > + ~(1 << DIRTY_MEMORY_MIGRATION);
> > > +result = memory_region_is_ram(section->mr) &&
> > >  !memory_region_is_rom(section->mr);
> > > +
> > > +/* Vhost doesn't handle any block which is doing dirty-tracking other
> > > + * than migration; this typically fires on VGA areas.
> > > + */
> > > +result &= !log_dirty;
> > before patch even if log_dirty, vhost_set_memory will still proceed
> > and may remove dirty section from memmap and set memory_changed = true
> > 
> > but with this patch it will just ignore such section,
> > I'm not sure it's right (it might be right with new approach add/nop
> > but then this patch should go after new code in place and old one
> > if gone).
> 
> I thought about that, but then I came to the conclusion that the whole
> idea is that we're supposed to be ignoring these regions - so why
> should they cause any change to the behaviour of vhost at all?
> Thus this way seems safer.
the thing is that with originally with old code backend will use memap
with not yet dirty region, then after region becomes dirty current code
will REMOVE dirty region from map and NOTIFY backend.

While this patch is fine for new approach as memmap is build from scratch,
it doesn't look correct for current code since region that just became
dirty will not be removed from memap and might be used backend.
Whether it would really break something or not I'm not sure,
maybe Michael may Ack this patch ii using dirty region is fine.

> 
> Dave
> 
> > > +
> > > +trace_vhost_section(section->mr->name, result);
> > > +return result;
> > >  }
> > >  
> > >  static void vhost_begin(MemoryListener *listener)
> > 
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v5 3/7] vhost: Simplify ring verification checks

[Igor Mammedov]

On Mon, 18 Dec 2017 20:13:36 +
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> vhost_verify_ring_mappings() were used to verify that
> rings are still accessible and related memory hasn't
> been moved after flatview is updated.
> 
> It was doing checks by mapping ring's GPA+len and
> checking that HVA hadn't changed with new memory map.
> To avoid maybe expensive mapping call, we were
> identifying address range that changed and were doing
> mapping only if ring was in changed range.
> 
> However it's not neccessary to perform ring's GPA
> mapping as we already have its current HVA and all
> we need is to verify that ring's GPA translates to
> the same HVA in updated flatview.
> 
> This will allow the following patches to simplify the range
> comparison that was previously needed to avoid expensive
> verify_ring_mapping calls.
> 
> Signed-off-by: Igor Mammedov 
> with modifications by:
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Igor Mammedov 

> ---
>  hw/virtio/vhost.c | 75 
> +--
>  1 file changed, 40 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index d4710fc05c..18611f0d40 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -450,35 +450,37 @@ static void vhost_memory_unmap(struct vhost_dev *dev, 
> void *buffer,
>  }
>  }
>  
> -static int vhost_verify_ring_part_mapping(struct vhost_dev *dev,
> -  void *part,
> -  uint64_t part_addr,
> -  uint64_t part_size,
> -  uint64_t start_addr,
> -  uint64_t size)
> +static int vhost_verify_ring_part_mapping(void *ring_hva,
> +  uint64_t ring_gpa,
> +  uint64_t ring_size,
> +  void *reg_hva,
> +  uint64_t reg_gpa,
> +  uint64_t reg_size)
>  {
> -hwaddr l;
> -void *p;
> -int r = 0;
> +uint64_t hva_ring_offset;
> +uint64_t ring_last = range_get_last(ring_gpa, ring_size);
> +uint64_t reg_last = range_get_last(reg_gpa, reg_size);
>  
> -if (!ranges_overlap(start_addr, size, part_addr, part_size)) {
> +if (ring_last < reg_gpa || ring_gpa > reg_last) {
>  return 0;
>  }
> -l = part_size;
> -p = vhost_memory_map(dev, part_addr, &l, 1);
> -if (!p || l != part_size) {
> -r = -ENOMEM;
> +/* check that whole ring's is mapped */
> +if (ring_last > reg_last) {
> +return -ENOMEM;
>  }
> -if (p != part) {
> -r = -EBUSY;
> +/* check that ring's MemoryRegion wasn't replaced */
> +hva_ring_offset = ring_gpa - reg_gpa;
> +if (ring_hva != reg_hva + hva_ring_offset) {
> +return -EBUSY;
>  }
> -vhost_memory_unmap(dev, p, l, 0, 0);
> -return r;
> +
> +return 0;
>  }
>  
>  static int vhost_verify_ring_mappings(struct vhost_dev *dev,
> -  uint64_t start_addr,
> -  uint64_t size)
> +  void *reg_hva,
> +  uint64_t reg_gpa,
> +  uint64_t reg_size)
>  {
>  int i, j;
>  int r = 0;
> @@ -492,22 +494,25 @@ static int vhost_verify_ring_mappings(struct vhost_dev 
> *dev,
>  struct vhost_virtqueue *vq = dev->vqs + i;
>  
>  j = 0;
> -r = vhost_verify_ring_part_mapping(dev, vq->desc, vq->desc_phys,
> -   vq->desc_size, start_addr, size);
> +r = vhost_verify_ring_part_mapping(
> +vq->desc, vq->desc_phys, vq->desc_size,
> +reg_hva, reg_gpa, reg_size);
>  if (r) {
>  break;
>  }
>  
>  j++;
> -r = vhost_verify_ring_part_mapping(dev, vq->avail, vq->avail_phys,
> -   vq->avail_size, start_addr, size);
> +r = vhost_verify_ring_part_mapping(
> +vq->desc, vq->desc_phys, vq->desc_size,
> +reg_hva, reg_gpa, reg_size);
>  if (r) {
>  break;
>  }
>  
>  j++;
> -r = vhost_verify_ring_part_mapping(dev, vq->used, vq->used_phys,
> -   vq->used_size, start_addr, size);
> +r = vhost_verify_ring_part_mapping(
> +vq->desc, vq->desc_phys, vq->desc_size,
> +reg_hva, reg_gpa, reg_size);
>  if (r) {
>  break;
>  }
> @@ -635,13 +640,11 @@ static void vhost_commit(MemoryListener *listener)
>  {
>  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
>   

Re: [Qemu-devel] [PATCH] migration: fix small leaks

[Vladimir Sementsov-Ogievskiy]

Hi all!

Hmm, looks like leak is not fixed here: I've checked it while running 
iotest 181, that

migration_instance_finalize is not called.

If I understand correct, to call it we need unref current_migration 
object somewhere.


Or, may be I'm missing something..

01.08.2017 19:04, Marc-André Lureau wrote:

Spotted thanks to valgrind and tests/device-introspect-test:

==11711== 1 bytes in 1 blocks are definitely lost in loss record 6 of 14,537
==11711==at 0x4C2EB6B: malloc (vg_replace_malloc.c:299)
==11711==by 0x1E0CDBD8: g_malloc (gmem.c:94)
==11711==by 0x1E0E696E: g_strdup (gstrfuncs.c:363)
==11711==by 0x695693: migration_instance_init (migration.c:2226)
==11711==by 0x717C4B: object_init_with_type (object.c:344)
==11711==by 0x717E80: object_initialize_with_type (object.c:375)
==11711==by 0x7182EB: object_new_with_type (object.c:483)
==11711==by 0x718328: object_new (object.c:493)
==11711==by 0x4B8A29: qmp_device_list_properties (qmp.c:542)
==11711==by 0x4A9561: qmp_marshal_device_list_properties 
(qmp-marshal.c:1425)
==11711==by 0x819D4A: do_qmp_dispatch (qmp-dispatch.c:104)
==11711==by 0x819E82: qmp_dispatch (qmp-dispatch.c:131)

Signed-off-by: Marc-André Lureau 
---
  migration/migration.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 085c32c994..c3fe0ed9ca 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2214,6 +2214,15 @@ static void migration_class_init(ObjectClass *klass, 
void *data)
  dc->props = migration_properties;
  }
  
+static void migration_instance_finalize(Object *obj)

+{
+MigrationState *ms = MIGRATION_OBJ(obj);
+MigrationParameters *params = &ms->parameters;
+
+g_free(params->tls_hostname);
+g_free(params->tls_creds);
+}
+
  static void migration_instance_init(Object *obj)
  {
  MigrationState *ms = MIGRATION_OBJ(obj);
@@ -2282,6 +2291,7 @@ static const TypeInfo migration_type = {
  .class_size = sizeof(MigrationClass),
  .instance_size = sizeof(MigrationState),
  .instance_init = migration_instance_init,
+.instance_finalize = migration_instance_finalize,
  };
  
  static void register_migration_types(void)



--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v5 4/7] vhost: Merge sections added to temporary list

[Igor Mammedov]

On Mon, 18 Dec 2017 20:13:37 +
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> As sections are reported by the listener to the _nop and _add
> methods, add them to the temporary section list but now merge them
> with the previous section if the new one abuts and the backend allows.
> 
> Signed-off-by: Dr. David Alan Gilbert 
beside small nit patch looks good to me, so with it fixed

Reviewed-by: Igor Mammedov 

> ---
>  hw/virtio/trace-events |  2 ++
>  hw/virtio/vhost.c  | 70 
> +++---
>  2 files changed, 63 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 4a493bcd46..0e63c8739d 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -1,6 +1,8 @@
>  # See docs/devel/tracing.txt for syntax documentation.
>  
>  # hw/virtio/vhost.c
> +vhost_region_add_section(const char *name, uint64_t gpa, uint64_t size, 
> uint64_t host) "%s: 0x%"PRIx64"+0x%"PRIx64" @ 0x%"PRIx64
> +vhost_region_add_section_abut(const char *name, uint64_t new_size) "%s: 
> 0x%"PRIx64
>  vhost_section(const char *name, int r) "%s:%d"
>  
>  # hw/virtio/virtio.c
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 18611f0d40..57d15acd2b 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -709,14 +709,65 @@ out:
>  return;
>  }
>  
> -static void vhost_add_section(struct vhost_dev *dev,
> -  MemoryRegionSection *section)
> +/* Adds the section data to the tmp_section structure.
> + * It relies on the listener calling us in memory address order
> + * and for each region (via the _add and _nop methods) to
> + * join neighbours.
> + */
> +static void vhost_region_add_section(struct vhost_dev *dev,
> + MemoryRegionSection *section)
wrong alignment, should be on '('

>  {
> -++dev->n_tmp_sections;
> -dev->tmp_sections = g_renew(MemoryRegionSection, dev->tmp_sections,
> -dev->n_tmp_sections);
> -dev->tmp_sections[dev->n_tmp_sections - 1] = *section;
> -memory_region_ref(section->mr);
> +bool need_add = true;
> +uint64_t mrs_size = int128_get64(section->size);
> +uint64_t mrs_gpa = section->offset_within_address_space;
> +uintptr_t mrs_host = (uintptr_t)memory_region_get_ram_ptr(section->mr) +
> + section->offset_within_region;
> +
> +trace_vhost_region_add_section(section->mr->name, mrs_gpa, mrs_size,
> +   mrs_host);
> +
> +if (dev->n_tmp_sections) {
> +/* Since we already have at least one section, lets see if
> + * this extends it; since we're scanning in order, we only
> + * have to look at the last one, and the FlatView that calls
> + * us shouldn't have overlaps.
> + */
> +MemoryRegionSection *prev_sec = dev->tmp_sections +
> +   (dev->n_tmp_sections - 1);
> +uint64_t prev_gpa_start = prev_sec->offset_within_address_space;
> +uint64_t prev_size = int128_get64(prev_sec->size);
> +uint64_t prev_gpa_end   = range_get_last(prev_gpa_start, prev_size);
> +uint64_t prev_host_start =
> +(uintptr_t)memory_region_get_ram_ptr(prev_sec->mr) +
> +prev_sec->offset_within_region;
> +uint64_t prev_host_end   = range_get_last(prev_host_start, 
> prev_size);
> +
> +if (prev_gpa_end + 1 == mrs_gpa &&
> +prev_host_end + 1 == mrs_host &&
> +section->mr == prev_sec->mr &&
> +(!dev->vhost_ops->vhost_backend_can_merge ||
> +dev->vhost_ops->vhost_backend_can_merge(dev,
> +mrs_host, mrs_size,
> +prev_host_start, prev_size))) {
> +/* The two sections abut */
> +need_add = false;
> +prev_sec->size = int128_add(prev_sec->size, section->size);
> +trace_vhost_region_add_section_abut(section->mr->name,
> +mrs_size + prev_size);
> +}
> +}
> +
> +if (need_add) {
> +++dev->n_tmp_sections;
> +dev->tmp_sections = g_renew(MemoryRegionSection, dev->tmp_sections,
> +dev->n_tmp_sections);
> +dev->tmp_sections[dev->n_tmp_sections - 1] = *section;
> +/* The flatview isn't stable and we don't use it, making it NULL
> + * means we can memcmp the list.
> + */
> +dev->tmp_sections[dev->n_tmp_sections - 1].fv = NULL;
> +memory_region_ref(section->mr);
> +}
>  }
>  
>  static void vhost_region_add(MemoryListener *listener,
> @@ -728,11 +779,12 @@ static void vhost_region_add(MemoryListener *listener,
>  if (!vhost_section(section)) {
>  return;
>  }
> +vhost_region_add_section(dev, section);
>  
> -vhost_add_section(dev

Re: [Qemu-devel] [PATCH v5 5/7] vhost: Regenerate region list from changed sections list

[Igor Mammedov]

On Mon, 18 Dec 2017 20:13:38 +
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> Compare the sections list that's just been generated, and if it's
> different from the old one regenerate the region list.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  hw/virtio/trace-events |  1 +
>  hw/virtio/vhost.c  | 34 ++
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 0e63c8739d..742ff0f90b 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -1,6 +1,7 @@
>  # See docs/devel/tracing.txt for syntax documentation.
>  
>  # hw/virtio/vhost.c
> +vhost_commit(bool started, bool changed) "Started: %d Changed: %d"
>  vhost_region_add_section(const char *name, uint64_t gpa, uint64_t size, 
> uint64_t host) "%s: 0x%"PRIx64"+0x%"PRIx64" @ 0x%"PRIx64
>  vhost_region_add_section_abut(const char *name, uint64_t new_size) "%s: 
> 0x%"PRIx64
>  vhost_section(const char *name, int r) "%s:%d"
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 57d15acd2b..4394ac0275 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -643,21 +643,47 @@ static void vhost_commit(MemoryListener *listener)
>  MemoryRegionSection *old_sections;
>  int n_old_sections;
>  uint64_t log_size;
> +size_t regions_size;
>  int r;
>  int i;
> +bool changed = false;
>  
>  old_sections = dev->mem_sections;
>  n_old_sections = dev->n_mem_sections;
>  dev->mem_sections = dev->tmp_sections;
>  dev->n_mem_sections = dev->n_tmp_sections;
>  
> -if (!dev->memory_changed) {
> -goto out;
> +if (dev->n_mem_sections != n_old_sections) {
> +changed = true;
> +} else {
> +/* Same size, lets check the contents */
> +changed = memcmp(dev->mem_sections, old_sections,
> + n_old_sections * sizeof(old_sections[0])) != 0;
>  }
> -if (!dev->started) {
> +
> +trace_vhost_commit(dev->started, changed);
> +if (!changed) {
>  goto out;
>  }
> -if (dev->mem_changed_start_addr > dev->mem_changed_end_addr) {
> +
> +/* Rebuild the regions list from the new sections list */
> +regions_size = offsetof(struct vhost_memory, regions) +
> +   dev->n_mem_sections * sizeof dev->mem->regions[0];
> +dev->mem = g_realloc(dev->mem, regions_size);
> +dev->mem->nregions = dev->n_mem_sections;
> +for (i = 0; i < dev->n_mem_sections; i++) {
> +struct vhost_memory_region *cur_vmr = dev->mem->regions + i;
> +struct MemoryRegionSection *mrs = dev->mem_sections + i;
> +
> +cur_vmr->guest_phys_addr = mrs->offset_within_address_space;
> +cur_vmr->memory_size = int128_get64(mrs->size);
> +cur_vmr->userspace_addr  =
> +(uintptr_t)memory_region_get_ram_ptr(mrs->mr) +
> +mrs->offset_within_region;
> +cur_vmr->flags_padding   = 0;
> +}
is there a reason to keep dev->mem around after vhost_commit executed?
I'd suggest to use temporary local variable and free it on exit from function.

> +if (!dev->started) {
also why memmap is generated this early and not right before first use?
here we might generate memap but not actually use it.

>  goto out;
>  }
>  

Re: [Qemu-devel] [PATCH v5 7/7] vhost: Merge and delete unused callbacks

[Igor Mammedov]

On Mon, 18 Dec 2017 20:13:40 +
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> Now that the olf vhost_set_memory code is gone, the _nop and _add
> callbacks are identical and can be merged.  The _del callback is
> no longer needed.
> 
> Signed-off-by: Dr. David Alan Gilbert 
With style nit fixed

Reviewed-by: Igor Mammedov 

> ---
>  hw/virtio/vhost.c | 31 ---
>  1 file changed, 4 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 358ceb3033..4eaa4f889f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -555,7 +555,8 @@ static void vhost_region_add_section(struct vhost_dev 
> *dev,
>  }
>  }
>  
> -static void vhost_region_add(MemoryListener *listener,
> +/* Used for both add and nop callbacks */
> +static void vhost_region_addnop(MemoryListener *listener,
>   MemoryRegionSection *section)
argument alignment should on '('

>  {
>  struct vhost_dev *dev = container_of(listener, struct vhost_dev,
> @@ -567,29 +568,6 @@ static void vhost_region_add(MemoryListener *listener,
>  vhost_region_add_section(dev, section);
>  }
>  
> -/* Called on regions that have not changed */
> -static void vhost_region_nop(MemoryListener *listener,
> - MemoryRegionSection *section)
> -{
> -struct vhost_dev *dev = container_of(listener, struct vhost_dev,
> - memory_listener);
> -
> -if (!vhost_section(section)) {
> -return;
> -}
> -
> -vhost_region_add_section(dev, section);
> -}
> -
> -static void vhost_region_del(MemoryListener *listener,
> - MemoryRegionSection *section)
> -{
> -if (!vhost_section(section)) {
> -return;
> -}
> -
> -}
> -
>  static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>  {
>  struct vhost_iommu *iommu = container_of(n, struct vhost_iommu, n);
> @@ -1158,9 +1136,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>  hdev->memory_listener = (MemoryListener) {
>  .begin = vhost_begin,
>  .commit = vhost_commit,
> -.region_add = vhost_region_add,
> -.region_del = vhost_region_del,
> -.region_nop = vhost_region_nop,
> +.region_add = vhost_region_addnop,
> +.region_nop = vhost_region_addnop,
>  .log_start = vhost_log_start,
>  .log_stop = vhost_log_stop,
>  .log_sync = vhost_log_sync,

[Qemu-devel] [Bug 1740219] [NEW] static linux-user emulation has several-second startup time

[LukeShu]

Public bug reported:

static linux-user emulation has several-second startup time

My problem: I'm a Parabola packager, and I'm updating our
qemu-user-static package from 2.8 to 2.11.  With my new
statically-linked 2.11, running `qemu-arm /my/arm-chroot/bin/true`
went from taking 0.006s to 3s!  This does not happen with the normal
dynamically linked 2.11, or the old static 2.8.

What happens is it gets stuck in
`linux-user/elfload.c:init_guest_space()`.  What `init_guest_space`
does is map 2 parts of the address space: `[base, base+guest_size]`
and `[base+0x, base+0x+page_size]`; where it must find
an acceptable `base`.  Its strategy is to `mmap(NULL, guest_size,
...)` decide where the first range is, and then check if that
+0x is also available.  If it isn't, then it starts trying
`mmap(base, ...)` for the entire address space from low-address to
high-address.

"Normally," it finds an accaptable `base` within the first 2 tries.
With a static 2.11, it's taking thousands of tries.



Now, from my understanding, there are 2 factors working together to
cause that in static 2.11 but not the other builds:

 - 2.11 increased the default `guest_size` from 0xf700 to 0x
 - PIE (and thus ASLR) is disabled for static builds

For some reason that I don't understand, with the smaller
`guest_size` the initial `mmap(NULL, guest_size, ...)` usually
returns an acceptable address range; but larger `guest_size` makes it
consistently return a block of memory that butts right up against
another already mapped chunk of memory.  This isn't just true on the
older builds, it's true with the 2.11 builds if I use the `-R` flag to
shrink the `guest_size` back down to 0xf700.  That is with
linux-hardened 4.13.13 on x86-64.

So then, it it falls back to crawling the entire address space; so it
tries base=0x1000.  With ASLR, that probably succeeds.  But with
ASLR being disabled on static builds, the text segment is at
0x6000; which is does not leave room for the needed
0x1000-size block before it.  So then it tries base=0x2000.
And so on, more than 6000 times until it finally gets to and passes
the text segment; calling mmap more than 12000 times.



I'm not sure what the fix is.  Perhaps try to mmap a continuous chunk
of size 0x1000, then munmap it and then mmap the 2 chunks that we
actually need.  The disadvantage to that is that it does not support
the sparse address space that the current algorithm supports for
`guest_size < 0x`.  If `guest_size < 0x` *and* the big
mmap fails, then it could fall back to a sparse search; though I'm not
sure the current algorithm is a good choice for it, as we see in this
bug.  Perhaps it should inspect /proc/self/maps to try to find a
suitable range before ever calling mmap?

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  static linux-user emulation has several-second startup time

Status in QEMU:
  New

Bug description:
  static linux-user emulation has several-second startup time

  My problem: I'm a Parabola packager, and I'm updating our
  qemu-user-static package from 2.8 to 2.11.  With my new
  statically-linked 2.11, running `qemu-arm /my/arm-chroot/bin/true`
  went from taking 0.006s to 3s!  This does not happen with the normal
  dynamically linked 2.11, or the old static 2.8.

  What happens is it gets stuck in
  `linux-user/elfload.c:init_guest_space()`.  What `init_guest_space`
  does is map 2 parts of the address space: `[base, base+guest_size]`
  and `[base+0x, base+0x+page_size]`; where it must find
  an acceptable `base`.  Its strategy is to `mmap(NULL, guest_size,
  ...)` decide where the first range is, and then check if that
  +0x is also available.  If it isn't, then it starts trying
  `mmap(base, ...)` for the entire address space from low-address to
  high-address.

  "Normally," it finds an accaptable `base` within the first 2 tries.
  With a static 2.11, it's taking thousands of tries.

  

  Now, from my understanding, there are 2 factors working together to
  cause that in static 2.11 but not the other builds:

   - 2.11 increased the default `guest_size` from 0xf700 to 0x
   - PIE (and thus ASLR) is disabled for static builds

  For some reason that I don't understand, with the smaller
  `guest_size` the initial `mmap(NULL, guest_size, ...)` usually
  returns an acceptable address range; but larger `guest_size` makes it
  consistently return a block of memory that butts right up against
  another already mapped chunk of memory.  This isn't just true on the
  older builds, it's true with the 2.11 builds if I use the `-R` flag to
  shrink the `guest_size` back down to 0xf700.  That is with
  linux-hardened 4.13.13 on x86-64.

  So then, it it falls back to crawling the enti

[Qemu-devel] [Bug 1740219] Re: static linux-user emulation has several-second startup time

[LukeShu]

Actually, it seems that the `[base+0x,
base+0x+page_size]` segment is only mapped on 32-bit ARM.  So
this is 32-bit ARM-specific.

** Tags added: arm linux-user

** Summary changed:

- static linux-user emulation has several-second startup time
+ static linux-user ARM emulation has several-second startup time

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

Title:
  static linux-user ARM emulation has several-second startup time

Status in QEMU:
  New

Bug description:
  static linux-user emulation has several-second startup time

  My problem: I'm a Parabola packager, and I'm updating our
  qemu-user-static package from 2.8 to 2.11.  With my new
  statically-linked 2.11, running `qemu-arm /my/arm-chroot/bin/true`
  went from taking 0.006s to 3s!  This does not happen with the normal
  dynamically linked 2.11, or the old static 2.8.

  What happens is it gets stuck in
  `linux-user/elfload.c:init_guest_space()`.  What `init_guest_space`
  does is map 2 parts of the address space: `[base, base+guest_size]`
  and `[base+0x, base+0x+page_size]`; where it must find
  an acceptable `base`.  Its strategy is to `mmap(NULL, guest_size,
  ...)` decide where the first range is, and then check if that
  +0x is also available.  If it isn't, then it starts trying
  `mmap(base, ...)` for the entire address space from low-address to
  high-address.

  "Normally," it finds an accaptable `base` within the first 2 tries.
  With a static 2.11, it's taking thousands of tries.

  

  Now, from my understanding, there are 2 factors working together to
  cause that in static 2.11 but not the other builds:

   - 2.11 increased the default `guest_size` from 0xf700 to 0x
   - PIE (and thus ASLR) is disabled for static builds

  For some reason that I don't understand, with the smaller
  `guest_size` the initial `mmap(NULL, guest_size, ...)` usually
  returns an acceptable address range; but larger `guest_size` makes it
  consistently return a block of memory that butts right up against
  another already mapped chunk of memory.  This isn't just true on the
  older builds, it's true with the 2.11 builds if I use the `-R` flag to
  shrink the `guest_size` back down to 0xf700.  That is with
  linux-hardened 4.13.13 on x86-64.

  So then, it it falls back to crawling the entire address space; so it
  tries base=0x1000.  With ASLR, that probably succeeds.  But with
  ASLR being disabled on static builds, the text segment is at
  0x6000; which is does not leave room for the needed
  0x1000-size block before it.  So then it tries base=0x2000.
  And so on, more than 6000 times until it finally gets to and passes
  the text segment; calling mmap more than 12000 times.

  

  I'm not sure what the fix is.  Perhaps try to mmap a continuous chunk
  of size 0x1000, then munmap it and then mmap the 2 chunks that we
  actually need.  The disadvantage to that is that it does not support
  the sparse address space that the current algorithm supports for
  `guest_size < 0x`.  If `guest_size < 0x` *and* the big
  mmap fails, then it could fall back to a sparse search; though I'm not
  sure the current algorithm is a good choice for it, as we see in this
  bug.  Perhaps it should inspect /proc/self/maps to try to find a
  suitable range before ever calling mmap?

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

Re: [Qemu-devel] [PATCH v5 0/7] Rework vhost memory region updates

[Igor Mammedov]

On Mon, 18 Dec 2017 20:13:33 +
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> Hi,
>   This patch set reworks the way the vhost code handles changes in
> physical address space layout that came from a discussion with Igor.
> 
> Its intention is to simplify a lot of the update code,
> and to make it easier for the postcopy+shared code to
> do the hugepage alignments that are needed.
> 
> Instead of inserting/removing each section during the add/del
> callbacks of the listener, we start afresh and build a list
> from the add and nop callbacks, then at the end compare the list
> we've built with the exisiting list.
> 
> v5
>   Solve the unref race found by Igor with a new 1st patch
>   Now we've got a temporary section list rework the rest of the set
>around that.
> 
> Dr. David Alan Gilbert (7):
>   vhost: Build temporary section list and deref after commit
>   vhost: Move log_dirty check
>   vhost: Simplify ring verification checks
>   vhost: Merge sections added to temporary list
>   vhost: Regenerate region list from changed sections list
>   vhost: Clean out old vhost_set_memory and friends
>   vhost: Merge and delete unused callbacks
> 
>  hw/virtio/trace-events|   6 +
>  hw/virtio/vhost.c | 490 
> --
>  include/hw/virtio/vhost.h |   5 +-
>  3 files changed, 174 insertions(+), 327 deletions(-)
Nice diffstat and more importantly it should be easier to reason about changes 
to memmap in future patches,
Thanks for making it easier to read/understand how memap updates work.

The series looks almost ready, I've Acked most of it modulo
"vhost: Move log_dirty check"
  - I'm not convinced that doing it while old code still around is right 
thing to do as it might break bisectability,
Maybe Michael may ack it if doesn't really affect old code,
see discussion at '[PATCH v4 1/6] vhost: Move log_dirty check'

"vhost: Regenerate region list from changed sections list"
  - suggested an additional cleanup

"vhost: Clean out old vhost_set_memory and friends"
  - removes update to used_memslots without providing an alternative,
it could lead to crashes when region count goes above backend's limit
related thread "[PATCH v2 0/2] vhost: two fixes"

the rest of comments are just minor issues

[Qemu-devel] [PATCH] i386/cpu/kvm: look at PMU's CPUID before setting MSRs

[Jan Dakinevich]

Certain PMU-related MSRs are not supported for CPUs with PMU
architecture below version 2. KVM rejects any access to them (see
intel_is_valid_msr_idx routine in KVM), and QEMU fails on the following
assertion:

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

QEMU also could fail if KVM exposes less fixed counters then 3. It could
happen if host system run inside another hypervisor, which is tweaking
PMU-related CPUID. To prevent possible fail, number of fixed counters now is
obtained in the same way as number of GP counters.

Reviewed-by: Roman Kagan 
Signed-off-by: Jan Dakinevich 
---
 target/i386/kvm.c | 80 +--
 1 file changed, 48 insertions(+), 32 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 351b64f..a6790db 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -92,8 +92,9 @@ static bool has_msr_hv_stimer;
 static bool has_msr_hv_frequencies;
 static bool has_msr_xss;
 
-static bool has_msr_architectural_pmu;
-static uint32_t num_architectural_pmu_counters;
+static uint32_t has_architectural_pmu_version;
+static uint32_t num_architectural_pmu_gp_counters;
+static uint32_t num_architectural_pmu_fixed_counters;
 
 static int has_xsave;
 static int has_xcrs;
@@ -872,19 +873,28 @@ int kvm_arch_init_vcpu(CPUState *cs)
 }
 
 if (limit >= 0x0a) {
-uint32_t ver;
+uint32_t eax, edx;
 
-cpu_x86_cpuid(env, 0x0a, 0, &ver, &unused, &unused, &unused);
-if ((ver & 0xff) > 0) {
-has_msr_architectural_pmu = true;
-num_architectural_pmu_counters = (ver & 0xff00) >> 8;
+cpu_x86_cpuid(env, 0x0a, 0, &eax, &unused, &unused, &edx);
+
+has_architectural_pmu_version = eax & 0xff;
+if (has_architectural_pmu_version > 0) {
+num_architectural_pmu_gp_counters = (eax & 0xff00) >> 8;
 
 /* Shouldn't be more than 32, since that's the number of bits
  * available in EBX to tell us _which_ counters are available.
  * Play it safe.
  */
-if (num_architectural_pmu_counters > MAX_GP_COUNTERS) {
-num_architectural_pmu_counters = MAX_GP_COUNTERS;
+if (num_architectural_pmu_gp_counters > MAX_GP_COUNTERS) {
+num_architectural_pmu_gp_counters = MAX_GP_COUNTERS;
+}
+
+if (has_architectural_pmu_version > 1) {
+num_architectural_pmu_fixed_counters = edx & 0x1f;
+
+if (num_architectural_pmu_fixed_counters > MAX_FIXED_COUNTERS) 
{
+num_architectural_pmu_fixed_counters = MAX_FIXED_COUNTERS;
+}
 }
 }
 }
@@ -1652,32 +1662,36 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
 if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
 kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr);
 }
-if (has_msr_architectural_pmu) {
-/* Stop the counter.  */
-kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
-kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, 0);
+if (has_architectural_pmu_version > 0) {
+if (has_architectural_pmu_version > 1) {
+/* Stop the counter.  */
+kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
+kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL, 0);
+}
 
 /* Set the counter values.  */
-for (i = 0; i < MAX_FIXED_COUNTERS; i++) {
+for (i = 0; i < num_architectural_pmu_fixed_counters; i++) {
 kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR0 + i,
   env->msr_fixed_counters[i]);
 }
-for (i = 0; i < num_architectural_pmu_counters; i++) {
+for (i = 0; i < num_architectural_pmu_gp_counters; i++) {
 kvm_msr_entry_add(cpu, MSR_P6_PERFCTR0 + i,
   env->msr_gp_counters[i]);
 kvm_msr_entry_add(cpu, MSR_P6_EVNTSEL0 + i,
   env->msr_gp_evtsel[i]);
 }
-kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_STATUS,
-  env->msr_global_status);
-kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_OVF_CTRL,
-  env->msr_global_ovf_ctrl);
-
-/* Now start the PMU.  */
-kvm_msr_entry_add(cpu, MSR_CORE_PERF_FIXED_CTR_CTRL,
-  env->msr_fixed_ctr_ctrl);
-kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_CTRL,
-  env->msr_global_ctrl);
+if (has_architectural_pmu_version > 1) {
+kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_STATUS,
+  env->msr_global_status);
+kvm_msr_entry_add(cpu, MSR_CORE_PERF_GLOBAL_OVF_CTRL,
+   

Re: [Qemu-devel] [PATCH v5 3/7] vhost: Simplify ring verification checks

[Igor Mammedov]

On Mon, 18 Dec 2017 20:13:36 +
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> vhost_verify_ring_mappings() were used to verify that
> rings are still accessible and related memory hasn't
> been moved after flatview is updated.
> 
> It was doing checks by mapping ring's GPA+len and
> checking that HVA hadn't changed with new memory map.
> To avoid maybe expensive mapping call, we were
> identifying address range that changed and were doing
> mapping only if ring was in changed range.
> 
> However it's not neccessary to perform ring's GPA
> mapping as we already have its current HVA and all
> we need is to verify that ring's GPA translates to
> the same HVA in updated flatview.
> 
> This will allow the following patches to simplify the range
> comparison that was previously needed to avoid expensive
> verify_ring_mapping calls.
> 
> Signed-off-by: Igor Mammedov 
> with modifications by:
> Signed-off-by: Dr. David Alan Gilbert 

an additional question,

in iommu case ring_hva == ring_gpa if we look in to vhost_memory_map()
have you check if iommu case is working with new code?


> ---
>  hw/virtio/vhost.c | 75 
> +--
>  1 file changed, 40 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index d4710fc05c..18611f0d40 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -450,35 +450,37 @@ static void vhost_memory_unmap(struct vhost_dev *dev, 
> void *buffer,
>  }
>  }
>  
> -static int vhost_verify_ring_part_mapping(struct vhost_dev *dev,
> -  void *part,
> -  uint64_t part_addr,
> -  uint64_t part_size,
> -  uint64_t start_addr,
> -  uint64_t size)
> +static int vhost_verify_ring_part_mapping(void *ring_hva,
> +  uint64_t ring_gpa,
> +  uint64_t ring_size,
> +  void *reg_hva,
> +  uint64_t reg_gpa,
> +  uint64_t reg_size)
>  {
> -hwaddr l;
> -void *p;
> -int r = 0;
> +uint64_t hva_ring_offset;
> +uint64_t ring_last = range_get_last(ring_gpa, ring_size);
> +uint64_t reg_last = range_get_last(reg_gpa, reg_size);
>  
> -if (!ranges_overlap(start_addr, size, part_addr, part_size)) {
> +if (ring_last < reg_gpa || ring_gpa > reg_last) {
>  return 0;
>  }
> -l = part_size;
> -p = vhost_memory_map(dev, part_addr, &l, 1);
> -if (!p || l != part_size) {
> -r = -ENOMEM;
> +/* check that whole ring's is mapped */
> +if (ring_last > reg_last) {
> +return -ENOMEM;
>  }
> -if (p != part) {
> -r = -EBUSY;
> +/* check that ring's MemoryRegion wasn't replaced */
> +hva_ring_offset = ring_gpa - reg_gpa;
> +if (ring_hva != reg_hva + hva_ring_offset) {
> +return -EBUSY;
>  }
> -vhost_memory_unmap(dev, p, l, 0, 0);
> -return r;
> +
> +return 0;
>  }
>  
>  static int vhost_verify_ring_mappings(struct vhost_dev *dev,
> -  uint64_t start_addr,
> -  uint64_t size)
> +  void *reg_hva,
> +  uint64_t reg_gpa,
> +  uint64_t reg_size)
>  {
>  int i, j;
>  int r = 0;
> @@ -492,22 +494,25 @@ static int vhost_verify_ring_mappings(struct vhost_dev 
> *dev,
>  struct vhost_virtqueue *vq = dev->vqs + i;
>  
>  j = 0;
> -r = vhost_verify_ring_part_mapping(dev, vq->desc, vq->desc_phys,
> -   vq->desc_size, start_addr, size);
> +r = vhost_verify_ring_part_mapping(
> +vq->desc, vq->desc_phys, vq->desc_size,
> +reg_hva, reg_gpa, reg_size);
>  if (r) {
>  break;
>  }
>  
>  j++;
> -r = vhost_verify_ring_part_mapping(dev, vq->avail, vq->avail_phys,
> -   vq->avail_size, start_addr, size);
> +r = vhost_verify_ring_part_mapping(
> +vq->desc, vq->desc_phys, vq->desc_size,
> +reg_hva, reg_gpa, reg_size);
>  if (r) {
>  break;
>  }
>  
>  j++;
> -r = vhost_verify_ring_part_mapping(dev, vq->used, vq->used_phys,
> -   vq->used_size, start_addr, size);
> +r = vhost_verify_ring_part_mapping(
> +vq->desc, vq->desc_phys, vq->desc_size,
> +reg_hva, reg_gpa, reg_size);
>  if (r) {
>  break;
>  }
> @@ -635,13 +640,11 @@ static void vhost_commit(Mem

Re: [Qemu-devel] [PATCH v3 09/13] tpm: Introduce condition to notify waiters of completed command

[Stefan Berger]

On 12/22/2017 08:24 AM, Marc-André Lureau wrote:

On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger
 wrote:

Introduce a lock and a condition to notify anyone waiting for the completion
of the execution of a TPM command by the backend (thread). The backend
uses the condition to signal anyone waiting for command completion.
We need to place the condition in two locations: one is invoked by the
backend thread, the other by the bottom half thread.
We will use the signaling to wait for command completion before VM
suspend.

Signed-off-by: Stefan Berger 
---
  hw/tpm/tpm_tis.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 035c6ef..86e9a92 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -80,6 +80,9 @@ typedef struct TPMState {
  TPMVersion be_tpm_version;

  size_t be_buffer_size;
+
+QemuMutex state_lock;
+QemuCond cmd_complete;

Looks like the cond is unused in the following patches.


You are right. I will drop this patch. It may have been needed before 
the refactoring you did.


   Stefan

Re: [Qemu-devel] [PATCH v3 10/13] tpm: Introduce condition in TPM backend for notification

[Stefan Berger]

On 11/10/2017 09:11 AM, Stefan Berger wrote:

TPM backends will suspend independently of the frontends. Also
here we need to be able to wait for the TPM command to have been
completely processed.

Signed-off-by: Stefan Berger 
---
  backends/tpm.c   | 19 +++
  include/sysemu/tpm_backend.h | 14 ++
  2 files changed, 33 insertions(+)

diff --git a/backends/tpm.c b/backends/tpm.c
index 91222c5..bf0e120 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -20,6 +20,14 @@
  #include "qemu/thread.h"
  #include "qemu/main-loop.h"

+void tpm_backend_cmd_completed(TPMBackend *s)
+{
+qemu_mutex_lock(&s->state_lock);
+s->tpm_busy = false;
+qemu_cond_signal(&s->cmd_complete);
+qemu_mutex_unlock(&s->state_lock);
+}
+
  static void tpm_backend_request_completed_bh(void *opaque)
  {
  TPMBackend *s = TPM_BACKEND(opaque);
@@ -36,6 +44,9 @@ static void tpm_backend_worker_thread(gpointer data, gpointer 
user_data)
  k->handle_request(s, (TPMBackendCmd *)data);

  qemu_bh_schedule(s->bh);
+
+/* result delivered */
+tpm_backend_cmd_completed(s);
  }

  static void tpm_backend_thread_end(TPMBackend *s)
@@ -64,6 +75,10 @@ int tpm_backend_init(TPMBackend *s, TPMIf *tpmif, Error 
**errp)
  object_ref(OBJECT(tpmif));

  s->had_startup_error = false;
+s->tpm_busy = false;
+
+qemu_mutex_init(&s->state_lock);
+qemu_cond_init(&s->cmd_complete);

  return 0;
  }
@@ -93,6 +108,10 @@ bool tpm_backend_had_startup_error(TPMBackend *s)

  void tpm_backend_deliver_request(TPMBackend *s, TPMBackendCmd *cmd)
  {
+qemu_mutex_lock(&s->state_lock);
+s->tpm_busy = true;
+qemu_mutex_unlock(&s->state_lock);
+
  g_thread_pool_push(s->thread_pool, cmd, NULL);
  }

diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 0d6c994..39598e3 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -18,6 +18,7 @@
  #include "qapi-types.h"
  #include "qemu/option.h"
  #include "sysemu/tpm.h"
+#include "qemu/thread.h"

  #define TYPE_TPM_BACKEND "tpm-backend"
  #define TPM_BACKEND(obj) \
@@ -53,6 +54,10 @@ struct TPMBackend {
  char *id;

  QLIST_ENTRY(TPMBackend) list;
+
+QemuMutex state_lock;


This lock only locks the tpm_busy flag. So maybe I should rename it to 
tpm_busy_lock. It's not meant to be a general state lock.


   Stefan


+QemuCond cmd_complete; /* signaled once tpm_busy is false */
+bool tpm_busy;
  };

  struct TPMBackendClass {
@@ -206,6 +211,15 @@ size_t tpm_backend_get_buffer_size(TPMBackend *s);
   */
  TPMInfo *tpm_backend_query_tpm(TPMBackend *s);

+/**
+ * tpm_backend_cmd_completed:
+ * @s: the backend
+ *
+ * Mark the backend as not busy and notify anyone interested
+ * in the state changed
+ */
+void tpm_backend_cmd_completed(TPMBackend *s);
+
  TPMBackend *qemu_find_tpm_be(const char *id);

  #endif

Re: [Qemu-devel] [PATCH v3 00/13] tpm: Extend TPM with state migration support (not 2.11)

[Stefan Berger]

On 12/22/2017 11:13 AM, Marc-André Lureau wrote:

Hi

On Fri, Dec 22, 2017 at 4:59 PM, Stefan Berger
 wrote:

On 12/22/2017 07:49 AM, Marc-André Lureau wrote:

Hi

On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger
 wrote:

This set of patches implements support for migrating the state of the
external 'swtpm' TPM emulator as well as that of the emulated device
interfaces. I have primarily tested this with the TIS and TPM 1.2 so
far, but it also seems to work with TPM 2.

The TIS is simplified first by reducing the number of buffers and read
and write offsets into these buffers. Following the state machine of the
TIS, a single buffer and r/w offset is enough for all localities since
only one locality can ever be active.

This series applies on top of my tpm-next branch.

One of the challenges that is addressed by this set of patches is the
fact
that the TPM emulator may be processing a command while the state
serialization of the devices is supposed to happen. A necessary first
step
has been implemented here that ensures that a response has been received
from the exernal emulator and the bottom half function, which delivers
the
response and adjusts device registers (TIS or CRB), has been executed,
before the device's state is serialized.

A subsequent extension may need to address the live migration loop and
delay
the serialization of devices until the response from the external TPM has
been received. Though the likelihood that someone executes a long-lasting
TPM command while this is occurring is certainly rare.

 Stefan

Stefan Berger (13):
tpm_tis: convert uint32_t to size_t
tpm_tis: limit size of buffer from backend
tpm_tis: remove TPMSizeBuffer usage
tpm_tis: move buffers from localities into common location
tpm_tis: merge read and write buffer into single buffer
tpm_tis: move r/w_offsets to TPMState
tpm_tis: merge r/w_offset into rw_offset
tpm: Implement tpm_sized_buffer_reset

ok for the above (you could queue/pull-req this now)


tpm: Introduce condition to notify waiters of completed command
tpm: Introduce condition in TPM backend for notification
tpm: implement tpm_backend_wait_cmd_completed
tpm: extend TPM emulator with state migration support
tpm_tis: extend TPM TIS with state migration support

Much of the complexity from this migration series comes with the
handling & synchronization of the IO thread.

I think having a seperate thread doesn't bring much today TPM thread.
it is a workaround for the chardev API being mostly synchronous. Am I
wrong? (yes, passthrough doesn't use chardev, but it should probably
use qio or chardev internally)

Other kind of devices using a seperate process suffer the same
problem. Just grep for qemu_chr_fe_write and look for the preceding
comment, it's a common flaw in qemu code base. Code use the
synchronous API, and sometime use non-blocking write
(hw/usb/redirect.c seems quite better!)

I would like to get rid of the seperate thread in TPM before we add
migration support. We should try to improve the chardev API to make it
easier to do non-blocking IO. This should considerably simplify the
above patches and benefit the rest of qemu (instead of having everyone
doing its own thing with seperate thread or other kind of continuation
state).

What do you think?


I am wondering whether it will help us to get rid of the
conditions/notifications, like patches 9-11 of this series. I doubt 12 and
13 will change. At some point device state is supposed to be written out and
in case of the TPM we have to wait for the response to have come back into
the backend. We won't start listening on the file descriptor for an
outstanding response, so I guess we will still wait for a notification in
that case as well. So I am not sure which parts are going to be simpler...

Why would qemu need to wait for completion of emulator? Couldn't qemu
& emulator save the current state, including in-flights commands?
That's apparently what usbredir does.


The thread is sending the commands to the external emulator and waits 
for the reception of the response. Once the response is received, the 
delivery of the response is handed off to the bottom half. Using the 
bottom half avoids synchronization primitives since we know that no 
other thread is in the device emulator when it runs (except for our 
thread sending to and receiving from the emulator).


This series of patches introduces a tpm_busy flag that the device 
emulation thread sets once it hands off a command to the thread for 
processing. It does this in tpm_backend_deliver_request(). It resets 
that flag once it has scheduled the bottom half to run for delivering 
the response to the device emulator (TIS, CRB etc.). This in turn is 
happening in tpm_backend_worker_thread().


For the migration we have the following cases once the device (frontend 
or backend) is supposed to write out its state:
[Once the state of the device is supposed to be written out we know what 
the OS is paused

Re: [Qemu-devel] [PATCH v7 02/26] tcg: Add generic vector expanders

[Kirill Batuzov]

On Mon, 18 Dec 2017, Richard Henderson wrote:

> Signed-off-by: Richard Henderson 
> ---
>  Makefile.target  |2 +-
>  accel/tcg/tcg-runtime.h  |   29 ++
>  tcg/tcg-gvec-desc.h  |   49 ++
>  tcg/tcg-op-gvec.h|  152 ++
>  tcg/tcg-op.h |1 +
>  accel/tcg/tcg-runtime-gvec.c |  295 
>  tcg/tcg-op-gvec.c| 1099 
> ++
>  tcg/tcg-op-vec.c |   36 +-
>  accel/tcg/Makefile.objs  |2 +-
>  9 files changed, 1655 insertions(+), 10 deletions(-)
>  create mode 100644 tcg/tcg-gvec-desc.h
>  create mode 100644 tcg/tcg-op-gvec.h
>  create mode 100644 accel/tcg/tcg-runtime-gvec.c
>  create mode 100644 tcg/tcg-op-gvec.c
> 

> diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
> new file mode 100644
> index 00..120e301096
> --- /dev/null
> +++ b/tcg/tcg-op-gvec.c

> +/* Set OPRSZ bytes at DOFS to replications of IN or IN_C.  */
> +static void do_dup_i32(unsigned vece, uint32_t dofs, uint32_t oprsz,
> +   uint32_t maxsz, TCGv_i32 in, uint32_t in_c,
> +   void (*ool)(TCGv_ptr, TCGv_i32, TCGv_i32))
> +{
> +TCGType type;
> +TCGv_vec t_vec;
> +uint32_t i;
> +
> +assert(vece <= MO_32);
> +
> +if (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)) {
> +type = TCG_TYPE_V256;
> +} else if (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)) {
> +type = TCG_TYPE_V128;
> +} else if (TCG_TARGET_HAS_v64 && check_size_impl(oprsz, 8)) {
> +type = TCG_TYPE_V64;
> +} else {
> +if (check_size_impl(oprsz, 4)) {
> +TCGv_i32 t_i32 = tcg_temp_new_i32();
> +
> +if (in) {
> +switch (vece) {
> +case MO_8:
> +tcg_gen_deposit_i32(t_i32, in, in, 8, 24);
> +in = t_i32;
> +/* fallthru */
> +case MO_16:
> +tcg_gen_deposit_i32(t_i32, in, in, 16, 16);
> +break;
> +}

If vece == MO_32 then t_i32 will be left uninitialized here... 

> +} else {
> +switch (vece) {
> +case MO_8:
> +in_c = (in_c & 0xff) * 0x01010101;
> +break;
> +case MO_16:
> +in_c = deposit32(in_c, 16, 16, in_c);
> +break;
> +}
> +tcg_gen_movi_i32(t_i32, in_c);
> +}
> +
> +for (i = 0; i < oprsz; i += 4) {
> +tcg_gen_st_i32(t_i32, cpu_env, dofs + i);
> +}

...and used uninitialized here.

> +tcg_temp_free_i32(t_i32);
> +goto done;
> +} else {
> +TCGv_i32 t_i32 = in ? in : tcg_const_i32(in_c);
> +TCGv_ptr a0 = tcg_temp_new_ptr();
> +TCGv_i32 desc = tcg_const_i32(simd_desc(oprsz, maxsz, 0));
> +
> +tcg_gen_addi_ptr(a0, cpu_env, dofs);
> +ool(a0, desc, t_i32);
> +
> +tcg_temp_free_ptr(a0);
> +tcg_temp_free_i32(desc);
> +if (in == NULL) {
> +tcg_temp_free_i32(t_i32);
> +}
> +return;
> +}
> +}
> +
> +t_vec = tcg_temp_new_vec(type);
> +if (in) {
> +tcg_gen_dup_i32_vec(vece, t_vec, in);
> +} else {
> +switch (vece) {
> +case MO_8:
> +tcg_gen_dup8i_vec(t_vec, in_c);
> +break;
> +case MO_16:
> +tcg_gen_dup16i_vec(t_vec, in_c);
> +break;
> +default:
> +tcg_gen_dup32i_vec(t_vec, in_c);
> +break;
> +}
> +}
> +
> +i = 0;
> +if (TCG_TARGET_HAS_v256) {
> +for (; i + 32 <= oprsz; i += 32) {
> +tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V256);
> +}
> +}
> +if (TCG_TARGET_HAS_v128) {
> +for (; i + 16 <= oprsz; i += 16) {
> +tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V128);
> +}
> +}
> +if (TCG_TARGET_HAS_v64) {
> +for (; i < oprsz; i += 8) {
> +tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V64);
> +}
> +}
> +tcg_temp_free_vec(t_vec);
> +
> + done:
> +tcg_debug_assert(i == oprsz);
> +if (i < maxsz) {
> +expand_clr(dofs + i, maxsz - i);
> +}
> +}
> +
> +/* Likewise, but with 64-bit quantities.  */
> +static void do_dup_i64(unsigned vece, uint32_t dofs, uint32_t oprsz,
> +   uint32_t maxsz, TCGv_i64 in, uint64_t in_c)
> +{
> +TCGType type;
> +TCGv_vec t_vec;
> +uint32_t i;
> +
> +assert(vece <= MO_64);
> +
> +if (TCG_TARGET_HAS_v256 && check_size_impl(oprsz, 32)) {
> +type = TCG_TYPE_V256;
> +} else if (TCG_TARGET_HAS_v128 && check_size_impl(oprsz, 16)) {
> +type = TCG_TYPE_V128;
> +} else if (TCG_TARGET_HAS_v64 && TCG_TARGET_REG_BITS =

Re: [Qemu-devel] [PATCH v7 08/26] tcg/i386: Add vector operations

[Kirill Batuzov]

On Mon, 18 Dec 2017, Richard Henderson wrote:

> The x86 vector instruction set is extremely irregular.  With newer
> editions, Intel has filled in some of the blanks.  However, we don't
> get many 64-bit operations until SSE4.2, introduced in 2009.
> 
> The subsequent edition was for AVX1, introduced in 2011, which added
> three-operand addressing, and adjusts how all instructions should be
> encoded.
> 
> Given the relatively narrow 2 year window between possible to support
> and desirable to support, and to vastly simplify code maintainence,
> I am only planning to support AVX1 and later cpus.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/i386/tcg-target.h |  36 ++-
>  tcg/i386/tcg-target.inc.c | 561 
> ++
>  2 files changed, 546 insertions(+), 51 deletions(-)
> 

> diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
> index 63d27f10e7..e9a4d92598 100644
> --- a/tcg/i386/tcg-target.inc.c
> +++ b/tcg/i386/tcg-target.inc.c

> -static inline void tcg_out_mov(TCGContext *s, TCGType type,
> -   TCGReg ret, TCGReg arg)
> +static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
> +{
> +int rexw = 0;
> +
> +if (arg == ret) {
> +return;
> +}
> +switch (type) {
> +case TCG_TYPE_I64:
> +rexw = P_REXW;
> +/* fallthru */
> +case TCG_TYPE_I32:
> +if (ret < 16) {
> +if (arg < 16) {
> +tcg_out_modrm(s, OPC_MOVL_GvEv + rexw, ret, arg);
> +} else {
> +tcg_out_vex_modrm(s, OPC_MOVD_EyVy + rexw, ret, 0, arg);
> +}
> +} else {
> +if (arg < 16) {
> +tcg_out_vex_modrm(s, OPC_MOVD_VyEy + rexw, ret, 0, arg);
> +} else {
> +tcg_out_vex_modrm(s, OPC_MOVQ_VqWq, ret, 0, arg);
> +}
> +}
> +break;
> +
> +case TCG_TYPE_V64:
> +tcg_debug_assert(ret >= 16 && arg >= 16);
> +tcg_out_vex_modrm(s, OPC_MOVQ_VqWq, ret, 0, arg);
> +break;
> +case TCG_TYPE_V128:
> +tcg_debug_assert(ret >= 16 && arg >= 16);
> +tcg_out_vex_modrm(s, OPC_MOVDQA_VxWx, ret, 0, arg);
> +break;
> +case TCG_TYPE_V256:
> +tcg_debug_assert(ret >= 16 && arg >= 16);
> +tcg_out_vex_modrm(s, OPC_MOVDQA_VxWx | P_VEXL, ret, 0, arg);
> +break;
> +
> +default:
> +g_assert_not_reached();
> +}
> +}

I think something is wrong with instruction encodings here. Looks like
  tcg_out_mov(&tcg_ctx, TCG_TYPE_I64, TCG_REG_EBP, TCG_REG_XMM0)
produces
  vmovq %xmm5, %rax
instead.

Here is the dump.

IN: 
0x00400580:  4e040c41  dup  v1.4s, w2
0x00400584:  4b0203e2  neg  w2, w2
0x00400588:  3d800021  str  q1, [x1]
0x0040058c:  d65f03c0  ret  

OP after optimization and liveness analysis:
 ld_i32 tmp0,env,$0xffec  dead: 1
 movi_i32 tmp1,$0x0
 brcond_i32 tmp0,tmp1,lt,$L0  dead: 0 1

  00400580  
 dup_vec v128,e32,tmp2,x2
 st_vec v128,e8,tmp2,env,$0x8b0   dead: 0

  00400584  
 ext32u_i64 tmp4,x2   dead: 1
 neg_i64 tmp5,tmp4dead: 1
 ext32u_i64 x2,tmp5   sync: 0  dead: 0 1

<...>

OUT: [size=111]
0x6075bf40:  41 8b 6e ec  movl -0x14(%r14), %ebp
0x6075bf44:  85 edtestl%ebp, %ebp
0x6075bf46:  0f 8c 59 00 00 00jl   0x6075bfa5
0x6075bf4c:  c4 c1 7a 7e 46 50vmovq0x50(%r14), %xmm0
0x6075bf52:  c5 f9 70 c8 00   vpshufd  $0, %xmm0, %xmm1
0x6075bf57:  c4 c1 7a 7f 8e b0 08 00  vmovdqu  %xmm1, 0x8b0(%r14)
0x6075bf5f:  00
0x6075bf60:  c4 e1 f9 7e e8   vmovq%xmm5, %rax
0x6075bf65:  8b edmovl %ebp, %ebp
0x6075bf67:  48 f7 dd negq %rbp
0x6075bf6a:  8b edmovl %ebp, %ebp
0x6075bf6c:  49 89 6e 50  movq %rbp, 0x50(%r14)
<...>

%xmm5 is used uninitialized, there is no move from either %xmm0 or
0x50(%r14) to %ebp, there are two unnecessary movl %ebp, %ebp.

-- 
Kirill

Re: [Qemu-devel] [PATCH] i386/cpu/kvm: look at PMU's CPUID before setting MSRs

[Eduardo Habkost]

On Wed, Dec 27, 2017 at 05:04:26PM +0300, Jan Dakinevich wrote:
> Certain PMU-related MSRs are not supported for CPUs with PMU
> architecture below version 2. KVM rejects any access to them (see
> intel_is_valid_msr_idx routine in KVM), and QEMU fails on the following
> assertion:
> 
>   kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
> 
> QEMU also could fail if KVM exposes less fixed counters then 3. It could
> happen if host system run inside another hypervisor, which is tweaking
> PMU-related CPUID. To prevent possible fail, number of fixed counters now is
> obtained in the same way as number of GP counters.
> 
> Reviewed-by: Roman Kagan 
> Signed-off-by: Jan Dakinevich 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

[Qemu-devel] [PATCH 00/16] target/xtensa updates

[Max Filippov]

Hi Peter,

please pull the following batch of updates for the target/xtensa.

The following changes since commit 0a0dc59d27527b78a195c2d838d28b7b49e5a639:

  Update version for v2.11.0 release (2017-12-13 14:31:09 +)

are available in the git repository at:

  git://github.com/OSLL/qemu-xtensa.git tags/20171227-xtensa

for you to fetch changes up to d462b6b6b79f092327927b3fb3ac20505c90a706:

  target/xtensa: implement disassembler (2017-12-18 21:26:20 -0800)


target/xtensa updates:

- add libisa to the xtensa target;
- change xtensa instruction translator to use it;
- switch existing xtensa cores to use it;
- add support for a number of instructions: salt/saltu, const16,
  GPIO32 group, debug mode and MMU-related;
- add disassembler for Xtensa.


Max Filippov (16):
  target/xtensa: pass actual frame size to the entry helper
  target/xtensa: import libisa source
  target/xtensa: extract core opcode translators
  target/xtensa: extract FPU2000 opcode translators
  target/xtensa: update import_core.sh script for libisa
  target/xtensa: switch dc232b to libisa
  target/xtensa: switch dc233c to libisa
  target/xtensa: switch fsf to libisa
  target/xtensa: use libisa for instruction decoding
  target/xtensa: tests: fix memctl SR test
  target/xtensa: drop DisasContext::litbase
  target/xtensa: add internal/noop SRs and opcodes
  target/xtensa: implement salt/saltu
  target/xtensa: implement GPIO32
  target/xtensa: implement const16
  target/xtensa: implement disassembler

 MAINTAINERS| 1 +
 disas/Makefile.objs| 1 +
 disas/xtensa.c |   133 +
 include/disas/bfd.h| 1 +
 include/hw/xtensa/xtensa-isa.h |   838 ++
 target/xtensa/Makefile.objs| 1 +
 target/xtensa/core-dc232b.c| 4 +
 target/xtensa/core-dc232b/xtensa-modules.c | 14105 +
 target/xtensa/core-dc233c.c| 4 +
 target/xtensa/core-dc233c/xtensa-modules.c | 15232 +++
 target/xtensa/core-fsf.c   | 5 +
 target/xtensa/core-fsf/xtensa-modules.c|  9841 +
 target/xtensa/cpu.c| 9 +
 target/xtensa/cpu.h|31 +
 target/xtensa/helper.c |37 +
 target/xtensa/import_core.sh   |15 +
 target/xtensa/op_helper.c  | 2 +-
 target/xtensa/translate.c  |  5883 +++
 target/xtensa/xtensa-isa-internal.h|   231 +
 target/xtensa/xtensa-isa.c |  1745 +++
 target/xtensa/xtensa-isa.h | 1 +
 tests/tcg/xtensa/test_sr.S | 2 +-
 22 files changed, 45949 insertions(+), 2173 deletions(-)
 create mode 100644 disas/xtensa.c
 create mode 100644 include/hw/xtensa/xtensa-isa.h
 create mode 100644 target/xtensa/core-dc232b/xtensa-modules.c
 create mode 100644 target/xtensa/core-dc233c/xtensa-modules.c
 create mode 100644 target/xtensa/core-fsf/xtensa-modules.c
 create mode 100644 target/xtensa/xtensa-isa-internal.h
 create mode 100644 target/xtensa/xtensa-isa.c
 create mode 100644 target/xtensa/xtensa-isa.h

-- 
2.1.4

[Qemu-devel] [Bug 1738283] Re: 'Less than' (<), 'more than' (>), and 'pipe' (|) can't be typed via VNC

[Michal Nowak]

I found Adam's patch from Fedora Rawhide
(https://src.fedoraproject.org/rpms/qemu/c/f81be8f0261cce74799f946e99f23d57f8db7e17?branch=master)
when applied to openSUSE's 2.11.0 QEMU effective in openQA as well as
manually with vncviewer.

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

Title:
  'Less than' (<), 'more than' (>), and 'pipe' (|) can't be typed via
  VNC

Status in QEMU:
  New

Bug description:
  If I start QEMU 2.11 (from
  https://build.opensuse.org/package/show/Virtualization/qemu) VM with
  VNC, I am unable to type following three characters: 'less than' (<),
  'more than' (>), and 'pipe' (|) on en_US QWERTY keyboard. Other
  characters work fine. QEMu version 2.10.1 worked fine.

  /usr/bin/qemu-kvm -m 2048 -cpu kvm64 -drive
  media=cdrom,if=none,id=cd0,format=raw,file=OI-hipster-
  minimal-20171031.iso -device ide-cd,drive=cd0 -boot once=d,menu=on
  ,splash-time=5000 -device usb-ehci -device usb-tablet -smp 1 -enable-
  kvm -vnc :91,share=force-shared

  The ISO can be downloaded here: https://www.openindiana.org/download/

  Also tried Fedora-Server-dvd-x86_64-25-1.3.iso and it's the same
  situation.

  If I run the same command without '-vnc :91,share=force-shared',
  everything works just fine.

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

Re: [Qemu-devel] [PATCH net-next v2 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor

[David Miller]

From: Jason Baron 
Date: Fri, 22 Dec 2017 16:54:01 -0500

> The ability to set speed and duplex for virtio_net in useful in various
> scenarios as described here:
> 
> 16032be virtio_net: add ethtool support for set and get of settings
> 
> However, it would be nice to be able to set this from the hypervisor,
> such that virtio_net doesn't require custom guest ethtool commands.
> 
> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> the hypervisor to export a linkspeed and duplex setting. The user can
> subsequently overwrite it later if desired via: 'ethtool -s'.
> 
> Signed-off-by: Jason Baron 
> Cc: "Michael S. Tsirkin" 
> Cc: Jason Wang 

Looks mostly fine to me but need some virtio_net reviewers on this one.

> @@ -57,6 +57,8 @@
>* Steering */
>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */
>  
> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Host set linkspeed and duplex */
> +

Why use a value so far away from the largest existing one?

Just curious.

Re: [Qemu-devel] qemu 2.9.0 qcow2 file failed to open after hard server reset

[Vasiliy Tolstov]

2017-12-22 12:23 GMT+03:00 Daniel P. Berrange :
> On Thu, Dec 21, 2017 at 05:58:47PM -0500, John Snow wrote:
>>
>>
>> On 12/21/2017 05:13 PM, Vasiliy Tolstov wrote:
>> > Hi! Today my server have forced reboot and one of my vm can't start
>> > with message:
>> > qcow2: Marking image as corrupt: L2 table offset 0x3f786d6c207600
>> > unaligned (L1 index: 0); further corruption events will be suppressed
>> >
>> > i'm use debian jessie with hand builded qemu 2.9.0, i'm try to
>> > qemu-img check but it not helps. How can i recover data inside qcow2
>> > file? (i'm not use compression or encryption inside it).
>> >
>>
>> Not looking good if you're missing the very first L2 table in its entirety.
>>
>> You might be able to go through this thing by hand and learn for
>> yourself where the L2 table is (it will be a 64KiB region, aligned to a
>> 64KiB boundary, that all contain 64bit, 64KiB aligned pointers that will
>> be less than the size of the file. the offset of this missing region is
>> not likely to be referenced elsewhere in your file.)

Too hard, client restored backup because he don't want to wait for me =))

>
> Fun. That rather makes you wish that every single distinct type of table
> in QCow2 files had a unique UUID value stored in it, to make forensics
> like this easier :-)
>

=)

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

Re: [Qemu-devel] qemu 2.9.0 qcow2 file failed to open after hard server reset

[Vasiliy Tolstov]

2017-12-22 1:58 GMT+03:00 John Snow :
>
>
> On 12/21/2017 05:13 PM, Vasiliy Tolstov wrote:
>> Hi! Today my server have forced reboot and one of my vm can't start
>> with message:
>> qcow2: Marking image as corrupt: L2 table offset 0x3f786d6c207600
>> unaligned (L1 index: 0); further corruption events will be suppressed
>>
>> i'm use debian jessie with hand builded qemu 2.9.0, i'm try to
>> qemu-img check but it not helps. How can i recover data inside qcow2
>> file? (i'm not use compression or encryption inside it).
>>
>
> Not looking good if you're missing the very first L2 table in its entirety.
>
> You might be able to go through this thing by hand and learn for
> yourself where the L2 table is (it will be a 64KiB region, aligned to a
> 64KiB boundary, that all contain 64bit, 64KiB aligned pointers that will
> be less than the size of the file. the offset of this missing region is
> not likely to be referenced elsewhere in your file.)
>
> and then, once you've found it, you can update the pointer that's wrong.
> However, where there's smoke there's often fire, so...
>
> best of luck.
>
> --js

If i use raw image as i understand this corruption can't happening in raw?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

Re: [Qemu-devel] [PATCH] migration: Guard ram_bytes_remaining against early call

[Juan Quintela]

"Dr. David Alan Gilbert (git)"  wrote:
> From: "Dr. David Alan Gilbert" 
>
> Calling ram_bytes_remaining during the early part of setup is unsafe
> because the ram_state isn't yet initialised.
>
> This can happen in the sequence:
>migrate
>migrate_cancel
>info migrate
>
> if the migrate sticks trying to connect (e.g. to an unresponsive
> destination due to the connect timeout).  Here 'info migrate' sees
> a state of CANCELLING and so assumes the migrate has partially happened.
>
> partial fix for:
> RH bz: https://bugzilla.redhat.com/show_bug.cgi?id=1525899
> Reported-by: Xianxian Wang 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] [PATCH v7 08/26] tcg/i386: Add vector operations

[Richard Henderson]

On 12/27/2017 07:31 AM, Kirill Batuzov wrote:
> I think something is wrong with instruction encodings here. Looks like
>   tcg_out_mov(&tcg_ctx, TCG_TYPE_I64, TCG_REG_EBP, TCG_REG_XMM0)
> produces
>   vmovq %xmm5, %rax
> instead.

Bah.  The operands are swapped -- ebp == 5 and eax == 0.


r~

[Qemu-devel] [PATCH] Update dtc to fix compilation problem on Mac OS 10.6

[John Arbuckle]

Currently QEMU does not build on Mac OS 10.6
because of a missing patch in the dtc
subproject. Updating dtc to make the patch
available fixes this problem.

Signed-off-by: John Arbuckle 
---
 dtc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dtc b/dtc
index 558cd81bdd..e671852042 16
--- a/dtc
+++ b/dtc
@@ -1 +1 @@
-Subproject commit 558cd81bdd432769b59bff01240c44f82cfb1a9d
+Subproject commit e671852042a77b15ec72ca908291c7d647e4fb01
-- 
2.14.3 (Apple Git-98)

[Qemu-devel] [PATCH v3] Virt: ACPI: fix qemu assert due to re-assigned table data address

[Shannon Zhao]

From: Zhaoshenglong 

acpi_data_push uses g_array_set_size to resize the memory size. If there
is no enough contiguous memory, the address will be changed. If we use
the old value, it will assert.
qemu-kvm: hw/acpi/bios-linker-loader.c:214: bios_linker_loader_add_checksum:
Assertion `start_offset < file->blob->len' failed.`

This issue only happens in building SRAT table now but here we unify the
pattern for other tables as well to avoid possible issues in the future.

Signed-off-by: Zhaoshenglong 
---
v3: unify the pattern for MCFG
v2: update the commit message according to Andrew's comments
---
 hw/arm/virt-acpi-build.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 3d78ff6..f7fa795 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -453,6 +453,7 @@ build_spcr(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 AcpiSerialPortConsoleRedirection *spcr;
 const MemMapEntry *uart_memmap = &vms->memmap[VIRT_UART];
 int irq = vms->irqmap[VIRT_UART] + ARM_SPI_BASE;
+int spcr_start = table_data->len;
 
 spcr = acpi_data_push(table_data, sizeof(*spcr));
 
@@ -476,8 +477,8 @@ build_spcr(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 spcr->pci_device_id = 0x;  /* PCI Device ID: not a PCI device */
 spcr->pci_vendor_id = 0x;  /* PCI Vendor ID: not a PCI device */
 
-build_header(linker, table_data, (void *)spcr, "SPCR", sizeof(*spcr), 2,
- NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + spcr_start),
+ "SPCR", table_data->len - spcr_start, 2, NULL, NULL);
 }
 
 static void
@@ -512,8 +513,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 mem_base += numa_info[i].node_mem;
 }
 
-build_header(linker, table_data, (void *)srat, "SRAT",
- table_data->len - srat_start, 3, NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + srat_start),
+ "SRAT", table_data->len - srat_start, 3, NULL, NULL);
 }
 
 static void
@@ -522,6 +523,7 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 AcpiTableMcfg *mcfg;
 const MemMapEntry *memmap = vms->memmap;
 int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
+int mcfg_start = table_data->len;
 
 mcfg = acpi_data_push(table_data, len);
 mcfg->allocation[0].address = cpu_to_le64(memmap[VIRT_PCIE_ECAM].base);
@@ -532,7 +534,8 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 mcfg->allocation[0].end_bus_number = (memmap[VIRT_PCIE_ECAM].size
   / PCIE_MMCFG_SIZE_MIN) - 1;
 
-build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + mcfg_start),
+ "MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
 }
 
 /* GTDT */
@@ -651,6 +654,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 static void build_fadt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms, unsigned dsdt_tbl_offset)
 {
+int fadt_start = table_data->len;
 AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt));
 unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt - table_data->data;
 uint16_t bootflags;
@@ -681,8 +685,8 @@ static void build_fadt(GArray *table_data, BIOSLinker 
*linker,
 ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt),
 ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
 
-build_header(linker, table_data,
- (void *)fadt, "FACP", sizeof(*fadt), 5, NULL, NULL);
+build_header(linker, table_data, (void *)(table_data->data + fadt_start),
+ "FACP", table_data->len - fadt_start, 5, NULL, NULL);
 }
 
 /* DSDT */
-- 
2.0.4

Re: [Qemu-devel] [PATCH] migration: fix small leaks

[Peter Xu]

On Wed, Dec 27, 2017 at 03:25:23PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> Hmm, looks like leak is not fixed here: I've checked it while running iotest
> 181, that
> migration_instance_finalize is not called.
> 
> If I understand correct, to call it we need unref current_migration object
> somewhere.
> 
> Or, may be I'm missing something..

I think you are right.

It does not matter much though since we don't dynamically allocate
migration object (there is only one and it lives forever).  Do you
want to post a patch?  I guess the safest place to unref it is at the
end of main() to make sure no one will be using it any more.

(Hmm, the incoming migration state is still static)

Thanks,

> 
> 01.08.2017 19:04, Marc-André Lureau wrote:
> > Spotted thanks to valgrind and tests/device-introspect-test:
> > 
> > ==11711== 1 bytes in 1 blocks are definitely lost in loss record 6 of 14,537
> > ==11711==at 0x4C2EB6B: malloc (vg_replace_malloc.c:299)
> > ==11711==by 0x1E0CDBD8: g_malloc (gmem.c:94)
> > ==11711==by 0x1E0E696E: g_strdup (gstrfuncs.c:363)
> > ==11711==by 0x695693: migration_instance_init (migration.c:2226)
> > ==11711==by 0x717C4B: object_init_with_type (object.c:344)
> > ==11711==by 0x717E80: object_initialize_with_type (object.c:375)
> > ==11711==by 0x7182EB: object_new_with_type (object.c:483)
> > ==11711==by 0x718328: object_new (object.c:493)
> > ==11711==by 0x4B8A29: qmp_device_list_properties (qmp.c:542)
> > ==11711==by 0x4A9561: qmp_marshal_device_list_properties 
> > (qmp-marshal.c:1425)
> > ==11711==by 0x819D4A: do_qmp_dispatch (qmp-dispatch.c:104)
> > ==11711==by 0x819E82: qmp_dispatch (qmp-dispatch.c:131)
> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >   migration/migration.c | 10 ++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 085c32c994..c3fe0ed9ca 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2214,6 +2214,15 @@ static void migration_class_init(ObjectClass *klass, 
> > void *data)
> >   dc->props = migration_properties;
> >   }
> > +static void migration_instance_finalize(Object *obj)
> > +{
> > +MigrationState *ms = MIGRATION_OBJ(obj);
> > +MigrationParameters *params = &ms->parameters;
> > +
> > +g_free(params->tls_hostname);
> > +g_free(params->tls_creds);
> > +}
> > +
> >   static void migration_instance_init(Object *obj)
> >   {
> >   MigrationState *ms = MIGRATION_OBJ(obj);
> > @@ -2282,6 +2291,7 @@ static const TypeInfo migration_type = {
> >   .class_size = sizeof(MigrationClass),
> >   .instance_size = sizeof(MigrationState),
> >   .instance_init = migration_instance_init,
> > +.instance_finalize = migration_instance_finalize,
> >   };
> >   static void register_migration_types(void)
> 
> 
> -- 
> Best regards,
> Vladimir
> 
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 05/17] timer: generalize Dallas/Maxim RTC i2c devices

[Michael Davidsaver]

On 12/06/2017 05:14 AM, David Gibson wrote:
> On Sun, Dec 03, 2017 at 03:15:10PM -0600, Michael Davidsaver wrote:
>> On 11/29/2017 11:13 PM, David Gibson wrote:
>>> On Sun, Nov 26, 2017 at 03:59:03PM -0600, Michael Davidsaver wrote:
 Support for: ds1307, ds1337, ds1338, ds1339,
 ds1340, ds1375, ds1388, and ds3231.

 Tested with ds1338 and ds1375.

 Signed-off-by: Michael Davidsaver 
>>>
>>> I certainly like the idea of consolidating this code, but reviewing to
>>> see that the new code really is a generalization of the old is
>>> something I won't have time for for a while.
>>>
>>> Also, hw/timer is not within my purview so it'll probably need to go
>>> another path to merge.
>>
>> Could you be a bit more explicit about what, if anything, I need to do
>> to move this forward?
> 
> Ugh.. that's pretty tough, since ds1338 doesn't have an activate
> maintainer.  You can look at the git history for some possible
> candidates of people to ask about it, but it hasn't been touched much
> in quite a while.
> 
> One approach that could help is to re-order so that your testing
> rework goes before the change to ds1338.  If your new generalization
> can pass the same set of tests as the original ds1338 code, that's at
> least a good start on being convincing that it's a true superset of
> the previous functionality.

A slight wrinkle is that my testing found two bugs with the original
ds1338 model (also one in my new code).  The two don't seem
significant practically.  It actually isn't possible to switch to
12-hour mode.  And there is an obscure off by one situation possible
with wday_offset and timezones.  Of course most guests use 24-hour mode
and ignore the RTC day-of-week.

The upshot of this is that several test cases now fail when run against 
ds1338.c.

My recommendation would be to start by looking at the test code.
This could be compared against guest code.  When I send the next
iteration I'll include some links.

> The other approach is to do the rework in a rather longer series of
> patches.  Start by simply moving ds1338.c, then do a mechanical
> replacement of the names within it, then start generalizing and
> altering.  That's a lot of work for you, but it makes it much easier
> to review each step

I know from my first foray into QEMU land that this is preferable for review.
Unfortunately, I expanded from my previous ds1375 model instead of the ds1338.



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1358287] Re: -readconfig file doesn't interpret memory size correctly

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  -readconfig file doesn't interpret memory size correctly

Status in QEMU:
  Expired

Bug description:
  I'm running Qemu 2.1 and have issues with the config file format.

  Specifically Qemu wrote the following snippet with '-writeconfig':

  [memory]
size = "1024"

  However, upon starting a VM with this setting it only receives 128MiB
  (the default size).

  I'm reverting back to using the command line option now - that works.

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

[Qemu-devel] [Bug 1247478] Re: usb passthrough mass storage write data corruption

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  usb passthrough mass storage write data corruption

Status in QEMU:
  Expired

Bug description:
  the windows 7 professional guest writes to usb high speed mass storage 
devices connected via host-libusb
  in bulk packages of either size 20480 or 4096 (as far as the actual file data 
is concerned and
  except for the last packet for odd-sized files). The pattern is:
  3 times bulk out 20480
  1 time bulk out 4096

  and that repeats for files longer than 65536 bytes.

  the file on the usb disk is corrupted and it is always corrupt in the last 
4096 bytes of each
  20480 byte sized transfer. that means a file is corrupt at 16384-20480 and 
36864-40960 and
  57344-61440.
  and so on. and because the 4096 sized  bulk out is always error free, the 
next corrupt span is from
  81920-86016.

  the last 4096 bytes of the 20480 sized transfer is always identical to the 
first 4096 bytes of the same
  transfer.

  to reproduce: run windows7 guest on and pass through usb2.0 disk with 
host-libusb. write a large file.
  (possibly check the bulk transfer sizes with usbmon).
  note that attaching usb disks with hw/usb/dev-storage does work just fine.
  cannot reproduce with linux as it always writes just 4096 bytes and writes 
with a linux guest are
  always ok even with usb passthrough.

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

[Qemu-devel] [Bug 1297781] Re: Network device cannot communicate with host machine

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  Network device cannot communicate with host machine

Status in QEMU:
  Expired

Bug description:
  I know this used to work but it doesnt work any more using qemu 1.4.2  on 
fedora 19 everything works fine
  except when i add a NIC sharing the main interface from the host (not the 
virtual network)

  the hosts ip is 10.0.0.4, the router is 10.0.0.1 so when i boot my virtual 
machine it gets an ip from the router no problem 
  i get on the internet fine, and i can connect (ping,ftp, samba whatever) to 
all the computers on the network except the host
  10.0.0.4 i can't ping it, i cant do anything to it, but i can connect to all 
the other computers on the network and i know i used to be able to do this 
because thats how i shared files between the host and windows guest, with 
samba... but now i can't browse the host computer because it can't 
communicate... please help.

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

[Qemu-devel] [Bug 1279500] Re: system_powerdown causes SMP OpenBSD guest to freeze

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  system_powerdown causes SMP OpenBSD guest to freeze

Status in QEMU:
  Expired

Bug description:
  system_powerdown causes an SMP OpenBSD guest to freeze. I can
  reproduce it with the following systems/versions:

- Debian 6: QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5)
- Fedora 20:
   qemu-system-x86-1.6.1 (from Fedora repository)
   qemu-1.7.0 (latest release version)
   qemu-1.7.50 (latest development snapshot, "git cloned" today, 20140212)

  all of the above hosts are running x86_64 linux.

  The first OpenBSD version that I ran as a VM, v5.1, experienced the
  problem. All subsequent versions experience the problem. The above
  tests were performed using OpenBSD v5.4 (amd64).

  I will open an OpenBSD bug report for this problem as well, and update
  this report with the OpenBSD bug ID.

  There's an interesting RedHat bug report concerning this problem:
URL: https://bugzilla.redhat.com/show_bug.cgi?id=508801#c34

  Here an excerpt:
  -snip-
  Gleb Natapov 2009-12-23 10:37:44 EST

  I posted patch to provide correct PCI irq routing info in mptable to kvm 
  mailing list. It works for all devices except for SCI interrupt. BIOS
  programs SCI interrupt to be 9 as spec requires, but OpenBSD thinks that
  it is smarter and moves it to interrupts 10. Qemu will still send it on
  vector 9 and OpenBSD will enter the same infinity recursion. This can
  be triggered by issuing system_powerdown on qemu monitor.
  -snip-

  Michael Tokarev reported this problem on the kvm mailing list in 2011:
URL: http://www.spinics.net/lists/kvm/msg51311.html

  I compiled qemu as follows:
  -snip-
  cd qemu-src-dir
  mkdir -p bin/native
  cd bin/native
  ../../configure \
--prefix=/usr/local/qemu-dev-snapshot-20140212 \
--target-list=x86_64-softmmu \
--enable-kvm \
--enable-spice \
--with-gtkabi="3.0" \
--audio-drv-list=pa,sdl,alsa,oss \
--extra-cflags='-I/usr/include/SDL'
  -snip-

  I'm running OpenBSD with the following command:
  -snip-
  #!/bin/bash

  DEF=/usr/bin/qemu-system-x86_64
  QEMU_LATEST=/usr/local/qemu-1.7.0/bin/qemu-system-x86_64
  QEMU_DEV=/usr/local/qemu-dev-snapshot-20140212/bin/qemu-system-x86_64

  $QEMU_DEV \
-machine accel=kvm \
-name obsdtest-v54 \
-S \
-machine pc-i440fx-1.6,accel=kvm,usb=off \
-boot c \
-m 2048 \
-realtime mlock=off \
-smp 2,sockets=2,cores=1,threads=1 \
-uuid 8b685793-2510-473e-b97e-822a4cf2fbca \
-no-user-config \
-monitor stdio \
-rtc base=utc,driftfix=slew \
-global kvm-pit.lost_tick_policy=discard \
-no-hpet \
-drive 
file=/guest_images/obsdtest_v54.raw,if=none,id=drive-virtio-disk0,format=raw,cache=none
 \
-device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 \
-drive if=none,id=drive-ide0-0-0,readonly=on,format=raw \
-device ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-chardev pty,id=charserial0 \
-device isa-serial,chardev=charserial0,id=serial0 \
-k en-us \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x3 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 \
-net nic \
-net user
  -snip-

  The OpenBSD disk image I used for testing is 143MB compressed, 10G
  uncompressed. It can be found here:

http://www.spielwiese.de/OpenBSD/obsd54.raw.7z

  The root password is "x".

  Rob Urban

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

[Qemu-devel] [Bug 1336123] Re: bad switch, segfault in hw/pci-host/bonito.c bonito_readl

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  bad switch, segfault in hw/pci-host/bonito.c bonito_readl

Status in QEMU:
  Expired

Bug description:
  http://git.qemu.org/?p=qemu.git;a=blob;f=hw/pci-
  host/bonito.c;h=56292adb03cd1a9873c2c9e5a0b2978fd0572214;hb=master#l301

  The switch statement is error-prone, since two branches return the
  same result.

  Segfault reproducing steps:
  1. make a Linux kernel(for example 3.16.0-rc2) with fuloong2e_defconfig
  2. use 'qemu-system-mips64el -machine fulong2e' to boot the vmlinux

  qemu versions tried: 2.0.0, 1.6.2

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

[Qemu-devel] [Bug 1239008] Re: qemu fails to scroll screen on ^Vidmem output

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  qemu fails to scroll screen on ^Vidmem output

Status in QEMU:
  Expired
Status in qemu package in Ubuntu:
  Expired

Bug description:
  Pascal uses ^Vidmem for B800 console output. The terminal does not
  oblige the Pascal OS code to scroll the output. Virtualbox emulation
  works, so this must be a qemu bug. Using QEMU in KVM mode as Ubuntu
  LTS.

  Source line to trip bug(in theory pushes VideoMem up one line):

  procedure Scroll;
  //this is whats causing crashes. FIXME:Virtualbox not affected.QEMU BUG?
  begin
if scrolldisabled then exit;
if (CursorPosY >= 24) then begin  //in case called before end of screen
  blank:= $20 or (TextAttr shl 8);
  Move((VidMem+(2*80))^,VidMem^,24*(2*80));
  // Empty last line
  FillWord((VidMem+(24*2*80))^,80,Blank);
  CursorPosX:=1;
  CursorPosY:=23;
  update_cursor;
end;
  end;

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

[Qemu-devel] [Bug 1332297] Re: qemu-img: crash on check of an image with large value in the 'size' header field

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  qemu-img: crash on check of an image with large value in the 'size'
  header field

Status in QEMU:
  Expired

Bug description:
  The qemu-img crashes on the next command:

  qemu-img check test_image

  'test_image' can be found in the attachment. It's a fuzzed test image
  with the qcow2 image header only. Suppositional cause of the failure
  is the value of 'size' header field set to maximum uint_64 value.

  System information:

  qemu.git: 6baa963f4dcc2118
  Host: Linux 3.14.7-200.fc20.x86_64 #1 SMP Wed Jun 11 22:38:05 UTC 2014 x86_64 
 GNU/Linux

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

[Qemu-devel] [Bug 1007490] Re: Missing binfmt string for init script.

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  Missing binfmt string for init script.

Status in QEMU:
  Expired

Bug description:
  ./scripts/qemu-binfmt-conf.sh

  needs

  echo
  
':armCompiler:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x08\x00\x02\x00\x28\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/usr/bin
  /qemu-static-arm-binfmt:P' > /proc/sys/fs/binfmt_misc/register

  Some executables (specifically compilers like /usr/libexec/gcc/armv7a-
  unknown-linux-gnueabi/4.5.3/cc1 on gentoo) have unusual headers, and
  don't get recognized as arm binaries.

  Bug also mentioned on my blog:
  
http://mirage335.dyndns.org/forums/viewtopic.php?f=4&t=11&sid=01f0ca9cc76c78b6f600fa25cc99d62b

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

[Qemu-devel] [Bug 1292037] Re: Solaris 10 x86 guest crashes qemu with -icount 1 option

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  Solaris 10 x86 guest crashes qemu with -icount 1 option

Status in QEMU:
  Expired

Bug description:
  Solaris image: Solaris 10 x86 (32 bit)

  command: ./i386-softmmu/qemu-system-i386 -hda  -m 2G
  -icount 1 -monitor stdio

  Crashes saying:
  qemu: Fatal: Raised interrupt while not in I/O function

  Host:
  ubuntu x86_64 3.2.0-56 generic
  intel xeon E5649 @ 2.53GHz

  UPDATE:
  Tested on QEMU v2.0.50
  Also affects OpenIndiana (151a8 - Server Build 32bit)
  http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg06365.html

  Workaround: Rename the kvmvapic.bin file under the pc-bios directory
  to something different.

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

[Qemu-devel] [Bug 1276879] Re: lots of dma command 10, 14 not supported

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  lots of dma command 10, 14 not supported

Status in QEMU:
  Expired

Bug description:
  Trying to install NeXTSTEP 3.3 onto a 2GB file with QEMU 1.7.0.
  In the terminal that started QEMU, there are a lot of
  dma: command 10 not supported
  and 
  dma: command 14 not supported
  messages.  When the installation of NeXTSTEP gets to
  'preparing disk for nextstep installation', there are a lot
  of messages that ATA command c5 failed and other info.
  The result is a failed installation.

  Is this a bug in QEMU?  Is there a workaround, e.g. by
  disabling DMA altogether?

  thank you

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

[Qemu-devel] [Bug 1272796] Re: Windows 98 First Edition emulation problems

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

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

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

Title:
  Windows 98 First Edition emulation problems

Status in QEMU:
  Expired

Bug description:
  System: Debian SID x86 with latest updates

  1) QEMU compiled from latest main GIT branch(at the time of writing, 1.7.50) 
(and 1.7 stable version)
  ./configure options: ./configure --enable-sdl --target-list=i386-softmmu 
--cpu=i686 --audio-drv-list=alsa

  When you try to boot Windows 98 First Edition (Italian), it does not simply 
boot. It stays on booting screen.
  If you try to install, the installation goes flawless, but when it boots it 
freeze.

  I am launching VM with this: qemu-system-i386 -hda main.img -cpu
  pentium -m 256 -fda floppy1.img -boot c -soundhw gus -vga cirrus

  I have tried with -M option "pc-i440fx-1.6" since 1.6 have no problems
  with the booting of Win98, but nothing. No fix found.

  2) QEMU 1.6.2 (same compile and launching options)
  gus soundboard seems not recognized even with real dos drivers (tried to 
install theme into real dos mode).
  with SoundBlaster 16 i have following error: WARNING: I/O thread spun for 
1000 iterations, making the emulation impossible (too slow, and sound is 
stuttering) . Tried to compile with oss and sdl option on audio-drv-list but no 
fix found.

  Any ideas? thank you

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

[Qemu-devel] [Bug 1239008] Re: qemu fails to scroll screen on ^Vidmem output

[Launchpad Bug Tracker]

[Expired for qemu (Ubuntu) because there has been no activity for 60
days.]

** Changed in: qemu (Ubuntu)
   Status: Incomplete => Expired

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

Title:
  qemu fails to scroll screen on ^Vidmem output

Status in QEMU:
  Expired
Status in qemu package in Ubuntu:
  Expired

Bug description:
  Pascal uses ^Vidmem for B800 console output. The terminal does not
  oblige the Pascal OS code to scroll the output. Virtualbox emulation
  works, so this must be a qemu bug. Using QEMU in KVM mode as Ubuntu
  LTS.

  Source line to trip bug(in theory pushes VideoMem up one line):

  procedure Scroll;
  //this is whats causing crashes. FIXME:Virtualbox not affected.QEMU BUG?
  begin
if scrolldisabled then exit;
if (CursorPosY >= 24) then begin  //in case called before end of screen
  blank:= $20 or (TextAttr shl 8);
  Move((VidMem+(2*80))^,VidMem^,24*(2*80));
  // Empty last line
  FillWord((VidMem+(24*2*80))^,80,Blank);
  CursorPosX:=1;
  CursorPosY:=23;
  update_cursor;
end;
  end;

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

Re: [Qemu-devel] [PATCH v9 01/13] block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Enabling bitmap successor is necessary to enable successors of bitmaps
> being migrated before target vm start.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: John Snow 
> ---
>  include/block/dirty-bitmap.h | 1 +
>  block/dirty-bitmap.c | 8 
>  2 files changed, 9 insertions(+)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 3579a7597c..93d4336505 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -20,6 +20,7 @@ BdrvDirtyBitmap 
> *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
>  BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
> BdrvDirtyBitmap *bitmap,
> Error **errp);
> +void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap);
>  BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
>  const char *name);
>  void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index bd04e991b1..81adbeb6d4 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -237,6 +237,14 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState 
> *bs,
>  return 0;
>  }
>  
> +/* Called with BQL taken. */
> +void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
> +{
> +qemu_mutex_lock(bitmap->mutex);
> +bdrv_enable_dirty_bitmap(bitmap->successor);
> +qemu_mutex_unlock(bitmap->mutex);
> +}
> +
>  /**
>   * For a bitmap with a successor, yield our name to the successor,
>   * delete the old bitmap, and return a handle to the new bitmap.
> -- 
> 2.11.1
> 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 02/13] block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Like other setters here these functions should take a lock.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  include/block/dirty-bitmap.h |  3 +++
>  block/dirty-bitmap.c | 25 -
>  2 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 93d4336505..caf1f3d861 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -92,5 +92,8 @@ bool bdrv_has_changed_persistent_bitmaps(BlockDriverState 
> *bs);
>  BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
>  BdrvDirtyBitmap *bitmap);
>  char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
> +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
> +  BdrvDirtyBitmap *bitmap,
> +  Error **errp);
>  
>  #endif
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index d83da077d5..fe27ddfb83 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -327,15 +327,11 @@ BdrvDirtyBitmap 
> *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
>   * The merged parent will be un-frozen, but not explicitly re-enabled.
>   * Called with BQL taken.

Maybe update the comment as

s/Called with BQL taken/Called within bdrv_dirty_bitmap_lock..unlock/

and ...

>   */
> -BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
> -   BdrvDirtyBitmap *parent,
> -   Error **errp)
> +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
> +  BdrvDirtyBitmap *parent,
> +  Error **errp)
>  {
> -BdrvDirtyBitmap *successor;
> -
> -qemu_mutex_lock(parent->mutex);
> -
> -successor = parent->successor;
> +BdrvDirtyBitmap *successor = parent->successor;
>  
>  if (!successor) {
>  error_setg(errp, "Cannot reclaim a successor when none is present");
> @@ -349,9 +345,20 @@ BdrvDirtyBitmap 
> *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
>  bdrv_release_dirty_bitmap_locked(bs, successor);
>  parent->successor = NULL;
>  
> +return parent;
> +}
> +

... move the "Called with BQL taken" comment here?

> +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
> +   BdrvDirtyBitmap *parent,
> +   Error **errp)
> +{
> +BdrvDirtyBitmap *ret;
> +
> +qemu_mutex_lock(parent->mutex);
> +ret = bdrv_reclaim_dirty_bitmap_locked(bs, parent, errp);
>  qemu_mutex_unlock(parent->mutex);
>  
> -return parent;
> +return ret;
>  }
>  
>  /**
> -- 
> 2.11.1
> 

[Qemu-devel] [PATCH v14 7/9] ARM: ACPI: Add GPIO notification type for hardware RAS error

[Dongjiu Geng]

In ARM platform we implements a notification of error events
via a GPIO pin. For this GPIO-signaled events, we choose GPIO
pin 4 for hardware error device (PNP0C33), So _E04 should be
added to ACPI DSDT table. When GPIO-pin 4 signaled a events,
the guest ACPI driver will receive this notification and
handing the error.

In order to better trigger the GPIO IRQ, we defined a notifier
hardware_error_notifiers. If Qemu wants to deliver a GPIO-Signal
notification, will call it.

Signed-off-by: Dongjiu Geng 
---
1. Address discussion result about guest APEI notification type for 
SIGBUS_MCEERR_AO SIGBUS in [1],
   the discussion conclusion is using GPIO-Signal

[1]:
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03397.html
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03467.html
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03601.html
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03775.html

2. The ASL dump for the GPIO and hardware error device


Device (GPO0)
{
Name (_AEI, ResourceTemplate ()  // _AEI: ACPI Event Interrupts
{
.
GpioInt (Edge, ActiveHigh, Exclusive, PullUp, 0x,
"GPO0", 0x00, ResourceConsumer, ,
)
{   // Pin list
0x0004
}
})
Method (_E04, 0, NotSerialized)  // _Exx: Edge-Triggered GPE
{
Notify (ERRD, 0x80) // Status Change
}
}
Device (ERRD)
{
Name (_HID, EisaId ("PNP0C33") /* Error Device */)  // _HID: Hardware ID
Name (_UID, Zero)  // _UID: Unique ID
Method (_STA, 0, NotSerialized)  // _STA: Status
{
Return (0x0F)
}
}

3. Below is the guest log when Qemu notifies guest using GPIO-signal after 
record a CPER
[  504.164899] {1}[Hardware Error]: Hardware error from APEI Generic Hardware 
Error Source: 7
[  504.166970] {1}[Hardware Error]: event severity: recoverable
[  504.251650] {1}[Hardware Error]:  Error 0, type: recoverable
[  504.252974] {1}[Hardware Error]:   section_type: memory error
[  504.254380] {1}[Hardware Error]:   physical_address: 0x03ec
[  504.255879] {1}[Hardware Error]:   error_type: 3, multi-bit ECC
---
 hw/arm/virt-acpi-build.c | 31 ++-
 hw/arm/virt.c| 18 ++
 include/sysemu/sysemu.h  |  3 +++
 vl.c | 12 
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index b7d45cd..06c14b3 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -49,6 +49,7 @@
 
 #define ARM_SPI_BASE 32
 #define ACPI_POWER_BUTTON_DEVICE "PWRB"
+#define ACPI_HARDWARE_ERROR_DEVICE "ERRD"
 
 static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
 {
@@ -340,7 +341,13 @@ static void acpi_dsdt_add_gpio(Aml *scope, const 
MemMapEntry *gpio_memmap,
 
 Aml *aei = aml_resource_template();
 /* Pin 3 for power button */
-const uint32_t pin_list[1] = {3};
+uint32_t pin_list[1] = {3};
+aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
+ AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
+ "GPO0", NULL, 0));
+
+/* Pin 4 for hardware error device */
+pin_list[0] = 4;
 aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
  AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
  "GPO0", NULL, 0));
@@ -351,6 +358,13 @@ static void acpi_dsdt_add_gpio(Aml *scope, const 
MemMapEntry *gpio_memmap,
 aml_append(method, aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
   aml_int(0x80)));
 aml_append(dev, method);
+
+/* _E04 is handle for hardware error */
+method = aml_method("_E04", 0, AML_NOTSERIALIZED);
+aml_append(method, aml_notify(aml_name(ACPI_HARDWARE_ERROR_DEVICE),
+  aml_int(0x80)));
+aml_append(dev, method);
+
 aml_append(scope, dev);
 }
 
@@ -363,6 +377,20 @@ static void acpi_dsdt_add_power_button(Aml *scope)
 aml_append(scope, dev);
 }
 
+static void acpi_dsdt_add_error_device(Aml *scope)
+{
+Aml *dev = aml_device(ACPI_HARDWARE_ERROR_DEVICE);
+Aml *method;
+
+aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C33")));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+aml_append(method, aml_return(aml_int(0x0f)));
+aml_append(dev, method);
+aml_append(scope, dev);
+}
+
 /* RSDP */
 static GArray *
 build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
@@ -716,6 +744,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
(irqmap[VIRT_GPIO] + ARM_SPI_BASE));
 acpi_dsdt_add_power_button(scope);
+acpi_dsdt_add_error_device(scope);
 
 aml_app

[Qemu-devel] [PATCH v14 4/9] ACPI: enable APEI GHES in the configure file

[Dongjiu Geng]

Add CONFIG_ACPI_APEI configuration in the arm-softmmu.mak
and add build choice in the Makefile.objs.

Signed-off-by: Dongjiu Geng 
---
 default-configs/arm-softmmu.mak | 1 +
 hw/acpi/Makefile.objs   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index bbdd3c1..c362113 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -129,3 +129,4 @@ CONFIG_ACPI=y
 CONFIG_SMBIOS=y
 CONFIG_ASPEED_SOC=y
 CONFIG_GPIO_KEY=y
+CONFIG_ACPI_APEI=y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 11c35bc..bafb148 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
 common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
 common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
+common-obj-$(CONFIG_ACPI_APEI) += hest_ghes.o
 common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
 
 common-obj-y += acpi_interface.o
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 5/9] target-arm: kvm64: inject synchronous External Abort

[Dongjiu Geng]

Add synchronous external abort injection logic, setup
exception type and syndrome value. When switch to guest,
guest will jump to the synchronous external abort vector
table entry.

The ESR_ELx.DFSC is set to synchronous external abort(0x10),
and ESR_ELx.FnV is set to not valid(0x1), which will tell
guest that FAR is not valid and holds an UNKNOWN value.
These value will be set to KVM register structures through
KVM_SET_ONE_REG IOCTL.

Signed-off-by: Dongjiu Geng 
---
Marc is against that KVM inject the synchronous external abort(SEA) in [1],
so user space how to inject it. The test result that injection SEA to guest by 
Qemu
is shown in [2].

[1]: https://lkml.org/lkml/2017/3/2/110
[2]:
Taking exception 4 [Data Abort]
...from EL0 to EL1
...with ESR 0x24/0x92000410
...with FAR 0x0
...with ELR 0x40cf04
...to EL1 PC 0xffc84c00 PSTATE 0x3c5
after kvm_inject_arm_sea
Unhandled fault: synchronous external abort (0x92000410) at 0x007fa234c12c
CPU: 0 PID: 536 Comm: devmem Not tainted 4.1.0+ #20
Hardware name: linux,dummy-virt (DT)
task: ffc019ab2b00 ti: ffc008134000 task.ti: ffc008134000
PC is at 0x40cf04
LR is at 0x40cdec
pc : [<0040cf04>] lr : [<0040cdec>] pstate: 6000
sp : 007ff7b24130
x29: 007ff7b24260 x28: 
x27: 00ad x26: 0049c000
x25: 0048904b x24: 0049c000
x23: 4060 x22: 007ff7b243a0
x21: 0002 x20: 
x19: 0020 x18: 
x17: 0049c6d0 x16: 007fa22c85c0
x15: 5798 x14: 007fa2205f1c
x13: 007fa241ccb0 x12: 0137
x11:  x10: 
x9 :  x8 : 00de
x7 :  x6 : 2000
x5 : 4060 x4 : 0003
x3 : 0001 x2 : 
x1 :  x0 : 007fa2418000
---
 target/arm/kvm64.c | 65 ++
 1 file changed, 65 insertions(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index a16abc8..c00450d 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -582,6 +582,71 @@ int kvm_arm_cpreg_level(uint64_t regidx)
 return KVM_PUT_RUNTIME_STATE;
 }
 
+static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t fieldoffset)
+{
+int i;
+
+for (i = 0; i < cpu->cpreg_array_len; i++) {
+uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
+const ARMCPRegInfo *ri;
+ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
+if (!ri) {
+continue;
+}
+
+if (ri->type & ARM_CP_NO_RAW) {
+continue;
+}
+
+if (ri->fieldoffset == fieldoffset) {
+cpu->cpreg_values[i] = read_raw_cp_reg(&cpu->env, ri);
+return 0;
+}
+}
+return -EINVAL;
+}
+
+/* Inject synchronous external abort */
+static void kvm_inject_arm_sea(CPUState *c)
+{
+ARMCPU *cpu = ARM_CPU(c);
+CPUARMState *env = &cpu->env;
+unsigned long cpsr = pstate_read(env);
+uint32_t esr, ret;
+
+/* This exception is synchronous data abort*/
+c->exception_index = EXCP_DATA_ABORT;
+/* Inject the exception to guest El1 */
+env->exception.target_el = 1;
+CPUClass *cc = CPU_GET_CLASS(c);
+
+/* Set the DFSC to synchronous external abort and set FnV to not valid,
+ * this will tell guest the FAR_ELx is UNKNOWN for this abort.
+ */
+esr = (0x10 | (1 << 10));
+
+/* This exception comes from lower or current exception level. */
+if ((cpsr & 0xf) == PSTATE_MODE_EL0t) {
+esr |= (EC_DATAABORT << ARM_EL_EC_SHIFT);
+} else {
+esr |= (EC_DATAABORT_SAME_EL << ARM_EL_EC_SHIFT);
+}
+
+/* For the AArch64, instruction length is 32-bit */
+esr |= ARM_EL_IL;
+env->exception.syndrome = esr;
+
+cc->do_interrupt(c);
+
+/* set ESR_EL1 */
+ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1]));
+
+if (ret) {
+fprintf(stderr, "<%s> failed to set esr_el1\n", __func__);
+abort();
+}
+}
+
 #define AARCH64_CORE_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
  KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 9/9] target-arm: kvm64: handle SIGBUS signal from kernel or KVM

[Dongjiu Geng]

Add SIGBUS signal handler. In this handler, it checks the SIGBUS type,
translates the host VA which is delivered by host to guest PA, then fill
this PA to CPER and fill the CPER to guest APEI GHES memory, finally
notify guest according to the SIGBUS type. There are two kinds of SIGBUS
that QEMU needs to handle, which are BUS_MCEERR_AO and BUS_MCEERR_AR.

If guest accesses the poisoned memory, it generates Synchronous External
Abort(SEA). Then host kernel gets an APEI notification and call memory_failure()
to unmapped the affected page from the guest's stage2, and SIGBUS_MCEERR_AO
is delivered to Qemu's main thread. If Qemu receives this SIGBUS, it will
create a new CPER and add it to guest APEI GHES memory, then notify the
guest with a GPIO-Signal notification.

When guest hits a PG_hwpoison page, it will trap to KVM as stage2 fault, then a
SIGBUS_MCEERR_AR synchronous signal is delivered to Qemu, Qemu record this error
into guest APEI GHES memory and notify guest using 
Synchronous-External-Abort(SEA).

Suggested-by: James Morse 
Signed-off-by: Dongjiu Geng 
---
Address James's comments to record CPER and notify guest for SIGBUS signal 
handling.
Shown some discussion in [1].

[1]:
https://lkml.org/lkml/2017/2/27/246
https://lkml.org/lkml/2017/9/14/241
https://lkml.org/lkml/2017/9/22/499
---
 include/sysemu/kvm.h |  2 +-
 target/arm/kvm.c |  2 ++
 target/arm/kvm64.c   | 34 ++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 3a458f5..90c1605 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -361,7 +361,7 @@ bool kvm_vcpu_id_is_valid(int vcpu_id);
 /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
 unsigned long kvm_arch_vcpu_id(CPUState *cpu);
 
-#ifdef TARGET_I386
+#if defined(TARGET_I386) || defined(TARGET_AARCH64)
 #define KVM_HAVE_MCE_INJECTION 1
 void kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
 #endif
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 7c17f0d..9d25f51 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -26,6 +26,7 @@
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
 #include "qemu/log.h"
+#include "exec/ram_addr.h"
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 KVM_CAP_LAST_INFO
@@ -182,6 +183,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 
 cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
 
+qemu_register_reset(kvm_unpoison_all, NULL);
 type_register_static(&host_arm_cpu_type_info);
 
 return 0;
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index c00450d..6955d85 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -27,6 +27,9 @@
 #include "kvm_arm.h"
 #include "internals.h"
 #include "hw/arm/arm.h"
+#include "exec/ram_addr.h"
+#include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/hest_ghes.h"
 
 static bool have_guest_debug;
 
@@ -944,6 +947,37 @@ int kvm_arch_get_registers(CPUState *cs)
 return ret;
 }
 
+void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
+{
+ram_addr_t ram_addr;
+hwaddr paddr;
+
+assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
+if (addr) {
+ram_addr = qemu_ram_addr_from_host(addr);
+if (ram_addr != RAM_ADDR_INVALID &&
+kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
+kvm_hwpoison_page_add(ram_addr);
+if (code == BUS_MCEERR_AR) {
+kvm_cpu_synchronize_state(c);
+ghes_record_errors(ACPI_HEST_NOTIFY_SEA, paddr);
+kvm_inject_arm_sea(c);
+} else if (code == BUS_MCEERR_AO) {
+ghes_record_errors(ACPI_HEST_NOTIFY_GPIO, paddr);
+qemu_hardware_error_notify();
+}
+return;
+}
+fprintf(stderr, "Hardware memory error for memory used by "
+"QEMU itself instead of guest system!\n");
+}
+
+if (code == BUS_MCEERR_AR) {
+fprintf(stderr, "Hardware memory error!\n");
+exit(1);
+}
+}
+
 /* C6.6.29 BRK instruction */
 static const uint32_t brk_insn = 0xd420;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 6/9] Move related hwpoison page functions to accel/kvm/ folder

[Dongjiu Geng]

kvm_hwpoison_page_add() and kvm_unpoison_all() will be used both
by X86 and ARM platforms, so move these functions to a common
accel/kvm/ folder to avoid duplicate code.

Signed-off-by: Dongjiu Geng 
---
Address Peter's comments to move related hwpoison page function to accel/kvm 
folder in [1]
Address Paolo's comments to move HWPoisonPage definition back to 
accel/kvm/kvm-all.c

[1]:
https://lists.gnu.org/archive/html/qemu-arm/2017-09/msg00077.html
https://lists.gnu.org/archive/html/qemu-arm/2017-09/msg00152.html
---
 accel/kvm/kvm-all.c | 33 +
 include/exec/ram_addr.h |  5 +
 target/i386/kvm.c   | 33 -
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 46ce479..2412ea6 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -564,6 +564,39 @@ int kvm_vm_check_extension(KVMState *s, unsigned int 
extension)
 return ret;
 }
 
+typedef struct HWPoisonPage {
+ram_addr_t ram_addr;
+QLIST_ENTRY(HWPoisonPage) list;
+} HWPoisonPage;
+
+static QLIST_HEAD(, HWPoisonPage) hwpoison_page_list =
+QLIST_HEAD_INITIALIZER(hwpoison_page_list);
+
+void kvm_unpoison_all(void *param)
+{
+HWPoisonPage *page, *next_page;
+
+QLIST_FOREACH_SAFE(page, &hwpoison_page_list, list, next_page) {
+QLIST_REMOVE(page, list);
+qemu_ram_remap(page->ram_addr, TARGET_PAGE_SIZE);
+g_free(page);
+}
+}
+
+void kvm_hwpoison_page_add(ram_addr_t ram_addr)
+{
+HWPoisonPage *page;
+
+QLIST_FOREACH(page, &hwpoison_page_list, list) {
+if (page->ram_addr == ram_addr) {
+return;
+}
+}
+page = g_new(HWPoisonPage, 1);
+page->ram_addr = ram_addr;
+QLIST_INSERT_HEAD(&hwpoison_page_list, page, list);
+}
+
 static uint32_t adjust_ioeventfd_endianness(uint32_t val, uint32_t size)
 {
 #if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index d017639..41d9c37 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -80,6 +80,11 @@ void qemu_ram_free(RAMBlock *block);
 
 int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp);
 
+/* Add a poisoned page to the list */
+void kvm_hwpoison_page_add(ram_addr_t ram_addr);
+/* Free and remove all the poisoned pages in the list */
+void kvm_unpoison_all(void *param);
+
 #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1)
 #define DIRTY_CLIENTS_NOCODE  (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE))
 
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6db7783..3e1afb6 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -390,39 +390,6 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, 
uint32_t function,
 return ret;
 }
 
-typedef struct HWPoisonPage {
-ram_addr_t ram_addr;
-QLIST_ENTRY(HWPoisonPage) list;
-} HWPoisonPage;
-
-static QLIST_HEAD(, HWPoisonPage) hwpoison_page_list =
-QLIST_HEAD_INITIALIZER(hwpoison_page_list);
-
-static void kvm_unpoison_all(void *param)
-{
-HWPoisonPage *page, *next_page;
-
-QLIST_FOREACH_SAFE(page, &hwpoison_page_list, list, next_page) {
-QLIST_REMOVE(page, list);
-qemu_ram_remap(page->ram_addr, TARGET_PAGE_SIZE);
-g_free(page);
-}
-}
-
-static void kvm_hwpoison_page_add(ram_addr_t ram_addr)
-{
-HWPoisonPage *page;
-
-QLIST_FOREACH(page, &hwpoison_page_list, list) {
-if (page->ram_addr == ram_addr) {
-return;
-}
-}
-page = g_new(HWPoisonPage, 1);
-page->ram_addr = ram_addr;
-QLIST_INSERT_HEAD(&hwpoison_page_list, page, list);
-}
-
 static int kvm_get_mce_cap_supported(KVMState *s, uint64_t *mce_cap,
  int *max_banks)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 0/9] Add ARMv8 RAS virtualization support in QEMU

[Dongjiu Geng]

From: gengdongjiu 

In the ARMv8 platform, the CPU error type are synchronous external
abort(SEA) and SError Interrupt (SEI). If exception happens to guest,
sometimes  guest itself do the recovery is better, because host 
does not know guest's detailed information. For example, if a guest
user-space application happen exception, host doe not which application
encounter errors.

For the ARMv8 SEA/SEI, KVM or host kernel will deliver SIGBUS or
use other interface to notify user space. After user space gets 
the notification, it will record the CPER to guest GHES buffer
for guest and inject a exception or IRQ to KVM.

In the current implement, if the SIGBUS is BUS_MCEERR_AR, we will
treat it as synchronous exception, and use ARMv8 SEA notification type
to notify guest after recording CPER for guest; If the SIGBUS is
BUS_MCEERR_AO, we will treat it as asynchronous exception, and use
GPIO-Signal to notify guest after recording CPER for guest.

This series patches are based on Qemu 2.10, which have two parts:
1. Generate APEI/GHES table and record CPER for guest in runtime.
2. Handle the SIGBUS signal, record the CPER and fill into guest memory,
   then according to SIGBUS type(BUS_MCEERR_AR or BUS_MCEERR_AO), using
   different ACPI notification type to notify guest.

Whole solution was suggested by James(james.mo...@arm.com); inject RAS SEA 
abort and specify guest ESR
in user space are suggested by Marc(marc.zyng...@arm.com), APEI part solution 
is suggested by
Laszlo(ler...@redhat.com). Shown some discussion in [1].


This series patches have already tested on ARM64 platform with RAS feature 
enabled:
Show the APEI part verification result in [2]
Show the BUS_MCEERR_AR and BUS_MCEERR_AO SIGBUS handling verification result in 
[3]

---
Change since v13:
1. Move the patches that set guest ESR and inject virtual SError out of this 
series
2. Clean and optimize the APEI part patches 
3. Update the commit messages and add some comments for the code

Change since v12:
1. Address Paolo's comments to move HWPoisonPage definition to 
accel/kvm/kvm-all.c
2. Only call kvm_cpu_synchronize_state() when get the BUS_MCEERR_AR signal
3. Only add and enable GPIO-Signal and ARMv8 SEA two hardware error sources
4. Address Michael's comments to not sync SPDX from Linux kernel header file 

Change since v11:
Address James's comments(james.mo...@arm.com)
1. Check whether KVM has the capability to to set ESR instead of detecting host 
CPU RAS capability
2. For SIGBUS_MCEERR_AR SIGBUS, use Synchronous-External-Abort(SEA) 
notification type
   for SIGBUS_MCEERR_AO SIGBUS, use GPIO-Signal notification


Address Shannon's comments(for ACPI part):
1. Unify hest_ghes.c and hest_ghes.h license declaration
2. Remove unnecessary including "qmp-commands.h" in hest_ghes.c
3. Unconditionally add guest APEI table based on James's 
comments(james.mo...@arm.com) 
4. Add a option to virt machine for migration compatibility. On new virt 
machine it's on
   by default while off for old ones, we enabled it since 2.10
5. Refer to the ACPI spec version which introduces Hardware Error Notification 
first time
6. Add ACPI_HEST_NOTIFY_RESERVED notification type

Address Igor's comments(for ACPI part):
1. Add doc patch first which will describe how it's supposed to work between 
QEMU/firmware/guest
   OS with expected flows.
2. Move APEI diagrams into doc/spec patch
3. Remove redundant g_malloc in ghes_record_cper()
4. Use build_append_int_noprefix() API to compose whole error status block and 
whole APEI table, 
   and try to get rid of most structures in patch 1, as they will be left 
unused after that
5. Reuse something like 
https://github.com/imammedo/qemu/commit/3d2fd6d13a3ea298d2ee814835495ce6241d085c
   to build GAS
6. Remove much offsetof() in the function
7. Build independent tables first and only then build dependent tables passing 
to it pointers
   to previously build table if necessary.
8. Redefine macro GHES_ACPI_HEST_NOTIFY_RESERVED to 
ACPI_HEST_ERROR_SOURCE_COUNT to avoid confusion


Address Peter Maydell's comments
1. linux-headers is done as a patch of their own created using 
scripts/update-linux-headers.sh run against a
   mainline kernel tree 
2. Tested whether this patchset builds OK on aarch32  
3. Abstract Hwpoison page adding code  out properly into a cpu-independent 
source file from target/i386/kvm.c,
   such as kvm-all.c
4. Add doc-comment formatted documentation comment for new globally-visible 
function prototype in a header

---
[1]:
https://lkml.org/lkml/2017/2/27/246
https://patchwork.kernel.org/patch/9633105/
https://patchwork.kernel.org/patch/9925227/

[2]:
Note: the UEFI(QEMU_EFI.fd) is needed if guest want to use ACPI table.

After guest boot up, dump the APEI table, then can see the initialized table
(1) # iasl -p ./HEST -d /sys/firmware/acpi/tables/HEST
(2) # cat HEST.dsl
/*
 * Intel ACPI Component Architecture
 * AML/ASL+ Disassembler version 20170728 (64-bit version)
 * Copyright (c) 2000 - 2017 I

[Qemu-devel] [PATCH v14 1/9] ACPI: add some GHES structures and macros definition

[Dongjiu Geng]

Add Generic Error Status Block structures and some macros
definitions, which is referred to the ACPI 4.0 or ACPI 6.1. The
HEST table generation and CPER record will use them.

Signed-off-by: Dongjiu Geng 
---
Change since v13:
1. Clean the new added structures and macros definition

Change since v12:
1. Address Igor's comments to to get rid of most structures and use
build_append_int_noprefix() API to compose whole error status block
and APEI table in [1]

[1]: https://lkml.org/lkml/2017/8/29/187
---
 include/hw/acpi/acpi-defs.h | 52 +
 1 file changed, 52 insertions(+)

diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 72be675..fb2110c 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -298,6 +298,25 @@ typedef struct AcpiMultipleApicTable AcpiMultipleApicTable;
 #define ACPI_APIC_RESERVED  16   /* 16 and greater are reserved */
 
 /*
+ * Hardware Error Notification
+ */
+enum AcpiHestNotifyType {
+ACPI_HEST_NOTIFY_POLLED = 0,
+ACPI_HEST_NOTIFY_EXTERNAL = 1,
+ACPI_HEST_NOTIFY_LOCAL = 2,
+ACPI_HEST_NOTIFY_SCI = 3,
+ACPI_HEST_NOTIFY_NMI = 4,
+ACPI_HEST_NOTIFY_CMCI = 5,  /* ACPI 5.0 */
+ACPI_HEST_NOTIFY_MCE = 6,   /* ACPI 5.0 */
+ACPI_HEST_NOTIFY_GPIO = 7,  /* ACPI 6.0 */
+ACPI_HEST_NOTIFY_SEA = 8,   /* ACPI 6.1 */
+ACPI_HEST_NOTIFY_SEI = 9,   /* ACPI 6.1 */
+ACPI_HEST_NOTIFY_GSIV = 10, /* ACPI 6.1 */
+ACPI_HEST_NOTIFY_SDEI = 11, /* ACPI 6.2 */
+ACPI_HEST_NOTIFY_RESERVED = 12 /* 12 and greater are reserved */
+};
+
+/*
  * MADT sub-structures (Follow MULTIPLE_APIC_DESCRIPTION_TABLE)
  */
 #define ACPI_SUB_HEADER_DEF   /* Common ACPI sub-structure header */\
@@ -474,6 +493,39 @@ struct AcpiSystemResourceAffinityTable {
 } QEMU_PACKED;
 typedef struct AcpiSystemResourceAffinityTable AcpiSystemResourceAffinityTable;
 
+/*
+ * Generic Error Status Block
+ */
+struct AcpiGenericErrorStatus {
+/* It is a bitmask composed of ACPI_GEBS_xxx macros */
+uint32_t block_status;
+uint32_t raw_data_offset;
+uint32_t raw_data_length;
+uint32_t data_length;
+uint32_t error_severity;
+} QEMU_PACKED;
+typedef struct AcpiGenericErrorStatus AcpiGenericErrorStatus;
+
+/*
+ * Masks for Block Status field above
+ */
+#define ACPI_GEBS_UNCORRECTABLE  (1)
+
+/*
+ * Value for Error Severity field above
+ */
+enum AcpiGenericErrorSeverity {
+ACPI_CPER_SEV_RECOVERABLE,
+ACPI_CPER_SEV_FATAL,
+ACPI_CPER_SEV_CORRECTED,
+ACPI_CPER_SEV_NONE,
+};
+
+/*
+ * Generic Hardware Error Source version 2
+ */
+#define ACPI_HEST_SOURCE_GENERIC_ERROR_V2(10)
+
 #define ACPI_SRAT_PROCESSOR_APIC 0
 #define ACPI_SRAT_MEMORY 1
 #define ACPI_SRAT_PROCESSOR_x2APIC   2
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 8/9] hw/arm/virt: Add RAS platform version for migration

[Dongjiu Geng]

Support this feature since version 2.10, disable it by
default in the old version.

Signed-off-by: Dongjiu Geng 
---
Address Shannon's comments to add platform version in [1].

[1]: https://lkml.org/lkml/2017/8/25/821

Signed-off-by: Dongjiu Geng 
---
 hw/arm/virt-acpi-build.c | 14 +-
 hw/arm/virt.c|  4 
 include/hw/arm/virt.h|  1 +
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 06c14b3..b6974ef 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -801,10 +801,11 @@ void virt_acpi_build(VirtMachineState *vms, 
AcpiBuildTables *tables)
 acpi_add_table(table_offsets, tables_blob);
 build_spcr(tables_blob, tables->linker, vms);
 
-acpi_add_table(table_offsets, tables_blob);
-build_hardware_error_table(tables->hardware_errors, tables->linker);
-build_apei_ghes(tables_blob, tables->hardware_errors, tables->linker);
-
+if (!vmc->no_ras) {
+acpi_add_table(table_offsets, tables_blob);
+build_hardware_error_table(tables->hardware_errors, tables->linker);
+build_apei_ghes(tables_blob, tables->hardware_errors, tables->linker);
+}
 
 if (nb_numa_nodes > 0) {
 acpi_add_table(table_offsets, tables_blob);
@@ -891,6 +892,7 @@ static const VMStateDescription vmstate_virt_acpi_build = {
 
 void virt_acpi_setup(VirtMachineState *vms)
 {
+VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
 AcpiBuildTables tables;
 AcpiBuildState *build_state;
 
@@ -922,7 +924,9 @@ void virt_acpi_setup(VirtMachineState *vms)
 fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
 acpi_data_len(tables.tcpalog));
 
-ghes_add_fw_cfg(vms->fw_cfg, tables.hardware_errors);
+if (!vmc->no_ras) {
+ghes_add_fw_cfg(vms->fw_cfg, tables.hardware_errors);
+}
 
 build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
   ACPI_BUILD_RSDP_FILE, 0);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 68495c2..ab79988 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1732,8 +1732,12 @@ static void virt_2_9_instance_init(Object *obj)
 
 static void virt_machine_2_9_options(MachineClass *mc)
 {
+VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
 virt_machine_2_10_options(mc);
 SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_9);
+/* memory recovery feature was introduced with 2.10 */
+vmc->no_ras = true;
 }
 DEFINE_VIRT_MACHINE(2, 9)
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 33b0ff3..8fbd664 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -84,6 +84,7 @@ typedef struct {
 bool disallow_affinity_adjustment;
 bool no_its;
 bool no_pmu;
+bool no_ras;
 bool claim_edge_triggered_timers;
 } VirtMachineClass;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 3/9] docs: APEI GHES generation and CPER record description

[Dongjiu Geng]

Add APEI/GHES detailed design document

Signed-off-by: Dongjiu Geng 
---
Address Igor's comments to add a doc
---
 docs/specs/acpi_hest_ghes.txt | 97 +++
 1 file changed, 97 insertions(+)
 create mode 100644 docs/specs/acpi_hest_ghes.txt

diff --git a/docs/specs/acpi_hest_ghes.txt b/docs/specs/acpi_hest_ghes.txt
new file mode 100644
index 000..fbfc787
--- /dev/null
+++ b/docs/specs/acpi_hest_ghes.txt
@@ -0,0 +1,97 @@
+APEI tables generating and CPER record
+=
+
+Copyright (C) 2017 HuaWei Corporation.
+
+Design Details:
+---
+
+   etc/acpi/tables etc/hardware_errors
+  
==
++ +--++---+
+| | HEST ||address|
+--+
+| +--+|registers  |
| Error Status |
+| | GHES1|| +-+
| Data Block 1 |
+| +--+ +->| |error_block_address1 
|--->| ++
+| | .| |  | +-+
| |  CPER  |
+| | error_status_address-+-+ +--->| |error_block_address2 |+   
| |  CPER  |
+| | .|   || +-+|   
| |    |
+| | read_ack_register+-+ || |..   ||   
| |  CPER  |
+| | read_ack_preserve| | |+---+|   
| ++
+| | read_ack_write   | | | +->| |error_block_addressN |--+ |   
| Error Status |
++ +--+ | | |  | +-+  | |   
| Data Block 2 |
+| | GHES2| +-+-+->| |read_ack_register1   |  | 
+-->| ++
++ +--+   | |  | +-+  | 
| |  CPER  |
+| | .|   | | +--->| |read_ack_register2   |  | 
| |  CPER  |
+| | error_status_address-+---+ | || +-+  | 
| |    |
+| | .| | || |  .  |  | 
| |  CPER  |
+| | read_ack_register+-+-+| +-+  | 
+-++
+| | read_ack_preserve| |   +->| |read_ack_registerN   |  | 
| |..  |
+| | read_ack_write   | |   |  | +-+  | 
| ++
++ +--| |   | | 
| Error Status |
+| | ...  | |   | | 
| Data Block N |
++ +--+ |   | 
+>| ++
+| | GHESN| |   |   
| |  CPER  |
++ +--+ |   |   
| |  CPER  |
+| | .| |   |   
| |    |
+| | error_status_address-+-+   |   
| |  CPER  |
+| | .| |   
+-++
+| | read_ack_register+-+
+| | read_ack_preserve|
+| | read_ack_write   |
++ +--+
+
+(1) QEMU generates the ACPI HEST table. This table goes in the current
+"etc/acpi/tables" fw_cfg blob. Each error source has different
+notification type.
+
+(2) A new fw_cfg blob called "etc/hardware_errors" is introduced. QEMU
+also need to populate this blob. The "etc/hardwre_errors" fw_cfg blob
+contains one address registers table and one Error Status Data Block
+table, all of which are pre-allocated.
+
+(3) The address registers table contains N Error Block Address entries
+and N Read Ack Address entries, the size for each entry is 8-byte.
+The Error Status Data Block table contains N Error Status Data Block
+entries, the size for each entry is 4096(0x1000) bytes. The total size
+for "etc/hardware_errors" fw_cfg blob is (N * 8 * 2 + N * 4096) bytes.
+
+(4) QEMU generates the ACPI linker/loader script for the firmware
+
+(4a) The HEST table is part of "etc/acpi/tables", the firmware already
+allocates the memory for it, because QEMU already generates an ALLOCATE
+linker/loader command for it
+
+(4b) QEMU creates another ALLOCATE command for the "etc/hardware_errors"
+blob. The firmware allocates memory for this blob and downloads it.
+
+(5) QEMU generates N ADD_POINTER commands, which patch address in the
+"error_status_address" fields of the HEST table with a pointer to the
+  

Re: [Qemu-devel] [PATCH v9 04/13] dirty-bitmap: add locked state

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index fe27ddfb83..6218740c95 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -40,6 +40,8 @@ struct BdrvDirtyBitmap {
>  QemuMutex *mutex;
>  HBitmap *bitmap;/* Dirty bitmap implementation */
>  HBitmap *meta;  /* Meta dirty bitmap */
> +bool qmp_locked;/* Bitmap is frozen, it can't be modified
> +   through QMP */

Please fix the alignment when you respin. Thanks.

Fam

[Qemu-devel] [PATCH v14 2/9] ACPI: Add APEI GHES table generation and CPER record support

[Dongjiu Geng]

This implements APEI GHES Table generation and record CPER in
runtime via fw_cfg blobs.Now we only support two types of GHESv2,
which are GPIO-Signal and ARMv8 SEA. Afterwards, we can extend the
supported type if needed. For the CPER section type, currently it
is memory section because kernel manly wants userspace to handle
the memory errors. In order to simulation, we hard code the error
type to Multi-bit ECC.

For GHESv2 error source, the OSPM must acknowledges the error via
Read ACK register. So user space must check the ACK value before
recording a new CPER to avoid read-write race condition.

Suggested-by: Laszlo Ersek 
Signed-off-by: Dongjiu Geng 
---
The basic solution is suggested by Laszlo in [1]
[1]: https://lkml.org/lkml/2017/3/29/342
---
 hw/acpi/aml-build.c |   2 +
 hw/acpi/hest_ghes.c | 390 
 hw/arm/virt-acpi-build.c|   8 +
 include/hw/acpi/aml-build.h |   1 +
 include/hw/acpi/hest_ghes.h |  82 ++
 5 files changed, 483 insertions(+)
 create mode 100644 hw/acpi/hest_ghes.c
 create mode 100644 include/hw/acpi/hest_ghes.h

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 36a6cc4..6849e5f 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1561,6 +1561,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables)
 tables->table_data = g_array_new(false, true /* clear */, 1);
 tables->tcpalog = g_array_new(false, true /* clear */, 1);
 tables->vmgenid = g_array_new(false, true /* clear */, 1);
+tables->hardware_errors = g_array_new(false, true /* clear */, 1);
 tables->linker = bios_linker_loader_init();
 }
 
@@ -1571,6 +1572,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, 
bool mfre)
 g_array_free(tables->table_data, true);
 g_array_free(tables->tcpalog, mfre);
 g_array_free(tables->vmgenid, mfre);
+g_array_free(tables->hardware_errors, mfre);
 }
 
 /* Build rsdt table */
diff --git a/hw/acpi/hest_ghes.c b/hw/acpi/hest_ghes.c
new file mode 100644
index 000..86ec99e
--- /dev/null
+++ b/hw/acpi/hest_ghes.c
@@ -0,0 +1,390 @@
+/* Support for generating APEI tables and record CPER for Guests
+ *
+ * Copyright (C) 2017 HuaWei Corporation.
+ *
+ * Author: Dongjiu Geng 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/hest_ghes.h"
+#include "hw/nvram/fw_cfg.h"
+#include "sysemu/sysemu.h"
+#include "qemu/error-report.h"
+
+/* Generic Address Structure (GAS)
+ * ACPI 2.0/3.0: 5.2.3.1 Generic Address Structure
+ * 2.0 compat note:
+ *@access_width must be 0, see ACPI 2.0:Table 5-1
+ */
+static void build_append_gas(GArray *table, AmlRegionSpace as,
+  uint8_t bit_width, uint8_t bit_offset,
+  uint8_t access_width, uint64_t address)
+{
+build_append_int_noprefix(table, as, 1);
+build_append_int_noprefix(table, bit_width, 1);
+build_append_int_noprefix(table, bit_offset, 1);
+build_append_int_noprefix(table, access_width, 1);
+build_append_int_noprefix(table, address, 8);
+}
+
+/* Hardware Error Notification
+ * ACPI 4.0: 17.3.2.7 Hardware Error Notification
+ */
+static void build_append_notify(GArray *table, const uint8_t type,
+uint8_t length)
+{
+build_append_int_noprefix(table, type, 1); /* type */
+build_append_int_noprefix(table, length, 1);
+build_append_int_noprefix(table, 0, 26);
+}
+
+/* Generic Error Status Block
+ * ACPI 4.0: 17.3.2.6.1 Generic Error Data
+ */
+static void build_append_gesb_header(GArray *table, uint32_t block_status,
+  uint32_t raw_data_offset, uint32_t raw_data_length,
+  uint32_t data_length, uint32_t error_severity)
+{
+build_append_int_noprefix(table, block_status, 4);
+build_append_int_noprefix(table, raw_data_offset, 4);
+build_append_int_noprefix(table, raw_data_length, 4);
+build_append_int_noprefix(table, data_length, 4);
+build_append_int_noprefix(table, error_severity, 4);
+}
+
+/* Generic Error Data Entry
+ * ACPI 4.0: 17.3.2.6.1 Generic Error Data
+ */
+static void build_append_gede_header(GArray *table, const char *section_type,
+  const uint32_t error_severity, const uint16_t revision,
+  const uint32_

Re: [Qemu-devel] [PATCH v9 06/13] qapi: add dirty-bitmaps migration capability

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: John Snow 
> Reviewed-by: Eric Blake 
> Reviewed-by: Juan Quintela 
> ---
>  qapi/migration.json   | 6 +-
>  migration/migration.h | 1 +
>  migration/migration.c | 9 +
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 03f57c9616..9f20b645a7 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -352,12 +352,16 @@
>  #
>  # @x-multifd: Use more than one fd for migration (since 2.11)
>  #
> +# @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps.
> +# (since 2.11)

s/2.11/2.12/

But it should be fine to keep all r-b lines when you update that, and add one
more:

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 07/13] migration: include migrate_dirty_bitmaps in migrate_postcopy

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Enable postcopy if dirty bitmap migration is enabled.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Juan Quintela 
> Reviewed-by: John Snow 
> ---
>  migration/migration.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 1526cd4bff..e973837bfd 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1487,7 +1487,7 @@ bool migrate_postcopy_ram(void)
>  
>  bool migrate_postcopy(void)
>  {
> -return migrate_postcopy_ram();
> +return migrate_postcopy_ram() || migrate_dirty_bitmaps();
>  }
>  
>  bool migrate_auto_converge(void)
> -- 
> 2.11.1
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 08/13] migration/qemu-file: add qemu_put_counted_string()

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Add function opposite to qemu_get_counted_string.
> qemu_put_counted_string puts one-byte length of the string (string
> should not be longer than 255 characters), and then it puts the string,
> without last zero byte.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: John Snow 
> Reviewed-by: Juan Quintela 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 09/13] migration: add is_active_iterate handler

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Only-postcopy savevm states (dirty-bitmap) don't need live iteration, so
> to disable them and stop transporting empty sections there is a new
> savevm handler.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Juan Quintela 
> Reviewed-by: John Snow 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 10/13] migration: add postcopy migration of dirty bitmaps

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Postcopy migration of dirty bitmaps. Only named dirty bitmaps,
> associated with root nodes and non-root named nodes are migrated.
> 
> If destination qemu is already containing a dirty bitmap with the same name
> as a migrated bitmap (for the same node), then, if their granularities are
> the same the migration will be done, otherwise the error will be generated.
> 
> If destination qemu doesn't contain such bitmap it will be created.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Looks good in general, a few nits below.

> +static void send_bitmap_bits(QEMUFile *f, DirtyBitmapMigBitmapState *dbms,
> + uint64_t start_sector, uint32_t nr_sectors)
> +{
> +/* align for buffer_is_zero() */
> +uint64_t align = 4 * sizeof(long);
> +uint64_t unaligned_size =
> +bdrv_dirty_bitmap_serialization_size(
> +dbms->bitmap, start_sector << BDRV_SECTOR_BITS,
> +(uint64_t)nr_sectors << BDRV_SECTOR_BITS);
> +uint64_t buf_size = (unaligned_size + align - 1) & ~(align - 1);

QEMU_ALIGN_UP() ?

> +uint8_t *buf = g_malloc0(buf_size);
> +uint32_t flags = DIRTY_BITMAP_MIG_FLAG_BITS;
> +
> +bdrv_dirty_bitmap_serialize_part(
> +dbms->bitmap, buf, start_sector << BDRV_SECTOR_BITS,
> +(uint64_t)nr_sectors << BDRV_SECTOR_BITS);
> +
> +if (buffer_is_zero(buf, buf_size)) {
> +g_free(buf);
> +buf = NULL;
> +flags |= DIRTY_BITMAP_MIG_FLAG_ZEROES;
> +}
> +
> +trace_send_bitmap_bits(flags, start_sector, nr_sectors, buf_size);
> +
> +send_bitmap_header(f, dbms, flags);
> +
> +qemu_put_be64(f, start_sector);
> +qemu_put_be32(f, nr_sectors);
> +
> +/* if a block is zero we need to flush here since the network
> + * bandwidth is now a lot higher than the storage device bandwidth.
> + * thus if we queue zero blocks we slow down the migration. */
> +if (flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
> +qemu_fflush(f);
> +} else {
> +qemu_put_be64(f, buf_size);
> +qemu_put_buffer(f, buf, buf_size);
> +}
> +
> +g_free(buf);
> +}
> +



> +static void dirty_bitmap_save_pending(QEMUFile *f, void *opaque,
> +  uint64_t max_size,
> +  uint64_t *res_precopy_only,
> +  uint64_t *res_compatible,
> +  uint64_t *res_postcopy_only)
> +{
> +DirtyBitmapMigBitmapState *dbms;
> +uint64_t pending = 0;
> +
> +qemu_mutex_lock_iothread();
> +
> +QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
> +uint64_t gran = bdrv_dirty_bitmap_granularity(dbms->bitmap);
> +uint64_t sectors = dbms->bulk_completed ? 0 :
> +   dbms->total_sectors - dbms->cur_sector;
> +
> +pending += (sectors * BDRV_SECTOR_SIZE + gran - 1) / gran;

DIV_ROUND_UP()?

> +}
> +
> +qemu_mutex_unlock_iothread();
> +
> +trace_dirty_bitmap_save_pending(pending, max_size);
> +
> +*res_postcopy_only += pending;
> +}



> +static int dirty_bitmap_load_header(QEMUFile *f, DirtyBitmapLoadState *s)
> +{
> +Error *local_err = NULL;
> +s->flags = qemu_get_bitmap_flags(f);
> +trace_dirty_bitmap_load_header(s->flags);
> +
> +if (s->flags & DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME) {
> +if (!qemu_get_counted_string(f, s->node_name)) {
> +error_report("Unable to read node name string");
> +return -EINVAL;
> +}
> +s->bs = bdrv_lookup_bs(s->node_name, s->node_name, &local_err);
> +if (!s->bs) {
> +error_report_err(local_err);
> +return -EINVAL;
> +}
> +} else if (!s->bs) {
> +error_report("Error: block device name is not set");
> +return -EINVAL;
> +}
> +
> +if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) {
> +if (!qemu_get_counted_string(f, s->bitmap_name)) {
> +error_report("Unable to read node name string");

s/node name/bitmap name/

> +return -EINVAL;
> +}
> +s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
> +
> +/* bitmap may be NULL here, it wouldn't be an error if it is the
> + * first occurrence of the bitmap */
> +if (!s->bitmap && !(s->flags & DIRTY_BITMAP_MIG_FLAG_START)) {
> +error_report("Error: unknown dirty bitmap "
> + "'%s' for block device '%s'",
> + s->bitmap_name, s->node_name);
> +return -EINVAL;
> +}
> +} else if (!s->bitmap) {
> +error_report("Error: block device name is not set");
> +return -EINVAL;
> +}
> +
> +return 0;
> +}
> +

Fam

Re: [Qemu-devel] [PATCH v9 11/13] iotests: add default node-name

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> When testing migration, auto-generated by qemu node-names differs in
> source and destination qemu and migration fails. After this patch,
> auto-generated by iotest nodenames will be the same.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Max Reitz 
> ---
>  tests/qemu-iotests/iotests.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 6f057904a9..95454c1893 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -216,6 +216,8 @@ class VM(qtest.QEMUQtestMachine):
>  options.append('file=%s' % path)
>  options.append('format=%s' % format)
>  options.append('cache=%s' % cachemode)
> +if 'node-name' not in opts:
> +options.append('node-name=drivenode%d' % self._num_drives)
>  
>  if opts:
>  options.append(opts)
> -- 
> 2.11.1
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 12/13] iotests: add dirty bitmap migration test

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> +def inject_test_case(klass, name, method, *args, **kwargs):
> +mc = operator.methodcaller(method, *args, **kwargs)
> +setattr(klass, 'test_' + name, new.instancemethod(mc, None, klass))
> +
> +
> +for cmb in list(itertools.product((True, False), repeat=4)):
> +name = ('_' if cmb[0] else '_not_') + 'persistent_'
> +name += ('_' if cmb[1] else '_not_') + 'shared_'
> +name += ('_' if cmb[2] else '_not_') + 'migbitmap_'
> +name += '_online' if cmb[3] else '_offline'
> +
> +inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration', 
> *cmb)

Personally I'd just spell out the 16 method names and call do_test_migration.
It's much easier to read and modify. Either way,

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v9 13/13] iotests: add dirty bitmap postcopy test

[Fam Zheng]

On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote:
> Test
> - start two vms (vm_a, vm_b)
> 
> - in a
> - do writes from set A
> - do writes from set B
> - fix bitmap sha256
> - clear bitmap
> - do writes from set A
> - start migration
> - than, in b
> - wait vm start (postcopy should start)
> - do writes from set B
> - check bitmap sha256
> 
> The test should verify postcopy migration and then merging with delta
> (changes in target, during postcopy process).
> 
> Reduce supported cache modes to only 'none', because with cache on time
> from source.STOP to target.RESUME is unpredictable and we can fail with
> timout while waiting for target.RESUME.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/199| 105 
> ++
>  tests/qemu-iotests/199.out|   5 ++
>  tests/qemu-iotests/group  |   1 +
>  tests/qemu-iotests/iotests.py |   7 ++-
>  4 files changed, 117 insertions(+), 1 deletion(-)
>  create mode 100755 tests/qemu-iotests/199
>  create mode 100644 tests/qemu-iotests/199.out
> 
> diff --git a/tests/qemu-iotests/199 b/tests/qemu-iotests/199
> new file mode 100755
> index 00..f872040a81
> --- /dev/null
> +++ b/tests/qemu-iotests/199
> @@ -0,0 +1,105 @@
> +#!/usr/bin/env python
> +#
> +# Tests for dirty bitmaps postcopy migration.
> +#
> +# Copyright (c) 2016-2017 Virtuozzo International GmbH. All rights reserved.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +import os
> +import iotests
> +import time
> +from iotests import qemu_img
> +
> +disk_a = os.path.join(iotests.test_dir, 'disk_a')
> +disk_b = os.path.join(iotests.test_dir, 'disk_b')
> +size = '256G'
> +fifo = os.path.join(iotests.test_dir, 'mig_fifo')
> +
> +class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
> +
> +def tearDown(self):
> +self.vm_a.shutdown()
> +self.vm_b.shutdown()
> +os.remove(disk_a)
> +os.remove(disk_b)
> +os.remove(fifo)
> +
> +def setUp(self):
> +os.mkfifo(fifo)
> +qemu_img('create', '-f', iotests.imgfmt, disk_a, size)
> +qemu_img('create', '-f', iotests.imgfmt, disk_b, size)
> +self.vm_a = iotests.VM(path_suffix='a').add_drive(disk_a)
> +self.vm_b = iotests.VM(path_suffix='b').add_drive(disk_b)
> +self.vm_b.add_incoming("exec: cat '" + fifo + "'")
> +self.vm_a.launch()
> +self.vm_b.launch()
> +
> +def test_postcopy(self):
> +write_size = 0x4000
> +granularity = 512
> +chunk = 4096
> +
> +result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
> +   name='bitmap', granularity=granularity)
> +self.assert_qmp(result, 'return', {});
> +
> +s = 0
> +while s < write_size:
> +self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
> +s += 0x1
> +s = 0x8000
> +while s < write_size:
> +self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
> +s += 0x1
> +
> +result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
> +   node='drive0', name='bitmap')
> +sha256 = result['return']['sha256']
> +
> +result = self.vm_a.qmp('block-dirty-bitmap-clear', node='drive0',
> +   name='bitmap')
> +self.assert_qmp(result, 'return', {});
> +s = 0
> +while s < write_size:
> +self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
> +s += 0x1
> +
> +result = self.vm_a.qmp('migrate-set-capabilities',
> +   capabilities=[{'capability': 'dirty-bitmaps',
> +  'state': True}])
> +self.assert_qmp(result, 'return', {})
> +
> +result = self.vm_a.qmp('migrate', uri='exec:cat>' + fifo)
> +self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
> +self.assertNotEqual(self.vm_b.event_wait("RESUME"), None)
> +
> +s = 0x8000
> +while s < write_size:
> +self.vm_b.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
> +s += 0x1
> +
> +result = self.vm_b.qmp('query-block');
> +while len(result['return'][0]['dirty-bit

[Qemu-devel] [PATCH 0/3] chardev: convert leftover glib APIs to use dedicate gcontext

[Peter Xu]

There were existing work that tried to allow chardev to be run in a
dedicated gcontext rather than the default main context/thread.
Basically that work passed in the correct gcontext during
g_source_attach().  However, I found something missing along the way,
that some legacy glib APIs are used by chardev code which take the
main context as default:

   g_timeout_add_seconds
   g_timeout_add
   g_idle_add

To fully allow the chardevs to be run in dedicated gcontext, we need
to convert all these legacy APIs into g_source_attach() calls as well,
with the correct gcontext passed in.

This series tries to clean the rest of things up.

I picked up patch 1 from monitor-oob series into this series (which is
a missing of chardev frontend call fix for g_source_attach()), so that
this series can be a complete fix.

Please review.  Thanks,

Peter Xu (3):
  chardev: use backend chr context when watch for fe
  chardev: let g_idle_add() be with chardev gcontext
  chardev: introduce qemu_chr_timeout_add() and use

 chardev/char-fe.c  |  2 +-
 chardev/char-pty.c | 16 
 chardev/char-socket.c  |  4 ++--
 chardev/char.c | 20 
 hw/char/terminal3270.c |  7 ---
 include/chardev/char.h |  2 ++
 6 files changed, 37 insertions(+), 14 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 3/3] chardev: introduce qemu_chr_timeout_add() and use

[Peter Xu]

It's a replacement of g_timeout_add[_seconds]() for chardevs.  Chardevs
now can have dedicated gcontext, we should always bind chardev tasks
onto those gcontext rather than the default main context.  Since there
are quite a few of g_timeout_add[_seconds]() callers, a new function
qemu_chr_timeout_add() is introduced.

One thing to mention is that, terminal3270 is still always running on
main gcontext.  However let's convert that as well since it's still part
of chardev codes and in case one day we'll miss that when we move it out
of main gcontext too.

Signed-off-by: Peter Xu 
---
 chardev/char-pty.c |  9 ++---
 chardev/char-socket.c  |  4 ++--
 chardev/char.c | 20 
 hw/char/terminal3270.c |  7 ---
 include/chardev/char.h |  2 ++
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index dd17b1b823..cbd8ac5eb7 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -78,13 +78,8 @@ static void pty_chr_rearm_timer(Chardev *chr, int ms)
 s->timer_tag = 0;
 }
 
-if (ms == 1000) {
-name = g_strdup_printf("pty-timer-secs-%s", chr->label);
-s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
-} else {
-name = g_strdup_printf("pty-timer-ms-%s", chr->label);
-s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
-}
+name = g_strdup_printf("pty-timer-ms-%s", chr->label);
+s->timer_tag = qemu_chr_timeout_add(chr, ms, pty_chr_timer, chr);
 g_source_set_name_by_id(s->timer_tag, name);
 g_free(name);
 }
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 630a7f2995..644a620599 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -73,8 +73,8 @@ static void qemu_chr_socket_restart_timer(Chardev *chr)
 char *name;
 
 assert(s->connected == 0);
-s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time,
-   socket_reconnect_timeout, chr);
+s->reconnect_timer = qemu_chr_timeout_add(chr, s->reconnect_time,
+  socket_reconnect_timeout, chr);
 name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label);
 g_source_set_name_by_id(s->reconnect_timer, name);
 g_free(name);
diff --git a/chardev/char.c b/chardev/char.c
index 8c3765ee99..a1de662fec 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1084,6 +1084,26 @@ void qmp_chardev_send_break(const char *id, Error **errp)
 qemu_chr_be_event(chr, CHR_EVENT_BREAK);
 }
 
+/*
+ * Add a timeout callback for the chardev (in milliseconds). Please
+ * use this to add timeout hook for chardev instead of g_timeout_add()
+ * and g_timeout_add_seconds(), to make sure the gcontext that the
+ * task bound to is correct.
+ */
+guint qemu_chr_timeout_add(Chardev *chr, guint ms, GSourceFunc func,
+   void *private)
+{
+GSource *source = g_timeout_source_new(ms);
+guint id;
+
+assert(func);
+g_source_set_callback(source, func, private, NULL);
+id = g_source_attach(source, chr->gcontext);
+g_source_unref(source);
+
+return id;
+}
+
 void qemu_chr_cleanup(void)
 {
 object_unparent(get_chardevs_root());
diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c
index a109ce5987..479e4554d6 100644
--- a/hw/char/terminal3270.c
+++ b/hw/char/terminal3270.c
@@ -94,8 +94,8 @@ static void terminal_read(void *opaque, const uint8_t *buf, 
int size)
 g_source_remove(t->timer_tag);
 t->timer_tag = 0;
 }
-t->timer_tag = g_timeout_add_seconds(600, send_timing_mark_cb, t);
-
+t->timer_tag = qemu_chr_timeout_add(t->chr.chr, 600,
+send_timing_mark_cb, t);
 memcpy(&t->inv[t->in_len], buf, size);
 t->in_len += size;
 if (t->in_len < 2) {
@@ -157,7 +157,8 @@ static void chr_event(void *opaque, int event)
  * char-socket.c. Once qemu receives the terminal-type of the
  * client, mark handshake done and trigger everything rolling again.
  */
-t->timer_tag = g_timeout_add_seconds(600, send_timing_mark_cb, t);
+t->timer_tag = qemu_chr_timeout_add(t->chr.chr, 600,
+send_timing_mark_cb, t);
 break;
 case CHR_EVENT_CLOSED:
 sch->curr_status.scsw.dstat = SCSW_DSTAT_DEVICE_END;
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 778d610295..4970e46457 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -258,5 +258,7 @@ extern int term_escape_char;
 
 /* console.c */
 void qemu_chr_parse_vc(QemuOpts *opts, ChardevBackend *backend, Error **errp);
+guint qemu_chr_timeout_add(Chardev *chr, guint ms, GSourceFunc func,
+   void *private);
 
 #endif
-- 
2.14.3

[Qemu-devel] [PATCH 1/3] chardev: use backend chr context when watch for fe

[Peter Xu]

In commit 6bbb6c0644 ("chardev: use per-dev context for
io_add_watch_poll", 2017-09-22) all the chardev watches are converted to
use per-chardev gcontext to support chardev to be run outside default
main thread.  However that's still missing one call from the frontend
code.  Touch that up.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
 chardev/char-fe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index ee6d596100..c611b3fa3e 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -356,7 +356,7 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition 
cond,
 }
 
 g_source_set_callback(src, (GSourceFunc)func, user_data, NULL);
-tag = g_source_attach(src, NULL);
+tag = g_source_attach(src, s->gcontext);
 g_source_unref(src);
 
 return tag;
-- 
2.14.3

[Qemu-devel] [PATCH 2/3] chardev: let g_idle_add() be with chardev gcontext

[Peter Xu]

The idle task will be attached to main gcontext even if the chardev
backend is running in another gcontext.  Fix the only caller by
extending the g_idle_add() logic into the more powerful
g_source_attach().  It's basically g_idle_add_full() implementation, but
with the chardev's gcontext passed in.

Signed-off-by: Peter Xu 
---
 chardev/char-pty.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 761ae6dec1..dd17b1b823 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -210,9 +210,14 @@ static void pty_chr_state(Chardev *chr, int connected)
 s->timer_tag = 0;
 }
 if (!s->connected) {
+GSource *source = g_idle_source_new();
+
 g_assert(s->open_tag == 0);
 s->connected = 1;
-s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr);
+g_source_set_callback(source, qemu_chr_be_generic_open_func,
+  chr, NULL);
+s->open_tag = g_source_attach(source, chr->gcontext);
+g_source_unref(source);
 }
 if (!chr->gsource) {
 chr->gsource = io_add_watch_poll(chr, s->ioc,
-- 
2.14.3

[Qemu-devel] [Bug 1740364] [NEW] qemu-img: fails to get shared 'write' lock

[Yaniv Kaul]

Public bug reported:

Description of problem:
Somewhere in F27 (did not see it happening before), I'm getting while running 
libguestfs (via libvirt or direct), a qemu-img failure. Note: multiple qcow2 
snapshots are on the same backing file, and a parallel libguestfs command is 
running on all. However, it seems to be failing to get a lock on the leaf, 
which is unique, non-shared.

The VM is up and running. I'm not sure why qemu-img is even trying to get a 
write lock on it. Even 'info' fails:
ykaul@ykaul ovirt-system-tests]$ qemu-img info 
/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2
qemu-img: Could not open 
'/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2':
 Failed to get shared "write" lock
Is another process using the image?
[ykaul@ykaul ovirt-system-tests]$ lsof |grep qcow2
[ykaul@ykaul ovirt-system-tests]$ file 
/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2
/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2:
 QEMU QCOW Image (v3), has backing file (path 
/var/lib/lago/store/phx_repo:el7.4-base:v1), 6442450944 bytes


And it's OK if I kill the VM of course.


Version-Release number of selected component (if applicable):
[ykaul@ykaul ovirt-system-tests]$ rpm -qa |grep qemu
qemu-block-nfs-2.10.1-2.fc27.x86_64
qemu-block-dmg-2.10.1-2.fc27.x86_64
qemu-guest-agent-2.10.1-2.fc27.x86_64
qemu-system-x86-core-2.10.1-2.fc27.x86_64
qemu-block-curl-2.10.1-2.fc27.x86_64
qemu-img-2.10.1-2.fc27.x86_64
qemu-common-2.10.1-2.fc27.x86_64
qemu-kvm-2.10.1-2.fc27.x86_64
qemu-block-ssh-2.10.1-2.fc27.x86_64
qemu-block-iscsi-2.10.1-2.fc27.x86_64
libvirt-daemon-driver-qemu-3.7.0-3.fc27.x86_64
qemu-block-gluster-2.10.1-2.fc27.x86_64
ipxe-roms-qemu-20161108-2.gitb991c67.fc26.noarch
qemu-system-x86-2.10.1-2.fc27.x86_64
qemu-block-rbd-2.10.1-2.fc27.x86_64


How reproducible:
Sometimes.

Steps to Reproduce:
1. Running Lago (ovirt-system-tests) on my laptop, it happens quite a lot.

Additional info:
libguestfs: trace: set_verbose true
libguestfs: trace: set_verbose = 0
libguestfs: trace: set_backend "direct"
libguestfs: trace: set_backend = 0
libguestfs: create: flags = 0, handle = 0x7f1314006430, program = python2
libguestfs: trace: set_program "lago"
libguestfs: trace: set_program = 0
libguestfs: trace: add_drive_ro 
"/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2"
libguestfs: trace: add_drive 
"/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2"
 "readonly:true"
libguestfs: creating COW overlay to protect original drive content
libguestfs: trace: get_tmpdir
libguestfs: trace: get_tmpdir = "/tmp"
libguestfs: trace: disk_create "/tmp/libguestfsWrA7Dh/overlay1.qcow2" "qcow2" 
-1 
"backingfile:/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2"
libguestfs: command: run: qemu-img
libguestfs: command: run: \ create
libguestfs: command: run: \ -f qcow2
libguestfs: command: run: \ -o 
backing_file=/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2
libguestfs: command: run: \ /tmp/libguestfsWrA7Dh/overlay1.qcow2
qemu-img: /tmp/libguestfsWrA7Dh/overlay1.qcow2: Failed to get shared "write" 
lock
Is another process using the image?
Could not open backing image to determine size.
libguestfs: trace: disk_create = -1 (error)
libguestfs: trace: add_drive = -1 (error)
libguestfs: trace: add_drive_ro = -1 (error)


And:
[ykaul@ykaul ovirt-system-tests]$ strace qemu-img info 
/home/ykaul/ovirt-system-tests/deployment-basic-suite-master/default/images/lago-basic-suite-master-host-1_root.qcow2
execve("/usr/bin/qemu-img", ["qemu-img", "info", 
"/home/ykaul/ovirt-system-tests/d"...], 0x7fffb36ccfc0 /* 59 vars */) = 0
brk(NULL)   = 0x562790488000
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0x7f20cea08000
access("/etc/ld.so.preload", R_OK)  = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=93275, ...}) = 0
mmap(NULL, 93275, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f20ce9f1000
close(3)= 0
openat(AT_FDCWD, "/lib64/libz.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\320#\0\0\0\0\0\0"..., 
832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=94232, ...}) = 0
mmap(NULL, 2187272, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 
0x7f20ce5cc000
mprotect(0x7f20ce5e2000, 2093056, PROT_NONE) = 0
mmap(0x7f20ce7e1000, 4096, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x15000) = 0