[Qemu-devel] [PATCH] hw/acpi: extract acpi_add_rom_blob()

[Wei Yang]

arm and i386 has almost the same function acpi_add_rom_blob(), except
giving different FWCfgCallback function.

This patch extract acpi_add_rom_blob() to aml-build.c by passing
FWCfgCallback to it.

Signed-off-by: Wei Yang 
---
 hw/acpi/aml-build.c |  8 
 hw/arm/virt-acpi-build.c| 24 +---
 hw/i386/acpi-build.c| 24 +---
 include/hw/acpi/aml-build.h |  4 
 4 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 555c24f21d..ed427f8310 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1559,6 +1559,14 @@ void *acpi_data_push(GArray *table_data, unsigned size)
 return table_data->data + off;
 }
 
+MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque,
+GArray *blob, const char *name,
+uint64_t max_size)
+{
+return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
+name, update, opaque, NULL, true);
+}
+
 unsigned acpi_data_len(GArray *table)
 {
 assert(g_array_get_element_size(table) == 1);
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 57679a89bf..ea4ac68b60 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -881,14 +881,6 @@ static void virt_acpi_build_reset(void *build_opaque)
 build_state->patched = false;
 }
 
-static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
-   GArray *blob, const char *name,
-   uint64_t max_size)
-{
-return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
-name, virt_acpi_build_update, build_state, NULL, true);
-}
-
 static const VMStateDescription vmstate_virt_acpi_build = {
 .name = "virt_acpi_build",
 .version_id = 1,
@@ -920,20 +912,22 @@ void virt_acpi_setup(VirtMachineState *vms)
 virt_acpi_build(vms, &tables);
 
 /* Now expose it all to Guest */
-build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
-   ACPI_BUILD_TABLE_FILE,
-   ACPI_BUILD_TABLE_MAX_SIZE);
+build_state->table_mr = acpi_add_rom_blob(virt_acpi_build_update,
+  build_state, tables.table_data,
+  ACPI_BUILD_TABLE_FILE,
+  ACPI_BUILD_TABLE_MAX_SIZE);
 assert(build_state->table_mr != NULL);
 
 build_state->linker_mr =
-acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
-  "etc/table-loader", 0);
+acpi_add_rom_blob(virt_acpi_build_update, build_state,
+  tables.linker->cmd_blob, "etc/table-loader", 0);
 
 fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
 acpi_data_len(tables.tcpalog));
 
-build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
-  ACPI_BUILD_RSDP_FILE, 0);
+build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
+ build_state, tables.rsdp,
+ ACPI_BUILD_RSDP_FILE, 0);
 
 qemu_register_reset(virt_acpi_build_reset, build_state);
 virt_acpi_build_reset(build_state);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 416da318ae..af2a944dd2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2842,14 +2842,6 @@ static void acpi_build_reset(void *build_opaque)
 build_state->patched = 0;
 }
 
-static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
-   GArray *blob, const char *name,
-   uint64_t max_size)
-{
-return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
-name, acpi_build_update, build_state, NULL, true);
-}
-
 static const VMStateDescription vmstate_acpi_build = {
 .name = "acpi_build",
 .version_id = 1,
@@ -2891,14 +2883,15 @@ void acpi_setup(void)
 acpi_build(&tables, MACHINE(pcms));
 
 /* Now expose it all to Guest */
-build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
-   ACPI_BUILD_TABLE_FILE,
-   ACPI_BUILD_TABLE_MAX_SIZE);
+build_state->table_mr = acpi_add_rom_blob(acpi_build_update,
+  build_state, tables.table_data,
+  ACPI_BUILD_TABLE_FILE,
+  ACPI_BUILD_TABLE_MAX_SIZE);
 assert(build_state->table_mr != NULL);
 
 build_state->linker_mr =
-acpi_add_rom_blob(build_

Re: [Qemu-devel] [PULL] target/riscv: Convert to decodetree

[Peter Maydell]

On Wed, 13 Mar 2019 at 14:37, Palmer Dabbelt  wrote:
>
> merged tag 'pull-request-2019-03-12'
> Primary key fingerprint: 27B8 8847 EEE0 2501 18F3  EAB9 2ED9 D774 FE70 2DB5
> The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1:
>
>   Merge remote-tracking branch 
> 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 
> 21:06:26 +)
>
> are available in the Git repository at:
>
>   git://github.com/palmer-dabbelt/qemu.git tags/riscv-for-master-4.0-sf4
>
> for you to fetch changes up to 25e6ca30c668783cd72ff97080ff44e141b99f9b:
>
>   target/riscv: Remove decode_RV32_64G() (2019-03-13 10:40:50 +0100)
Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

[Qemu-devel] [PATCH v2] hw/acpi: extract acpi_add_rom_blob()

[Wei Yang]

arm and i386 has almost the same function acpi_add_rom_blob(), except
giving different FWCfgCallback function.

This patch extract acpi_add_rom_blob() to aml-build.c by passing
FWCfgCallback to it.

Signed-off-by: Wei Yang 

---
v2:
  * remove unused header in original source file
---
 hw/acpi/aml-build.c |  8 
 hw/arm/virt-acpi-build.c| 25 +
 hw/i386/acpi-build.c| 25 +
 include/hw/acpi/aml-build.h |  4 
 4 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 555c24f21d..ed427f8310 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1559,6 +1559,14 @@ void *acpi_data_push(GArray *table_data, unsigned size)
 return table_data->data + off;
 }
 
+MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque,
+GArray *blob, const char *name,
+uint64_t max_size)
+{
+return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
+name, update, opaque, NULL, true);
+}
+
 unsigned acpi_data_len(GArray *table)
 {
 assert(g_array_get_element_size(table) == 1);
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 57679a89bf..f7fd0cf06a 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -37,7 +37,6 @@
 #include "hw/acpi/acpi.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
-#include "hw/loader.h"
 #include "hw/hw.h"
 #include "hw/acpi/aml-build.h"
 #include "hw/pci/pcie_host.h"
@@ -881,14 +880,6 @@ static void virt_acpi_build_reset(void *build_opaque)
 build_state->patched = false;
 }
 
-static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
-   GArray *blob, const char *name,
-   uint64_t max_size)
-{
-return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
-name, virt_acpi_build_update, build_state, NULL, true);
-}
-
 static const VMStateDescription vmstate_virt_acpi_build = {
 .name = "virt_acpi_build",
 .version_id = 1,
@@ -920,20 +911,22 @@ void virt_acpi_setup(VirtMachineState *vms)
 virt_acpi_build(vms, &tables);
 
 /* Now expose it all to Guest */
-build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
-   ACPI_BUILD_TABLE_FILE,
-   ACPI_BUILD_TABLE_MAX_SIZE);
+build_state->table_mr = acpi_add_rom_blob(virt_acpi_build_update,
+  build_state, tables.table_data,
+  ACPI_BUILD_TABLE_FILE,
+  ACPI_BUILD_TABLE_MAX_SIZE);
 assert(build_state->table_mr != NULL);
 
 build_state->linker_mr =
-acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
-  "etc/table-loader", 0);
+acpi_add_rom_blob(virt_acpi_build_update, build_state,
+  tables.linker->cmd_blob, "etc/table-loader", 0);
 
 fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
 acpi_data_len(tables.tcpalog));
 
-build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
-  ACPI_BUILD_RSDP_FILE, 0);
+build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
+ build_state, tables.rsdp,
+ ACPI_BUILD_RSDP_FILE, 0);
 
 qemu_register_reset(virt_acpi_build_reset, build_state);
 virt_acpi_build_reset(build_state);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 416da318ae..bc6e4b1f01 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -37,7 +37,6 @@
 #include "hw/acpi/cpu.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
-#include "hw/loader.h"
 #include "hw/isa/isa.h"
 #include "hw/block/fdc.h"
 #include "hw/acpi/memory_hotplug.h"
@@ -2842,14 +2841,6 @@ static void acpi_build_reset(void *build_opaque)
 build_state->patched = 0;
 }
 
-static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
-   GArray *blob, const char *name,
-   uint64_t max_size)
-{
-return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
-name, acpi_build_update, build_state, NULL, true);
-}
-
 static const VMStateDescription vmstate_acpi_build = {
 .name = "acpi_build",
 .version_id = 1,
@@ -2891,14 +2882,15 @@ void acpi_setup(void)
 acpi_build(&tables, MACHINE(pcms));
 
 /* Now expose it all to Guest */
-build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
-  

Re: [Qemu-devel] [PATCH v2 07/14] ui/vnc: Use qcrypto_random_bytes for make_challenge

[Gerd Hoffmann]

On Wed, Mar 13, 2019 at 09:55:19PM -0700, Richard Henderson wrote:
> Use a better interface for random numbers than rand,
> plus some useless floating point arithmetic.
> 
> Cc: Gerd Hoffmann 
> Signed-off-by: Richard Henderson 

Reviewed-by: Gerd Hoffmann 

Re: [Qemu-devel] [PATCH v2 04/13] spapr/xive: add state synchronization with KVM

[Cédric Le Goater]

On 3/14/19 3:10 AM, David Gibson wrote:
> On Mon, Mar 11, 2019 at 09:41:12PM +0100, Cédric Le Goater wrote:
>> On 2/26/19 1:01 AM, David Gibson wrote:
>>> On Fri, Feb 22, 2019 at 02:13:13PM +0100, Cédric Le Goater wrote:
 This extends the KVM XIVE device backend with 'synchronize_state'
 methods used to retrieve the state from KVM. The HW state of the
 sources, the KVM device and the thread interrupt contexts are
 collected for the monitor usage and also migration.

 These get operations rely on their KVM counterpart in the host kernel
 which acts as a proxy for OPAL, the host firmware. The set operations
 will be added for migration support later.

 Signed-off-by: Cédric Le Goater 
 ---
  include/hw/ppc/spapr_xive.h |  8 
  include/hw/ppc/xive.h   |  1 +
  hw/intc/spapr_xive.c| 17 ---
  hw/intc/spapr_xive_kvm.c| 89 +
  hw/intc/xive.c  | 10 +
  5 files changed, 118 insertions(+), 7 deletions(-)

 diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
 index 749c6cbc2c56..ebd65e7fe36b 100644
 --- a/include/hw/ppc/spapr_xive.h
 +++ b/include/hw/ppc/spapr_xive.h
 @@ -44,6 +44,13 @@ typedef struct sPAPRXive {
  void  *tm_mmap;
  } sPAPRXive;
  
 +/*
 + * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
 + * to the controller block id value. It can nevertheless be changed
 + * for testing purpose.
 + */
 +#define SPAPR_XIVE_BLOCK_ID 0x0
 +
  bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
  bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
  void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
 @@ -74,5 +81,6 @@ void kvmppc_xive_set_queue_config(sPAPRXive *xive, 
 uint8_t end_blk,
  void kvmppc_xive_get_queue_config(sPAPRXive *xive, uint8_t end_blk,
   uint32_t end_idx, XiveEND *end,
   Error **errp);
 +void kvmppc_xive_synchronize_state(sPAPRXive *xive, Error **errp);
  
  #endif /* PPC_SPAPR_XIVE_H */
 diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
 index 061d43fea24d..f3766fd881a2 100644
 --- a/include/hw/ppc/xive.h
 +++ b/include/hw/ppc/xive.h
 @@ -431,5 +431,6 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, 
 int srcno, Error **errp);
  void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp);
  void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val);
  void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp);
 +void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp);
  
  #endif /* PPC_XIVE_H */
 diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
 index 3db24391e31c..9f07567f4d78 100644
 --- a/hw/intc/spapr_xive.c
 +++ b/hw/intc/spapr_xive.c
 @@ -40,13 +40,6 @@
  
  #define SPAPR_XIVE_NVT_BASE 0x400
  
 -/*
 - * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
 - * to the controller block id value. It can nevertheless be changed
 - * for testing purpose.
 - */
 -#define SPAPR_XIVE_BLOCK_ID 0x0
 -
  /*
   * sPAPR NVT and END indexing helpers
   */
 @@ -153,6 +146,16 @@ void spapr_xive_pic_print_info(sPAPRXive *xive, 
 Monitor *mon)
  XiveSource *xsrc = &xive->source;
  int i;
  
 +if (kvm_irqchip_in_kernel()) {
 +Error *local_err = NULL;
 +
 +kvmppc_xive_synchronize_state(xive, &local_err);
 +if (local_err) {
 +error_report_err(local_err);
 +return;
 +}
 +}
 +
  monitor_printf(mon, "  LSIN PQEISN CPU/PRIO EQ\n");
  
  for (i = 0; i < xive->nr_irqs; i++) {
 diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
 index 6b50451b4f85..4b1ffb9835f9 100644
 --- a/hw/intc/spapr_xive_kvm.c
 +++ b/hw/intc/spapr_xive_kvm.c
 @@ -60,6 +60,57 @@ static void kvm_cpu_enable(CPUState *cs)
  /*
   * XIVE Thread Interrupt Management context (KVM)
   */
 +static void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp)
 +{
 +uint64_t state[4] = { 0 };
 +int ret;
 +
 +ret = kvm_get_one_reg(tctx->cs, KVM_REG_PPC_NVT_STATE, state);
 +if (ret != 0) {
 +error_setg_errno(errp, errno,
 + "XIVE: could not capture KVM state of CPU %ld",
 + kvm_arch_vcpu_id(tctx->cs));
 +return;
 +}
 +
 +/* word0 and word1 of the OS ring. */
 +*((uint64_t *) &tctx->regs[TM_QW1_OS]) = state[0];
 +
 +/*
 + * KVM also returns word2 containing the OS CAM line which is
 + * interesting 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 1/2] spapr: helper functions to get valid host fields

[Greg Kurz]

On Wed, 13 Mar 2019 18:16:18 -0300
"Maxiwell S. Garcia"  wrote:

> On Tue, Mar 12, 2019 at 11:52:24AM +0100, Greg Kurz wrote:
> 
> Hi Greg,
> 

Hi Maxiwell,

> > On Mon, 11 Mar 2019 19:57:08 -0300
> > "Maxiwell S. Garcia"  wrote:
> >   
> > > The pseries options 'host-serial' and 'host-model' accepts
> > > 'none', 'passthrough', or  content. The helper
> > > functions in this commit return a valid host field based on
> > > user options.
> > > 
> > > Signed-off-by: Maxiwell S. Garcia 
> > > ---
> > >  hw/ppc/spapr.c | 58 ++
> > >  include/hw/ppc/spapr.h |  3 +++
> > >  2 files changed, 39 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 9e01226e18..a3078f0261 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1202,6 +1202,34 @@ static void spapr_dt_chosen(sPAPRMachineState 
> > > *spapr, void *fdt)
> > >  g_free(bootlist);
> > >  }
> > >  
> > > +char *spapr_get_valid_host_serial(sPAPRMachineState *spapr)
> > > +{
> > > +char *host_serial = NULL;
> > > +if (spapr->host_serial && !g_str_equal(spapr->host_serial, "none")) {
> > > +if (g_str_equal(spapr->host_serial, "passthrough")) {
> > > +/* -M host-serial=passthrough */
> > > +kvmppc_get_host_serial(&host_serial);
> > > +} else {
> > > +host_serial = g_strdup(spapr->host_serial);
> > > +}
> > > +}
> > > +return host_serial;
> > > +}
> > > +
> > > +char *spapr_get_valid_host_model(sPAPRMachineState *spapr)
> > > +{
> > > +char *host_model = NULL;
> > > +if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
> > > +if (g_str_equal(spapr->host_model, "passthrough")) {
> > > +/* -M host-model=passthrough */
> > > +kvmppc_get_host_model(&host_model);
> > > +} else {
> > > +host_model = g_strdup(spapr->host_model);
> > > +}
> > > +}
> > > +return host_model;
> > > +}
> > > +  
> > 
> > These two functions only differ because of the host or serial wording.
> > Maybe consolidate the boiler plate to a macro ?
> >   
> 
> Do you suggest something like that?
> 
> #define SPAPR_GET_VALID_HOST_(FIELD, buf) \
> if (spapr->FIELD && !g_str_equal(spapr->FIELD, "none")) { \
> if (g_str_equal(spapr->FIELD, "passthrough")) {   \
> kvmppc_get_##FIELD(&buf); \
> } else {  \
> buf = g_strdup(spapr->FIELD); \
> } \
> } \
> 
> #define SPAPR_GET_VALID_HOST_SERIAL(buf) SPAPR_GET_VALID_HOST_(host_serial, 
> buf)
> #define SPAPR_GET_VALID_HOST_MODEL(buf)  SPAPR_GET_VALID_HOST_(host_model, 
> buf)
> 

Yeah that's the idea. I was more specifically thinking to
something like:

#define SPAPR_GET_VALID_HOST(attr)\
char *spapr_get_valid_host_##attr(sPAPRMachineState *spapr)   \
{ \
char *host_##attr = NULL; \
if (spapr->host_##attr && \
!g_str_equal(spapr->host_##attr, "none")) {   \
if (g_str_equal(spapr->host_##attr, "passthrough")) { \
kvmppc_get_host_##attr(&host_##attr); \
} else {  \
host_##attr = g_strdup(spapr->host_##attr);   \
} \
} \
return host_##attr;   \
}

SPAPR_GET_VALID_HOST(serial);
SPAPR_GET_VALID_HOST(model);

> > >  static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt)
> > >  {
> > >  /* The /hypervisor node isn't in PAPR - this is a hack to allow PR
> > > @@ -1247,30 +1275,16 @@ static void *spapr_build_fdt(sPAPRMachineState 
> > > *spapr)
> > >   * Add info to guest to indentify which host is it being run on
> > >   * and what is the uuid of the guest
> > >   */
> > > -if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
> > > -if (g_str_equal(spapr->host_model, "passthrough")) {
> > > -/* -M host-model=passthrough */
> > > -if (kvmppc_get_host_model(&buf)) {
> > > -_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
> > > -g_free(buf);
> > > -}
> > > -} else {
> > > -/* -M host-model= */
> > > -_FDT(fdt_setprop_string(fdt, 0, "host-model", 
> > > spapr->host_model));
> > > -}
> > > +buf = spapr_get_valid_host_model(spapr);
> > > +if (buf) {
> > > +_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
> > > 

[Qemu-devel] [PATCH 1/5] aspeed/timer: Fix behaviour running Linux

[Cédric Le Goater]

From: Joel Stanley 

The Linux kernel driver was updated in commit 4451d3f59f2a
("clocksource/drivers/fttmr010: Fix set_next_event handler) to fix an
issue observed on hardware:

 > RELOAD register is loaded into COUNT register when the aspeed timer
 > is enabled, which means the next event may be delayed because timer
 > interrupt won't be generated until <0x - current_count +
 > cycles>.

When running under Qemu, the system appeared "laggy". The guest is now
scheduling timer events too regularly, starving the host of CPU time.

This patch modifies the timer model to attempt to schedule the timer
expiry as the guest requests, but if we have missed the deadline we
re interrupt and try again, which allows the guest to catch up.

Provides expected behaviour with old and new guest code.

Fixes: c04bd47db6b9 ("hw/timer: Add ASPEED timer device model")
Signed-off-by: Joel Stanley 
[clg: - merged a fix from Andrew Jeffery 
"Fire interrupt on failure to meet deadline"
https://lists.ozlabs.org/pipermail/openbmc/2019-January/014641.html
  - adapted commit log
  - checkpatch fixes ]
Signed-off-by: Cédric Le Goater 
---
 hw/timer/aspeed_timer.c | 59 ++---
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 5c786e512815..9ffd8e09f670 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -109,37 +109,40 @@ static inline uint64_t calculate_time(struct AspeedTimer 
*t, uint32_t ticks)
 
 static uint64_t calculate_next(struct AspeedTimer *t)
 {
-uint64_t next = 0;
-uint32_t rate = calculate_rate(t);
+uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+uint64_t next;
 
-while (!next) {
-/* We don't know the relationship between the values in the match
- * registers, so sort using MAX/MIN/zero. We sort in that order as the
- * timer counts down to zero. */
-uint64_t seq[] = {
-calculate_time(t, MAX(t->match[0], t->match[1])),
-calculate_time(t, MIN(t->match[0], t->match[1])),
-calculate_time(t, 0),
-};
-uint64_t reload_ns;
-uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-
-if (now < seq[0]) {
-next = seq[0];
-} else if (now < seq[1]) {
-next = seq[1];
-} else if (now < seq[2]) {
-next = seq[2];
-} else if (t->reload) {
-reload_ns = muldiv64(t->reload, NANOSECONDS_PER_SECOND, rate);
-t->start = now - ((now - t->start) % reload_ns);
-} else {
-/* no reload value, return 0 */
-break;
-}
+/*
+ * We don't know the relationship between the values in the match
+ * registers, so sort using MAX/MIN/zero. We sort in that order as
+ * the timer counts down to zero.
+ */
+
+next = calculate_time(t, MAX(t->match[0], t->match[1]));
+if (now < next) {
+return next;
+}
+
+next = calculate_time(t, MIN(t->match[0], t->match[1]));
+if (now < next) {
+return next;
+}
+
+next = calculate_time(t, 0);
+if (now < next) {
+return next;
+}
+
+/* We've missed all deadlines, fire interrupt and try again */
+timer_del(&t->timer);
+
+if (timer_overflow_interrupt(t)) {
+t->level = !t->level;
+qemu_set_irq(t->irq, t->level);
 }
 
-return next;
+t->start = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+return calculate_time(t, MAX(MAX(t->match[0], t->match[1]), 0));
 }
 
 static void aspeed_timer_mod(AspeedTimer *t)
-- 
2.20.1

[Qemu-devel] [PATCH 0/5] aspeed/timer: Fix slowdowns in recent Linux

[Cédric Le Goater]

Hello,

This series was initially sent by Andrew on the OpenBMC list :

  https://lists.ozlabs.org/pipermail/openbmc/2019-January/014640.html

We had an issue with the introduction of 4451d3f59f2a
("clocksource/drivers/fttmr010: Fix set_next_event handler") in Linux
where our QEMU Aspeed machine were becoming very slow.

The series' approach is to provide back-pressure and it has shown
significant improvement on the initial behavior which became normal
again.

Sorry for this last minute fix but I think it's worth merging for all
the QEMU Aspeed machine. It's a bit late for QEMU-4.0 so that might be
4.1 material.

Thanks,

C. 

Andrew Jeffery (3):
  aspeed/timer: Status register contains reload for stopped timer
  aspeed/timer: Fix match calculations
  aspeed/timer: Provide back-pressure information for short periods

Christian Svensson (1):
  aspeed/timer: Use signed muldiv for timer resets

Joel Stanley (1):
  aspeed/timer: Fix behaviour running Linux

 include/hw/timer/aspeed_timer.h |  1 +
 hw/misc/aspeed_scu.c|  6 +++
 hw/timer/aspeed_timer.c | 81 -
 3 files changed, 57 insertions(+), 31 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 2/5] aspeed/timer: Status register contains reload for stopped timer

[Cédric Le Goater]

From: Andrew Jeffery 

>From the datasheet:

  This register stores the current status of counter #N. When timer
  enable bit TMC30[N * b] is disabled, the reload register will be
  loaded into this counter. When timer bit TMC30[N * b] is set, the
  counter will start to decrement. CPU can update this register value
  when enable bit is set.

Signed-off-by: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
---
 hw/timer/aspeed_timer.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 9ffd8e09f670..2f8522a597d3 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -187,7 +187,11 @@ static uint64_t aspeed_timer_get_value(AspeedTimer *t, int 
reg)
 
 switch (reg) {
 case TIMER_REG_STATUS:
-value = calculate_ticks(t, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+if (timer_enabled(t)) {
+value = calculate_ticks(t, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+} else {
+value = t->reload;
+}
 break;
 case TIMER_REG_RELOAD:
 value = t->reload;
-- 
2.20.1

[Qemu-devel] [PATCH 3/5] aspeed/timer: Fix match calculations

[Cédric Le Goater]

From: Andrew Jeffery 

If the match value exceeds reload then we don't want to include it in
calculations for the next event.

Signed-off-by: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
---
 hw/timer/aspeed_timer.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 2f8522a597d3..6c184572ea04 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -107,6 +107,11 @@ static inline uint64_t calculate_time(struct AspeedTimer 
*t, uint32_t ticks)
 return t->start + delta_ns;
 }
 
+static inline uint32_t calculate_match(struct AspeedTimer *t, int i)
+{
+return t->match[i] < t->reload ? t->match[i] : 0;
+}
+
 static uint64_t calculate_next(struct AspeedTimer *t)
 {
 uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
@@ -118,12 +123,12 @@ static uint64_t calculate_next(struct AspeedTimer *t)
  * the timer counts down to zero.
  */
 
-next = calculate_time(t, MAX(t->match[0], t->match[1]));
+next = calculate_time(t, MAX(calculate_match(t, 0), calculate_match(t, 
1)));
 if (now < next) {
 return next;
 }
 
-next = calculate_time(t, MIN(t->match[0], t->match[1]));
+next = calculate_time(t, MIN(calculate_match(t, 0), calculate_match(t, 
1)));
 if (now < next) {
 return next;
 }
@@ -141,8 +146,10 @@ static uint64_t calculate_next(struct AspeedTimer *t)
 qemu_set_irq(t->irq, t->level);
 }
 
+next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
 t->start = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-return calculate_time(t, MAX(MAX(t->match[0], t->match[1]), 0));
+
+return calculate_time(t, next);
 }
 
 static void aspeed_timer_mod(AspeedTimer *t)
-- 
2.20.1

[Qemu-devel] [PATCH 4/5] aspeed/timer: Provide back-pressure information for short periods

[Cédric Le Goater]

From: Andrew Jeffery 

First up: This is not the way the hardware behaves.

However, it helps resolve real-world problems with short periods being
used under Linux. Commit 4451d3f59f2a ("clocksource/drivers/fttmr010:
Fix set_next_event handler") in Linux fixed the timer driver to
correctly schedule the next event for the Aspeed controller, and in
combination with 5daa8212c08e ("ARM: dts: aspeed: Describe random number
device") Linux will now set a timer with a period as low as 1us.

Configuring a qemu timer with such a short period results in spending
time handling the interrupt in the model rather than executing guest
code, leading to noticeable "sticky" behaviour in the guest.

The behaviour of Linux is correct with respect to the hardware, so we
need to improve our handling under emulation. The approach chosen is to
provide back-pressure information by calculating an acceptable minimum
number of ticks to be set on the model. Under Linux an additional read
is added in the timer configuration path to detect back-pressure, which
will never occur on hardware. However if back-pressure is observed, the
driver alerts the clock event subsystem, which then performs its own
next event dilation via a config option - d1748302f70b ("clockevents:
Make minimum delay adjustments configurable")

A minimum period of 5us was experimentally determined on a Lenovo
T480s, which I've increased to 20us for "safety".

Signed-off-by: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
---
 include/hw/timer/aspeed_timer.h | 1 +
 hw/misc/aspeed_scu.c| 6 ++
 hw/timer/aspeed_timer.c | 6 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/hw/timer/aspeed_timer.h b/include/hw/timer/aspeed_timer.h
index 1fb949e16710..10c851ebb6d7 100644
--- a/include/hw/timer/aspeed_timer.h
+++ b/include/hw/timer/aspeed_timer.h
@@ -41,6 +41,7 @@ typedef struct AspeedTimer {
  * interrupts, signalling with both the rising and falling edge.
  */
 int32_t level;
+uint32_t min_ticks;
 uint32_t reload;
 uint32_t match[2];
 uint64_t start;
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 51498ce70be6..797a07c2c45a 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -427,6 +427,12 @@ static void aspeed_scu_realize(DeviceState *dev, Error 
**errp)
   TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE);
 
 sysbus_init_mmio(sbd, &s->iomem);
+
+/*
+ * Reset on realize to ensure the APB clock value is calculated in time for
+ * use by the timer model, which is reset before the SCU.
+ */
+aspeed_scu_reset(dev);
 }
 
 static const VMStateDescription vmstate_aspeed_scu = {
diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 6c184572ea04..9988b8fbbf17 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -259,7 +259,7 @@ static void aspeed_timer_set_value(AspeedTimerCtrlState *s, 
int timer, int reg,
 switch (reg) {
 case TIMER_REG_RELOAD:
 old_reload = t->reload;
-t->reload = value;
+t->reload = value < t->min_ticks ? t->min_ticks : value;
 
 /* If the reload value was not previously set, or zero, and
  * the current value is valid, try to start the timer if it is
@@ -311,7 +311,11 @@ static void aspeed_timer_ctrl_enable(AspeedTimer *t, bool 
enable)
 
 static void aspeed_timer_ctrl_external_clock(AspeedTimer *t, bool enable)
 {
+AspeedTimerCtrlState *s = timer_to_ctrl(t);
+uint32_t rate = enable ? TIMER_CLOCK_EXT_HZ : s->scu->apb_freq;
+
 trace_aspeed_timer_ctrl_external_clock(t->id, enable);
+t->min_ticks = muldiv64(20 * SCALE_US, rate, NANOSECONDS_PER_SECOND);
 }
 
 static void aspeed_timer_ctrl_overflow_interrupt(AspeedTimer *t, bool enable)
-- 
2.20.1

[Qemu-devel] [PATCH 5/5] aspeed/timer: Use signed muldiv for timer resets

[Cédric Le Goater]

From: Christian Svensson 

If the host decrements the counter register that results in a negative
delta. This is then passed to muldiv64 which only handles unsigned
numbers resulting in bogus results.

This fix ensures the data being operated on is signed before it is
ultimately casted to the final unsigned value.

Test case: kexec a kernel using aspeed_timer and it will freeze on the
second bootup when the kernel initializes the timer. With this patch
that no longer happens and the timer appears to run OK.

Signed-off-by: Christian Svensson 
[clg: - checkpatch fixes ]
Signed-off-by: Cédric Le Goater 
---
 hw/timer/aspeed_timer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 9988b8fbbf17..0b16eac8970c 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -275,7 +275,8 @@ static void aspeed_timer_set_value(AspeedTimerCtrlState *s, 
int timer, int reg,
 int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
now);
 uint32_t rate = calculate_rate(t);
 
-t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
+t->start = (int64_t)t->start +
+((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
 aspeed_timer_mod(t);
 }
 break;
-- 
2.20.1

Re: [Qemu-devel] [PATCH 0/5] aspeed/timer: Fix slowdowns in recent Linux

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190314084235.9887-1-...@kaod.org/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  migration/xbzrle.o
  CC  migration/postcopy-ram.o
/tmp/qemu-test/src/hw/timer/aspeed_timer.c: In function 
'aspeed_timer_set_value':
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: error: '__int128_t' 
undeclared (first use in this function); did you mean '__int128'?
 ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
   ^~
   __int128
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: note: each undeclared 
identifier is reported only once for each function it appears in
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:30: error: expected ')' before 
'delta'
 ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
 ~^
  )
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:276:22: error: unused variable 
'rate' [-Werror=unused-variable]
 uint32_t rate = calculate_rate(t);
  ^~~~
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:275:21: error: unused variable 
'delta' [-Werror=unused-variable]
 int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
now);
 ^
cc1: all warnings being treated as errors


The full log is available at
http://patchew.org/logs/20190314084235.9887-1-...@kaod.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH] qapi: fix block-latency-histogram-set description and examples

[Vladimir Sementsov-Ogievskiy]

There no @device parameter, only the @id one.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/block-core.json | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index ca684a8a04..7c1d47365d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -565,7 +565,7 @@
 #
 # Manage read, write and flush latency histograms for the device.
 #
-# If only @device parameter is specified, remove all present latency histograms
+# If only @id parameter is specified, remove all present latency histograms
 # for the device. Otherwise, add/reset some of (or all) latency histograms.
 #
 # @id: The name or QOM path of the guest device.
@@ -597,7 +597,7 @@
 # [0, 10), [10, 50), [50, 100), [100, +inf):
 #
 # -> { "execute": "block-latency-histogram-set",
-#  "arguments": { "device": "drive0",
+#  "arguments": { "id": "drive0",
 # "boundaries": [10, 50, 100] } }
 # <- { "return": {} }
 #
@@ -605,7 +605,7 @@
 # not changed (or not created):
 #
 # -> { "execute": "block-latency-histogram-set",
-#  "arguments": { "device": "drive0",
+#  "arguments": { "id": "drive0",
 # "boundaries-write": [10, 50, 100] } }
 # <- { "return": {} }
 #
@@ -614,7 +614,7 @@
 #   write: [0, 1000), [1000, 5000), [5000, +inf)
 #
 # -> { "execute": "block-latency-histogram-set",
-#  "arguments": { "device": "drive0",
+#  "arguments": { "id": "drive0",
 # "boundaries": [10, 50, 100],
 # "boundaries-write": [1000, 5000] } }
 # <- { "return": {} }
@@ -622,7 +622,7 @@
 # Example: remove all latency histograms:
 #
 # -> { "execute": "block-latency-histogram-set",
-#  "arguments": { "device": "drive0" } }
+#  "arguments": { "id": "drive0" } }
 # <- { "return": {} }
 ##
 { 'command': 'block-latency-histogram-set',
-- 
2.18.0

Re: [Qemu-devel] [PULL 03/28] qapi: move to QOM path for x-block-latency-histogram-set

[Vladimir Sementsov-Ogievskiy]

12.03.2019 20:30, Kevin Wolf wrote:
> From: Vladimir Sementsov-Ogievskiy 
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Signed-off-by: Kevin Wolf 

Not critical, but it is v4, when in v5 description and examples were fixed to 
be s/device/id.
I'll send a follow-up.

> ---
>   qapi/block-core.json |  4 ++--
>   blockdev.c   | 12 ++--
>   2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 919d0530b2..3f0a5cb1e8 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -550,7 +550,7 @@
>   # If only @device parameter is specified, remove all present latency 
> histograms
>   # for the device. Otherwise, add/reset some of (or all) latency histograms.
>   #
> -# @device: device name to set latency histogram for.
> +# @id: The name or QOM path of the guest device.
>   #
>   # @boundaries: list of interval boundary values (see description in
>   #  BlockLatencyHistogramInfo definition). If specified, all
> @@ -608,7 +608,7 @@
>   # <- { "return": {} }
>   ##
>   { 'command': 'x-block-latency-histogram-set',
> -  'data': {'device': 'str',
> +  'data': {'id': 'str',
>  '*boundaries': ['uint64'],
>  '*boundaries-read': ['uint64'],
>  '*boundaries-write': ['uint64'],
> diff --git a/blockdev.c b/blockdev.c
> index 871966ca13..850fb34c52 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -4453,21 +4453,21 @@ void qmp_x_blockdev_set_iothread(const char 
> *node_name, StrOrNull *iothread,
>   }
>   
>   void qmp_x_block_latency_histogram_set(
> -const char *device,
> +const char *id,
>   bool has_boundaries, uint64List *boundaries,
>   bool has_boundaries_read, uint64List *boundaries_read,
>   bool has_boundaries_write, uint64List *boundaries_write,
>   bool has_boundaries_flush, uint64List *boundaries_flush,
>   Error **errp)
>   {
> -BlockBackend *blk = blk_by_name(device);
> +BlockBackend *blk = qmp_get_blk(NULL, id, errp);
>   BlockAcctStats *stats;
>   int ret;
>   
>   if (!blk) {
> -error_setg(errp, "Device '%s' not found", device);
>   return;
>   }
> +
>   stats = blk_get_stats(blk);
>   
>   if (!has_boundaries && !has_boundaries_read && !has_boundaries_write &&
> @@ -4482,7 +4482,7 @@ void qmp_x_block_latency_histogram_set(
>   stats, BLOCK_ACCT_READ,
>   has_boundaries_read ? boundaries_read : boundaries);
>   if (ret) {
> -error_setg(errp, "Device '%s' set read boundaries fail", device);
> +error_setg(errp, "Device '%s' set read boundaries fail", id);
>   return;
>   }
>   }
> @@ -4492,7 +4492,7 @@ void qmp_x_block_latency_histogram_set(
>   stats, BLOCK_ACCT_WRITE,
>   has_boundaries_write ? boundaries_write : boundaries);
>   if (ret) {
> -error_setg(errp, "Device '%s' set write boundaries fail", 
> device);
> +error_setg(errp, "Device '%s' set write boundaries fail", id);
>   return;
>   }
>   }
> @@ -4502,7 +4502,7 @@ void qmp_x_block_latency_histogram_set(
>   stats, BLOCK_ACCT_FLUSH,
>   has_boundaries_flush ? boundaries_flush : boundaries);
>   if (ret) {
> -error_setg(errp, "Device '%s' set flush boundaries fail", 
> device);
> +error_setg(errp, "Device '%s' set flush boundaries fail", id);
>   return;
>   }
>   }
> 


-- 
Best regards,
Vladimir

[Qemu-devel] [PATCH] ati-vga: i2c fix

[Gerd Hoffmann]

gets radeonfb going for me, on top of your i2c patches.
---
 hw/display/ati_int.h  |  1 +
 hw/display/ati_regs.h |  1 +
 hw/display/ati.c  | 35 +++
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index 3f4a06f1e1ed..7289db206cd2 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -38,6 +38,7 @@ typedef struct ATIVGARegs {
 uint32_t crtc_ext_cntl;
 uint32_t dac_cntl;
 uint32_t gpio_vga_ddc;
+uint32_t gpio_dvi_ddc;
 uint32_t gpio_monid;
 uint32_t crtc_h_total_disp;
 uint32_t crtc_h_sync_strt_wid;
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 90384c886ecb..1ec3498b731c 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -38,6 +38,7 @@
 #define CRTC_EXT_CNTL   0x0054
 #define DAC_CNTL0x0058
 #define GPIO_VGA_DDC0x0060
+#define GPIO_DVI_DDC0x0064
 #define GPIO_MONID  0x0068
 #define I2C_CNTL_1  0x0094
 #define PALETTE_INDEX   0x00b0
diff --git a/hw/display/ati.c b/hw/display/ati.c
index e2efc6f2225e..ffced39aad9c 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -272,6 +272,9 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, 
unsigned int size)
 case GPIO_VGA_DDC:
 val = s->regs.gpio_vga_ddc;
 break;
+case GPIO_DVI_DDC:
+val = s->regs.gpio_dvi_ddc;
+break;
 case GPIO_MONID:
 val = s->regs.gpio_monid;
 break;
@@ -426,6 +429,22 @@ static inline void ati_reg_write_offs(uint32_t *reg, int 
offs,
 }
 }
 
+static uint64_t ati_i2c(bitbang_i2c_interface *i2c,
+uint64_t data)
+{
+bool clk = !(data & BIT(17));
+bool out = !(data & BIT(16));
+bool in;
+
+bitbang_i2c_set(i2c, BITBANG_I2C_SCL, clk);
+in = bitbang_i2c_set(i2c, BITBANG_I2C_SDA, out);
+
+data &= 0xf000f;
+data |= clk ? BIT(9) : 0;
+data |= in  ? BIT(8) : 0;
+return data;
+}
+
 static void ati_mm_write(void *opaque, hwaddr addr,
uint64_t data, unsigned int size)
 {
@@ -512,15 +531,15 @@ static void ati_mm_write(void *opaque, hwaddr addr,
 if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
 break;
 }
-s->regs.gpio_vga_ddc = data & 0xf000f;
-if (data & BIT(17)) {
-s->regs.gpio_monid |= !!(data & BIT(1)) << 9;
-bitbang_i2c_set(s->bbi2c, BITBANG_I2C_SCL, (data & BIT(1)) != 0);
-}
-if (data & BIT(16)) {
-s->regs.gpio_monid |= bitbang_i2c_set(s->bbi2c, BITBANG_I2C_SDA,
-  data & BIT(0)) << 8;
+#if 0
+s->regs.gpio_vga_ddc = ati_i2c(s->bbi2c, data);
+#endif
+break;
+case GPIO_DVI_DDC:
+if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
+break;
 }
+s->regs.gpio_dvi_ddc = ati_i2c(s->bbi2c, data);
 break;
 case GPIO_MONID:
 if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
-- 
2.18.1

Re: [Qemu-devel] [PATCH 5/5] aspeed/timer: Use signed muldiv for timer resets

[Cédric Le Goater]

Christian,

Could you please provide a fix for this patch ? patchew complains, see
attached log.

Thanks,

C. 

On 3/14/19 9:42 AM, Cédric Le Goater wrote:
> From: Christian Svensson 
> 
> If the host decrements the counter register that results in a negative
> delta. This is then passed to muldiv64 which only handles unsigned
> numbers resulting in bogus results.
> 
> This fix ensures the data being operated on is signed before it is
> ultimately casted to the final unsigned value.
> 
> Test case: kexec a kernel using aspeed_timer and it will freeze on the
> second bootup when the kernel initializes the timer. With this patch
> that no longer happens and the timer appears to run OK.
> 
> Signed-off-by: Christian Svensson 
> [clg: - checkpatch fixes ]
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/timer/aspeed_timer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
> index 9988b8fbbf17..0b16eac8970c 100644
> --- a/hw/timer/aspeed_timer.c
> +++ b/hw/timer/aspeed_timer.c
> @@ -275,7 +275,8 @@ static void aspeed_timer_set_value(AspeedTimerCtrlState 
> *s, int timer, int reg,
>  int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
> now);
>  uint32_t rate = calculate_rate(t);
>  
> -t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
> +t->start = (int64_t)t->start +
> +((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
>  aspeed_timer_mod(t);
>  }
>  break;
> 

--- Begin Message ---
Patchew URL: https://patchew.org/QEMU/20190314084235.9887-1-...@kaod.org/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  migration/xbzrle.o
  CC  migration/postcopy-ram.o
/tmp/qemu-test/src/hw/timer/aspeed_timer.c: In function 
'aspeed_timer_set_value':
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: error: '__int128_t' 
undeclared (first use in this function); did you mean '__int128'?
 ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
   ^~
   __int128
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: note: each undeclared 
identifier is reported only once for each function it appears in
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:30: error: expected ')' before 
'delta'
 ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
 ~^
  )
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:276:22: error: unused variable 
'rate' [-Werror=unused-variable]
 uint32_t rate = calculate_rate(t);
  ^~~~
/tmp/qemu-test/src/hw/timer/aspeed_timer.c:275:21: error: unused variable 
'delta' [-Werror=unused-variable]
 int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
now);
 ^
cc1: all warnings being treated as errors


The full log is available at
http://patchew.org/logs/20190314084235.9887-1-...@kaod.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com--- End Message ---

Re: [Qemu-devel] [PATCH] ui/cocoa: Adding cursor support

[Chen Zhang via Qemu-devel]

> On Mar 14, 2019, at 1:35 AM, BALATON Zoltan  wrote:
> 
> On Wed, 13 Mar 2019, Chen Zhang wrote:
>> I sympathize with your situation, but the things on macOS seems a little 
>> different.
>> 
>> The QEMU Cocoa UI starts in the `main` thread and detach a `qemu_main` 
>> thread which runs stuff in vl.c etc. On the contrary, QEMU with gtk and 
>> other UIs just scheduled its event loop in the qemu_main thread without 
>> detaching a dedicated thread. These two kinds of event loop(, the cocoa one 
>> and the glib one,) do not mix well.
> 
> I was following the recent ui/cocoa.m changes so I'm aware of this.
> 
>> A second problem is that recently Cocoa UI event handling routine is locking 
>> io threads with qemu_mutex_lock_iothread(), and this may delay bottom halves 
>> and subsequently the dpy_mouse_set calls, as glib_pollfds_poll() is also 
>> protected by qemu_mutex_lock_iothread().
>> 
>> My preliminary guess is that all mouse NSEvents must be queued and processed 
>> in a way not breaking any causal order.  Maybe the next NSEvent shall not be 
>> processed before the corresponding dpy_mouse_set call.
> 
> Sorry if this was not clear but the cursor change to black square problem is 
> not cocoa UI related and we've actually seen it on Linux with gtk (I think, 
> it was reported to me, not reproduced by me). Maybe I should not have mixed 
> that in here and report independently.
> 
> Based on my experience with the ati-vga model where I've implemented hardware 
> cursor both with dpy_cursor_define()/dpy_mouse_set() and with 
> cursor_invalidate()/cursor_draw_line() callbacks I've found the latter to 
> work more smoothly. I don't know why jumpy mouse pointer happens with the 
> dpy_mouse_set() way but that's what I see. With mouse drawn from screen 
> refresh the pointer may stop when guest is busy but it won't jump around 
> unlike when it's updated on the host side on register writes not tied to 
> screen refresh. I don't know how much does it relate to your case with Cocoa 
> UI but there may be some similarities that's why I've brought this up here. 
> If it's not related just ignore my comment.
For the Cocoa UI, I tried to isolate the cursor movement issue after calling 
CGWarpMouseCursorPosition(). 

In an empty Cocoa window app with a minimal overridden -sendEvent:, a minimal 
-handleEvent:, calling CGWarpMouseCursorPosition calls upon mouse-moved event 
showed the similar cursor movement issue as in QEMU.

It was suggested that Apple throttled how frequently the cursor could be moved 
in earlier version of macOS. [1][2] However none of these workaround methods I 
found worked on Mojave so far.

Best Regards,

[1] 
https://stackoverflow.com/questions/10196603/using-cgeventsourcesetlocaleventssuppressioninterval-instead-of-the-deprecated

[2] https://lists.apple.com/archives/cocoa-dev/2012/Feb/msg00372.html
> 
> Regards,
> BALATON Zoltan
> 
>> 
>>> On Mar 12, 2019, at 8:32 PM, BALATON Zoltan  wrote:
>>> 
>>> On Tue, 12 Mar 2019, Chen Zhang via Qemu-devel wrote:
 Hi,
 
 I did try to utilize NSCursor and CGWarpMouseCursorPosition API before 
 this compromise. In cocoa_mouse_set, the position of cursor should to be 
 modified, but the bottom half that called it was not scheduled on main 
 thread. UI operations have to be queued on main thread asynchronously 
 thereafter. This introduced troubles.
 
 One issue was that once CGWarpMouseCursorPosition(), which should not 
 trigger any mouse events, was called from cocoa_mouse_set(), the cursor 
 position accelerated in a positive feedback manner. This was independent 
 from the association state between mouse movement and cursor.
 
 Another issue was that the cursor moved several steps later than the Cocoa 
 mouse events.
 
 All these phenomena made me wonder if I was messing up with the host input 
 source, runloop, the bottom halves and the asynchronous changes made to 
 cursor positions.
 
 On the other hand, rendering the cursor in the window frame buffer only 
 introduce a few more dirty rectangles per second and this does not seem to 
 add up any significant amount of overhead. At least it keeps the troubles 
 above away.
>>> 
>>> We've also found similar mouse jumping problems with the ati-vga after 
>>> using define_cursor and host side cursor but also another problem where 
>>> cursor turned into a black square sometimes. I think the latter happens 
>>> when guest sets the memory offset to the cursor (which is when I call 
>>> cursor_define) but only writes cursor data there later. This works if 
>>> cursor data is read only when rendering the cursor but fails in QEMU with 
>>> cursor_define. Problem does not happen with guest_hwcursor option of 
>>> ati-vga that uses the cirrus callbacks that read cursor data when rendering 
>>> but that needs another display surface to render cursor into. I cannot fix 
>>> this by calling define_cursor on all reg

Re: [Qemu-devel] [PATCH 0/5] aspeed/timer: Fix slowdowns in recent Linux

[Cédric Le Goater]

Hello,

For the time being, we can drop patch 5 from this series. 

Thanks,

C. 

On 3/14/19 9:59 AM, no-re...@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190314084235.9887-1-...@kaod.org/
> 
> 
> 
> Hi,
> 
> This series failed the docker-mingw@fedora build test. Please find the 
> testing commands and
> their output below. If you have Docker installed, you can probably reproduce 
> it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
>   CC  migration/xbzrle.o
>   CC  migration/postcopy-ram.o
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c: In function 
> 'aspeed_timer_set_value':
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: error: '__int128_t' 
> undeclared (first use in this function); did you mean '__int128'?
>  ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
>^~
>__int128
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: note: each undeclared 
> identifier is reported only once for each function it appears in
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:30: error: expected ')' before 
> 'delta'
>  ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
>  ~^
>   )
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:276:22: error: unused variable 
> 'rate' [-Werror=unused-variable]
>  uint32_t rate = calculate_rate(t);
>   ^~~~
> /tmp/qemu-test/src/hw/timer/aspeed_timer.c:275:21: error: unused variable 
> 'delta' [-Werror=unused-variable]
>  int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
> now);
>  ^
> cc1: all warnings being treated as errors
> 
> 
> The full log is available at
> http://patchew.org/logs/20190314084235.9887-1-...@kaod.org/testing.docker-mingw@fedora/?type=message.
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-de...@redhat.com
> 

Re: [Qemu-devel] [PATCH v2] hw/acpi: extract acpi_add_rom_blob()

[Igor Mammedov]

On Thu, 14 Mar 2019 15:38:36 +0800
Wei Yang  wrote:

> arm and i386 has almost the same function acpi_add_rom_blob(), except
> giving different FWCfgCallback function.
> 
> This patch extract acpi_add_rom_blob() to aml-build.c by passing
> FWCfgCallback to it.
> 
> Signed-off-by: Wei Yang 
> 
> ---
> v2:
>   * remove unused header in original source file
> ---
>  hw/acpi/aml-build.c |  8 
>  hw/arm/virt-acpi-build.c| 25 +
>  hw/i386/acpi-build.c| 25 +
>  include/hw/acpi/aml-build.h |  4 
>  4 files changed, 30 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 555c24f21d..ed427f8310 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
I'd like to keep aml-build.c clean from not spec related code,
Lets create a separate source file for QEMU specific code, something like
hw/acpi/utils.(h|c)

otherwise patch looks fine to me


> @@ -1559,6 +1559,14 @@ void *acpi_data_push(GArray *table_data, unsigned size)
>  return table_data->data + off;
>  }
>  
> +MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque,
> +GArray *blob, const char *name,
> +uint64_t max_size)
> +{
> +return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
> +name, update, opaque, NULL, true);
> +}
> +
>  unsigned acpi_data_len(GArray *table)
>  {
>  assert(g_array_get_element_size(table) == 1);
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 57679a89bf..f7fd0cf06a 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -37,7 +37,6 @@
>  #include "hw/acpi/acpi.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/acpi/bios-linker-loader.h"
> -#include "hw/loader.h"
>  #include "hw/hw.h"
>  #include "hw/acpi/aml-build.h"
>  #include "hw/pci/pcie_host.h"
> @@ -881,14 +880,6 @@ static void virt_acpi_build_reset(void *build_opaque)
>  build_state->patched = false;
>  }
>  
> -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
> -   GArray *blob, const char *name,
> -   uint64_t max_size)
> -{
> -return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
> -name, virt_acpi_build_update, build_state, NULL, 
> true);
> -}
> -
>  static const VMStateDescription vmstate_virt_acpi_build = {
>  .name = "virt_acpi_build",
>  .version_id = 1,
> @@ -920,20 +911,22 @@ void virt_acpi_setup(VirtMachineState *vms)
>  virt_acpi_build(vms, &tables);
>  
>  /* Now expose it all to Guest */
> -build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
> -   ACPI_BUILD_TABLE_FILE,
> -   ACPI_BUILD_TABLE_MAX_SIZE);
> +build_state->table_mr = acpi_add_rom_blob(virt_acpi_build_update,
> +  build_state, tables.table_data,
> +  ACPI_BUILD_TABLE_FILE,
> +  ACPI_BUILD_TABLE_MAX_SIZE);
>  assert(build_state->table_mr != NULL);
>  
>  build_state->linker_mr =
> -acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
> -  "etc/table-loader", 0);
> +acpi_add_rom_blob(virt_acpi_build_update, build_state,
> +  tables.linker->cmd_blob, "etc/table-loader", 0);
>  
>  fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, 
> tables.tcpalog->data,
>  acpi_data_len(tables.tcpalog));
>  
> -build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
> -  ACPI_BUILD_RSDP_FILE, 0);
> +build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
> + build_state, tables.rsdp,
> + ACPI_BUILD_RSDP_FILE, 0);
>  
>  qemu_register_reset(virt_acpi_build_reset, build_state);
>  virt_acpi_build_reset(build_state);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 416da318ae..bc6e4b1f01 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -37,7 +37,6 @@
>  #include "hw/acpi/cpu.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/acpi/bios-linker-loader.h"
> -#include "hw/loader.h"
>  #include "hw/isa/isa.h"
>  #include "hw/block/fdc.h"
>  #include "hw/acpi/memory_hotplug.h"
> @@ -2842,14 +2841,6 @@ static void acpi_build_reset(void *build_opaque)
>  build_state->patched = 0;
>  }
>  
> -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
> -   GArray *blob, const char *name,
> -   uint64_t max_size)
> -{
> -return rom_add_blo

Re: [Qemu-devel] [PATCH v6 05/11] {hmp, hw/pvrdma}: Expose device internals via monitor interface

[Marcel Apfelbaum]

On 3/11/19 12:29 PM, Yuval Shaia wrote:

Allow interrogating device internals through HMP interface.
The exposed indicators can be used for troubleshooting by developers or
sysadmin.
There is no need to expose these attributes to a management system (e.x.
libvirt) because (1) most of them are not "device-management' related
info and (2) there is no guarantee the interface is stable.

Signed-off-by: Yuval Shaia 
Acked-by: Dr. David Alan Gilbert 
Acked-by: Markus Armbruster 
---
  hmp-commands-info.hx  | 14 +
  hmp.c | 27 
  hmp.h |  1 +
  hw/rdma/Makefile.objs |  2 +-
  hw/rdma/rdma.c| 30 +++
  hw/rdma/rdma_rm.c | 53 +++
  hw/rdma/rdma_rm.h |  1 +
  hw/rdma/vmw/pvrdma_main.c | 26 +++
  include/hw/rdma/rdma.h| 40 +++
  9 files changed, 193 insertions(+), 1 deletion(-)
  create mode 100644 hw/rdma/rdma.c
  create mode 100644 include/hw/rdma/rdma.h

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index cbee8b9..c59444c 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -205,6 +205,20 @@ Show PIC state.
  ETEXI
  
  {

+.name   = "rdma",
+.args_type  = "",
+.params = "",
+.help   = "show RDMA state",
+.cmd= hmp_info_rdma,
+},
+
+STEXI
+@item info rdma
+@findex info rdma
+Show RDMA state.
+ETEXI
+
+{
  .name   = "pci",
  .args_type  = "",
  .params = "",
diff --git a/hmp.c b/hmp.c
index 4a702d5..fa1e59a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -51,6 +51,7 @@
  #include "qemu/error-report.h"
  #include "exec/ramlist.h"
  #include "hw/intc/intc.h"
+#include "hw/rdma/rdma.h"
  #include "migration/snapshot.h"
  #include "migration/misc.h"
  
@@ -1013,6 +1014,32 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)

 hmp_info_pic_foreach, mon);
  }
  
+static int hmp_info_rdma_foreach(Object *obj, void *opaque)

+{
+RdmaProvider *rdma;
+RdmaProviderClass *k;
+Monitor *mon = opaque;
+
+if (object_dynamic_cast(obj, INTERFACE_RDMA_PROVIDER)) {
+rdma = RDMA_PROVIDER(obj);
+k = RDMA_PROVIDER_GET_CLASS(obj);
+if (k->print_statistics) {
+k->print_statistics(mon, rdma);
+} else {
+monitor_printf(mon, "RDMA statistics not available for %s.\n",
+   object_get_typename(obj));
+}
+}
+
+return 0;
+}
+
+void hmp_info_rdma(Monitor *mon, const QDict *qdict)
+{
+object_child_foreach_recursive(object_get_root(),
+   hmp_info_rdma_foreach, mon);
+}
+
  void hmp_info_pci(Monitor *mon, const QDict *qdict)
  {
  PciInfoList *info_list, *info;
diff --git a/hmp.h b/hmp.h
index e0f32f0..43617f2 100644
--- a/hmp.h
+++ b/hmp.h
@@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
  void hmp_info_irq(Monitor *mon, const QDict *qdict);
  void hmp_info_pic(Monitor *mon, const QDict *qdict);
+void hmp_info_rdma(Monitor *mon, const QDict *qdict);
  void hmp_info_pci(Monitor *mon, const QDict *qdict);
  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
  void hmp_info_tpm(Monitor *mon, const QDict *qdict);
diff --git a/hw/rdma/Makefile.objs b/hw/rdma/Makefile.objs
index bd36cbf..c354e60 100644
--- a/hw/rdma/Makefile.objs
+++ b/hw/rdma/Makefile.objs
@@ -1,5 +1,5 @@
  ifeq ($(CONFIG_PVRDMA),y)
-obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o
+obj-$(CONFIG_PCI) += rdma_utils.o rdma_backend.o rdma_rm.o rdma.o
  obj-$(CONFIG_PCI) += vmw/pvrdma_dev_ring.o vmw/pvrdma_cmd.o \
   vmw/pvrdma_qp_ops.o vmw/pvrdma_main.o
  endif
diff --git a/hw/rdma/rdma.c b/hw/rdma/rdma.c
new file mode 100644
index 000..7bec0d0
--- /dev/null
+++ b/hw/rdma/rdma.c
@@ -0,0 +1,30 @@
+/*
+ * RDMA device interface
+ *
+ * Copyright (C) 2018 Oracle
+ * Copyright (C) 2018 Red Hat Inc
+ *
+ * Authors:
+ * Yuval Shaia 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/rdma/rdma.h"
+#include "qemu/module.h"
+
+static const TypeInfo rdma_hmp_info = {
+.name = INTERFACE_RDMA_PROVIDER,
+.parent = TYPE_INTERFACE,
+.class_size = sizeof(RdmaProviderClass),
+};
+
+static void rdma_register_types(void)
+{
+type_register_static(&rdma_hmp_info);
+}
+
+type_init(rdma_register_types)
diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 35fa1ab..b50e192 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -16,6 +16,7 @@
  #include "qemu/osdep.h"
  #include "qapi/error.h"
  #include "cpu.h"
+#include "monitor/monitor.h"
  
  #include "trace.h"

  #include "rdma_utils.h"
@@ -26,6 +27,

Re: [Qemu-devel] [PATCH for-4.0] maint: Ignore built elf2dmp

[Stefano Garzarella]

On Wed, Mar 13, 2019 at 09:45:57AM -0500, Eric Blake wrote:
> Commit 3fa2d384 added a binary 'elf2dmp' but did not ignore it
> during an in-tree build.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> Yes, I know we want to get rid of in-tree builds for 4.1; but
> for 4.0, this patch may still make sense.
> 
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Stefano Garzarella 

Re: [Qemu-devel] [PATCH v6 11/11] hw/pvrdma: Provide correct value to object_get_typename

[Marcel Apfelbaum]

On 3/11/19 12:29 PM, Yuval Shaia wrote:

Use base object of PCIDevice in call to object_get_typename().

Signed-off-by: Yuval Shaia 
---
  hw/rdma/vmw/pvrdma_main.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 49bfbd6..0b46561 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -593,7 +593,7 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
  
  func0 = pci_get_function_0(pdev);

  /* Break if not vmxnet3 device in slot 0 */
-if (strcmp(object_get_typename(&func0->qdev.parent_obj), TYPE_VMXNET3)) {
+if (strcmp(object_get_typename(OBJECT(func0)), TYPE_VMXNET3)) {
  error_setg(errp, "Device on %x.0 must be %s", PCI_SLOT(pdev->devfn),
 TYPE_VMXNET3);
  return;


Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [RFC PATCH 3/3] hw/acpi: Extract build_mcfg

[Igor Mammedov]

On Wed, 13 Mar 2019 21:31:37 +
Wei Yang  wrote:

> On Wed, Mar 13, 2019 at 05:09:43PM +0100, Igor Mammedov wrote:
> >On Wed, 13 Mar 2019 13:33:59 +
> >Wei Yang  wrote:
> >  
> >> On Wed, Mar 13, 2019 at 01:23:00PM +0100, Igor Mammedov wrote:  
> >> >On Wed, 13 Mar 2019 12:42:53 +0800
> >> >Wei Yang  wrote:
> >> >
> >> >> Now we have two identical build_mcfg function.
> >> >> 
> >> >> Extract them to aml-build.c.
> >> >> 
> >> >> Signed-off-by: Wei Yang 
> >> >> ---
> >> >>  hw/acpi/aml-build.c | 30 ++
> >> >>  hw/arm/virt-acpi-build.c| 16 
> >> >>  hw/i386/acpi-build.c| 31 +--
> >> >>  include/hw/acpi/aml-build.h |  1 +
> >> >>  4 files changed, 32 insertions(+), 46 deletions(-)
> >> >> 
> >> >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> >> >> index 555c24f21d..58d3b8f31d 100644
> >> >> --- a/hw/acpi/aml-build.c
> >> >> +++ b/hw/acpi/aml-build.c
> >> >
> >> >I don't like polluting aml-build.c with PCI stuff,
> >> >we have a lot of PCI related code that needs generalizing
> >> >lets create a new file for that, something like
> >> >hw/acpi/pci.c + include/hw/acpi/pci.h
> >> >
> >> >> @@ -25,6 +25,7 @@
> >> >>  #include "qemu/bswap.h"
> >> >>  #include "qemu/bitops.h"
> >> >>  #include "sysemu/numa.h"
> >> >> +#include "hw/pci/pcie_host.h"
> >> >>  
> >> >>  static GArray *build_alloc_array(void)
> >> >>  {
> >> >> @@ -1870,3 +1871,32 @@ build_hdr:
> >> >>  build_header(linker, tbl, (void *)(tbl->data + fadt_start),
> >> >>   "FACP", tbl->len - fadt_start, f->rev, oem_id, 
> >> >> oem_table_id);
> >> >>  }
> >> >> +
> >> >> +void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo 
> >> >> *info)
> >> >> +{
> >> >> +AcpiTableMcfg *mcfg;
> >> >> +const char *sig;
> >> >> +int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
> >> >> +
> >> >> +mcfg = acpi_data_push(table_data, len);
> >> >> +mcfg->allocation[0].address = cpu_to_le64(info->mcfg_base);
> >> >> +/* Only a single allocation so no need to play with segments */
> >> >> +mcfg->allocation[0].pci_segment = cpu_to_le16(0);
> >> >> +mcfg->allocation[0].start_bus_number = 0;
> >> >> +mcfg->allocation[0].end_bus_number = 
> >> >> PCIE_MMCFG_BUS(info->mcfg_size - 1);
> >> >
> >> >> +/*
> >> >> + * MCFG is used for ECAM which can be enabled or disabled by guest.
> >> >> + * To avoid table size changes (which create migration issues),
> >> >> + * always create the table even if there are no allocations,
> >> >> + * but set the signature to a reserved value in this case.
> >> >> + * ACPI spec requires OSPMs to ignore such tables.
> >> >> + */
> >> >> +if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
> >> >> +/* Reserved signature: ignored by OSPM */
> >> >> +sig = "QEMU";
> >> >> +} else {
> >> >> +sig = "MCFG";
> >> >> +}
> >> >I'd leave these hack at acpi-build.c, just push it up call chain.
> >> 
> >> Assign sig in acpi-build.c and pass it to build_mcfg()?  
> >nope, see more below
> >
> >   
> >> >More over we don't really need it since resizeable memory region was 
> >> >introduced.
> >> >
> >> >So we need to keep table_blob size only for legacy usecase (pre resizable)
> >> >and for that just padding table_blob on required size would be sufficient,
> >> >there is no need to create dummy QEMU table.
> >> >As for newer machines (since resizeable memory region) we don't need to
> >> >do even that i.e. just skip table generation altogether if guest disabled 
> >> >it.
> >> >
> >> 
> >> I am lost at this place.
> >> 
> >> sig is a part of ACPI table header, you mean the sig is not necessary to
> >> be set in ACPI table header?
> >> 
> >> "skip table generation" means remove build_header() in build_mcfg()?  
> >I mean do not call build_mcfg() at all when you don't have to.
> >
> >And when you need to keep table_blob the same size (for old machines)
> >using acpi_data_push() to reserve space instead of build_mcfg(sig="QEMU")
> >might just work as well. it's still hack but it can live in x86 specific
> >acpi_build() keeping build_mcfg() generic.
> >  
> 
> Seems got your idea.
> 
> >As for defining what to use as criteria to decide when we need to keep
> >table_blob size the same, I don't remember history of it, so I'd suggest
> >to look at commit a1666142, study history of acpi_ram_update() and
> >legacy_acpi_table_size to figure out since which machine type one doesn't
> >have to keep table_blob size the same.
> >  
> 
> OK, let me study the history first.
> 
> BTW, the legacy here is hardware specification level or qemu software
> design level?
it's QEMU only, you need to find a version of QEMU (machine type)
which didn't have re-sizable MemoryRegion and the next version most likely
would have a knob somewhere in machine class definition saying that we don't
care about sizes anymore or

Re: [Qemu-devel] [PATCH 2/4] hw/rdma: Remove unused parameter from rdma_poll_cq()

[Marcel Apfelbaum]

On 3/13/19 10:46 AM, Kamal Heib wrote:

The 'rdma_dev_res' parameter is not used in rdma_poll_cq(), so remove it.

Signed-off-by: Kamal Heib 
---
  hw/rdma/rdma_backend.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 66185bd487b6..78bafc13642a 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -94,8 +94,7 @@ static void clean_recv_mads(RdmaBackendDev *backend_dev)
  } while (cqe_ctx_id != -ENOENT);
  }
  
-static int rdma_poll_cq(RdmaBackendDev *backend_dev,

-RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
+static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
  {
  int i, ne, total_ne = 0;
  BackendCtx *bctx;
@@ -181,7 +180,7 @@ static void *comp_handler_thread(void *arg)
  }
  
  backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;

-rdma_poll_cq(backend_dev, backend_dev->rdma_dev_res, ev_cq);
+rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
  
  ibv_ack_cq_events(ev_cq, 1);

  }
@@ -315,7 +314,7 @@ void rdma_backend_poll_cq(RdmaDeviceResources 
*rdma_dev_res, RdmaBackendCQ *cq)
  int polled;
  
  rdma_dev_res->stats.poll_cq_from_guest++;

-polled = rdma_poll_cq(cq->backend_dev, rdma_dev_res, cq->ibcq);
+polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
  if (!polled) {
  rdma_dev_res->stats.poll_cq_from_guest_empty++;
  }


Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH v6 01/11] hw/rdma: Switch to generic error reporting way

[Marcel Apfelbaum]

On 3/12/19 4:23 PM, Yuval Shaia wrote:

On Tue, Mar 12, 2019 at 02:23:22PM +0200, Kamal Heib wrote:


On 3/11/19 12:29 PM, Yuval Shaia wrote:

Utilize error_report for all pr_err calls and some pr_dbg that are
considered as errors.
For the remaining pr_dbg calls, the important ones were replaced by
trace points while other deleted.


I suggest updating the commit message to include that some of the functions got
renamed to include prefix "rdma/pvrdma" in the function name.

Marcel, in case there will be no v7, can you add this note to commit message?


Sure.

Thanks,
Marcel

[...]

Re: [Qemu-devel] [PATCH] qcow2: Fix data file error condition in qcow2_co_create()

[Stefano Garzarella]

On Wed, Mar 13, 2019 at 03:25:19PM +0100, Kevin Wolf wrote:
> We were trying to check whether bdrv_open_blockdev_ref() returned
> success, but accidentally checked the wrong variable. Spotted by
> Coverity (CID 1399703).
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/qcow2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella 

Re: [Qemu-devel] [PATCH] xen-block: Replace qdict_put_obj() by qdict_put() where appropriate

[Paul Durrant]

> -Original Message-
> From: Markus Armbruster [mailto:arm...@redhat.com]
> Sent: 13 March 2019 17:45
> To: qemu-devel@nongnu.org
> Cc: sstabell...@kernel.org; Anthony Perard ; Paul 
> Durrant
> ; xen-de...@lists.xenproject.org; 
> qemu-bl...@nongnu.org
> Subject: [PATCH] xen-block: Replace qdict_put_obj() by qdict_put() where 
> appropriate
> 
> Patch created mechanically by rerunning:
> 
> $ spatch --sp-file scripts/coccinelle/qobject.cocci \
>  --macro-file scripts/cocci-macro-file.h \
>  --dir hw/block --in-place
> 
> Signed-off-by: Markus Armbruster 

Acked-by: Paul Durrant 

> ---
>  hw/block/xen-block.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index 70fc2455e8..9c722b9b95 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -771,7 +771,7 @@ static XenBlockDrive *xen_block_drive_create(const char 
> *id,
>  QDict *cache_qdict = qdict_new();
> 
>  qdict_put_bool(cache_qdict, "direct", true);
> -qdict_put_obj(file_layer, "cache", QOBJECT(cache_qdict));
> +qdict_put(file_layer, "cache", cache_qdict);
> 
>  qdict_put_str(file_layer, "aio", "native");
>  }
> @@ -796,7 +796,7 @@ static XenBlockDrive *xen_block_drive_create(const char 
> *id,
>  qdict_put_str(driver_layer, "driver", driver);
>  g_free(driver);
> 
> -qdict_put_obj(driver_layer, "file", QOBJECT(file_layer));
> +qdict_put(driver_layer, "file", file_layer);
> 
>  g_assert(!drive->node_name);
>  drive->node_name = xen_block_blockdev_add(drive->id, driver_layer,
> --
> 2.17.2

Re: [Qemu-devel] [PATCH 3/4] hw/rdma: Use {} instead of {0}

[Marcel Apfelbaum]

On 3/13/19 10:46 AM, Kamal Heib wrote:

Signed-off-by: Kamal Heib 
---
  hw/rdma/rdma_backend.c  | 18 +-
  hw/rdma/vmw/pvrdma_cmd.c|  2 +-
  hw/rdma/vmw/pvrdma_qp_ops.c |  2 +-
  3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 78bafc13642a..55b633e451c1 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -57,7 +57,7 @@ static void dummy_comp_handler(void *ctx, struct ibv_wc *wc)
  static inline void complete_work(enum ibv_wc_status status, uint32_t 
vendor_err,
   void *ctx)
  {
-struct ibv_wc wc = {0};
+struct ibv_wc wc = {};
  
  wc.status = status;

  wc.vendor_err = vendor_err;
@@ -273,7 +273,7 @@ static void stop_backend_thread(RdmaBackendThread *thread)
  
  static void start_comp_thread(RdmaBackendDev *backend_dev)

  {
-char thread_name[THR_NAME_LEN] = {0};
+char thread_name[THR_NAME_LEN] = {};
  
  stop_backend_thread(&backend_dev->comp_thread);
  
@@ -483,7 +483,7 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,

  struct ibv_sge new_sge[MAX_SGE];
  uint32_t bctx_id;
  int rc;
-struct ibv_send_wr wr = {0}, *bad_wr;
+struct ibv_send_wr wr = {}, *bad_wr;
  
  if (!qp->ibqp) { /* This field is not initialized for QP0 and QP1 */

  if (qp_type == IBV_QPT_SMI) {
@@ -600,7 +600,7 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
  struct ibv_sge new_sge[MAX_SGE];
  uint32_t bctx_id;
  int rc;
-struct ibv_recv_wr wr = {0}, *bad_wr;
+struct ibv_recv_wr wr = {}, *bad_wr;
  
  if (!qp->ibqp) { /* This field does not get initialized for QP0 and QP1 */

  if (qp_type == IBV_QPT_SMI) {
@@ -737,7 +737,7 @@ int rdma_backend_create_qp(RdmaBackendQP *qp, uint8_t 
qp_type,
 uint32_t max_recv_wr, uint32_t max_send_sge,
 uint32_t max_recv_sge)
  {
-struct ibv_qp_init_attr attr = {0};
+struct ibv_qp_init_attr attr = {};
  
  qp->ibqp = 0;
  
@@ -782,7 +782,7 @@ int rdma_backend_create_qp(RdmaBackendQP *qp, uint8_t qp_type,

  int rdma_backend_qp_state_init(RdmaBackendDev *backend_dev, RdmaBackendQP *qp,
 uint8_t qp_type, uint32_t qkey)
  {
-struct ibv_qp_attr attr = {0};
+struct ibv_qp_attr attr = {};
  int rc, attr_mask;
  
  attr_mask = IBV_QP_STATE | IBV_QP_PKEY_INDEX | IBV_QP_PORT;

@@ -821,7 +821,7 @@ int rdma_backend_qp_state_rtr(RdmaBackendDev *backend_dev, 
RdmaBackendQP *qp,
union ibv_gid *dgid, uint32_t dqpn,
uint32_t rq_psn, uint32_t qkey, bool use_qkey)
  {
-struct ibv_qp_attr attr = {0};
+struct ibv_qp_attr attr = {};
  union ibv_gid ibv_gid = {
  .global.interface_id = dgid->global.interface_id,
  .global.subnet_prefix = dgid->global.subnet_prefix
@@ -880,7 +880,7 @@ int rdma_backend_qp_state_rtr(RdmaBackendDev *backend_dev, 
RdmaBackendQP *qp,
  int rdma_backend_qp_state_rts(RdmaBackendQP *qp, uint8_t qp_type,
uint32_t sq_psn, uint32_t qkey, bool use_qkey)
  {
-struct ibv_qp_attr attr = {0};
+struct ibv_qp_attr attr = {};
  int rc, attr_mask;
  
  attr.qp_state = IBV_QPS_RTS;

@@ -1012,7 +1012,7 @@ static void process_incoming_mad_req(RdmaBackendDev 
*backend_dev,
  complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_INV_MAD_BUFF,
bctx->up_ctx);
  } else {
-struct ibv_wc wc = {0};
+struct ibv_wc wc = {};
  memset(mad, 0, bctx->sge.length);
  build_mad_hdr((struct ibv_grh *)mad,
(union ibv_gid *)&msg->umad.hdr.addr.gid, 
&msg->hdr.sgid,
diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index 21a55e225a61..a0f2b13a2438 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -123,7 +123,7 @@ static int query_port(PVRDMADev *dev, union pvrdma_cmd_req 
*req,
  {
  struct pvrdma_cmd_query_port *cmd = &req->query_port;
  struct pvrdma_cmd_query_port_resp *resp = &rsp->query_port_resp;
-struct pvrdma_port_attr attrs = {0};
+struct pvrdma_port_attr attrs = {};
  
  if (cmd->port_num > MAX_PORTS) {

  return -EINVAL;
diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
index 508d8fca3c9b..5b9786efbe4b 100644
--- a/hw/rdma/vmw/pvrdma_qp_ops.c
+++ b/hw/rdma/vmw/pvrdma_qp_ops.c
@@ -114,7 +114,7 @@ static void pvrdma_qp_ops_comp_handler(void *ctx, struct 
ibv_wc *wc)
  
  static void complete_with_error(uint32_t vendor_err, void *ctx)

  {
-struct ibv_wc wc = {0};
+struct ibv_wc wc = {};
  
  wc.status = IBV_WC_GENERAL_ERR;

  wc.vendor_err = vendor_err;


Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH] Re-evaluate SVE vector length everytime ADDVL is executed

[Amir CHARIF]

Hello,
Thanks for your answer.

The wrong size was definitely being stored in the TB, and, it only affected 
ADDVL/RDVL/ADDPL (i.e. not all instructions are wrong). Here is what I think 
was happening:

- The kernel disables SVE in EL0 (ZEN= 01).
- When the user space application is entered, the TB containing ADDVL has its 
length set to 0 (16 bytes), as we are in EL0 (so sve_exception_el!=0), and FP 
is enabled.
- ADDVL is executed (without trapping) on the basis of the current length (16). 
(Nested function calls in the same context will cause a ton of ADDVL 
instructions to be executed with a vecsize of 16.)
- The next different SVE instruction traps as it should and the size is 
adjusted for the rest of the execution (but the stack space that was initially 
allocated is wrong so execution fails).

So another fix would be to allow these instructions to trap by calling 
sve_access_check(), as is done with the rest of SVE instructions.

I'm not that familiar with the instruction set, so could you please let me know 
if it is correct to call sve_access_check() in these instructions ?

Thanks in advance,

Best regards,
Amir 

-Message d'origine-
De : Richard Henderson  
Envoyé : mercredi 13 mars 2019 17:35
À : Amir CHARIF ; qemu-devel@nongnu.org
Cc : qemu-...@nongnu.org
Objet : Re: [Qemu-devel] [PATCH] Re-evaluate SVE vector length everytime ADDVL 
is executed

On 3/13/19 7:41 AM, Amir Charif wrote:
> In system emulation mode, the kernel may internally use 16-byte vectors.
> If this size is saved in the DisasContext before entering a userspace 
> app that uses higher SVE sizes, the wrong size may be allocated on the 
> stack resulting in corruption (segfaults in user space).
> This fix evaluates the vector size at runtime (as opposed to 
> translation time) to always allocate the correct size on the stack (when 
> ADDVL is used).

This is wrong.

In particular, if the computation of VL is wrong for ADDVL, it is wrong for 
every other SVE instruction as well.  Most of which cannot have VL computed at 
runtime like this.

That is why we break the TB at every change to VL.

Where do we "enter a userspace app" without breaking the TB and recomputing?
As far as I know this must have executed an ERET to return from EL1 to EL0, 
which most definitely happens between TBs, or else no system calls would work 
at all.

Do you have an example that provokes this failure?


r~

Re: [Qemu-devel] [PULL 04/14] audio: -audiodev command line option basic implementation

[Peter Maydell]

On Tue, 12 Mar 2019 at 07:13, Gerd Hoffmann  wrote:
>
> From: Kővágó, Zoltán 
>
> Audio drivers now get an Audiodev * as config paramters, instead of the
> global audio_option structs.  There is some code in audio/audio_legacy.c
> that converts the old environment variables to audiodev options (this
> way backends do not have to worry about legacy options).  It also
> contains a replacement of -audio-help, which prints out the equivalent
> -audiodev based config of the currently specified environment variables.

Hi; Coverity complains (CID 1399706) about this, which isn't
a change in this patch as such, but the code change has
probably caused it to reanalyze:

>
>  if (!done) {
>  driver = audio_driver_lookup("none");
> -done = !audio_driver_init(s, driver, false);
> +done = !audio_driver_init(s, driver, false, dev);

Everywhere else we call audio_driver_lookup() we check
whether the return value is NULL before using it,
but here we don't. I guess this is a false positive
because the "none" driver must always exist ?
If so, I can just silence the warning in the coverity UI.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] ppc/xics/spapr: Fix H_IPOLL implementation

[David Gibson]

On Thu, Mar 14, 2019 at 07:53:15AM +0100, Greg Kurz wrote:
> On Thu, 14 Mar 2019 07:38:55 +0100
> Cédric Le Goater  wrote:
> 
> > From: Benjamin Herrenschmidt 
> > 
> > H_IPOLL takes the CPU# of the processor to poll as an argument,
> > it doesn't operate on self.
> > 
> 
> True.

Applied to ppc-for-4.0.

> 
> > Signed-off-by: Benjamin Herrenschmidt 
> > Signed-off-by: Cédric Le Goater 
> > ---
> 
> Reviewed-by: Greg Kurz 
> 
> >  hw/intc/xics_spapr.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> > index 607e1c167ba2..9d2b8adef7c5 100644
> > --- a/hw/intc/xics_spapr.c
> > +++ b/hw/intc/xics_spapr.c
> > @@ -95,8 +95,15 @@ static target_ulong h_eoi(PowerPCCPU *cpu, 
> > SpaprMachineState *spapr,
> >  static target_ulong h_ipoll(PowerPCCPU *cpu, SpaprMachineState *spapr,
> >  target_ulong opcode, target_ulong *args)
> >  {
> > +ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
> >  uint32_t mfrr;
> > -uint32_t xirr = icp_ipoll(spapr_cpu_state(cpu)->icp, &mfrr);
> > +uint32_t xirr;
> > +
> > +if (!icp) {
> > +return H_PARAMETER;
> > +}
> > +
> > +xirr = icp_ipoll(icp, &mfrr);
> >  
> >  args[0] = xirr;
> >  args[1] = mfrr;
> 

-- 
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

Re: [Qemu-devel] [PULL 0/2] Slirp updates

[Peter Maydell]

On Wed, 13 Mar 2019 at 21:18, Samuel Thibault
 wrote:
>
> The following changes since commit cd82b1e170019c4b722ed53116ee9346315d7791:
>
>   slirp: remove empty state.h (2019-03-13 22:12:23 +0100)
>
> are available in the Git repository at:
>
>   https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 1f773d9dd3187281b29ceae38541fa2d945b1d0f:
>
>   configure: remove slirp submodule support that doesn't exist yet 
> (2019-03-13 22:15:30 +0100)
>
> 
> Slirp updates
>
> Daniel P. Berrangé (1):
>   configure: remove slirp submodule support that doesn't exist yet
>
> Marc-André Lureau (1):
>   slirp: remove empty state.h
>
Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH] configure: remove slirp submodule support that doesn't exist yet

[Stefano Garzarella]

On Wed, Mar 13, 2019 at 05:31:57PM +, Daniel P. Berrangé wrote:
> The slirp code is not yet split off into a separate repository, so
> configuring QEMU to use slirp as a submodule is premature. This
> causes the non-existant "slirp" to be requested from git when syncing
> submodules. This in turn appears to be cause of non-deterministic
> failures some developers are seeing with QEMU's submodule sync process.
> 

Fixes: 675b9b53687 "build-sys: link with slirp as an external project"

> Signed-off-by: Daniel P. Berrangé 
> ---
> 
> NB, this change should be reverted only once slirp exists in
> the .git-modules file.
> 
>  configure | 11 ++-
>  1 file changed, 2 insertions(+), 9 deletions(-)


Reviewed-by: Stefano Garzarella 

Re: [Qemu-devel] [PATCH for-4.1 0/7] Add qemu_getrandom and ARMv8.5-RNG

[Daniel P . Berrangé]

On Wed, Mar 13, 2019 at 11:35:33AM -0700, Richard Henderson wrote:
> On 3/13/19 10:57 AM, Daniel P. Berrangé wrote:
> > We already have an internal API for providing strong random bytes in
> > QEMU qcrypto_random_bytes. It is preferentially backed by gnutls or
> > gcrypt, but if those aren't built-in it falls back to a platform
> > native API like /dev/random. I've got a todo item to make that use
> > getrandom on Linux/BSD when available.
> > 
> > I don't think we should be adding a new APIs for getting random
> > numbers that aren't backed by the qcrypto_random_bytes.
> 
> That's all very well and good for one particular use case, when we really want
> random numbers.  But with -seed, we want to be able to replicate one 
> particular
> execution, with fully deterministic numbers.
>
> But certainly I can look into making the non-deterministic execution use your
> existing interface.  I simply wasn't aware of it.

The random interfact is pluggable with different impls though they are
chosen at build time currently. I guess we could provide an impl that
is backed by a deterministic source and require a special CLI option or
env var to switch to this insecure mode at runtime.

> Do you happen to know off-hand the maximum latency of the gnutls/gcrypt
> interfaces?  I do want the interface to be able to return "timed-out" in
> certain cases.  With getrandom(2) there is a handy GRND_NONBLOCK parameter 
> that
> pretty much matches exactly what I want.  With /dev/{u,}random, one would have
> to use O_NONBLOCK.  With BSD getentropy, I think you'd need an alarm to time
> out the operation (unless EIO covers all sorts of failures like empty entropy
> pool; the manual isn't clear)?

I don't think there's any documented latency rules for the library
interfaces. Most of the time I think they should have deterministic
execution time as they'll just be running a cryptographic hash
algorithm of some kind, but they might periodically re-seed entropy
from the host OS. They're really supposed to be considered a black
box from the application user POV.

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

Re: [Qemu-devel] Seccomp profile for swtpm as default

[Daniel P . Berrangé]

On Wed, Mar 13, 2019 at 03:43:13PM -0400, Stefan Berger wrote:
> Hello!
> 
>  If you have some feedback regarding a seccomp profile extension for swtpm
> for v0.2, please let me know. I created this github issue here:
> 
> 
> https://github.com/stefanberger/swtpm/issues/115
> 
> 
> Basically the choice is whether to make the creation of the seccomp profile
> a default behavior or have the caller explicitly pass the '--seccomp
> profile=default' on the command line, which then in turn would require
> libvirt for example to check whether this current version of swtpm supports
> the feature either by swtpm version or by strstr() the help page.

In QEMU we can't enable seccomp by default because its wide range of
features means any default profile would be effectively useless. Libvirt
knows that it uses a restricted set of QEMU features, so it can enable
a more useful seccomp by default.

I think swtpm won't have this complexity problem. Its functionality is
relatively narrow & well understood & so it is practical to define a
good seccomp profile & use that by default. So personally I'd merely
provide an opt-out to turn it off unless you think this is likely to
break something important.

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

Re: [Qemu-devel] [PULL 4/5] hw/display: Add basic ATI VGA emulation

[Peter Maydell]

On Mon, 11 Mar 2019 at 08:54, Gerd Hoffmann  wrote:
>
> From: BALATON Zoltan 
>
> At least two machines, the PPC mac99 and MIPS fulong2e, have an ATI
> gfx chip by default (Rage 128 Pro and M6/RV100 respectively) and
> guests running on these and the PMON2000 firmware of the fulong2e
> expect this to be available. Fortunately these are very similar chips
> so they can be mostly emulated in the same device model. This patch
> adds basic emulation of these ATI VGA chips.
>
> While this is incomplete and currently only enough to run the MIPS
> firmware and get framebuffer output with Linux, it allows the fulong2e
> board to work more like the real hardware and having it in QEMU in
> this state provides a way to experiment with it and allows others to
> contribute to improve it. It is compiled for all archs but only the
> fulong2e (which currently has no display output at all) is set to use
> it by default (in a separate patch).

Hi; Coverity points out (CID 1399700) an infinite loop here:

> +static void ati_mm_write(void *opaque, hwaddr addr,
> +   uint64_t data, unsigned int size)
> +{
> +ATIVGAState *s = opaque;
> +
> +if (addr < CUR_OFFSET || addr > CUR_CLR1 || ATI_DEBUG_HW_CURSOR) {
> +trace_ati_mm_write(size, addr, ati_reg_name(addr & ~3ULL), data);
> +}
> +switch (addr) {
> +case MM_INDEX:
> +s->regs.mm_index = data;
> +break;
> +case MM_DATA ... MM_DATA + 3:
> +/* indexed access to regs or memory */
> +if (s->regs.mm_index & BIT(31)) {
> +if (s->regs.mm_index <= s->vga.vram_size - size) {
> +int i = 0;
> +while (i < size) {
> +s->vga.vram_ptr[s->regs.mm_index + i] = data & 0xff;
> +data >>= 8;
> +}

This while loop doesn't change either 'i' or 'size' in the loop body,
so it will loop forever. Presumably we should be updating i ?

thanks
-- PMM

[Qemu-devel] [PATCH] block/io: fix bdrv_co_do_copy_on_readv error handling

[Vladimir Sementsov-Ogievskiy]

It's not safe to treat bdrv_is_allocated error as unallocated: if we
mistake we may rewrite guest data.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/io.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/io.c b/block/io.c
index 2ba603c7bc..dccad64d46 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1193,11 +1193,13 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 ret = bdrv_is_allocated(bs, cluster_offset,
 MIN(cluster_bytes, max_transfer), &pnum);
 if (ret < 0) {
-/* Safe to treat errors in querying allocation as if
- * unallocated; we'll probably fail again soon on the
- * read, but at least that will set a decent errno.
+/*
+ * We must fail here, and can't treat error as allocated or
+ * unallocated: if we mistake, it will lead to not done 
copy-on-read
+ * in first case and to rewriting guest data that is already in top
+ * layer in the second case.
  */
-pnum = MIN(cluster_bytes, max_transfer);
+goto err;
 }
 
 /* Stop at EOF if the image ends in the middle of the cluster */
@@ -1208,7 +1210,7 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 
 assert(skip_bytes < pnum);
 
-if (ret <= 0) {
+if (ret == 0) {
 /* Must copy-on-read; use the bounce buffer */
 pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
 qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
-- 
2.18.0

Re: [Qemu-devel] [PATCH] intel-iommu: optimize nodmar memory regions

[Sergio Lopez]

Paolo Bonzini writes:

> On 13/03/19 12:45, Sergio Lopez wrote:
>> 
>> Peter Xu writes:
>> 
>>> Previously we have per-device system memory aliases when DMAR is
>>> disabled by the system.  It will slow the system down if there are
>>> lots of devices especially when DMAR is disabled, because each of the
>>> aliased system address space will contain O(N) slots, and rendering
>>> such N address spaces will be O(N^2) complexity.
>>>
>>> This patch introduces a shared nodmar memory region and for each
>>> device we only create an alias to the shared memory region.  With the
>>> aliasing, QEMU memory core API will be able to detect when devices are
>>> sharing the same address space (which is the nodmar address space)
>>> when rendering the FlatViews and the total number of FlatViews can be
>>> dramatically reduced when there are a lot of devices.
>>>
>>> Suggested-by: Paolo Bonzini 
>>> Signed-off-by: Peter Xu 
>>> ---
>>>
>>> Hi, Sergio,
>>>
>>> This patch implements the optimization that Paolo proposed in the
>>> other thread.  Would you please try this patch to see whether it could
>>> help for your case?  Thanks,
>> 
>> Hi,
>> 
>> I've just gave a try and it fixes the issue here. The number of
>> FlatViews goes down from 119 to 4, and the initialization time for PCI
>> devices on the Guest is back to normal levels.
>
> You must be using "iommu=pt" then.  Can you also test performance
> without it?  It should be fine even if the number of FlatViews goes back
> to 119.

Hm... I don't have "iommu=pt" in either the Host nor the Guest (the
issue can also be perceived in SeaBIOS). After taking some traces of
what QEMU is doing during that time, I'm quite convinced the slowness
comes from having to construct that amount of FlatViews, each one with
up to 200 regions.

Thanks,
Sergio.

Re: [Qemu-devel] [PATCH 6/7] linux-user: add IBT support to x86 safe-syscall.S

[Paolo Bonzini]

On 14/03/19 00:52, Richard Henderson wrote:
> On 3/13/19 5:40 AM, Paolo Bonzini wrote:
>> Because safe-syscall.S does not go through the C compiler, the
>> .note.gnu.property note has to be added manually.  Safe syscalls do not
>> involve any indirect branch or stack unwinding, so they are trivially
>> safe for IBT or shadow stacks.
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  linux-user/host/i386/safe-syscall.inc.S   | 19 +++
>>  linux-user/host/x86_64/safe-syscall.inc.S | 19 +++
>>  2 files changed, 38 insertions(+)
> 
> I suppose it's not worth trying to share these 19 lines...

They aren't exactly shared, one is .p2align 2, the other is .p2align 3.

Paolo

> Reviewed-by: Richard Henderson 
> 
> 
> r~
> 

[Qemu-devel] [PATCH for-4.0 2/2] tests/.gitignore: ignore test-qapi-emit-events.[ch] for in-tree builds

[Stefano Garzarella]

Commit 5d75648b56e generates 'tests/test-qapi-emit-events.[ch]' but
did not ignore them for in-tree builds.

Signed-off-by: Stefano Garzarella 
---
 tests/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/.gitignore b/tests/.gitignore
index c88f8f2537..f9c0170881 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -14,6 +14,7 @@ test-*
 test-qapi-commands.[ch]
 include/test-qapi-commands-sub-module.[ch]
 test-qapi-commands-sub-sub-module.[ch]
+test-qapi-emit-events.[ch]
 test-qapi-events.[ch]
 include/test-qapi-events-sub-module.[ch]
 test-qapi-events-sub-sub-module.[ch]
-- 
2.20.1

Re: [Qemu-devel] [PULL for-4.0 0/1] Block patches

[Peter Maydell]

On Wed, 13 Mar 2019 at 11:08, Stefan Hajnoczi  wrote:
>
> The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1:
>
>   Merge remote-tracking branch 
> 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 
> 21:06:26 +)
>
> are available in the Git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to f357fcd890a8d6ced6d261338b859a41414561e9:
>
>   file-posix: add drop-cache=on|off option (2019-03-13 10:54:55 +)
>
> 
> Pull request
>
>  * Add 'drop-cache=on|off' option to file-posix.c.  The default is on.
>Disabling the option fixes a QEMU 3.0.0 performance regression when live
>migrating on the same host with cache.direct=off.
>
> 

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

[Qemu-devel] [PATCH for-4.0 0/2] Update .gitignore and tests/.gitignore for in-tree builds

[Stefano Garzarella]

This series could be useless when we will no longer support in-tree builds,
but for 4.0 I think it's useful to ignore these files.

Stefano Garzarella (2):
  .gitignore: ignore docs/built created for in-tree builds
  tests/.gitignore: ignore test-qapi-emit-events.[ch] for in-tree builds

 .gitignore   | 1 +
 tests/.gitignore | 1 +
 2 files changed, 2 insertions(+)

-- 
2.20.1

[Qemu-devel] [PATCH for-4.0 1/2] .gitignore: ignore docs/built created for in-tree builds

[Stefano Garzarella]

Commit 1290e6711 creates 'docs/built' for in-tree builds of
Sphinx manuals but did not ignore it.

Signed-off-by: Stefano Garzarella 
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 77522561b8..0adb17347f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -119,6 +119,7 @@
 /pc-bios/optionrom/kvmvapic.img
 /pc-bios/s390-ccw/s390-ccw.elf
 /pc-bios/s390-ccw/s390-ccw.img
+/docs/built
 /docs/interop/qemu-ga-qapi.texi
 /docs/interop/qemu-ga-ref.html
 /docs/interop/qemu-ga-ref.info*
-- 
2.20.1

Re: [Qemu-devel] [PATCH 3/7] configure: add CET support

[Paolo Bonzini]

On 14/03/19 01:56, Richard Henderson wrote:
> Hmm.  The gcc for aarch64 names the similar feature -mbranch-protection.  I'm
> rather annoyed that the i386 gcc folk appropriated a generic -f name without
> actually making the feature generic at the same time.

Wouldn't -fcf-protection=branch also apply to ARM BTI?  Pointer
authentication can even be enabled by default on GCC 9 if I remember
correctly, so it doesn't need an option at all.

> Thankfully the aarch64 version does not include shadow stacks, and so is less
> invasive into the normal abi -- ARM uses pointer authentication instead.

Branch target authentication should probably should be one or more
separate -fcf-protection options, but it is reasonable to make it
generic as well.

One could even implement a (much) weaker version of pointer
authentication without hardware support.  You could mangle the return
address on entry and return, for example with a XOR/XOR or ADD/SUB of a
per-thread datum, and likewise mangle function pointers with a
per-process datum or with a hash based on the function's type signature.
 Both would need debugger support, and the latter would require
modifying hand-written assembly.

Paolo

Re: [Qemu-devel] [PATCH 5/7] tcg/i386: add support for IBT

[Paolo Bonzini]

On 14/03/19 02:05, Richard Henderson wrote:
> On 3/13/19 5:40 AM, Paolo Bonzini wrote:
>> +static void tcg_out_endbr(TCGContext *s)
>> +{
>> +#if defined __CET__ && (__CET__ & 1)
>> +#ifdef __x86_64__
>> +tcg_out32(s, 0xfa1e0ff3);
>> +#else
>> +tcg_out32(s, 0xfb1e0ff3);
>> +#endif
>> +#endif
>> +}
> 
> Normally we'd use a runtime test for the feature.
> 
> Just because we compiled with CET does not mean we're running on CET-enabled
> hardware, since IIRC this is a nop otherwise.  I assume there's a cpuid/xgetbv
> bit that indicates when IBT is present and/or active?

No, there is a way to deduce whether shadow stack is active, but IBT is
only detectable by SIGSEGV'ing.  It is enabled through an MSR which of
course is not available from user space.

There is a prctl to query the state, ARCH_X86_CET_STATUS, but I'm a bit
wary of adding support for it before it hits the kernel; IBT only needs
compiler support because the instructions/prefixes are a nop if
disabled, unlike SHSTK which needs the "allocate shadow stack" prctl.
It's a small fixed cost per TB, it seemed not a big deal to me.

Paolo

Re: [Qemu-devel] [PATCH] Re-evaluate SVE vector length everytime ADDVL is executed

[Alex Bennée]

Amir CHARIF  writes:

> Hello,
> Thanks for your answer.
>
> The wrong size was definitely being stored in the TB, and, it only affected 
> ADDVL/RDVL/ADDPL (i.e. not all instructions are wrong). Here is what I think 
> was happening:
>
> - The kernel disables SVE in EL0 (ZEN= 01).
> - When the user space application is entered, the TB containing ADDVL has its 
> length set to 0 (16 bytes), as we are in EL0 (so sve_exception_el!=0), and FP 
> is enabled.
> - ADDVL is executed (without trapping) on the basis of the current
> length (16). (Nested function calls in the same context will cause a
> ton of ADDVL instructions to be executed with a vecsize of 16.)

So this looks like the error. Certainly the pseudo code says:

  CheckSVEEnabled();
  bits(64) operand1 = if n == 31 then SP[] else X[n];
  bits(64) result = operand1 + (imm * (VL DIV 8));

  if d == 31 then
  SP[] = result;
  else
  X[d] = result;

so we should trap to the kernel and we won't without sve_access_check()

> - The next different SVE instruction traps as it should and the size is 
> adjusted for the rest of the execution (but the stack space that was 
> initially allocated is wrong so execution fails).
>
> So another fix would be to allow these instructions to trap by calling 
> sve_access_check(), as is done with the rest of SVE instructions.
>
> I'm not that familiar with the instruction set, so could you please let me 
> know if it is correct to call sve_access_check() in these instructions ?
>
> Thanks in advance,
>
> Best regards,
> Amir
>
> -Message d'origine-
> De : Richard Henderson 
> Envoyé : mercredi 13 mars 2019 17:35
> À : Amir CHARIF ; qemu-devel@nongnu.org
> Cc : qemu-...@nongnu.org
> Objet : Re: [Qemu-devel] [PATCH] Re-evaluate SVE vector length everytime 
> ADDVL is executed
>
> On 3/13/19 7:41 AM, Amir Charif wrote:
>> In system emulation mode, the kernel may internally use 16-byte vectors.
>> If this size is saved in the DisasContext before entering a userspace
>> app that uses higher SVE sizes, the wrong size may be allocated on the
>> stack resulting in corruption (segfaults in user space).
>> This fix evaluates the vector size at runtime (as opposed to
>> translation time) to always allocate the correct size on the stack (when 
>> ADDVL is used).
>
> This is wrong.
>
> In particular, if the computation of VL is wrong for ADDVL, it is wrong for 
> every other SVE instruction as well.  Most of which cannot have VL computed 
> at runtime like this.
>
> That is why we break the TB at every change to VL.
>
> Where do we "enter a userspace app" without breaking the TB and recomputing?
> As far as I know this must have executed an ERET to return from EL1 to EL0, 
> which most definitely happens between TBs, or else no system calls would work 
> at all.
>
> Do you have an example that provokes this failure?
>
>
> r~


--
Alex Bennée

[Qemu-devel] why does our coverity-model.c g_strdup() say it is a size-sink?

[Peter Maydell]

Our coverity model of g_strdup() includes:
  __coverity_string_size_sink__(s);

This seems to be causing Coverity to report false positives like
CID1399705 and 1399699 where we take a string from getenv() and
pass it to g_strdup() The getenv() string is untrusted data of unknown
length, and g_strdup() being marked as a size-sink makes Coverity
think the function wants "a string of a particular size".

Markus, you wrote this model initially -- can you remember why it's
marked as a size-sink? Unfortunately I can't find any documentation
online about what the coverity model annotation here means :-(

Should we just mark up the issues as false-positives, or should
we change our model ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 7/7] coroutine-x86: add CET shadow stack support

[Paolo Bonzini]

On 14/03/19 01:04, Richard Henderson wrote:
> On 3/13/19 5:40 AM, Paolo Bonzini wrote:
>> +static bool have_cet(void)
>> +{
>> +#if defined CONFIG_CET
>> +uint64_t ssp;
>> +asm ("xor %0, %0; rdsspq %0\n" : "=rm" (ssp));
> 
> The xor is incompatible with a memory output.
> I don't think you really wanted that in the first place.
> Just use "=r".
> 
> The rest is hard to review because of ARCH_X86_CET_ALLOC_SHSTK.
> I'm surprised that a prctl actually allocates memory...

Yeah, it allocates memory and writes the top 8 bytes so that the address
can be passed to RSTORSSP.

Shadow stacks are not writable by user space, which is also why I'm
using an explicit jmp (which will be a call in the next version) in
qemu_coroutine_new.  Pushing the return address on the new coroutine's
stack, and doing a "ret" there, would fail because the return address is
not matched on the shadow stack!

Paolo

Re: [Qemu-devel] [PATCH 5/5] aspeed/timer: Use signed muldiv for timer resets

[Cédric Le Goater]

On 3/14/19 11:47 AM, Christian Svensson wrote:
> Hi all,
> 
> I have a new patch but I'm not sure how you want me to post it.
> Should I do a "PATCH v2" with a single patch and this thread as the thread ID?

I would wait for feedback from Peter first. 

Thanks,

C. 

> Thanks,
> - Chris
> 
> 
> On Thu, Mar 14, 2019 at 10:05 AM Cédric Le Goater  > wrote:
> 
> Christian,
> 
> Could you please provide a fix for this patch ? patchew complains, see
> attached log.
> 
> Thanks,
> 
> C.
> 
> On 3/14/19 9:42 AM, Cédric Le Goater wrote:
> > From: Christian Svensson  >
> >
> > If the host decrements the counter register that results in a negative
> > delta. This is then passed to muldiv64 which only handles unsigned
> > numbers resulting in bogus results.
> >
> > This fix ensures the data being operated on is signed before it is
> > ultimately casted to the final unsigned value.
> >
> > Test case: kexec a kernel using aspeed_timer and it will freeze on the
> > second bootup when the kernel initializes the timer. With this patch
> > that no longer happens and the timer appears to run OK.
> >
> > Signed-off-by: Christian Svensson  >
> > [clg: - checkpatch fixes ]
> > Signed-off-by: Cédric Le Goater mailto:c...@kaod.org>>
> > ---
> >  hw/timer/aspeed_timer.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
> > index 9988b8fbbf17..0b16eac8970c 100644
> > --- a/hw/timer/aspeed_timer.c
> > +++ b/hw/timer/aspeed_timer.c
> > @@ -275,7 +275,8 @@ static void 
> aspeed_timer_set_value(AspeedTimerCtrlState *s, int timer, int reg,
> >              int64_t delta = (int64_t) value - (int64_t) 
> calculate_ticks(t, now);
> >              uint32_t rate = calculate_rate(t);
> > 
> > -            t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
> > +            t->start = (int64_t)t->start +
> > +                ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
> >              aspeed_timer_mod(t);
> >          }
> >          break;
> >
> 

Re: [Qemu-devel] [PATCH 5/5] aspeed/timer: Use signed muldiv for timer resets

[Peter Maydell]

On Thu, 14 Mar 2019 at 08:43, Cédric Le Goater  wrote:
>
> From: Christian Svensson 
>
> If the host decrements the counter register that results in a negative
> delta. This is then passed to muldiv64 which only handles unsigned
> numbers resulting in bogus results.
>
> This fix ensures the data being operated on is signed before it is
> ultimately casted to the final unsigned value.
>
> Test case: kexec a kernel using aspeed_timer and it will freeze on the
> second bootup when the kernel initializes the timer. With this patch
> that no longer happens and the timer appears to run OK.
>
> Signed-off-by: Christian Svensson 
> [clg: - checkpatch fixes ]
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/timer/aspeed_timer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
> index 9988b8fbbf17..0b16eac8970c 100644
> --- a/hw/timer/aspeed_timer.c
> +++ b/hw/timer/aspeed_timer.c
> @@ -275,7 +275,8 @@ static void aspeed_timer_set_value(AspeedTimerCtrlState 
> *s, int timer, int reg,
>  int64_t delta = (int64_t) value - (int64_t) calculate_ticks(t, 
> now);
>  uint32_t rate = calculate_rate(t);
>
> -t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
> +t->start = (int64_t)t->start +
> +((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
>  aspeed_timer_mod(t);

You can't guarantee that __int128_t exists. If you must
use it then you need an alternate code path for the ifndef
CONFIG_INT128 case (which is going to be complicated enough
that this should really be abstracted away into a
signed version of muldiv64().)

But overall I'm a little sceptical that the aspeed timer is
really a special case that needs a signed version of this
when no other timer in the system does...

thanks
-- PMM

Re: [Qemu-devel] Seccomp profile for swtpm as default

[Stefan Berger]

On 3/14/19 5:59 AM, Daniel P. Berrangé wrote:

On Wed, Mar 13, 2019 at 03:43:13PM -0400, Stefan Berger wrote:

Hello!

  If you have some feedback regarding a seccomp profile extension for swtpm
for v0.2, please let me know. I created this github issue here:


https://github.com/stefanberger/swtpm/issues/115


Basically the choice is whether to make the creation of the seccomp profile
a default behavior or have the caller explicitly pass the '--seccomp
profile=default' on the command line, which then in turn would require
libvirt for example to check whether this current version of swtpm supports
the feature either by swtpm version or by strstr() the help page.

In QEMU we can't enable seccomp by default because its wide range of
features means any default profile would be effectively useless. Libvirt
knows that it uses a restricted set of QEMU features, so it can enable
a more useful seccomp by default.

I think swtpm won't have this complexity problem. Its functionality is
relatively narrow & well understood & so it is practical to define a
good seccomp profile & use that by default. So personally I'd merely
provide an opt-out to turn it off unless you think this is likely to
break something important.


Thanks. What makes me a bit hesitant now to activate it is the fact that 
I found several unexpected syscalls in places like openssl DRBG and 
glibc. The latter for example uses some #define ARCH_FORK (iirc), which 
was not fork or vfork but clone on F29. It will obviously break if it is 
something different on another machine or if it runs into branches that 
I haven't exercised with the limited coverage the test suite I have 
provides...


  Stefan




Regards,
Daniel

Re: [Qemu-devel] [PATCH v7 6/7] vhost-user-blk: Add support to reconnect backend

[Michael S. Tsirkin]

On Wed, Mar 13, 2019 at 10:05:51AM +0800, Yongji Xie wrote:
> On Wed, 13 Mar 2019 at 00:49, Michael S. Tsirkin  wrote:
> >
> > On Thu, Feb 28, 2019 at 04:53:54PM +0800, elohi...@gmail.com wrote:
> > > From: Xie Yongji 
> > >
> > > Since we now support the message VHOST_USER_GET_INFLIGHT_FD
> > > and VHOST_USER_SET_INFLIGHT_FD. The backend is able to restart
> > > safely because it can track inflight I/O in shared memory.
> > > This patch allows qemu to reconnect the backend after
> > > connection closed.
> > >
> > > Signed-off-by: Xie Yongji 
> > > Signed-off-by: Ni Xun 
> > > Signed-off-by: Zhang Yu 
> > > ---
> > >  hw/block/vhost-user-blk.c  | 205 +++--
> > >  include/hw/virtio/vhost-user-blk.h |   4 +
> > >  2 files changed, 167 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index 9682df1a7b..539ea2e571 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -103,7 +103,7 @@ const VhostDevConfigOps blk_ops = {
> > >  .vhost_dev_config_notifier = vhost_user_blk_handle_config_change,
> > >  };
> > >
> > > -static void vhost_user_blk_start(VirtIODevice *vdev)
> > > +static int vhost_user_blk_start(VirtIODevice *vdev)
> > >  {
> > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > >  BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> > > @@ -112,13 +112,13 @@ static void vhost_user_blk_start(VirtIODevice *vdev)
> > >
> > >  if (!k->set_guest_notifiers) {
> > >  error_report("binding does not support guest notifiers");
> > > -return;
> > > +return -ENOSYS;
> > >  }
> > >
> > >  ret = vhost_dev_enable_notifiers(&s->dev, vdev);
> > >  if (ret < 0) {
> > >  error_report("Error enabling host notifiers: %d", -ret);
> > > -return;
> > > +return ret;
> > >  }
> > >
> > >  ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, true);
> > > @@ -157,12 +157,13 @@ static void vhost_user_blk_start(VirtIODevice *vdev)
> > >  vhost_virtqueue_mask(&s->dev, vdev, i, false);
> > >  }
> > >
> > > -return;
> > > +return ret;
> > >
> > >  err_guest_notifiers:
> > >  k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
> > >  err_host_notifiers:
> > >  vhost_dev_disable_notifiers(&s->dev, vdev);
> > > +return ret;
> > >  }
> > >
> > >  static void vhost_user_blk_stop(VirtIODevice *vdev)
> > > @@ -181,7 +182,6 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
> > >  ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
> > >  if (ret < 0) {
> > >  error_report("vhost guest notifier cleanup failed: %d", ret);
> > > -return;
> > >  }
> > >
> > >  vhost_dev_disable_notifiers(&s->dev, vdev);
> > > @@ -191,21 +191,43 @@ static void vhost_user_blk_set_status(VirtIODevice 
> > > *vdev, uint8_t status)
> > >  {
> > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > >  bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
> > > +int ret;
> > >
> > >  if (!vdev->vm_running) {
> > >  should_start = false;
> > >  }
> > >
> > > -if (s->dev.started == should_start) {
> > > +if (s->should_start == should_start) {
> > > +return;
> > > +}
> > > +
> > > +if (!s->connected || s->dev.started == should_start) {
> > > +s->should_start = should_start;
> > >  return;
> > >  }
> > >
> > >  if (should_start) {
> > > -vhost_user_blk_start(vdev);
> > > +s->should_start = true;
> > > +/*
> > > + * make sure vhost_user_blk_handle_output() ignores fake
> > > + * guest kick by vhost_dev_enable_notifiers()
> > > + */
> > > +barrier();
> > > +ret = vhost_user_blk_start(vdev);
> > > +if (ret < 0) {
> > > +error_report("vhost-user-blk: vhost start failed: %s",
> > > + strerror(-ret));
> > > +qemu_chr_fe_disconnect(&s->chardev);
> > > +}
> > >  } else {
> > >  vhost_user_blk_stop(vdev);
> > > +/*
> > > + * make sure vhost_user_blk_handle_output() ignore fake
> > > + * guest kick by vhost_dev_disable_notifiers()
> > > + */
> > > +barrier();
> > > +s->should_start = false;
> > >  }
> > > -
> > >  }
> > >
> > >  static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
> > > @@ -237,13 +259,22 @@ static uint64_t 
> > > vhost_user_blk_get_features(VirtIODevice *vdev,
> > >  static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue 
> > > *vq)
> > >  {
> > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > > -int i;
> > > +int i, ret;
> > >
> > >  if (!(virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> > >  !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1))) {
> > >  return;
> > >  }
> > >
> > > +if (s->should_start) {
> > > +return;
> >

[Qemu-devel] [RFC PATCH] QEMU may write to system_memory before guest starts

[Yury Kotov]

This patch isn't intended to merge. Just to reproduce a problem.

The test for x-ignore-shread capability fails on aarch64 + tcg:
Memory content inconsistency at 44c0 first_byte = 2 last_byte = 1 current = 
d1 hit_edge = 1
Memory content inconsistency at 44c01000 first_byte = 2 last_byte = 1 current = 
77 hit_edge = 1

I expected that QEMU doesn't write to guest RAM until VM starts, but it
happens in this test. By this patch I found what causes this problem.

Backtrace:
0  0x7fb9e40affea in __memcpy_avx_unaligned () at 
../sysdeps/x86_64/multiarch/memcpy-avx-unaligned.S:118
1  0x55f4a296dd84 in address_space_write_rom_internal () at exec.c:3458
2  0x55f4a296de3a in address_space_write_rom () at exec.c:3479
3  0x55f4a2d519ff in rom_reset () at hw/core/loader.c:1101
4  0x55f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
5  0x55f4a2c90a28 in qemu_system_reset () at vl.c:1675
6  0x55f4a2c9851d in main () at vl.c:4552

To fix this particular we can skip these writes for the first system_reset
if -incoming is set. But I'm not sure how to fix this problem in general.
May be to introduce a contract which forbids to write to system_ram in such
case?

What do you think?

Signed-off-by: Yury Kotov 
---
 backends/hostmem-file.c   |  2 +-
 exec.c| 15 +--
 include/exec/cpu-common.h |  2 ++
 include/exec/memory.h |  3 +++
 include/qemu/mmap-alloc.h |  2 +-
 migration/ram.c   |  2 ++
 util/mmap-alloc.c |  6 --
 util/oslib-posix.c|  2 +-
 vl.c  |  8 
 9 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index ce54788048..146fa2bc70 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -61,7 +61,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
  name,
  backend->size, fb->align,
- (backend->share ? RAM_SHARED : 0) |
+ (backend->share ? (RAM_SHARED | 
RAM_READONLY) : 0) |
  (fb->is_pmem ? RAM_PMEM : 0),
  fb->mem_path, errp);
 g_free(name);
diff --git a/exec.c b/exec.c
index 1d4f3784d6..cd86e9b837 100644
--- a/exec.c
+++ b/exec.c
@@ -1863,7 +1863,7 @@ static void *file_ram_alloc(RAMBlock *block,
 }
 
 area = qemu_ram_mmap(fd, memory, block->mr->align,
- block->flags & RAM_SHARED);
+ block->flags & RAM_SHARED, block->flags & 
RAM_READONLY);
 if (area == MAP_FAILED) {
 error_setg_errno(errp, errno,
  "unable to map backing store for guest RAM");
@@ -2259,7 +2259,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, 
MemoryRegion *mr,
 int64_t file_size;
 
 /* Just support these ram flags by now. */
-assert((ram_flags & ~(RAM_SHARED | RAM_PMEM)) == 0);
+assert((ram_flags & ~(RAM_SHARED | RAM_PMEM | RAM_READONLY)) == 0);
 
 if (xen_enabled()) {
 error_setg(errp, "-mem-path not supported with Xen");
@@ -2485,6 +2485,17 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 }
 }
 }
+
+void qemu_ram_unprotect_all(void)
+{
+RAMBlock *block;
+rcu_read_lock();
+RAMBLOCK_FOREACH(block) {
+int ret = mprotect(block->host, block->max_length, PROT_READ | 
PROT_WRITE);
+assert(ret == 0);
+}
+rcu_read_unlock();
+}
 #endif /* !_WIN32 */
 
 /* Return a host pointer to ram allocated with qemu_ram_alloc.
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index cef8b88a2a..2ad875be06 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -63,6 +63,8 @@ typedef void CPUWriteMemoryFunc(void *opaque, hwaddr addr, 
uint32_t value);
 typedef uint32_t CPUReadMemoryFunc(void *opaque, hwaddr addr);
 
 void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
+void qemu_ram_unprotect_all(void);
+
 /* This should not be used by devices.  */
 ram_addr_t qemu_ram_addr_from_host(void *ptr);
 RAMBlock *qemu_ram_block_by_name(const char *name);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1625913f84..b1cb5a48d7 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -126,6 +126,9 @@ typedef struct IOMMUNotifier IOMMUNotifier;
 /* RAM is a persistent kind memory */
 #define RAM_PMEM (1 << 5)
 
+/* RAM is readonly*/
+#define RAM_READONLY (1 << 6)
+
 static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
IOMMUNotifierFlag flags,
hwaddr start, hwaddr end,
diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
index ef04f0ed5b..f704d9b7e3 100644
--- a/include/qemu/mmap-alloc.h
+++ b/include/qemu/mmap-alloc.h
@

Re: [Qemu-devel] [PATCH 2/2] virtio-gpu: clear command queue on reset

[Marc-André Lureau]

Hi

On Thu, Mar 14, 2019 at 7:17 AM Gerd Hoffmann  wrote:
>
> It was never correct to not clear them.  Due to commit "3912e66a3feb
> virtio-vga: fix reset." this became more obvious though.  The virtio
> rings get properly reset now, and trying to process the stale commands
> will trigger an assert in the virtio core.
>
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Marc-André Lureau 

what about the fences and related data?

> ---
>  hw/display/virtio-gpu.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index fbd8d908ad32..cdab327658d2 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1356,6 +1356,7 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
>  {
>  VirtIOGPU *g = VIRTIO_GPU(vdev);
>  struct virtio_gpu_simple_resource *res, *tmp;
> +struct virtio_gpu_ctrl_command *cmd;
>  int i;
>
>  g->enable = 0;
> @@ -1372,6 +1373,12 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
>  g->scanout[i].ds = NULL;
>  }
>
> +while (!QTAILQ_EMPTY(&g->cmdq)) {
> +cmd = QTAILQ_FIRST(&g->cmdq);
> +QTAILQ_REMOVE(&g->cmdq, cmd, next);
> +g_free(cmd);
> +}
> +
>  #ifdef CONFIG_VIRGL
>  if (g->use_virgl_renderer) {
>  if (g->renderer_blocked) {
> --
> 2.18.1
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v7 6/7] vhost-user-blk: Add support to reconnect backend

[Michael S. Tsirkin]

On Wed, Mar 13, 2019 at 10:47:08AM +0800, Yongji Xie wrote:
> On Wed, 13 Mar 2019 at 09:16, Michael S. Tsirkin  wrote:
> >
> > On Thu, Feb 28, 2019 at 04:53:54PM +0800, elohi...@gmail.com wrote:
> > > From: Xie Yongji 
> > >
> > > Since we now support the message VHOST_USER_GET_INFLIGHT_FD
> > > and VHOST_USER_SET_INFLIGHT_FD. The backend is able to restart
> > > safely because it can track inflight I/O in shared memory.
> > > This patch allows qemu to reconnect the backend after
> > > connection closed.
> > >
> > > Signed-off-by: Xie Yongji 
> > > Signed-off-by: Ni Xun 
> > > Signed-off-by: Zhang Yu 
> > > ---
> > >  hw/block/vhost-user-blk.c  | 205 +++--
> > >  include/hw/virtio/vhost-user-blk.h |   4 +
> > >  2 files changed, 167 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index 9682df1a7b..539ea2e571 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -103,7 +103,7 @@ const VhostDevConfigOps blk_ops = {
> > >  .vhost_dev_config_notifier = vhost_user_blk_handle_config_change,
> > >  };
> > >
> > > -static void vhost_user_blk_start(VirtIODevice *vdev)
> > > +static int vhost_user_blk_start(VirtIODevice *vdev)
> > >  {
> > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > >  BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> > > @@ -112,13 +112,13 @@ static void vhost_user_blk_start(VirtIODevice *vdev)
> > >
> > >  if (!k->set_guest_notifiers) {
> > >  error_report("binding does not support guest notifiers");
> > > -return;
> > > +return -ENOSYS;
> > >  }
> > >
> > >  ret = vhost_dev_enable_notifiers(&s->dev, vdev);
> > >  if (ret < 0) {
> > >  error_report("Error enabling host notifiers: %d", -ret);
> > > -return;
> > > +return ret;
> > >  }
> > >
> > >  ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, true);
> > > @@ -157,12 +157,13 @@ static void vhost_user_blk_start(VirtIODevice *vdev)
> > >  vhost_virtqueue_mask(&s->dev, vdev, i, false);
> > >  }
> > >
> > > -return;
> > > +return ret;
> > >
> > >  err_guest_notifiers:
> > >  k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
> > >  err_host_notifiers:
> > >  vhost_dev_disable_notifiers(&s->dev, vdev);
> > > +return ret;
> > >  }
> > >
> > >  static void vhost_user_blk_stop(VirtIODevice *vdev)
> > > @@ -181,7 +182,6 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
> > >  ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
> > >  if (ret < 0) {
> > >  error_report("vhost guest notifier cleanup failed: %d", ret);
> > > -return;
> > >  }
> > >
> > >  vhost_dev_disable_notifiers(&s->dev, vdev);
> > > @@ -191,21 +191,43 @@ static void vhost_user_blk_set_status(VirtIODevice 
> > > *vdev, uint8_t status)
> > >  {
> > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > >  bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
> > > +int ret;
> > >
> > >  if (!vdev->vm_running) {
> > >  should_start = false;
> > >  }
> > >
> > > -if (s->dev.started == should_start) {
> > > +if (s->should_start == should_start) {
> > > +return;
> > > +}
> > > +
> > > +if (!s->connected || s->dev.started == should_start) {
> > > +s->should_start = should_start;
> > >  return;
> > >  }
> > >
> > >  if (should_start) {
> > > -vhost_user_blk_start(vdev);
> > > +s->should_start = true;
> > > +/*
> > > + * make sure vhost_user_blk_handle_output() ignores fake
> > > + * guest kick by vhost_dev_enable_notifiers()
> > > + */
> > > +barrier();
> > > +ret = vhost_user_blk_start(vdev);
> > > +if (ret < 0) {
> > > +error_report("vhost-user-blk: vhost start failed: %s",
> > > + strerror(-ret));
> > > +qemu_chr_fe_disconnect(&s->chardev);
> > > +}
> > >  } else {
> > >  vhost_user_blk_stop(vdev);
> > > +/*
> > > + * make sure vhost_user_blk_handle_output() ignore fake
> > > + * guest kick by vhost_dev_disable_notifiers()
> > > + */
> > > +barrier();
> > > +s->should_start = false;
> > >  }
> >
> > I don't understand the comment about ignoring fake guest kicks.
> > Please add an explanation about what does barrier do here.
> 
> We may get stack like this:
> 
> vhost_user_blk_stop()
>   vhost_dev_disable_notifiers()
> virtio_bus_cleanup_host_notifier()
>   virtio_queue_host_notifier_read()
> virtio_queue_notify_vq()  /* fake guest kick */
>   vhost_user_blk_handle_output()
> 
> vhost_user_blk_handle_output() will re-start vhost-user if
> should_start is false. This is not what we expect. The same to
> vhost_dev_enable_notifiers().

Well what does a compiler barrier have to do with it?

An

Re: [Qemu-devel] [PATCH v3 0/5] Add ignore-external migration capability

[Юрий Котов]

Hi,

I've sent a mail about this problem:
[RFC PATCH] QEMU may write to system_memory before guest starts

Regards,
Yury

06.03.2019, 13:47, "Yury Kotov" :
> 05.03.2019, 21:06, "Dr. David Alan Gilbert" :
>>  * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
>>>   * Yury Kotov (yury-ko...@yandex-team.ru) wrote:
>>>   > Hi,
>>>   >
>>>   > The series adds a migration capability, which allows to skip shared RAM 
>>> blocks
>>>   > during the migration. It's useful for fast local migration. E.g. to 
>>> update QEMU
>>>   > for the running guests.
>>>
>>>   Queued
>>
>>  Hi Yury,
>>    The test is failing badly on aarch64 tcg for me (i.e. aarch64 tcg on
>>  x86); I'm going to keep it merged but keep the test disabled for now.
>>  Can you have a look to see if you can see what's going wrong?
>>
>>  Dave
>
> Hi,
>
> Yes, of course. I'll look at this.
>
> Regards,
> Yury
>
>>>   >
>>>   > Usage example:
>>>   > 1. Start source VM:
>>>   > qemu-system-x86 \
>>>   > -m 4G \
>>>   > -object 
>>> memory-backend-file,id=mem0,size=4G,share=on,mem-path=/dev/shm/mem0 \
>>>   > -numa node,memdev=mem0 \
>>>   > -qmp unix:/tmp/qemu-qmp-1.sock,server,nowait \
>>>   >
>>>   > 2. Start target VM:
>>>   > qemu-system-x86 \
>>>   > -m 4G \
>>>   > -object 
>>> memory-backend-file,id=mem0,size=4G,share=on,mem-path=/dev/shm/mem0 \
>>>   > -numa node,memdev=mem0 \
>>>   > -qmp unix:/tmp/qemu-qmp-2.sock,server,nowait \
>>>   > -incoming defer
>>>   >
>>>   > 3. Enable ignore-shared capability on both VMs:
>>>   > { "execute": "migrate-set-capabilities" , "arguments":
>>>   > { "capabilities": [ { "capability": "x-ignore-shared", "state": true } 
>>> ] } }
>>>   >
>>>   > 4. Start migration.
>>>   >
>>>   > Another use case I keep in mind is to migrate to file. Usage is very 
>>> similar.
>>>   >
>>>   > V2 to V3:
>>>   > * Split "migration: Introduce ignore-shared capability"
>>>   > * Serialize the capabilities as strings rather than as indexes
>>>   > * Don't allow to enable postcopy and ignore-shared together
>>>   > * Skip the test for OSs which don't have /dev/shm
>>>   > * Add a check whether shared RAM has been really skipped
>>>   >
>>>   > V1 to V2:
>>>   > * Keep migration stream compatibility
>>>   > * Reuse the existing code to ignore unwanted RAMBlocks
>>>   > * Add capability validation feature
>>>   > * ignore-external -> ignore-shared
>>>   >
>>>   > Regards,
>>>   > Yury
>>>   >
>>>   > Yury Kotov (5):
>>>   > exec: Change RAMBlockIterFunc definition
>>>   > migration: Introduce ignore-shared capability
>>>   > migration: Add an ability to ignore shared RAM blocks
>>>   > tests/migration-test: Add a test for ignore-shared capability
>>>   > migration: Add capabilities validation
>>>   >
>>>   > exec.c | 38 +--
>>>   > include/exec/cpu-common.h | 7 +-
>>>   > migration/migration.c | 14 
>>>   > migration/migration.h | 5 +-
>>>   > migration/postcopy-ram.c | 48 +++--
>>>   > migration/ram.c | 110 ++
>>>   > migration/rdma.c | 9 ++-
>>>   > migration/savevm.c | 137 ++
>>>   > qapi/migration.json | 5 +-
>>>   > stubs/ram-block.c | 15 +
>>>   > tests/migration-test.c | 131 +---
>>>   > util/vfio-helpers.c | 6 +-
>>>   > 12 files changed, 420 insertions(+), 105 deletions(-)
>>>   >
>>>   > --
>>>   > 2.20.1
>>>   >
>>>   >
>>>   --
>>>   Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>>  --
>>  Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] why does our coverity-model.c g_strdup() say it is a size-sink?

[Paolo Bonzini]

On 14/03/19 11:51, Peter Maydell wrote:
> Our coverity model of g_strdup() includes:
>   __coverity_string_size_sink__(s);
> 
> This seems to be causing Coverity to report false positives like
> CID1399705 and 1399699 where we take a string from getenv() and
> pass it to g_strdup() The getenv() string is untrusted data of unknown
> length, and g_strdup() being marked as a size-sink makes Coverity
> think the function wants "a string of a particular size".
> 
> Markus, you wrote this model initially -- can you remember why it's
> marked as a size-sink? Unfortunately I can't find any documentation
> online about what the coverity model annotation here means :-(

I think it means that we don't want a g_strdup that can potentially do
an unbounded allocation.

Old versions of Coverity distributed the internal models as source, but
unfortunately the new ones don't.  I would not be surprised if it was
just a cut-and-paste of the original strdup model, just with a different
marker for the g_malloc/g_free family of allocation functions.

Paolo

> Should we just mark up the issues as false-positives, or should
> we change our model ?
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v7 6/7] vhost-user-blk: Add support to reconnect backend

[Daniel P . Berrangé]

On Tue, Mar 12, 2019 at 12:49:35PM -0400, Michael S. Tsirkin wrote:
> On Thu, Feb 28, 2019 at 04:53:54PM +0800, elohi...@gmail.com wrote:
> > From: Xie Yongji 
> > 
> > Since we now support the message VHOST_USER_GET_INFLIGHT_FD
> > and VHOST_USER_SET_INFLIGHT_FD. The backend is able to restart
> > safely because it can track inflight I/O in shared memory.
> > This patch allows qemu to reconnect the backend after
> > connection closed.
> > 
> > Signed-off-by: Xie Yongji 
> > Signed-off-by: Ni Xun 
> > Signed-off-by: Zhang Yu 
> > ---
> >  hw/block/vhost-user-blk.c  | 205 +++--
> >  include/hw/virtio/vhost-user-blk.h |   4 +
> >  2 files changed, 167 insertions(+), 42 deletions(-)


> >  static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> >  {
> >  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> >  VhostUserState *user;
> > -struct vhost_virtqueue *vqs = NULL;
> >  int i, ret;
> > +Error *err = NULL;
> >  
> >  if (!s->chardev.chr) {
> >  error_setg(errp, "vhost-user-blk: chardev is mandatory");
> > @@ -312,27 +442,28 @@ static void vhost_user_blk_device_realize(DeviceState 
> > *dev, Error **errp)
> >  }
> >  
> >  s->inflight = g_new0(struct vhost_inflight, 1);
> > -
> > -s->dev.nvqs = s->num_queues;
> > -s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
> > -s->dev.vq_index = 0;
> > -s->dev.backend_features = 0;
> > -vqs = s->dev.vqs;
> > -
> > -vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > -
> > -ret = vhost_dev_init(&s->dev, s->vhost_user, VHOST_BACKEND_TYPE_USER, 
> > 0);
> > -if (ret < 0) {
> > -error_setg(errp, "vhost-user-blk: vhost initialization failed: %s",
> > -   strerror(-ret));
> > -goto virtio_err;
> > -}
> > +s->vqs = g_new(struct vhost_virtqueue, s->num_queues);
> > +s->watch = 0;
> > +s->should_start = false;
> > +s->connected = false;
> > +
> > +qemu_chr_fe_set_handlers(&s->chardev,  NULL, NULL, 
> > vhost_user_blk_event,
> > + NULL, (void *)dev, NULL, true);
> > +
> > +reconnect:
> > +do {
> > +if (qemu_chr_fe_wait_connected(&s->chardev, &err) < 0) {
> > +error_report_err(err);
> > +err = NULL;
> > +sleep(1);
> 
> Seems arbitrary. Is this basically waiting until backend will reconnect?
> Why not block until event on the fd triggers?
> 
> Also, it looks like this will just block forever with no monitor input
> and no way for user to figure out what is going on short of
> crashing QEMU.

FWIW, the current vhost-user-net device does exactly the same thing
with calling qemu_chr_fe_wait_connected during its realize() function.

Can't say I'm thrilled about the existing usage either, but I don't
see a particularly nice alternative if it isn't possible to realize
the device without having a backend available.

I don't think this an especially serious problem during cold startup
though since doesn't expect the monitor to become responsive until
the device model is fully realized & all backends connected.

The real concern is if you need to hotplug this kind of device
at runtime. In that case blocking the main loop / monitor is a
serious problem that will actively harm both the guest OS and
mgmt applications.

The vhost-user-net device will already suffer from that.

To solve the problem with hotplug, the mgmt app would need to
plug the chardev backend, then wait for it to become connected,
and only then plug the device frontend. In that case we would not
want this loop. We'd expect it to use the already connected
chardev, and fail the realize() function if the chardev has
lost its connection.

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

Re: [Qemu-devel] [PATCH v6 08/12] vhost-user: add vhost_user_input_get_config()

[Michael S. Tsirkin]

On Wed, Mar 13, 2019 at 03:27:27PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Mar 13, 2019 at 3:16 PM Michael S. Tsirkin  wrote:
> >
> > On Tue, Mar 12, 2019 at 09:19:01PM +0100, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Tue, Mar 12, 2019 at 4:49 PM Michael S. Tsirkin  
> > > wrote:
> > > >
> > > > On Fri, Mar 08, 2019 at 03:04:50PM +0100, Marc-André Lureau wrote:
> > > > > Ask vhost user input backend the list of virtio_input_config.
> > > > >
> > > > > Signed-off-by: Marc-André Lureau 
> > > > > Reviewed-by: Gerd Hoffmann 
> > > >
> > > > I was pushing this and I am puzzled now.
> > > >
> > > >
> > > > > ---
> > > > >  contrib/libvhost-user/libvhost-user.h |  1 +
> > > > >  include/hw/virtio/vhost-backend.h |  4 ++
> > > > >  hw/virtio/vhost-user.c| 60 
> > > > > +++
> > > > >  docs/interop/vhost-user.txt   |  8 
> > > > >  4 files changed, 73 insertions(+)
> > > > >
> > > > > diff --git a/contrib/libvhost-user/libvhost-user.h 
> > > > > b/contrib/libvhost-user/libvhost-user.h
> > > > > index c0133b7f3f..b0c798fa1a 100644
> > > > > --- a/contrib/libvhost-user/libvhost-user.h
> > > > > +++ b/contrib/libvhost-user/libvhost-user.h
> > > > > @@ -91,6 +91,7 @@ typedef enum VhostUserRequest {
> > > > >  VHOST_USER_POSTCOPY_ADVISE  = 28,
> > > > >  VHOST_USER_POSTCOPY_LISTEN  = 29,
> > > > >  VHOST_USER_POSTCOPY_END = 30,
> > > > > +VHOST_USER_INPUT_GET_CONFIG = 31,
> > > > >  VHOST_USER_MAX
> > > > >  } VhostUserRequest;
> > > > >
> > > > > diff --git a/include/hw/virtio/vhost-backend.h 
> > > > > b/include/hw/virtio/vhost-backend.h
> > > > > index 81283ec50f..1fca321d8a 100644
> > > > > --- a/include/hw/virtio/vhost-backend.h
> > > > > +++ b/include/hw/virtio/vhost-backend.h
> > > > > @@ -12,6 +12,7 @@
> > > > >  #define VHOST_BACKEND_H
> > > > >
> > > > >  #include "exec/memory.h"
> > > > > +#include "standard-headers/linux/virtio_input.h"
> > > > >
> > > > >  typedef enum VhostBackendType {
> > > > >  VHOST_BACKEND_TYPE_NONE = 0,
> > > > > @@ -160,4 +161,7 @@ int vhost_backend_invalidate_device_iotlb(struct 
> > > > > vhost_dev *dev,
> > > > >  int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
> > > > >struct vhost_iotlb_msg 
> > > > > *imsg);
> > > > >
> > > > > +int vhost_user_input_get_config(struct vhost_dev *dev,
> > > > > +struct virtio_input_config **config);
> > > > > +
> > > > >  #endif /* VHOST_BACKEND_H */
> > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > > index 5df73405bc..cf3fd39035 100644
> > > > > --- a/hw/virtio/vhost-user.c
> > > > > +++ b/hw/virtio/vhost-user.c
> > > > > @@ -93,6 +93,7 @@ typedef enum VhostUserRequest {
> > > > >  VHOST_USER_POSTCOPY_ADVISE  = 28,
> > > > >  VHOST_USER_POSTCOPY_LISTEN  = 29,
> > > > >  VHOST_USER_POSTCOPY_END = 30,
> > > > > +VHOST_USER_INPUT_GET_CONFIG = 31,
> > > > >  VHOST_USER_MAX
> > > > >  } VhostUserRequest;
> > > > >
> > > > > @@ -342,6 +343,65 @@ static int vhost_user_write(struct vhost_dev 
> > > > > *dev, VhostUserMsg *msg,
> > > > >  return 0;
> > > > >  }
> > > > >
> > > > > +static void *vhost_user_read_size(struct vhost_dev *dev, uint32_t 
> > > > > size)
> > > > > +{
> > > > > +struct vhost_user *u = dev->opaque;
> > > > > +CharBackend *chr = u->user->chr;
> > > > > +int r;
> > > > > +uint8_t *p = g_malloc(size);
> > > > > +
> > > > > +r = qemu_chr_fe_read_all(chr, p, size);
> > > > > +if (r != size) {
> > > > > +error_report("Failed to read msg payload."
> > > > > + " Read %d instead of %u.", r, size);
> > > > > +g_free(p);
> > > > > +return NULL;
> > > > > +}
> > > > > +
> > > > > +return p;
> > > > > +}
> > > > > +
> > > > > +int vhost_user_input_get_config(struct vhost_dev *dev,
> > > > > +struct virtio_input_config **config)
> > > > > +{
> > > > > +void *p = NULL;
> > > > > +VhostUserMsg msg = {
> > > > > +.hdr.request = VHOST_USER_INPUT_GET_CONFIG,
> > > > > +.hdr.flags = VHOST_USER_VERSION,
> > > > > +};
> > > > > +
> > > > > +if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > > > > +goto err;
> > > > > +}
> > > > > +
> > > > > +if (vhost_user_read_header(dev, &msg) < 0) {
> > > > > +goto err;
> > > > > +}
> > > > > +
> > > > > +if (msg.hdr.request != VHOST_USER_INPUT_GET_CONFIG) {
> > > > > +error_report("Received unexpected msg type. Expected %d 
> > > > > received %d",
> > > > > + VHOST_USER_INPUT_GET_CONFIG, msg.hdr.request);
> > > > > +goto err;
> > > > > +}
> > > > > +
> > > > > +if (msg.hdr.size % sizeof(struct virtio_input_config)) {
> > > > > +error_report("Invalid msg size");
> > > > > +goto err;
> > > > > +}
> > > > > +
> > > > > +p = vhost_user_read_

[Qemu-devel] About Revert this patch: arm: Allow system registers for KVM guests to be changed by QEMU code

[gengdongjiu]

 Hi   Peter/Eric,
   I think we should fix the regression issue instead of revert this patch,  I 
think the reason of
   this issue is that QEMU modified some unexpected resisters, we should find 
out.


Revert "arm: Allow system registers for KVM guests to be changed by QEMU 
code"

This reverts commit 823e1b3818f9b10b824ddcd756983b6e2fa68730,
which introduces a regression running EDK2 guest firmware
under KVM:

error: kvm run failed Function not implemented
 PC=00013f5a6208 X00=404003c4 X01=003a
X02= X03=404003c4 X04=
X05=9646 X06=00013d2ef270 X07=00013e3d1710
X08=09010755ffaf8ba8 X09=ffaf8b9cfeeb5468 X10=feeb546409010756
X11=09010757ffaf8b90 X12=feeb50680903068b X13=090306a1ffaf8bc0
X14= X15= X16=00013f872da0
X17=a6ab X18= X19=00013f5a92d0
X20=00013f5a7a78 X21=003a X22=00013f5a7ab2
X23=00013f5a92e8 X24=00013f631090 X25=0010
X26=0100 X27=00013f89501b X28=00013e3d14e0
X29=00013e3d12a0 X30=00013f5a2518  SP=00013b7be0b0
PSTATE=404003c4 -Z-- EL1t

with
[ 3507.926571] kvm [35042]: load/store instruction decoding not implemented
in the host dmesg.

Revert the change for the moment until we can investigate the
cause of the regression.

Reported-by: Eric Auger 
Signed-off-by: Peter Maydell 

Re: [Qemu-devel] [PATCH v2 09/11] hw/acpi: Add ACPI Generic Event Device Support

[Shameerali Kolothum Thodi]

Hi Eric,

> -Original Message-
> From: Auger Eric [mailto:eric.au...@redhat.com]
> Sent: 11 March 2019 20:24
> To: Shameerali Kolothum Thodi ;
> qemu-devel@nongnu.org; qemu-...@nongnu.org; imamm...@redhat.com;
> peter.mayd...@linaro.org; shannon.zha...@gmail.com;
> sa...@linux.intel.com; sebastien.bo...@intel.com
> Cc: Linuxarm ; xuwei (O) 
> Subject: Re: [PATCH v2 09/11] hw/acpi: Add ACPI Generic Event Device Support
> 
> Hi,
> 
> On 3/8/19 12:42 PM, Shameer Kolothum wrote:
> > From: Samuel Ortiz 
> >
> > The ACPI Generic Event Device (GED) is a hardware-reduced specific
> > device that handles all platform events, including the hotplug ones.
> > This patch generate the AML code that defines GEDs.
> s/generate/generates
> > Platforms need to specify their own GedEvent array to describe what kind
> > of events they want to support through GED. The build_ged_aml routine
> > takes a GedEvent array that maps a specific GED event to an IRQ number.
> > Then we use that array to build both the _CRS and the _EVT section
> > of the GED device.
> >
> > This is in preparation for making use of GED for ARM/virt
> > platform and for now supports only memory hotplug.
> >
> > Signed-off-by: Samuel Ortiz 
> > Signed-off-by: Sebastien Boeuf 
> > Signed-off-by: Shameer Kolothum 
> > ---
> >  hw/acpi/Makefile.objs |   1 +
> >  hw/acpi/ged.c | 198
> ++
> >  include/hw/acpi/ged.h |  61 
> >  3 files changed, 260 insertions(+)
> >  create mode 100644 hw/acpi/ged.c
> >  create mode 100644 include/hw/acpi/ged.h
> >
> > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> > index 2d46e37..6cf572b 100644
> > --- a/hw/acpi/Makefile.objs
> > +++ b/hw/acpi/Makefile.objs
> > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) +=
> memory_hotplug.o
> >  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
> >  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> >  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> > +common-obj-$(CONFIG_ACPI_HW_REDUCED) += ged.o
> >  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> >
> >  common-obj-y += acpi_interface.o
> > diff --git a/hw/acpi/ged.c b/hw/acpi/ged.c
> > new file mode 100644
> > index 000..5076fbc
> > --- /dev/null
> > +++ b/hw/acpi/ged.c
> > @@ -0,0 +1,198 @@
> > +/*
> > + *
> > + * Copyright (c) 2018 Intel Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "hw/acpi/ged.h"
> Add a comment giving a short description of what is the GED, available
> since 6.1 spec?

Sure.

> > +
> > +static hwaddr ged_io_base;
>  Couldn't we add a comment such as, "read by the GED _EVT AML dynamic
> method"?


Ok

> > +static uint64_t ged_read(void *opaque, hwaddr addr, unsigned size)
> > +{
> > +uint64_t val = 0;
> > +GEDState *ged_st = opaque;
> > +
> > +switch (addr) {
> > +case ACPI_GED_IRQ_SEL_OFFSET:
> > +/* Read the selector value and reset it */
> > +qemu_mutex_lock(&ged_st->lock);
> > +val = ged_st->sel;
> > +ged_st->sel = ACPI_GED_IRQ_SEL_INIT;
> using 0 instead of ACPI_GED_IRQ_SEL_INIT may be clearer?

I will check that.

> > +qemu_mutex_unlock(&ged_st->lock);
> > +break;
> > +default:
> > +break;
> > +}
> > +
> > +return val;
> > +}
> > +
> > +/* Nothing is expected to be written to the GED memory region */
> > +static void ged_write(void *opaque, hwaddr addr, uint64_t data,
> > +  unsigned int size)
> > +{
> > +}
> > +
> > +static const MemoryRegionOps ged_ops = {
> > +.read = ged_read,
> > +.write = ged_write,
> > +.endianness = DEVICE_LITTLE_ENDIAN,
> > +.valid = {
> > +.min_access_size = 4,
> > +.max_access_size = 4,
> > +},
> > +};
> > +
> > +void acpi_ged_init(MemoryRegion *as, Object *owner, GEDState *ged_st,
> > +   hwaddr base_addr, uint32_t ged_irq)
> Maybe directly pass the qemu_irq* and store it into the GEDState

Yes, I think that makes sense.

> > +{
> > +
> > +assert(!ged_io_base);
> > +
> > +ged_io_base = base_addr;
> > +ged_st->irq = ged_irq;
> > +qemu_mutex_init(&ged_st->lock);
> > +memory_region_init_io(&ged_st->io, owner, &ged_ops, ged_st,
> > +  "acpi-ged-event", ACPI_GED_REG_LEN);
> > +memory_region_add_subregion(as, base_addr, &

Re: [Qemu-devel] [PATCH v7 6/7] vhost-user-blk: Add support to reconnect backend

[Michael S. Tsirkin]

On Thu, Mar 14, 2019 at 11:24:22AM +, Daniel P. Berrangé wrote:
> On Tue, Mar 12, 2019 at 12:49:35PM -0400, Michael S. Tsirkin wrote:
> > On Thu, Feb 28, 2019 at 04:53:54PM +0800, elohi...@gmail.com wrote:
> > > From: Xie Yongji 
> > > 
> > > Since we now support the message VHOST_USER_GET_INFLIGHT_FD
> > > and VHOST_USER_SET_INFLIGHT_FD. The backend is able to restart
> > > safely because it can track inflight I/O in shared memory.
> > > This patch allows qemu to reconnect the backend after
> > > connection closed.
> > > 
> > > Signed-off-by: Xie Yongji 
> > > Signed-off-by: Ni Xun 
> > > Signed-off-by: Zhang Yu 
> > > ---
> > >  hw/block/vhost-user-blk.c  | 205 +++--
> > >  include/hw/virtio/vhost-user-blk.h |   4 +
> > >  2 files changed, 167 insertions(+), 42 deletions(-)
> 
> 
> > >  static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> > >  {
> > >  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > >  VhostUserState *user;
> > > -struct vhost_virtqueue *vqs = NULL;
> > >  int i, ret;
> > > +Error *err = NULL;
> > >  
> > >  if (!s->chardev.chr) {
> > >  error_setg(errp, "vhost-user-blk: chardev is mandatory");
> > > @@ -312,27 +442,28 @@ static void 
> > > vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> > >  }
> > >  
> > >  s->inflight = g_new0(struct vhost_inflight, 1);
> > > -
> > > -s->dev.nvqs = s->num_queues;
> > > -s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
> > > -s->dev.vq_index = 0;
> > > -s->dev.backend_features = 0;
> > > -vqs = s->dev.vqs;
> > > -
> > > -vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > > -
> > > -ret = vhost_dev_init(&s->dev, s->vhost_user, 
> > > VHOST_BACKEND_TYPE_USER, 0);
> > > -if (ret < 0) {
> > > -error_setg(errp, "vhost-user-blk: vhost initialization failed: 
> > > %s",
> > > -   strerror(-ret));
> > > -goto virtio_err;
> > > -}
> > > +s->vqs = g_new(struct vhost_virtqueue, s->num_queues);
> > > +s->watch = 0;
> > > +s->should_start = false;
> > > +s->connected = false;
> > > +
> > > +qemu_chr_fe_set_handlers(&s->chardev,  NULL, NULL, 
> > > vhost_user_blk_event,
> > > + NULL, (void *)dev, NULL, true);
> > > +
> > > +reconnect:
> > > +do {
> > > +if (qemu_chr_fe_wait_connected(&s->chardev, &err) < 0) {
> > > +error_report_err(err);
> > > +err = NULL;
> > > +sleep(1);
> > 
> > Seems arbitrary. Is this basically waiting until backend will reconnect?
> > Why not block until event on the fd triggers?
> > 
> > Also, it looks like this will just block forever with no monitor input
> > and no way for user to figure out what is going on short of
> > crashing QEMU.
> 
> FWIW, the current vhost-user-net device does exactly the same thing
> with calling qemu_chr_fe_wait_connected during its realize() function.

Hmm yes. It doesn't sleep for an arbitrary 1 sec so less of an eyesore :)

> Can't say I'm thrilled about the existing usage either, but I don't
> see a particularly nice alternative if it isn't possible to realize
> the device without having a backend available.
> 
> I don't think this an especially serious problem during cold startup
> though since doesn't expect the monitor to become responsive until
> the device model is fully realized & all backends connected.
> 
> The real concern is if you need to hotplug this kind of device
> at runtime. In that case blocking the main loop / monitor is a
> serious problem that will actively harm both the guest OS and
> mgmt applications.
> 
> The vhost-user-net device will already suffer from that.

Right.

> To solve the problem with hotplug, the mgmt app would need to
> plug the chardev backend, then wait for it to become connected,
> and only then plug the device frontend. In that case we would not
> want this loop. We'd expect it to use the already connected
> chardev, and fail the realize() function if the chardev has
> lost its connection.
> 
> Regards,
> Daniel

I think as step one we should maybe just busy-wait with no sleep(1).
But respecting reconnect would be nice too.

And I think if we can't respect nowait we should
detect it and fail, in both net and block ...


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

Re: [Qemu-devel] [PATCH v7 6/7] vhost-user-blk: Add support to reconnect backend

[Daniel P . Berrangé]

On Thu, Mar 14, 2019 at 07:34:03AM -0400, Michael S. Tsirkin wrote:
> On Thu, Mar 14, 2019 at 11:24:22AM +, Daniel P. Berrangé wrote:
> > On Tue, Mar 12, 2019 at 12:49:35PM -0400, Michael S. Tsirkin wrote:
> > > On Thu, Feb 28, 2019 at 04:53:54PM +0800, elohi...@gmail.com wrote:
> > > > From: Xie Yongji 
> > > > 
> > > > Since we now support the message VHOST_USER_GET_INFLIGHT_FD
> > > > and VHOST_USER_SET_INFLIGHT_FD. The backend is able to restart
> > > > safely because it can track inflight I/O in shared memory.
> > > > This patch allows qemu to reconnect the backend after
> > > > connection closed.
> > > > 
> > > > Signed-off-by: Xie Yongji 
> > > > Signed-off-by: Ni Xun 
> > > > Signed-off-by: Zhang Yu 
> > > > ---
> > > >  hw/block/vhost-user-blk.c  | 205 +++--
> > > >  include/hw/virtio/vhost-user-blk.h |   4 +
> > > >  2 files changed, 167 insertions(+), 42 deletions(-)
> > 
> > 
> > > >  static void vhost_user_blk_device_realize(DeviceState *dev, Error 
> > > > **errp)
> > > >  {
> > > >  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > > >  VhostUserState *user;
> > > > -struct vhost_virtqueue *vqs = NULL;
> > > >  int i, ret;
> > > > +Error *err = NULL;
> > > >  
> > > >  if (!s->chardev.chr) {
> > > >  error_setg(errp, "vhost-user-blk: chardev is mandatory");
> > > > @@ -312,27 +442,28 @@ static void 
> > > > vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> > > >  }
> > > >  
> > > >  s->inflight = g_new0(struct vhost_inflight, 1);
> > > > -
> > > > -s->dev.nvqs = s->num_queues;
> > > > -s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
> > > > -s->dev.vq_index = 0;
> > > > -s->dev.backend_features = 0;
> > > > -vqs = s->dev.vqs;
> > > > -
> > > > -vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > > > -
> > > > -ret = vhost_dev_init(&s->dev, s->vhost_user, 
> > > > VHOST_BACKEND_TYPE_USER, 0);
> > > > -if (ret < 0) {
> > > > -error_setg(errp, "vhost-user-blk: vhost initialization failed: 
> > > > %s",
> > > > -   strerror(-ret));
> > > > -goto virtio_err;
> > > > -}
> > > > +s->vqs = g_new(struct vhost_virtqueue, s->num_queues);
> > > > +s->watch = 0;
> > > > +s->should_start = false;
> > > > +s->connected = false;
> > > > +
> > > > +qemu_chr_fe_set_handlers(&s->chardev,  NULL, NULL, 
> > > > vhost_user_blk_event,
> > > > + NULL, (void *)dev, NULL, true);
> > > > +
> > > > +reconnect:
> > > > +do {
> > > > +if (qemu_chr_fe_wait_connected(&s->chardev, &err) < 0) {
> > > > +error_report_err(err);
> > > > +err = NULL;
> > > > +sleep(1);
> > > 
> > > Seems arbitrary. Is this basically waiting until backend will reconnect?
> > > Why not block until event on the fd triggers?
> > > 
> > > Also, it looks like this will just block forever with no monitor input
> > > and no way for user to figure out what is going on short of
> > > crashing QEMU.
> > 
> > FWIW, the current vhost-user-net device does exactly the same thing
> > with calling qemu_chr_fe_wait_connected during its realize() function.
> 
> Hmm yes. It doesn't sleep for an arbitrary 1 sec so less of an eyesore :)

The sleep(1) in this patch simply needs to be removed. I think that
probably dates from when it was written against the earlier broken
version of qemu_chr_fe_wait_connected(). That would not correctly
deal with the "reconnect" flag, and so needing this loop with a sleep
in it.

In fact the while loop can be removed as well in this code. It just
needs to call qemu_chr_fe_wait_connected() once. It is guaranteed
to have a connected peer once that returns 0.

qemu_chr_fe_wait_connected() only returns -1 if the operating in
client mode, and it failed to connect and reconnect is *not*
requested. In such case the caller should honour the failure and
quit, not loop to retry.


The reason vhost-user-net does a loop is because once it has a
connection it tries todo a protocol handshake, and if that
handshake fails it closes the chardev and tries to connect
again. That's not the case in this blk code os the loop is
not needed.

> > Can't say I'm thrilled about the existing usage either, but I don't
> > see a particularly nice alternative if it isn't possible to realize
> > the device without having a backend available.
> > 
> > I don't think this an especially serious problem during cold startup
> > though since doesn't expect the monitor to become responsive until
> > the device model is fully realized & all backends connected.
> > 
> > The real concern is if you need to hotplug this kind of device
> > at runtime. In that case blocking the main loop / monitor is a
> > serious problem that will actively harm both the guest OS and
> > mgmt applications.
> > 
> > The vhost-user-net device will already suffer from that.
> 
> Right.
> 
> > 

Re: [Qemu-devel] [PULL 00/10] Migration patches

[Peter Maydell]

On Wed, 13 Mar 2019 at 12:14, Juan Quintela  wrote:
>
> The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1:
>
>   Merge remote-tracking branch 
> 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 
> 21:06:26 +)
>
> are available in the Git repository at:
>
>   https://github.com/juanquintela/qemu.git tags/migration-pull-request
>
> for you to fetch changes up to d677dc8d443e00bb9472fda6cc95ed2256f49670:
>
>   migration: add support for a "tls-authz" migration parameter (2019-03-13 
> 12:28:21 +0100)
>
> 
> Migration fixes
>
> - multifd "upgrade" from experimental
> - tls auth change  (danp)
>
> Later, Juan.
>
> 

For aarch32 host i386 guest I got this failure on the
migration-test /i386/migration/multifd/tcp test:

*** Error in `i386-softmmu/qemu-system-i386': malloc(): smallbin
double linked list corrupted: 0x01b564b8 ***
qemu-system-i386: check_section_footer: Read section footer failed: -5
Broken pipe
qemu-system-i386: load of migration failed: Invalid argument
/home/peter.maydell/qemu/tests/libqtest.c:135: kill_qemu() tried to
terminate QEMU process but encountered exit status 1
Aborted
ERROR - too few tests run (expected 8, got 7)

I did a retry by hand of 'make check-qtest-i386' and this time
it hung entirely when trying to run that test case.

I also tried a run under valgrind on x86 (you'll need to
nobble KVM somehow as valgrind can't deal with it):
QTEST_QEMU_BINARY='valgrind --smc-check=all
i386-softmmu/qemu-system-i386' QTEST_QEMU_IMG=qemu-img
tests/migration-test -m=quick -p '/i386/migration/multifd/tcp'

and that seems to have hung too.

thanks
-- PMM

Re: [Qemu-devel] About Revert this patch: arm: Allow system registers for KVM guests to be changed by QEMU code

[Peter Maydell]

On Thu, 14 Mar 2019 at 11:31, gengdongjiu  wrote:
>
>  Hi   Peter/Eric,
>I think we should fix the regression issue instead of revert this patch,  
> I think the reason of
>this issue is that QEMU modified some unexpected resisters, we should find 
> out.

Yes, I agree that we need to actually fix the underlying bug.
But I committed the revert as an interim fix because it's
taking me some time to identify what is going wrong.

thanks
-- PMM

[Qemu-devel] [PATCH v2 1/2] virtio-gpu: delay virglrenderer reset when blocked.

[Gerd Hoffmann]

If renderer_blocked is set do not call virtio_gpu_virgl_reset().
Instead set a flag indicating that virglrenderer needs a reset.
When renderer_blocked gets cleared do the actual reset call.

Without this we can trigger an assert in spice due to calling
spice_qxl_gl_scanout() while another operation is still running:

spice_qxl_gl_scanout: condition `qxl_state->gl_draw_cookie == 
GL_DRAW_COOKIE_INVALID' failed

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  1 +
 hw/display/virtio-gpu.c| 12 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ce0ca7217175..60425c5d58dc 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -113,6 +113,7 @@ typedef struct VirtIOGPU {
 bool use_virgl_renderer;
 bool renderer_inited;
 int renderer_blocked;
+bool renderer_reset;
 QEMUTimer *fence_poll;
 QEMUTimer *print_stats;
 
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4dbf48e42482..fbd8d908ad32 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1084,6 +1084,12 @@ static void virtio_gpu_gl_block(void *opaque, bool block)
 assert(g->renderer_blocked >= 0);
 
 if (g->renderer_blocked == 0) {
+#ifdef CONFIG_VIRGL
+if (g->renderer_reset) {
+g->renderer_reset = false;
+virtio_gpu_virgl_reset(g);
+}
+#endif
 virtio_gpu_process_cmdq(g);
 }
 }
@@ -1368,7 +1374,11 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 
 #ifdef CONFIG_VIRGL
 if (g->use_virgl_renderer) {
-virtio_gpu_virgl_reset(g);
+if (g->renderer_blocked) {
+g->renderer_reset = true;
+} else {
+virtio_gpu_virgl_reset(g);
+}
 g->use_virgl_renderer = 0;
 }
 #endif
-- 
2.18.1

[Qemu-devel] [PATCH v2 2/2] virtio-gpu: clear command and fence queues on reset

[Gerd Hoffmann]

It was never correct to not clear them.  Due to commit "3912e66a3feb
virtio-vga: fix reset." this became more obvious though.  The virtio
rings get properly reset now, and trying to process the stale commands
will trigger an assert in the virtio core.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-gpu.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index fbd8d908ad32..9e37e0ac96b7 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1356,6 +1356,7 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
 struct virtio_gpu_simple_resource *res, *tmp;
+struct virtio_gpu_ctrl_command *cmd;
 int i;
 
 g->enable = 0;
@@ -1372,6 +1373,19 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 g->scanout[i].ds = NULL;
 }
 
+while (!QTAILQ_EMPTY(&g->cmdq)) {
+cmd = QTAILQ_FIRST(&g->cmdq);
+QTAILQ_REMOVE(&g->cmdq, cmd, next);
+g_free(cmd);
+}
+
+while (!QTAILQ_EMPTY(&g->fenceq)) {
+cmd = QTAILQ_FIRST(&g->fenceq);
+QTAILQ_REMOVE(&g->fenceq, cmd, next);
+g->inflight--;
+g_free(cmd);
+}
+
 #ifdef CONFIG_VIRGL
 if (g->use_virgl_renderer) {
 if (g->renderer_blocked) {
-- 
2.18.1

[Qemu-devel] [PATCH v2 0/2] virtio-gpu: more reset fixes.

[Gerd Hoffmann]

Gerd Hoffmann (2):
  virtio-gpu: delay virglrenderer reset when blocked.
  virtio-gpu: clear command and fence queues on reset

 include/hw/virtio/virtio-gpu.h |  1 +
 hw/display/virtio-gpu.c| 26 +-
 2 files changed, 26 insertions(+), 1 deletion(-)

-- 
2.18.1

Re: [Qemu-devel] [RFC PATCH 3/3] hw/acpi: Extract build_mcfg

[Wei Yang]

On Thu, Mar 14, 2019 at 10:18:30AM +0100, Igor Mammedov wrote:
>On Wed, 13 Mar 2019 21:31:37 +
>Wei Yang  wrote:
>
>> On Wed, Mar 13, 2019 at 05:09:43PM +0100, Igor Mammedov wrote:
>> >On Wed, 13 Mar 2019 13:33:59 +
>> >Wei Yang  wrote:
>> >  
>> >> On Wed, Mar 13, 2019 at 01:23:00PM +0100, Igor Mammedov wrote:  
>> >> >On Wed, 13 Mar 2019 12:42:53 +0800
>> >> >Wei Yang  wrote:
>> >> >
>> >> >> Now we have two identical build_mcfg function.
>> >> >> 
>> >> >> Extract them to aml-build.c.
>> >> >> 
>> >> >> Signed-off-by: Wei Yang 
>> >> >> ---
>> >> >>  hw/acpi/aml-build.c | 30 ++
>> >> >>  hw/arm/virt-acpi-build.c| 16 
>> >> >>  hw/i386/acpi-build.c| 31 +--
>> >> >>  include/hw/acpi/aml-build.h |  1 +
>> >> >>  4 files changed, 32 insertions(+), 46 deletions(-)
>> >> >> 
>> >> >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> >> >> index 555c24f21d..58d3b8f31d 100644
>> >> >> --- a/hw/acpi/aml-build.c
>> >> >> +++ b/hw/acpi/aml-build.c
>> >> >
>> >> >I don't like polluting aml-build.c with PCI stuff,
>> >> >we have a lot of PCI related code that needs generalizing
>> >> >lets create a new file for that, something like
>> >> >hw/acpi/pci.c + include/hw/acpi/pci.h
>> >> >
>> >> >> @@ -25,6 +25,7 @@
>> >> >>  #include "qemu/bswap.h"
>> >> >>  #include "qemu/bitops.h"
>> >> >>  #include "sysemu/numa.h"
>> >> >> +#include "hw/pci/pcie_host.h"
>> >> >>  
>> >> >>  static GArray *build_alloc_array(void)
>> >> >>  {
>> >> >> @@ -1870,3 +1871,32 @@ build_hdr:
>> >> >>  build_header(linker, tbl, (void *)(tbl->data + fadt_start),
>> >> >>   "FACP", tbl->len - fadt_start, f->rev, oem_id, 
>> >> >> oem_table_id);
>> >> >>  }
>> >> >> +
>> >> >> +void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo 
>> >> >> *info)
>> >> >> +{
>> >> >> +AcpiTableMcfg *mcfg;
>> >> >> +const char *sig;
>> >> >> +int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
>> >> >> +
>> >> >> +mcfg = acpi_data_push(table_data, len);
>> >> >> +mcfg->allocation[0].address = cpu_to_le64(info->mcfg_base);
>> >> >> +/* Only a single allocation so no need to play with segments */
>> >> >> +mcfg->allocation[0].pci_segment = cpu_to_le16(0);
>> >> >> +mcfg->allocation[0].start_bus_number = 0;
>> >> >> +mcfg->allocation[0].end_bus_number = 
>> >> >> PCIE_MMCFG_BUS(info->mcfg_size - 1);
>> >> >
>> >> >> +/*
>> >> >> + * MCFG is used for ECAM which can be enabled or disabled by 
>> >> >> guest.
>> >> >> + * To avoid table size changes (which create migration issues),
>> >> >> + * always create the table even if there are no allocations,
>> >> >> + * but set the signature to a reserved value in this case.
>> >> >> + * ACPI spec requires OSPMs to ignore such tables.
>> >> >> + */
>> >> >> +if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
>> >> >> +/* Reserved signature: ignored by OSPM */
>> >> >> +sig = "QEMU";
>> >> >> +} else {
>> >> >> +sig = "MCFG";
>> >> >> +}
>> >> >I'd leave these hack at acpi-build.c, just push it up call chain.
>> >> 
>> >> Assign sig in acpi-build.c and pass it to build_mcfg()?  
>> >nope, see more below
>> >
>> >   
>> >> >More over we don't really need it since resizeable memory region was 
>> >> >introduced.
>> >> >
>> >> >So we need to keep table_blob size only for legacy usecase (pre 
>> >> >resizable)
>> >> >and for that just padding table_blob on required size would be 
>> >> >sufficient,
>> >> >there is no need to create dummy QEMU table.
>> >> >As for newer machines (since resizeable memory region) we don't need to
>> >> >do even that i.e. just skip table generation altogether if guest 
>> >> >disabled it.
>> >> >
>> >> 
>> >> I am lost at this place.
>> >> 
>> >> sig is a part of ACPI table header, you mean the sig is not necessary to
>> >> be set in ACPI table header?
>> >> 
>> >> "skip table generation" means remove build_header() in build_mcfg()?  
>> >I mean do not call build_mcfg() at all when you don't have to.
>> >
>> >And when you need to keep table_blob the same size (for old machines)
>> >using acpi_data_push() to reserve space instead of build_mcfg(sig="QEMU")
>> >might just work as well. it's still hack but it can live in x86 specific
>> >acpi_build() keeping build_mcfg() generic.
>> >  
>> 
>> Seems got your idea.
>> 
>> >As for defining what to use as criteria to decide when we need to keep
>> >table_blob size the same, I don't remember history of it, so I'd suggest
>> >to look at commit a1666142, study history of acpi_ram_update() and
>> >legacy_acpi_table_size to figure out since which machine type one doesn't
>> >have to keep table_blob size the same.
>> >  
>> 
>> OK, let me study the history first.
>> 
>> BTW, the legacy here is hardware specification level or qemu software
>> design level?
>it's QEMU only, you need to find a v

Re: [Qemu-devel] [PATCH 2/2] virtio-gpu: clear command queue on reset

[Gerd Hoffmann]

On Thu, Mar 14, 2019 at 12:01:27PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Mar 14, 2019 at 7:17 AM Gerd Hoffmann  wrote:
> >
> > It was never correct to not clear them.  Due to commit "3912e66a3feb
> > virtio-vga: fix reset." this became more obvious though.  The virtio
> > rings get properly reset now, and trying to process the stale commands
> > will trigger an assert in the virtio core.
> >
> > Signed-off-by: Gerd Hoffmann 
> 
> Reviewed-by: Marc-André Lureau 
> 
> what about the fences and related data?

Good point.  v2 sent.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] hw/acpi: extract acpi_add_rom_blob()

[Wei Yang]

On Thu, Mar 14, 2019 at 10:25:14AM +0100, Igor Mammedov wrote:
>On Thu, 14 Mar 2019 15:38:36 +0800
>Wei Yang  wrote:
>
>> arm and i386 has almost the same function acpi_add_rom_blob(), except
>> giving different FWCfgCallback function.
>> 
>> This patch extract acpi_add_rom_blob() to aml-build.c by passing
>> FWCfgCallback to it.
>> 
>> Signed-off-by: Wei Yang 
>> 
>> ---
>> v2:
>>   * remove unused header in original source file
>> ---
>>  hw/acpi/aml-build.c |  8 
>>  hw/arm/virt-acpi-build.c| 25 +
>>  hw/i386/acpi-build.c| 25 +
>>  include/hw/acpi/aml-build.h |  4 
>>  4 files changed, 30 insertions(+), 32 deletions(-)
>> 
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 555c24f21d..ed427f8310 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>I'd like to keep aml-build.c clean from not spec related code,
>Lets create a separate source file for QEMU specific code, something like
>hw/acpi/utils.(h|c)
>
>otherwise patch looks fine to me
>

Reasonable, thanks for this comment.

>
>> @@ -1559,6 +1559,14 @@ void *acpi_data_push(GArray *table_data, unsigned 
>> size)
>>  return table_data->data + off;
>>  }
>>  
>> +MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque,
>> +GArray *blob, const char *name,
>> +uint64_t max_size)
>> +{
>> +return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
>> +name, update, opaque, NULL, true);
>> +}
>> +
>>  unsigned acpi_data_len(GArray *table)
>>  {
>>  assert(g_array_get_element_size(table) == 1);
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 57679a89bf..f7fd0cf06a 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -37,7 +37,6 @@
>>  #include "hw/acpi/acpi.h"
>>  #include "hw/nvram/fw_cfg.h"
>>  #include "hw/acpi/bios-linker-loader.h"
>> -#include "hw/loader.h"
>>  #include "hw/hw.h"
>>  #include "hw/acpi/aml-build.h"
>>  #include "hw/pci/pcie_host.h"
>> @@ -881,14 +880,6 @@ static void virt_acpi_build_reset(void *build_opaque)
>>  build_state->patched = false;
>>  }
>>  
>> -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
>> -   GArray *blob, const char *name,
>> -   uint64_t max_size)
>> -{
>> -return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
>> -name, virt_acpi_build_update, build_state, NULL, 
>> true);
>> -}
>> -
>>  static const VMStateDescription vmstate_virt_acpi_build = {
>>  .name = "virt_acpi_build",
>>  .version_id = 1,
>> @@ -920,20 +911,22 @@ void virt_acpi_setup(VirtMachineState *vms)
>>  virt_acpi_build(vms, &tables);
>>  
>>  /* Now expose it all to Guest */
>> -build_state->table_mr = acpi_add_rom_blob(build_state, 
>> tables.table_data,
>> -   ACPI_BUILD_TABLE_FILE,
>> -   ACPI_BUILD_TABLE_MAX_SIZE);
>> +build_state->table_mr = acpi_add_rom_blob(virt_acpi_build_update,
>> +  build_state, 
>> tables.table_data,
>> +  ACPI_BUILD_TABLE_FILE,
>> +  ACPI_BUILD_TABLE_MAX_SIZE);
>>  assert(build_state->table_mr != NULL);
>>  
>>  build_state->linker_mr =
>> -acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
>> -  "etc/table-loader", 0);
>> +acpi_add_rom_blob(virt_acpi_build_update, build_state,
>> +  tables.linker->cmd_blob, "etc/table-loader", 0);
>>  
>>  fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, 
>> tables.tcpalog->data,
>>  acpi_data_len(tables.tcpalog));
>>  
>> -build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
>> -  ACPI_BUILD_RSDP_FILE, 0);
>> +build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
>> + build_state, tables.rsdp,
>> + ACPI_BUILD_RSDP_FILE, 0);
>>  
>>  qemu_register_reset(virt_acpi_build_reset, build_state);
>>  virt_acpi_build_reset(build_state);
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 416da318ae..bc6e4b1f01 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -37,7 +37,6 @@
>>  #include "hw/acpi/cpu.h"
>>  #include "hw/nvram/fw_cfg.h"
>>  #include "hw/acpi/bios-linker-loader.h"
>> -#include "hw/loader.h"
>>  #include "hw/isa/isa.h"
>>  #include "hw/block/fdc.h"
>>  #include "hw/acpi/memory_hotplug.h"
>> @@ -2842,14 +2841,6 @@ static void acpi_build_reset(void *build_opaque)
>>  build_state->patched = 0;
>>  }
>>  

Re: [Qemu-devel] [PULL 00/10] Migration patches

[Peter Maydell]

On Thu, 14 Mar 2019 at 11:48, Peter Maydell  wrote:
>
> On Wed, 13 Mar 2019 at 12:14, Juan Quintela  wrote:
> >
> > The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1:
> >
> >   Merge remote-tracking branch 
> > 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 
> > 21:06:26 +)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/juanquintela/qemu.git tags/migration-pull-request
> >
> > for you to fetch changes up to d677dc8d443e00bb9472fda6cc95ed2256f49670:
> >
> >   migration: add support for a "tls-authz" migration parameter (2019-03-13 
> > 12:28:21 +0100)
> >
> > 
> > Migration fixes
> >
> > - multifd "upgrade" from experimental
> > - tls auth change  (danp)
> >
> > Later, Juan.
> >
> > 
>
> For aarch32 host i386 guest I got this failure on the
> migration-test /i386/migration/multifd/tcp test:
>
> *** Error in `i386-softmmu/qemu-system-i386': malloc(): smallbin
> double linked list corrupted: 0x01b564b8 ***
> qemu-system-i386: check_section_footer: Read section footer failed: -5
> Broken pipe
> qemu-system-i386: load of migration failed: Invalid argument
> /home/peter.maydell/qemu/tests/libqtest.c:135: kill_qemu() tried to
> terminate QEMU process but encountered exit status 1
> Aborted
> ERROR - too few tests run (expected 8, got 7)

I ran the test on the arm box under valgrind and got this, which might
or might not be helpful:

==18714== Thread 5 multifdsend_1:
==18714== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
==18714==at 0xF809B14: sendmsg (syscall-template.S:84)
==18714==by 0x303825: qio_channel_socket_writev (channel-socket.c:543)
==18714==by 0x302021: qio_channel_writev (channel.c:206)
==18714==by 0x302021: qio_channel_writev_all (channel.c:170)
==18714==by 0x302093: qio_channel_write_all (channel.c:256)
==18714==by 0x7C12F: multifd_send_initial_packet (ram.c:713)
==18714==by 0x7C12F: multifd_send_thread (ram.c:1090)
==18714==by 0x33A19F: qemu_thread_start (qemu-thread-posix.c:502)
==18714==by 0xF8025B3: start_thread (pthread_create.c:335)
==18714==by 0xF8B3BEB: ??? (clone.S:89)
==18714==  Address 0x2123e67d is on thread 5's stack
==18714==  in frame #4, created by multifd_send_thread (ram.c:1082)
==18714==
==18714== Thread 6 multifdsend_0:
==18714== Invalid write of size 4
==18714==at 0x7C25A: multifd_send_fill_packet (ram.c:808)
==18714==by 0x7C25A: multifd_send_thread (ram.c:1106)
==18714==by 0x33A19F: qemu_thread_start (qemu-thread-posix.c:502)
==18714==by 0xF8025B3: start_thread (pthread_create.c:335)
==18714==by 0xF8B3BEB: ??? (clone.S:89)
==18714==  Address 0x1e9bc108 is 0 bytes after a block of size 832 alloc'd
==18714==at 0x4840AB4: calloc (vg_replace_malloc.c:711)
==18714==by 0xF59BB99: g_malloc0 (in
/lib/arm-linux-gnueabihf/libglib-2.0.so.0.4800.2)
==18714==
==18714== Invalid write of size 4
==18714==at 0x7C260: multifd_send_fill_packet (ram.c:808)
==18714==by 0x7C260: multifd_send_thread (ram.c:1106)
==18714==by 0x33A19F: qemu_thread_start (qemu-thread-posix.c:502)
==18714==by 0xF8025B3: start_thread (pthread_create.c:335)
==18714==by 0xF8B3BEB: ??? (clone.S:89)
==18714==  Address 0x1e9bc10c is 4 bytes after a block of size 832 alloc'd
==18714==at 0x4840AB4: calloc (vg_replace_malloc.c:711)
==18714==by 0xF59BB99: g_malloc0 (in
/lib/arm-linux-gnueabihf/libglib-2.0.so.0.4800.2)
==18714==
==18714== Invalid read of size 4
==18714==at 0x3018F6: qio_channel_writev_full (channel.c:85)
==18714==by 0x302021: qio_channel_writev (channel.c:206)
==18714==by 0x302021: qio_channel_writev_all (channel.c:170)
==18714==by 0x302093: qio_channel_write_all (channel.c:256)
==18714==by 0x7C2B9: multifd_send_thread (ram.c:1116)
==18714==by 0x33A19F: qemu_thread_start (qemu-thread-posix.c:502)
==18714==by 0xF8025B3: start_thread (pthread_create.c:335)
==18714==by 0xF8B3BEB: ??? (clone.S:89)
==18714==  Address 0x30 is not stack'd, malloc'd or (recently) free'd
==18714==
==18714==
==18714== Process terminating with default action of signal 11 (SIGSEGV)
==18714==  Access not within mapped region at address 0x30
==18714==at 0x3018F6: qio_channel_writev_full (channel.c:85)
==18714==by 0x302021: qio_channel_writev (channel.c:206)
==18714==by 0x302021: qio_channel_writev_all (channel.c:170)
==18714==by 0x302093: qio_channel_write_all (channel.c:256)
==18714==by 0x7C2B9: multifd_send_thread (ram.c:1116)
==18714==by 0x33A19F: qemu_thread_start (qemu-thread-posix.c:502)
==18714==by 0xF8025B3: start_thread (pthread_create.c:335)
==18714==by 0xF8B3BEB: ??? (clone.S:89)
==18714==  If you believe this happened as a result of a stack
==18714==  overflow in your program's main thread (unlikely but
==18714==  possible), you can try t

Re: [Qemu-devel] [build-error] possible build error at the tip of the trunk? (dtc-related?)

[Peter Maydell]

On Mon, 11 Mar 2019 at 16:59, Aleksandar Markovic
 wrote:
>
> Hello, all
>
> All of the sudden, the latest code doesn't build on my host: (the code from 
> several days ago builds fine)
>
> Do you know what would be the culprit? Is it my environment, or a genuine 
> build error?

Hi -- we've now committed what we think should be the
fix for this in the configure script -- could you try again?
(You might need to delete the git tree and check it out again
from scratch, I'm not sure.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] virtio-gpu: clear command and fence queues on reset

[Marc-André Lureau]

Hi

On Thu, Mar 14, 2019 at 12:54 PM Gerd Hoffmann  wrote:
>
> It was never correct to not clear them.  Due to commit "3912e66a3feb
> virtio-vga: fix reset." this became more obvious though.  The virtio
> rings get properly reset now, and trying to process the stale commands
> will trigger an assert in the virtio core.
>
> Signed-off-by: Gerd Hoffmann 
> ---
>  hw/display/virtio-gpu.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index fbd8d908ad32..9e37e0ac96b7 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1356,6 +1356,7 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
>  {
>  VirtIOGPU *g = VIRTIO_GPU(vdev);
>  struct virtio_gpu_simple_resource *res, *tmp;
> +struct virtio_gpu_ctrl_command *cmd;
>  int i;
>
>  g->enable = 0;
> @@ -1372,6 +1373,19 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
>  g->scanout[i].ds = NULL;
>  }
>
> +while (!QTAILQ_EMPTY(&g->cmdq)) {
> +cmd = QTAILQ_FIRST(&g->cmdq);
> +QTAILQ_REMOVE(&g->cmdq, cmd, next);
> +g_free(cmd);
> +}
> +
> +while (!QTAILQ_EMPTY(&g->fenceq)) {
> +cmd = QTAILQ_FIRST(&g->fenceq);
> +QTAILQ_REMOVE(&g->fenceq, cmd, next);
> +g->inflight--;
> +g_free(cmd);
> +}
> +

Reviewed-by: Marc-André Lureau 

>  #ifdef CONFIG_VIRGL
>  if (g->use_virgl_renderer) {
>  if (g->renderer_blocked) {
> --
> 2.18.1
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v7 6/7] vhost-user-blk: Add support to reconnect backend

[Michael S. Tsirkin]

On Thu, Mar 14, 2019 at 11:43:26AM +, Daniel P. Berrangé wrote:
> On Thu, Mar 14, 2019 at 07:34:03AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Mar 14, 2019 at 11:24:22AM +, Daniel P. Berrangé wrote:
> > > On Tue, Mar 12, 2019 at 12:49:35PM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, Feb 28, 2019 at 04:53:54PM +0800, elohi...@gmail.com wrote:
> > > > > From: Xie Yongji 
> > > > > 
> > > > > Since we now support the message VHOST_USER_GET_INFLIGHT_FD
> > > > > and VHOST_USER_SET_INFLIGHT_FD. The backend is able to restart
> > > > > safely because it can track inflight I/O in shared memory.
> > > > > This patch allows qemu to reconnect the backend after
> > > > > connection closed.
> > > > > 
> > > > > Signed-off-by: Xie Yongji 
> > > > > Signed-off-by: Ni Xun 
> > > > > Signed-off-by: Zhang Yu 
> > > > > ---
> > > > >  hw/block/vhost-user-blk.c  | 205 
> > > > > +++--
> > > > >  include/hw/virtio/vhost-user-blk.h |   4 +
> > > > >  2 files changed, 167 insertions(+), 42 deletions(-)
> > > 
> > > 
> > > > >  static void vhost_user_blk_device_realize(DeviceState *dev, Error 
> > > > > **errp)
> > > > >  {
> > > > >  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > > > >  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > > > >  VhostUserState *user;
> > > > > -struct vhost_virtqueue *vqs = NULL;
> > > > >  int i, ret;
> > > > > +Error *err = NULL;
> > > > >  
> > > > >  if (!s->chardev.chr) {
> > > > >  error_setg(errp, "vhost-user-blk: chardev is mandatory");
> > > > > @@ -312,27 +442,28 @@ static void 
> > > > > vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> > > > >  }
> > > > >  
> > > > >  s->inflight = g_new0(struct vhost_inflight, 1);
> > > > > -
> > > > > -s->dev.nvqs = s->num_queues;
> > > > > -s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
> > > > > -s->dev.vq_index = 0;
> > > > > -s->dev.backend_features = 0;
> > > > > -vqs = s->dev.vqs;
> > > > > -
> > > > > -vhost_dev_set_config_notifier(&s->dev, &blk_ops);
> > > > > -
> > > > > -ret = vhost_dev_init(&s->dev, s->vhost_user, 
> > > > > VHOST_BACKEND_TYPE_USER, 0);
> > > > > -if (ret < 0) {
> > > > > -error_setg(errp, "vhost-user-blk: vhost initialization 
> > > > > failed: %s",
> > > > > -   strerror(-ret));
> > > > > -goto virtio_err;
> > > > > -}
> > > > > +s->vqs = g_new(struct vhost_virtqueue, s->num_queues);
> > > > > +s->watch = 0;
> > > > > +s->should_start = false;
> > > > > +s->connected = false;
> > > > > +
> > > > > +qemu_chr_fe_set_handlers(&s->chardev,  NULL, NULL, 
> > > > > vhost_user_blk_event,
> > > > > + NULL, (void *)dev, NULL, true);
> > > > > +
> > > > > +reconnect:
> > > > > +do {
> > > > > +if (qemu_chr_fe_wait_connected(&s->chardev, &err) < 0) {
> > > > > +error_report_err(err);
> > > > > +err = NULL;
> > > > > +sleep(1);
> > > > 
> > > > Seems arbitrary. Is this basically waiting until backend will reconnect?
> > > > Why not block until event on the fd triggers?
> > > > 
> > > > Also, it looks like this will just block forever with no monitor input
> > > > and no way for user to figure out what is going on short of
> > > > crashing QEMU.
> > > 
> > > FWIW, the current vhost-user-net device does exactly the same thing
> > > with calling qemu_chr_fe_wait_connected during its realize() function.
> > 
> > Hmm yes. It doesn't sleep for an arbitrary 1 sec so less of an eyesore :)
> 
> The sleep(1) in this patch simply needs to be removed. I think that
> probably dates from when it was written against the earlier broken
> version of qemu_chr_fe_wait_connected(). That would not correctly
> deal with the "reconnect" flag, and so needing this loop with a sleep
> in it.
> 
> In fact the while loop can be removed as well in this code. It just
> needs to call qemu_chr_fe_wait_connected() once. It is guaranteed
> to have a connected peer once that returns 0.
> 
> qemu_chr_fe_wait_connected() only returns -1 if the operating in
> client mode, and it failed to connect and reconnect is *not*
> requested. In such case the caller should honour the failure and
> quit, not loop to retry.
> 
> 
> The reason vhost-user-net does a loop is because once it has a
> connection it tries todo a protocol handshake, and if that
> handshake fails it closes the chardev and tries to connect
> again. That's not the case in this blk code os the loop is
> not needed.
> 
> > > Can't say I'm thrilled about the existing usage either, but I don't
> > > see a particularly nice alternative if it isn't possible to realize
> > > the device without having a backend available.
> > > 
> > > I don't think this an especially serious problem during cold startup
> > > though since doesn't expect the monitor to become responsive until
> > > the device model is fully realized & all backends connected.
> > > 
> >

Re: [Qemu-devel] why does our coverity-model.c g_strdup() say it is a size-sink?

[Peter Maydell]

On Thu, 14 Mar 2019 at 11:23, Paolo Bonzini  wrote:
>
> On 14/03/19 11:51, Peter Maydell wrote:
> > Our coverity model of g_strdup() includes:
> >   __coverity_string_size_sink__(s);
> >
> > This seems to be causing Coverity to report false positives like
> > CID1399705 and 1399699 where we take a string from getenv() and
> > pass it to g_strdup() The getenv() string is untrusted data of unknown
> > length, and g_strdup() being marked as a size-sink makes Coverity
> > think the function wants "a string of a particular size".
> >
> > Markus, you wrote this model initially -- can you remember why it's
> > marked as a size-sink? Unfortunately I can't find any documentation
> > online about what the coverity model annotation here means :-(
>
> I think it means that we don't want a g_strdup that can potentially do
> an unbounded allocation.

Mmm, that makes sense. So in this particular case, do we
want to try to avoid doing an unbounded allocation based
on whatever rubbish the user passed us in the environment,
or do we say "this particular case is OK" and mark it
as a false-positive ?

Cc'ing Gerd since the Coverity issues in question are
in the audio code (in get_str() and in
audio_handle_legacy_opts()).

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.0] qapi: fix block-latency-histogram-set description and examples

[Eric Blake]

On 3/14/19 3:52 AM, Vladimir Sementsov-Ogievskiy wrote:
> There no @device parameter, only the @id one.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  qapi/block-core.json | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Documentation fixes are appropriate during freeze.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qapi: fix block-latency-histogram-set description and examples

[Kevin Wolf]

Am 14.03.2019 um 09:52 hat Vladimir Sementsov-Ogievskiy geschrieben:
> There no @device parameter, only the @id one.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] About Revert this patch: arm: Allow system registers for KVM guests to be changed by QEMU code

[gengdongjiu]

If you think this patch will introduce some issue, we can add one function 
write_part_cpustate_to_list()[2] to change the specified
register instead of all the registers[1].

Below function that you added will modified all the register if new value is 
different with old value.
[1]:
bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)
{

}

[2]:
+bool write_part_cpustate_to_list(ARMCPU *cpu, ptrdiff_t fieldoffset)
+{
+const ARMCPRegInfo *ri;
+uint32_t regidx, i;
+
+for (i = 0; i < cpu->cpreg_array_len; i++) {
+regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
+ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
+if (!ri) {
+continue;
+}
+
+if (ri->type & ARM_CP_NO_RAW) {
+continue;
+}
+if (ri->fieldoffset == fieldoffset) {
+cpu->cpreg_values[i] = read_raw_cp_reg(&cpu->env, ri);
+return true;
+}
+}
+return false;
+}
+



On 2019/3/14 19:31, gengdongjiu wrote:
>  Hi   Peter/Eric,
>I think we should fix the regression issue instead of revert this patch,  
> I think the reason of
>this issue is that QEMU modified some unexpected resisters, we should find 
> out.
> 
> 
> Revert "arm: Allow system registers for KVM guests to be changed by QEMU 
> code"
> 
> This reverts commit 823e1b3818f9b10b824ddcd756983b6e2fa68730,
> which introduces a regression running EDK2 guest firmware
> under KVM:
> 
> error: kvm run failed Function not implemented
>  PC=00013f5a6208 X00=404003c4 X01=003a
> X02= X03=404003c4 X04=
> X05=9646 X06=00013d2ef270 X07=00013e3d1710
> X08=09010755ffaf8ba8 X09=ffaf8b9cfeeb5468 X10=feeb546409010756
> X11=09010757ffaf8b90 X12=feeb50680903068b X13=090306a1ffaf8bc0
> X14= X15= X16=00013f872da0
> X17=a6ab X18= X19=00013f5a92d0
> X20=00013f5a7a78 X21=003a X22=00013f5a7ab2
> X23=00013f5a92e8 X24=00013f631090 X25=0010
> X26=0100 X27=00013f89501b X28=00013e3d14e0
> X29=00013e3d12a0 X30=00013f5a2518  SP=00013b7be0b0
> PSTATE=404003c4 -Z-- EL1t
> 
> with
> [ 3507.926571] kvm [35042]: load/store instruction decoding not 
> implemented
> in the host dmesg.
> 
> Revert the change for the moment until we can investigate the
> cause of the regression.
> 
> Reported-by: Eric Auger 
> Signed-off-by: Peter Maydell 
> 

Re: [Qemu-devel] [PULL 03/28] qapi: move to QOM path for x-block-latency-histogram-set

[Kevin Wolf]

Am 14.03.2019 um 09:47 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 12.03.2019 20:30, Kevin Wolf wrote:
> > From: Vladimir Sementsov-Ogievskiy 
> > 
> > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > Signed-off-by: Kevin Wolf 
> 
> Not critical, but it is v4, when in v5 description and examples were
> fixed to be s/device/id.  I'll send a follow-up.

Oops, sorry. Thanks for noticing and sending a follow-up so quickly.

Kevin

Re: [Qemu-devel] why does our coverity-model.c g_strdup() say it is a size-sink?

[Paolo Bonzini]

On 14/03/19 13:16, Peter Maydell wrote:
> Mmm, that makes sense. So in this particular case, do we
> want to try to avoid doing an unbounded allocation based
> on whatever rubbish the user passed us in the environment,
> or do we say "this particular case is OK" and mark it
> as a false-positive ?
> 
> Cc'ing Gerd since the Coverity issues in question are
> in the audio code (in get_str() and in
> audio_handle_legacy_opts()).

I think the solution for that is deprecating the old legacy options and
getting rid of them in one year...

Paolo

Re: [Qemu-devel] [PATCH for-4.0 0/2] Update .gitignore and tests/.gitignore for in-tree builds

[Eric Blake]

On 3/14/19 5:46 AM, Stefano Garzarella wrote:
> This series could be useless when we will no longer support in-tree builds,
> but for 4.0 I think it's useful to ignore these files.

series:
Reviewed-by: Eric Blake 

> 
> Stefano Garzarella (2):
>   .gitignore: ignore docs/built created for in-tree builds
>   tests/.gitignore: ignore test-qapi-emit-events.[ch] for in-tree builds
> 
>  .gitignore   | 1 +
>  tests/.gitignore | 1 +
>  2 files changed, 2 insertions(+)
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v7 6/7] vhost-user-blk: Add support to reconnect backend

[Yury Kotov]

Hi,

14.03.2019, 14:44, "Daniel P. Berrangé" :
> On Thu, Mar 14, 2019 at 07:34:03AM -0400, Michael S. Tsirkin wrote:
>>  On Thu, Mar 14, 2019 at 11:24:22AM +, Daniel P. Berrangé wrote:
>>  > On Tue, Mar 12, 2019 at 12:49:35PM -0400, Michael S. Tsirkin wrote:
>>  > > On Thu, Feb 28, 2019 at 04:53:54PM +0800, elohi...@gmail.com wrote:
>>  > > > From: Xie Yongji 
>>  > > >
>>  > > > Since we now support the message VHOST_USER_GET_INFLIGHT_FD
>>  > > > and VHOST_USER_SET_INFLIGHT_FD. The backend is able to restart
>>  > > > safely because it can track inflight I/O in shared memory.
>>  > > > This patch allows qemu to reconnect the backend after
>>  > > > connection closed.
>>  > > >
>>  > > > Signed-off-by: Xie Yongji 
>>  > > > Signed-off-by: Ni Xun 
>>  > > > Signed-off-by: Zhang Yu 
>>  > > > ---
>>  > > > hw/block/vhost-user-blk.c | 205 +++--
>>  > > > include/hw/virtio/vhost-user-blk.h | 4 +
>>  > > > 2 files changed, 167 insertions(+), 42 deletions(-)
>>  >
>>  >
>>  > > > static void vhost_user_blk_device_realize(DeviceState *dev, Error 
>> **errp)
>>  > > > {
>>  > > > VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>  > > > VHostUserBlk *s = VHOST_USER_BLK(vdev);
>>  > > > VhostUserState *user;
>>  > > > - struct vhost_virtqueue *vqs = NULL;
>>  > > > int i, ret;
>>  > > > + Error *err = NULL;
>>  > > >
>>  > > > if (!s->chardev.chr) {
>>  > > > error_setg(errp, "vhost-user-blk: chardev is mandatory");
>>  > > > @@ -312,27 +442,28 @@ static void 
>> vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>>  > > > }
>>  > > >
>>  > > > s->inflight = g_new0(struct vhost_inflight, 1);
>>  > > > -
>>  > > > - s->dev.nvqs = s->num_queues;
>>  > > > - s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
>>  > > > - s->dev.vq_index = 0;
>>  > > > - s->dev.backend_features = 0;
>>  > > > - vqs = s->dev.vqs;
>>  > > > -
>>  > > > - vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>>  > > > -
>>  > > > - ret = vhost_dev_init(&s->dev, s->vhost_user, 
>> VHOST_BACKEND_TYPE_USER, 0);
>>  > > > - if (ret < 0) {
>>  > > > - error_setg(errp, "vhost-user-blk: vhost initialization failed: %s",
>>  > > > - strerror(-ret));
>>  > > > - goto virtio_err;
>>  > > > - }
>>  > > > + s->vqs = g_new(struct vhost_virtqueue, s->num_queues);
>>  > > > + s->watch = 0;
>>  > > > + s->should_start = false;
>>  > > > + s->connected = false;
>>  > > > +
>>  > > > + qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, 
>> vhost_user_blk_event,
>>  > > > + NULL, (void *)dev, NULL, true);
>>  > > > +
>>  > > > +reconnect:
>>  > > > + do {
>>  > > > + if (qemu_chr_fe_wait_connected(&s->chardev, &err) < 0) {
>>  > > > + error_report_err(err);
>>  > > > + err = NULL;
>>  > > > + sleep(1);
>>  > >
>>  > > Seems arbitrary. Is this basically waiting until backend will reconnect?
>>  > > Why not block until event on the fd triggers?
>>  > >
>>  > > Also, it looks like this will just block forever with no monitor input
>>  > > and no way for user to figure out what is going on short of
>>  > > crashing QEMU.
>>  >
>>  > FWIW, the current vhost-user-net device does exactly the same thing
>>  > with calling qemu_chr_fe_wait_connected during its realize() function.
>>
>>  Hmm yes. It doesn't sleep for an arbitrary 1 sec so less of an eyesore :)
>
> The sleep(1) in this patch simply needs to be removed. I think that
> probably dates from when it was written against the earlier broken
> version of qemu_chr_fe_wait_connected(). That would not correctly
> deal with the "reconnect" flag, and so needing this loop with a sleep
> in it.
>
> In fact the while loop can be removed as well in this code. It just
> needs to call qemu_chr_fe_wait_connected() once. It is guaranteed
> to have a connected peer once that returns 0.
>
> qemu_chr_fe_wait_connected() only returns -1 if the operating in
> client mode, and it failed to connect and reconnect is *not*
> requested. In such case the caller should honour the failure and
> quit, not loop to retry.
>
> The reason vhost-user-net does a loop is because once it has a
> connection it tries todo a protocol handshake, and if that
> handshake fails it closes the chardev and tries to connect
> again. That's not the case in this blk code os the loop is
> not needed.
>

But vhost-user-blk also has a handshake in device realize. What happens if the
connection is broken during realization? IIUC we have to retry a handshake in
such case just like vhost-user-net.

>>  > Can't say I'm thrilled about the existing usage either, but I don't
>>  > see a particularly nice alternative if it isn't possible to realize
>>  > the device without having a backend available.
>>  >
>>  > I don't think this an especially serious problem during cold startup
>>  > though since doesn't expect the monitor to become responsive until
>>  > the device model is fully realized & all backends connected.
>>  >
>>  > The real concern is if you need to hotplug this kind of device
>>  > at runtime. In that case blocking the m

Re: [Qemu-devel] [PATCH] block/io: fix bdrv_co_do_copy_on_readv error handling

[Vladimir Sementsov-Ogievskiy]

14.03.2019 13:14, Vladimir Sementsov-Ogievskiy wrote:
> It's not safe to treat bdrv_is_allocated error as unallocated: if we
> mistake we may rewrite guest data.

... with same data, which is not so bad.

So, it's ok, I'm wrong, drop it.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>   block/io.c | 12 +++-
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 2ba603c7bc..dccad64d46 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1193,11 +1193,13 @@ static int coroutine_fn 
> bdrv_co_do_copy_on_readv(BdrvChild *child,
>   ret = bdrv_is_allocated(bs, cluster_offset,
>   MIN(cluster_bytes, max_transfer), &pnum);
>   if (ret < 0) {
> -/* Safe to treat errors in querying allocation as if
> - * unallocated; we'll probably fail again soon on the
> - * read, but at least that will set a decent errno.
> +/*
> + * We must fail here, and can't treat error as allocated or
> + * unallocated: if we mistake, it will lead to not done 
> copy-on-read
> + * in first case and to rewriting guest data that is already in 
> top
> + * layer in the second case.
>*/
> -pnum = MIN(cluster_bytes, max_transfer);
> +goto err;
>   }
>   
>   /* Stop at EOF if the image ends in the middle of the cluster */
> @@ -1208,7 +1210,7 @@ static int coroutine_fn 
> bdrv_co_do_copy_on_readv(BdrvChild *child,
>   
>   assert(skip_bytes < pnum);
>   
> -if (ret <= 0) {
> +if (ret == 0) {
>   /* Must copy-on-read; use the bounce buffer */
>   pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
>   qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] why does our coverity-model.c g_strdup() say it is a size-sink?

[Daniel P . Berrangé]

On Thu, Mar 14, 2019 at 12:16:06PM +, Peter Maydell wrote:
> On Thu, 14 Mar 2019 at 11:23, Paolo Bonzini  wrote:
> >
> > On 14/03/19 11:51, Peter Maydell wrote:
> > > Our coverity model of g_strdup() includes:
> > >   __coverity_string_size_sink__(s);
> > >
> > > This seems to be causing Coverity to report false positives like
> > > CID1399705 and 1399699 where we take a string from getenv() and
> > > pass it to g_strdup() The getenv() string is untrusted data of unknown
> > > length, and g_strdup() being marked as a size-sink makes Coverity
> > > think the function wants "a string of a particular size".
> > >
> > > Markus, you wrote this model initially -- can you remember why it's
> > > marked as a size-sink? Unfortunately I can't find any documentation
> > > online about what the coverity model annotation here means :-(
> >
> > I think it means that we don't want a g_strdup that can potentially do
> > an unbounded allocation.
> 
> Mmm, that makes sense. So in this particular case, do we
> want to try to avoid doing an unbounded allocation based
> on whatever rubbish the user passed us in the environment,
> or do we say "this particular case is OK" and mark it
> as a false-positive ?

I'd mark it a false positive. QEMU trusts whatever parent process
has spawned it to provide required environment variables, so it should
honour whatever env is set. ie the mgmt app above QEMU is not an attack
vector QEMU protects itself against.

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

Re: [Qemu-devel] About Revert this patch: arm: Allow system registers for KVM guests to be changed by QEMU code

[Peter Maydell]

On Thu, 14 Mar 2019 at 12:24, gengdongjiu  wrote:
>
> If you think this patch will introduce some issue, we can add one function 
> write_part_cpustate_to_list()[2] to change the specified
> register instead of all the registers[1].

It definitely does introduce an issue, but we need to address
it by working out why it breaks something that I wasn't
expecting to break. Then we can fix it properly. Looking
it this is on my todo list for this week.

thanks
-- PMM

Re: [Qemu-devel] About Revert this patch: arm: Allow system registers for KVM guests to be changed by QEMU code

[gengdongjiu]

On 2019/3/14 20:43, Peter Maydell wrote:
>> If you think this patch will introduce some issue, we can add one function 
>> write_part_cpustate_to_list()[2] to change the specified
>> register instead of all the registers[1].
> It definitely does introduce an issue, but we need to address
> it by working out why it breaks something that I wasn't
> expecting to break. Then we can fix it properly. Looking
> it this is on my todo list for this week.
Got it, great, thanks very much to Peter.

Re: [Qemu-devel] [PATCH RFC] seccomp: don't kill process for resource control syscalls

[Mathias Fröhlich]

Hi,

Thanks for not just killing processes anymore!
See the mesa thread
https://www.mail-archive.com/mesa-dev@lists.freedesktop.org/msg214474.html
for some background.

Thanks a lot and best

Mathias

On Wednesday, 13 March 2019 10:49:03 CET Daniel P. Berrangé wrote:
> The Mesa library tries to set process affinity on some of its threads in
> order to optimize its performance. Currently this results in QEMU being
> immediately terminated when seccomp is enabled.
> 
> Mesa doesn't consider failure of the process affinity settings to be
> fatal to its operation, but our seccomp policy gives it no choice in
> gracefully handling this denial.
> 
> It is reasonable to consider that malicious code using the resource
> control syscalls to be a less serious attack than if they were trying
> to spawn processes or change UIDs and other such things. Generally
> speaking changing the resource control setting will "merely" affect
> quality of service of processes on the host. With this in mind, rather
> than kill the process, we can relax the policy for these syscalls to
> return the EPERM errno value. This allows callers to detect that QEMU
> does not want them to change resource allocations, and apply some
> reasonable fallback logic.
> 
> The main downside to this is for code which uses these syscalls but does
> not check the return value, blindly assuming they will always
> succeeed. Returning an errno could result in sub-optimal behaviour.
> Arguably though such code is already broken & needs fixing regardless.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  qemu-seccomp.c | 32 +---
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index 36d5829831..9776c9ef40 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -121,20 +121,37 @@ qemu_seccomp(unsigned int operation, unsigned int 
> flags, void *args)
>  #endif
>  }
>  
> -static uint32_t qemu_seccomp_get_kill_action(void)
> +static uint32_t qemu_seccomp_get_kill_action(int set)
>  {
> +switch (set) {
> +case QEMU_SECCOMP_SET_DEFAULT:
> +case QEMU_SECCOMP_SET_OBSOLETE:
> +case QEMU_SECCOMP_SET_PRIVILEGED:
> +case QEMU_SECCOMP_SET_SPAWN: {
>  #if defined(SECCOMP_GET_ACTION_AVAIL) && defined(SCMP_ACT_KILL_PROCESS) && \
>  defined(SECCOMP_RET_KILL_PROCESS)
> -{
> -uint32_t action = SECCOMP_RET_KILL_PROCESS;
> +static int kill_process = -1;
> +if (kill_process == -1) {
> +uint32_t action = SECCOMP_RET_KILL_PROCESS;
>  
> -if (qemu_seccomp(SECCOMP_GET_ACTION_AVAIL, 0, &action) == 0) {
> +if (qemu_seccomp(SECCOMP_GET_ACTION_AVAIL, 0, &action) == 0) {
> +kill_process = 1;
> +}
> +kill_process = 0;
> +}
> +if (kill_process == 1) {
>  return SCMP_ACT_KILL_PROCESS;
>  }
> -}
>  #endif
> +return SCMP_ACT_TRAP;
> +}
> +
> +case QEMU_SECCOMP_SET_RESOURCECTL:
> +return SCMP_ACT_ERRNO(EPERM);
>  
> -return SCMP_ACT_TRAP;
> +default:
> +g_assert_not_reached();
> +}
>  }
>  
>  
> @@ -143,7 +160,6 @@ static int seccomp_start(uint32_t seccomp_opts)
>  int rc = 0;
>  unsigned int i = 0;
>  scmp_filter_ctx ctx;
> -uint32_t action = qemu_seccomp_get_kill_action();
>  
>  ctx = seccomp_init(SCMP_ACT_ALLOW);
>  if (ctx == NULL) {
> @@ -157,10 +173,12 @@ static int seccomp_start(uint32_t seccomp_opts)
>  }
>  
>  for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> +uint32_t action;
>  if (!(seccomp_opts & blacklist[i].set)) {
>  continue;
>  }
>  
> +action = qemu_seccomp_get_kill_action(blacklist[i].set);
>  rc = seccomp_rule_add_array(ctx, action, blacklist[i].num,
>  blacklist[i].narg, blacklist[i].arg_cmp);
>  if (rc < 0) {
> 

Re: [Qemu-devel] [PATCH 5/5] aspeed/timer: Use signed muldiv for timer resets

[Christian Svensson via Qemu-devel]

Thanks for the feedback,

On Thu, Mar 14, 2019, 11:57 Peter Maydell  wrote:

> But overall I'm a little sceptical that the aspeed timer is
> really a special case that needs a signed version of this
> when no other timer in the system does...


I agree, and the v2 of the patch doesn't require it. Happy to submit it
when you folks think it's a good time to merge it.

If you're curious you can get a sneak peek of the patch in
https://github.com/u-root/u-bmc/issues/134.

Regards,

Re: [Qemu-devel] [PATCH 0/5] aspeed/timer: Fix slowdowns in recent Linux

[Christian Svensson via Qemu-devel]

Hi,

My apologies, I don't know how I missed those warnings.
I'll create a new patch.

- Chris


On Thu, Mar 14, 2019 at 10:06 AM Cédric Le Goater  wrote:

> Hello,
>
> For the time being, we can drop patch 5 from this series.
>
> Thanks,
>
> C.
>
> On 3/14/19 9:59 AM, no-re...@patchew.org wrote:
> > Patchew URL:
> https://patchew.org/QEMU/20190314084235.9887-1-...@kaod.org/
> >
> >
> >
> > Hi,
> >
> > This series failed the docker-mingw@fedora build test. Please find the
> testing commands and
> > their output below. If you have Docker installed, you can probably
> reproduce it
> > locally.
> >
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> > time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
> > === TEST SCRIPT END ===
> >
> >   CC  migration/xbzrle.o
> >   CC  migration/postcopy-ram.o
> > /tmp/qemu-test/src/hw/timer/aspeed_timer.c: In function
> 'aspeed_timer_set_value':
> > /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: error: '__int128_t'
> undeclared (first use in this function); did you mean '__int128'?
> >  ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
> >^~
> >__int128
> > /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:19: note: each undeclared
> identifier is reported only once for each function it appears in
> > /tmp/qemu-test/src/hw/timer/aspeed_timer.c:279:30: error: expected ')'
> before 'delta'
> >  ((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
> >  ~^
> >   )
> > /tmp/qemu-test/src/hw/timer/aspeed_timer.c:276:22: error: unused
> variable 'rate' [-Werror=unused-variable]
> >  uint32_t rate = calculate_rate(t);
> >   ^~~~
> > /tmp/qemu-test/src/hw/timer/aspeed_timer.c:275:21: error: unused
> variable 'delta' [-Werror=unused-variable]
> >  int64_t delta = (int64_t) value - (int64_t)
> calculate_ticks(t, now);
> >  ^
> > cc1: all warnings being treated as errors
> >
> >
> > The full log is available at
> >
> http://patchew.org/logs/20190314084235.9887-1-...@kaod.org/testing.docker-mingw@fedora/?type=message
> .
> > ---
> > Email generated automatically by Patchew [http://patchew.org/].
> > Please send your feedback to patchew-de...@redhat.com
> >
>
>

Re: [Qemu-devel] [PATCH 5/5] aspeed/timer: Use signed muldiv for timer resets

[Christian Svensson via Qemu-devel]

Hi all,

I have a new patch but I'm not sure how you want me to post it.
Should I do a "PATCH v2" with a single patch and this thread as the thread
ID?

Thanks,
- Chris


On Thu, Mar 14, 2019 at 10:05 AM Cédric Le Goater  wrote:

> Christian,
>
> Could you please provide a fix for this patch ? patchew complains, see
> attached log.
>
> Thanks,
>
> C.
>
> On 3/14/19 9:42 AM, Cédric Le Goater wrote:
> > From: Christian Svensson 
> >
> > If the host decrements the counter register that results in a negative
> > delta. This is then passed to muldiv64 which only handles unsigned
> > numbers resulting in bogus results.
> >
> > This fix ensures the data being operated on is signed before it is
> > ultimately casted to the final unsigned value.
> >
> > Test case: kexec a kernel using aspeed_timer and it will freeze on the
> > second bootup when the kernel initializes the timer. With this patch
> > that no longer happens and the timer appears to run OK.
> >
> > Signed-off-by: Christian Svensson 
> > [clg: - checkpatch fixes ]
> > Signed-off-by: Cédric Le Goater 
> > ---
> >  hw/timer/aspeed_timer.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
> > index 9988b8fbbf17..0b16eac8970c 100644
> > --- a/hw/timer/aspeed_timer.c
> > +++ b/hw/timer/aspeed_timer.c
> > @@ -275,7 +275,8 @@ static void
> aspeed_timer_set_value(AspeedTimerCtrlState *s, int timer, int reg,
> >  int64_t delta = (int64_t) value - (int64_t)
> calculate_ticks(t, now);
> >  uint32_t rate = calculate_rate(t);
> >
> > -t->start += muldiv64(delta, NANOSECONDS_PER_SECOND, rate);
> > +t->start = (int64_t)t->start +
> > +((__int128_t)delta * NANOSECONDS_PER_SECOND / rate);
> >  aspeed_timer_mod(t);
> >  }
> >  break;
> >
>
>

Re: [Qemu-devel] Several questions about " [PATCH v3 0/5] virtio-9p: hotplug and migration support"

[wangyan]

On 2019/3/11 17:33, Greg Kurz wrote:
> On Tue, 5 Mar 2019 19:28:37 +0800
> wangyan  wrote:
>
>> I am very happy for your reply. 
>>
>> I am very interested in your old WIP and want to take a look. 
>>
>> Our project still use 9pfs, and it needs to live migrate the
>> device while it is being used by the guest, is there any advice
>> for us to make it come true or any WIP about this? 
>>
> It isn't a trivial task, really. The first thing to address is to be able
> to drain outstanding I/O requests. Then, about migration itself, the current
> idea was to close files at pre-save so that they get re-open automatically
> on the target if migration succeeds or on the source if migration fails.
> Then we get to some more gory details: files open with O_EXCL and unlinked
> open files...
I think about the point "unlinked open files...", and have a method to solve 
this problem.
For example, if the guest opens a file, write some data in it, unlinks the file 
but keeps the fd,
then we can use the fd to read all content in the file and put it to the target 
host, and create
a file to save these content, and then unlink the file after the guest opens 
this file in the target
host. It can recover to the original state.

But this is not a good idea, it does a lot of meaningless work. Do you have any 
good idea for this?
>> On 2019/2/26 17:11, Greg Kurz wrote:
>>> On Tue, 26 Feb 2019 11:17:16 +0800
>>> wangyan  wrote:
>>>  
 Dear Kurz,

 Good day.

 This is Yan Wang from china. Glad to contact with you.
  
>>> Hi Yan Wang,
>>>  
 I found your patch "[Qemu-devel] [PATCH v3 0/5] virtio-9p: hotplug and 
 migration support" on the internet. Recently, our project need to use the 
 feature of 9pfs living migration, and I want to know if you have the plan 
 to merge the patch to the master branch of qemu, or do you have any other 
 plans about the migration feature?
  
>>> AFAIK some of these patches have already been merged.
>>>
>>> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=4652f1640e029e1f2433fa77ba6af285c7cd923a
>>> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=6cecf093735f2e5af7d0e29d957350320044e354
>>>
>>> These are enough to live-migrate a quiescent virtio-9p device, ie,
>>> only if the 9p filesystem is NOT mounted in the guest.
>>>
>>> If your need is to live migrate the device while it is being used by
>>> the guest, then this is a different story. Basically, this requires
>>> to quiesce the device so that its state can be captured. Also, some
>>> state belongs to the host and maybe hard to capture/migrate. For
>>> example, if the guest opens a file, write some data in it, unlinks the
>>> file but keeps the fd, it expects to be able to read the same data
>>> back, even after migration to some other host system.
>>>
>>> 9pfs is no longer a priority for anyone in the industry. I have no
>>> plan to make this go forward, an not much time to work on it. But
>>> if you want to have a look, I guess I can share some old WIP I have
>>> somewhere.
>>>  
 Sincerely,

 Yan Wang
  
>>> Cheers,
>>>
>>> --
>>> Greg
>>>
>>> .
>>>  
>>
>
> .
>

Re: [Qemu-devel] [PULL 15/54] build: convert pci.mak to Kconfig

[Andrea Bolognani]

On Mon, 2019-03-04 at 19:19 +0100, Paolo Bonzini wrote:
> Instead of including the same list of devices for each target,
> set CONFIG_PCI to true, and make the devices default to present
> whenever PCI is available.  However, s390x does not want all the
> PCI devices, so there is a separate symbol to enable them.
[...]
> +++ b/default-configs/riscv32-softmmu.mak
> @@ -1,8 +1,8 @@
>  # Default configuration for riscv-softmmu
>  
> -include pci.mak
>  include usb.mak
> -
> +CONFIG_PCI=y
> +CONFIG_PCI_DEVICES=y
>  CONFIG_SERIAL=y
>  CONFIG_VIRTIO_MMIO=y

I *think* this might have caused some unwanted changes for RISC-V.

pcie-root-port is built into qemu-system-riscv64 by default as of
dbbc277510aa (along with ioh3420!), but if you actually try to use it
you'll get:

  $ ./riscv64-softmmu/qemu-system-riscv64 \
-M virt \
-device pcie-root-port
  qemu-system-riscv64: -device pcie-root-port: MSI-X is not
   supported by interrupt controller

This is a limitation we have been aware of, and the plan was to
enable the device in QEMU once it had been addressed: from the
libvirt side, the availability of the device would have meant that it
was safe to use it, but if the device is enabled in QEMU before it
can actually be used, then that makes detection on the libvirt side
problematic.

I haven't spent time digging further - and I'm not familiar enough
with the QEMU build system anyway O:-) - but I wouldn't be surprised
if the same happened for other architectures, too.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [Qemu-devel] [PULL 15/54] build: convert pci.mak to Kconfig

[Paolo Bonzini]

On 14/03/19 13:53, Andrea Bolognani wrote:
> 
>   $ ./riscv64-softmmu/qemu-system-riscv64 \
> -M virt \
> -device pcie-root-port
>   qemu-system-riscv64: -device pcie-root-port: MSI-X is not
>supported by interrupt controller
> 
> This is a limitation we have been aware of, and the plan was to
> enable the device in QEMU once it had been addressed: from the
> libvirt side, the availability of the device would have meant that it
> was safe to use it, but if the device is enabled in QEMU before it
> can actually be used, then that makes detection on the libvirt side
> problematic.

Interesting, I didn't know that.  Kconfig is perfect for expressing this
kind of dependency though.  I'll send a series soon.

Paolo

Re: [Qemu-devel] [PATCH v3 0/8] slirp: clarify license of slirp as BSD-3

[Samuel Thibault]

Hello,

Should this go through my tree, or perhaps it can be directly pushed to
master by Peter since it's no-code-only-copyright changes?

Samuel

Marc-André Lureau, le jeu. 14 mars 2019 14:10:41 +0100, a ecrit:
> In order to make slirp a standalone project, the project must have a
> clear license, and be compatible with the GPL or LGPL.
> 
> Since commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove the
> advertising clause from the slirp license"), slirp is BSD-3. But new
> files have been added under slirp/ with QEMU GPL license since then.
> 
> v3:
>  - add preliminary "slirp: update COPYRIGHT to use full 3-Clause BSD
>License"
>  - split "slirp: clarify license of slirp files using SPDX" patch,
>  - fix SPDX for files with MIT license header
>  - add r-b tags, wording/spelling changes
> 
> v2:
>  - split the initial patch to add BSD-3 header & then SPDX lines
>  - do not modify existing copyright headers without copyright holder
>authorization
>  - drop the weak/ambiguous notice to the COPYRIGHT file
>  - added a RFC patch to remove Kelly Price from the maintainer duties
> 
> Marc-André Lureau (8):
>   slirp: update COPYRIGHT to use full 3-Clause BSD License
>   slirp: relicense GPL files to BSD-3
>   slirp: clarify license of slirp files using SPDX
>   slirp: clarify license of slirp files using SPDX
>   slirp: clarify license of slirp files using SPDX
>   slirp: clarify license of slirp files using SPDX
>   slirp: remove reference to COPYRIGHT file
>   slirp: is not maintained by Kelly Price for a long time
> 
>  slirp/src/bootp.h  |  1 +
>  slirp/src/debug.h  |  4 +---
>  slirp/src/dhcpv6.h | 32 +--
>  slirp/src/if.h |  4 +---
>  slirp/src/ip.h |  1 +
>  slirp/src/ip6.h|  1 +
>  slirp/src/ip6_icmp.h   |  1 +
>  slirp/src/ip_icmp.h|  1 +
>  slirp/src/libslirp.h   |  1 +
>  slirp/src/main.h   |  4 +---
>  slirp/src/mbuf.h   |  1 +
>  slirp/src/misc.h   |  4 +---
>  slirp/src/ncsi-pkt.h   | 34 +
>  slirp/src/qtailq.h |  1 +
>  slirp/src/sbuf.h   |  4 +---
>  slirp/src/slirp.h  |  1 +
>  slirp/src/socket.h |  4 +---
>  slirp/src/stream.h |  1 +
>  slirp/src/tcp.h|  1 +
>  slirp/src/tcp_timer.h  |  1 +
>  slirp/src/tcp_var.h|  1 +
>  slirp/src/tcpip.h  |  1 +
>  slirp/src/tftp.h   |  1 +
>  slirp/src/udp.h|  1 +
>  slirp/src/util.h   |  1 +
>  slirp/src/vmstate.h| 43 +++---
>  slirp/src/arp_table.c  |  1 +
>  slirp/src/bootp.c  |  1 +
>  slirp/src/cksum.c  |  1 +
>  slirp/src/dhcpv6.c | 38 +++--
>  slirp/src/dnssearch.c  |  1 +
>  slirp/src/if.c |  4 +---
>  slirp/src/ip6_icmp.c   |  1 +
>  slirp/src/ip6_input.c  |  1 +
>  slirp/src/ip6_output.c |  1 +
>  slirp/src/ip_icmp.c|  1 +
>  slirp/src/ip_input.c   |  4 +---
>  slirp/src/ip_output.c  |  4 +---
>  slirp/src/mbuf.c   |  4 +---
>  slirp/src/misc.c   |  4 +---
>  slirp/src/ncsi.c   | 32 +--
>  slirp/src/ndp_table.c  |  1 +
>  slirp/src/sbuf.c   |  4 +---
>  slirp/src/slirp.c  |  1 +
>  slirp/src/socket.c |  4 +---
>  slirp/src/state.c  |  1 +
>  slirp/src/stream.c |  1 +
>  slirp/src/tcp_input.c  |  4 +---
>  slirp/src/tcp_output.c |  4 +---
>  slirp/src/tcp_subr.c   |  4 +---
>  slirp/src/tcp_timer.c  |  1 +
>  slirp/src/tftp.c   |  1 +
>  slirp/src/udp.c|  1 +
>  slirp/src/udp6.c   |  1 +
>  slirp/src/util.c   |  1 +
>  slirp/src/vmstate.c| 32 +--
>  slirp/COPYRIGHT|  5 +++--
>  57 files changed, 229 insertions(+), 85 deletions(-)
> 
> -- 
> 2.21.0.4.g36eb1cb9cf
> 

-- 
Samuel
As usual, this being a 1.3.x release, I haven't even compiled this
kernel yet.  So if it works, you should be doubly impressed.
(Linus Torvalds, announcing kernel 1.3.3 on the linux-kernel mailing list.)

Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix SMMUv3 GSIV values

[Peter Maydell]

On Tue, 12 Mar 2019 at 09:10, Eric Auger  wrote:
>
> The GSIV numbers of the SPI based interrupts is not correct as
> ARM_SPI_BASE was not added to the irqmap[VIRT_SMMU] value. So
> this may collide with VIRTIO_MMIO irq window.
>
> Signed-off-by: Eric Auger 
> ---
>  hw/arm/virt-acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index d7e2e4885b..aa02d8d74e 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -405,7 +405,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  its->identifiers[0] = 0; /* MADT translation_id */
>
>  if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> -int irq =  vms->irqmap[VIRT_SMMU];
> +int irq =  vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
>
>  /* SMMUv3 node */
>  smmu_offset = iort_node_offset + node_size;
> --



Applied to target-arm.next, thanks.

-- PMM

[Qemu-devel] [PATCH v3 7/8] slirp: remove reference to COPYRIGHT file

[Marc-André Lureau]

The slirp COPYRIGHT file is a BSD-3 license. Instead of referring to
another project file, the SPDX license notice present in all source
files states that unequivocally.

Signed-off-by: Marc-André Lureau 
---
 slirp/src/debug.h  | 3 ---
 slirp/src/if.h | 3 ---
 slirp/src/main.h   | 3 ---
 slirp/src/misc.h   | 3 ---
 slirp/src/sbuf.h   | 3 ---
 slirp/src/socket.h | 3 ---
 slirp/src/if.c | 3 ---
 slirp/src/ip_input.c   | 3 ---
 slirp/src/ip_output.c  | 3 ---
 slirp/src/mbuf.c   | 3 ---
 slirp/src/misc.c   | 3 ---
 slirp/src/sbuf.c   | 3 ---
 slirp/src/socket.c | 3 ---
 slirp/src/tcp_input.c  | 3 ---
 slirp/src/tcp_output.c | 3 ---
 slirp/src/tcp_subr.c   | 3 ---
 16 files changed, 48 deletions(-)

diff --git a/slirp/src/debug.h b/slirp/src/debug.h
index 2e503ad7fa..c95fd8ffd2 100644
--- a/slirp/src/debug.h
+++ b/slirp/src/debug.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef DEBUG_H_
diff --git a/slirp/src/if.h b/slirp/src/if.h
index 8a60c4e052..b71c37d6ea 100644
--- a/slirp/src/if.h
+++ b/slirp/src/if.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef IF_H
diff --git a/slirp/src/main.h b/slirp/src/main.h
index a88774215f..3b3f883703 100644
--- a/slirp/src/main.h
+++ b/slirp/src/main.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef SLIRP_MAIN_H
diff --git a/slirp/src/misc.h b/slirp/src/misc.h
index 4eaa2466d7..23b7490448 100644
--- a/slirp/src/misc.h
+++ b/slirp/src/misc.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef MISC_H
diff --git a/slirp/src/sbuf.h b/slirp/src/sbuf.h
index ece616e317..337af1bbde 100644
--- a/slirp/src/sbuf.h
+++ b/slirp/src/sbuf.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef SBUF_H
diff --git a/slirp/src/socket.h b/slirp/src/socket.h
index 10a0c78a26..25403898cd 100644
--- a/slirp/src/socket.h
+++ b/slirp/src/socket.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef SLIRP_SOCKET_H
diff --git a/slirp/src/if.c b/slirp/src/if.c
index b8cddebf66..6eaac7292a 100644
--- a/slirp/src/if.c
+++ b/slirp/src/if.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/ip_input.c b/slirp/src/ip_input.c
index 6ad6765938..a714fecd58 100644
--- a/slirp/src/ip_input.c
+++ b/slirp/src/ip_input.c
@@ -34,9 +34,6 @@
 /*
  * Changes and additions relating to SLiRP are
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/ip_output.c b/slirp/src/ip_output.c
index 927efb..8560197cf6 100644
--- a/slirp/src/ip_output.c
+++ b/slirp/src/ip_output.c
@@ -34,9 +34,6 @@
 /*
  * Changes and additions relating to SLiRP are
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/mbuf.c b/slirp/src/mbuf.c
index f079a86d78..800406ca9e 100644
--- a/slirp/src/mbuf.c
+++ b/slirp/src/mbuf.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 /*
diff --git a/slirp/src/misc.c b/slirp/src/misc.c
index da41f3bb5f..7c5db0e0aa 100644
--- a/slirp/src/misc.c
+++ b/slirp/src/misc.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/sbuf.c b/slirp/src/sbuf.c
index 815823ffbe..9c0b31b513 100644
--- a/slirp/src/sbuf.c
+++ b/slirp/src/sbuf.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyr

[Qemu-devel] [PATCH v3 8/8] slirp: is not maintained by Kelly Price for a long time

[Marc-André Lureau]

slirp has been maintained by the QEMU maintainers and will be
maintained under an independent project soon.

Reviewed-by: Eric Blake 
Signed-off-by: Kelly Price 
Signed-off-by: Marc-André Lureau 
---
 slirp/COPYRIGHT | 2 --
 1 file changed, 2 deletions(-)

diff --git a/slirp/COPYRIGHT b/slirp/COPYRIGHT
index 9863ea31cb..ed49512dbc 100644
--- a/slirp/COPYRIGHT
+++ b/slirp/COPYRIGHT
@@ -1,8 +1,6 @@
 Slirp was written by Danny Gasparovski.
 Copyright (c), 1995,1996 All Rights Reserved.
 
-Slirp is maintained by Kelly Price 
-
 Slirp is free software; "free" as in you don't have to pay for it, and you
 are free to do whatever you want with it.  I do not accept any donations,
 monetary or otherwise, for Slirp.  Instead, I would ask you to pass this
-- 
2.21.0.4.g36eb1cb9cf

[Qemu-devel] [PATCH v3 6/8] slirp: clarify license of slirp files using SPDX

[Marc-André Lureau]

Add SPDX license identifier to clarify the license of files without
explicit license header.

Signed-off-by: Marc-André Lureau 
---
 slirp/src/bootp.h  | 1 +
 slirp/src/ip6.h| 1 +
 slirp/src/ip6_icmp.h   | 1 +
 slirp/src/libslirp.h   | 1 +
 slirp/src/slirp.h  | 1 +
 slirp/src/stream.h | 1 +
 slirp/src/tftp.h   | 1 +
 slirp/src/ip6_icmp.c   | 1 +
 slirp/src/ip6_input.c  | 1 +
 slirp/src/ip6_output.c | 1 +
 slirp/src/ndp_table.c  | 1 +
 slirp/src/udp6.c   | 1 +
 12 files changed, 12 insertions(+)

diff --git a/slirp/src/bootp.h b/slirp/src/bootp.h
index 4043489835..d881ad620a 100644
--- a/slirp/src/bootp.h
+++ b/slirp/src/bootp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /* bootp/dhcp defines */
 
 #ifndef SLIRP_BOOTP_H
diff --git a/slirp/src/ip6.h b/slirp/src/ip6.h
index 1b3364f960..33683c8e20 100644
--- a/slirp/src/ip6.h
+++ b/slirp/src/ip6.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ip6_icmp.h b/slirp/src/ip6_icmp.h
index e8ed753db5..d8d13e30fc 100644
--- a/slirp/src/ip6_icmp.h
+++ b/slirp/src/ip6_icmp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/libslirp.h b/slirp/src/libslirp.h
index 2d13950065..3b28764bec 100644
--- a/slirp/src/libslirp.h
+++ b/slirp/src/libslirp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 #ifndef LIBSLIRP_H
 #define LIBSLIRP_H
 
diff --git a/slirp/src/slirp.h b/slirp/src/slirp.h
index 8068ba1d1e..39580934f3 100644
--- a/slirp/src/slirp.h
+++ b/slirp/src/slirp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 #ifndef SLIRP_H
 #define SLIRP_H
 
diff --git a/slirp/src/stream.h b/slirp/src/stream.h
index 985334c043..08bb5b6610 100644
--- a/slirp/src/stream.h
+++ b/slirp/src/stream.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 #ifndef STREAM_H_
 #define STREAM_H_
 
diff --git a/slirp/src/tftp.h b/slirp/src/tftp.h
index a4c4a64e64..3fe3b70205 100644
--- a/slirp/src/tftp.h
+++ b/slirp/src/tftp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /* tftp defines */
 
 #ifndef SLIRP_TFTP_H
diff --git a/slirp/src/ip6_icmp.c b/slirp/src/ip6_icmp.c
index c1e3d30470..5642457fdd 100644
--- a/slirp/src/ip6_icmp.c
+++ b/slirp/src/ip6_icmp.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ip6_input.c b/slirp/src/ip6_input.c
index 1b8c003c66..d9d2b7e9cd 100644
--- a/slirp/src/ip6_input.c
+++ b/slirp/src/ip6_input.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ip6_output.c b/slirp/src/ip6_output.c
index 19d1ae7748..b86110662c 100644
--- a/slirp/src/ip6_output.c
+++ b/slirp/src/ip6_output.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ndp_table.c b/slirp/src/ndp_table.c
index 34ea4fdf1f..78324877e2 100644
--- a/slirp/src/ndp_table.c
+++ b/slirp/src/ndp_table.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/udp6.c b/slirp/src/udp6.c
index be5cba1f54..bfcc7ec6fa 100644
--- a/slirp/src/udp6.c
+++ b/slirp/src/udp6.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron
-- 
2.21.0.4.g36eb1cb9cf