Re: [PATCH 3/4] hw/arm/virt-acpi-build: Only expose flash on older machine types

[Andrew Jones]

On Mon, Jul 13, 2020 at 10:49:07AM +0200, Igor Mammedov wrote:
> On Mon, 29 Jun 2020 16:09:37 +0200
> Andrew Jones  wrote:
> 
> > The flash device is exclusively for the host-controlled firmware, so
> > we should not expose it to the OS. Exposing it risks the OS messing
> > with it, which could break firmware runtime services and surprise the
> > OS when all its changes disappear after reboot.
> > 
> > As firmware needs the device and uses DT, we leave the device exposed
> > there. It's up to firmware to remove the nodes from DT before sending
> > it on to the OS. However, there's no need to force firmware to remove
> > tables from ACPI (which it doesn't know how to do anyway), so we
> > simply don't add the tables in the first place. But, as we've been
> > adding the tables for quite some time and don't want to change the
> > default hardware exposed to versioned machines, then we only stop
> > exposing the flash device tables for 5.1 and later machine types.
> > 
> > Suggested-by: Ard Biesheuvel 
> > Suggested-by: Laszlo Ersek 
> > Signed-off-by: Andrew Jones 
> > ---
> >  hw/arm/virt-acpi-build.c | 5 -
> >  hw/arm/virt.c| 3 +++
> >  include/hw/arm/virt.h| 1 +
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 1384a2cf2ab4..91f0df7b13a3 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -749,6 +749,7 @@ static void build_fadt_rev5(GArray *table_data, 
> > BIOSLinker *linker,
> >  static void
> >  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >  {
> > +VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> >  Aml *scope, *dsdt;
> >  MachineState *ms = MACHINE(vms);
> >  const MemMapEntry *memmap = vms->memmap;
> > @@ -767,7 +768,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
> > VirtMachineState *vms)
> >  acpi_dsdt_add_cpus(scope, vms->smp_cpus);
> >  acpi_dsdt_add_uart(scope, [VIRT_UART],
> > (irqmap[VIRT_UART] + ARM_SPI_BASE));
> > -acpi_dsdt_add_flash(scope, [VIRT_FLASH]);
> > +if (vmc->acpi_expose_flash) {
> > +acpi_dsdt_add_flash(scope, [VIRT_FLASH]);
> > +}
> >  acpi_dsdt_add_fw_cfg(scope, [VIRT_FW_CFG]);
> >  acpi_dsdt_add_virtio(scope, [VIRT_MMIO],
> >  (irqmap[VIRT_MMIO] + ARM_SPI_BASE), 
> > NUM_VIRTIO_TRANSPORTS);
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index cd0834ce7faf..5adc9ff799ef 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -2482,9 +2482,12 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 1)
> >  
> >  static void virt_machine_5_0_options(MachineClass *mc)
> >  {
> > +VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> > +
> >  virt_machine_5_1_options(mc);
> >  compat_props_add(mc->compat_props, hw_compat_5_0, hw_compat_5_0_len);
> >  mc->numa_mem_supported = true;
> > +vmc->acpi_expose_flash = true;
> 
> we usually do not version ACPI tables changes
> (unless we have a good reason to do so)

Even when the change is to remove the exposure of hardware from the guest?
Before this change, if a guest looked, it had a flash, after this change,
if a guest looks, it doesn't. I'd feel much better versioning a change
like that, than not.

Thanks,
drew

> 
> >  }
> >  DEFINE_VIRT_MACHINE(5, 0)
> >  
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > index 31878ddc7223..c65be5fe0bb6 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -119,6 +119,7 @@ typedef struct {
> >  bool no_highmem_ecam;
> >  bool no_ged;   /* Machines < 4.2 has no support for ACPI GED device */
> >  bool kvm_no_adjvtime;
> > +bool acpi_expose_flash;
> >  } VirtMachineClass;
> >  
> >  typedef struct {
> 
> 

Re: [PATCH v2 08/11] docs/devel: fix grammar in multi-thread-tcg

[Thomas Huth]

On 13/07/2020 22.04, Alex Bennée wrote:
> Review comment came just too late ;-)
> 
> Signed-off-by: Alex Bennée 
> ---
>  docs/devel/multi-thread-tcg.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/devel/multi-thread-tcg.rst b/docs/devel/multi-thread-tcg.rst
> index 42158b77c7..21483870db 100644
> --- a/docs/devel/multi-thread-tcg.rst
> +++ b/docs/devel/multi-thread-tcg.rst
> @@ -107,7 +107,7 @@ including:
>  
>- debugging operations (breakpoint insertion/removal)
>- some CPU helper functions
> -  - linux-user spawning it's first thread
> +  - linux-user spawning its first thread
>  
>  This is done with the async_safe_run_on_cpu() mechanism to ensure all
>  vCPUs are quiescent when changes are being made to shared global
> 

Reviewed-by: Thomas Huth 

Re: [PATCH v2 05/11] tests/plugins: don't unconditionally add -Wpsabi

[Thomas Huth]

On 13/07/2020 22.04, Alex Bennée wrote:
> Not all compilers support the -Wpsabi (clang-9 in my case). To handle
> this gracefully we pare back the shared build machinery so the
> Makefile is relatively "standalone". We still take advantage of
> config-host.mak as configure has done a bunch of probing for us but
> that is it.
> 
> Fixes: bac8d222a
> Signed-off-by: Alex Bennée 
> 
> ---
> v2
>   - separate from main build system and check probe
> ---
>  configure |  3 +++
>  tests/plugin/Makefile | 22 ++
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index e1de2f5b24..08eaa99d19 100755
> --- a/configure
> +++ b/configure
> @@ -7112,6 +7112,9 @@ echo "GIT_UPDATE=$git_update" >> $config_host_mak
>  
>  echo "ARCH=$ARCH" >> $config_host_mak
>  
> +echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
> +echo "GLIB_LDFLAGS=$glib_ldflags" >> $config_host_mak
> +
>  if test "$default_devices" = "yes" ; then
>echo "CONFIG_MINIKCONF_MODE=--defconfig" >> $config_host_mak
>  else
> diff --git a/tests/plugin/Makefile b/tests/plugin/Makefile
> index 3a50451428..e9348fde4a 100644
> --- a/tests/plugin/Makefile
> +++ b/tests/plugin/Makefile
> @@ -1,9 +1,16 @@
> +# -*- Mode: makefile -*-
> +#
> +# This Makefile example is fairly independent from the main makefile
> +# so users can take and adapt it for their build. We only really
> +# include config-host.mak so we don't have to repeat probing for
> +# cflags that the main configure has already done for us.
> +#
> +
>  BUILD_DIR := $(CURDIR)/../..
>  
>  include $(BUILD_DIR)/config-host.mak
> -include $(SRC_PATH)/rules.mak
>  
> -$(call set-vpath, $(SRC_PATH)/tests/plugin)
> +VPATH += $(SRC_PATH)/tests/plugin
>  
>  NAMES :=
>  NAMES += bb
> @@ -17,11 +24,18 @@ NAMES += lockstep
>  
>  SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>  
> -QEMU_CFLAGS += -fPIC -Wpsabi
> -QEMU_CFLAGS += -I$(SRC_PATH)/include/qemu
> +# The main QEMU uses Glib extensively so it's perfectly fine to use it
> +# in plugins (which many example do).
> +CFLAGS = $(GLIB_CFLAGS)
> +CFLAGS += -fPIC
> +CFLAGS += $(if $(findstring no-psabi,$(QEMU_CFLAGS)),-Wpsabi)
> +CFLAGS += -I$(SRC_PATH)/include/qemu
>  
>  all: $(SONAMES)
>  
> +%.o: %.c
> + $(CC) $(CFLAGS) -c -o $@ $<
> +
>  lib%.so: %.o
>   $(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)
>  
> 

Reviewed-by: Thomas Huth 

[PULL 1/4] qga-win: Fix QGA VSS Provider service stop failure

[Michael Roth]

From: Basil Salman 

On one hand "guest-fsfreeze-freeze" command, "COM+ System Application service" 
is
stopped, on the other hand "guest-fsfreeze-thaw" stops QGA VSS Provider service 
from
"COM+ Application Admin Catalog".
Invoking a series of freeze and thaw commands may result in QGA failing to stop
VSS Provider service as "COM+ System Application service" is stopped, which can
cause some delay in qga response.
In this commit StopService function was changed and VSS Provider service is now
stopped using Winsvc library API.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1549425

Signed-off-by: Basil Salman 
Signed-off-by: Basil Salman 
Signed-off-by: Michael Roth 
---
 qga/vss-win32/install.cpp | 33 -
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index a456841360..40de133774 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define BUFFER_SIZE 1024
 
@@ -509,26 +510,32 @@ namespace _com_util
 }
 }
 
-/* Stop QGA VSS provider service from COM+ Application Admin Catalog */
-
+/* Stop QGA VSS provider service using Winsvc API  */
 STDAPI StopService(void)
 {
 HRESULT hr;
-COMInitializer initializer;
-COMPointer pUnknown;
-COMPointer pCatalog;
+SC_HANDLE manager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
+SC_HANDLE service = NULL;
 
-int count = 0;
+if (!manager) {
+errmsg(E_FAIL, "Failed to open service manager");
+hr = E_FAIL;
+goto out;
+}
+service = OpenService(manager, QGA_PROVIDER_NAME, SC_MANAGER_ALL_ACCESS);
 
-chk(QGAProviderFind(QGAProviderCount, (void *)));
-if (count) {
-chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER,
-IID_IUnknown, (void **)pUnknown.replace()));
-chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog2,
-(void **)pCatalog.replace()));
-chk(pCatalog->ShutdownApplication(_bstr_t(QGA_PROVIDER_LNAME)));
+if (!service) {
+errmsg(E_FAIL, "Failed to open service");
+hr =  E_FAIL;
+goto out;
+}
+if (!(ControlService(service, SERVICE_CONTROL_STOP, NULL))) {
+errmsg(E_FAIL, "Failed to stop service");
+hr = E_FAIL;
 }
 
 out:
+CloseServiceHandle(service);
+CloseServiceHandle(manager);
 return hr;
 }
-- 
2.17.1

[PULL 4/4] qga: Use qemu_get_host_name() instead of g_get_host_name()

[Michael Roth]

From: Michal Privoznik 

Problem with g_get_host_name() is that on the first call it saves
the hostname into a global variable and from then on, every
subsequent call returns the saved hostname. Even if the hostname
changes. This doesn't play nicely with guest agent, because if
the hostname is acquired before the guest is set up (e.g. on the
first boot, or before DHCP) we will report old, invalid hostname.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1845127

Signed-off-by: Michal Privoznik 
Reviewed-by: Daniel P. Berrangé 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael Roth 
---
 qga/commands.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/qga/commands.c b/qga/commands.c
index efc8b90281..d3fec807c1 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -515,11 +515,20 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
 GuestHostName *qmp_guest_get_host_name(Error **errp)
 {
 GuestHostName *result = NULL;
-gchar const *hostname = g_get_host_name();
-if (hostname != NULL) {
-result = g_new0(GuestHostName, 1);
-result->host_name = g_strdup(hostname);
+g_autofree char *hostname = qemu_get_host_name(errp);
+
+/*
+ * We want to avoid using g_get_host_name() because that
+ * caches the result and we wouldn't reflect changes in the
+ * host name.
+ */
+
+if (!hostname) {
+hostname = g_strdup("localhost");
 }
+
+result = g_new0(GuestHostName, 1);
+result->host_name = g_steal_pointer();
 return result;
 }
 
-- 
2.17.1

[PULL 0/4] qemu-ga patch queue for hard-freeze

[Michael Roth]

The following changes since commit 20c1df5476e1e9b5d3f5b94f9f3ce01d21f14c46:

  Merge remote-tracking branch 
'remotes/kraxel/tags/fixes-20200713-pull-request' into staging (2020-07-13 
16:58:44 +0100)

are available in the Git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2020-07-13-tag

for you to fetch changes up to 0d3a8f32b1e0eca279da1b0cc793efc7250c3daf:

  qga: Use qemu_get_host_name() instead of g_get_host_name() (2020-07-13 
17:44:58 -0500)


qemu-ga patch queue for hard-freeze

* fix erroneously reporting stale hostname in guest-get-host-name
* fix regression where guest-shutdown asserts when called
* fix race condition with guest-fs-freeze/thaw on w32


Basil Salman (1):
  qga-win: Fix QGA VSS Provider service stop failure

Marc-André Lureau (1):
  qga: fix assert regression on guest-shutdown

Michal Privoznik (2):
  util: Introduce qemu_get_host_name()
  qga: Use qemu_get_host_name() instead of g_get_host_name()

 include/qemu/osdep.h  | 10 ++
 qga/commands.c| 17 +
 qga/main.c|  6 +-
 qga/vss-win32/install.cpp | 33 -
 util/oslib-posix.c| 35 +++
 util/oslib-win32.c| 13 +
 6 files changed, 96 insertions(+), 18 deletions(-)

[PULL 3/4] util: Introduce qemu_get_host_name()

[Michael Roth]

From: Michal Privoznik 

This function offers operating system agnostic way to fetch host
name. It is implemented for both POSIX-like and Windows systems.

Signed-off-by: Michal Privoznik 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael Roth 
---
 include/qemu/osdep.h | 10 ++
 util/oslib-posix.c   | 35 +++
 util/oslib-win32.c   | 13 +
 3 files changed, 58 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 979a403984..4841b5c6b5 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -655,4 +655,14 @@ static inline void qemu_reset_optind(void)
 #endif
 }
 
+/**
+ * qemu_get_host_name:
+ * @errp: Error object
+ *
+ * Operating system agnostic way of querying host name.
+ *
+ * Returns allocated hostname (caller should free), NULL on failure.
+ */
+char *qemu_get_host_name(Error **errp);
+
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 72907d4d7f..e60aea85b6 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -794,3 +794,38 @@ void sigaction_invoke(struct sigaction *action,
 }
 action->sa_sigaction(info->ssi_signo, , NULL);
 }
+
+#ifndef HOST_NAME_MAX
+# ifdef _POSIX_HOST_NAME_MAX
+#  define HOST_NAME_MAX _POSIX_HOST_NAME_MAX
+# else
+#  define HOST_NAME_MAX 255
+# endif
+#endif
+
+char *qemu_get_host_name(Error **errp)
+{
+long len = -1;
+g_autofree char *hostname = NULL;
+
+#ifdef _SC_HOST_NAME_MAX
+len = sysconf(_SC_HOST_NAME_MAX);
+#endif /* _SC_HOST_NAME_MAX */
+
+if (len < 0) {
+len = HOST_NAME_MAX;
+}
+
+/* Unfortunately, gethostname() below does not guarantee a
+ * NULL terminated string. Therefore, allocate one byte more
+ * to be sure. */
+hostname = g_new0(char, len + 1);
+
+if (gethostname(hostname, len) < 0) {
+error_setg_errno(errp, errno,
+ "cannot get hostname");
+return NULL;
+}
+
+return g_steal_pointer();
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e9b14ab178..3b49d27297 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -808,3 +808,16 @@ bool qemu_write_pidfile(const char *filename, Error **errp)
 }
 return true;
 }
+
+char *qemu_get_host_name(Error **errp)
+{
+wchar_t tmp[MAX_COMPUTERNAME_LENGTH + 1];
+DWORD size = G_N_ELEMENTS(tmp);
+
+if (GetComputerNameW(tmp, ) == 0) {
+error_setg_win32(errp, GetLastError(), "failed close handle");
+return NULL;
+}
+
+return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
+}
-- 
2.17.1

[PULL 2/4] qga: fix assert regression on guest-shutdown

[Michael Roth]

From: Marc-André Lureau 

Since commit 781f2b3d1e ("qga: process_event() simplification"),
send_response() is called unconditionally, but will assert when "rsp" is
NULL. This may happen with QCO_NO_SUCCESS_RESP commands, such as
"guest-shutdown".

Fixes: 781f2b3d1e5ef389b44016a897fd55e7a780bf35
Cc: Michael Roth 
Reported-by: Christian Ehrhardt 
Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Christian Ehrhardt 
Tested-by: Christian Ehrhardt 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael Roth 
---
 qga/main.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index f0e454f28d..3febf3b0fd 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -531,7 +531,11 @@ static int send_response(GAState *s, const QDict *rsp)
 QString *payload_qstr, *response_qstr;
 GIOStatus status;
 
-g_assert(rsp && s->channel);
+g_assert(s->channel);
+
+if (!rsp) {
+return 0;
+}
 
 payload_qstr = qobject_to_json(QOBJECT(rsp));
 if (!payload_qstr) {
-- 
2.17.1

Re: [RFC 05/65] target/riscv: remove vsll.vi, vsrl.vi, vsra.vi insns from using gvec

[Frank Chang]

On Tue, Jul 14, 2020 at 11:35 AM LIU Zhiwei  wrote:

>
>
> On 2020/7/14 10:59, Frank Chang wrote:
>
> On Sat, Jul 11, 2020 at 12:27 AM Richard Henderson <
> richard.hender...@linaro.org> wrote:
>
>> On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
>> > From: Frank Chang 
>> >
>> > vsll.vi, vsrl.vi, vsra.vi cannot use shli gvec as it requires the
>> > shift immediate value to be within the range: [0.. SEW bits].
>> > Otherwise, it will hit the assertion:
>> > tcg_debug_assert(shift >= 0 && shift < (8 << vece));
>> >
>> > However, RVV spec does not have such constraint, therefore we have to
>> > use helper functions instead.
>>
>> Why do you say that?  It does have such a constraint:
>>
>> # Only the low lg2(SEW) bits are read to obtain the shift amount from a
>> register value.
>>
>> While that only talks about the register value, I sincerely doubt that
>> the same
>> truncation does not actually apply to immediates.
>>
>> And if the entire immediate value does apply, the manual should certainly
>> specify what should happen in that case.  And at present it doesn't.
>>
>> It seems to me the bug is the bare use of GEN_OPIVI_GVEC_TRANS and thence
>> do_opivi_gvec.  The ZX parameter should be extended to more than just
>> "zero vs
>> sign-extend", it should have an option for truncating the immediate to
>> s->sew.
>>
>>
>> r~
>>
>
> The latest spec specified:
>
> Only the low *lg2(SEW) bits* are read to obtain the shift amount from a 
> *register
> value*.
> The *immediate* is treated as an *unsigned shift amount*, with a *maximum
> shift amount of 31*.
>
> Looks like the shift amount in the immediate value is not relevant with
> SEW setting.
> If so, is it better to just use do_opivi_gvec() and implement the logic by
> our own rather than using gvec IR?
>
>
> In my opinion, it doesn't matter to truncate the immediate to s->sew
> before calling the gvec IR,
> whether the constraint of immediate exits or not.
>
> Zhiwei
>
>
> Frank Chang
>
>
>
The current issue I've encountered is the test like:

*vsetvli t0,t0,e8,m1,tu,mu,d1*
*vsll.vi  v30, v30, 27*
where the SEW is 8 (i.e. vece = 0), but the immediate value is: 27.
The instruction doesn't violate the requirement specified in spec as its
value is less then 31.
However, it can't pass *tcg_debug_assert(shift >= 0 && shift < (8 <<
vece));* assertion if tcg debug option is enabled.

Frank Chang

Re: [PATCH 1/1] python: add check-python target

[Cleber Rosa]

On Mon, Jul 13, 2020 at 09:30:26PM -0400, John Snow wrote:
> Move pylintrc and flake8 up to the root of the python folder where
> they're the most useful. Add a requirements.cqa.txt file to house
> the requirements necessary to build a venv sufficient for running
> code quality analysis on the python folder. Add a makefile that
> will build the venv and run the tests.
>
> Signed-off-by: John Snow 
> ---
>  Makefile|  1 +
>  python/{qemu => }/.flake8   |  0
>  python/Makefile.include | 33 +
>  python/{qemu => }/pylintrc  |  1 +
>  python/requirements.cqa.txt |  3 +++
>  5 files changed, 38 insertions(+)
>  rename python/{qemu => }/.flake8 (100%)
>  create mode 100644 python/Makefile.include
>  rename python/{qemu => }/pylintrc (99%)
>  create mode 100644 python/requirements.cqa.txt
> 
> diff --git a/Makefile b/Makefile
> index b1b8a5a6d0..41808be392 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -478,6 +478,7 @@ dummy := $(call unnest-vars,, \
>  trace-obj-y)
>  
>  include $(SRC_PATH)/tests/Makefile.include
> +include $(SRC_PATH)/python/Makefile.include
>  
>  all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) 
> recurse-all modules $(vhost-user-json-y)
>  
> diff --git a/python/qemu/.flake8 b/python/.flake8
> similarity index 100%
> rename from python/qemu/.flake8
> rename to python/.flake8
> diff --git a/python/Makefile.include b/python/Makefile.include
> new file mode 100644
> index 00..917808e2f1
> --- /dev/null
> +++ b/python/Makefile.include
> @@ -0,0 +1,33 @@
> +# -*- Mode: makefile -*-
> +
> +PYLIB_VENV_DIR=$(BUILD_DIR)/venv/cqa
> +PYLIB_VENV_REQ=$(SRC_PATH)/python/requirements.cqa.txt
> +
> +$(PYLIB_VENV_DIR): $(PYLIB_VENV_REQ)
> + $(call quiet-command, \
> + $(PYTHON) -m venv $@, \
> + VENV, $@)
> + $(call quiet-command, \
> + $(PYLIB_VENV_DIR)/bin/python3 -m pip -q install -r 
> $(PYLIB_VENV_REQ), \
> + PIP, $(PYLIB_VENV_REQ))
> + $(call quiet-command, touch $@)
> +

Maybe we should try to create a generic rule that takes a directory
name and a requirements file and creates the venv accordingly, instead
of duplicating the other similar rules under tests/Makefile.include?

> +pylib-venv: $(PYLIB_VENV_DIR)
> +
> +check-python: pylib-venv
> + $(call quiet-command, cd $(SRC_PATH)/python && \
> + $(PYLIB_VENV_DIR)/bin/python3 -m flake8 qemu, \
> + FLAKE8, \
> + $(SRC_PATH)/python/qemu \

I can see how this venv would be very useful to run the same checks on
other Python code (for instance, the acceptance tests themselves), so
we'd also need another "check-python"-like rule, or include those on
the same call.

Ideas? :)

Thanks!
- Cleber.


signature.asc
Description: PGP signature

[Bug 1686980] Re: qemu is very slow when adding 16, 384 virtio-scsi drives

[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/1686980

Title:
  qemu is very slow when adding 16,384 virtio-scsi drives

Status in QEMU:
  Expired

Bug description:
  qemu runs very slowly when adding many virtio-scsi drives.  I have
  attached a small reproducer shell script which demonstrates this.

  Using perf shows the following stack trace taking all the time:

  72.42%71.15%  qemu-system-x86  qemu-system-x86_64   [.] drive_get
  |  
   --72.32%--drive_get
 |  
  --1.24%--__irqentry_text_start
|  
 --1.22%--smp_apic_timer_interrupt
   |  

--1.00%--local_apic_timer_interrupt
  |  
   
--1.00%--hrtimer_interrupt
 |  
  
--0.83%--__hrtimer_run_queues

|  

 --0.64%--tick_sched_timer

  21.70%21.34%  qemu-system-x86  qemu-system-x86_64   [.] 
blk_legacy_dinfo
  |
  ---blk_legacy_dinfo

   3.65% 3.59%  qemu-system-x86  qemu-system-x86_64   [.] blk_next
  |
  ---blk_next

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

Re: [PATCH v5 12/12] python/machine.py: change default wait timeout to 3 seconds

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:49AM -0400, John Snow wrote:
> Machine.wait() does not appear to be used except in the acceptance tests,
> and an infinite timeout by default in a test suite is not the most helpful.
> 
> Change it to 3 seconds, like the default shutdown timeout.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Well, for the acceptance tests, there's usually a test wide timeout,
but this is indeed a good idea!

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v5 11/12] python/machine.py: re-add sigkill warning suppression

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:48AM -0400, John Snow wrote:
> If the user kills QEMU on purpose, we don't need to warn them about that
> having happened: they know already.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 

Reviewed-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v5 10/12] python/machine.py: split shutdown into hard and soft flavors

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:47AM -0400, John Snow wrote:
> This is done primarily to avoid the 'bare except' pattern, which
> suppresses all exceptions during shutdown and can obscure errors.
> 
> Replace this with a pattern that isolates the different kind of shutdown
> paradigms (_hard_shutdown and _soft_shutdown), and a new fallback shutdown
> handler (_do_shutdown) that gracefully attempts one before the other.
> 
> This split now also ensures that no matter what happens,
> _post_shutdown() is always invoked.
> 
> shutdown() changes in behavior such that if it attempts to do a graceful
> shutdown and is unable to, it will now always raise an exception to
> indicate this. This can be avoided by the test writer in three ways:
> 
> 1. If the VM is expected to have already exited or is in the process of
> exiting, wait() can be used instead of shutdown() to clean up resources
> instead. This helps avoid race conditions in shutdown.
> 
> 2. If a test writer is expecting graceful shutdown to fail, shutdown
> should be called in a try...except block.
> 
> 3. If the test writer has no interest in performing a graceful shutdown
> at all, kill() can be used instead.
> 
> 
> Handling shutdown in this way makes it much more explicit which type of
> shutdown we want and allows the library to report problems with this
> process.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 95 +++---
>  1 file changed, 80 insertions(+), 15 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index aaa173f046..b24ce8a268 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -48,6 +48,12 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
>  """
>  
>  
> +class AbnormalShutdown(QEMUMachineError):
> +"""
> +Exception raised when a graceful shutdown was requested, but not 
> performed.
> +"""
> +
> +
>  class MonitorResponseError(qmp.QMPError):
>  """
>  Represents erroneous QMP monitor reply
> @@ -365,6 +371,7 @@ def _early_cleanup(self) -> None:
>  """
>  Perform any cleanup that needs to happen before the VM exits.
>  
> +May be invoked by both soft and hard shutdown in failover scenarios.
>  Called additionally by _post_shutdown for comprehensive cleanup.
>  """
>  # If we keep the console socket open, we may deadlock waiting
> @@ -374,32 +381,90 @@ def _early_cleanup(self) -> None:
>  self._console_socket.close()
>  self._console_socket = None
>  
> +def _hard_shutdown(self) -> None:
> +"""
> +Perform early cleanup, kill the VM, and wait for it to terminate.
> +
> +:raise subprocess.Timeout: When timeout is exceeds 60 seconds
> +waiting for the QEMU process to terminate.
> +"""
> +self._early_cleanup()

Like I commented on patch 5, I don't think the *current* type of
cleanup done is needed on a scenario like this...

> +self._popen.kill()

... as I don't remember QEMU's SIGKILL handler to be susceptible to
the race condition that motivated the closing of the console file in
the first place.  But, I also can not prove it's not susceptible at
this time.

Note: I have some old patches that added tests for QEMUMachine itself.
I intend to respin them on top of your work, so we may have a clearer
understanding of the QEMU behaviors we need to handle.  So, feel free
to take the prudent route here, and keep the early cleanup.

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v5 09/12] tests/acceptance: Don't test reboot on cubieboard

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:46AM -0400, John Snow wrote:
> cubieboard does not have a functioning reboot, it halts and QEMU does
> not exit.
> 
> vm.shutdown() is modified in a forthcoming patch that makes it less tolerant
> of race conditions on shutdown; tests should consciously decide to WAIT
> or to SHUTDOWN qemu.
> 
> So long as this test is attempting to reboot, the correct choice would
> be to WAIT for the VM to exit. However, since that's broken, we should
> SHUTDOWN instead.
> 
> SHUTDOWN is indeed what already happens when the test performs teardown,
> however, if anyone fixes cubieboard reboot in the future, this test will
> develop a new race condition that might be hard to debug.
> 
> Therefore: remove the reboot test and make it obvious that the VM is
> still running when the test concludes, where the test teardown will do
> the right thing.
> 
> Signed-off-by: John Snow 
> ---
>  tests/acceptance/boot_linux_console.py | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v5 08/12] tests/acceptance: wait() instead of shutdown() where appropriate

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:45AM -0400, John Snow wrote:
> When issuing 'reboot' to a VM with the no-reboot option, that VM will
> exit. When then issuing a shutdown command, the cleanup may race.
> 
> Add calls to vm.wait() which will gracefully mark the VM as having
> exited. Subsequent vm.shutdown() calls in generic tearDown code will not
> race when called after completion of the call.
> 
> Signed-off-by: John Snow 
> ---
>  tests/acceptance/boot_linux_console.py   | 10 ++
>  tests/acceptance/linux_ssh_mips_malta.py |  2 ++
>  2 files changed, 12 insertions(+)
> 

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [RFC 05/65] target/riscv: remove vsll.vi, vsrl.vi, vsra.vi insns from using gvec

[LIU Zhiwei]

On 2020/7/14 10:59, Frank Chang wrote:
On Sat, Jul 11, 2020 at 12:27 AM Richard Henderson 
mailto:richard.hender...@linaro.org>> 
wrote:


On 7/10/20 3:48 AM, frank.ch...@sifive.com
 wrote:
> From: Frank Chang mailto:frank.ch...@sifive.com>>
>
> vsll.vi , vsrl.vi , vsra.vi
 cannot use shli gvec as it requires the
> shift immediate value to be within the range: [0.. SEW bits].
> Otherwise, it will hit the assertion:
> tcg_debug_assert(shift >= 0 && shift < (8 << vece));
>
> However, RVV spec does not have such constraint, therefore we
have to
> use helper functions instead.

Why do you say that?  It does have such a constraint:

# Only the low lg2(SEW) bits are read to obtain the shift amount
from a
register value.

While that only talks about the register value, I sincerely doubt
that the same
truncation does not actually apply to immediates.

And if the entire immediate value does apply, the manual should
certainly
specify what should happen in that case.  And at present it doesn't.

It seems to me the bug is the bare use of GEN_OPIVI_GVEC_TRANS and
thence
do_opivi_gvec.  The ZX parameter should be extended to more than
just "zero vs
sign-extend", it should have an option for truncating the
immediate to s->sew.


r~


The latest spec specified:

Only the low *lg2(SEW) bits* are read to obtain the shift amount from 
a *register value*.
The *immediate* is treated as an *unsigned shift amount*, with a 
*maximum shift amount of 31*.


Looks like the shift amount in the immediate value is not relevant 
with SEW setting.
If so, is it better to just use do_opivi_gvec() and implement the 
logic by our own rather than using gvec IR?


In my opinion, it doesn't matter to truncate the immediate to s->sew 
before calling the gvec IR,

whether the constraint of immediate exits or not.

Zhiwei


Frank Chang

Re: [PATCH v2 4/9] tests/acceptance/boot_linux: Expand SD card image to power of 2

[Cleber Rosa]

On Mon, Jul 13, 2020 at 08:32:04PM +0200, Philippe Mathieu-Daudé wrote:
> In few commits we won't allow SD card images with invalid size
> (not aligned to a power of 2). Prepare the tests: add the
> pow2ceil() and image_pow2ceil_expand() methods and resize the
> images (expanding) of the tests using SD cards.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Since v1: Addressed review comments
> - truncate -> expand reword (Alistair Francis)
> - expand after uncompress (Niek Linnenbank)
> ---
>  tests/acceptance/boot_linux_console.py | 27 +-
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> index b7e8858c2d..8f2a6aa8a4 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -28,6 +28,18 @@
>  except CmdNotFoundError:
>  P7ZIP_AVAILABLE = False
>  
> +# round up to next power of 2
> +def pow2ceil(x):
> +return 1 if x == 0 else 2**(x - 1).bit_length()
> +

Nitpick: turn the comment into a docstring.

Then, I was going to have a second nitpick about the method name, but
realized it was following qemu-common.h's implementation.

> +# expand file size to next power of 2
> +def image_pow2ceil_expand(path):
> +size = os.path.getsize(path)
> +size_aligned = pow2ceil(size)
> +if size != size_aligned:
> +with open(path, 'ab+') as fd:
> +fd.truncate(size_aligned)
> +

Same nitpick comment about comment -> docstring here.

Either way,

Reviewed-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v2 3/9] tests/acceptance/boot_linux: Tag tests using a SD card with 'device:sd'

[Cleber Rosa]

On Mon, Jul 13, 2020 at 08:32:03PM +0200, Philippe Mathieu-Daudé wrote:
> Avocado tags are handy to automatically select tests matching
> the tags. Since these tests use a SD card, tag them.
> 
> We can run all the tests using a SD card at once with:
> 
>   $ avocado --show=app run -t u-boot tests/acceptance/
>   $ AVOCADO_ALLOW_LARGE_STORAGE=ok \
> avocado --show=app \
>   run -t device:sd tests/acceptance/
>   Fetching asset from 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd
>   Fetching asset from 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic
>   Fetching asset from 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9
>(1/3) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd: 
> PASS (19.56 s)
>(2/3) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic:
>  PASS (49.97 s)
>(3/3) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
>  PASS (20.06 s)
>   RESULTS: PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
> CANCEL 0
>   JOB TIME   : 90.02 s
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/acceptance/boot_linux_console.py | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v5 07/12] python/machine.py: Make wait() call shutdown()

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:44AM -0400, John Snow wrote:
> At this point, shutdown(has_quit=True) and wait() do essentially the
> same thing; they perform cleanup without actually instructing QEMU to
> quit.
>
> Define one in terms of the other.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [RFC 05/65] target/riscv: remove vsll.vi, vsrl.vi, vsra.vi insns from using gvec

[Frank Chang]

On Sat, Jul 11, 2020 at 12:27 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> > From: Frank Chang 
> >
> > vsll.vi, vsrl.vi, vsra.vi cannot use shli gvec as it requires the
> > shift immediate value to be within the range: [0.. SEW bits].
> > Otherwise, it will hit the assertion:
> > tcg_debug_assert(shift >= 0 && shift < (8 << vece));
> >
> > However, RVV spec does not have such constraint, therefore we have to
> > use helper functions instead.
>
> Why do you say that?  It does have such a constraint:
>
> # Only the low lg2(SEW) bits are read to obtain the shift amount from a
> register value.
>
> While that only talks about the register value, I sincerely doubt that the
> same
> truncation does not actually apply to immediates.
>
> And if the entire immediate value does apply, the manual should certainly
> specify what should happen in that case.  And at present it doesn't.
>
> It seems to me the bug is the bare use of GEN_OPIVI_GVEC_TRANS and thence
> do_opivi_gvec.  The ZX parameter should be extended to more than just
> "zero vs
> sign-extend", it should have an option for truncating the immediate to
> s->sew.
>
>
> r~
>

The latest spec specified:

Only the low *lg2(SEW) bits* are read to obtain the shift amount from
a *register
value*.
The *immediate* is treated as an *unsigned shift amount*, with a *maximum
shift amount of 31*.

Looks like the shift amount in the immediate value is not relevant with SEW
setting.
If so, is it better to just use do_opivi_gvec() and implement the logic by
our own rather than using gvec IR?

Frank Chang

Re: [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj

[Havard Skinnemoen]

On Mon, Jul 13, 2020 at 7:57 AM Cédric Le Goater  wrote:
>
> On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
> > This allows these NPCM7xx-based boards to boot from a flash image, e.g.
> > one built with OpenBMC. For example like this:
> >
> > IMAGE=${OPENBMC}/build/tmp/deploy/images/gsj/image-bmc
> > qemu-system-arm -machine quanta-gsj -nographic \
> >   -bios ~/qemu/bootrom/npcm7xx_bootrom.bin \
> >   -drive file=${IMAGE},if=mtd,bus=0,unit=0,format=raw,snapshot=on
> >
> > Reviewed-by: Tyrone Ting 
> > Signed-off-by: Havard Skinnemoen 
>
> May be we don't need to create the flash object if dinfo is NULL.

It's soldered on the board, so you can't really boot the board without
it. But if you think it's better to remove it altogether if we don't
have an image to load into it, I can do that.

>
> Reviewed-by: Cédric Le Goater 
> Tested-by: Cédric Le Goater 
>
> Nice !
>
> We need a SPI controller model and a network device model now.

Yeah, and i2c, PWM, GPIO, etc., but if you're referring to the kernel
crash, see below.

> npcm7xx_bootrom.bin is a bit of a pain. Could we include it in
> the QEMU roms ?

Yeah, I was planning to include this in v6.

> spi_master spi0: /ahb/fiu@fb00/spi-nor@0 has no valid 'spi-max-frequency' 
> property (-22)
> spi_master spi0: Failed to create SPI device for /ahb/fiu@fb00/spi-nor@0

This is a device tree bug:

https://github.com/hskinnemoen/openbmc/commit/99b172f88002f4fac939f85debe1187b9c569871

> libphy: Fixed MDIO Bus: probed
> 8<--- cut here ---
> Unable to handle kernel paging request at virtual address fffe

I believe this is a kernel bug:

https://github.com/hskinnemoen/openbmc/commit/77e9f58ba157eabc976f15fa49892128fe2b2382

I needed two additional patches to get all the way to the login prompt:

https://github.com/hskinnemoen/openbmc/commits/20200711-gsj-qemu-0

Re: [PATCH v5 06/12] python/machine.py: Add a configurable timeout to shutdown()

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:43AM -0400, John Snow wrote:
> Three seconds is hardcoded. Use it as a default parameter instead, and use 
> that
> value for both waits that may occur in the function.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v5 05/12] python/machine.py: Prohibit multiple shutdown() calls

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:42AM -0400, John Snow wrote:
> If the VM is not launched, don't try to shut it down. As a change,
> _post_shutdown now unconditionally also calls _early_cleanup in order to
> offer comprehensive object cleanup in failure cases.
> 
> As a courtesy, treat it as a NOP instead of rejecting it as an
> error. This is slightly nicer for acceptance tests where vm.shutdown()
> is issued unconditionally in tearDown callbacks.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index cac466fbe6..e66a7d66dd 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -283,6 +283,13 @@ def _post_launch(self):
>  self._qmp.accept()
>  
>  def _post_shutdown(self):
> +"""
> +Called to cleanup the VM instance after the process has exited.
> +May also be called after a failed launch.
> +"""
> +# Comprehensive reset for the failed launch case:
> +self._early_cleanup()
> +

I'm not following why this is needed here.  AFAICT, the closing of the
console socket file was introduced when the QEMU process was alive,
and doing I/O on the serial device attached to the console file (and
was only necessary because of that).  In those scenarios, a race
between the time of sending the QMP command to quit, and getting a
response from QMP could occur.

But here, IIUC, there's no expectations for the QEMU process to be alive.
To the best of my understanding and testing, this line did not contribute
to the robustness of the shutdown behavior.

Finally, given that currently, only the closing of the console socket
is done within _early_cleanup(), and that is conditional, this also does
no harm AFAICT.  If more early cleanups operations were needed, then I
would feel less bothered about calling it here.

>  if self._qmp:
>  self._qmp.close()
>  self._qmp = None
> @@ -328,7 +335,7 @@ def launch(self):
>  self._launch()
>  self._launched = True
>  except:
> -self.shutdown()
> +self._post_shutdown()
>  
>  LOG.debug('Error launching VM')
>  if self._qemu_full_args:
> @@ -357,6 +364,8 @@ def _launch(self):
>  def _early_cleanup(self) -> None:
>  """
>  Perform any cleanup that needs to happen before the VM exits.
> +
> +Called additionally by _post_shutdown for comprehensive cleanup.
>  """
>  # If we keep the console socket open, we may deadlock waiting
>  # for QEMU to exit, while QEMU is waiting for the socket to
> @@ -377,6 +386,9 @@ def shutdown(self, has_quit=False, hard=False):
>  """
>  Terminate the VM and clean up
>  """
> +if not self._launched:
> +return
> +

Because of the additional logic, it'd be a good opportunity to make
the docstring more accurate.  This method may now *not* do *any* of if
termination and cleaning (while previously it would attempt those
anyhow).

- Cleber.


signature.asc
Description: PGP signature

Re: [PATCH v5 09/11] hw/ssi: NPCM7xx Flash Interface Unit device model

[Havard Skinnemoen]

On Mon, Jul 13, 2020 at 10:39 AM Philippe Mathieu-Daudé  wrote:
>
> On 7/12/20 7:42 AM, Havard Skinnemoen wrote:
> > On Thu, Jul 9, 2020 at 10:00 AM Philippe Mathieu-Daudé  
> > wrote:
> >> On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
> >>> This implements a device model for the NPCM7xx SPI flash controller.
> >>>
> >>> Direct reads and writes, and user-mode transactions have been tested in
> >>> various modes. Protection features are not implemented yet.
> >>>
> >>> All the FIU instances are available in the SoC's address space,
> >>> regardless of whether or not they're connected to actual flash chips.
> >>>
> >>> Reviewed-by: Tyrone Ting 
> >>> Reviewed-by: Cédric Le Goater 
> >>> Signed-off-by: Havard Skinnemoen 
> >>> ---
> >>>  include/hw/arm/npcm7xx.h |   2 +
> >>>  include/hw/ssi/npcm7xx_fiu.h | 100 +++
> >>>  hw/arm/npcm7xx.c |  53 
> >>>  hw/ssi/npcm7xx_fiu.c | 510 +++
> >>>  hw/arm/Kconfig   |   1 +
> >>>  hw/ssi/Makefile.objs |   1 +
> >>>  hw/ssi/trace-events  |   9 +
> >>>  7 files changed, 676 insertions(+)
> >>>  create mode 100644 include/hw/ssi/npcm7xx_fiu.h
> >>>  create mode 100644 hw/ssi/npcm7xx_fiu.c
> [...]
>
> >>> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> >>> index 4d227bb74b..c9ff3dab25 100644
> >>> --- a/hw/arm/npcm7xx.c
> >>> +++ b/hw/arm/npcm7xx.c
> >>> @@ -98,6 +98,37 @@ static const hwaddr npcm7xx_uart_addr[] = {
> >>>  0xf0004000,
> >>>  };
> >>>
> >>> +static const hwaddr npcm7xx_fiu0_flash_addr[] = {
> >>
> >> So per
> >> https://github.com/Nuvoton-Israel/bootblock/blob/master/SWC_HAL/Chips/npcm750/npcm750.h
> >> this is SPI0 on AHB18,
> >>
> >>> +0x8000,
> >>> +0x8800,
> >>
> >> CS0 & CS1,
> >>
> >> also listed:
> >>
> >> 0x9000, // CS2
> >> 0x9800, // CS3
> >
> > Confirmed with Nuvoton off-list that these do not exist. SPI0 only
> > supports two chip-selects for direct access.
>
> I suppose Novoton confirmed for the particular npcm750, but you aim
> to model the npcm7xx family. I doubt 2 similar IP blocks are that
> different ;) Anyway with a comment this is good.

OK. I'm mostly concerned about modeling the chips I know about for
now. If a chip with four AXI endpoints and four chipselects on SPI0
appears, it will be a fairly small change to move these arrays into
the SoC class struct, but I'd rather not do that without evidence that
such a chip exist.

The IP blocks may be identical for all I know, but they are not wired
up identically, and that's really what matters here in the SoC-level
model.

> >
> > I'll add comments.
> >
> >>> +};
> >>> +
> >>> +static const hwaddr npcm7xx_fiu3_flash_addr[] = {
> >>
> >> Ditto SPI3 on AHB3, and CS0 to CS3.
> >>
> >>> +0xa000,
> >>> +0xa800,
> >>> +0xb000,
> >>> +0xb800,
> >>> +};
> >>> +
> >>> +static const struct {
> >>> +const char *name;
> >>> +hwaddr regs_addr;
> >>> +int cs_count;
> >>> +const hwaddr *flash_addr;
> >>> +} npcm7xx_fiu[] = {
> >>> +{
> >>> +.name = "fiu0",
> >>> +.regs_addr = 0xfb00,
> >>> +.cs_count = ARRAY_SIZE(npcm7xx_fiu0_flash_addr),
> >>
> >> Hmm without the datasheet, can't tell, but I'd expect 4 CS
> >> regardless.
> >
> > FIU0/SPI0 only has 2 chip selects.
> >
> >>> +.flash_addr = npcm7xx_fiu0_flash_addr,
> >>> +}, {
> >>> +.name = "fiu3",
> >>> +.regs_addr = 0xc000,
> >>> +.cs_count = ARRAY_SIZE(npcm7xx_fiu3_flash_addr),
> >>> +.flash_addr = npcm7xx_fiu3_flash_addr,
> >>> +},
> >>> +};
> [...]
>
> >>> +
> >>> +/* Flash chip model expects one transfer per dummy bit, not byte */
> >>> +dummy_cycles =
> >>> +(FIU_DRD_CFG_DBW(drd_cfg) * 8) >> FIU_DRD_CFG_ACCTYPE(drd_cfg);
> >>> +for (i = 0; i < dummy_cycles; i++) {
> >>> +ssi_transfer(fiu->spi, 0);
> >>
> >> Note to self, might worth a ssi_shift_dummy(count) generic method.
> >
> > I'm not a huge fan of this interface to be honest. It requires the
> > flash controller to have intimate knowledge of the flash chip, and if
> > it doesn't predict the dummy phase correctly, or the guest programs
> > the wrong number of dummy cycles, the end result is very different
> > from what you'll see on a real system. I'd love to see something like
> > a number-of-bits parameter to ssi_transfer instead.
>
> Do you mean like these?
>
> - ssi_transfer_bit(bool value);
> - ssi_shift_dummy_bits(size_t bits);
>
> Some interfaces allow bit shifting. SPI doesn't simply because
> nobody had the use :)

I mean I don't like how the semantics of ssi_transfer changes
implicitly in the middle of the transfer. One call shifts a whole
byte, the next 8 shift individual bits, then it's back to bytes again.
If a mistake is made, the flash controller might end up thinking it's
shifting 16 bits, but if the flash device only expects 8 dummy bits,
it will see 8 bits + 8 bytes for a total of 9 bytes of dummies. 

[PATCH 0/1] python: add make check-python target

[John Snow]

Quick try at getting the code quality analysis tools into a makefile
target.

Based-on: 20200710052220.3306-1-js...@redhat.com
Based-on: 20200710050649.32434-1-js...@redhat.com

There are other python patches pulled since that cause new errors to
show up, so this definitely can't be enforced just yet.

John Snow (1):
  python: add check-python target

 Makefile|  1 +
 python/{qemu => }/.flake8   |  0
 python/Makefile.include | 33 +
 python/{qemu => }/pylintrc  |  1 +
 python/requirements.cqa.txt |  3 +++
 5 files changed, 38 insertions(+)
 rename python/{qemu => }/.flake8 (100%)
 create mode 100644 python/Makefile.include
 rename python/{qemu => }/pylintrc (99%)
 create mode 100644 python/requirements.cqa.txt

-- 
2.21.3

[PATCH 1/1] python: add check-python target

[John Snow]

Move pylintrc and flake8 up to the root of the python folder where
they're the most useful. Add a requirements.cqa.txt file to house
the requirements necessary to build a venv sufficient for running
code quality analysis on the python folder. Add a makefile that
will build the venv and run the tests.

Signed-off-by: John Snow 
---
 Makefile|  1 +
 python/{qemu => }/.flake8   |  0
 python/Makefile.include | 33 +
 python/{qemu => }/pylintrc  |  1 +
 python/requirements.cqa.txt |  3 +++
 5 files changed, 38 insertions(+)
 rename python/{qemu => }/.flake8 (100%)
 create mode 100644 python/Makefile.include
 rename python/{qemu => }/pylintrc (99%)
 create mode 100644 python/requirements.cqa.txt

diff --git a/Makefile b/Makefile
index b1b8a5a6d0..41808be392 100644
--- a/Makefile
+++ b/Makefile
@@ -478,6 +478,7 @@ dummy := $(call unnest-vars,, \
 trace-obj-y)
 
 include $(SRC_PATH)/tests/Makefile.include
+include $(SRC_PATH)/python/Makefile.include
 
 all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) recurse-all 
modules $(vhost-user-json-y)
 
diff --git a/python/qemu/.flake8 b/python/.flake8
similarity index 100%
rename from python/qemu/.flake8
rename to python/.flake8
diff --git a/python/Makefile.include b/python/Makefile.include
new file mode 100644
index 00..917808e2f1
--- /dev/null
+++ b/python/Makefile.include
@@ -0,0 +1,33 @@
+# -*- Mode: makefile -*-
+
+PYLIB_VENV_DIR=$(BUILD_DIR)/venv/cqa
+PYLIB_VENV_REQ=$(SRC_PATH)/python/requirements.cqa.txt
+
+$(PYLIB_VENV_DIR): $(PYLIB_VENV_REQ)
+   $(call quiet-command, \
+   $(PYTHON) -m venv $@, \
+   VENV, $@)
+   $(call quiet-command, \
+   $(PYLIB_VENV_DIR)/bin/python3 -m pip -q install -r 
$(PYLIB_VENV_REQ), \
+   PIP, $(PYLIB_VENV_REQ))
+   $(call quiet-command, touch $@)
+
+pylib-venv: $(PYLIB_VENV_DIR)
+
+check-python: pylib-venv
+   $(call quiet-command, cd $(SRC_PATH)/python && \
+   $(PYLIB_VENV_DIR)/bin/python3 -m flake8 qemu, \
+   FLAKE8, \
+   $(SRC_PATH)/python/qemu \
+   )
+   $(call quiet-command, cd $(SRC_PATH)/python && \
+   $(PYLIB_VENV_DIR)/bin/python3 -m pylint qemu, \
+   PYLINT, \
+   $(SRC_PATH)/python/qemu \
+   )
+   $(call quiet-command, cd $(SRC_PATH)/python && \
+   $(PYLIB_VENV_DIR)/bin/python3 -m mypy \
+   --strict --no-error-summary qemu, \
+   MYPY, \
+   "--strict $(SRC_PATH)/python/qemu" \
+   )
diff --git a/python/qemu/pylintrc b/python/pylintrc
similarity index 99%
rename from python/qemu/pylintrc
rename to python/pylintrc
index 5d6ae7367d..65b4545a6b 100644
--- a/python/qemu/pylintrc
+++ b/python/pylintrc
@@ -16,6 +16,7 @@ disable=too-many-arguments,
 too-many-public-methods,
 
 [REPORTS]
+score=no
 
 [REFACTORING]
 
diff --git a/python/requirements.cqa.txt b/python/requirements.cqa.txt
new file mode 100644
index 00..dbf45984bc
--- /dev/null
+++ b/python/requirements.cqa.txt
@@ -0,0 +1,3 @@
+pylint==2.5.3
+mypy==0.782
+flake8==3.8.3
-- 
2.21.3

Re: [PATCH v2 2/2] hw/riscv: sifive_u: Provide a reliable way for bootloader to detect whether it is running in QEMU

[Alistair Francis]

On Sun, Jul 12, 2020 at 6:16 PM Bin Meng  wrote:
>
> Hi Alistair,
>
> On Sun, Jul 12, 2020 at 12:04 AM Alistair Francis  
> wrote:
> >
> > On Thu, Jul 9, 2020 at 5:50 PM Bin Meng  wrote:
> > >
> > > Hi Palmer,
> > >
> > > On Fri, Jul 10, 2020 at 8:45 AM Palmer Dabbelt  
> > > wrote:
> > > >
> > > > On Thu, 09 Jul 2020 15:09:18 PDT (-0700), alistai...@gmail.com wrote:
> > > > > On Thu, Jul 9, 2020 at 3:07 AM Bin Meng  wrote:
> > > > >>
> > > > >> From: Bin Meng 
> > > > >>
> > > > >> The reset vector codes are subject to change, e.g.: with recent
> > > > >> fw_dynamic type image support, it breaks oreboot again.
> > > > >
> > > > > This is a recurring problem, I have another patch for Oreboot to fix
> > > > > the latest breakage.
> > > > >
> > > > >>
> > > > >> Add a subregion in the MROM, with the size of machine RAM stored,
> > > > >> so that we can provide a reliable way for bootloader to detect
> > > > >> whether it is running in QEMU.
> > > > >
> > > > > I don't really like this though. I would prefer that we don't
> > > > > encourage guest software to behave differently on QEMU. I don't think
> > > > > other upstream boards do this.
> > > >
> > > > I agree.  If you want an explicitly virtual board, use the virt board.  
> > > > Users
> > > > of sifive_u are presumably trying to do their best to test against what 
> > > > the
> > > > hardware does without actually using the hardware.  Otherwise there 
> > > > should be
> > > > no reason to use the sifive_u board, as it's just sticking a layer of
> > > > complexity in the middle of everything.
> > >
> > > Understood. Then let's drop this patch.
> > >
> > > >
> > > > > Besides Oreboot setting up the clocks are there any other users of 
> > > > > this?
> > > >
> > > > IIRC we have a scheme for handling the clock setup in QEMU where we 
> > > > accept
> > > > pretty much any control write and then just return reads that say the 
> > > > PLLs have
> > > > locked.  I'd be in favor of improving the scheme to improve 
> > > > compatibility with
> > > > the actual hardware, but adding some way for programs to skip the clocks
> > > > because they know they're in QEMU seems like the wrong way to go.
> > > >
> > >
> > > Yep, that's my question to Oreboot too.
> > >
> > > U-Boot SPL can boot with QEMU and no problem was seen with clock
> > > settings in PRCI model in QEMU.
> >
> > I don't think it's an unsolvable problem. There is just little work on
> > Oreboot to run on QEMU. I can dig into it a bit and see if I can find
> > a better fix on the Oreboot side.
> >
>
> Can we remove the QEMU detect logic completely in Oreboot? Except the
> QSPI controller QEMU should be able to run Oreboot since it runs
> U-Boot SPL.

That is the eventual goal.

Alistair

>
> Regards,
> Bin

[PULL 14/15] tcg/riscv: Remove superfluous breaks

[Alistair Francis]

From: Liao Pingfang 

Remove superfluous breaks, as there is a "return" before them.

Signed-off-by: Liao Pingfang 
Signed-off-by: Yi Wang 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Reviewed-by: Alistair Francis 
Message-Id: <1594600421-22942-1-git-send-email-wang.y...@zte.com.cn>
Signed-off-by: Alistair Francis 
---
 tcg/riscv/tcg-target.inc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 2bc0ba71f2..3c11ab8b7a 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -502,10 +502,8 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
 break;
 case R_RISCV_JAL:
 return reloc_jimm20(code_ptr, (tcg_insn_unit *)value);
-break;
 case R_RISCV_CALL:
 return reloc_call(code_ptr, (tcg_insn_unit *)value);
-break;
 default:
 tcg_abort();
 }
-- 
2.27.0

[PULL 12/15] hw/char: Convert the Ibex UART to use the qdev Clock model

[Alistair Francis]

Conver the Ibex UART to use the recently added qdev-clock functions.

Signed-off-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 
b0136fad870a29049959ec161c1217b967d7e19d.1594332223.git.alistair.fran...@wdc.com
Message-Id: 

---
 include/hw/char/ibex_uart.h |  3 +++
 hw/char/ibex_uart.c | 30 +++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
index 2bec772615..6d81051161 100644
--- a/include/hw/char/ibex_uart.h
+++ b/include/hw/char/ibex_uart.h
@@ -72,6 +72,7 @@
 #define IBEX_UART_TIMEOUT_CTRL 0x2c
 
 #define IBEX_UART_TX_FIFO_SIZE 16
+#define IBEX_UART_CLOCK 5000 /* 50MHz clock */
 
 #define TYPE_IBEX_UART "ibex-uart"
 #define IBEX_UART(obj) \
@@ -101,6 +102,8 @@ typedef struct {
 uint32_t uart_val;
 uint32_t uart_timeout_ctrl;
 
+Clock *f_clk;
+
 CharBackend chr;
 qemu_irq tx_watermark;
 qemu_irq rx_watermark;
diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
index 45cd724998..ab6247de89 100644
--- a/hw/char/ibex_uart.c
+++ b/hw/char/ibex_uart.c
@@ -28,6 +28,7 @@
 #include "qemu/osdep.h"
 #include "hw/char/ibex_uart.h"
 #include "hw/irq.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/log.h"
@@ -203,6 +204,17 @@ static void ibex_uart_reset(DeviceState *dev)
 ibex_uart_update_irqs(s);
 }
 
+static uint64_t ibex_uart_get_baud(IbexUartState *s)
+{
+uint64_t baud;
+
+baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
+baud *= clock_get_hz(s->f_clk);
+baud >>= 20;
+
+return baud;
+}
+
 static uint64_t ibex_uart_read(void *opaque, hwaddr addr,
unsigned int size)
 {
@@ -329,9 +341,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
   "%s: UART_CTRL_RXBLVL is not supported\n", __func__);
 }
 if (value & UART_CTRL_NCO) {
-uint64_t baud = ((value & UART_CTRL_NCO) >> 16);
-baud *= 1000;
-baud >>= 20;
+uint64_t baud = ibex_uart_get_baud(s);
 
 s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
 }
@@ -385,6 +395,16 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
 }
 }
 
+static void ibex_uart_clk_update(void *opaque)
+{
+IbexUartState *s = opaque;
+
+/* recompute uart's speed on clock change */
+uint64_t baud = ibex_uart_get_baud(s);
+
+s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
+}
+
 static void fifo_trigger_update(void *opaque)
 {
 IbexUartState *s = opaque;
@@ -444,6 +464,10 @@ static void ibex_uart_init(Object *obj)
 {
 IbexUartState *s = IBEX_UART(obj);
 
+s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock",
+  ibex_uart_clk_update, s);
+clock_set_hz(s->f_clk, IBEX_UART_CLOCK);
+
 sysbus_init_irq(SYS_BUS_DEVICE(obj), >tx_watermark);
 sysbus_init_irq(SYS_BUS_DEVICE(obj), >rx_watermark);
 sysbus_init_irq(SYS_BUS_DEVICE(obj), >tx_empty);
-- 
2.27.0

[PULL 10/15] target/riscv: fix return value of do_opivx_widen()

[Alistair Francis]

From: Frank Chang 

do_opivx_widen() should return false if check function returns false.

Signed-off-by: Frank Chang 
Reviewed-by: Richard Henderson 
Message-Id: <20200710104920.13550-4-frank.ch...@sifive.com>
Signed-off-by: Alistair Francis 
---
 target/riscv/insn_trans/trans_rvv.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/insn_trans/trans_rvv.inc.c 
b/target/riscv/insn_trans/trans_rvv.inc.c
index 7cd08f0868..c0b7375927 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -1151,7 +1151,7 @@ static bool do_opivx_widen(DisasContext *s, arg_rmrr *a,
 if (opivx_widen_check(s, a)) {
 return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
 }
-return true;
+return false;
 }
 
 #define GEN_OPIVX_WIDEN_TRANS(NAME) \
-- 
2.27.0

[PULL 08/15] target/riscv: fix rsub gvec tcg_assert_listed_vecop assertion

[Alistair Francis]

From: Frank Chang 

gvec should provide vecop_list to avoid:
"tcg_tcg_assert_listed_vecop: code should not be reached bug" assertion.

Signed-off-by: Frank Chang 
Reviewed-by: Richard Henderson 
Message-Id: <20200710104920.13550-2-frank.ch...@sifive.com>
Signed-off-by: Alistair Francis 
---
 target/riscv/insn_trans/trans_rvv.inc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/target/riscv/insn_trans/trans_rvv.inc.c 
b/target/riscv/insn_trans/trans_rvv.inc.c
index dc333e6a91..433cdacbe1 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -958,22 +958,27 @@ static void gen_rsub_vec(unsigned vece, TCGv_vec r, 
TCGv_vec a, TCGv_vec b)
 static void tcg_gen_gvec_rsubs(unsigned vece, uint32_t dofs, uint32_t aofs,
TCGv_i64 c, uint32_t oprsz, uint32_t maxsz)
 {
+static const TCGOpcode vecop_list[] = { INDEX_op_sub_vec, 0 };
 static const GVecGen2s rsub_op[4] = {
 { .fni8 = gen_vec_rsub8_i64,
   .fniv = gen_rsub_vec,
   .fno = gen_helper_vec_rsubs8,
+  .opt_opc = vecop_list,
   .vece = MO_8 },
 { .fni8 = gen_vec_rsub16_i64,
   .fniv = gen_rsub_vec,
   .fno = gen_helper_vec_rsubs16,
+  .opt_opc = vecop_list,
   .vece = MO_16 },
 { .fni4 = gen_rsub_i32,
   .fniv = gen_rsub_vec,
   .fno = gen_helper_vec_rsubs32,
+  .opt_opc = vecop_list,
   .vece = MO_32 },
 { .fni8 = gen_rsub_i64,
   .fniv = gen_rsub_vec,
   .fno = gen_helper_vec_rsubs64,
+  .opt_opc = vecop_list,
   .prefer_i64 = TCG_TARGET_REG_BITS == 64,
   .vece = MO_64 },
 };
-- 
2.27.0

[PULL 09/15] target/riscv: correct the gvec IR called in gen_vec_rsub16_i64()

[Alistair Francis]

From: Frank Chang 

Signed-off-by: Frank Chang 
Reviewed-by: Richard Henderson 
Message-Id: <20200710104920.13550-3-frank.ch...@sifive.com>
Signed-off-by: Alistair Francis 
---
 target/riscv/insn_trans/trans_rvv.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/insn_trans/trans_rvv.inc.c 
b/target/riscv/insn_trans/trans_rvv.inc.c
index 433cdacbe1..7cd08f0868 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -937,7 +937,7 @@ static void gen_vec_rsub8_i64(TCGv_i64 d, TCGv_i64 a, 
TCGv_i64 b)
 
 static void gen_vec_rsub16_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
 {
-tcg_gen_vec_sub8_i64(d, b, a);
+tcg_gen_vec_sub16_i64(d, b, a);
 }
 
 static void gen_rsub_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2)
-- 
2.27.0

Re: [PULL 00/15] riscv-to-apply queue

[Alistair Francis]

On Mon, Jul 13, 2020 at 5:43 PM Alistair Francis
 wrote:
>
> The following changes since commit 20c1df5476e1e9b5d3f5b94f9f3ce01d21f14c46:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/fixes-20200713-pull-request' into staging (2020-07-13 
> 16:58:44 +0100)
>
> are available in the Git repository at:
>
>   g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200713
>
> for you to fetch changes up to cfad709bceb629a4ebeb5d8a3acd1871b9a6436b:
>
>   target/riscv: Fix pmp NA4 implementation (2020-07-13 17:25:37 -0700)

Sorry these are a little late, I was hoping to send them out last week
but I was chasing down some bugs and waiting on a few patches.

Alistair

>
> 
> This is a colection of bug fixes and small imrprovements for RISC-V.
>
> This includes some vector extensions fixes, a PMP bug fix, OpenTitan
> UART bug fix and support for OpenSBI dynamic firmware.
>
> 
> Alexandre Mergnat (1):
>   target/riscv: Fix pmp NA4 implementation
>
> Alistair Francis (2):
>   hw/char: Convert the Ibex UART to use the qdev Clock model
>   hw/char: Convert the Ibex UART to use the registerfields API
>
> Atish Patra (4):
>   riscv: Unify Qemu's reset vector code path
>   RISC-V: Copy the fdt in dram instead of ROM
>   riscv: Add opensbi firmware dynamic support
>   RISC-V: Support 64 bit start address
>
> Bin Meng (3):
>   MAINTAINERS: Add an entry for OpenSBI firmware
>   hw/riscv: virt: Sort the SoC memmap table entries
>   hw/riscv: Modify MROM size to end at 0x1
>
> Frank Chang (4):
>   target/riscv: fix rsub gvec tcg_assert_listed_vecop assertion
>   target/riscv: correct the gvec IR called in gen_vec_rsub16_i64()
>   target/riscv: fix return value of do_opivx_widen()
>   target/riscv: fix vill bit index in vtype register
>
> Liao Pingfang (1):
>   tcg/riscv: Remove superfluous breaks
>
>  include/hw/char/ibex_uart.h |  79 
>  include/hw/riscv/boot.h |   7 ++
>  include/hw/riscv/boot_opensbi.h |  58 
>  target/riscv/cpu.h  |   2 +-
>  hw/char/ibex_uart.c | 158 
> ++--
>  hw/riscv/boot.c | 107 +
>  hw/riscv/sifive_u.c |  53 ++-
>  hw/riscv/spike.c|  59 
>  hw/riscv/virt.c |  63 -
>  target/riscv/insn_trans/trans_rvv.inc.c |   9 +-
>  target/riscv/pmp.c  |   2 +-
>  tcg/riscv/tcg-target.inc.c  |   2 -
>  MAINTAINERS |   7 ++
>  13 files changed, 387 insertions(+), 219 deletions(-)
>  create mode 100644 include/hw/riscv/boot_opensbi.h
>

[PULL 07/15] hw/riscv: Modify MROM size to end at 0x10000

[Alistair Francis]

From: Bin Meng 

At present the size of Mask ROM for sifive_u / spike / virt machines
is set to 0x11000, which ends at an unusual address. This changes the
size to 0xf000 so that it ends at 0x1.

Signed-off-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <1594289144-24723-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Alistair Francis 
---
 hw/riscv/sifive_u.c | 2 +-
 hw/riscv/spike.c| 2 +-
 hw/riscv/virt.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 6595ab3f87..19a976c9a6 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -70,7 +70,7 @@ static const struct MemmapEntry {
 hwaddr size;
 } sifive_u_memmap[] = {
 [SIFIVE_U_DEBUG] ={0x0,  0x100 },
-[SIFIVE_U_MROM] = { 0x1000,0x11000 },
+[SIFIVE_U_MROM] = { 0x1000, 0xf000 },
 [SIFIVE_U_CLINT] ={  0x200,0x1 },
 [SIFIVE_U_L2LIM] ={  0x800,  0x200 },
 [SIFIVE_U_PLIC] = {  0xc00,  0x400 },
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index b17d96aec7..7b23a297fc 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -52,7 +52,7 @@ static const struct MemmapEntry {
 hwaddr base;
 hwaddr size;
 } spike_memmap[] = {
-[SPIKE_MROM] = { 0x1000,0x11000 },
+[SPIKE_MROM] = { 0x1000, 0xf000 },
 [SPIKE_CLINT] ={  0x200,0x1 },
 [SPIKE_DRAM] = { 0x8000,0x0 },
 };
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bc25ec69f7..55a907bb35 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -53,7 +53,7 @@ static const struct MemmapEntry {
 hwaddr size;
 } virt_memmap[] = {
 [VIRT_DEBUG] =   {0x0, 0x100 },
-[VIRT_MROM] ={ 0x1000,   0x11000 },
+[VIRT_MROM] ={ 0x1000,0xf000 },
 [VIRT_TEST] ={   0x10,0x1000 },
 [VIRT_RTC] = {   0x101000,0x1000 },
 [VIRT_CLINT] =   {  0x200,   0x1 },
-- 
2.27.0

[PULL 03/15] riscv: Unify Qemu's reset vector code path

[Alistair Francis]

From: Atish Patra 

Currently, all riscv machines except sifive_u have identical reset vector
code implementations with memory addresses being different for all machines.
They can be easily combined into a single function in common code.

Move it to common function and let all the machines use the common function.

Signed-off-by: Atish Patra 
Reviewed-by: Alistair Francis 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Message-Id: <20200701183949.398134-2-atish.pa...@wdc.com>
Signed-off-by: Alistair Francis 
---
 include/hw/riscv/boot.h |  2 ++
 hw/riscv/boot.c | 46 +
 hw/riscv/sifive_u.c |  1 -
 hw/riscv/spike.c| 41 +++-
 hw/riscv/virt.c | 40 +++
 5 files changed, 54 insertions(+), 76 deletions(-)

diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 9daa98da08..3e9759c89a 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -35,5 +35,7 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
symbol_fn_t sym_cb);
 hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
  uint64_t kernel_entry, hwaddr *start);
+void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
+   hwaddr rom_size, void *fdt);
 
 #endif /* RISCV_BOOT_H */
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index adb421b91b..3df802380a 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -26,8 +26,11 @@
 #include "hw/loader.h"
 #include "hw/riscv/boot.h"
 #include "elf.h"
+#include "sysemu/device_tree.h"
 #include "sysemu/qtest.h"
 
+#include 
+
 #if defined(TARGET_RISCV32)
 # define KERNEL_BOOT_ADDRESS 0x8040
 #else
@@ -155,3 +158,46 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t 
mem_size,
 
 return *start + size;
 }
+
+void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
+   hwaddr rom_size, void *fdt)
+{
+int i;
+
+/* reset vector */
+uint32_t reset_vec[8] = {
+0x0297,  /* 1:  auipc  t0, %pcrel_hi(dtb) */
+0x02028593,  /* addi   a1, t0, %pcrel_lo(1b) */
+0xf1402573,  /* csrr   a0, mhartid  */
+#if defined(TARGET_RISCV32)
+0x0182a283,  /* lw t0, 24(t0) */
+#elif defined(TARGET_RISCV64)
+0x0182b283,  /* ld t0, 24(t0) */
+#endif
+0x00028067,  /* jr t0 */
+0x,
+start_addr,  /* start: .dword */
+0x,
+ /* dtb: */
+};
+
+/* copy in the reset vector in little_endian byte order */
+for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
+reset_vec[i] = cpu_to_le32(reset_vec[i]);
+}
+rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
+  rom_base, _space_memory);
+
+/* copy in the device tree */
+if (fdt_pack(fdt) || fdt_totalsize(fdt) >
+rom_size - sizeof(reset_vec)) {
+error_report("not enough space to store device-tree");
+exit(1);
+}
+qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
+rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
+   rom_base + sizeof(reset_vec),
+   _space_memory);
+
+return;
+}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7851326988..0695c93d2c 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -56,7 +56,6 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
-#include "exec/address-spaces.h"
 
 #include 
 
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index c107bf3ba1..a8a0588824 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -41,9 +41,6 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/qtest.h"
 #include "sysemu/sysemu.h"
-#include "exec/address-spaces.h"
-
-#include 
 
 #if defined(TARGET_RISCV32)
 # define BIOS_FILENAME "opensbi-riscv32-spike-fw_jump.elf"
@@ -165,7 +162,6 @@ static void spike_board_init(MachineState *machine)
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
 MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
-int i;
 unsigned int smp_cpus = machine->smp.cpus;
 
 /* Initialize SOC */
@@ -212,40 +208,9 @@ static void spike_board_init(MachineState *machine)
 }
 }
 
-/* reset vector */
-uint32_t reset_vec[8] = {
-0x0297,  /* 1:  auipc  t0, %pcrel_hi(dtb) */
-0x02028593,  /* addi   a1, t0, %pcrel_lo(1b) */
-0xf1402573,  /* csrr   a0, mhartid  */
-#if defined(TARGET_RISCV32)
-0x0182a283,  /* lw t0, 24(t0) */
-#elif defined(TARGET_RISCV64)
-0x0182b283,   

[PULL 15/15] target/riscv: Fix pmp NA4 implementation

[Alistair Francis]

From: Alexandre Mergnat 

The end address calculation for NA4 mode is wrong because the address
used isn't shifted.

It doesn't watch 4 bytes but a huge range because the end address
calculation is wrong.

The solution is to use the shifted address calculated for start address
variable.

Modifications are tested on Zephyr OS userspace test suite which works
for other RISC-V boards (E31 and E34 core).

Signed-off-by: Alexandre Mergnat 
Reviewed-by: Alistair Francis 
Message-id: 20200706084550.24117-1-amerg...@baylibre.com
Message-Id: <20200706084550.24117-1-amerg...@baylibre.com>
[ Changes by AF:
 - Improve the commit title and message
]
Signed-off-by: Alistair Francis 
---
 target/riscv/pmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 9418660f1b..2a2b9f5363 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -171,7 +171,7 @@ static void pmp_update_rule(CPURISCVState *env, uint32_t 
pmp_index)
 
 case PMP_AMATCH_NA4:
 sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
-ea = (this_addr + 4u) - 1u;
+ea = (sa + 4u) - 1u;
 break;
 
 case PMP_AMATCH_NAPOT:
-- 
2.27.0

Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models

[Havard Skinnemoen]

On Mon, Jul 13, 2020 at 8:02 AM Cédric Le Goater  wrote:
>
> On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
> > The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> > Management Controllers in servers. While the family includes four SoCs,
> > this patch implements limited support for two of them: NPCM730 (targeted
> > for Data Center applications) and NPCM750 (targeted for Enterprise
> > applications).
> >
> > This patch includes little more than the bare minimum needed to boot a
> > Linux kernel built with NPCM7xx support in direct-kernel mode:
> >
> >   - Two Cortex-A9 CPU cores with built-in periperhals.
> >   - Global Configuration Registers.
> >   - Clock Management.
> >   - 3 Timer Modules with 5 timers each.
> >   - 4 serial ports.
> >
> > The chips themselves have a lot more features, some of which will be
> > added to the model at a later stage.
> >
> > Reviewed-by: Tyrone Ting 
> > Reviewed-by: Joel Stanley 
> > Signed-off-by: Havard Skinnemoen 
> > ---
> >  include/hw/arm/npcm7xx.h |  86 +
> >  hw/arm/npcm7xx.c | 376 +++
> >  hw/arm/Kconfig   |   5 +
> >  hw/arm/Makefile.objs |   1 +
> >  4 files changed, 468 insertions(+)
> >  create mode 100644 include/hw/arm/npcm7xx.h
> >  create mode 100644 hw/arm/npcm7xx.c
> >
> > diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
> > new file mode 100644
> > index 00..95d9224f59
> > --- /dev/null
> > +++ b/include/hw/arm/npcm7xx.h
> > @@ -0,0 +1,86 @@
> > +/*
> > + * Nuvoton NPCM7xx SoC family.
> > + *
> > + * Copyright 2020 Google LLC
> > + *
> > + * 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.
> > + */
> > +#ifndef NPCM7XX_H
> > +#define NPCM7XX_H
> > +
> > +#include "hw/boards.h"
> > +#include "hw/cpu/a9mpcore.h"
> > +#include "hw/misc/npcm7xx_clk.h"
> > +#include "hw/misc/npcm7xx_gcr.h"
> > +#include "hw/timer/npcm7xx_timer.h"
> > +#include "target/arm/cpu.h"
> > +
> > +#define NPCM7XX_MAX_NUM_CPUS(2)
> > +
> > +/* The first half of the address space is reserved for DDR4 DRAM. */
> > +#define NPCM7XX_DRAM_BA (0x)
> > +#define NPCM7XX_DRAM_SZ (2 * GiB)
> > +
> > +/* Magic addresses for setting up direct kernel booting and SMP boot 
> > stubs. */
> > +#define NPCM7XX_LOADER_START(0x)  /* Start of SDRAM */
> > +#define NPCM7XX_SMP_LOADER_START(0x)  /* Boot ROM */
> > +#define NPCM7XX_SMP_BOOTREG_ADDR(0xf080013c)  /* GCR.SCRPAD */
> > +#define NPCM7XX_GIC_CPU_IF_ADDR (0xf03fe100)  /* GIC within A9 */
> > +
> > +typedef struct NPCM7xxState {
> > +DeviceState parent;
> > +
> > +ARMCPU  cpu[NPCM7XX_MAX_NUM_CPUS];
> > +A9MPPrivState   a9mpcore;
> > +
> > +MemoryRegionsram;
> > +MemoryRegionirom;
> > +MemoryRegionram3;
> > +MemoryRegion*dram;
> > +
> > +NPCM7xxGCRState gcr;
> > +NPCM7xxCLKState clk;
> > +NPCM7xxTimerCtrlState tim[3];
> > +} NPCM7xxState;
> > +
> > +#define TYPE_NPCM7XX"npcm7xx"
> > +#define NPCM7XX(obj)OBJECT_CHECK(NPCM7xxState, (obj), TYPE_NPCM7XX)
> > +
> > +#define TYPE_NPCM730"npcm730"
> > +#define TYPE_NPCM750"npcm750"
> > +
> > +typedef struct NPCM7xxClass {
> > +DeviceClass parent;
> > +
> > +/* Bitmask of modules that are permanently disabled on this chip. */
> > +uint32_tdisabled_modules;
> > +/* Number of CPU cores enabled in this SoC class (may be 1 or 2). */
> > +uint32_tnum_cpus;
> > +} NPCM7xxClass;
> > +
> > +#define NPCM7XX_CLASS(klass)\
> > +OBJECT_CLASS_CHECK(NPCM7xxClass, (klass), TYPE_NPCM7XX)
> > +#define NPCM7XX_GET_CLASS(obj)  \
> > +OBJECT_GET_CLASS(NPCM7xxClass, (obj), TYPE_NPCM7XX)
> > +
> > +/**
> > + * npcm7xx_write_secondary_boot - Write stub for booting secondary CPU.
> > + * @cpu: The CPU to be booted.
> > + * @info: Boot info structure for the board.
> > + *
> > + * This will write a short code stub to the internal ROM that will keep the
> > + * secondary CPU spinning until the primary CPU writes an address to the 
> > SCRPAD
> > + * register in the GCR, after which the secondary CPU will jump there.
> > + */
> > +extern void npcm7xx_write_secondary_boot(ARMCPU *cpu,
> > + const struct arm_boot_info *info);
> > +
> > +#endif /* NPCM7XX_H */
> > diff --git 

[PULL 11/15] target/riscv: fix vill bit index in vtype register

[Alistair Francis]

From: Frank Chang 

vill bit is at vtype[XLEN-1].

Signed-off-by: Frank Chang 
Reviewed-by: Richard Henderson 
Message-Id: <20200710104920.13550-5-frank.ch...@sifive.com>
Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index eef20ca6e5..a804a5d0ba 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -98,7 +98,7 @@ FIELD(VTYPE, VLMUL, 0, 2)
 FIELD(VTYPE, VSEW, 2, 3)
 FIELD(VTYPE, VEDIV, 5, 2)
 FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
-FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 2, 1)
+FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 1, 1)
 
 struct CPURISCVState {
 target_ulong gpr[32];
-- 
2.27.0

[PULL 13/15] hw/char: Convert the Ibex UART to use the registerfields API

[Alistair Francis]

Signed-off-by: Alistair Francis 
Message-id: 
06372c9cdeec715077899e71c858d9f0a2a3395b.1594332223.git.alistair.fran...@wdc.com
Message-Id: 
<06372c9cdeec715077899e71c858d9f0a2a3395b.1594332223.git.alistair.fran...@wdc.com>
---
 include/hw/char/ibex_uart.h |  76 ++---
 hw/char/ibex_uart.c | 130 ++--
 2 files changed, 100 insertions(+), 106 deletions(-)

diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
index 6d81051161..b6bd5a6700 100644
--- a/include/hw/char/ibex_uart.h
+++ b/include/hw/char/ibex_uart.h
@@ -26,50 +26,44 @@
 #define HW_IBEX_UART_H
 
 #include "hw/sysbus.h"
+#include "hw/registerfields.h"
 #include "chardev/char-fe.h"
 #include "qemu/timer.h"
 
-#define IBEX_UART_INTR_STATE   0x00
-#define INTR_STATE_TX_WATERMARK (1 << 0)
-#define INTR_STATE_RX_WATERMARK (1 << 1)
-#define INTR_STATE_TX_EMPTY (1 << 2)
-#define INTR_STATE_RX_OVERFLOW  (1 << 3)
-#define IBEX_UART_INTR_ENABLE  0x04
-#define IBEX_UART_INTR_TEST0x08
-
-#define IBEX_UART_CTRL 0x0c
-#define UART_CTRL_TX_ENABLE (1 << 0)
-#define UART_CTRL_RX_ENABLE (1 << 1)
-#define UART_CTRL_NF(1 << 2)
-#define UART_CTRL_SLPBK (1 << 4)
-#define UART_CTRL_LLPBK (1 << 5)
-#define UART_CTRL_PARITY_EN (1 << 6)
-#define UART_CTRL_PARITY_ODD(1 << 7)
-#define UART_CTRL_RXBLVL(3 << 8)
-#define UART_CTRL_NCO   (0x << 16)
-
-#define IBEX_UART_STATUS   0x10
-#define UART_STATUS_TXFULL  (1 << 0)
-#define UART_STATUS_RXFULL  (1 << 1)
-#define UART_STATUS_TXEMPTY (1 << 2)
-#define UART_STATUS_RXIDLE  (1 << 4)
-#define UART_STATUS_RXEMPTY (1 << 5)
-
-#define IBEX_UART_RDATA0x14
-#define IBEX_UART_WDATA0x18
-
-#define IBEX_UART_FIFO_CTRL0x1c
-#define FIFO_CTRL_RXRST  (1 << 0)
-#define FIFO_CTRL_TXRST  (1 << 1)
-#define FIFO_CTRL_RXILVL (7 << 2)
-#define FIFO_CTRL_RXILVL_SHIFT   (2)
-#define FIFO_CTRL_TXILVL (3 << 5)
-#define FIFO_CTRL_TXILVL_SHIFT   (5)
-
-#define IBEX_UART_FIFO_STATUS  0x20
-#define IBEX_UART_OVRD 0x24
-#define IBEX_UART_VAL  0x28
-#define IBEX_UART_TIMEOUT_CTRL 0x2c
+REG32(INTR_STATE, 0x00)
+FIELD(INTR_STATE, TX_WATERMARK, 0, 1)
+FIELD(INTR_STATE, RX_WATERMARK, 1, 1)
+FIELD(INTR_STATE, TX_EMPTY, 2, 1)
+FIELD(INTR_STATE, RX_OVERFLOW, 3, 1)
+REG32(INTR_ENABLE, 0x04)
+REG32(INTR_TEST, 0x08)
+REG32(CTRL, 0x0C)
+FIELD(CTRL, TX_ENABLE, 0, 1)
+FIELD(CTRL, RX_ENABLE, 1, 1)
+FIELD(CTRL, NF, 2, 1)
+FIELD(CTRL, SLPBK, 4, 1)
+FIELD(CTRL, LLPBK, 5, 1)
+FIELD(CTRL, PARITY_EN, 6, 1)
+FIELD(CTRL, PARITY_ODD, 7, 1)
+FIELD(CTRL, RXBLVL, 8, 2)
+FIELD(CTRL, NCO, 16, 16)
+REG32(STATUS, 0x10)
+FIELD(STATUS, TXFULL, 0, 1)
+FIELD(STATUS, RXFULL, 1, 1)
+FIELD(STATUS, TXEMPTY, 2, 1)
+FIELD(STATUS, RXIDLE, 4, 1)
+FIELD(STATUS, RXEMPTY, 5, 1)
+REG32(RDATA, 0x14)
+REG32(WDATA, 0x18)
+REG32(FIFO_CTRL, 0x1c)
+FIELD(FIFO_CTRL, RXRST, 0, 1)
+FIELD(FIFO_CTRL, TXRST, 1, 1)
+FIELD(FIFO_CTRL, RXILVL, 2, 3)
+FIELD(FIFO_CTRL, TXILVL, 5, 2)
+REG32(FIFO_STATUS, 0x20)
+REG32(OVRD, 0x24)
+REG32(VAL, 0x28)
+REG32(TIMEOUT_CTRL, 0x2c)
 
 #define IBEX_UART_TX_FIFO_SIZE 16
 #define IBEX_UART_CLOCK 5000 /* 50MHz clock */
diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
index ab6247de89..cc49a35013 100644
--- a/hw/char/ibex_uart.c
+++ b/hw/char/ibex_uart.c
@@ -36,25 +36,25 @@
 
 static void ibex_uart_update_irqs(IbexUartState *s)
 {
-if (s->uart_intr_state & s->uart_intr_enable & INTR_STATE_TX_WATERMARK) {
+if (s->uart_intr_state & s->uart_intr_enable & 
R_INTR_STATE_TX_WATERMARK_MASK) {
 qemu_set_irq(s->tx_watermark, 1);
 } else {
 qemu_set_irq(s->tx_watermark, 0);
 }
 
-if (s->uart_intr_state & s->uart_intr_enable & INTR_STATE_RX_WATERMARK) {
+if (s->uart_intr_state & s->uart_intr_enable & 
R_INTR_STATE_RX_WATERMARK_MASK) {
 qemu_set_irq(s->rx_watermark, 1);
 } else {
 qemu_set_irq(s->rx_watermark, 0);
 }
 
-if (s->uart_intr_state & s->uart_intr_enable & INTR_STATE_TX_EMPTY) {
+if (s->uart_intr_state & s->uart_intr_enable & R_INTR_STATE_TX_EMPTY_MASK) 
{
 qemu_set_irq(s->tx_empty, 1);
 } else {
 qemu_set_irq(s->tx_empty, 0);
 }
 
-if (s->uart_intr_state & s->uart_intr_enable & INTR_STATE_RX_OVERFLOW) {
+if (s->uart_intr_state & s->uart_intr_enable & 
R_INTR_STATE_RX_OVERFLOW_MASK) {
 qemu_set_irq(s->rx_overflow, 1);
 } else {
 qemu_set_irq(s->rx_overflow, 0);
@@ -65,7 +65,7 @@ static int ibex_uart_can_receive(void *opaque)
 {
 IbexUartState *s = opaque;
 
-if (s->uart_ctrl & UART_CTRL_RX_ENABLE) {
+if (s->uart_ctrl & R_CTRL_RX_ENABLE_MASK) {
 return 1;
 }
 
@@ -75,16 +75,16 @@ static int ibex_uart_can_receive(void *opaque)
 static 

[PULL 05/15] riscv: Add opensbi firmware dynamic support

[Alistair Francis]

From: Atish Patra 

OpenSBI is the default firmware in Qemu and has various firmware loading
options. Currently, qemu loader uses fw_jump which has a compile time
pre-defined address where fdt & kernel image must reside. This puts a
constraint on image size of the Linux kernel depending on the fdt location
and available memory. However, fw_dynamic allows the loader to specify
the next stage location (i.e. Linux kernel/U-Boot) in memory and other
configurable boot options available in OpenSBI.

Add support for OpenSBI dynamic firmware loading support. This doesn't
break existing setup and fw_jump will continue to work as it is. Any
other firmware will continue to work without any issues as long as it
doesn't expect anything specific from loader in "a2" register.

Signed-off-by: Atish Patra 
Reviewed-by: Alistair Francis 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Message-Id: <20200701183949.398134-4-atish.pa...@wdc.com>
Signed-off-by: Alistair Francis 
---
 include/hw/riscv/boot.h |  5 ++-
 include/hw/riscv/boot_opensbi.h | 58 +
 hw/riscv/boot.c | 42 +---
 hw/riscv/sifive_u.c | 20 +---
 hw/riscv/spike.c| 13 ++--
 hw/riscv/virt.c | 12 +--
 6 files changed, 134 insertions(+), 16 deletions(-)
 create mode 100644 include/hw/riscv/boot_opensbi.h

diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 35b6ddf710..451338780a 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -37,7 +37,10 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t 
mem_size,
  uint64_t kernel_entry, hwaddr *start);
 uint32_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
 void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
-   hwaddr rom_size,
+   hwaddr rom_size, uint64_t kernel_entry,
uint32_t fdt_load_addr, void *fdt);
+void riscv_rom_copy_firmware_info(hwaddr rom_base, hwaddr rom_size,
+  uint32_t reset_vec_size,
+  uint64_t kernel_entry);
 
 #endif /* RISCV_BOOT_H */
diff --git a/include/hw/riscv/boot_opensbi.h b/include/hw/riscv/boot_opensbi.h
new file mode 100644
index 00..0d5ddd6c3d
--- /dev/null
+++ b/include/hw/riscv/boot_opensbi.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (c) 2019 Western Digital Corporation or its affiliates.
+ *
+ * Based on include/sbi/{fw_dynamic.h,sbi_scratch.h} from the OpenSBI project.
+ */
+#ifndef OPENSBI_H
+#define OPENSBI_H
+
+/** Expected value of info magic ('OSBI' ascii string in hex) */
+#define FW_DYNAMIC_INFO_MAGIC_VALUE 0x4942534f
+
+/** Maximum supported info version */
+#define FW_DYNAMIC_INFO_VERSION 0x2
+
+/** Possible next mode values */
+#define FW_DYNAMIC_INFO_NEXT_MODE_U 0x0
+#define FW_DYNAMIC_INFO_NEXT_MODE_S 0x1
+#define FW_DYNAMIC_INFO_NEXT_MODE_M 0x3
+
+enum sbi_scratch_options {
+/** Disable prints during boot */
+SBI_SCRATCH_NO_BOOT_PRINTS = (1 << 0),
+/** Enable runtime debug prints */
+SBI_SCRATCH_DEBUG_PRINTS = (1 << 1),
+};
+
+/** Representation dynamic info passed by previous booting stage */
+struct fw_dynamic_info {
+/** Info magic */
+target_long magic;
+/** Info version */
+target_long version;
+/** Next booting stage address */
+target_long next_addr;
+/** Next booting stage mode */
+target_long next_mode;
+/** Options for OpenSBI library */
+target_long options;
+/**
+ * Preferred boot HART id
+ *
+ * It is possible that the previous booting stage uses same link
+ * address as the FW_DYNAMIC firmware. In this case, the relocation
+ * lottery mechanism can potentially overwrite the previous booting
+ * stage while other HARTs are still running in the previous booting
+ * stage leading to boot-time crash. To avoid this boot-time crash,
+ * the previous booting stage can specify last HART that will jump
+ * to the FW_DYNAMIC firmware as the preferred boot HART.
+ *
+ * To avoid specifying a preferred boot HART, the previous booting
+ * stage can set it to -1UL which will force the FW_DYNAMIC firmware
+ * to use the relocation lottery mechanism.
+ */
+target_long boot_hart;
+};
+
+#endif
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index c62f545f15..feff6e3f4e 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -25,6 +25,7 @@
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/riscv/boot.h"
+#include "hw/riscv/boot_opensbi.h"
 #include "elf.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/qtest.h"
@@ -33,8 +34,10 @@
 
 #if defined(TARGET_RISCV32)
 # define KERNEL_BOOT_ADDRESS 0x8040
+#define fw_dynamic_info_data(__val) cpu_to_le32(__val)
 #else
 # define KERNEL_BOOT_ADDRESS 0x8020

[PULL 01/15] MAINTAINERS: Add an entry for OpenSBI firmware

[Alistair Francis]

From: Bin Meng 

List me as the maintainer for OpenSBI firmware related files.

Signed-off-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <1593177220-28143-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Alistair Francis 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fe8139f367..80fa8837e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2681,6 +2681,13 @@ F: hw/i386/intel_iommu.c
 F: hw/i386/intel_iommu_internal.h
 F: include/hw/i386/intel_iommu.h
 
+OpenSBI Firmware
+M: Bin Meng 
+S: Supported
+F: pc-bios/opensbi-*
+F: .gitlab-ci.d/opensbi.yml
+F: .gitlab-ci.d/opensbi/
+
 Usermode Emulation
 --
 Overall usermode emulation
-- 
2.27.0

[PULL 02/15] hw/riscv: virt: Sort the SoC memmap table entries

[Alistair Francis]

From: Bin Meng 

Adjust the PCIe memory maps to follow the order.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
Message-Id: <1593746511-19517-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Alistair Francis 
---
 hw/riscv/virt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index f7630c8a89..18283e262e 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -60,14 +60,14 @@ static const struct MemmapEntry {
 [VIRT_TEST] ={   0x10,0x1000 },
 [VIRT_RTC] = {   0x101000,0x1000 },
 [VIRT_CLINT] =   {  0x200,   0x1 },
+[VIRT_PCIE_PIO] ={  0x300,   0x1 },
 [VIRT_PLIC] ={  0xc00, 0x400 },
 [VIRT_UART0] =   { 0x1000, 0x100 },
 [VIRT_VIRTIO] =  { 0x10001000,0x1000 },
 [VIRT_FLASH] =   { 0x2000, 0x400 },
-[VIRT_DRAM] ={ 0x8000,   0x0 },
-[VIRT_PCIE_MMIO] =   { 0x4000,0x4000 },
-[VIRT_PCIE_PIO] ={ 0x0300,0x0001 },
 [VIRT_PCIE_ECAM] =   { 0x3000,0x1000 },
+[VIRT_PCIE_MMIO] =   { 0x4000,0x4000 },
+[VIRT_DRAM] ={ 0x8000,   0x0 },
 };
 
 #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
-- 
2.27.0

[PULL 00/15] riscv-to-apply queue

[Alistair Francis]

The following changes since commit 20c1df5476e1e9b5d3f5b94f9f3ce01d21f14c46:

  Merge remote-tracking branch 
'remotes/kraxel/tags/fixes-20200713-pull-request' into staging (2020-07-13 
16:58:44 +0100)

are available in the Git repository at:

  g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200713

for you to fetch changes up to cfad709bceb629a4ebeb5d8a3acd1871b9a6436b:

  target/riscv: Fix pmp NA4 implementation (2020-07-13 17:25:37 -0700)


This is a colection of bug fixes and small imrprovements for RISC-V.

This includes some vector extensions fixes, a PMP bug fix, OpenTitan
UART bug fix and support for OpenSBI dynamic firmware.


Alexandre Mergnat (1):
  target/riscv: Fix pmp NA4 implementation

Alistair Francis (2):
  hw/char: Convert the Ibex UART to use the qdev Clock model
  hw/char: Convert the Ibex UART to use the registerfields API

Atish Patra (4):
  riscv: Unify Qemu's reset vector code path
  RISC-V: Copy the fdt in dram instead of ROM
  riscv: Add opensbi firmware dynamic support
  RISC-V: Support 64 bit start address

Bin Meng (3):
  MAINTAINERS: Add an entry for OpenSBI firmware
  hw/riscv: virt: Sort the SoC memmap table entries
  hw/riscv: Modify MROM size to end at 0x1

Frank Chang (4):
  target/riscv: fix rsub gvec tcg_assert_listed_vecop assertion
  target/riscv: correct the gvec IR called in gen_vec_rsub16_i64()
  target/riscv: fix return value of do_opivx_widen()
  target/riscv: fix vill bit index in vtype register

Liao Pingfang (1):
  tcg/riscv: Remove superfluous breaks

 include/hw/char/ibex_uart.h |  79 
 include/hw/riscv/boot.h |   7 ++
 include/hw/riscv/boot_opensbi.h |  58 
 target/riscv/cpu.h  |   2 +-
 hw/char/ibex_uart.c | 158 ++--
 hw/riscv/boot.c | 107 +
 hw/riscv/sifive_u.c |  53 ++-
 hw/riscv/spike.c|  59 
 hw/riscv/virt.c |  63 -
 target/riscv/insn_trans/trans_rvv.inc.c |   9 +-
 target/riscv/pmp.c  |   2 +-
 tcg/riscv/tcg-target.inc.c  |   2 -
 MAINTAINERS |   7 ++
 13 files changed, 387 insertions(+), 219 deletions(-)
 create mode 100644 include/hw/riscv/boot_opensbi.h

[PULL 06/15] RISC-V: Support 64 bit start address

[Alistair Francis]

From: Atish Patra 

Even though the start address in ROM code is declared as a 64 bit address
for RV64, it can't be used as upper bits are set to zero in ROM code.

Update the ROM code correctly to reflect the 64bit value.

Signed-off-by: Atish Patra 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Message-Id: <20200701183949.398134-5-atish.pa...@wdc.com>
Signed-off-by: Alistair Francis 
---
 hw/riscv/boot.c | 6 +-
 hw/riscv/sifive_u.c | 6 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index feff6e3f4e..4c6c101ff1 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -226,7 +226,11 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr 
rom_base,
uint32_t fdt_load_addr, void *fdt)
 {
 int i;
+uint32_t start_addr_hi32 = 0x;
 
+#if defined(TARGET_RISCV64)
+start_addr_hi32 = start_addr >> 32;
+#endif
 /* reset vector */
 uint32_t reset_vec[10] = {
 0x0297,  /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
@@ -241,7 +245,7 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr 
rom_base,
 #endif
 0x00028067,  /* jr t0 */
 start_addr,  /* start: .dword */
-0x,
+start_addr_hi32,
 fdt_load_addr,   /* fdt_laddr: .dword */
 0x,
  /* fw_dyn: */
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 46e6ed90ca..6595ab3f87 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -378,6 +378,7 @@ static void sifive_u_machine_init(MachineState *machine)
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
 MemoryRegion *flash0 = g_new(MemoryRegion, 1);
 target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
+uint32_t start_addr_hi32 = 0x;
 int i;
 uint32_t fdt_load_addr;
 uint64_t kernel_entry;
@@ -460,6 +461,9 @@ static void sifive_u_machine_init(MachineState *machine)
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DRAM].base,
machine->ram_size, s->fdt);
+#if defined(TARGET_RISCV64)
+start_addr_hi32 = start_addr >> 32;
+#endif
 
 /* reset vector */
 uint32_t reset_vec[11] = {
@@ -476,7 +480,7 @@ static void sifive_u_machine_init(MachineState *machine)
 #endif
 0x00028067,/* jr t0 */
 start_addr,/* start: .dword */
-0x,
+start_addr_hi32,
 fdt_load_addr, /* fdt_laddr: .dword */
 0x,
/* fw_dyn: */
-- 
2.27.0

[PULL 04/15] RISC-V: Copy the fdt in dram instead of ROM

[Alistair Francis]

From: Atish Patra 

Currently, the fdt is copied to the ROM after the reset vector. The firmware
has to copy it to DRAM. Instead of this, directly copy the device tree to a
pre-computed dram address. The device tree load address should be as far as
possible from kernel and initrd images. That's why it is kept at the end of
the DRAM or 4GB whichever is lesser.

Signed-off-by: Atish Patra 
Reviewed-by: Alistair Francis 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Message-Id: <20200701183949.398134-3-atish.pa...@wdc.com>
Signed-off-by: Alistair Francis 
---
 include/hw/riscv/boot.h |  4 +++-
 hw/riscv/boot.c | 53 +
 hw/riscv/sifive_u.c | 28 ++
 hw/riscv/spike.c|  7 +-
 hw/riscv/virt.c |  7 +-
 5 files changed, 66 insertions(+), 33 deletions(-)

diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 3e9759c89a..35b6ddf710 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -35,7 +35,9 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
symbol_fn_t sym_cb);
 hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
  uint64_t kernel_entry, hwaddr *start);
+uint32_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
 void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
-   hwaddr rom_size, void *fdt);
+   hwaddr rom_size,
+   uint32_t fdt_load_addr, void *fdt);
 
 #endif /* RISCV_BOOT_H */
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 3df802380a..c62f545f15 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -159,45 +159,68 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t 
mem_size,
 return *start + size;
 }
 
+uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
+{
+uint32_t temp, fdt_addr;
+hwaddr dram_end = dram_base + mem_size;
+int fdtsize = fdt_totalsize(fdt);
+
+if (fdtsize <= 0) {
+error_report("invalid device-tree");
+exit(1);
+}
+
+/*
+ * We should put fdt as far as possible to avoid kernel/initrd overwriting
+ * its content. But it should be addressable by 32 bit system as well.
+ * Thus, put it at an aligned address that less than fdt size from end of
+ * dram or 4GB whichever is lesser.
+ */
+temp = MIN(dram_end, 4096 * MiB);
+fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
+
+fdt_pack(fdt);
+/* copy in the device tree */
+qemu_fdt_dumpdtb(fdt, fdtsize);
+
+rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
+  _space_memory);
+
+return fdt_addr;
+}
+
 void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
-   hwaddr rom_size, void *fdt)
+   hwaddr rom_size,
+   uint32_t fdt_load_addr, void *fdt)
 {
 int i;
 
 /* reset vector */
-uint32_t reset_vec[8] = {
+uint32_t reset_vec[10] = {
 0x0297,  /* 1:  auipc  t0, %pcrel_hi(dtb) */
-0x02028593,  /* addi   a1, t0, %pcrel_lo(1b) */
 0xf1402573,  /* csrr   a0, mhartid  */
 #if defined(TARGET_RISCV32)
+0x0202a583,  /* lw a1, 32(t0) */
 0x0182a283,  /* lw t0, 24(t0) */
 #elif defined(TARGET_RISCV64)
+0x0202b583,  /* ld a1, 32(t0) */
 0x0182b283,  /* ld t0, 24(t0) */
 #endif
 0x00028067,  /* jr t0 */
 0x,
 start_addr,  /* start: .dword */
+0x,
+fdt_load_addr,   /* fdt_laddr: .dword */
 0x,
  /* dtb: */
 };
 
 /* copy in the reset vector in little_endian byte order */
-for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
+for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
 reset_vec[i] = cpu_to_le32(reset_vec[i]);
 }
 rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
   rom_base, _space_memory);
 
-/* copy in the device tree */
-if (fdt_pack(fdt) || fdt_totalsize(fdt) >
-rom_size - sizeof(reset_vec)) {
-error_report("not enough space to store device-tree");
-exit(1);
-}
-qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
-rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
-   rom_base + sizeof(reset_vec),
-   _space_memory);
-
 return;
 }
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 0695c93d2c..39923209f4 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -379,6 +379,7 @@ static void sifive_u_machine_init(MachineState *machine)
 

Re: [PATCH v4 06/11] Update PowerPC AT_HWCAP2 definition

[David Gibson]

On Mon, Jul 13, 2020 at 02:20:20PM -0500, Lijun Pan wrote:
> 
> 
> > On Jul 13, 2020, at 12:14 AM, David Gibson  
> > wrote:
> > 
> > On Wed, Jul 01, 2020 at 06:43:41PM -0500, Lijun Pan wrote:
> >> Add PPC2_FEATURE2_ARCH_3_10 to the PowerPC AT_HWCAP2 definitions.
> >> 
> >> Signed-off-by: Lijun Pan 
> >> ---
> >> v4: add missing changes, and split to 5/11, 6/11, 7/11
> >> v3: use tcg_gen_gvec_mul()
> >> v2: fix coding style
> >>use Power ISA 3.1 flag
> >> 
> >> include/elf.h | 1 +
> >> 1 file changed, 1 insertion(+)
> >> 
> >> diff --git a/include/elf.h b/include/elf.h
> >> index 8fbfe60e09..1858b95acf 100644
> >> --- a/include/elf.h
> >> +++ b/include/elf.h
> >> @@ -554,6 +554,7 @@ typedef struct {
> >> #define PPC_FEATURE2_HTM_NOSC   0x0100
> >> #define PPC_FEATURE2_ARCH_3_00  0x0080
> >> #define PPC_FEATURE2_HAS_IEEE1280x0040
> >> +#define PPC_FEATURE2_ARCH_3_10  0x0020
> >> 
> >> /* Bits present in AT_HWCAP for Sparc.  */
> > 
> > 
> > Um.. in the corresponding #defines in the kernel 0x0020 is given
> > to PPC_FEATURE2_DARN, and several more bits are allocated past that
> > point.
> 
> Then what do you recommend to use?

This is part of exposed userland ABI, so it needs to be standardized
at least semi-formally.  I'm not actually sure who allocates these in
general.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

device compatibility interface for live migration with assigned devices

[Yan Zhao]

hi folks,
we are defining a device migration compatibility interface that helps upper
layer stack like openstack/ovirt/libvirt to check if two devices are
live migration compatible.
The "devices" here could be MDEVs, physical devices, or hybrid of the two.
e.g. we could use it to check whether
- a src MDEV can migrate to a target MDEV,
- a src VF in SRIOV can migrate to a target VF in SRIOV,
- a src MDEV can migration to a target VF in SRIOV.
  (e.g. SIOV/SRIOV backward compatibility case)

The upper layer stack could use this interface as the last step to check
if one device is able to migrate to another device before triggering a real
live migration procedure.
we are not sure if this interface is of value or help to you. please don't
hesitate to drop your valuable comments.


(1) interface definition
The interface is defined in below way:

 __userspace
  /\  \
 / \write
/ read  \
   /__   ___\|/_
  | migration_version | | migration_version |-->check migration
  - -   compatibility
 device Adevice B


a device attribute named migration_version is defined under each device's
sysfs node. e.g. 
(/sys/bus/pci/devices/\:00\:02.0/$mdev_UUID/migration_version).
userspace tools read the migration_version as a string from the source device,
and write it to the migration_version sysfs attribute in the target device.

The userspace should treat ANY of below conditions as two devices not 
compatible:
- any one of the two devices does not have a migration_version attribute
- error when reading from migration_version attribute of one device
- error when writing migration_version string of one device to
  migration_version attribute of the other device

The string read from migration_version attribute is defined by device vendor
driver and is completely opaque to the userspace.
for a Intel vGPU, string format can be defined like
"parent device PCI ID" + "version of gvt driver" + "mdev type" + "aggregator 
count".

for an NVMe VF connecting to a remote storage. it could be
"PCI ID" + "driver version" + "configured remote storage URL"

for a QAT VF, it may be
"PCI ID" + "driver version" + "supported encryption set".

(to avoid namespace confliction from each vendor, we may prefix a driver name to
each migration_version string. e.g. i915-v1-8086-591d-i915-GVTg_V5_8-1)


(2) backgrounds

The reason we hope the migration_version string is opaque to the userspace
is that it is hard to generalize standard comparing fields and comparing
methods for different devices from different vendors.
Though userspace now could still do a simple string compare to check if
two devices are compatible, and result should also be right, it's still
too limited as it excludes the possible candidate whose migration_version
string fails to be equal.
e.g. an MDEV with mdev_type_1, aggregator count 3 is probably compatible
with another MDEV with mdev_type_3, aggregator count 1, even their
migration_version strings are not equal.
(assumed mdev_type_3 is of 3 times equal resources of mdev_type_1).

besides that, driver version + configured resources are all elements demanding
to take into account.

So, we hope leaving the freedom to vendor driver and let it make the final 
decision
in a simple reading from source side and writing for test in the target side 
way.


we then think the device compatibility issues for live migration with assigned
devices can be divided into two steps:
a. management tools filter out possible migration target devices.
   Tags could be created according to info from product specification.
   we think openstack/ovirt may have vendor proprietary components to create
   those customized tags for each product from each vendor.
   e.g.
   for Intel vGPU, with a vGPU(a MDEV device) in source side, the tags to
   search target vGPU are like:
   a tag for compatible parent PCI IDs,
   a tag for a range of gvt driver versions,
   a tag for a range of mdev type + aggregator count

   for NVMe VF, the tags to search target VF may be like:
   a tag for compatible PCI IDs,
   a tag for a range of driver versions,
   a tag for URL of configured remote storage.

b. with the output from step a, openstack/ovirt/libvirt could use our proposed
   device migration compatibility interface to make sure the two devices are
   indeed live migration compatible before launching the real live migration
   process to start stream copying, src device stopping and target device
   resuming.
   It is supposed that this step would not bring any performance penalty as
   -in kernel it's just a simple string decoding and comparing
   -in openstack/ovirt, it could be done by extending current function
check_can_live_migrate_destination, along side claiming target resources.[1]


[1] 

Re: cve patch wanted

[Michael Roth]

Quoting Philippe Mathieu-Daudé (2020-07-13 03:16:37)
> Hi,
> 
> On 7/11/20 2:28 PM, 林奕帆 wrote:
> > Hello
> >    I am a student from Fudan University in China. I am doing research on
> > CVE patch recently. But i can not find the PATCH COMMIT of
> > CVE-2019-12247 cve-2019-12155 cve-2019-6778.Can you give me the commit
> > fix this cve?
> 
> * CVE-2019-12247
> 
> I don't know about this one, maybe related to CVE-2018-12617 fixed
> by commit 1329651fb4 ("qga: Restrict guest-file-read count to 48 MB")
> Cc'ing Michael for CVE-2019-12247.

For CVE-2019-12247 is was determined the existing limits for input to
QEMU's QMP parser make it non-exploitable:

  https://bugzilla.redhat.com/show_bug.cgi?id=1712834

A patch to enforce/document some set limits rather than relying on
parser error messages (like what was done with 1329651fb4 for CVE-2018-12617)
might be nice, but it doesn't appear to be a security risk.

> 
> * CVE-2019-12155
> 
> I don't have access to the information (still marked 'private'
> one year after), but I *guess* it has been fixed by commit
> d52680fc93 ("qxl: check release info object").
> Cc'ing Gerd and Prasad.
> 
> * CVE-2019-6778
> 
> This one is in SLiRP, Cc'ing Samuel and Marc-André.
> 

Re: [PATCH] hw/net/can: Add missing fallthrough statements

[Pavel Pisa]

Hello Laurent and others,

On Monday 06 of July 2020 18:35:50 Laurent Vivier wrote:
> Le 30/06/2020 à 09:55, Thomas Huth a écrit :
> > Add fallthrough annotations to be able to compile the code without
> > warnings when using -Wimplicit-fallthrough in our CFLAGS. Looking
> > at the code, it seems like the fallthrough is indeed intended here,
> > so the comments should be appropriate.
> >
> > Signed-off-by: Thomas Huth 
> > ---
> >  hw/net/can/can_sja1000.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> > index ea915a023a..299932998a 100644
> > --- a/hw/net/can/can_sja1000.c
> > +++ b/hw/net/can/can_sja1000.c
> > @@ -523,6 +523,7 @@ void can_sja_mem_write(CanSJA1000State *s, hwaddr
> > addr, uint64_t val, break;
> >  case 16: /* RX frame information addr16-28. */
> >  s->status_pel |= (1 << 5); /* Set transmit status. */
> > +/* fallthrough */
> >  case 17 ... 28:
> >  if (s->mode & 0x01) { /* Reset mode */
> >  if (addr < 24) {
> > @@ -620,6 +621,7 @@ void can_sja_mem_write(CanSJA1000State *s, hwaddr
> > addr, uint64_t val, break;
> >  case 10:
> >  s->status_bas |= (1 << 5); /* Set transmit status. */
> > +/* fallthrough */
> >  case 11 ... 19:
> >  if ((s->control & 0x01) == 0) { /* Operation mode */
> >  s->tx_buff[addr - 10] = val; /* Store to TX buffer
> > directly. */
>
> cc: Pavel Pisa 
>
> Reviewed-by: Laurent Vivier 

The fallthrough is intentional for sure but I have gone
through datasheet and checked why the status bit is set
there and my conclusion is that to mimic real HW the status
bit should not be set there. In the fact, it should be set
and immediately (in a future delayed) reset after SJA_CMR
transmit request write. This would mimic real hardware
more closely. May it be I send patch in future when more
of our developed CAN support is added to QEMU. The status
bit behavior has no influence on actual Linux SJA1000 driver
anyway.

So for now, I confirm that adding  /* fallthrough */ is correct
step forward.

Reviewed-by: Pavel Pisa 

By the way, we have prepared CAN FD support for QEMU,
the CAN core update and device model to emulate
our open-source/design/hardware CTU CAN FD IP core

  https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core

QEMU emulation

  https://gitlab.fel.cvut.cz/canbus/qemu-canbus/-/commits/charvj10-canfd

I hope to find time to add patch to document CAN support to CAN FD
extension and send whole series this week. Stay tuned, please.

Best wishes,
 
Pavel

-- 

Pavel Pisa
phone:  +420 603531357
e-mail: p...@cmp.felk.cvut.cz
Department of Control Engineering FEE CVUT
Karlovo namesti 13, 121 35, Prague 2
university: http://dce.fel.cvut.cz/
personal:   http://cmp.felk.cvut.cz/~pisa
projects:   https://www.openhub.net/accounts/ppisa
CAN related:http://canbus.pages.fel.cvut.cz/

Re: [PATCH v2 10/11] plugins: add API to return a name for a IO device

[Richard Henderson]

On 7/13/20 1:04 PM, Alex Bennée wrote:
> This may well end up being anonymous but it should always be unique.
> 
> Signed-off-by: Alex Bennée 
> [r-b provisional given change to g_intern_string]
> Reviewed-by: Clement Deschamps 
> Reviewed-by: Emilio G. Cota 
> 
> ---
> v3
>   - return a non-freeable const g_intern_string()
>   - checkpatch cleanups
> ---
>  include/qemu/qemu-plugin.h |  6 ++
>  plugins/api.c  | 20 
>  2 files changed, 26 insertions(+)
> 
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index bab8b0d4b3..c98c18d6b0 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -335,6 +335,12 @@ struct qemu_plugin_hwaddr 
> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>  bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
>  uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr 
> *haddr);
>  
> +/*
> + * Returns a string representing the device. The string is valid for
> + * the lifetime of the plugin.
> + */
> +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr 
> *h);
> +
>  typedef void
>  (*qemu_plugin_vcpu_mem_cb_t)(unsigned int vcpu_index,
>   qemu_plugin_meminfo_t info, uint64_t vaddr,
> diff --git a/plugins/api.c b/plugins/api.c
> index bbdc5a4eb4..4304e63f0c 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -303,6 +303,26 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct 
> qemu_plugin_hwaddr *haddr
>  return 0;
>  }
>  
> +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr 
> *h)
> +{
> +#ifdef CONFIG_SOFTMMU
> +if (h && h->is_io) {
> +MemoryRegionSection *mrs = h->v.io.section;
> +if (!mrs->mr->name) {
> +unsigned long maddr = 0x & (uintptr_t) mrs->mr;
> +g_autofree char *temp = g_strdup_printf("anon%08lx", maddr);
> +return g_intern_string(temp);
> +} else {
> +return g_intern_string(mrs->mr->name);
> +}
> +} else {
> +return g_intern_string("RAM");

g_intern_static_string.

> +}
> +#else
> +return g_intern_string("Invalid");

Likewise.

Otherwise,
Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 08/11] docs/devel: fix grammar in multi-thread-tcg

[Richard Henderson]

On 7/13/20 1:04 PM, Alex Bennée wrote:
> Review comment came just too late ;-)
> 
> Signed-off-by: Alex Bennée 
> ---
>  docs/devel/multi-thread-tcg.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 06/11] cputlb: ensure we save the IOTLB data in case of reset

[Richard Henderson]

On 7/13/20 1:04 PM, Alex Bennée wrote:
> Any write to a device might cause a re-arrangement of memory
> triggering a TLB flush and potential re-size of the TLB invalidating
> previous entries. This would cause users of qemu_plugin_get_hwaddr()
> to see the warning:
> 
>   invalid use of qemu_plugin_get_hwaddr
> 
> because of the failed tlb_lookup which should always succeed. To
> prevent this we save the IOTLB data in case it is later needed by a
> plugin doing a lookup.
> 
> Signed-off-by: Alex Bennée 

Reviewed-by: Richard Henderson 

r~

Re: [RFC PATCH 5/8] fpu/softfloat: define brain floating-point types

[Richard Henderson]

On 7/13/20 1:22 PM, LIU Zhiwei wrote:
> Should we just make bfloat16 different or remove all other references?

If you have time to do a global remove, I would be grateful.  Otherwise, let's
just make bfloat16 different.


>> The word "brain" is better translated as "neural-network" in english.
> Do you mean the comment here should be
> 
> +/*
> + * Software neural-network floating-point types
> + */

Yes, thanks.


r~

[PATCH v10 01/10] qcow2: Fix capitalization of header extension constant.

[Andrey Shinkevich]

Make the capitalization of the hexadecimal numbers consistent for the
QCOW2 header extension constants in docs/interop/qcow2.txt.

Suggested-by: Eric Blake 
Signed-off-by: Andrey Shinkevich 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2.c  | 2 +-
 docs/interop/qcow2.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ea33673..6c6bee3 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -66,7 +66,7 @@ typedef struct {
 } QEMU_PACKED QCowExtension;
 
 #define  QCOW2_EXT_MAGIC_END 0
-#define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
+#define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xe2792aca
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
 #define  QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77
 #define  QCOW2_EXT_MAGIC_BITMAPS 0x23852875
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index cb72346..f072e27 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -231,7 +231,7 @@ be stored. Each extension has a structure like the 
following:
 
 Byte  0 -  3:   Header extension type:
 0x - End of the header extension area
-0xE2792ACA - Backing file format name string
+0xe2792aca - Backing file format name string
 0x6803f857 - Feature name table
 0x23852875 - Bitmaps extension
 0x0537be77 - Full disk encryption header pointer
-- 
1.8.3.1

[PATCH v10 07/10] qcow2_format.py: Dump bitmap table serialized entries

[Andrey Shinkevich]

Add bitmap table information to the QCOW2 metadata dump.

Bitmap name   bitmap-1
...
Bitmap table   typeoffset   size
0  serialized  4718592  65536
1  serialized  4294967296   65536
2  serialized  5348033147437056 65536
3  serialized  1379227385882214465536
4  serialized  4718592  65536
5  serialized  4294967296   65536
6  serialized  4503608217305088 65536
7  serialized  1407374883553280065536

Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index d9c8513..2c78d46 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -175,14 +175,56 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 entry_raw_size = self.bitmap_dir_entry_raw_size()
 padding = ((entry_raw_size + 7) & ~7) - entry_raw_size
 fd.seek(padding, 1)
+position = fd.tell()
+self.read_bitmap_table(fd)
+fd.seek(position)
 
 def bitmap_dir_entry_raw_size(self):
 return struct.calcsize(self.fmt) + self.name_size + \
 self.extra_data_size
 
+def read_bitmap_table(self, fd):
+fd.seek(self.bitmap_table_offset)
+table_size = self.bitmap_table_size * 8 * 8
+table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
+self.bitmap_table = Qcow2BitmapTable(raw_table=table,
+ cluster_size=self.cluster_size)
+
 def dump(self):
 print(f'{"Bitmap name":<25} {self.name}')
 super(Qcow2BitmapDirEntry, self).dump()
+self.bitmap_table.dump()
+
+
+class Qcow2BitmapTableEntry:
+
+BME_TABLE_ENTRY_OFFSET_MASK = 0x00fffe00
+BME_TABLE_ENTRY_FLAG_ALL_ONES = 1
+
+def __init__(self, entry):
+self.offset = entry & self.BME_TABLE_ENTRY_OFFSET_MASK
+if self.offset:
+self.type = 'serialized'
+elif entry & self.BME_TABLE_ENTRY_FLAG_ALL_ONES:
+self.type = 'all-ones'
+else:
+self.type = 'all-zeroes'
+
+
+class Qcow2BitmapTable:
+
+def __init__(self, raw_table, cluster_size):
+self.entries = []
+self.cluster_size = cluster_size
+for entry in raw_table:
+self.entries.append(Qcow2BitmapTableEntry(entry))
+
+def dump(self):
+size = self.cluster_size
+bitmap_table = enumerate(self.entries)
+print(f'{"Bitmap table":<14} {"type":<15} {"offset":<24} {"size"}')
+for i, entry in bitmap_table:
+print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
 
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
-- 
1.8.3.1

[PATCH v10 05/10] qcow2_format.py: Dump bitmap directory information

[Andrey Shinkevich]

Read and dump entries from the bitmap directory of QCOW2 image.

Header extension:
magic 0x23852875 (Bitmaps)
...
Bitmap name   bitmap-1
bitmap_table_offset   0xf
bitmap_table_size 1
flags 0x2 (['auto'])
type  1
granularity_bits  16
name_size 8
extra_data_size   0

Suggested-by: Kevin Wolf 
Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index e77c831..f0db9f4 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -134,6 +134,53 @@ class Qcow2BitmapExt(Qcow2Struct):
 pad = (struct.calcsize(self.fmt) + 7) & ~7
 if pad:
 fd.seek(pad, 1)
+position = fd.tell()
+self.read_bitmap_directory(fd)
+fd.seek(position)
+
+def read_bitmap_directory(self, fd):
+fd.seek(self.bitmap_directory_offset)
+self.bitmap_directory = \
+[Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+
+def dump(self):
+super().dump()
+for entry in self.bitmap_directory:
+print()
+entry.dump()
+
+
+class Qcow2BitmapDirEntry(Qcow2Struct):
+
+fields = (
+('u64', '{:#x}', 'bitmap_table_offset'),
+('u32', '{}', 'bitmap_table_size'),
+('u32', BitmapFlags, 'flags'),
+('u8',  '{}', 'type'),
+('u8',  '{}', 'granularity_bits'),
+('u16', '{}', 'name_size'),
+('u32', '{}', 'extra_data_size')
+)
+
+def __init__(self, fd):
+super().__init__(fd=fd)
+# Seek relative to the current position in the file
+fd.seek(self.extra_data_size, 1)
+bitmap_name = fd.read(self.name_size)
+self.name = bitmap_name.decode('ascii')
+# Move position to the end of the entry in the directory
+entry_raw_size = self.bitmap_dir_entry_raw_size()
+padding = ((entry_raw_size + 7) & ~7) - entry_raw_size
+fd.seek(padding, 1)
+
+def bitmap_dir_entry_raw_size(self):
+return struct.calcsize(self.fmt) + self.name_size + \
+self.extra_data_size
+
+def dump(self):
+print(f'{"Bitmap name":<25} {self.name}')
+super(Qcow2BitmapDirEntry, self).dump()
+
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
-- 
1.8.3.1

[PATCH v10 03/10] qcow2_format.py: change Qcow2BitmapExt initialization method

[Andrey Shinkevich]

There are two ways to initialize a class derived from Qcow2Struct:
1. Pass a block of binary data to the constructor.
2. Pass the file descriptor to allow reading the file from constructor.
Let's change the Qcow2BitmapExt initialization method from 1 to 2 to
support a scattered reading in the initialization chain.
The implementation comes with the patch that follows.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 33 +++--
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 2f3681b..cbaffc4 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -113,6 +113,11 @@ class Qcow2BitmapExt(Qcow2Struct):
 ('u64', '{:#x}', 'bitmap_directory_offset')
 )
 
+def __init__(self, fd):
+super().__init__(fd=fd)
+pad = (struct.calcsize(self.fmt) + 7) & ~7
+if pad:
+fd.seek(pad, 1)
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
@@ -161,21 +166,21 @@ class QcowHeaderExtension(Qcow2Struct):
 else:
 assert all(v is None for v in (magic, length, data))
 super().__init__(fd=fd)
-padded = (self.length + 7) & ~7
-self.data = fd.read(padded)
-assert self.data is not None
-
-data_str = self.data[:self.length]
-if all(c in string.printable.encode('ascii') for c in data_str):
-data_str = f"'{ data_str.decode('ascii') }'"
-else:
-data_str = ''
-self.data_str = data_str
+if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
+self.obj = Qcow2BitmapExt(fd=fd)
+else:
+padded = (self.length + 7) & ~7
+self.data = fd.read(padded)
+assert self.data is not None
+self.obj = None
+data_str = self.data[:self.length]
+if all(c in string.printable.encode(
+'ascii') for c in data_str):
+data_str = f"'{ data_str.decode('ascii') }'"
+else:
+data_str = ''
+self.data_str = data_str
 
-if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-self.obj = Qcow2BitmapExt(data=self.data)
-else:
-self.obj = None
 
 def dump(self):
 super().dump()
-- 
1.8.3.1

[PATCH v10 02/10] qcow2_format.py: make printable data an extension class member

[Andrey Shinkevich]

Let us differ binary data type from string one for the extension data
variable and keep the string as the QcowHeaderExtension class member.

Signed-off-by: Andrey Shinkevich 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/qcow2_format.py | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index cc432e7..2f3681b 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -165,6 +165,13 @@ class QcowHeaderExtension(Qcow2Struct):
 self.data = fd.read(padded)
 assert self.data is not None
 
+data_str = self.data[:self.length]
+if all(c in string.printable.encode('ascii') for c in data_str):
+data_str = f"'{ data_str.decode('ascii') }'"
+else:
+data_str = ''
+self.data_str = data_str
+
 if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
 self.obj = Qcow2BitmapExt(data=self.data)
 else:
@@ -174,12 +181,7 @@ class QcowHeaderExtension(Qcow2Struct):
 super().dump()
 
 if self.obj is None:
-data = self.data[:self.length]
-if all(c in string.printable.encode('ascii') for c in data):
-data = f"'{ data.decode('ascii') }'"
-else:
-data = ''
-print(f'{"data":<25} {data}')
+print(f'{"data":<25} {self.data_str}')
 else:
 self.obj.dump()
 
-- 
1.8.3.1

[PATCH v10 04/10] qcow2_format.py: dump bitmap flags in human readable way.

[Andrey Shinkevich]

Introduce the class BitmapFlags that parses a bitmap flags mask.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 16 
 1 file changed, 16 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index cbaffc4..e77c831 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -40,6 +40,22 @@ class Flags64(Qcow2Field):
 return str(bits)
 
 
+class BitmapFlags(Qcow2Field):
+
+flags = {
+0x1: 'in-use',
+0x2: 'auto'
+}
+
+def __str__(self):
+bits = []
+for bit in range(64):
+flag = self.value & (1 << bit)
+if flag:
+bits.append(self.flags.get(flag, f'bit-{bit}'))
+return f'{self.value:#x} ({bits})'
+
+
 class Enum(Qcow2Field):
 
 def __str__(self):
-- 
1.8.3.1

[PATCH v10 00/10] iotests: Dump QCOW2 dirty bitmaps metadata

[Andrey Shinkevich]

Add dirty bitmap information to QCOW2 metadata dump in the qcow2_format.py.

v10:
  01: The modification of the 291.out was removed.
  02: Qcow2BitmapExt modified to take control over file cursor position while
  reading the bitmap extension data from the file (suggested by Vladimir).
  03: The format string for bitmap flags output modified
  (suggested by Vladimir).

Andrey Shinkevich (10):
  qcow2: Fix capitalization of header extension constant.
  qcow2_format.py: make printable data an extension class member
  qcow2_format.py: change Qcow2BitmapExt initialization method
  qcow2_format.py: dump bitmap flags in human readable way.
  qcow2_format.py: Dump bitmap directory information
  qcow2_format.py: pass cluster size to substructures
  qcow2_format.py: Dump bitmap table serialized entries
  qcow2.py: Introduce '-j' key to dump in JSON format
  qcow2_format.py: collect fields to dump in JSON format
  qcow2_format.py: support dumping metadata in JSON format

 block/qcow2.c  |   2 +-
 docs/interop/qcow2.txt |   2 +-
 tests/qemu-iotests/qcow2.py|  19 +++-
 tests/qemu-iotests/qcow2_format.py | 223 +
 5 files changed, 218 insertions(+), 28 deletions(-)

-- 
1.8.3.1

[PATCH v10 06/10] qcow2_format.py: pass cluster size to substructures

[Andrey Shinkevich]

The cluster size of an image is the QcowHeader class member and may be
obtained by dependent extension structures such as Qcow2BitmapExt for
further bitmap table details print.

Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index f0db9f4..d9c8513 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -129,19 +129,21 @@ class Qcow2BitmapExt(Qcow2Struct):
 ('u64', '{:#x}', 'bitmap_directory_offset')
 )
 
-def __init__(self, fd):
+def __init__(self, fd, cluster_size):
 super().__init__(fd=fd)
 pad = (struct.calcsize(self.fmt) + 7) & ~7
 if pad:
 fd.seek(pad, 1)
 position = fd.tell()
+self.cluster_size = cluster_size
 self.read_bitmap_directory(fd)
 fd.seek(position)
 
 def read_bitmap_directory(self, fd):
 fd.seek(self.bitmap_directory_offset)
 self.bitmap_directory = \
-[Qcow2BitmapDirEntry(fd) for _ in range(self.nb_bitmaps)]
+[Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
+ for _ in range(self.nb_bitmaps)]
 
 def dump(self):
 super().dump()
@@ -162,8 +164,9 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 ('u32', '{}', 'extra_data_size')
 )
 
-def __init__(self, fd):
+def __init__(self, fd, cluster_size):
 super().__init__(fd=fd)
+self.cluster_size = cluster_size
 # Seek relative to the current position in the file
 fd.seek(self.extra_data_size, 1)
 bitmap_name = fd.read(self.name_size)
@@ -203,11 +206,13 @@ class QcowHeaderExtension(Qcow2Struct):
 # then padding to next multiply of 8
 )
 
-def __init__(self, magic=None, length=None, data=None, fd=None):
+def __init__(self, magic=None, length=None, data=None, fd=None,
+ cluster_size=None):
 """
 Support both loading from fd and creation from user data.
 For fd-based creation current position in a file will be used to read
 the data.
+The cluster_size value may be obtained by dependent structures.
 
 This should be somehow refactored and functionality should be moved to
 superclass (to allow creation of any qcow2 struct), but then, fields
@@ -230,7 +235,7 @@ class QcowHeaderExtension(Qcow2Struct):
 assert all(v is None for v in (magic, length, data))
 super().__init__(fd=fd)
 if self.magic == QCOW2_EXT_MAGIC_BITMAPS:
-self.obj = Qcow2BitmapExt(fd=fd)
+self.obj = Qcow2BitmapExt(fd=fd, cluster_size=cluster_size)
 else:
 padded = (self.length + 7) & ~7
 self.data = fd.read(padded)
@@ -244,7 +249,6 @@ class QcowHeaderExtension(Qcow2Struct):
 data_str = ''
 self.data_str = data_str
 
-
 def dump(self):
 super().dump()
 
@@ -316,7 +320,7 @@ class QcowHeader(Qcow2Struct):
 end = self.cluster_size
 
 while fd.tell() < end:
-ext = QcowHeaderExtension(fd=fd)
+ext = QcowHeaderExtension(fd=fd, cluster_size=self.cluster_size)
 if ext.magic == 0:
 break
 else:
-- 
1.8.3.1

[PATCH v10 08/10] qcow2.py: Introduce '-j' key to dump in JSON format

[Andrey Shinkevich]

Add the command key to the qcow2.py arguments list to dump QCOW2
metadata in JSON format. Here is the suggested way to do that. The
implementation of the dump in JSON format is in the patch that follows.

Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2.py| 19 +++
 tests/qemu-iotests/qcow2_format.py | 16 
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 0910e6a..7402279 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -26,16 +26,19 @@ from qcow2_format import (
 )
 
 
+dump_json = False
+
+
 def cmd_dump_header(fd):
 h = QcowHeader(fd)
-h.dump()
+h.dump(dump_json)
 print()
-h.dump_extensions()
+h.dump_extensions(dump_json)
 
 
 def cmd_dump_header_exts(fd):
 h = QcowHeader(fd)
-h.dump_extensions()
+h.dump_extensions(dump_json)
 
 
 def cmd_set_header(fd, name, value):
@@ -134,6 +137,11 @@ cmds = [
 
 
 def main(filename, cmd, args):
+global dump_json
+dump_json = '-j' in sys.argv
+if dump_json:
+sys.argv.remove('-j')
+args.remove('-j')
 fd = open(filename, "r+b")
 try:
 for name, handler, num_args, desc in cmds:
@@ -151,11 +159,14 @@ def main(filename, cmd, args):
 
 
 def usage():
-print("Usage: %s   [, ...]" % sys.argv[0])
+print("Usage: %s   [, ...] [, ...]" % sys.argv[0])
 print("")
 print("Supported commands:")
 for name, handler, num_args, desc in cmds:
 print("%-20s - %s" % (name, desc))
+print("")
+print("Supported keys:")
+print("%-20s - %s" % ('-j', 'Dump in JSON format'))
 
 
 if __name__ == '__main__':
diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 2c78d46..e0e14b5 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -109,7 +109,7 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
 self.__dict__ = dict((field[2], values[i])
  for i, field in enumerate(self.fields))
 
-def dump(self):
+def dump(self, dump_json=None):
 for f in self.fields:
 value = self.__dict__[f[2]]
 if isinstance(f[1], str):
@@ -145,8 +145,8 @@ class Qcow2BitmapExt(Qcow2Struct):
 [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
  for _ in range(self.nb_bitmaps)]
 
-def dump(self):
-super().dump()
+def dump(self, dump_json=None):
+super().dump(dump_json)
 for entry in self.bitmap_directory:
 print()
 entry.dump()
@@ -190,7 +190,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 self.bitmap_table = Qcow2BitmapTable(raw_table=table,
  cluster_size=self.cluster_size)
 
-def dump(self):
+def dump(self, dump_json=None):
 print(f'{"Bitmap name":<25} {self.name}')
 super(Qcow2BitmapDirEntry, self).dump()
 self.bitmap_table.dump()
@@ -291,13 +291,13 @@ class QcowHeaderExtension(Qcow2Struct):
 data_str = ''
 self.data_str = data_str
 
-def dump(self):
+def dump(self, dump_json=None):
 super().dump()
 
 if self.obj is None:
 print(f'{"data":<25} {self.data_str}')
 else:
-self.obj.dump()
+self.obj.dump(dump_json)
 
 @classmethod
 def create(cls, magic, data):
@@ -396,8 +396,8 @@ class QcowHeader(Qcow2Struct):
 buf = buf[0:header_bytes-1]
 fd.write(buf)
 
-def dump_extensions(self):
+def dump_extensions(self, dump_json=None):
 for ex in self.extensions:
 print('Header extension:')
-ex.dump()
+ex.dump(dump_json)
 print()
-- 
1.8.3.1

[PATCH v10 09/10] qcow2_format.py: collect fields to dump in JSON format

[Andrey Shinkevich]

As __dict__ is being extended with class members we do not want to
print, make a light copy of the initial __dict__ and extend the copy
by adding lists we have to print in the JSON output.

Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index e0e14b5..83c3482 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -109,6 +109,8 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
 self.__dict__ = dict((field[2], values[i])
  for i, field in enumerate(self.fields))
 
+self.fields_dict = self.__dict__.copy()
+
 def dump(self, dump_json=None):
 for f in self.fields:
 value = self.__dict__[f[2]]
@@ -144,6 +146,7 @@ class Qcow2BitmapExt(Qcow2Struct):
 self.bitmap_directory = \
 [Qcow2BitmapDirEntry(fd, cluster_size=self.cluster_size)
  for _ in range(self.nb_bitmaps)]
+self.fields_dict.update(bitmap_directory=self.bitmap_directory)
 
 def dump(self, dump_json=None):
 super().dump(dump_json)
@@ -189,6 +192,7 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 table = [e[0] for e in struct.iter_unpack('>Q', fd.read(table_size))]
 self.bitmap_table = Qcow2BitmapTable(raw_table=table,
  cluster_size=self.cluster_size)
+self.fields_dict.update(bitmap_table=self.bitmap_table)
 
 def dump(self, dump_json=None):
 print(f'{"Bitmap name":<25} {self.name}')
-- 
1.8.3.1

[PATCH v10 10/10] qcow2_format.py: support dumping metadata in JSON format

[Andrey Shinkevich]

Implementation of dumping QCOW2 image metadata.
The sample output:
{
"Header_extensions": [
{
"name": "Feature table",
"magic": 1745090647,
"length": 192,
"data_str": ""
},
{
"name": "Bitmaps",
"magic": 595929205,
"length": 24,
"data": {
"nb_bitmaps": 2,
"reserved32": 0,
"bitmap_directory_size": 64,
"bitmap_directory_offset": 1048576,
"bitmap_directory": [
{
"name": "bitmap-1",
"bitmap_table_offset": 589824,
"bitmap_table_size": 1,
"flags": 2,
"type": 1,
"granularity_bits": 15,
"name_size": 8,
"extra_data_size": 0,
"bitmap_table": {
"entries": [
{
"type": "serialized",
"offset": 655360
},
...

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Andrey Shinkevich 
---
 tests/qemu-iotests/qcow2_format.py | 59 ++
 1 file changed, 59 insertions(+)

diff --git a/tests/qemu-iotests/qcow2_format.py 
b/tests/qemu-iotests/qcow2_format.py
index 83c3482..a263858 100644
--- a/tests/qemu-iotests/qcow2_format.py
+++ b/tests/qemu-iotests/qcow2_format.py
@@ -19,6 +19,15 @@
 
 import struct
 import string
+import json
+
+
+class ComplexEncoder(json.JSONEncoder):
+def default(self, obj):
+if hasattr(obj, 'get_fields_dict'):
+return obj.get_fields_dict()
+else:
+return json.JSONEncoder.default(self, obj)
 
 
 class Qcow2Field:
@@ -112,6 +121,11 @@ class Qcow2Struct(metaclass=Qcow2StructMeta):
 self.fields_dict = self.__dict__.copy()
 
 def dump(self, dump_json=None):
+if dump_json:
+print(json.dumps(self.get_fields_dict(), indent=4,
+ cls=ComplexEncoder))
+return
+
 for f in self.fields:
 value = self.__dict__[f[2]]
 if isinstance(f[1], str):
@@ -154,6 +168,9 @@ class Qcow2BitmapExt(Qcow2Struct):
 print()
 entry.dump()
 
+def get_fields_dict(self):
+return self.fields_dict
+
 
 class Qcow2BitmapDirEntry(Qcow2Struct):
 
@@ -199,6 +216,11 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
 super(Qcow2BitmapDirEntry, self).dump()
 self.bitmap_table.dump()
 
+def get_fields_dict(self):
+bmp_name = dict(name=self.name)
+bme_dict = {**bmp_name, **self.fields_dict}
+return bme_dict
+
 
 class Qcow2BitmapTableEntry:
 
@@ -214,6 +236,9 @@ class Qcow2BitmapTableEntry:
 else:
 self.type = 'all-zeroes'
 
+def get_fields_dict(self):
+return dict(type=self.type, offset=self.offset)
+
 
 class Qcow2BitmapTable:
 
@@ -230,6 +255,18 @@ class Qcow2BitmapTable:
 for i, entry in bitmap_table:
 print(f'{i:<14} {entry.type:<15} {entry.offset:<24} {size}')
 
+def get_fields_dict(self):
+return dict(entries=self.entries)
+
+
+class Qcow2HeaderExtensionsDoc:
+
+def __init__(self, extensions):
+self.extensions = extensions
+
+def get_fields_dict(self):
+return dict(Header_extensions=self.extensions)
+
 
 QCOW2_EXT_MAGIC_BITMAPS = 0x23852875
 
@@ -245,6 +282,9 @@ class QcowHeaderExtension(Qcow2Struct):
 0x44415441: 'Data file'
 }
 
+def get_fields_dict(self):
+return self.mapping.get(self.value, "")
+
 fields = (
 ('u32', Magic, 'magic'),
 ('u32', '{}', 'length')
@@ -303,6 +343,16 @@ class QcowHeaderExtension(Qcow2Struct):
 else:
 self.obj.dump(dump_json)
 
+def get_fields_dict(self):
+ext_name = dict(name=self.Magic(self.magic))
+he_dict = {**ext_name, **self.fields_dict}
+if self.obj is not None:
+he_dict.update(data=self.obj)
+else:
+he_dict.update(data_str=self.data_str)
+
+return he_dict
+
 @classmethod
 def create(cls, magic, data):
 return QcowHeaderExtension(magic, len(data), data)
@@ -401,7 +451,16 @@ class QcowHeader(Qcow2Struct):
 fd.write(buf)
 
 def dump_extensions(self, dump_json=None):
+if dump_json:
+ext_doc = Qcow2HeaderExtensionsDoc(self.extensions)
+print(json.dumps(ext_doc.get_fields_dict(), indent=4,
+ cls=ComplexEncoder))
+return
+
 for ex in self.extensions:
 print('Header extension:')
 ex.dump(dump_json)
 print()
+
+def get_fields_dict(self):
+return 

[PATCH 3/3] hw/arm/virt: Disable memory hotplug when MTE is enabled

[Richard Henderson]

When MTE is enabled, tag memory must exist for all RAM.

It might be possible to simultaneously hot plug tag memory
alongside the corresponding normal memory, but for now just
disable hotplug.

Signed-off-by: Richard Henderson 
---
 hw/arm/virt.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a7f3d442db..ecfee362a1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2194,6 +2194,11 @@ static void virt_memory_pre_plug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 return;
 }
 
+if (vms->mte) {
+error_setg(errp, "memory hotplug is not enabled: MTE is enabled");
+return;
+}
+
 if (is_nvdimm && !ms->nvdimms_state->is_enabled) {
 error_setg(errp, "nvdimm is not enabled: add 'nvdimm=on' to '-M'");
 return;
-- 
2.25.1

[PATCH 2/3] hw/arm/virt: Error for MTE enabled with KVM

[Richard Henderson]

While we expect KVM to support MTE at some future point,
it certainly won't be ready in time for qemu 5.1.

Signed-off-by: Richard Henderson 
---
 hw/arm/virt.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5866c4ce20..a7f3d442db 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1773,6 +1773,12 @@ static void machvirt_init(MachineState *machine)
 exit(1);
 }
 
+if (vms->mte && kvm_enabled()) {
+error_report("mach-virt: KVM does not support providing "
+ "MTE to the guest CPU");
+exit(1);
+}
+
 create_fdt(vms);
 
 possible_cpus = mc->possible_cpu_arch_ids(machine);
-- 
2.25.1

[PATCH for-5.1 0/3] target/arm: MTE improvements

[Richard Henderson]

Since MTE is new, and guest support for MTE is still under
development, let's disable it by default.

Peter mentioned memory hotplug on IRC as something that
would break MTE's assumptions.  By putting the enable flag
on the machine it's much easier to control.

For 5.1, we won't have kvm support, so error for that combo.


r~


Richard Henderson (3):
  hw/arm/virt: Enable MTE via a machine property
  hw/arm/virt: Error for MTE enabled with KVM
  hw/arm/virt: Disable memory hotplug when MTE is enabled

 include/hw/arm/virt.h |  1 +
 hw/arm/virt.c | 50 ++-
 target/arm/cpu.c  | 19 +---
 target/arm/cpu64.c|  5 +++--
 4 files changed, 60 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH 1/3] hw/arm/virt: Enable MTE via a machine property

[Richard Henderson]

Control this cpu feature via a machine property, much as we do
with secure=on, since both require specialized support in the
machine setup to be functional.

Default MTE to off, since this feature implies extra overhead.

Signed-off-by: Richard Henderson 
---
 include/hw/arm/virt.h |  1 +
 hw/arm/virt.c | 39 ++-
 target/arm/cpu.c  | 19 +++
 target/arm/cpu64.c|  5 +++--
 4 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 54bcf17afd..dff67e1bef 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -140,6 +140,7 @@ typedef struct {
 bool its;
 bool virt;
 bool ras;
+bool mte;
 OnOffAuto acpi;
 VirtGICType gic_version;
 VirtIOMMUType iommu;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9005dae356..5866c4ce20 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1837,12 +1837,19 @@ static void machvirt_init(MachineState *machine)
  OBJECT(secure_sysmem), _abort);
 }
 
-/*
- * The cpu adds the property if and only if MemTag is supported.
- * If it is, we must allocate the ram to back that up.
- */
-if (object_property_find(cpuobj, "tag-memory", NULL)) {
+if (vms->mte) {
+/* Create the memory region only once, but link to all cpus. */
 if (!tag_sysmem) {
+/*
+ * The property exists only if MemTag is supported.
+ * If it is, we must allocate the ram to back that up.
+ */
+if (!object_property_find(cpuobj, "tag-memory", NULL)) {
+error_report("MTE requested, but not supported "
+ "by the guest CPU");
+exit(1);
+}
+
 tag_sysmem = g_new(MemoryRegion, 1);
 memory_region_init(tag_sysmem, OBJECT(machine),
"tag-memory", UINT64_MAX / 32);
@@ -2061,6 +2068,20 @@ static void virt_set_ras(Object *obj, bool value, Error 
**errp)
 vms->ras = value;
 }
 
+static bool virt_get_mte(Object *obj, Error **errp)
+{
+VirtMachineState *vms = VIRT_MACHINE(obj);
+
+return vms->mte;
+}
+
+static void virt_set_mte(Object *obj, bool value, Error **errp)
+{
+VirtMachineState *vms = VIRT_MACHINE(obj);
+
+vms->mte = value;
+}
+
 static char *virt_get_gic_version(Object *obj, Error **errp)
 {
 VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -2481,6 +2502,14 @@ static void virt_instance_init(Object *obj)
 "Set on/off to enable/disable reporting 
host memory errors "
 "to a KVM guest using ACPI and guest 
external abort exceptions");
 
+/* MTE is disabled by default.  */
+vms->mte = false;
+object_property_add_bool(obj, "mte", virt_get_mte, virt_set_mte);
+object_property_set_description(obj, "mte",
+"Set on/off to enable/disable emulating a "
+"guest CPU which implements the ARM "
+"Memory Tagging Extension");
+
 vms->irqmap = a15irqmap;
 
 virt_flash_create(vms);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5050e1843a..111579554f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1698,6 +1698,17 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 cpu->id_pfr1 &= ~0xf000;
 }
 
+#ifndef CONFIG_USER_ONLY
+if (cpu->tag_memory == NULL && cpu_isar_feature(aa64_mte, cpu)) {
+/*
+ * Disable the MTE feature bits if we do not have tag-memory
+ * provided by the machine.
+ */
+cpu->isar.id_aa64pfr1 =
+FIELD_DP64(cpu->isar.id_aa64pfr1, ID_AA64PFR1, MTE, 0);
+}
+#endif
+
 /* MPU can be configured out of a PMSA CPU either by setting has-mpu
  * to false or by setting pmsav7-dregion to 0.
  */
@@ -1787,14 +1798,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 cpu_address_space_init(cs, ARMASIdx_TagS, "cpu-tag-memory",
cpu->secure_tag_memory);
 }
-} else if (cpu_isar_feature(aa64_mte, cpu)) {
-/*
- * Since there is no tag memory, we can't meaningfully support MTE
- * to its fullest.  To avoid problems later, when we would come to
- * use the tag memory, downgrade support to insns only.
- */
-cpu->isar.id_aa64pfr1 =
-FIELD_DP64(cpu->isar.id_aa64pfr1, ID_AA64PFR1, MTE, 1);
 }
 
 cpu_address_space_init(cs, ARMASIdx_NS, "cpu-memory", cs->memory);
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 15494002d2..dd696183df 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -646,8 +646,9 @@ static void aarch64_max_initfn(Object *obj)
 t = 

Re: [PATCH] vfio: fix use-after-free in display

[Alex Williamson]

On Mon, 13 Jul 2020 14:45:20 +0200
Gerd Hoffmann  wrote:

> Calling ramfb_display_update() might replace the DisplaySurface with the
> boot display, which in turn will free the currently active
> DisplaySurface.
> 
> So clear our DisplaySurface pinter (dpy->region.surface pointer) to (a)
   ^o

> avoid use-after-free and (b) force replacing the boot display with the
> real display when switching back.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  hw/vfio/display.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index a57a22674d62..342054193b3c 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -405,6 +405,7 @@ static void vfio_display_region_update(void *opaque)
>  if (!plane.drm_format || !plane.size) {
>  if (dpy->ramfb) {
>  ramfb_display_update(dpy->con, dpy->ramfb);
> +dpy->region.surface = NULL;
>  }
>  return;
>  }

Tricky, but I think I follow that dpy->region.surface is only ever
allocated to replace dpy->con->surface, so when ramfb_display_update()
then replaces and frees dpy->con->surface with dpy->ramfb->ds, that's
where the object point to by dpy->region.surface was freed.  Right?

If so, looks ok to me.  If you're constructing a pull request, I'll
give you an:

Acked-by: Alex Williamson 
Reviewed-by: Alex Williamson 

If you need me to send a pull, let me know.  Thanks,

Alex

Re: [PATCH v2 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes

[Peter Maydell]

On Mon, 13 Jul 2020 at 19:32, Philippe Mathieu-Daudé  wrote:
>
> QEMU allows to create SD card with unrealistic sizes. This could
> work, but some guests (at least Linux) consider sizes that are not
> a power of 2 as a firmware bug and fix the card size to the next
> power of 2.
>

> +error_setg(errp, "Invalid SD card size: %s", blk_size_str);
> +g_free(blk_size_str);
> +
> +blk_size_str = size_to_str(blk_size_aligned);
> +error_append_hint(errp,
> +  "SD card size has to be a power of 2, e.g. 
> %s.\n"
> +  "You can resize disk images with "
> +  "'qemu-img resize  '\n"
> +  "(note that this will lose data if you make 
> the "
> +  "image smaller than it currently is).\n",
> +  blk_size_str);
> +g_free(blk_size_str);

Some places that create multi-line hints with error_append_hint()
do it by calling it once per line, eg in target/arm/cpu64.c:
error_setg(errp, "cannot disable sve128");
error_append_hint(errp, "Disabling sve128 results in all "
  "vector lengths being disabled.\n");
error_append_hint(errp, "With SVE enabled, at least one "
  "vector length must be enabled.\n");

Some places don't, eg in block/vhdx-log.c:
error_append_hint(errp,  "To replay the log, run:\n"
  "qemu-img check -r all '%s'\n",
  bs->filename);

Most places terminate with a '\n', but some places don't, eg
crypto/block-luks.c:
   error_append_hint(errp, "Failed to write to keyslot %i", keyslot);
   return -1;

The documentation says
 * May be called multiple times.  The resulting hint should end with a
 * newline.

which isn't very clear -- you can call it multiple times, but
must you, if it's multiline?

I assume that "should end with a newline" means "must end
with a newline", and places like block-luks.c are bugs.

Markus, do you know what the intended API here is?

It looks like the implementation just tacks the hint
string onto the end of any existing hint string, in
which case multiple-line strings are fine and the same
behaviour as calling the function multiple times.
(I had assumed we might be accumulating an array of strings,
or requiring multiline strings to be multiple calls so we
could have the argument not need to be \n-terminated,
to match error_setg(), but both those assumptions
are obviously wrong.)

Anyway, I guess this multiline-message usage is something
we do already and it will DTRT, so

Reviewed-by: Peter Maydell 


thanks
-- PMM

Re: [PATCH v5 03/12] python/machine.py: Add _early_cleanup hook

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:40AM -0400, John Snow wrote:
> Some parts of cleanup need to occur prior to shutdown, otherwise
> shutdown might break. Move this into a suitably named method/callback.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 24 +++-
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 938c891b1d..4280aab380 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -354,16 +354,9 @@ def _launch(self):
> close_fds=False)
>  self._post_launch()
>  
> -def wait(self):
> +def _early_cleanup(self) -> None:

Inaugurating type hints "around here", huh? :)

WRT method name, up to the end of this series, this method is only about
closing the console socket, so *maybe* just name it accordingly and move
on to an abstract method name when/if the need arises?

Either way,

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH v5 04/12] python/machine.py: Perform early cleanup for wait() calls, too

[Cleber Rosa]

On Fri, Jul 10, 2020 at 01:06:41AM -0400, John Snow wrote:
> This is primarily for consistency, and is a step towards wait() and
> shutdown() sharing the same implementation so that the two cleanup paths
> cannot diverge.
> 
> Signed-off-by: John Snow 
> ---
>  python/qemu/machine.py | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [RFC PATCH 5/8] fpu/softfloat: define brain floating-point types

[LIU Zhiwei]

On 2020/7/14 3:26, Richard Henderson wrote:

On 7/12/20 4:45 PM, LIU Zhiwei wrote:

Signed-off-by: LIU Zhiwei 
---
  include/fpu/softfloat-types.h | 8 
  1 file changed, 8 insertions(+)

diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 7680193ebc..8f8fdfeecf 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -112,6 +112,14 @@ typedef struct {
  #define make_float128(high_, low_) ((float128) { .high = high_, .low = low_ })
  #define make_float128_init(high_, low_) { .high = high_, .low = low_ }
  
+/*

+ * Software brain floating-point types
+ */
+typedef uint16_t bfloat16;
+#define bfloat16_val(x) (x)
+#define make_bfloat16(x) (x)
+#define const_bfloat16(x) (x)

I do not like the val/make/const macros.  I've been meaning to get them 
everywhere.

Yes, but they have been spread to everywhere.

Should we just make bfloat16 different or remove all other references?

The word "brain" is better translated as "neural-network" in english.

Do you mean the comment here should be

+/*
+ * Software neural-network floating-point types
+ */

Zhiwei


r~

Re: [PATCH] gitlab-ci/containers: Add missing wildcard where we should look for changes

[Philippe Mathieu-Daudé]

On 7/13/20 8:22 PM, Thomas Huth wrote:
> The tests/docker/* wildcard seems to only match the files that are directly
> in the tests/docker folder - but changes to the files in the directory
> tests/docker/dockerfiles are currently ignored. Seems like we need a
> separate entry to match the files in that folder. With this wildcard added,
> the stages now get re-run successfully when something in the dockerfiles
> has been changed.
> 
> Signed-off-by: Thomas Huth 
> ---
>  .gitlab-ci.d/containers.yml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
> index f3c0ca4d61..8c89efeb6d 100644
> --- a/.gitlab-ci.d/containers.yml
> +++ b/.gitlab-ci.d/containers.yml
> @@ -24,6 +24,7 @@
>  - changes:
>- .gitlab-ci.d/containers.yml
>- tests/docker/*

The test-* and common.rc are used outside, maybe we can
amend:

-- tests/docker/*
+- tests/docker/Makefile.include
+- tests/docker/docker.py

Anyway,
Reviewed-by: Philippe Mathieu-Daudé 

> +  - tests/docker/dockerfiles/*
>  - if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH'
>  - if: '$CI_COMMIT_REF_NAME == "testing/next"'
>  
> 

Re: [PATCH v2 01/11] configure: remove all dependencies on a (re)configure

[Philippe Mathieu-Daudé]

On 7/13/20 10:04 PM, Alex Bennée wrote:
> The previous code was brittle and missed cases such as the mipn32
> variants which for some reason has the 64 bit syscalls. This leads to
> a number of binary targets having deps lines like:
> 
>   all.clang-sanitizer/mipsn32el-linux-user/linux-user/signal.d
>   140:  /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
>   455:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:
> 
>   all.clang-sanitizer/mipsn32el-linux-user/linux-user/syscall.d
>   146:  /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
>   485:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:
> 
> which in turn would trigger the re-generation of syscall_nr.h in the
> source tree (thanks to generic %/syscall_nr.h rules). The previous
> code attempts to clean it out but misses edge cases but fails.
> 
> After spending a day trying to understand how this was happening I'm
> unconvinced that there are not other such breakages possible with this
> "caching". As we add more auto-generated code to the build it is likely
> to trip up again. Apply a hammer to the problem.
> 
> Fixes: 91e5998f18 (which fixes 5f29856b852d and 4d6a835dea47)
> Signed-off-by: Alex Bennée 
> Reviewed-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  configure | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/configure b/configure
> index bc3b9ad931..e1de2f5b24 100755
> --- a/configure
> +++ b/configure
> @@ -1955,23 +1955,20 @@ EOF
>  exit 0
>  fi
>  
> -# Remove old dependency files to make sure that they get properly regenerated
> -rm -f */config-devices.mak.d
> -
>  # Remove syscall_nr.h to be sure they will be regenerated in the build
>  # directory, not in the source directory
>  for arch in alpha hppa m68k xtensa sh4 microblaze arm ppc s390x sparc 
> sparc64 \
>  i386 x86_64 mips mips64 ; do
>  # remove the file if it has been generated in the source directory
>  rm -f "${source_path}/linux-user/${arch}/syscall_nr.h"
> -# remove the dependency files
> -for target in ${arch}*-linux-user ; do
> -test -d "${target}" && find "${target}" -type f -name "*.d" \
> - -exec grep -q "${source_path}/linux-user/${arch}/syscall_nr.h" 
> {} \; \
> - -print | while read file ; do rm "${file}" "${file%.d}.o" ; done
> -done
>  done
>  
> +# Clean out all old dependency files. As more files are generated we
> +# run the risk of old dependencies triggering generation in the wrong
> +# places. Previous brittle attempts to be surgical tend to miss edge
> +# cases leading to wasted time and much confusion.
> +find -type f -name "*.d" -exec rm -f {} \;
> +
>  if test -z "$python"
>  then
>  error_exit "Python not found. Use --python=/path/to/python"
> 

Re: [RFC PATCH 2/8] fpu/softfloat: use the similiar logic to recognize sNaN and qNaN

[LIU Zhiwei]

On 2020/7/14 3:17, Richard Henderson wrote:

On 7/12/20 4:45 PM, LIU Zhiwei wrote:

Signed-off-by: LIU Zhiwei 
---
  fpu/softfloat-specialize.inc.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fpu/softfloat-specialize.inc.c b/fpu/softfloat-specialize.inc.c
index 034d18199c..6b778a7830 100644
--- a/fpu/softfloat-specialize.inc.c
+++ b/fpu/softfloat-specialize.inc.c
@@ -292,7 +292,7 @@ bool float32_is_quiet_nan(float32 a_, float_status *status)
  if (snan_bit_is_one(status)) {
  return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003F);
  } else {
-return ((uint32_t)(a << 1) >= 0xFF80);
+return ((a >> 22) & 0x1FF) == 0x1FF;
  }
  #endif
  }

I don't see a reason for this.  The previous was a bug, but this isn't.

It's not a bug,  just a clean up.

As you can see, we have already recognized a quiet nan by

 if (snan_bit_is_one(status)) {
 return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003F);
 }

We need not to give another method to recognize it again.

Zhiwei


r~

Re: [PATCH] gitlab-ci/containers: Add missing wildcard where we should look for changes

[Alex Bennée]

Thomas Huth  writes:

> The tests/docker/* wildcard seems to only match the files that are directly
> in the tests/docker folder - but changes to the files in the directory
> tests/docker/dockerfiles are currently ignored. Seems like we need a
> separate entry to match the files in that folder. With this wildcard added,
> the stages now get re-run successfully when something in the dockerfiles
> has been changed.
>
> Signed-off-by: Thomas Huth 

Queued to misc/for-5.1-rc0, thanks.

-- 
Alex Bennée

[PATCH v2 07/11] plugins: expand the bb plugin to be thread safe and track per-cpu

[Alex Bennée]

While there isn't any easy way to make the inline counts thread safe
we can ensure the callback based ones are. While we are at it we can
reduce introduce a new option ("idle") to dump a report of the current
bb and insn count each time a vCPU enters the idle state.

Signed-off-by: Alex Bennée 
Reviewed-by: Robert Foley 
Cc: Dave Bort 

---
v2
  - fixup for non-inline linux-user case
  - minor cleanup and re-factor
v3
  - checkpatch
---
 tests/plugin/bb.c | 97 ---
 1 file changed, 84 insertions(+), 13 deletions(-)

diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index df19fd359d..e4cc7fdd6e 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -16,24 +16,67 @@
 
 QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
 
-static uint64_t bb_count;
-static uint64_t insn_count;
+typedef struct {
+GMutex lock;
+int index;
+uint64_t bb_count;
+uint64_t insn_count;
+} CPUCount;
+
+/* Used by the inline & linux-user counts */
 static bool do_inline;
+static CPUCount inline_count;
+
+/* Dump running CPU total on idle? */
+static bool idle_report;
+static GPtrArray *counts;
+static int max_cpus;
+
+static void gen_one_cpu_report(CPUCount *count, GString *report)
+{
+if (count->bb_count) {
+g_string_append_printf(report, "CPU%d: "
+   "bb's: %" PRIu64", insns: %" PRIu64 "\n",
+   count->index,
+   count->bb_count, count->insn_count);
+}
+}
 
 static void plugin_exit(qemu_plugin_id_t id, void *p)
 {
-g_autofree gchar *out = g_strdup_printf(
-"bb's: %" PRIu64", insns: %" PRIu64 "\n",
-bb_count, insn_count);
-qemu_plugin_outs(out);
+g_autoptr(GString) report = g_string_new("");
+
+if (do_inline || !max_cpus) {
+g_string_printf(report, "bb's: %" PRIu64", insns: %" PRIu64 "\n",
+inline_count.bb_count, inline_count.insn_count);
+} else {
+g_ptr_array_foreach(counts, (GFunc) gen_one_cpu_report, report);
+}
+qemu_plugin_outs(report->str);
+}
+
+static void vcpu_idle(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+CPUCount *count = g_ptr_array_index(counts, cpu_index);
+g_autoptr(GString) report = g_string_new("");
+gen_one_cpu_report(count, report);
+
+if (report->len > 0) {
+g_string_prepend(report, "Idling ");
+qemu_plugin_outs(report->str);
+}
 }
 
 static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
 {
-unsigned long n_insns = (unsigned long)udata;
+CPUCount *count = max_cpus ?
+g_ptr_array_index(counts, cpu_index) : _count;
 
-insn_count += n_insns;
-bb_count++;
+unsigned long n_insns = (unsigned long)udata;
+g_mutex_lock(>lock);
+count->insn_count += n_insns;
+count->bb_count++;
+g_mutex_unlock(>lock);
 }
 
 static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
@@ -42,9 +85,10 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
qemu_plugin_tb *tb)
 
 if (do_inline) {
 qemu_plugin_register_vcpu_tb_exec_inline(tb, 
QEMU_PLUGIN_INLINE_ADD_U64,
- _count, 1);
+ _count.bb_count, 1);
 qemu_plugin_register_vcpu_tb_exec_inline(tb, 
QEMU_PLUGIN_INLINE_ADD_U64,
- _count, n_insns);
+ _count.insn_count,
+ n_insns);
 } else {
 qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
  QEMU_PLUGIN_CB_NO_REGS,
@@ -56,8 +100,35 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t 
id,
const qemu_info_t *info,
int argc, char **argv)
 {
-if (argc && strcmp(argv[0], "inline") == 0) {
-do_inline = true;
+int i;
+
+for (i = 0; i < argc; i++) {
+char *opt = argv[i];
+if (g_strcmp0(opt, "inline") == 0) {
+do_inline = true;
+} else if (g_strcmp0(opt, "idle") == 0) {
+idle_report = true;
+} else {
+fprintf(stderr, "option parsing failed: %s\n", opt);
+return -1;
+}
+}
+
+if (info->system_emulation && !do_inline) {
+max_cpus = info->system.max_vcpus;
+counts = g_ptr_array_new();
+for (i = 0; i < max_cpus; i++) {
+CPUCount *count = g_new0(CPUCount, 1);
+g_mutex_init(>lock);
+count->index = i;
+g_ptr_array_add(counts, count);
+}
+} else if (!do_inline) {
+g_mutex_init(_count.lock);
+}
+
+if (idle_report) {
+qemu_plugin_register_vcpu_idle_cb(id, vcpu_idle);
 }
 
 qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
-- 
2.20.1

[PATCH v2 11/11] plugins: new hwprofile plugin

[Alex Bennée]

This is a plugin intended to help with profiling access to various
bits of system hardware. It only really makes sense for system
emulation.

It takes advantage of the recently exposed helper API that allows us
to see the device name (memory region name) associated with a device.

You can specify arg=read or arg=write to limit the tracking to just
reads or writes (by default it does both).

The pattern option:

  -plugin ./tests/plugin/libhwprofile.so,arg=pattern

will allow you to see the access pattern to devices, eg:

  gic_cpu @ 0xffc01004
off:, 8, 1, 8, 1
off:, 4, 1, 4, 1
off:, 2, 1, 2, 1
off:, 1, 1, 1, 1

The source option:

  -plugin ./tests/plugin/libhwprofile.so,arg=source

will track the virtual source address of the instruction making the
access:

  pl011 @ 0xffc010031000
pc:ffc0104c785c, 1, 4, 0, 0
pc:ffc0104c7898, 1, 4, 0, 0
pc:ffc010512bcc, 2, 1867, 0, 0

You cannot mix source and pattern.

Finally the match option allow you to limit the tracking to just the
devices you care about.

Signed-off-by: Alex Bennée 
Reviewed-by: Robert Foley 
Tested-by: Robert Foley 

---
v2
  - use 64 bit cpu masks
  - warn if we will exceed cpu mask capability
  - don't run in linux-user mode
  - skip in check-tcg for linux-user test
v3
  - update device name API
  - re-factor and clean-up
  - add source tracking
  - use direct hash for source/pattern
  - add match function
  - expand the commit message
  - checkpatch cleanups
---
 tests/plugin/hwprofile.c  | 305 ++
 tests/plugin/Makefile |   1 +
 tests/tcg/Makefile.target |  12 +-
 3 files changed, 316 insertions(+), 2 deletions(-)
 create mode 100644 tests/plugin/hwprofile.c

diff --git a/tests/plugin/hwprofile.c b/tests/plugin/hwprofile.c
new file mode 100644
index 00..6dac1d5f85
--- /dev/null
+++ b/tests/plugin/hwprofile.c
@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2020, Alex Bennée 
+ *
+ * HW Profile - breakdown access patterns for IO to devices
+ *
+ * License: GNU GPL, version 2 or later.
+ *   See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
+typedef struct {
+uint64_t cpu_read;
+uint64_t cpu_write;
+uint64_t reads;
+uint64_t writes;
+} IOCounts;
+
+typedef struct {
+uint64_t off_or_pc;
+IOCounts counts;
+} IOLocationCounts;
+
+typedef struct {
+const char *name;
+uint64_t   base;
+IOCounts   totals;
+GHashTable *detail;
+} DeviceCounts;
+
+static GMutex lock;
+static GHashTable *devices;
+
+/* track the access pattern to a piece of HW */
+static bool pattern;
+/* track the source address of access to HW */
+static bool source;
+/* track only matched regions of HW */
+static bool check_match;
+static gchar **matches;
+
+static enum qemu_plugin_mem_rw rw = QEMU_PLUGIN_MEM_RW;
+
+static inline bool track_reads(void)
+{
+return rw == QEMU_PLUGIN_MEM_RW || rw == QEMU_PLUGIN_MEM_R;
+}
+
+static inline bool track_writes(void)
+{
+return rw == QEMU_PLUGIN_MEM_RW || rw == QEMU_PLUGIN_MEM_W;
+}
+
+static void plugin_init(void)
+{
+devices = g_hash_table_new(NULL, NULL);
+}
+
+static gint sort_cmp(gconstpointer a, gconstpointer b)
+{
+DeviceCounts *ea = (DeviceCounts *) a;
+DeviceCounts *eb = (DeviceCounts *) b;
+return ea->totals.reads + ea->totals.writes >
+   eb->totals.reads + eb->totals.writes ? -1 : 1;
+}
+
+static gint sort_loc(gconstpointer a, gconstpointer b)
+{
+IOLocationCounts *ea = (IOLocationCounts *) a;
+IOLocationCounts *eb = (IOLocationCounts *) b;
+return ea->off_or_pc > eb->off_or_pc;
+}
+
+static void fmt_iocount_record(GString *s, IOCounts *rec)
+{
+if (track_reads()) {
+g_string_append_printf(s, ", %"PRIx64", %"PRId64,
+   rec->cpu_read, rec->reads);
+}
+if (track_writes()) {
+g_string_append_printf(s, ", %"PRIx64", %"PRId64,
+   rec->cpu_write, rec->writes);
+}
+}
+
+static void fmt_dev_record(GString *s, DeviceCounts *rec)
+{
+g_string_append_printf(s, "%s, 0x%"PRIx64,
+   rec->name, rec->base);
+fmt_iocount_record(s, >totals);
+g_string_append_c(s, '\n');
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+g_autoptr(GString) report = g_string_new("");
+GList *counts;
+
+if (!(pattern || source)) {
+g_string_printf(report, "Device, Address");
+if (track_reads()) {
+g_string_append_printf(report, ", RCPUs, Reads");
+}
+if (track_writes()) {
+g_string_append_printf(report, ",  WCPUs, Writes");
+}
+g_string_append_c(report, '\n');
+}
+
+counts = g_hash_table_get_values(devices);
+  

[PATCH v2 03/11] docker.py: fix fetching of FROM layers

[Alex Bennée]

This worked on a system that was already bootstrapped because the
stage 2 images already existed even if they wouldn't be used. What we
should have pulled down was the FROM line containers first because
building on gitlab doesn't have the advantage of using our build
system to build the pre-requisite bits.

We still pull the image we want to build just in case we can use the
cached data.

Signed-off-by: Alex Bennée 
Reviewed-by: Daniel P. Berrangé 
Tested-by: Philippe Mathieu-Daudé 
---
 tests/docker/docker.py | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 2d67bbd15a..c9f20d8d09 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -306,14 +306,18 @@ class Docker(object):
 checksum = _text_checksum(_dockerfile_preprocess(dockerfile))
 
 if registry is not None:
-# see if we can fetch a cache copy, may fail...
-pull_args = ["pull", "%s/%s" % (registry, tag)]
-if self._do(pull_args, quiet=quiet) == 0:
+sources = re.findall("FROM qemu\/(.*)", dockerfile)
+# Fetch any cache layers we can, may fail
+for s in sources:
+pull_args = ["pull", "%s/qemu/%s" % (registry, s)]
+if self._do(pull_args, quiet=quiet) != 0:
+registry = None
+break
+# Make substitutions
+if registry is not None:
 dockerfile = dockerfile.replace("FROM qemu/",
 "FROM %s/qemu/" %
 (registry))
-else:
-registry = None
 
 tmp_df = tempfile.NamedTemporaryFile(mode="w+t",
  encoding='utf-8',
@@ -339,6 +343,8 @@ class Docker(object):
 build_args += ["--build-arg", "BUILDKIT_INLINE_CACHE=1"]
 
 if registry is not None:
+pull_args = ["pull", "%s/%s" % (registry, tag)]
+self._do(pull_args, quiet=quiet)
 cache = "%s/%s" % (registry, tag)
 build_args += ["--cache-from", cache]
 build_args += argv
-- 
2.20.1

[PATCH v2 09/11] hw/virtio/pci: include vdev name in registered PCI sections

[Alex Bennée]

When viewing/debugging memory regions it is sometimes hard to figure
out which PCI device something belongs to. Make the names unique by
including the vdev name in the name string.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael S. Tsirkin 

---
v2
  - swap ()'s for an extra -
---
 hw/virtio/virtio-pci.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 8554cf2a03..215e680c71 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1406,7 +1406,8 @@ static void virtio_pci_device_write(void *opaque, hwaddr 
addr,
 }
 }
 
-static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy)
+static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy,
+   const char *vdev_name)
 {
 static const MemoryRegionOps common_ops = {
 .read = virtio_pci_common_read,
@@ -1453,36 +1454,41 @@ static void 
virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy)
 },
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
+g_autoptr(GString) name = g_string_new(NULL);
 
-
+g_string_printf(name, "virtio-pci-common-%s", vdev_name);
 memory_region_init_io(>common.mr, OBJECT(proxy),
   _ops,
   proxy,
-  "virtio-pci-common",
+  name->str,
   proxy->common.size);
 
+g_string_printf(name, "virtio-pci-isr-%s", vdev_name);
 memory_region_init_io(>isr.mr, OBJECT(proxy),
   _ops,
   proxy,
-  "virtio-pci-isr",
+  name->str,
   proxy->isr.size);
 
+g_string_printf(name, "virtio-pci-device-%s", vdev_name);
 memory_region_init_io(>device.mr, OBJECT(proxy),
   _ops,
   virtio_bus_get_device(>bus),
-  "virtio-pci-device",
+  name->str,
   proxy->device.size);
 
+g_string_printf(name, "virtio-pci-notify-%s", vdev_name);
 memory_region_init_io(>notify.mr, OBJECT(proxy),
   _ops,
   virtio_bus_get_device(>bus),
-  "virtio-pci-notify",
+  name->str,
   proxy->notify.size);
 
+g_string_printf(name, "virtio-pci-notify-pio-%s", vdev_name);
 memory_region_init_io(>notify_pio.mr, OBJECT(proxy),
   _pio_ops,
   virtio_bus_get_device(>bus),
-  "virtio-pci-notify-pio",
+  name->str,
   proxy->notify_pio.size);
 }
 
@@ -1623,7 +1629,7 @@ static void virtio_pci_device_plugged(DeviceState *d, 
Error **errp)
 
 struct virtio_pci_cfg_cap *cfg_mask;
 
-virtio_pci_modern_regions_init(proxy);
+virtio_pci_modern_regions_init(proxy, vdev->name);
 
 virtio_pci_modern_mem_region_map(proxy, >common, );
 virtio_pci_modern_mem_region_map(proxy, >isr, );
-- 
2.20.1

[PATCH v2 05/11] tests/plugins: don't unconditionally add -Wpsabi

[Alex Bennée]

Not all compilers support the -Wpsabi (clang-9 in my case). To handle
this gracefully we pare back the shared build machinery so the
Makefile is relatively "standalone". We still take advantage of
config-host.mak as configure has done a bunch of probing for us but
that is it.

Fixes: bac8d222a
Signed-off-by: Alex Bennée 

---
v2
  - separate from main build system and check probe
---
 configure |  3 +++
 tests/plugin/Makefile | 22 ++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index e1de2f5b24..08eaa99d19 100755
--- a/configure
+++ b/configure
@@ -7112,6 +7112,9 @@ echo "GIT_UPDATE=$git_update" >> $config_host_mak
 
 echo "ARCH=$ARCH" >> $config_host_mak
 
+echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
+echo "GLIB_LDFLAGS=$glib_ldflags" >> $config_host_mak
+
 if test "$default_devices" = "yes" ; then
   echo "CONFIG_MINIKCONF_MODE=--defconfig" >> $config_host_mak
 else
diff --git a/tests/plugin/Makefile b/tests/plugin/Makefile
index 3a50451428..e9348fde4a 100644
--- a/tests/plugin/Makefile
+++ b/tests/plugin/Makefile
@@ -1,9 +1,16 @@
+# -*- Mode: makefile -*-
+#
+# This Makefile example is fairly independent from the main makefile
+# so users can take and adapt it for their build. We only really
+# include config-host.mak so we don't have to repeat probing for
+# cflags that the main configure has already done for us.
+#
+
 BUILD_DIR := $(CURDIR)/../..
 
 include $(BUILD_DIR)/config-host.mak
-include $(SRC_PATH)/rules.mak
 
-$(call set-vpath, $(SRC_PATH)/tests/plugin)
+VPATH += $(SRC_PATH)/tests/plugin
 
 NAMES :=
 NAMES += bb
@@ -17,11 +24,18 @@ NAMES += lockstep
 
 SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
 
-QEMU_CFLAGS += -fPIC -Wpsabi
-QEMU_CFLAGS += -I$(SRC_PATH)/include/qemu
+# The main QEMU uses Glib extensively so it's perfectly fine to use it
+# in plugins (which many example do).
+CFLAGS = $(GLIB_CFLAGS)
+CFLAGS += -fPIC
+CFLAGS += $(if $(findstring no-psabi,$(QEMU_CFLAGS)),-Wpsabi)
+CFLAGS += -I$(SRC_PATH)/include/qemu
 
 all: $(SONAMES)
 
+%.o: %.c
+   $(CC) $(CFLAGS) -c -o $@ $<
+
 lib%.so: %.o
$(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)
 
-- 
2.20.1

[PATCH v2 08/11] docs/devel: fix grammar in multi-thread-tcg

[Alex Bennée]

Review comment came just too late ;-)

Signed-off-by: Alex Bennée 
---
 docs/devel/multi-thread-tcg.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/devel/multi-thread-tcg.rst b/docs/devel/multi-thread-tcg.rst
index 42158b77c7..21483870db 100644
--- a/docs/devel/multi-thread-tcg.rst
+++ b/docs/devel/multi-thread-tcg.rst
@@ -107,7 +107,7 @@ including:
 
   - debugging operations (breakpoint insertion/removal)
   - some CPU helper functions
-  - linux-user spawning it's first thread
+  - linux-user spawning its first thread
 
 This is done with the async_safe_run_on_cpu() mechanism to ensure all
 vCPUs are quiescent when changes are being made to shared global
-- 
2.20.1

[PATCH v2 06/11] cputlb: ensure we save the IOTLB data in case of reset

[Alex Bennée]

Any write to a device might cause a re-arrangement of memory
triggering a TLB flush and potential re-size of the TLB invalidating
previous entries. This would cause users of qemu_plugin_get_hwaddr()
to see the warning:

  invalid use of qemu_plugin_get_hwaddr

because of the failed tlb_lookup which should always succeed. To
prevent this we save the IOTLB data in case it is later needed by a
plugin doing a lookup.

Signed-off-by: Alex Bennée 

---
v2
  - save the entry instead of re-running the tlb_fill.
v3
  - don't abuse TLS, use CPUState to store data
  - just use g_free_rcu() to avoid ugliness
  - verify addr matches before returning data
  - ws fix
v4
  - don't both with RCU, just store it in CPUState
  - clean-up #ifdef'ery
  - checkpatch
---
 include/hw/core/cpu.h   | 16 
 include/qemu/typedefs.h |  1 +
 accel/tcg/cputlb.c  | 38 +++---
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5542577d2b..8f145733ce 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -259,6 +259,18 @@ struct CPUWatchpoint {
 QTAILQ_ENTRY(CPUWatchpoint) entry;
 };
 
+#ifdef CONFIG_PLUGIN
+/*
+ * For plugins we sometime need to save the resolved iotlb data before
+ * the memory regions get moved around  by io_writex.
+ */
+typedef struct SavedIOTLB {
+hwaddr addr;
+MemoryRegionSection *section;
+hwaddr mr_offset;
+} SavedIOTLB;
+#endif
+
 struct KVMState;
 struct kvm_run;
 
@@ -417,7 +429,11 @@ struct CPUState {
 
 DECLARE_BITMAP(plugin_mask, QEMU_PLUGIN_EV_MAX);
 
+#ifdef CONFIG_PLUGIN
 GArray *plugin_mem_cbs;
+/* saved iotlb data from io_writex */
+SavedIOTLB saved_iotlb;
+#endif
 
 /* TODO Move common fields from CPUArchState here. */
 int cpu_index;
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 15f5047bf1..427027a970 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -116,6 +116,7 @@ typedef struct QObject QObject;
 typedef struct QString QString;
 typedef struct RAMBlock RAMBlock;
 typedef struct Range Range;
+typedef struct SavedIOTLB SavedIOTLB;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SSIBus SSIBus;
 typedef struct VirtIODevice VirtIODevice;
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 1e815357c7..d370aedb47 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1073,6 +1073,24 @@ static uint64_t io_readx(CPUArchState *env, 
CPUIOTLBEntry *iotlbentry,
 return val;
 }
 
+/*
+ * Save a potentially trashed IOTLB entry for later lookup by plugin.
+ *
+ * We also need to track the thread storage address because the RCU
+ * cleanup that runs when we leave the critical region (the current
+ * execution) is actually in a different thread.
+ */
+static void save_iotlb_data(CPUState *cs, hwaddr addr,
+MemoryRegionSection *section, hwaddr mr_offset)
+{
+#ifdef CONFIG_PLUGIN
+SavedIOTLB *saved = >saved_iotlb;
+saved->addr = addr;
+saved->section = section;
+saved->mr_offset = mr_offset;
+#endif
+}
+
 static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
   int mmu_idx, uint64_t val, target_ulong addr,
   uintptr_t retaddr, MemOp op)
@@ -1092,6 +1110,12 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 }
 cpu->mem_io_pc = retaddr;
 
+/*
+ * The memory_region_dispatch may trigger a flush/resize
+ * so for plugins we save the iotlb_data just in case.
+ */
+save_iotlb_data(cpu, iotlbentry->addr, section, mr_offset);
+
 if (mr->global_locking && !qemu_mutex_iothread_locked()) {
 qemu_mutex_lock_iothread();
 locked = true;
@@ -1381,8 +1405,11 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
  * in the softmmu lookup code (or helper). We don't handle re-fills or
  * checking the victim table. This is purely informational.
  *
- * This should never fail as the memory access being instrumented
- * should have just filled the TLB.
+ * This almost never fails as the memory access being instrumented
+ * should have just filled the TLB. The one corner case is io_writex
+ * which can cause TLB flushes and potential resizing of the TLBs
+ * loosing the information we need. In those cases we need to recover
+ * data from a copy of the io_tlb entry.
  */
 
 bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
@@ -1406,8 +1433,13 @@ bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, 
int mmu_idx,
 data->v.ram.hostaddr = addr + tlbe->addend;
 }
 return true;
+} else {
+SavedIOTLB *saved = >saved_iotlb;
+data->is_io = true;
+data->v.io.section = saved->section;
+data->v.io.offset = saved->mr_offset;
+return true;
 }
-return false;
 }
 
 #endif
-- 
2.20.1

[PATCH v2 01/11] configure: remove all dependencies on a (re)configure

[Alex Bennée]

The previous code was brittle and missed cases such as the mipn32
variants which for some reason has the 64 bit syscalls. This leads to
a number of binary targets having deps lines like:

  all.clang-sanitizer/mipsn32el-linux-user/linux-user/signal.d
  140:  /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
  455:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:

  all.clang-sanitizer/mipsn32el-linux-user/linux-user/syscall.d
  146:  /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
  485:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:

which in turn would trigger the re-generation of syscall_nr.h in the
source tree (thanks to generic %/syscall_nr.h rules). The previous
code attempts to clean it out but misses edge cases but fails.

After spending a day trying to understand how this was happening I'm
unconvinced that there are not other such breakages possible with this
"caching". As we add more auto-generated code to the build it is likely
to trip up again. Apply a hammer to the problem.

Fixes: 91e5998f18 (which fixes 5f29856b852d and 4d6a835dea47)
Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
---
 configure | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index bc3b9ad931..e1de2f5b24 100755
--- a/configure
+++ b/configure
@@ -1955,23 +1955,20 @@ EOF
 exit 0
 fi
 
-# Remove old dependency files to make sure that they get properly regenerated
-rm -f */config-devices.mak.d
-
 # Remove syscall_nr.h to be sure they will be regenerated in the build
 # directory, not in the source directory
 for arch in alpha hppa m68k xtensa sh4 microblaze arm ppc s390x sparc sparc64 \
 i386 x86_64 mips mips64 ; do
 # remove the file if it has been generated in the source directory
 rm -f "${source_path}/linux-user/${arch}/syscall_nr.h"
-# remove the dependency files
-for target in ${arch}*-linux-user ; do
-test -d "${target}" && find "${target}" -type f -name "*.d" \
- -exec grep -q "${source_path}/linux-user/${arch}/syscall_nr.h" {} 
\; \
- -print | while read file ; do rm "${file}" "${file%.d}.o" ; done
-done
 done
 
+# Clean out all old dependency files. As more files are generated we
+# run the risk of old dependencies triggering generation in the wrong
+# places. Previous brittle attempts to be surgical tend to miss edge
+# cases leading to wasted time and much confusion.
+find -type f -name "*.d" -exec rm -f {} \;
+
 if test -z "$python"
 then
 error_exit "Python not found. Use --python=/path/to/python"
-- 
2.20.1

[PATCH v2 10/11] plugins: add API to return a name for a IO device

[Alex Bennée]

This may well end up being anonymous but it should always be unique.

Signed-off-by: Alex Bennée 
[r-b provisional given change to g_intern_string]
Reviewed-by: Clement Deschamps 
Reviewed-by: Emilio G. Cota 

---
v3
  - return a non-freeable const g_intern_string()
  - checkpatch cleanups
---
 include/qemu/qemu-plugin.h |  6 ++
 plugins/api.c  | 20 
 2 files changed, 26 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index bab8b0d4b3..c98c18d6b0 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -335,6 +335,12 @@ struct qemu_plugin_hwaddr 
*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
 bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
 uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr 
*haddr);
 
+/*
+ * Returns a string representing the device. The string is valid for
+ * the lifetime of the plugin.
+ */
+const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h);
+
 typedef void
 (*qemu_plugin_vcpu_mem_cb_t)(unsigned int vcpu_index,
  qemu_plugin_meminfo_t info, uint64_t vaddr,
diff --git a/plugins/api.c b/plugins/api.c
index bbdc5a4eb4..4304e63f0c 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -303,6 +303,26 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct 
qemu_plugin_hwaddr *haddr
 return 0;
 }
 
+const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h)
+{
+#ifdef CONFIG_SOFTMMU
+if (h && h->is_io) {
+MemoryRegionSection *mrs = h->v.io.section;
+if (!mrs->mr->name) {
+unsigned long maddr = 0x & (uintptr_t) mrs->mr;
+g_autofree char *temp = g_strdup_printf("anon%08lx", maddr);
+return g_intern_string(temp);
+} else {
+return g_intern_string(mrs->mr->name);
+}
+} else {
+return g_intern_string("RAM");
+}
+#else
+return g_intern_string("Invalid");
+#endif
+}
+
 /*
  * Queries to the number and potential maximum number of vCPUs there
  * will be. This helps the plugin dimension per-vcpu arrays.
-- 
2.20.1

[PATCH v2 02/11] tests/docker: Remove the libssh workaround from the ubuntu 20.04 image

[Alex Bennée]

From: Thomas Huth 

The libssh problem only exists in Ubuntu 18.04 - we can enable it
in 20.04 again.

Signed-off-by: Thomas Huth 
Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200713185237.9419-1-th...@redhat.com>
---
 tests/docker/dockerfiles/ubuntu2004.docker | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
b/tests/docker/dockerfiles/ubuntu2004.docker
index f7aac840bf..8d10934a2a 100644
--- a/tests/docker/dockerfiles/ubuntu2004.docker
+++ b/tests/docker/dockerfiles/ubuntu2004.docker
@@ -65,9 +65,6 @@ RUN apt-get update && \
 RUN dpkg -l $PACKAGES | sort > /packages.txt
 ENV FEATURES clang tsan pyyaml sdl2
 
-# https://bugs.launchpad.net/qemu/+bug/1838763
-ENV QEMU_CONFIGURE_OPTS --disable-libssh
-
 # Apply patch https://reviews.llvm.org/D75820
 # This is required for TSan in clang-10 to compile with QEMU.
 RUN sed -i 's/^const/static const/g' 
/usr/lib/llvm-10/lib/clang/10.0.0/include/sanitizer/tsan_interface.h
-- 
2.20.1

[PATCH v2 00/11] misc fixes for rc0 (docker, plugins, softfloat)

[Alex Bennée]

Hi,

These are some candidate patches for rc0 along with a few plugin
patches that haven't yet gotten review. The new functionality won't
get added to the PR but I'd like to get the cputlb fix in. I've also
had another run at fixing the -Wpsabi problem.

The following still need review:

 - docs/devel: fix grammar in multi-thread-tcg
 - tests/plugins: don't unconditionally add -Wpsabi
 - cputlb: ensure we save the IOTLB data in case of reset

Alex Bennée (9):
  configure: remove all dependencies on a (re)configure
  docker.py: fix fetching of FROM layers
  tests/plugins: don't unconditionally add -Wpsabi
  cputlb: ensure we save the IOTLB data in case of reset
  plugins: expand the bb plugin to be thread safe and track per-cpu
  docs/devel: fix grammar in multi-thread-tcg
  hw/virtio/pci: include vdev name in registered PCI sections
  plugins: add API to return a name for a IO device
  plugins: new hwprofile plugin

LIU Zhiwei (1):
  fpu/softfloat: fix up float16 nan recognition

Thomas Huth (1):
  tests/docker: Remove the libssh workaround from the ubuntu 20.04 image

 docs/devel/multi-thread-tcg.rst|   2 +-
 configure  |  18 +-
 include/hw/core/cpu.h  |  16 ++
 include/qemu/qemu-plugin.h |   6 +
 include/qemu/typedefs.h|   1 +
 accel/tcg/cputlb.c |  38 ++-
 fpu/softfloat-specialize.inc.c |   4 +-
 hw/virtio/virtio-pci.c |  22 +-
 plugins/api.c  |  20 ++
 tests/plugin/bb.c  |  97 ++-
 tests/plugin/hwprofile.c   | 305 +
 tests/docker/docker.py |  16 +-
 tests/docker/dockerfiles/ubuntu2004.docker |   3 -
 tests/plugin/Makefile  |  23 +-
 tests/tcg/Makefile.target  |  12 +-
 15 files changed, 533 insertions(+), 50 deletions(-)
 create mode 100644 tests/plugin/hwprofile.c

-- 
2.20.1

[PATCH v2 04/11] fpu/softfloat: fix up float16 nan recognition

[Alex Bennée]

From: LIU Zhiwei 

Signed-off-by: LIU Zhiwei 
Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Message-Id: <20200712234521.3972-2-zhiwei_...@c-sky.com>
---
 fpu/softfloat-specialize.inc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fpu/softfloat-specialize.inc.c b/fpu/softfloat-specialize.inc.c
index 44f5b661f8..034d18199c 100644
--- a/fpu/softfloat-specialize.inc.c
+++ b/fpu/softfloat-specialize.inc.c
@@ -254,7 +254,7 @@ bool float16_is_quiet_nan(float16 a_, float_status *status)
 if (snan_bit_is_one(status)) {
 return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
 } else {
-return ((a & ~0x8000) >= 0x7C80);
+return ((a >> 9) & 0x3F) == 0x3F;
 }
 #endif
 }
@@ -271,7 +271,7 @@ bool float16_is_signaling_nan(float16 a_, float_status 
*status)
 #else
 uint16_t a = float16_val(a_);
 if (snan_bit_is_one(status)) {
-return ((a & ~0x8000) >= 0x7C80);
+return ((a >> 9) & 0x3F) == 0x3F;
 } else {
 return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
 }
-- 
2.20.1

Re: [RFC PATCH 1/8] fpu/softfloat: fix up float16 nan recognition

[Alex Bennée]

LIU Zhiwei  writes:

> Signed-off-by: LIU Zhiwei 

Queued to misc/for-5.1-rc0, thanks.

-- 
Alex Bennée

Re: [RFC PATCH] tests/docker: Remove the libssh workaround from the ubuntu 20.04 image

[Alex Bennée]

Thomas Huth  writes:

> The libssh problem only exists in Ubuntu 18.04 - we can enable it
> in 20.04 again.

Queued to misc/for-5.1-rc0, thanks.

>
> Signed-off-by: Thomas Huth 
> ---
>  tests/docker/dockerfiles/ubuntu2004.docker | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
> b/tests/docker/dockerfiles/ubuntu2004.docker
> index f7aac840bf..8d10934a2a 100644
> --- a/tests/docker/dockerfiles/ubuntu2004.docker
> +++ b/tests/docker/dockerfiles/ubuntu2004.docker
> @@ -65,9 +65,6 @@ RUN apt-get update && \
>  RUN dpkg -l $PACKAGES | sort > /packages.txt
>  ENV FEATURES clang tsan pyyaml sdl2
>  
> -# https://bugs.launchpad.net/qemu/+bug/1838763
> -ENV QEMU_CONFIGURE_OPTS --disable-libssh
> -
>  # Apply patch https://reviews.llvm.org/D75820
>  # This is required for TSan in clang-10 to compile with QEMU.
>  RUN sed -i 's/^const/static const/g' 
> /usr/lib/llvm-10/lib/clang/10.0.0/include/sanitizer/tsan_interface.h


-- 
Alex Bennée

[Bug 1884719] Re: Function not implemented when using libaio

[Laurent Vivier]

Martin,

do you want to propose some patches to fix the problem?

Thanks

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

Title:
  Function not implemented when using libaio

Status in QEMU:
  New

Bug description:
  Hello

  I experience "Function not implemented" errors when trying to use
  Linux libaio library in foreign architecture, e.g. aarch64.

  I've faced this problem while using 
https://github.com/multiarch/qemu-user-static, i.e. Docker+QEMU. 
  I understand that I do not use plain QEMU and you may count this report as a 
"distribution of QEMU"! Just let me know what are the steps to test it with 
plain QEMU and I will test and update this ticket!

  
  Here are the steps to reproduce the issue:

  1) On x86_64 machine register QEMU:

  `docker run -it --rm --privileged multiarch/qemu-user-static
  --reset --credential yes --persistent yes`

  2) Start a Docker image with foreign CPU architecture, e.g. aarch64

  `docker run -it arm64v8/centos:8 bash`

  3) Inside the Docker container install GCC and libaio

  `yum install gcc libaio libaio-devel`

  4) Compile the following C program

  ```
  #include 
  #include 
  #include 
  #include 

  struct io_control {
  io_context_t ioContext;
  };

  int main() {
  int queueSize = 10;

  struct io_control * theControl = (struct io_control *) 
malloc(sizeof(struct io_control));
  if (theControl == NULL) {
  printf("theControl is NULL");
  return 123;
  }

  int res = io_queue_init(queueSize, >ioContext);
  io_queue_release(theControl->ioContext);
  free(theControl);
  printf("res is: %d", res);
  }
  ```

  ```
  cat > test.c
  [PASTE THE CODE ABOVE HERE]
  ^D
  ```

  `gcc test.c -o out -laio && ./out`

  
  When executed directly on aarch64 machine (i.e. without emulation) or on 
x86_64 Docker image (e.g. centos:8) it prints `res is: 0`, i.e. it successfully 
initialized a LibAIO queue.

  But when executed on Docker image with foreign/emulated CPU
  architecture it prints `res is: -38` (ENOSYS). `man io_queue_init`
  says that error ENOSYS is returned when "Not implemented."

  Environment:

  QEMU version: 5.0.0.2  
(https://github.com/multiarch/qemu-user-static/blob/master/.travis.yml#L24-L28)
  Container application: Docker
  Output of `docker --version`:

  ```
  Client:
   Version:   19.03.8
   API version:   1.40
   Go version:go1.13.8
   Git commit:afacb8b7f0
   Built: Wed Mar 11 23:42:35 2020
   OS/Arch:   linux/amd64
   Experimental:  false

  Server:
   Engine:
Version:  19.03.8
API version:  1.40 (minimum version 1.12)
Go version:   go1.13.8
Git commit:   afacb8b7f0
Built:Wed Mar 11 22:48:33 2020
OS/Arch:  linux/amd64
Experimental: false
   containerd:
Version:  1.3.3-0ubuntu2
GitCommit:
   runc:
Version:  spec: 1.0.1-dev
GitCommit:
   docker-init:
Version:  0.18.0
GitCommit:
  ```

  Same happens with Ubuntu (arm64v8/ubuntu:focal).

  I've tried to `strace` it but :

  ```
  /usr/bin/strace: ptrace(PTRACE_TRACEME, ...): Function not implemented
  /usr/bin/strace: PTRACE_SETOPTIONS: Function not implemented
  /usr/bin/strace: detach: waitpid(112): No child processes
  /usr/bin/strace: Process 112 detached
  ```

  Here are the steps to reproduce the problem with strace:

   ```
   docker run --rm -it --security-opt seccomp:unconfined --security-opt 
apparmor:unconfined --privileged --cap-add ALL arm64v8/centos:8 bash

   yum install -y strace`

   strace echo Test
   ```

  Note: I used --privileged, disabled seccomp and apparmor, and added
  all capabilities

  Disabling security solves the "Permission denied" problem but then
  comes the "Not implemented" one.

  
  Any idea what could be the problem and how to work it around ?
  I've googled a lot but I wasn't able to find any problems related to libaio 
on QEMU.

  Thank you!
  Martin

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

Re: [PATCH v1] target/riscv: fix pmp implementation

[Alistair Francis]

On Mon, Jul 13, 2020 at 3:10 AM Alexandre Mergnat  wrote:
>
> Le ven. 10 juil. 2020 à 22:35, Alistair Francis  a 
> écrit :
> >
> > On Mon, Jul 6, 2020 at 2:45 AM Alexandre Mergnat  
> > wrote:
> > >
> > > The end address calculation for NA4 mode is wrong because the address
> > > used isn't shifted.
> > >
> > > That imply all NA4 setup are not applied by the PMP.
> >
> > I'm not sure what you mean here, can you clarify this?
>
> I'm just saying NA4 configuration doesn't work properly on QEMU (It
> doesn't watch 4byte but a huge range)
> because the end address calculation is wrong.

Ok, I replaced the original sentence with:

It doesn't watch 4 bytes but a huge range because the end address
calculation is wrong.

and changed the title to: target/riscv: Fix pmp NA4 implementation

and applied it to the RISC-V tree.

Alistair

>
> >
> > >
> > > The solution is to use the shifted address calculated for start address
> > > variable.
> > >
> > > Modifications are tested on Zephyr OS userspace test suite which works
> > > for other RISC-V boards (E31 and E34 core).
> > >
> > > Signed-off-by: Alexandre Mergnat 
> >
> > Otherwise:
> >
> > Reviewed-by: Alistair Francis 
> >
> > Alistair
> >
> > > ---
> > >  target/riscv/pmp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> > > index 9418660f1b..2a2b9f5363 100644
> > > --- a/target/riscv/pmp.c
> > > +++ b/target/riscv/pmp.c
> > > @@ -171,7 +171,7 @@ static void pmp_update_rule(CPURISCVState *env, 
> > > uint32_t pmp_index)
> > >
> > >  case PMP_AMATCH_NA4:
> > >  sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> > > -ea = (this_addr + 4u) - 1u;
> > > +ea = (sa + 4u) - 1u;
> > >  break;
> > >
> > >  case PMP_AMATCH_NAPOT:
> > > --
> > > 2.17.1
> > >
> > >

Re: [PATCH v4 07/11] target/ppc: add vmulld to INDEX_op_mul_vec case

[Lijun Pan]

> On Jul 1, 2020, at 6:43 PM, Lijun Pan  wrote:
> 
> Group vmuluwm and vmulld. Make vmulld-specific
> changes since it belongs to new ISA 3.1.
> 
> Signed-off-by: Lijun Pan 
> ---
> v4: add missing changes, and split to 5/11, 6/11, 7/11
> v3: use tcg_gen_gvec_mul()
> v2: fix coding style
>use Power ISA 3.1 flag
> 

Richard,
Do you have any opinion on this one?
Thanks,
Lijun


> tcg/ppc/tcg-target.h |  2 ++
> tcg/ppc/tcg-target.inc.c | 12 ++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
> index 4fa21f0e71..ff1249ef8e 100644
> --- a/tcg/ppc/tcg-target.h
> +++ b/tcg/ppc/tcg-target.h
> @@ -63,6 +63,7 @@ typedef enum {
> tcg_isa_2_06,
> tcg_isa_2_07,
> tcg_isa_3_00,
> +tcg_isa_3_10,
> } TCGPowerISA;
> 
> extern TCGPowerISA have_isa;
> @@ -72,6 +73,7 @@ extern bool have_vsx;
> #define have_isa_2_06  (have_isa >= tcg_isa_2_06)
> #define have_isa_2_07  (have_isa >= tcg_isa_2_07)
> #define have_isa_3_00  (have_isa >= tcg_isa_3_00)
> +#define have_isa_3_10  (have_isa >= tcg_isa_3_10)
> 
> /* optional instructions automatically implemented */
> #define TCG_TARGET_HAS_ext8u_i320 /* andi */
> diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
> index ee1f9227c1..caa8985b46 100644
> --- a/tcg/ppc/tcg-target.inc.c
> +++ b/tcg/ppc/tcg-target.inc.c
> @@ -564,6 +564,7 @@ static int tcg_target_const_match(tcg_target_long val, 
> TCGType type,
> #define VMULOUHVX4(72)
> #define VMULOUWVX4(136)   /* v2.07 */
> #define VMULUWMVX4(137)   /* v2.07 */
> +#define VMULLD VX4(457)   /* v3.10 */
> #define VMSUMUHM   VX4(38)
> 
> #define VMRGHB VX4(12)
> @@ -3015,6 +3016,8 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, 
> unsigned vece)
> return -1;
> case MO_32:
> return have_isa_2_07 ? 1 : -1;
> +case MO_64:
> +return have_isa_3_10;
> }
> return 0;
> case INDEX_op_bitsel_vec:
> @@ -3149,6 +3152,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
> static const uint32_t
> add_op[4] = { VADDUBM, VADDUHM, VADDUWM, VADDUDM },
> sub_op[4] = { VSUBUBM, VSUBUHM, VSUBUWM, VSUBUDM },
> +mul_op[4] = { 0, 0, VMULUWM, VMULLD },
> neg_op[4] = { 0, 0, VNEGW, VNEGD },
> eq_op[4]  = { VCMPEQUB, VCMPEQUH, VCMPEQUW, VCMPEQUD },
> ne_op[4]  = { VCMPNEB, VCMPNEH, VCMPNEW, 0 },
> @@ -3199,8 +3203,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
> a1 = 0;
> break;
> case INDEX_op_mul_vec:
> -tcg_debug_assert(vece == MO_32 && have_isa_2_07);
> -insn = VMULUWM;
> +insn = mul_op[vece];
> break;
> case INDEX_op_ssadd_vec:
> insn = ssadd_op[vece];
> @@ -3709,6 +3712,11 @@ static void tcg_target_init(TCGContext *s)
> have_isa = tcg_isa_3_00;
> }
> #endif
> +#ifdef PPC_FEATURE2_ARCH_3_10
> +if (hwcap2 & PPC_FEATURE2_ARCH_3_10) {
> +have_isa = tcg_isa_3_10;
> +}
> +#endif
> 
> #ifdef PPC_FEATURE2_HAS_ISEL
> /* Prefer explicit instruction from the kernel. */
> -- 
> 2.23.0
> 
> 

Re: [PATCH v3 0/9] memory: assert and define MemoryRegionOps callbacks

[Philippe Mathieu-Daudé]

On 7/13/20 8:58 PM, P J P wrote:
> +-- On Tue, 30 Jun 2020, P J P wrote --+
> | * This series asserts that MemoryRegionOps objects define read/write 
> |   callback methods. Thus avoids potential NULL pointer dereference.
> |   ex. -> 
> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2
> | 
> | * Also adds various undefined MemoryRegionOps read/write functions
> |   to avoid potential assert failure.
> | 
> | Thank you.
> | --
> | Prasad J Pandit (9):
> |   hw/pci-host: add pci-intack write method
> |   pci-host: add pcie-msi read method
> |   vfio: add quirk device write method
> |   prep: add ppc-parity write method
> |   nvram: add nrf51_soc flash read method
> |   spapr_pci: add spapr msi read method
> |   tz-ppc: add dummy read/write methods
> |   imx7-ccm: add digprog mmio write method
> |   memory: assert MemoryRegionOps callbacks are defined
> | 
> |  hw/misc/imx7_ccm.c   |  7 +++
> |  hw/misc/tz-ppc.c | 14 ++
> |  hw/nvram/nrf51_nvm.c |  5 +
> |  hw/pci-host/designware.c |  9 +
> |  hw/pci-host/prep.c   |  8 
> |  hw/ppc/prep_systemio.c   |  8 
> |  hw/ppc/spapr_pci.c   | 13 +++--
> |  hw/vfio/pci-quirks.c |  8 
> |  memory.c | 10 +-
> |  9 files changed, 79 insertions(+), 3 deletions(-)
> 
> 
> @Paolo: all patches in this series are reviewed/ack'd. Need any change/update 
> from me? (just checking)

Paolo isn't available, maybe ask each maintainer? ARM/PPC mostly.

Re: [PATCH v2 7/9] hw/sd/sdcard: Do not allow invalid SD card sizes

[Alistair Francis]

On Mon, Jul 13, 2020 at 11:33 AM Philippe Mathieu-Daudé  wrote:
>
> QEMU allows to create SD card with unrealistic sizes. This could
> work, but some guests (at least Linux) consider sizes that are not
> a power of 2 as a firmware bug and fix the card size to the next
> power of 2.
>
> While the possibility to use small SD card images has been seen as
> a feature, it became a bug with CVE-2020-13253, where the guest is
> able to do OOB read/write accesses past the image size end.
>
> In a pair of commits we will fix CVE-2020-13253 as:
>
> Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
> occurred and no data transfer is performed.
>
> Write command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
> occurred and no data transfer is performed.
>
> WP_VIOLATION errors are not modified: the error bit is set, we
> stay in receive-data state, wait for a stop command. All further
> data transfer is ignored. See the check on sd->card_status at the
> beginning of sd_read_data() and sd_write_data().
>
> While this is the correct behavior, in case QEMU create smaller SD
> cards, guests still try to access past the image size end, and QEMU
> considers this is an invalid address, thus "all further data transfer
> is ignored". This is wrong and make the guest looping until
> eventually timeouts.
>
> Fix by not allowing invalid SD card sizes (suggesting the expected
> size as a hint):
>
>   $ qemu-system-arm -M orangepi-pc -drive file=rootfs.ext2,if=sd,format=raw
>   qemu-system-arm: Invalid SD card size: 60 MiB
>   SD card size has to be a power of 2, e.g. 64 MiB.
>   You can resize disk images with 'qemu-img resize  '
>   (note that this will lose data if you make the image smaller than it 
> currently is).
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
> Since v1:
>   Addressed Alistair & Peter comments (error_append_hint message)
> ---
>  hw/sd/sd.c | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index edd60a09c0..5ab945dade 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -32,6 +32,7 @@
>
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
> +#include "qemu/cutils.h"
>  #include "hw/irq.h"
>  #include "hw/registerfields.h"
>  #include "sysemu/block-backend.h"
> @@ -2106,11 +2107,35 @@ static void sd_realize(DeviceState *dev, Error **errp)
>  }
>
>  if (sd->blk) {
> +int64_t blk_size;
> +
>  if (blk_is_read_only(sd->blk)) {
>  error_setg(errp, "Cannot use read-only drive as SD card");
>  return;
>  }
>
> +blk_size = blk_getlength(sd->blk);
> +if (blk_size > 0 && !is_power_of_2(blk_size)) {
> +int64_t blk_size_aligned = pow2ceil(blk_size);
> +char *blk_size_str;
> +
> +blk_size_str = size_to_str(blk_size);
> +error_setg(errp, "Invalid SD card size: %s", blk_size_str);
> +g_free(blk_size_str);
> +
> +blk_size_str = size_to_str(blk_size_aligned);
> +error_append_hint(errp,
> +  "SD card size has to be a power of 2, e.g. 
> %s.\n"
> +  "You can resize disk images with "
> +  "'qemu-img resize  '\n"
> +  "(note that this will lose data if you make 
> the "
> +  "image smaller than it currently is).\n",
> +  blk_size_str);
> +g_free(blk_size_str);
> +
> +return;
> +}
> +
>  ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | 
> BLK_PERM_WRITE,
> BLK_PERM_ALL, errp);
>  if (ret < 0) {
> --
> 2.21.3
>
>

Re: [PATCH v2 4/9] tests/acceptance/boot_linux: Expand SD card image to power of 2

[Alistair Francis]

On Mon, Jul 13, 2020 at 11:34 AM Philippe Mathieu-Daudé  wrote:
>
> In few commits we won't allow SD card images with invalid size
> (not aligned to a power of 2). Prepare the tests: add the
> pow2ceil() and image_pow2ceil_expand() methods and resize the
> images (expanding) of the tests using SD cards.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
> Since v1: Addressed review comments
> - truncate -> expand reword (Alistair Francis)
> - expand after uncompress (Niek Linnenbank)
> ---
>  tests/acceptance/boot_linux_console.py | 27 +-
>  1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> index b7e8858c2d..8f2a6aa8a4 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -28,6 +28,18 @@
>  except CmdNotFoundError:
>  P7ZIP_AVAILABLE = False
>
> +# round up to next power of 2
> +def pow2ceil(x):
> +return 1 if x == 0 else 2**(x - 1).bit_length()
> +
> +# expand file size to next power of 2
> +def image_pow2ceil_expand(path):
> +size = os.path.getsize(path)
> +size_aligned = pow2ceil(size)
> +if size != size_aligned:
> +with open(path, 'ab+') as fd:
> +fd.truncate(size_aligned)
> +
>  class LinuxKernelTest(Test):
>  KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
>
> @@ -636,6 +648,7 @@ def test_arm_orangepi_sd(self):
>  rootfs_path_xz = self.fetch_asset(rootfs_url, asset_hash=rootfs_hash)
>  rootfs_path = os.path.join(self.workdir, 'rootfs.cpio')
>  archive.lzma_uncompress(rootfs_path_xz, rootfs_path)
> +image_pow2ceil_expand(rootfs_path)
>
>  self.vm.set_console()
>  kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> @@ -673,7 +686,7 @@ def test_arm_orangepi_bionic(self):
>  :avocado: tags=device:sd
>  """
>
> -# This test download a 196MB compressed image and expand it to 
> 932MB...
> +# This test download a 196MB compressed image and expand it to 1GB
>  image_url = ('https://dl.armbian.com/orangepipc/archive/'
>   'Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.7z')
>  image_hash = '196a8ffb72b0123d92cea4a070894813d305c71e'
> @@ -681,6 +694,7 @@ def test_arm_orangepi_bionic(self):
>  image_name = 'Armbian_19.11.3_Orangepipc_bionic_current_5.3.9.img'
>  image_path = os.path.join(self.workdir, image_name)
>  process.run("7z e -o%s %s" % (self.workdir, image_path_7z))
> +image_pow2ceil_expand(image_path)
>
>  self.vm.set_console()
>  self.vm.add_args('-drive', 'file=' + image_path + 
> ',if=sd,format=raw',
> @@ -714,7 +728,7 @@ def test_arm_orangepi_uboot_netbsd9(self):
>  :avocado: tags=machine:orangepi-pc
>  :avocado: tags=device:sd
>  """
> -# This test download a 304MB compressed image and expand it to 
> 1.3GB...
> +# This test download a 304MB compressed image and expand it to 2GB
>  deb_url = ('http://snapshot.debian.org/archive/debian/'
> '20200108T145233Z/pool/main/u/u-boot/'
> 'u-boot-sunxi_2020.01%2Bdfsg-1_armhf.deb')
> @@ -731,8 +745,9 @@ def test_arm_orangepi_uboot_netbsd9(self):
>  image_hash = '2babb29d36d8360adcb39c09e31060945259917a'
>  image_path_gz = self.fetch_asset(image_url, asset_hash=image_hash)
>  image_path = os.path.join(self.workdir, 'armv7.img')
> -image_drive_args = 'if=sd,format=raw,snapshot=on,file=' + image_path
>  archive.gzip_uncompress(image_path_gz, image_path)
> +image_pow2ceil_expand(image_path)
> +image_drive_args = 'if=sd,format=raw,snapshot=on,file=' + image_path
>
>  # dd if=u-boot-sunxi-with-spl.bin of=armv7.img bs=1K seek=8 
> conv=notrunc
>  with open(uboot_path, 'rb') as f_in:
> @@ -740,12 +755,6 @@ def test_arm_orangepi_uboot_netbsd9(self):
>  f_out.seek(8 * 1024)
>  shutil.copyfileobj(f_in, f_out)
>
> -# Extend image, to avoid that NetBSD thinks the partition
> -# inside the image is larger than device size itself
> -f_out.seek(0, 2)
> -f_out.seek(64 * 1024 * 1024, 1)
> -f_out.write(bytearray([0x00]))
> -
>  self.vm.set_console()
>  self.vm.add_args('-nic', 'user',
>   '-drive', image_drive_args,
> --
> 2.21.3
>
>

Re: [PATCH v4 11/11] target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions

[Lijun Pan]

> On Jul 1, 2020, at 6:47 PM, Lijun Pan  wrote:
> 
> vdivsw: Vector Divide Signed Word
> vdivuw: Vector Divide Unsigned Word
> vdivsd: Vector Divide Signed Doubleword
> vdivud: Vector Divide Unsigned Doubleword
> vmodsw: Vector Modulo Signed Word
> vmoduw: Vector Modulo Unsigned Word
> vmodsd: Vector Modulo Signed Doubleword
> vmodud: Vector Modulo Unsigned Doubleword
> 
> Signed-off-by: Lijun Pan 
> ---
> v4: add a comment on undefined result of divide operation.
>fix if(){} coding style issue, remove blank line.
> v3: add missing divided-by-zero, divided-by-(-1) handling
> v2: fix coding style
>use Power ISA 3.1 flag
> 


Any feedback on this one?

Thanks,
Lijun

Re: [PATCH v4 10/11] target/ppc: add vmulh{su}d instructions

[Lijun Pan]

> On Jul 1, 2020, at 6:47 PM, Lijun Pan  wrote:
> 
> vmulhsd: Vector Multiply High Signed Doubleword
> vmulhud: Vector Multiply High Unsigned Doubleword
> 
> Signed-off-by: Lijun Pan 
> ---
> Reviewed-by: Richard Henderson 
> v3: simplify helper_vmulh{su}d 
> v2: fix coding style
>use Power ISA 3.1 flag
> 

any feedback on this one?