Re: [Qemu-devel] [PATCH v2 00/23] Add RISC-V TCG backend support

[Richard Henderson]

On 12/19/18 11:16 AM, Alistair Francis wrote:
> This patch set adds RISC-V backend support to QEMU. This is based on
> Michael Clark's original work with extra work on top.
> 
> This has been somewhat tested and can run other architecture softmmu
> code. It seems that any complex OS will eventually hang, but we can
> run the BIOS and OS startup code for a number of different operating
> systems.
> 
> I haven't tested linux user support at all yet. I think Michael had that
> working reliably though and hopefully my changes haven't broken it.
> 
> There are still some todos in the code (there are missing instructions
> and byte swapping) but these should assert instead of generating invalid
> code.

Queued to tcg-next, with the extrh fix.

Some of those todos are no longer todos, since e.g. bswap is now optional.
Those asserts should never fire (as a good assert should do, I suppose).

The missing instructions are only for riscv32, which afaik is just now making
its way to glibc.  So a chroot complete enough to build qemu is a ways away.
I'm ok with leaving that incomplete for now.


r~

Re: [Qemu-devel] [PATCH v3 1/2] intel-iommu: differentiate host address width from IOVA address width.

[Yu Zhang]

On Wed, Dec 19, 2018 at 11:47:23AM -0500, Michael S. Tsirkin wrote:
> On Wed, Dec 19, 2018 at 11:40:37AM +0100, Igor Mammedov wrote:
> > On Wed, 19 Dec 2018 10:57:17 +0800
> > Yu Zhang  wrote:
> > 
> > > On Tue, Dec 18, 2018 at 03:55:36PM +0100, Igor Mammedov wrote:
> > > > On Tue, 18 Dec 2018 17:27:23 +0800
> > > > Yu Zhang  wrote:
> > > >   
> > > > > On Mon, Dec 17, 2018 at 02:17:40PM +0100, Igor Mammedov wrote:  
> > > > > > On Wed, 12 Dec 2018 21:05:38 +0800
> > > > > > Yu Zhang  wrote:
> > > > > >   
> > > > > > > Currently, vIOMMU is using the value of IOVA address width, 
> > > > > > > instead of
> > > > > > > the host address width(HAW) to calculate the number of reserved 
> > > > > > > bits in
> > > > > > > data structures such as root entries, context entries, and 
> > > > > > > entries of
> > > > > > > DMA paging structures etc.
> > > > > > > 
> > > > > > > However values of IOVA address width and of the HAW may not 
> > > > > > > equal. For
> > > > > > > example, a 48-bit IOVA can only be mapped to host addresses no 
> > > > > > > wider than
> > > > > > > 46 bits. Using 48, instead of 46 to calculate the reserved bit 
> > > > > > > may result
> > > > > > > in an invalid IOVA being accepted.
> > > > > > > 
> > > > > > > To fix this, a new field - haw_bits is introduced in struct 
> > > > > > > IntelIOMMUState,
> > > > > > > whose value is initialized based on the maximum physical address 
> > > > > > > set to
> > > > > > > guest CPU.  
> > > > > >   
> > > > > > > Also, definitions such as VTD_HOST_AW_39/48BIT etc. are renamed
> > > > > > > to clarify.
> > > > > > > 
> > > > > > > Signed-off-by: Yu Zhang 
> > > > > > > Reviewed-by: Peter Xu 
> > > > > > > ---  
> > > > > > [...]
> > > > > >   
> > > > > > > @@ -3100,6 +3104,8 @@ static void 
> > > > > > > vtd_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n)
> > > > > > >  static void vtd_init(IntelIOMMUState *s)
> > > > > > >  {
> > > > > > >  X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
> > > > > > > +CPUState *cs = first_cpu;
> > > > > > > +X86CPU *cpu = X86_CPU(cs);
> > > > > > >  
> > > > > > >  memset(s->csr, 0, DMAR_REG_SIZE);
> > > > > > >  memset(s->wmask, 0, DMAR_REG_SIZE);
> > > > > > > @@ -3119,23 +3125,24 @@ static void vtd_init(IntelIOMMUState *s)
> > > > > > >  s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND |
> > > > > > >   VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS |
> > > > > > >   VTD_CAP_SAGAW_39bit | VTD_CAP_MGAW(s->aw_bits);
> > > > > > > -if (s->aw_bits == VTD_HOST_AW_48BIT) {
> > > > > > > +if (s->aw_bits == VTD_AW_48BIT) {
> > > > > > >  s->cap |= VTD_CAP_SAGAW_48bit;
> > > > > > >  }
> > > > > > >  s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO;
> > > > > > > +s->haw_bits = cpu->phys_bits;  
> > > > > > Is it possible to avoid accessing CPU fields directly or cpu 
> > > > > > altogether
> > > > > > and set phys_bits when iommu is created?  
> > > > > 
> > > > > Thanks for your comments, Igor.
> > > > > 
> > > > > Well, I guess you prefer not to query the CPU capabilities while 
> > > > > deciding
> > > > > the vIOMMU features. But to me, they are not that irrelevant.:)
> > > > > 
> > > > > Here the hardware address width in vt-d, and the one in 
> > > > > cpuid.MAXPHYSADDR
> > > > > are referring to the same concept. In VM, both are the maximum guest 
> > > > > physical
> > > > > address width. If we do not check the CPU field here, we will still 
> > > > > have to
> > > > > check the CPU field in other places such as build_dmar_q35(), and 
> > > > > reset the
> > > > > s->haw_bits again.
> > > > > 
> > > > > Is this explanation convincing enough? :)  
> > > > current build_dmar_q35() doesn't do it, it's all new code in this 
> > > > series that
> > > > contains not acceptable direct access from one device (iommu) to 
> > > > another (cpu).   
> > > > Proper way would be for the owner of iommu to fish limits from 
> > > > somewhere and set
> > > > values during iommu creation.  
> > > 
> > > Well, current build_dmar_q35() doesn't do it, because it is using the 
> > > incorrect value. :)
> > > According to the spec, the host address width is the maximum physical 
> > > address width,
> > > yet current implementation is using the DMA address width. For me, this 
> > > is not only
> > > wrong, but also unsecure. For this point, I think we all agree this need 
> > > to be fixed.
> > > 
> > > As to how to fix it - should we query the cpu fields, I still do not 
> > > understand why
> > > this is not acceptable. :)
> > > 
> > > I had thought of other approaches before, yet I did not choose:
> > > 
> > > 1> Introduce a new parameter, say, "x-haw-bits" which is used for iommu 
> > > to limit its  
> > > physical address width(similar to the "x-aw-bits" for IOVA). But what 
> > > should we check
> > > this parameter or not? What if this parameter is set to sth. different 
> > > than the "phys-bits"
> > > or not?
> > > 
> > > 2> Another choice I had 

Re: [Qemu-devel] [PATCH v2 18/23] riscv: tcg-target: Add the out op decoder

[Richard Henderson]

On 12/19/18 11:19 AM, Alistair Francis wrote:
> +case INDEX_op_ext32s_i64:
> +case INDEX_op_extrl_i64_i32:
> +case INDEX_op_extrh_i64_i32:
> +case INDEX_op_ext_i32_i64:
> +tcg_out_ext32s(s, a0, a1);
> +break;

This is the last bug that's easy to identify.  It shows up quickly in a
linux-user smoke test [1].  Obviously extrl (extract low) and extrh (extract
high) should not be implemented identically.

Fixed thus.

With this, most of the tests pass, when run in a qemu-riscv chroot.  The ones
that don't primarily never get started.  E.g. the alpha test fails to map
ld.so, for reasons that I have not examined, since it's not a tcg problem.


r~


[1] https://archive.li/o/1YduE/wiki.qemu.org/download/linux-user-test-0.3.tar.gz


---
 tcg/riscv/tcg-target.inc.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index f718542d63..6cf8de32b5 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -1614,11 +1614,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,

 case INDEX_op_ext32s_i64:
 case INDEX_op_extrl_i64_i32:
-case INDEX_op_extrh_i64_i32:
 case INDEX_op_ext_i32_i64:
 tcg_out_ext32s(s, a0, a1);
 break;

+case INDEX_op_extrh_i64_i32:
+tcg_out_opc_imm(s, OPC_SRAI, a0, a1, 32);
+break;
+
 case INDEX_op_mulsh_i32:
 case INDEX_op_mulsh_i64:
 tcg_out_opc_reg(s, OPC_MULH, a0, a1, a2);
---

Re: [Qemu-devel] [PATCH v3 2/2] intel-iommu: extend VTD emulation to allow 57-bit IOVA address width.

[Yu Zhang]

On Wed, Dec 19, 2018 at 10:23:44AM -0500, Michael S. Tsirkin wrote:
> On Wed, Dec 19, 2018 at 01:57:43PM +0800, Yu Zhang wrote:
> > On Tue, Dec 18, 2018 at 11:35:34PM -0500, Michael S. Tsirkin wrote:
> > > On Wed, Dec 19, 2018 at 11:40:06AM +0800, Yu Zhang wrote:
> > > > On Tue, Dec 18, 2018 at 09:49:02AM -0500, Michael S. Tsirkin wrote:
> > > > > On Tue, Dec 18, 2018 at 09:45:41PM +0800, Yu Zhang wrote:
> > > > > > On Tue, Dec 18, 2018 at 07:43:28AM -0500, Michael S. Tsirkin wrote:
> > > > > > > On Tue, Dec 18, 2018 at 06:01:16PM +0800, Yu Zhang wrote:
> > > > > > > > On Tue, Dec 18, 2018 at 05:47:14PM +0800, Yu Zhang wrote:
> > > > > > > > > On Mon, Dec 17, 2018 at 02:29:02PM +0100, Igor Mammedov wrote:
> > > > > > > > > > On Wed, 12 Dec 2018 21:05:39 +0800
> > > > > > > > > > Yu Zhang  wrote:
> > > > > > > > > > 
> > > > > > > > > > > A 5-level paging capable VM may choose to use 57-bit IOVA 
> > > > > > > > > > > address width.
> > > > > > > > > > > E.g. guest applications may prefer to use its VA as IOVA 
> > > > > > > > > > > when performing
> > > > > > > > > > > VFIO map/unmap operations, to avoid the burden of 
> > > > > > > > > > > managing the IOVA space.
> > > > > > > > > > > 
> > > > > > > > > > > This patch extends the current vIOMMU logic to cover the 
> > > > > > > > > > > extended address
> > > > > > > > > > > width. When creating a VM with 5-level paging feature, 
> > > > > > > > > > > one can choose to
> > > > > > > > > > > create a virtual VTD with 5-level paging capability, with 
> > > > > > > > > > > configurations
> > > > > > > > > > > like "-device intel-iommu,x-aw-bits=57".
> > > > > > > > > > > 
> > > > > > > > > > > Signed-off-by: Yu Zhang 
> > > > > > > > > > > Reviewed-by: Peter Xu 
> > > > > > > > > > > ---
> > > > > > > > > > > Cc: "Michael S. Tsirkin" 
> > > > > > > > > > > Cc: Marcel Apfelbaum 
> > > > > > > > > > > Cc: Paolo Bonzini 
> > > > > > > > > > > Cc: Richard Henderson 
> > > > > > > > > > > Cc: Eduardo Habkost 
> > > > > > > > > > > Cc: Peter Xu 
> > > > > > > > > > > ---
> > > > > > > > > > >  hw/i386/intel_iommu.c  | 53 
> > > > > > > > > > > --
> > > > > > > > > > >  hw/i386/intel_iommu_internal.h | 10 ++--
> > > > > > > > > > >  include/hw/i386/intel_iommu.h  |  1 +
> > > > > > > > > > >  3 files changed, 50 insertions(+), 14 deletions(-)
> > > > > > > > > > > 
> > > > > > > > > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > > > > > > > > > > index 0e88c63..871110c 100644
> > > > > > > > > > > --- a/hw/i386/intel_iommu.c
> > > > > > > > > > > +++ b/hw/i386/intel_iommu.c
> > > > > > > > > > > @@ -664,16 +664,16 @@ static inline bool 
> > > > > > > > > > > vtd_iova_range_check(uint64_t iova, VTDContextEntry *ce,
> > > > > > > > > > >  
> > > > > > > > > > >  /*
> > > > > > > > > > >   * Rsvd field masks for spte:
> > > > > > > > > > > - * Index [1] to [4] 4k pages
> > > > > > > > > > > - * Index [5] to [8] large pages
> > > > > > > > > > > + * Index [1] to [5] 4k pages
> > > > > > > > > > > + * Index [6] to [10] large pages
> > > > > > > > > > >   */
> > > > > > > > > > > -static uint64_t vtd_paging_entry_rsvd_field[9];
> > > > > > > > > > > +static uint64_t vtd_paging_entry_rsvd_field[11];
> > > > > > > > > > >  
> > > > > > > > > > >  static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, 
> > > > > > > > > > > uint32_t level)
> > > > > > > > > > >  {
> > > > > > > > > > >  if (slpte & VTD_SL_PT_PAGE_SIZE_MASK) {
> > > > > > > > > > >  /* Maybe large page */
> > > > > > > > > > > -return slpte & vtd_paging_entry_rsvd_field[level 
> > > > > > > > > > > + 4];
> > > > > > > > > > > +return slpte & vtd_paging_entry_rsvd_field[level 
> > > > > > > > > > > + 5];
> > > > > > > > > > >  } else {
> > > > > > > > > > >  return slpte & 
> > > > > > > > > > > vtd_paging_entry_rsvd_field[level];
> > > > > > > > > > >  }
> > > > > > > > > > > @@ -3127,6 +3127,8 @@ static void 
> > > > > > > > > > > vtd_init(IntelIOMMUState *s)
> > > > > > > > > > >   VTD_CAP_SAGAW_39bit | 
> > > > > > > > > > > VTD_CAP_MGAW(s->aw_bits);
> > > > > > > > > > >  if (s->aw_bits == VTD_AW_48BIT) {
> > > > > > > > > > >  s->cap |= VTD_CAP_SAGAW_48bit;
> > > > > > > > > > > +} else if (s->aw_bits == VTD_AW_57BIT) {
> > > > > > > > > > > +s->cap |= VTD_CAP_SAGAW_57bit | 
> > > > > > > > > > > VTD_CAP_SAGAW_48bit;
> > > > > > > > > > >  }
> > > > > > > > > > >  s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO;
> > > > > > > > > > >  s->haw_bits = cpu->phys_bits;
> > > > > > > > > > > @@ -3139,10 +3141,12 @@ static void 
> > > > > > > > > > > vtd_init(IntelIOMMUState *s)
> > > > > > > > > > >  vtd_paging_entry_rsvd_field[2] = 
> > > > > > > > > > > VTD_SPTE_PAGE_L2_RSVD_MASK(s->haw_bits);
> > > > > > > > > > >  vtd_paging_entry_rsvd_field[3] = 
> > > > > > > > > > > VTD_SPTE_PAGE_L3_RSVD_MASK(s->haw_bits);
> > > > > > > > > > > 

[Qemu-devel] [PATCH v2 2/3] x86-iommu: switch intr_supported to OnOffAuto type

[Peter Xu]

Switch the intr_supported variable from a boolean to OnOffAuto type so
that we can know whether the user specified it or not.  With that
we'll have a chance to help the user to choose more wisely where
possible.  Introduce x86_iommu_ir_supported() to mask these changes.

No functional change at all.

Signed-off-by: Peter Xu 
---
 hw/i386/acpi-build.c|  6 +++---
 hw/i386/amd_iommu.c |  2 +-
 hw/i386/intel_iommu.c   |  6 +++---
 hw/i386/pc.c|  2 +-
 hw/i386/x86-iommu.c | 15 +--
 include/hw/i386/x86-iommu.h |  4 +++-
 6 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 236a20eaa8..7012f97cac 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2426,7 +2426,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
 IntelIOMMUState *intel_iommu = INTEL_IOMMU_DEVICE(iommu);
 
 assert(iommu);
-if (iommu->intr_supported) {
+if (x86_iommu_ir_supported(iommu)) {
 dmar_flags |= 0x1;  /* Flags: 0x1: INT_REMAP */
 }
 
@@ -2499,7 +2499,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
  * When interrupt remapping is supported, we add a special IVHD device
  * for type IO-APIC.
  */
-if (x86_iommu_get_default()->intr_supported) {
+if (x86_iommu_ir_supported(x86_iommu_get_default())) {
 ivhd_table_len += 8;
 }
 /* IVHD length */
@@ -2535,7 +2535,7 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
  * Linux IOMMU driver checks for the special IVHD device (type IO-APIC).
  * See Linux kernel commit 'c2ff5cf5294bcbd7fa50f7d860e90a66db7e5059'
  */
-if (x86_iommu_get_default()->intr_supported) {
+if (x86_iommu_ir_supported(x86_iommu_get_default())) {
 build_append_int_noprefix(table_data,
  (0x1ull << 56) |   /* type IOAPIC */
  (IOAPIC_SB_DEVID << 40) |  /* IOAPIC devid */
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 353a810e6b..8ad707aba0 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1233,7 +1233,7 @@ static int amdvi_int_remap_msi(AMDVIState *iommu,
 }
 
 /* validate that we are configure with intremap=on */
-if (!X86_IOMMU_DEVICE(iommu)->intr_supported) {
+if (!x86_iommu_ir_supported(X86_IOMMU_DEVICE(iommu))) {
 trace_amdvi_err("Interrupt remapping is enabled in the guest but "
 "not in the host. Use intremap=on to enable interrupt "
 "remapping in amd-iommu.");
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index d97bcbc2f7..3df4b0a550 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3137,7 +3137,7 @@ static void vtd_init(IntelIOMMUState *s)
 vtd_paging_entry_rsvd_field[7] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits);
 vtd_paging_entry_rsvd_field[8] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s->aw_bits);
 
-if (x86_iommu->intr_supported) {
+if (x86_iommu_ir_supported(x86_iommu)) {
 s->ecap |= VTD_ECAP_IR | VTD_ECAP_MHMV;
 if (s->intr_eim == ON_OFF_AUTO_ON) {
 s->ecap |= VTD_ECAP_EIM;
@@ -3238,14 +3238,14 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
 
-if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) {
+if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu_ir_supported(x86_iommu)) {
 error_setg(errp, "eim=on cannot be selected without intremap=on");
 return false;
 }
 
 if (s->intr_eim == ON_OFF_AUTO_AUTO) {
 s->intr_eim = (kvm_irqchip_in_kernel() || s->buggy_eim)
-  && x86_iommu->intr_supported ?
+  && x86_iommu_ir_supported(x86_iommu) ?
   ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
 }
 if (s->intr_eim == ON_OFF_AUTO_ON && !s->buggy_eim) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 115bc2825c..d95a0e3ad1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1244,7 +1244,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 if (pcms->apic_id_limit > 255 && !xen_enabled()) {
 IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
 
-if (!iommu || !iommu->x86_iommu.intr_supported ||
+if (!iommu || !x86_iommu_ir_supported(X86_IOMMU_DEVICE(iommu)) ||
 iommu->intr_eim != ON_OFF_AUTO_ON) {
 error_report("current -smp configuration requires "
  "Extended Interrupt Mode enabled. "
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index abc3c03158..61ee0f1eaa 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -119,8 +119,13 @@ static void x86_iommu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+/* If the user didn't specify IR, choose a default value for it */
+if (x86_iommu->intr_supported == ON_OFF_AUTO_AUTO) 

Re: [Qemu-devel] Monitor and serial output window broken with SDL2

[Gerd Hoffmann]

On Wed, Dec 19, 2018 at 07:11:34PM +0100, BALATON Zoltan wrote:
> On Wed, 19 Dec 2018, Daniel P. Berrangé wrote:
> > On Tue, Dec 18, 2018 at 08:05:59PM +0100, BALATON Zoltan wrote:
> > > On Tue, 18 Dec 2018, Daniel P. Berrangé wrote:
> > > > I don't see any difference depending on whether I use Ctrl-Alt-3 before
> > > > or after the yellow screen - both work as expected. The fact that you
> > > > see a difference though does suggest that there's something odd being
> > > > done in the ppc emulation that is in turn doing something bad to SDLs
> > > > rendering with your graphics setup. I don't know what it could be 
> > > > though.
> > > 
> > > OK thanks for checking this, at least it seems this is something that not
> > > reproduces elsewhere so it's probably something specific to my setup. I 
> > > may
> > > try to debug this further when I'll have time.
> > 
> > I think there's a good chance that this is still a bug in QEMU, so
> > definitely still worth trying to debug what in your setup triggers
> > it.
> 
> Looks like Howard (cc'd) also has similar issue on Fedora 29 with NVidia
> vendor driver although newer version than mine so the proprietary NVidia
> driver seems to be a common point here. The difference is that for him the
> monitor window seems to hijack the guest screen not the serial screen like
> for me but this could be the same bug. I can't easily test with nouveau
> driver now so that's all we could find out so far. Also having some output
> on serial window seems to be relevant as this works before client starts if
> I start with qemu-system-ppc -S but breaks after continue in monitor window.
> qemu-system-x86_64 also does not show the problem but maybe because there's
> no content in serial window there?

qemu-system-x86_64 -machine graphics=off will make seabios print stuff
to the serial line.

cheers,
  Gerd

[Qemu-devel] [PATCH v2 1/3] q35: set split kernel irqchip as default

[Peter Xu]

Starting from QEMU 4.0, let's specify "split" as the default value for
kernel-irqchip.

So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
   for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N
   (omitting all the "kernel_irqchip_" prefix)

Note that this will let the default q35 machine type to depend on
Linux version 4.4 or newer because that's where split irqchip is
introduced in kernel.  But it's fine since we're boosting supported
Linux version for QEMU 4.0 to around Linux 4.5.  For more information
please refer to the discussion on AMD's RDTSCP:

  https://lore.kernel.org/lkml/20181210181328.ga...@zn.tnic/

Signed-off-by: Peter Xu 
---
 hw/core/machine.c   | 2 ++
 hw/i386/pc_q35.c| 2 ++
 include/hw/boards.h | 1 +
 3 files changed, 5 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index c51423b647..4439ea663f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -653,8 +653,10 @@ static void machine_class_base_init(ObjectClass *oc, void 
*data)
 static void machine_initfn(Object *obj)
 {
 MachineState *ms = MACHINE(obj);
+MachineClass *mc = MACHINE_GET_CLASS(obj);
 
 ms->kernel_irqchip_allowed = true;
+ms->kernel_irqchip_split = mc->default_kernel_irqchip_split;
 ms->kvm_shadow_mem = -1;
 ms->dump_guest_core = true;
 ms->mem_merge = true;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 58459bdab5..d2fb0fa49f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -304,6 +304,7 @@ static void pc_q35_machine_options(MachineClass *m)
 m->units_per_default_bus = 1;
 m->default_machine_opts = "firmware=bios-256k.bin";
 m->default_display = "std";
+m->default_kernel_irqchip_split = true;
 m->no_floppy = 1;
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
@@ -323,6 +324,7 @@ DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
 static void pc_q35_3_1_machine_options(MachineClass *m)
 {
 pc_q35_4_0_machine_options(m);
+m->default_kernel_irqchip_split = false;
 m->alias = NULL;
 SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
 }
diff --git a/include/hw/boards.h b/include/hw/boards.h
index f82f28468b..362384815e 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -195,6 +195,7 @@ struct MachineClass {
 const char *hw_version;
 ram_addr_t default_ram_size;
 const char *default_cpu_type;
+bool default_kernel_irqchip_split;
 bool option_rom_has_mr;
 bool rom_file_has_mr;
 int minimum_page_bits;
-- 
2.17.1

[Qemu-devel] [PATCH v2 3/3] x86-iommu: turn on IR by default if proper

[Peter Xu]

When the user didn't specify "intremap" for the IOMMU device, we turn
it on by default if it is supported.  This will turn IR on for the
default Q35 platform as long as the IOMMU device is specified on new
kernels.

Signed-off-by: Peter Xu 
---
 hw/i386/x86-iommu.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 61ee0f1eaa..d1534c1ae0 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -112,6 +112,7 @@ static void x86_iommu_realize(DeviceState *dev, Error 
**errp)
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
+bool irq_all_kernel = kvm_irqchip_in_kernel() && !kvm_irqchip_is_split();
 
 if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by IOMMU",
@@ -121,12 +122,12 @@ static void x86_iommu_realize(DeviceState *dev, Error 
**errp)
 
 /* If the user didn't specify IR, choose a default value for it */
 if (x86_iommu->intr_supported == ON_OFF_AUTO_AUTO) {
-x86_iommu->intr_supported = ON_OFF_AUTO_OFF;
+x86_iommu->intr_supported = irq_all_kernel ?
+ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
 }
 
 /* Both Intel and AMD IOMMU IR only support "kernel-irqchip={off|split}" */
-if (x86_iommu_ir_supported(x86_iommu) && kvm_irqchip_in_kernel() &&
-!kvm_irqchip_is_split()) {
+if (x86_iommu_ir_supported(x86_iommu) && irq_all_kernel) {
 error_setg(errp, "Interrupt Remapping cannot work with "
  "kernel-irqchip=on, please use 'split|off'.");
 return;
-- 
2.17.1

[Qemu-devel] [PATCH v2 0/3] q35: change defaults for kernel irqchip and IR

[Peter Xu]

v2:
- drop patch 1, so we don't even need to consider old kernels for the
  default value [Paolo, Eduardo]
- fix up commit message of the split irqchip patch to mention that

= Original cover letter 

This only changes q35.  Nothing else.

Before this series, we have these default parameters:

  - machine kernel-irqchip: on
  - intel-iommu IR: off

This series wants to change these default variables into:

  - machine kernel-irqchip: split
  - intel-iommu IR: on

and at the meantime we should keep compatibility with old kernels and
old versions of QEMU.

For old versions of QEMU: we used machine compat properties.

For old kernels (<4.4): if user didn't specify split kernel irqchip,
we'll take it only as the first priority if it's supported by the
kernel; otherwise, we will continue with complete kernel irqchip.

Both of these parameters should be good to have (split irqchip gains
more security, while IR gets it too but even more, like x2apic).  So
let's try to make them as default if capable.

Tests ("split" stands for whether kernel split irqchip enabled, "IR"
stands for whether IR is turned on):

   |+---+|
   | params | split | IR |
   |+---+|
   | -M q35 | 0 | /  |
   | -M q35,accel=kvm   | 1 | /  |
   | -M pc-q35-3.1,accel=kvm| 0 | /  |
   | -M q35,accel=kvm,kernel-irqchip=off| 0 | /  |
   | -M q35,accel=kvm,kernel-irqchip=on | 0 | /  |
   | -M q35 -device intel-iommu | 0 | 1  |
   | -M q35,accel=kvm -device intel-iommu   | 1 | 1  |
   | -M q35,accel=kvm,kernel-irqchip=on -device intel-iommu | 0 | 0  |
   |+---+|

I didn't try old kernels, though.

Any comment would be welcomed, thanks.

Peter Xu (3):
  q35: set split kernel irqchip as default
  x86-iommu: switch intr_supported to OnOffAuto type
  x86-iommu: turn on IR by default if proper

 hw/core/machine.c   |  2 ++
 hw/i386/acpi-build.c|  6 +++---
 hw/i386/amd_iommu.c |  2 +-
 hw/i386/intel_iommu.c   |  6 +++---
 hw/i386/pc.c|  2 +-
 hw/i386/pc_q35.c|  2 ++
 hw/i386/x86-iommu.c | 18 +++---
 include/hw/boards.h |  1 +
 include/hw/i386/x86-iommu.h |  4 +++-
 9 files changed, 31 insertions(+), 12 deletions(-)

-- 
2.17.1

Re: [Qemu-devel] [PATCH V7 6/6] hostmem-file: add 'sync' option

[Yi Zhang]

On 2018-12-19 at 22:42:07 -0500, Michael S. Tsirkin wrote:
> On Thu, Dec 20, 2018 at 11:03:12AM +0800, Yi Zhang wrote:
> > On 2018-12-19 at 10:59:10 -0500, Michael S. Tsirkin wrote:
> > > On Wed, Dec 19, 2018 at 05:10:18PM +0800, Yi Zhang wrote:
> > > > > > +
> > > > > > + - 'sync' option of memory-backend-file is not 'off', and
> > > > > > +
> > > > > > + - 'share' option of memory-backend-file is 'on'.
> > > > > > +
> > > > > > + - 'pmem' option of memory-backend-file is 'on'
> > > > > > +
> > > > > 
> > > > > Wait isn't this what pmem was supposed to do?
> > > > > Doesn't it mean "persistent memory"?
> > > > pmem is a option for memory-backend-file, user should know the backend
> > > > is in host persistent memory, with this flags on, while there is a host 
> > > > crash
> > > > or a power failures.
> > > > 
> > > > [1] Qemu will take necessary operations to guarantee the persistence.
> > > > https://patchwork.ozlabs.org/cover/944749/ 
> > > > 
> > > > [2] Host kernel also take opretions to consistent filesystem metadata.
> > > > Add MAP_SYNC flags.
> > > 
> > > OK so I'm a user. Can you educate me please?  
> > We suppose an administrator should know it, what is the back-end region 
> > coming from,
> > is it persistent? what is the font-end device is? a volatile dimm or an
> > nonvolatile dimm? then they can choice put the pmem=on[off] and 
> > sync=on[off].
> > If he didn't, we encourage that don't set these 2 flags.
> > 
> > > When should MAP_SYNC not
> > > be set? Are there any disadvantages (e.g.  performance?)?
> > Not only the performance, sometimes like the front-end device is an
> > volatile ram, we don't wanna set such option although the backend is a
> > novolatile memory, if power lose, all of thing should lose in this ram.
> 
> 
> 
> I am not sure how does above answer the questions. If I don't know,
> neither will the hypothetical administrator. Looks like a better
> interface is needed to make the choice on behalf of the user.
> 
By default, we have already ignore the 2 flags, unless the administrator
know how to make that use. By-now, seems we don't have a better way to detect 
what
memory-backend-file is, a persistent memory or not.
> 
> > > 
> > > -- 
> > > MST
> > > 
> 

Re: [Qemu-devel] [PATCH 2/4] q35: set split kernel irqchip as default

[Peter Xu]

On Wed, Dec 19, 2018 at 10:45:40PM +0100, Paolo Bonzini wrote:
> On 19/12/18 22:24, Eduardo Habkost wrote:
> > On Wed, Dec 19, 2018 at 09:12:55PM +0100, Paolo Bonzini wrote:
> >> On 19/12/18 09:50, Peter Xu wrote:
> >>> Starting from QEMU 4.0, let's specify "split" as the default value for
> >>> kernel-irqchip.
> >>>
> >>> So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
> >>>for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N
> >>>(omitting all the "kernel_irqchip_" prefix)
> >>>
> >>> Note that this "split" is optional - we'll first try to enable split
> >>> kernel irqchip, and we'll fall back to complete kernel irqchip if we
> >>> found that the kernel capability is missing.
> >>
> >> Please just fail completely and require a new kernel for the 4.0 machine
> >> type.  There are subtle differences between kernel and QEMU irqchip, I
> >> don't think we want to open that can of worms.
> > 
> > This would make existing VMs that are runnable with pc-q35-3.1.0
> > not runnable by only updating the machine-type.
> > 
> > The good news is that we can make this a non-issue by clearly
> > documenting that QEMU needs a more recent kernel (just like we'll
> > do for RDTSCP[1]).
> 
> Right, RDTSCP is exactly what came to mind.

Ok so I think I'll just make it even simpler by dropping patch 1.
Also I noticed that the documentation on linux kernel version
requirement has not yet reached master but I'll assume it'll be there
some day very soon so I'll ignore that part.

Thanks everyone!  I'll repost soon.

-- 
Peter Xu

[Qemu-devel] [Bug 1808824] Re: Mouse leaves VM window when Grab on Hover isn't selected Windows 10 and Intel HAX

[William Razgunas]

https://youtu.be/Vpi59ptOiyc
Here is a video demonstrating the main issue. This does not show any lag but, 
it does show how VM doesn't capture the mouse correctly.

After capture if you move the mouse far enough off the VM window the mouse goes 
out of the window in odd places that isn't representative of the location of 
where the mouse is.
The mouse will leave the VM from the middle of the of VM window at times.

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

Title:
  Mouse leaves VM window when Grab on Hover isn't selected Windows 10
  and Intel HAX

Status in QEMU:
  New

Bug description:
  On Windows 10.0.17134 I have been having the problem that the mouse
  will leave the VM window after a short time when grab on hover isn't
  selected.  The VM will then try to grab on Hover and the mouse will
  grab in weird places and it will become very unwieldy to control the
  mouse in the VM window.

  This is exasperated by super slow response making it nearly unusable
  if the Intel® Hardware Accelerated Execution Manager (Intel® HAXM) is
  not currently installed on my machine.

  I know they are different things but they compounded on each other
  when you have a mouse that is not staying in the VM window and the
  VM's visualized cpu is acting VERY slow the system is unusable.

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

[Qemu-devel] [PATCH] disas/microblaze: Remove unused REG_SP macro

[Richard Henderson]

This causes a build error with debian sid, riscv64 host:

disas/microblaze.c:179: error: "REG_SP" redefined [-Werror]
 #define REG_SP  1 /* stack pointer */

In file included from /usr/include/signal.h:306,
 from include/qemu/osdep.h:101,
 from disas/microblaze.c:36:
/usr/include/riscv64-linux-gnu/sys/ucontext.h:36: note: this is the location of 
the previous definition
 # define REG_SP 2

Signed-off-by: Richard Henderson 
---
 disas/microblaze.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/disas/microblaze.c b/disas/microblaze.c
index 598ecbc89d..c23605043a 100644
--- a/disas/microblaze.c
+++ b/disas/microblaze.c
@@ -176,7 +176,6 @@ enum microblaze_instr_type {
 #define REG_TLBSX 36869 /* MMU: TLB Search Index reg */
 
 /* alternate names for gen purpose regs */
-#define REG_SP  1 /* stack pointer */
 #define REG_ROSDP 2 /* read-only small data pointer */
 #define REG_RWSDP 13 /* read-write small data pointer */
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH v2 for-4.0 1/7] chardev: Add disconnected option for chardev socket

[Yongji Xie]

On Wed, 19 Dec 2018 at 23:55, Michael S. Tsirkin  wrote:
>
> On Tue, Dec 18, 2018 at 04:09:19PM +, Daniel P. Berrangé wrote:
> > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote:
> > > On Tue, Dec 18, 2018 at 03:25:20PM +, Daniel P. Berrangé wrote:
> > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote:
> > > > > Hi
> > > > >
> > > > > On Tue, Dec 18, 2018 at 2:01 PM  wrote:
> > > > > >
> > > > > > From: Xie Yongji 
> > > > > >
> > > > > > New option "disconnected" is added to init the chardev socket
> > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected()
> > > > > > to connect when necessary. Now it would be used for unix domain
> > > > > > socket of vhost-user-blk device to support reconnect.
> > > > >
> > > > > What difference does that make if you wait for connection in
> > > > > qemu_chr_fe_wait_connected(), or during chardev setup?
> > > > >
> > > > > "disconnected" is misleading, would it be possible to reuse
> > > > > "wait/nowait" instead?
> > > >
> > > > Currently we default to doing a blocking connect in foreground,
> > > > except if reconnect is non-zero, in which case we do a connect
> > > > async in the background. This "disconnected" proposal effectively
> > > > does a blocking connect, but delayed to later in startup.
> > > >
> > > > IOW, this could already be achieved if "reconnect" were set to
> > > > non-zero. If the usage doesn't want reconnect though, I tend
> > > > to agree that we should use the exisiting wait/nowait options
> > > > to let it be controlled. I don't see that this "disconnected"
> > > > option gives a compelling benefit over using wait/nowait.
> > >
> > > So the tricky thing is that we can not expose the
> > > device to guest until we get a connection and can query
> > > it for the first time. After that is completed,
> > > we can reasonably support disconnected operation at least for net.
> >
> > The device code would still use  qemu_chr_fe_wait_connected() in my 
> > proposal,
> > so that its no different to the situation with the proposed "disconnected"
> > flag.
> >
> > Regards,
> > Daniel
>
> I guess so, but wouldn't it be confusing to users that one says
> "nowait" and qemu still waits for a connection and does not
> run the VM? That's different from how nowait behaves normally.
>

With this patch:

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 2464d7a..ffe3a60 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -97,7 +97,10 @@ static void qemu_chr_socket_restart_timer(Chardev *chr)
 SocketChardev *s = SOCKET_CHARDEV(chr);
 char *name;

-assert(s->connected == 0);
+if (s->connected) {
+return;
+}
+
 name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label);
 s->reconnect_timer = qemu_chr_timeout_add_ms(chr,
  s->reconnect_time * 1000,

We can use qemu_chr_fe_wait_connected() without adding redundant
"wait" or "disconnected" option.
The disadavantage is that backend may have two connection from qemu
during initialization. But I think backend should support this case.

Thanks,
Yongji

Re: [Qemu-devel] [PATCH V7 6/6] hostmem-file: add 'sync' option

[Michael S. Tsirkin]

On Thu, Dec 20, 2018 at 11:03:12AM +0800, Yi Zhang wrote:
> On 2018-12-19 at 10:59:10 -0500, Michael S. Tsirkin wrote:
> > On Wed, Dec 19, 2018 at 05:10:18PM +0800, Yi Zhang wrote:
> > > > > +
> > > > > + - 'sync' option of memory-backend-file is not 'off', and
> > > > > +
> > > > > + - 'share' option of memory-backend-file is 'on'.
> > > > > +
> > > > > + - 'pmem' option of memory-backend-file is 'on'
> > > > > +
> > > > 
> > > > Wait isn't this what pmem was supposed to do?
> > > > Doesn't it mean "persistent memory"?
> > > pmem is a option for memory-backend-file, user should know the backend
> > > is in host persistent memory, with this flags on, while there is a host 
> > > crash
> > > or a power failures.
> > > 
> > > [1] Qemu will take necessary operations to guarantee the persistence.
> > > https://patchwork.ozlabs.org/cover/944749/ 
> > > 
> > > [2] Host kernel also take opretions to consistent filesystem metadata.
> > > Add MAP_SYNC flags.
> > 
> > OK so I'm a user. Can you educate me please?  
> We suppose an administrator should know it, what is the back-end region 
> coming from,
> is it persistent? what is the font-end device is? a volatile dimm or an
> nonvolatile dimm? then they can choice put the pmem=on[off] and sync=on[off].
> If he didn't, we encourage that don't set these 2 flags.
> 
> > When should MAP_SYNC not
> > be set? Are there any disadvantages (e.g.  performance?)?
> Not only the performance, sometimes like the front-end device is an
> volatile ram, we don't wanna set such option although the backend is a
> novolatile memory, if power lose, all of thing should lose in this ram.



I am not sure how does above answer the questions. If I don't know,
neither will the hypothetical administrator. Looks like a better
interface is needed to make the choice on behalf of the user.


> > 
> > -- 
> > MST
> > 

Re: [Qemu-devel] [PATCH for-4.0 v8 3/7] migration: fix the multifd code when receiving less channels

[Fei Li]

On 12/19/2018 10:11 PM, Markus Armbruster wrote:

Fei Li  writes:


On 12/13/2018 02:17 PM, Markus Armbruster wrote:

Fei Li  writes:


In our current code, when multifd is used during migration, if there
is an error before the destination receives all new channels, the
source keeps running, however the destination does not exit but keeps
waiting until the source is killed deliberately.

Fix this by dumping the specific error and let users decide whether
to quit from the destination side when failing to receive packet via
some channel.

Cc: Dr. David Alan Gilbert 
Signed-off-by: Fei Li 
Reviewed-by: Peter Xu 

[...]

diff --git a/migration/migration.h b/migration/migration.h
index e413d4d8b6..02b7304610 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -229,7 +229,7 @@ struct MigrationState
   void migrate_set_state(int *state, int old_state, int new_state);
 void migration_fd_process_incoming(QEMUFile *f);
-void migration_ioc_process_incoming(QIOChannel *ioc);
+void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp);
   void migration_incoming_process(void);
 bool  migration_has_all_channels(void);
diff --git a/migration/ram.c b/migration/ram.c
index 7e7deec4d8..c7e3d6b0fd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1323,7 +1323,7 @@ bool multifd_recv_all_channels_created(void)
   }
 /* Return true if multifd is ready for the migration, otherwise
false */
-bool multifd_recv_new_channel(QIOChannel *ioc)
+bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
   {
   MultiFDRecvParams *p;
   Error *local_err = NULL;
@@ -1331,6 +1331,10 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
 id = multifd_recv_initial_packet(ioc, &local_err);
   if (id < 0) {
+error_propagate_prepend(errp, local_err,
+"failed to receive packet"
+" via multifd channel %d: ",
+atomic_read(&multifd_recv_state->count));
   multifd_recv_terminate_threads(local_err);
   return false;

Here, we return false without setting an error.

I am not sure whether I understand correctly, but here I think the above
error_propagate_prepend() set the error to errp.

You're right, I got confused.

However, you shouldn't access @local_err after error_propagate() or
similar.  Please insert error_propagate_prepend() after
multifd_recv_terminate_threads(), lik you do in the next hunk.

Right, thanks for the reminder.

Have a nice day :)
Fei



   }
@@ -1340,6 +1344,7 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
   error_setg(&local_err, "multifd: received id '%d' already setup'",
  id);
   multifd_recv_terminate_threads(local_err);
+error_propagate(errp, local_err);
   return false;

Here, we return false with setting an error.


   }
   p->c = ioc;
@@ -1351,7 +1356,8 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
   qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
  QEMU_THREAD_JOINABLE);
   atomic_inc(&multifd_recv_state->count);
-return multifd_recv_state->count == migrate_multifd_channels();
+return atomic_read(&multifd_recv_state->count) ==
+   migrate_multifd_channels();

Here, we return either true of false without setting an error.

yes.

Taken together, there are three cases:

1. Succeed and return true

Yes, when all multifd channels are correctly received.

2. Succeed and return false

Yes, when the current multifd channel is received correctly, but
have not received all the channels.

Aha.


3. Fail (set an error) and return false.

Yes. And with the propagated error, the code just returns and
report the error in migration_channel_process_incoming().

Assuming that's what we want: please update the function comment to
spell them out.

Ok, I will update the three cases in the comment to clarify in detail.

Have a nice day, thanks :)

You're welcome!

Re: [Qemu-devel] [PATCH for-4.0 v8 6/7] qemu_thread_create: propagate the error to callers to handle

[Fei Li]

On 12/20/2018 01:29 AM, Eric Blake wrote:

On 12/19/18 6:14 AM, Fei Li wrote:


   28 files changed, 243 insertions(+), 101 deletions(-)


I recommend to split this patch.  First part adds the Error ** 
parameter

to qemu_thread_create(), passing &error_abort everywhere. No functional
change.  Subsequent patches then improve on &error_abort. This way,
each improvement patch can be cc'ed to just that part's maintainer(s).
Parts you don't want to touch you simply leave at &error_abort.  Makes
sense?
Yes, I think this makes sense, much clearer. :) But I am a little 
worried about

whether too many subsequent improvement patches (some of them are quite
small changes) are acceptable.


A long series of small patches, where each patch is cc'd to an 
appropriate maintainer, will likely get cumulative reviews faster than 
a single monolithic patch where no one person is the expert on every 
line touched.



Ok, thanks for the advice!

Have a nice day
Fei

Re: [Qemu-devel] [PATCH V7 6/6] hostmem-file: add 'sync' option

[Yi Zhang]

On 2018-12-19 at 10:59:10 -0500, Michael S. Tsirkin wrote:
> On Wed, Dec 19, 2018 at 05:10:18PM +0800, Yi Zhang wrote:
> > > > +
> > > > + - 'sync' option of memory-backend-file is not 'off', and
> > > > +
> > > > + - 'share' option of memory-backend-file is 'on'.
> > > > +
> > > > + - 'pmem' option of memory-backend-file is 'on'
> > > > +
> > > 
> > > Wait isn't this what pmem was supposed to do?
> > > Doesn't it mean "persistent memory"?
> > pmem is a option for memory-backend-file, user should know the backend
> > is in host persistent memory, with this flags on, while there is a host 
> > crash
> > or a power failures.
> > 
> > [1] Qemu will take necessary operations to guarantee the persistence.
> > https://patchwork.ozlabs.org/cover/944749/ 
> > 
> > [2] Host kernel also take opretions to consistent filesystem metadata.
> > Add MAP_SYNC flags.
> 
> OK so I'm a user. Can you educate me please?  
We suppose an administrator should know it, what is the back-end region coming 
from,
is it persistent? what is the font-end device is? a volatile dimm or an
nonvolatile dimm? then they can choice put the pmem=on[off] and sync=on[off].
If he didn't, we encourage that don't set these 2 flags.

> When should MAP_SYNC not
> be set? Are there any disadvantages (e.g.  performance?)?
Not only the performance, sometimes like the front-end device is an
volatile ram, we don't wanna set such option although the backend is a
novolatile memory, if power lose, all of thing should lose in this ram.

> 
> -- 
> MST
> 

Re: [Qemu-devel] [PATCH v5 11/11] iotests: add iotest 236 for testing bitmap merge

[Eric Blake]

On 12/19/18 8:29 PM, John Snow wrote:

New interface, new smoke test.

Signed-off-by: John Snow 
---
  tests/qemu-iotests/236 | 161 +
  tests/qemu-iotests/236.out | 351 +
  tests/qemu-iotests/group   |   1 +
  3 files changed, 513 insertions(+)
  create mode 100755 tests/qemu-iotests/236
  create mode 100644 tests/qemu-iotests/236.out



Reviewed-by: Eric Blake 

(and glad that my insistence on beefing up the test has caught bugs)

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

Re: [Qemu-devel] [PATCH v5 10/11] iotests: implement pretty-print for log and qmp_log

[Eric Blake]

On 12/19/18 8:29 PM, John Snow wrote:

If iotests have lines exceeding >998 characters long, git doesn't
want to send it plaintext to the list. We can solve this by allowing
the iotests to use pretty printed QMP output that we can match against
instead.

As a bonus, it's much nicer for human eyes too.

Signed-off-by: John Snow 
---
  tests/qemu-iotests/iotests.py | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)



Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v5 09/11] iotests: change qmp_log filters to expect QMP objects only

[Eric Blake]

On 12/19/18 8:29 PM, John Snow wrote:

As laid out in the previous commit's message:

```
Several places in iotests deal with serializing objects into JSON
strings, but to add pretty-printing it seems desireable to localize


s/desireable/desirable/


all of those cases.

log() seems like a good candidate for that centralized behavior.
log() can already serialize json objects, but when it does so,
it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP
objects into strings and filtering them assuming that filters=[]
are string filters.
```

Therefore:

Change qmp_log to treat filters as if they're always qmp object filters,
then change the logging call to rely on log()'s ability to serialize QMP
objects, so we're not duplicating that effort.

Add a qmp version of filter_testfiles and adjust the only caller using
it for qmp_log to use the qmp version.

Signed-off-by: John Snow  
Signed-off-by: John Snow 


Odd double S-o-B differing only by space.


---
  tests/qemu-iotests/206|  4 ++--
  tests/qemu-iotests/iotests.py | 24 +---
  2 files changed, 23 insertions(+), 5 deletions(-)



Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v5 08/11] iotests: remove default filters from qmp_log

[Eric Blake]

On 12/19/18 8:29 PM, John Snow wrote:

Several places in iotests deal with serializing objects into JSON
strings, but to add pretty-printing it seems desireable to localize


s/desireable/desirable/


all of those cases.

log() seems like a good candidate for that centralized behavior.
log() can already serialize json objects, but when it does so,
it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP
objects into strings and filtering them assuming that filters=[]
are string filters.

To have qmp_log use log's serialization, qmp_log will need to
accept only qmp filters, not text filters.

However, only a single caller of qmp_log actually requires any
filters at all. I remove the default filter and add it explicitly
to the caller in preparation for refactoring qmp_log to use rich
filters instead.

test 206 is amended to name the filter explicitly and the default
is removed.

Signed-off-by: John Snow 
---
  tests/qemu-iotests/206| 8 ++--
  tests/qemu-iotests/iotests.py | 2 +-
  2 files changed, 7 insertions(+), 3 deletions(-)



Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v5 03/11] blockdev: n-ary bitmap merge

[Eric Blake]

On 12/19/18 8:29 PM, John Snow wrote:

Especially outside of transactions, it is helpful to provide
all-or-nothing semantics for bitmap merges. This facilitates
the coalescing of multiple bitmaps into a single target for
the "checkpoint" interpretation when assembling bitmaps that
represent arbitrary points in time from component bitmaps.

This is an incompatible change from the preliminary version
of the API.


but that doesn't matter because it was in the x- namespace, and we're 
about to rename it anyway.




Signed-off-by: John Snow 
---
  blockdev.c   | 75 ++--
  qapi/block-core.json | 22 ++---
  2 files changed, 62 insertions(+), 35 deletions(-)




+static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
+const char *target,
+strList *bitmaps,
+HBitmap **backup,
+Error **errp)
  {



-bdrv_merge_dirty_bitmap(dst, src, NULL, errp);
+for (lst = bitmaps; lst; lst = lst->next) {
+src = bdrv_find_dirty_bitmap(bs, lst->value);
+if (!src) {
+error_setg(errp, "Dirty bitmap '%s' not found", lst->value);
+dst = NULL;
+goto out;
+}
+
+bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+dst = NULL;
+goto out;
+}
+}


Appears to be a silent no-op when given "bitmaps":[] as the source.  An 
alternative would be requiring at least one source in the list, but I 
don't see it as worth changing the patch to special-case an empty list 
differently from a no-op.



@@ -1943,23 +1943,23 @@
  ##
  # @x-block-dirty-bitmap-merge:
  #
-# FIXME: Rename @src_name and @dst_name to src-name and dst-name.
-#
-# Merge @src_name dirty bitmap to @dst_name dirty bitmap. @src_name dirty
-# bitmap is unchanged. On error, @dst_name is unchanged.
+# Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
+# The @bitmaps dirty bitmaps are unchanged.
+# On error, @target is unchanged.
  #
  # Returns: nothing on success
  #  If @node is not a valid block device, DeviceNotFound
-#  If @dst_name or @src_name is not found, GenericError
-#  If bitmaps has different sizes or granularities, GenericError
+#  If any bitmap in @bitmaps or @target is not found, GenericError
+#  If any of the bitmaps have different sizes or granularities,
+#  GenericError
  #
  # Since: 3.0


Could do s/3.0/4.0/ to match the incompatible change here, but you do it 
in the later patch where your remove the x-.


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v5 02/11] block/dirty-bitmap: remove assertion from restore

[Eric Blake]

On 12/19/18 8:29 PM, John Snow wrote:

When making a backup of a dirty bitmap (for transactions), we want to
restore that backup whether or not the bitmap is enabled or not.


drop one of the two 'or not'



It is perfectly valid to write into bitmaps that are disabled. It is
only illegitimate for the guest to have done so.

Remove this assertion.

Signed-off-by: John Snow 
---
  block/dirty-bitmap.c | 1 -
  1 file changed, 1 deletion(-)


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v5 07/11] iotests: add qmp recursive sorting function

[Eric Blake]

On 12/19/18 8:29 PM, John Snow wrote:

Python before 3.6 does not sort dictionaries (including kwargs).
Therefore, printing QMP objects involves sorting the keys to have
a predictable ordering in the iotests output.


It may be worth also mentioning that sometimes this sorting results in 
the log showing things in a different order than the source command 
(with no ill effect, as long as the output order is deterministic).




However, if we want to pretty-print QMP objects being sent to the
QEMU process, we need to build the entire command before logging it.
Ordinarily, this would then involve "arguments" being sorted above
"execute", which would necessitate a rather ugly and harder-to-read
change to many iotests outputs.

To facilitate pretty-printing AND maintaining predictable output AND
having "arguments" sort before "execute", add a custom sort function


s/before/after/


that takes a dictionary and recursively builds an OrderedDict that
maintains the specific key order we wish to see in iotests output.


namely, keys within subdicts are sorted by key name (even if that is not 
the order they were input), but the top-level struct with "execute" and 
"arguments" stays the way we want it.




Signed-off-by: John Snow 
---
  tests/qemu-iotests/iotests.py | 24 
  1 file changed, 20 insertions(+), 4 deletions(-)


Reviewed-by: Eric Blake 

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

[Qemu-devel] [PATCH v5 06/11] iotests: add filter_generated_node_ids

[John Snow]

To mimic the common filter of the same name, but for the python tests.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index a34e66813a..9595429fea 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -239,6 +239,9 @@ def filter_testfiles(msg):
 prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
 return msg.replace(prefix, 'TEST_DIR/PID-')
 
+def filter_generated_node_ids(msg):
+return re.sub("#block[0-9]+", "NODE_NAME", msg)
+
 def filter_img_info(output, filename):
 lines = []
 for line in output.split('\n'):
-- 
2.17.2

[Qemu-devel] [PATCH v5 09/11] iotests: change qmp_log filters to expect QMP objects only

[John Snow]

As laid out in the previous commit's message:

```
Several places in iotests deal with serializing objects into JSON
strings, but to add pretty-printing it seems desireable to localize
all of those cases.

log() seems like a good candidate for that centralized behavior.
log() can already serialize json objects, but when it does so,
it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP
objects into strings and filtering them assuming that filters=[]
are string filters.
```

Therefore:

Change qmp_log to treat filters as if they're always qmp object filters,
then change the logging call to rely on log()'s ability to serialize QMP
objects, so we're not duplicating that effort.

Add a qmp version of filter_testfiles and adjust the only caller using
it for qmp_log to use the qmp version.

Signed-off-by: John Snow  
Signed-off-by: John Snow 
---
 tests/qemu-iotests/206|  4 ++--
 tests/qemu-iotests/iotests.py | 24 +---
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
index e92550fa59..5bb738bf23 100755
--- a/tests/qemu-iotests/206
+++ b/tests/qemu-iotests/206
@@ -27,7 +27,7 @@ iotests.verify_image_format(supported_fmts=['qcow2'])
 
 def blockdev_create(vm, options):
 result = vm.qmp_log('blockdev-create',
-filters=[iotests.filter_testfiles],
+filters=[iotests.filter_qmp_testfiles],
 job_id='job0', options=options)
 
 if 'return' in result:
@@ -55,7 +55,7 @@ with iotests.FilePath('t.qcow2') as disk_path, \
   'size': 0 })
 
 vm.qmp_log('blockdev-add',
-   filters=[iotests.filter_testfiles],
+   filters=[iotests.filter_qmp_testfiles],
driver='file', filename=disk_path,
node_name='imgfile')
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 57fe20db45..dcd0c6f71d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -246,10 +246,29 @@ def filter_qmp_event(event):
 event['timestamp']['microseconds'] = 'USECS'
 return event
 
+def filter_qmp(qmsg, filter_fn):
+'''Given a string filter, filter a QMP object's values.
+filter_fn takes a (key, value) pair.'''
+for key in qmsg:
+if isinstance(qmsg[key], list):
+qmsg[key] = [filter_qmp(atom, filter_fn) for atom in qmsg[key]]
+elif isinstance(qmsg[key], dict):
+qmsg[key] = filter_qmp(qmsg[key], filter_fn)
+else:
+qmsg[key] = filter_fn(key, qmsg[key])
+return qmsg
+
 def filter_testfiles(msg):
 prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
 return msg.replace(prefix, 'TEST_DIR/PID-')
 
+def filter_qmp_testfiles(qmsg):
+def _filter(key, value):
+if key == 'filename' or key == 'backing-file':
+return filter_testfiles(value)
+return value
+return filter_qmp(qmsg, _filter)
+
 def filter_generated_node_ids(msg):
 return re.sub("#block[0-9]+", "NODE_NAME", msg)
 
@@ -465,10 +484,9 @@ class VM(qtest.QEMUQtestMachine):
 ("execute", cmd),
 ("arguments", ordered_kwargs(kwargs))
 ))
-logmsg = json.dumps(full_cmd)
-log(logmsg, filters)
+log(full_cmd, filters)
 result = self.qmp(cmd, **kwargs)
-log(json.dumps(result, sort_keys=True), filters)
+log(result, filters)
 return result
 
 def run_job(self, job, auto_finalize=True, auto_dismiss=False):
-- 
2.17.2

[Qemu-devel] [PATCH v5 05/11] iotests.py: don't abort if IMGKEYSECRET is undefined

[John Snow]

Instead of using os.environ[], use .get with a default of empty string
to match the setup in check to allow us to import the iotests module
(for debugging, say) without needing a crafted environment just to
import the module.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20181214231512.5295-5-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index d537538ba0..a34e66813a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -63,7 +63,7 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 
'socket_scm_helper')
 debug = False
 
 luks_default_secret_object = 'secret,id=keysec0,data=' + \
- os.environ['IMGKEYSECRET']
+ os.environ.get('IMGKEYSECRET', '')
 luks_default_key_secret_opt = 'key-secret=keysec0'
 
 
-- 
2.17.2

[Qemu-devel] [PATCH v5 07/11] iotests: add qmp recursive sorting function

[John Snow]

Python before 3.6 does not sort dictionaries (including kwargs).
Therefore, printing QMP objects involves sorting the keys to have
a predictable ordering in the iotests output.

However, if we want to pretty-print QMP objects being sent to the
QEMU process, we need to build the entire command before logging it.
Ordinarily, this would then involve "arguments" being sorted above
"execute", which would necessitate a rather ugly and harder-to-read
change to many iotests outputs.

To facilitate pretty-printing AND maintaining predictable output AND
having "arguments" sort before "execute", add a custom sort function
that takes a dictionary and recursively builds an OrderedDict that
maintains the specific key order we wish to see in iotests output.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 9595429fea..565eebb1ab 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -30,6 +30,7 @@ import signal
 import logging
 import atexit
 import io
+from collections import OrderedDict
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts'))
 import qtest
@@ -75,6 +76,16 @@ def qemu_img(*args):
 sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' 
'.join(qemu_img_args + list(args
 return exitcode
 
+def ordered_kwargs(kwargs):
+# kwargs prior to 3.6 are not ordered, so:
+od = OrderedDict()
+for k, v in sorted(kwargs.items()):
+if isinstance(v, dict):
+od[k] = ordered_kwargs(v)
+else:
+od[k] = v
+return od
+
 def qemu_img_create(*args):
 args = list(args)
 
@@ -257,8 +268,10 @@ def filter_img_info(output, filename):
 def log(msg, filters=[]):
 for flt in filters:
 msg = flt(msg)
-if type(msg) is dict or type(msg) is list:
-print(json.dumps(msg, sort_keys=True))
+if isinstance(msg, dict) or isinstance(msg, list):
+# Don't sort if it's already sorted
+do_sort = not isinstance(msg, OrderedDict)
+print(json.dumps(msg, sort_keys=do_sort))
 else:
 print(msg)
 
@@ -448,8 +461,11 @@ class VM(qtest.QEMUQtestMachine):
 return result
 
 def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
-logmsg = '{"execute": "%s", "arguments": %s}' % \
-(cmd, json.dumps(kwargs, sort_keys=True))
+full_cmd = OrderedDict((
+("execute", cmd),
+("arguments", ordered_kwargs(kwargs))
+))
+logmsg = json.dumps(full_cmd)
 log(logmsg, filters)
 result = self.qmp(cmd, **kwargs)
 log(json.dumps(result, sort_keys=True), filters)
-- 
2.17.2

[Qemu-devel] [PATCH v5 04/11] block: remove 'x' prefix from experimental bitmap APIs

[John Snow]

The 'x' prefix was added because I was uncertain of the direction we'd
take for the libvirt API. With the general approach solidified, I feel
comfortable committing to this API for 4.0.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 blockdev.c | 22 +++---
 qapi/block-core.json   | 34 +-
 qapi/transaction.json  | 12 ++--
 tests/qemu-iotests/223 |  4 ++--
 4 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6031c94121..8e37dd659e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1963,7 +1963,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState 
*common,
action->has_granularity, action->granularity,
action->has_persistent, action->persistent,
action->has_autoload, action->autoload,
-   action->has_x_disabled, action->x_disabled,
+   action->has_disabled, action->disabled,
&local_err);
 
 if (!local_err) {
@@ -2048,7 +2048,7 @@ static void 
block_dirty_bitmap_enable_prepare(BlkActionState *common,
 return;
 }
 
-action = common->action->u.x_block_dirty_bitmap_enable.data;
+action = common->action->u.block_dirty_bitmap_enable.data;
 state->bitmap = block_dirty_bitmap_lookup(action->node,
   action->name,
   NULL,
@@ -2089,7 +2089,7 @@ static void 
block_dirty_bitmap_disable_prepare(BlkActionState *common,
 return;
 }
 
-action = common->action->u.x_block_dirty_bitmap_disable.data;
+action = common->action->u.block_dirty_bitmap_disable.data;
 state->bitmap = block_dirty_bitmap_lookup(action->node,
   action->name,
   NULL,
@@ -2136,7 +2136,7 @@ static void 
block_dirty_bitmap_merge_prepare(BlkActionState *common,
 return;
 }
 
-action = common->action->u.x_block_dirty_bitmap_merge.data;
+action = common->action->u.block_dirty_bitmap_merge.data;
 
 state->bitmap = do_block_dirty_bitmap_merge(action->node, action->target,
 action->bitmaps, 
&state->backup,
@@ -2204,17 +2204,17 @@ static const BlkActionOps actions[] = {
 .commit = block_dirty_bitmap_free_backup,
 .abort = block_dirty_bitmap_restore,
 },
-[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_ENABLE] = {
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
 .instance_size = sizeof(BlockDirtyBitmapState),
 .prepare = block_dirty_bitmap_enable_prepare,
 .abort = block_dirty_bitmap_enable_abort,
 },
-[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_DISABLE] = {
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
 .instance_size = sizeof(BlockDirtyBitmapState),
 .prepare = block_dirty_bitmap_disable_prepare,
 .abort = block_dirty_bitmap_disable_abort,
 },
-[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_MERGE] = {
 .instance_size = sizeof(BlockDirtyBitmapState),
 .prepare = block_dirty_bitmap_merge_prepare,
 .commit = block_dirty_bitmap_free_backup,
@@ -2930,7 +2930,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const 
char *name,
 bdrv_clear_dirty_bitmap(bitmap, NULL);
 }
 
-void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
Error **errp)
 {
 BlockDriverState *bs;
@@ -2951,7 +2951,7 @@ void qmp_x_block_dirty_bitmap_enable(const char *node, 
const char *name,
 bdrv_enable_dirty_bitmap(bitmap);
 }
 
-void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
 Error **errp)
 {
 BlockDriverState *bs;
@@ -3018,8 +3018,8 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const 
char *node,
 return dst;
 }
 
-void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
-strList *bitmaps, Error **errp)
+void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
+  strList *bitmaps, Error **errp)
 {
 do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a153ea4420..91685be6c2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1806,15 +1806,15 @@
 #Currently, all dirty tracking bitmaps are loaded from Qcow2 on
 #open.
 #
-# @x-disabled: the bitmap is created 

[Qemu-devel] [PATCH v5 01/11] blockdev: abort transactions in reverse order

[John Snow]

Presently, we abort transactions in the same order they were processed in.
Bitmap commands, though, attempt to restore backup data structures on abort.

That's not valid, they need to be aborted in reverse chronological order.

Replace the QSIMPLEQ data structure with a QTAILQ one, so we can iterate
in reverse for the abort phase of the transaction.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 blockdev.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index a6f71f9d83..43e4c22da5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1339,7 +1339,7 @@ struct BlkActionState {
 const BlkActionOps *ops;
 JobTxn *block_job_txn;
 TransactionProperties *txn_props;
-QSIMPLEQ_ENTRY(BlkActionState) entry;
+QTAILQ_ENTRY(BlkActionState) entry;
 };
 
 /* internal snapshot private data */
@@ -2266,8 +2266,8 @@ void qmp_transaction(TransactionActionList *dev_list,
 BlkActionState *state, *next;
 Error *local_err = NULL;
 
-QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
-QSIMPLEQ_INIT(&snap_bdrv_states);
+QTAILQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
+QTAILQ_INIT(&snap_bdrv_states);
 
 /* Does this transaction get canceled as a group on failure?
  * If not, we don't really need to make a JobTxn.
@@ -2298,7 +2298,7 @@ void qmp_transaction(TransactionActionList *dev_list,
 state->action = dev_info;
 state->block_job_txn = block_job_txn;
 state->txn_props = props;
-QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, state, entry);
+QTAILQ_INSERT_TAIL(&snap_bdrv_states, state, entry);
 
 state->ops->prepare(state, &local_err);
 if (local_err) {
@@ -2307,7 +2307,7 @@ void qmp_transaction(TransactionActionList *dev_list,
 }
 }
 
-QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+QTAILQ_FOREACH(state, &snap_bdrv_states, entry) {
 if (state->ops->commit) {
 state->ops->commit(state);
 }
@@ -2318,13 +2318,13 @@ void qmp_transaction(TransactionActionList *dev_list,
 
 delete_and_fail:
 /* failure, and it is all-or-none; roll back all operations */
-QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+QTAILQ_FOREACH_REVERSE(state, &snap_bdrv_states, snap_bdrv_states, entry) {
 if (state->ops->abort) {
 state->ops->abort(state);
 }
 }
 exit:
-QSIMPLEQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) {
+QTAILQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) {
 if (state->ops->clean) {
 state->ops->clean(state);
 }
-- 
2.17.2

[Qemu-devel] [PATCH v5 03/11] blockdev: n-ary bitmap merge

[John Snow]

Especially outside of transactions, it is helpful to provide
all-or-nothing semantics for bitmap merges. This facilitates
the coalescing of multiple bitmaps into a single target for
the "checkpoint" interpretation when assembling bitmaps that
represent arbitrary points in time from component bitmaps.

This is an incompatible change from the preliminary version
of the API.

Signed-off-by: John Snow 
---
 blockdev.c   | 75 ++--
 qapi/block-core.json | 22 ++---
 2 files changed, 62 insertions(+), 35 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 43e4c22da5..6031c94121 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2119,33 +2119,28 @@ static void 
block_dirty_bitmap_disable_abort(BlkActionState *common)
 }
 }
 
+static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
+const char *target,
+strList *bitmaps,
+HBitmap **backup,
+Error **errp);
+
 static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
  Error **errp)
 {
 BlockDirtyBitmapMerge *action;
 BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
  common, common);
-BdrvDirtyBitmap *merge_source;
 
 if (action_check_completion_mode(common, errp) < 0) {
 return;
 }
 
 action = common->action->u.x_block_dirty_bitmap_merge.data;
-state->bitmap = block_dirty_bitmap_lookup(action->node,
-  action->dst_name,
-  &state->bs,
-  errp);
-if (!state->bitmap) {
-return;
-}
 
-merge_source = bdrv_find_dirty_bitmap(state->bs, action->src_name);
-if (!merge_source) {
-return;
-}
-
-bdrv_merge_dirty_bitmap(state->bitmap, merge_source, &state->backup, errp);
+state->bitmap = do_block_dirty_bitmap_merge(action->node, action->target,
+action->bitmaps, 
&state->backup,
+errp);
 }
 
 static void abort_prepare(BlkActionState *common, Error **errp)
@@ -2977,24 +2972,56 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, 
const char *name,
 bdrv_disable_dirty_bitmap(bitmap);
 }
 
-void qmp_x_block_dirty_bitmap_merge(const char *node, const char *dst_name,
-const char *src_name, Error **errp)
+static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
+const char *target,
+strList *bitmaps,
+HBitmap **backup,
+Error **errp)
 {
 BlockDriverState *bs;
-BdrvDirtyBitmap *dst, *src;
+BdrvDirtyBitmap *dst, *src, *anon;
+strList *lst;
+Error *local_err = NULL;
 
-dst = block_dirty_bitmap_lookup(node, dst_name, &bs, errp);
+dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
 if (!dst) {
-return;
+return NULL;
 }
 
-src = bdrv_find_dirty_bitmap(bs, src_name);
-if (!src) {
-error_setg(errp, "Dirty bitmap '%s' not found", src_name);
-return;
+anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
+NULL, errp);
+if (!anon) {
+return NULL;
 }
 
-bdrv_merge_dirty_bitmap(dst, src, NULL, errp);
+for (lst = bitmaps; lst; lst = lst->next) {
+src = bdrv_find_dirty_bitmap(bs, lst->value);
+if (!src) {
+error_setg(errp, "Dirty bitmap '%s' not found", lst->value);
+dst = NULL;
+goto out;
+}
+
+bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+dst = NULL;
+goto out;
+}
+}
+
+/* Merge into dst; dst is unchanged on failure. */
+bdrv_merge_dirty_bitmap(dst, anon, backup, errp);
+
+ out:
+bdrv_release_dirty_bitmap(bs, anon);
+return dst;
+}
+
+void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
+strList *bitmaps, Error **errp)
+{
+do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
 
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 762000f31f..a153ea4420 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1821,14 +1821,14 @@
 #
 # @node: name of device/node which the bitmap is tracking
 #
-# @dst_name: name of th

[Qemu-devel] [PATCH v5 10/11] iotests: implement pretty-print for log and qmp_log

[John Snow]

If iotests have lines exceeding >998 characters long, git doesn't
want to send it plaintext to the list. We can solve this by allowing
the iotests to use pretty printed QMP output that we can match against
instead.

As a bonus, it's much nicer for human eyes too.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index dcd0c6f71d..d65bcaf953 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -284,13 +284,18 @@ def filter_img_info(output, filename):
 lines.append(line)
 return '\n'.join(lines)
 
-def log(msg, filters=[]):
+def log(msg, filters=[], indent=None):
+'''Logs either a string message or a JSON serializable message (like QMP).
+If indent is provided, JSON serializable messages are pretty-printed.'''
 for flt in filters:
 msg = flt(msg)
 if isinstance(msg, dict) or isinstance(msg, list):
+# Python < 3.4 needs to know not to add whitespace when 
pretty-printing:
+separators = (', ', ': ') if indent is None else (',', ': ')
 # Don't sort if it's already sorted
 do_sort = not isinstance(msg, OrderedDict)
-print(json.dumps(msg, sort_keys=do_sort))
+print(json.dumps(msg, sort_keys=do_sort,
+ indent=indent, separators=separators))
 else:
 print(msg)
 
@@ -479,14 +484,14 @@ class VM(qtest.QEMUQtestMachine):
 result.append(filter_qmp_event(ev))
 return result
 
-def qmp_log(self, cmd, filters=[], **kwargs):
+def qmp_log(self, cmd, filters=[], indent=None, **kwargs):
 full_cmd = OrderedDict((
 ("execute", cmd),
 ("arguments", ordered_kwargs(kwargs))
 ))
-log(full_cmd, filters)
+log(full_cmd, filters, indent=indent)
 result = self.qmp(cmd, **kwargs)
-log(result, filters)
+log(result, filters, indent=indent)
 return result
 
 def run_job(self, job, auto_finalize=True, auto_dismiss=False):
-- 
2.17.2

[Qemu-devel] [PATCH v5 08/11] iotests: remove default filters from qmp_log

[John Snow]

Several places in iotests deal with serializing objects into JSON
strings, but to add pretty-printing it seems desireable to localize
all of those cases.

log() seems like a good candidate for that centralized behavior.
log() can already serialize json objects, but when it does so,
it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP
objects into strings and filtering them assuming that filters=[]
are string filters.

To have qmp_log use log's serialization, qmp_log will need to
accept only qmp filters, not text filters.

However, only a single caller of qmp_log actually requires any
filters at all. I remove the default filter and add it explicitly
to the caller in preparation for refactoring qmp_log to use rich
filters instead.

test 206 is amended to name the filter explicitly and the default
is removed.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/206| 8 ++--
 tests/qemu-iotests/iotests.py | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
index 128c334c7c..e92550fa59 100755
--- a/tests/qemu-iotests/206
+++ b/tests/qemu-iotests/206
@@ -26,7 +26,9 @@ from iotests import imgfmt
 iotests.verify_image_format(supported_fmts=['qcow2'])
 
 def blockdev_create(vm, options):
-result = vm.qmp_log('blockdev-create', job_id='job0', options=options)
+result = vm.qmp_log('blockdev-create',
+filters=[iotests.filter_testfiles],
+job_id='job0', options=options)
 
 if 'return' in result:
 assert result['return'] == {}
@@ -52,7 +54,9 @@ with iotests.FilePath('t.qcow2') as disk_path, \
   'filename': disk_path,
   'size': 0 })
 
-vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
+vm.qmp_log('blockdev-add',
+   filters=[iotests.filter_testfiles],
+   driver='file', filename=disk_path,
node_name='imgfile')
 
 blockdev_create(vm, { 'driver': imgfmt,
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 565eebb1ab..57fe20db45 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -460,7 +460,7 @@ class VM(qtest.QEMUQtestMachine):
 result.append(filter_qmp_event(ev))
 return result
 
-def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
+def qmp_log(self, cmd, filters=[], **kwargs):
 full_cmd = OrderedDict((
 ("execute", cmd),
 ("arguments", ordered_kwargs(kwargs))
-- 
2.17.2

[Qemu-devel] [PATCH v5 11/11] iotests: add iotest 236 for testing bitmap merge

[John Snow]

New interface, new smoke test.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/236 | 161 +
 tests/qemu-iotests/236.out | 351 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 513 insertions(+)
 create mode 100755 tests/qemu-iotests/236
 create mode 100644 tests/qemu-iotests/236.out

diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
new file mode 100755
index 00..42b93da3ad
--- /dev/null
+++ b/tests/qemu-iotests/236
@@ -0,0 +1,161 @@
+#!/usr/bin/env python
+#
+# Test bitmap merges.
+#
+# Copyright (c) 2018 John Snow for Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+# owner=js...@redhat.com
+
+import iotests
+from iotests import log
+
+iotests.verify_image_format(supported_fmts=['generic'])
+size = 64 * 1024 * 1024
+granularity = 64 * 1024
+
+patterns = [("0x5d", "0", "64k"),
+("0xd5", "1M","64k"),
+("0xdc", "32M",   "64k"),
+("0xcd", "0x3ff", "64k")]  # 64M - 64K
+
+overwrite = [("0xab", "0", "64k"), # Full overwrite
+ ("0xad", "0x00f8000", "64k"), # Partial-left (1M-32K)
+ ("0x1d", "0x2008000", "64k"), # Partial-right (32M+32K)
+ ("0xea", "0x3fe", "64k")] # Adjacent-left (64M - 128K)
+
+def query_bitmaps(vm):
+res = vm.qmp("query-block")
+return { "bitmaps": { device['device']: device.get('dirty-bitmaps', []) for
+  device in res['return'] } }
+
+with iotests.FilePath('img') as img_path, \
+ iotests.VM() as vm:
+
+log('--- Preparing image & VM ---\n')
+iotests.qemu_img_create('-f', iotests.imgfmt, img_path, str(size))
+vm.add_drive(img_path)
+vm.launch()
+
+log('\n--- Adding preliminary bitmaps A & B ---\n')
+vm.qmp_log("block-dirty-bitmap-add", node="drive0",
+   name="bitmapA", granularity=granularity)
+vm.qmp_log("block-dirty-bitmap-add", node="drive0",
+   name="bitmapB", granularity=granularity)
+
+# Dirties 4 clusters. count=262144
+log('\n--- Emulating writes ---\n')
+for p in patterns:
+cmd = "write -P%s %s %s" % p
+log(cmd)
+log(vm.hmp_qemu_io("drive0", cmd))
+
+log(query_bitmaps(vm), indent=2)
+
+log('\n--- Submitting Bad Transaction ---\n')
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapB" }},
+{ "type": "block-dirty-bitmap-add",
+  "data": { "node": "drive0", "name": "bitmapC",
+"granularity": granularity }},
+{ "type": "block-dirty-bitmap-clear",
+  "data": { "node": "drive0", "name": "bitmapA" }},
+{ "type": "abort", "data": {}}
+])
+log(query_bitmaps(vm), indent=2)
+
+log('\n--- Disabling B & Adding C ---\n')
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapB" }},
+{ "type": "block-dirty-bitmap-add",
+  "data": { "node": "drive0", "name": "bitmapC",
+"granularity": granularity }},
+# Purely extraneous, but test that it works:
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapC" }},
+{ "type": "block-dirty-bitmap-enable",
+  "data": { "node": "drive0", "name": "bitmapC" }},
+])
+
+log('\n--- Emulating further writes ---\n')
+# Dirties 6 clusters, 3 of which are new in contrast to "A".
+# A = 64 * 1024 * (4 + 3) = 458752
+# C = 64 * 1024 * 6   = 393216
+for p in overwrite:
+cmd = "write -P%s %s %s" % p
+log(cmd)
+log(vm.hmp_qemu_io("drive0", cmd))
+
+log('\n--- Disabling A & C ---\n')
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapA" }},
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapC" }}
+])
+
+# A: 7 clusters
+# B: 4 clusters
+# C: 6 clusters
+log(query_bitmaps(vm), indent=2)
+
+log('\n--- Submitting Bad Merge ---\n')
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-add",
+  "data": { "no

[Qemu-devel] [PATCH v5 00/11] bitmaps: remove x- prefix from QMP api

[John Snow]

Fix some outstanding bugs, change the design of an API element,
remove the x- prefix to signify stability, and add iotests.

V5:
001/11:[] [--] 'blockdev: abort transactions in reverse order'
002/11:[down] 'block/dirty-bitmap: remove assertion from restore'
003/11:[0029] [FC] 'blockdev: n-ary bitmap merge'
004/11:[] [-C] 'block: remove 'x' prefix from experimental bitmap APIs'
005/11:[] [--] 'iotests.py: don't abort if IMGKEYSECRET is undefined'
006/11:[] [--] 'iotests: add filter_generated_node_ids'
007/11:[0019] [FC] 'iotests: add qmp recursive sorting function'
008/11:[] [-C] 'iotests: remove default filters from qmp_log'
009/11:[] [-C] 'iotests: change qmp_log filters to expect QMP objects only'
010/11:[0005] [FC] 'iotests: implement pretty-print for log and qmp_log'
011/11:[0324] [FC] 'iotests: add iotest 236 for testing bitmap merge'

002: New bugfix.
003: I forgot to actually capture state->bitmap anywhere,
 which is needed for restoration...
007: Better commit message
 - use .items() instead of .keys() to save a lookup [Vladimir]
 - use a sequence of tuples to preserve ordering in
   the OrderedDict constructor [Vladimir]
 - Move the sort_keys boolean up from patch 010
008: Better commit message
009: Better commit message
010: Moved the sort_keys function up to patch 007
011: Expanded this test considerably:
 - query_bitmaps can now show empty results,
   and prefixes results with "bitmaps:" in the log
 - logging declarations are one line [Vladimir]
 - Added a bad version of the bitmap handoff transaction [Eric]
 - Added a bad version of the bitmap merge transaction,
   which revealed a problem that patch 02 now addresses [Eric]
 - Added bitmap removal / cleanup [Eric]
 - Added newline at end of file. [Eric]

V4:
 - Removed patches 1-5 which have been staged
 - Rewrite qmp_log entirely, split into three patches
 - Pretty-printing has been extended to log() as well as qmp_log()
 - Adjust iotest 236 to be format generic instead of qcow2 [Vladimir]
 - Adjust iotest 236 to not reduplicate serialization work [Vladimir]
 - Many other small touchups

V3:
 - Reworked qmp_log to pretty-print the outgoing command, too [Vladimir]
 - Modified test to log only bitmap information [Vladimir]
 - Test disable/enable transaction toggle [Eric]

John Snow (11):
  blockdev: abort transactions in reverse order
  block/dirty-bitmap: remove assertion from restore
  blockdev: n-ary bitmap merge
  block: remove 'x' prefix from experimental bitmap APIs
  iotests.py: don't abort if IMGKEYSECRET is undefined
  iotests: add filter_generated_node_ids
  iotests: add qmp recursive sorting function
  iotests: remove default filters from qmp_log
  iotests: change qmp_log filters to expect QMP objects only
  iotests: implement pretty-print for log and qmp_log
  iotests: add iotest 236 for testing bitmap merge

 block/dirty-bitmap.c  |   1 -
 blockdev.c| 107 +++
 qapi/block-core.json  |  56 +++---
 qapi/transaction.json |  12 +-
 tests/qemu-iotests/206|   8 +-
 tests/qemu-iotests/223|   4 +-
 tests/qemu-iotests/236| 161 
 tests/qemu-iotests/236.out| 351 ++
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |  60 +-
 10 files changed, 673 insertions(+), 88 deletions(-)
 create mode 100755 tests/qemu-iotests/236
 create mode 100644 tests/qemu-iotests/236.out

-- 
2.17.2

[Qemu-devel] [PATCH v5 02/11] block/dirty-bitmap: remove assertion from restore

[John Snow]

When making a backup of a dirty bitmap (for transactions), we want to
restore that backup whether or not the bitmap is enabled or not.

It is perfectly valid to write into bitmaps that are disabled. It is
only illegitimate for the guest to have done so.

Remove this assertion.

Signed-off-by: John Snow 
---
 block/dirty-bitmap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 89fd1d7f8b..6b688394e4 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -625,7 +625,6 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, 
HBitmap **out)
 void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)
 {
 HBitmap *tmp = bitmap->bitmap;
-assert(bdrv_dirty_bitmap_enabled(bitmap));
 assert(!bdrv_dirty_bitmap_readonly(bitmap));
 bitmap->bitmap = backup;
 hbitmap_free(tmp);
-- 
2.17.2

Re: [Qemu-devel] Booting Raspbian on RPi emulation

[BALATON Zoltan]

On Wed, 19 Dec 2018, Ben Hekster via Qemu-devel wrote:
	While the crashing has stopped, the window isn't responsive to 
keystrokes.  This includes the frame buffer emulation itself (so I can't 
actually log in)


I'm not sure that's supposed to work. I could be wrong but I think USB is 
not emulated yet so likely no keyboard is attached that you could type 
with. The framebuffer is emulated so you can see output but only way to 
input is via serial if I'm not mistaken. That's what works for me.



as well as the QEMU Monitor; nothing I type has any effect in either.


The monitor view should work though. Does it work without the patches? If 
you start with "qemu-system-arm -M raspi2 -S" you can probably try it 
before it crashes.


Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH v4 5/5] iotests: add iotest 236 for testing bitmap merge

[John Snow]

On 12/19/18 2:34 PM, Eric Blake wrote:
> On 12/18/18 7:52 PM, John Snow wrote:
>> New interface, new smoke test.
>> ---
>>   tests/qemu-iotests/236 | 131 
>>   tests/qemu-iotests/236.out | 198 +
>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 330 insertions(+)
>>   create mode 100755 tests/qemu-iotests/236
>>   create mode 100644 tests/qemu-iotests/236.out
>>
> 
>> +
>> +    log('')
>> +    log('--- Disabling B & Adding C ---\n')
>> +    vm.qmp_log("transaction", indent=2, actions=[
>> +    { "type": "block-dirty-bitmap-disable",
>> +  "data": { "node": "drive0", "name": "bitmapB" }},
>> +    { "type": "block-dirty-bitmap-add",
>> +  "data": { "node": "drive0", "name": "bitmapC",
>> +    "granularity": granularity }},
>> +    # Purely extraneous, but test that it works:
>> +    { "type": "block-dirty-bitmap-disable",
>> +  "data": { "node": "drive0", "name": "bitmapC" }},
>> +    { "type": "block-dirty-bitmap-enable",
>> +  "data": { "node": "drive0", "name": "bitmapC" }},
>> +    ])
> 
> One other possible addition just before this point:
> 
> qmp_log("transaction", indent=2, actions=[
>     { "type": "block-dirty-bitmap-disable",
>   "data": { "node": "drive0", "name": "bitmapB" }},
>     { "type": "block-dirty-bitmap-add",
>   "data": { "node": "drive0", "name": "bitmapC",
>     "granularity": granularity }},
>     { "type": "block-dirty-bitmap-remove",
>   "data": { "node": "drive0", "name": "bitmapA" }},
>     { "type": "abort", "data": {}}
>   ])
> 
> to check that we properly undo things on an aborted transaction (B
> should still be enabled, C should not exist, and A should not be damaged).
> 

Good suggestion, but remove is not a transaction item. I'll substitute
for clear, which should showcase the same effects.

>> +    # A and D should be equivalent>> +    # Some formats round the size of 
>> the disk, so don't print the
>> checksums.
>> +    check_a = vm.qmp('x-debug-block-dirty-bitmap-sha256',
>> + node="drive0", name="bitmapA")['return']['sha256']
>> +    check_b = vm.qmp('x-debug-block-dirty-bitmap-sha256',
>> + node="drive0", name="bitmapD")['return']['sha256']
>> +    assert(check_a == check_b)
> 
> Image agnostic also means that you avoid an 32- vs. 64-bit platform
> discrepancies (we've had issues in the past where the sum is different
> for some image sizes, because the bitmap is always in terms of 'long's,
> but there is one less 'long' in a 32-bit bitmap for the disk size).
> Makes sense.
> 
> Also, I don't see any tests of block-dirty-bitmap-remove - this would be
> a good point in the test to insert a final cleanup.
> 

OK, done.

> 
>> +++ b/tests/qemu-iotests/group
>> @@ -233,3 +233,4 @@
>>   233 auto quick
>>   234 auto quick migration
>>   235 auto quick
>> +236 auto quick
>> \ No newline at end of file
> 
> Umm - what's that still doing here?
> 
> 
:whistles:

Re: [Qemu-devel] Question about aio_poll and glib aio_ctx_dispatch

[Fam Zheng]

> On Dec 20, 2018, at 06:58, Li Qiang  wrote:
> 
> Hello Paolo
> 
> Thanks for your kind reply.
> 
> Yes, aio_poll and aio_ctx_dispatch mostly run in different threads, though
> Sometimes they can run in a thread nested from Fam’s slides:
> →http://events17.linuxfoundation.org/sites/events/files/slides/Improving%20the%20QEMU%20Event%20Loop%20-%203.pdf
> 
> So you mean we can poll the same fd in two threads? If so , the ‘fd’ in 
> ‘qemu_aio_context’ will be run twice, I think there’s 
> something I understand wrong.
> 
> Let’s take an example. In the ‘v9fs_reset’.
> 
> void v9fs_reset(V9fsState *s)
> {
>VirtfsCoResetData data = { .pdu = { .s = s }, .done = false };
>Coroutine *co;
> 
>while (!QLIST_EMPTY(&s->active_list)) {
>aio_poll(qemu_get_aio_context(), true);
>}
> 
>co = qemu_coroutine_create(virtfs_co_reset, &data);
>qemu_coroutine_enter(co);
> 
>while (!data.done) {
>aio_poll(qemu_get_aio_context(), true);
>}
> }
> 
> Here we use aio_poll to wait the pending action to complete.
> Here aio_poll will poll the fds in ‘qemu_aio_context’ in vcpu thread,
> However, the main loop is also poll the fds in ‘qemu_aio_context’.
> If some ‘fd’ in ‘qemu_aio_context’ has events, the two thread will be wakeup?
> You say both will call aio_dispatch_handlers and timerlistgroup_run_timers.
> But the are the same fd, how can this happen?


I think in this case BQL is used to synchronize them so when v9fs_reset runs, 
the main loop doesn't.

Thanks,
Fam

> 
> Thanks,
> Li Qiang
> 
> 
> 发件人: Paolo Bonzini
> 发送时间: 2018年12月20日 4:42
> 收件人: Li Qiang; stefa...@redhat.com; f...@euphon.net; Qemu Developers; 李强
> 主题: Re: Question about aio_poll and glib aio_ctx_dispatch
> 
> On 19/12/18 11:05, Li Qiang wrote:
>> Sent it to qemu-devel.
>> 
>> Li Qiang mailto:liq...@gmail.com>> 于2018年12月19日周
>> 三 下午6:04写道：
>> 
>>Hello Paolo, Stefan, Fam and all,
>> 
>>Here I have a question about 'aio_poll'.
>>IIUC the 'aio_poll' is (mostly) used for synchronous IO
>>as I see a lot of code like this: 
>>while(condition)
>> aio_poll();
>> 
>>However it seems the 'aio_poll' and 'aio_ctx_dispatch' both poll the fd.
>>So what happened when the 'fd' has events, which function will be
>>wakeup?
> 
> Roughly speaking, aio_poll is used for synchronous IO and within I/O
> threads; aio_ctx_dispatch is used within the main thread.
> 
> Both end up calling aio_dispatch_handlers and timerlistgroup_run_timers.
> 
> Paolo
> 

[Qemu-devel] [PATCH v2 1/6] contrib/elf2dmp: fix elf.h including

[Viktor Prutyanov]

Before this patch QEMU elf.h was not actually included.

Signed-off-by: Viktor Prutyanov 
---
 contrib/elf2dmp/qemu_elf.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
index d85d6558fa..19d1299954 100644
--- a/contrib/elf2dmp/qemu_elf.h
+++ b/contrib/elf2dmp/qemu_elf.h
@@ -5,11 +5,11 @@
  *
  */
 
-#ifndef QEMU_ELF_H
-#define QEMU_ELF_H
+#ifndef ELF2DMP_ELF_H
+#define ELF2DMP_ELF_H
 
 #include 
-#include 
+#include "elf.h"
 
 typedef struct QEMUCPUSegment {
 uint32_t selector;
@@ -48,4 +48,4 @@ void QEMU_Elf_exit(QEMU_Elf *qe);
 Elf64_Phdr *elf64_getphdr(void *map);
 Elf64_Half elf_getphdrnum(void *map);
 
-#endif /* QEMU_ELF_H */
+#endif /* ELF2DMP_ELF_H */
-- 
2.17.2

[Qemu-devel] [PATCH v2 5/6] contrib/elf2dmp: fix printf format

[Viktor Prutyanov]

Format strings for printf are changed for successful build for Windows
hosts.

Signed-off-by: Viktor Prutyanov 
---
 contrib/elf2dmp/main.c | 27 +++
 contrib/elf2dmp/pdb.c  |  4 +++-
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 9b93dab662..fdafb54900 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -5,6 +5,8 @@
  *
  */
 
+#include 
+
 #include "qemu/osdep.h"
 #include "err.h"
 #include "addrspace.h"
@@ -41,7 +43,8 @@ static const uint64_t SharedUserData = 0xf780;
 #define KUSD_OFFSET_PRODUCT_TYPE 0x264
 
 #define SYM_RESOLVE(base, r, s) ((s = pdb_resolve(base, r, #s)),\
-s ? printf(#s" = 0x%016lx\n", s) : eprintf("Failed to resolve "#s"\n"), s)
+s ? printf(#s" = 0x%016"PRIx64"\n", s) :\
+eprintf("Failed to resolve "#s"\n"), s)
 
 static uint64_t rol(uint64_t x, uint64_t y)
 {
@@ -98,8 +101,8 @@ static KDDEBUGGER_DATA64 *get_kdbg(uint64_t KernBase, struct 
pdb_reader *pdb,
 return NULL;
 }
 
-printf("[KiWaitNever] = 0x%016lx\n", kwn);
-printf("[KiWaitAlways] = 0x%016lx\n", kwa);
+printf("[KiWaitNever] = 0x%016"PRIx64"\n", kwn);
+printf("[KiWaitAlways] = 0x%016"PRIx64"\n", kwa);
 
 /*
  * If KDBG header can be decoded, KDBG size is available
@@ -202,7 +205,7 @@ static int fix_dtb(struct va_space *vs, QEMU_Elf *qe)
 
 if (is_system(s)) {
 va_space_set_dtb(vs, s->cr[3]);
-printf("DTB 0x%016lx has been found from CPU #%zu"
+printf("DTB 0x%016"PRIx64" has been found from CPU #%zu"
 " as system task CR3\n", vs->dtb, i);
 return !(va_space_resolve(vs, SharedUserData));
 }
@@ -222,7 +225,7 @@ static int fix_dtb(struct va_space *vs, QEMU_Elf *qe)
 }
 
 va_space_set_dtb(vs, *cr3);
-printf("DirectoryTableBase = 0x%016lx has been found from CPU #0"
+printf("DirectoryTableBase = 0x%016"PRIx64" has been found from CPU #0"
 " as interrupt handling CR3\n", vs->dtb);
 return !(va_space_resolve(vs, SharedUserData));
 }
@@ -393,8 +396,8 @@ static int pe_get_pdb_symstore_hash(uint64_t base, void 
*start_addr,
 return 1;
 }
 
-printf("Debug Directory RVA = 0x%016x\n",
-data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress);
+printf("Debug Directory RVA = 0x%08"PRIx32"\n",
+(uint32_t)data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress);
 
 if (va_space_rw(vs,
 base + data_dir[IMAGE_FILE_DEBUG_DIRECTORY].VirtualAddress,
@@ -488,7 +491,7 @@ int main(int argc, char *argv[])
 }
 
 state = qemu_elf.state[0];
-printf("CPU #0 CR3 is 0x%016lx\n", state->cr[3]);
+printf("CPU #0 CR3 is 0x%016"PRIx64"\n", state->cr[3]);
 
 va_space_create(&vs, &ps, state->cr[3]);
 if (fix_dtb(&vs, &qemu_elf)) {
@@ -497,7 +500,7 @@ int main(int argc, char *argv[])
 goto out_elf;
 }
 
-printf("CPU #0 IDT is at 0x%016lx\n", state->idt.base);
+printf("CPU #0 IDT is at 0x%016"PRIx64"\n", state->idt.base);
 
 if (va_space_rw(&vs, state->idt.base,
 &first_idt_desc, sizeof(first_idt_desc), 0)) {
@@ -505,10 +508,10 @@ int main(int argc, char *argv[])
 err = 1;
 goto out_ps;
 }
-printf("CPU #0 IDT[0] -> 0x%016lx\n", idt_desc_addr(first_idt_desc));
+printf("CPU #0 IDT[0] -> 0x%016"PRIx64"\n", idt_desc_addr(first_idt_desc));
 
 KernBase = idt_desc_addr(first_idt_desc) & ~(PAGE_SIZE - 1);
-printf("Searching kernel downwards from 0x%16lx...\n", KernBase);
+printf("Searching kernel downwards from 0x%016"PRIx64"...\n", KernBase);
 
 for (; KernBase >= 0xf780; KernBase -= PAGE_SIZE) {
 nt_start_addr = va_space_resolve(&vs, KernBase);
@@ -521,7 +524,7 @@ int main(int argc, char *argv[])
 }
 }
 
-printf("KernBase = 0x%16lx, signature is \'%.2s\'\n", KernBase,
+printf("KernBase = 0x%016"PRIx64", signature is \'%.2s\'\n", KernBase,
 (char *)nt_start_addr);
 
 if (pe_get_pdb_symstore_hash(KernBase, nt_start_addr, pdb_hash, &vs)) {
diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index 52e352df79..64af20f584 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -18,6 +18,8 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
+#include 
+
 #include "qemu/osdep.h"
 #include "pdb.h"
 #include "err.h"
@@ -66,7 +68,7 @@ uint64_t pdb_find_public_v3_symbol(struct pdb_reader *r, 
const char *name)
 uint32_t sect_rva = segment->dword[1];
 uint64_t rva = sect_rva + sym->public_v3.offset;
 
-printf("%s: 0x%016x(%d:\'%.8s\') + 0x%08x = 0x%09lx\n", name,
+printf("%s: 0x%016x(%d:\'%.8s\') + 0x%08x = 0x%09"PRIx64"\n", name,
 sect_rva, sym->public_v3.segment,
 ((char *)segment - 8),

[Qemu-devel] [PATCH v2 3/6] contrib/elf2dmp: use GLib in PDB processing

[Viktor Prutyanov]

Replace POSIX mmap with GLib g_mapped_file_new in PDB processing stage
to make elf2dmp cross-platform. There are no direct POSIX in elf2dmp
after this patch.

Signed-off-by: Viktor Prutyanov 
---
 contrib/elf2dmp/pdb.c | 29 -
 contrib/elf2dmp/pdb.h |  2 +-
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index bcb01b414f..52e352df79 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -277,28 +277,18 @@ static void pdb_reader_exit(struct pdb_reader *r)
 
 int pdb_init_from_file(const char *name, struct pdb_reader *reader)
 {
+GError *gerr = NULL;
 int err = 0;
-int fd;
 void *map;
-struct stat st;
 
-fd = open(name, O_RDONLY, 0);
-if (fd == -1) {
-eprintf("Failed to open PDB file \'%s\'\n", name);
+reader->gmf = g_mapped_file_new(name, TRUE, &gerr);
+if (gerr) {
+eprintf("Failed to map PDB file \'%s\'\n", name);
 return 1;
 }
-reader->fd = fd;
-
-fstat(fd, &st);
-reader->file_size = st.st_size;
-
-map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
-if (map == MAP_FAILED) {
-eprintf("Failed to map PDB file\n");
-err = 1;
-goto out_fd;
-}
 
+reader->file_size = g_mapped_file_get_length(reader->gmf);
+map = g_mapped_file_get_contents(reader->gmf);
 if (pdb_reader_init(reader, map)) {
 err = 1;
 goto out_unmap;
@@ -307,16 +297,13 @@ int pdb_init_from_file(const char *name, struct 
pdb_reader *reader)
 return 0;
 
 out_unmap:
-munmap(map, st.st_size);
-out_fd:
-close(fd);
+g_mapped_file_unref(reader->gmf);
 
 return err;
 }
 
 void pdb_exit(struct pdb_reader *reader)
 {
-munmap(reader->ds.header, reader->file_size);
-close(reader->fd);
+g_mapped_file_unref(reader->gmf);
 pdb_reader_exit(reader);
 }
diff --git a/contrib/elf2dmp/pdb.h b/contrib/elf2dmp/pdb.h
index 4351a2dd61..8e395119d1 100644
--- a/contrib/elf2dmp/pdb.h
+++ b/contrib/elf2dmp/pdb.h
@@ -218,7 +218,7 @@ typedef struct pdb_seg {
 #define IMAGE_FILE_MACHINE_AMD64 0x8664
 
 struct pdb_reader {
-int fd;
+GMappedFile *gmf;
 size_t file_size;
 struct {
 PDB_DS_HEADER *header;
-- 
2.17.2

[Qemu-devel] [PATCH v2 2/6] contrib/elf2dmp: use GLib in ELF processing

[Viktor Prutyanov]

Replace POSIX mmap with GLib g_mapped_file_new in ELF processing module
to make elf2dmp cross-platform.

Signed-off-by: Viktor Prutyanov 
---
 contrib/elf2dmp/qemu_elf.c | 27 ---
 contrib/elf2dmp/qemu_elf.h |  2 +-
 2 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
index e9c0d2534a..0db7816586 100644
--- a/contrib/elf2dmp/qemu_elf.c
+++ b/contrib/elf2dmp/qemu_elf.c
@@ -120,25 +120,17 @@ static void exit_states(QEMU_Elf *qe)
 
 int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
 {
+GError *gerr = NULL;
 int err = 0;
-struct stat st;
 
-qe->fd = open(filename, O_RDONLY, 0);
-if (qe->fd == -1) {
-eprintf("Failed to open ELF dump file \'%s\'\n", filename);
+qe->gmf = g_mapped_file_new(filename, TRUE, &gerr);
+if (gerr) {
+eprintf("Failed to map ELF dump file \'%s\'\n", filename);
 return 1;
 }
 
-fstat(qe->fd, &st);
-qe->size = st.st_size;
-
-qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE,
-MAP_PRIVATE, qe->fd, 0);
-if (qe->map == MAP_FAILED) {
-eprintf("Failed to map ELF file\n");
-err = 1;
-goto out_fd;
-}
+qe->map = g_mapped_file_get_contents(qe->gmf);
+qe->size = g_mapped_file_get_length(qe->gmf);
 
 if (init_states(qe)) {
 eprintf("Failed to extract QEMU CPU states\n");
@@ -149,9 +141,7 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
 return 0;
 
 out_unmap:
-munmap(qe->map, qe->size);
-out_fd:
-close(qe->fd);
+g_mapped_file_unref(qe->gmf);
 
 return err;
 }
@@ -159,6 +149,5 @@ out_fd:
 void QEMU_Elf_exit(QEMU_Elf *qe)
 {
 exit_states(qe);
-munmap(qe->map, qe->size);
-close(qe->fd);
+g_mapped_file_unref(qe->gmf);
 }
diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
index 19d1299954..fc69606d00 100644
--- a/contrib/elf2dmp/qemu_elf.h
+++ b/contrib/elf2dmp/qemu_elf.h
@@ -34,7 +34,7 @@ typedef struct QEMUCPUState {
 int is_system(QEMUCPUState *s);
 
 typedef struct QEMU_Elf {
-int fd;
+GMappedFile *gmf;
 size_t size;
 void *map;
 QEMUCPUState **state;
-- 
2.17.2

[Qemu-devel] [PATCH v2 6/6] configure: enable elf2dmp build for Windows hosts

[Viktor Prutyanov]

After this patch contrib/elf2dmp can be built for Windows x86 and x86_64
hosts by mingw.

Signed-off-by: Viktor Prutyanov 
---
 Makefile  | 4 ++--
 configure | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c8b9efdad4..25acb94aa6 100644
--- a/Makefile
+++ b/Makefile
@@ -565,8 +565,8 @@ ifneq ($(EXESUF),)
 qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
 endif
 
-elf2dmp: LIBS = $(CURL_LIBS)
-elf2dmp: $(elf2dmp-obj-y)
+elf2dmp$(EXESUF): LIBS += $(CURL_LIBS)
+elf2dmp$(EXESUF): $(elf2dmp-obj-y)
$(call LINK, $^)
 
 ifdef CONFIG_IVSHMEM
diff --git a/configure b/configure
index 224d3071ac..686ae2e093 100755
--- a/configure
+++ b/configure
@@ -5753,8 +5753,8 @@ if test "$want_tools" = "yes" ; then
   if [ "$ivshmem" = "yes" ]; then
 tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools"
   fi
-  if [ "$posix" = "yes" ] && [ "$curl" = "yes" ]; then
-tools="elf2dmp $tools"
+  if [ "$curl" = "yes" ]; then
+  tools="elf2dmp\$(EXESUF) $tools"
   fi
 fi
 if test "$softmmu" = yes ; then
-- 
2.17.2

[Qemu-devel] [PATCH v2 0/6] contrib/elf2dmp: elf2dmp for Windows hosts

[Viktor Prutyanov]

In most cases, it is more convenient to convert a dump on the same machine
on which the analysis is performed. Because of WinDbg, the analysis of guest
Windows problems needs Windows host anyway, so it is useful to have dump
convertion tool near the debugger.
After these patches elf2dmp can be built both for Linux and Windows (x86
and x86_64) hosts.

Viktor Prutyanov (6):
  contrib/elf2dmp: fix elf.h including
  contrib/elf2dmp: use GLib in ELF processing
  contrib/elf2dmp: use GLib in PDB processing
  contrib/elf2dmp: fix structures definitions
  contrib/elf2dmp: fix printf format
  configure: enable elf2dmp build for Windows hosts

 Makefile   |  4 ++--
 configure  |  4 ++--
 contrib/elf2dmp/kdbg.h | 12 
 contrib/elf2dmp/main.c | 27 +++
 contrib/elf2dmp/pdb.c  | 33 +++--
 contrib/elf2dmp/pdb.h  |  4 +++-
 contrib/elf2dmp/pe.h   |  6 --
 contrib/elf2dmp/qemu_elf.c | 27 ---
 contrib/elf2dmp/qemu_elf.h | 10 +-
 9 files changed, 58 insertions(+), 69 deletions(-)

-- 
2.17.2

[Qemu-devel] [PATCH v2 4/6] contrib/elf2dmp: fix structures definitions

[Viktor Prutyanov]

Remove duplicate structures definitions in case of build for Windows hosts.

Signed-off-by: Viktor Prutyanov 
---
 contrib/elf2dmp/kdbg.h | 12 
 contrib/elf2dmp/pdb.h  |  2 ++
 contrib/elf2dmp/pe.h   |  6 --
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/contrib/elf2dmp/kdbg.h b/contrib/elf2dmp/kdbg.h
index 851b57c321..002e3d0cd5 100644
--- a/contrib/elf2dmp/kdbg.h
+++ b/contrib/elf2dmp/kdbg.h
@@ -25,11 +25,15 @@ typedef struct DBGKD_GET_VERSION64 {
 uint64_t DebuggerDataList;
 } DBGKD_GET_VERSION64;
 
+#ifndef _WIN32
+typedef struct LIST_ENTRY64 {
+struct LIST_ENTRY64 *Flink;
+struct LIST_ENTRY64 *Blink;
+} LIST_ENTRY64;
+#endif
+
 typedef struct DBGKD_DEBUG_DATA_HEADER64 {
-struct LIST_ENTRY64 {
-   struct LIST_ENTRY64 *Flink;
-   struct LIST_ENTRY64 *Blink;
-} List;
+LIST_ENTRY64List;
 uint32_t   OwnerTag;
 uint32_t   Size;
 } DBGKD_DEBUG_DATA_HEADER64;
diff --git a/contrib/elf2dmp/pdb.h b/contrib/elf2dmp/pdb.h
index 8e395119d1..9a848f75e2 100644
--- a/contrib/elf2dmp/pdb.h
+++ b/contrib/elf2dmp/pdb.h
@@ -11,12 +11,14 @@
 #include 
 #include 
 
+#ifndef _WIN32
 typedef struct GUID {
 unsigned int Data1;
 unsigned short Data2;
 unsigned short Data3;
 unsigned char Data4[8];
 } GUID;
+#endif
 
 struct PDB_FILE {
 uint32_t size;
diff --git a/contrib/elf2dmp/pe.h b/contrib/elf2dmp/pe.h
index 374e06a9c5..0fd5c23c7f 100644
--- a/contrib/elf2dmp/pe.h
+++ b/contrib/elf2dmp/pe.h
@@ -10,6 +10,7 @@
 
 #include 
 
+#ifndef _WIN32
 typedef struct IMAGE_DOS_HEADER {
 uint16_t  e_magic;  /* 0x00: MZ Header signature */
 uint16_t  e_cblp;   /* 0x02: Bytes on last page of file */
@@ -88,8 +89,6 @@ typedef struct IMAGE_NT_HEADERS64 {
   IMAGE_OPTIONAL_HEADER64 OptionalHeader;
 } __attribute__ ((packed)) IMAGE_NT_HEADERS64;
 
-#define IMAGE_FILE_DEBUG_DIRECTORY  6
-
 typedef struct IMAGE_DEBUG_DIRECTORY {
   uint32_t Characteristics;
   uint32_t TimeDateStamp;
@@ -102,6 +101,9 @@ typedef struct IMAGE_DEBUG_DIRECTORY {
 } __attribute__ ((packed)) IMAGE_DEBUG_DIRECTORY;
 
 #define IMAGE_DEBUG_TYPE_CODEVIEW   2
+#endif
+
+#define IMAGE_FILE_DEBUG_DIRECTORY  6
 
 typedef struct guid_t {
 uint32_t a;
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

[John Snow]

On 12/17/18 4:14 PM, John Snow wrote:
> 
> 
> On 12/14/18 6:15 PM, John Snow wrote:
>> Touch up a few last things and remove the x- prefix.
>>
>> V3:
>>  - Reworked qmp_log to pretty-print the outgoing command, too [Vladimir]
>>  - Modified test to log only bitmap information [Vladimir]
>>  - Test disable/enable transaction toggle [Eric]
>>
>> Note that the filter I added is now unused, but I think we will want it
>> and it's small enough, so I'm going to check it in anyway. If you disagree,
>> I'll just drop the patch instead.
>>
>> --js
>>
>> John Snow (7):
>>   blockdev: abort transactions in reverse order
>>   blockdev: n-ary bitmap merge
>>   block: remove 'x' prefix from experimental bitmap APIs
>>   iotests.py: don't abort if IMGKEYSECRET is undefined
>>   iotests: add filter_generated_node_ids
>>   iotests: allow pretty-print for qmp_log
>>   iotests: add iotest 236 for testing bitmap merge
>>
>>  blockdev.c|  96 +---
>>  qapi/block-core.json  |  56 +-
>>  qapi/transaction.json |  12 +-
>>  tests/qemu-iotests/223|   4 +-
>>  tests/qemu-iotests/236| 124 +
>>  tests/qemu-iotests/236.out| 200 ++
>>  tests/qemu-iotests/group  |   1 +
>>  tests/qemu-iotests/iotests.py |  22 +++-
>>  8 files changed, 436 insertions(+), 79 deletions(-)
>>  create mode 100755 tests/qemu-iotests/236
>>  create mode 100644 tests/qemu-iotests/236.out
>>
> 
> Thanks, I'm staging patches 1-5 and I'll send the PR once we get to the
> bottom of patches 6 and 7, just to keep volume on the list down.
> 

NACK.

Patch 2 is incomplete an additional bugfix is needed.

--js

Re: [Qemu-devel] [PATCH 1/2] i386: remove the new CPUID 'PCONFIG' from Icelake-Server CPU model

[Robert Hoo]

On Wed, 2018-12-19 at 14:01 +, Daniel P. Berrangé wrote:
> On Wed, Dec 19, 2018 at 09:44:40PM +0800, Robert Hoo wrote:
> > Signed-off-by: Robert Hoo 
> > ---
> >  target/i386/cpu.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 677a3bd..b6113d0 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -2613,8 +2613,7 @@ static X86CPUDefinition builtin_x86_defs[] =
> > {
> >  CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG
> > |
> >  CPUID_7_0_ECX_AVX512_VPOPCNTDQ | CPUID_7_0_ECX_LA57,
> >  .features[FEAT_7_0_EDX] =
> > -CPUID_7_0_EDX_PCONFIG | CPUID_7_0_EDX_SPEC_CTRL |
> > -CPUID_7_0_EDX_SPEC_CTRL_SSBD,
> > +CPUID_7_0_EDX_SPEC_CTRL |
> > CPUID_7_0_EDX_SPEC_CTRL_SSBD,
> >  /* Missing: XSAVES (not supported by some Linux versions,
> >  * including v4.1 to v4.12).
> >  * KVM doesn't yet expose any XSAVES state save
> > component,
> 
> This was shipped in QEMU 3.1.0, so I don't think we can
> unconditionally
> remove it like this without breaking CPU model migration compat. 
> 
I think the sooner, the better. Take the time window that Icelake CPU
model has just shipped with QEMU 3.1.0 and is not publicly/widely used
yet.
> 
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH for-2.11 v3 12/16] travis/osx: silent texinfo warnings

[Peter Maydell]

On Wed, 19 Dec 2018 at 23:13, Philippe Mathieu-Daudé  wrote:
> I'm trying to reduce OSX warnings, as of current QEMU master I count
> 80 warnings using Xcode 10, which result in me not looking at them (or
> worst, at new ones...).

A lot of them are the address-of-packed-member warnings, which
I've been gradually looking at -- I think we're now down to
just the slirp ones, which I've been leaving until the big
refactor that's pending has landed.
Other than that there are a bunch of deprecated-declaration
warnings about SASL, which I never figured out an approach
for and so just suppress in my "grep logs for warnings"
script. I don't think there's too many other than that?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.11 v3 12/16] travis/osx: silent texinfo warnings

[Philippe Mathieu-Daudé]

On Wed, Dec 19, 2018 at 11:42 PM Peter Maydell  wrote:
> On Wed, 19 Dec 2018 at 21:34, Philippe Mathieu-Daudé  wrote:
> >
> > Hey Alex, Peter, what do you think about this patch?
> >
> > On Wed, Aug 9, 2017 at 10:27 PM Philippe Mathieu-Daudé  
> > wrote:
> > >
> > >   $ make info
> > > GEN qemu-doc.html
> > >   qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.
> > > GEN qemu-doc.txt
> > >   qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé 
> > > ---
> > >  .travis.yml | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/.travis.yml b/.travis.yml
> > > index 4bf69b0116..24f62fb7cf 100644
> > > --- a/.travis.yml
> > > +++ b/.travis.yml
> > > @@ -74,9 +74,11 @@ git:
> > ># we want to do this ourselves
> > >submodules: false
> > >  before_install:
> > > +  # silent texinfo warnings 
> > > (https://github.com/xiaohanyu/oh-my-emacs/blob/c664894e2f1c1cb0f95a9f2da88d41b00f190856/core/ome-basic.org#homebrew)
> > >- if [ "$TRAVIS_OS_NAME" == "osx" ]; then
> > >  brew update ;
> > > -brew install libffi gettext glib pixman ;
> > > +brew install libffi gettext glib pixman texinfo ;
> > > +brew link texinfo --force ;
> > >  fi
>
> The commit message is a bit lacking in detail about how it's
> fixing things. IIRC this message is caused because the stock
> OSX texinfo is too old, so installing a newer version via
> brew makes sense. But what does the "brew link" command do?

This was more than 1 year ago, now I try/force myself to write more
verbose commit message...
I don't remember exactly why I added the 'link' and will have to check
again (another thing I learned, write commit message for your self,
else you won't remember in 1 year ahead...).
I'm trying to reduce OSX warnings, as of current QEMU master I count
80 warnings using Xcode 10, which result in me not looking at them (or
worst, at new ones...).
I'll include this patch reworded in an OSX cleanup series.
Thanks,
Phil.

Re: [Qemu-devel] [PULL v2 00/35] Misc patches for 2018-12-18

[Peter Maydell]

On Wed, 19 Dec 2018 at 15:21, Paolo Bonzini  wrote:
>
> The following changes since commit e85c577158a2e8e252414959da9ef15c12eec63d:
>
>   Merge remote-tracking branch 
> 'remotes/huth-gitlab/tags/pull-request-2018-12-17' into staging (2018-12-18 
> 14:31:06 +)
>
> are available in the Git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to b4e8bbb9ae50045160957e7ab66c4f1cae535ea4:
>
>   avoid TABs in files that only contain a few (2018-12-19 16:03:22 +0100)
>
> 
> * HAX support for Linux hosts (Alejandro)
> * esp bugfixes (Guenter)
> * Windows build cleanup (Marc-André)
> * checkpatch logic improvements (Paolo)
> * coalesced range bugfix (Paolo)
> * switch testsuite to TAP (Paolo)
> * QTAILQ rewrite (Paolo)
> * block/iscsi.c cancellation fixes (Stefan)
> * improve selection of the default accelerator (Thomas)
>
> 


I get a compile failure on my OSX system:

  CC  tests/test-crypto-pbkdf.o
/Users/pm215/src/qemu-for-merges/tests/test-crypto-pbkdf.c:444:17:
error: too few arguments to function call, single argument 'msg' was
not specified
g_test_skip();
~~~ ^
/usr/local/Cellar/glib/2.58.1/include/glib-2.0/glib/gtestutils.h:209:1:
note: 'g_test_skip' declared here
GLIB_AVAILABLE_IN_2_38
^
/usr/local/Cellar/glib/2.58.1/include/glib-2.0/glib/gversionmacros.h:394:49:
note: expanded from macro 'GLIB_AVAILABLE_IN_2_38'
# define GLIB_AVAILABLE_IN_2_38 _GLIB_EXTERN
^
/usr/local/Cellar/glib/2.58.1/include/glib-2.0/glib/gmacros.h:456:22:
note: expanded from macro '_GLIB_EXTERN'
#define _GLIB_EXTERN extern
 ^
1 error generated.


I also get this splat trying to do 'make check-tcg':

make: Entering directory
'/home/petmay01/linaro/qemu-for-merges/build/all-linux-static'
  BUILD   debian9
The command '/bin/sh -c DEBIAN_FRONTEND=noninteractive eatmydata
apt install -y --no-install-recommends bison
build-essential ca-certificates clang flex
gettext git libtest-harness-perl
pkg-config psmisc python texinfo
$(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\
-f2)' returned a non-zero code: 100
Traceback (most recent call last):
  File "/home/petmay01/linaro/qemu-for-merges/tests/docker/docker.py",
line 563, in 
sys.exit(main())
  File "/home/petmay01/linaro/qemu-for-merges/tests/docker/docker.py",
line 560, in main
return args.cmdobj.run(args, argv)
  File "/home/petmay01/linaro/qemu-for-merges/tests/docker/docker.py",
line 376, in run
extra_files_cksum=cksum)
  File "/home/petmay01/linaro/qemu-for-merges/tests/docker/docker.py",
line 254, in build_image
quiet=quiet)
  File "/home/petmay01/linaro/qemu-for-merges/tests/docker/docker.py",
line 181, in _do_check
return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 541, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'build', '-t',
'qemu:debian9', '-f', '/tmp/docker_buildsvtS6R/tmp74LVkj.docker',
'/tmp/docker_buildsvtS6R']' returned non-zero exit status 100
/home/petmay01/linaro/qemu-for-merges/tests/docker/Makefile.include:53:
recipe for target 'docker-image-debian9' failed
make: *** [docker-image-debian9] Error 1


thanks
-- PMM

[Qemu-devel] 答复: Question about aio_poll and glib aio_ctx_dispatch

[Li Qiang]

Hello Paolo

Thanks for your kind reply.

Yes, aio_poll and aio_ctx_dispatch mostly run in different threads, though
Sometimes they can run in a thread nested from Fam’s slides:
→http://events17.linuxfoundation.org/sites/events/files/slides/Improving%20the%20QEMU%20Event%20Loop%20-%203.pdf

So you mean we can poll the same fd in two threads? If so , the ‘fd’ in 
‘qemu_aio_context’ will be run twice, I think there’s 
something I understand wrong.

Let’s take an example. In the ‘v9fs_reset’.

void v9fs_reset(V9fsState *s)
{
VirtfsCoResetData data = { .pdu = { .s = s }, .done = false };
Coroutine *co;

while (!QLIST_EMPTY(&s->active_list)) {
aio_poll(qemu_get_aio_context(), true);
}

co = qemu_coroutine_create(virtfs_co_reset, &data);
qemu_coroutine_enter(co);

while (!data.done) {
aio_poll(qemu_get_aio_context(), true);
}
}

Here we use aio_poll to wait the pending action to complete.
Here aio_poll will poll the fds in ‘qemu_aio_context’ in vcpu thread,
However, the main loop is also poll the fds in ‘qemu_aio_context’.
If some ‘fd’ in ‘qemu_aio_context’ has events, the two thread will be wakeup?
You say both will call aio_dispatch_handlers and timerlistgroup_run_timers.
But the are the same fd, how can this happen?

Thanks,
Li Qiang


发件人: Paolo Bonzini
发送时间: 2018年12月20日 4:42
收件人: Li Qiang; stefa...@redhat.com; f...@euphon.net; Qemu Developers; 李强
主题: Re: Question about aio_poll and glib aio_ctx_dispatch

On 19/12/18 11:05, Li Qiang wrote:
> Sent it to qemu-devel.
> 
> Li Qiang mailto:liq...@gmail.com>> 于2018年12月19日周
> 三 下午6:04写道：
> 
> Hello Paolo, Stefan, Fam and all,
> 
> Here I have a question about 'aio_poll'.
> IIUC the 'aio_poll' is (mostly) used for synchronous IO
> as I see a lot of code like this: 
> while(condition)
>  aio_poll();
> 
> However it seems the 'aio_poll' and 'aio_ctx_dispatch' both poll the fd.
> So what happened when the 'fd' has events, which function will be
> wakeup?

Roughly speaking, aio_poll is used for synchronous IO and within I/O
threads; aio_ctx_dispatch is used within the main thread.

Both end up calling aio_dispatch_handlers and timerlistgroup_run_timers.

Paolo

Re: [Qemu-devel] [PATCH for-2.11 v3 12/16] travis/osx: silent texinfo warnings

[Peter Maydell]

On Wed, 19 Dec 2018 at 21:34, Philippe Mathieu-Daudé  wrote:
>
> Hey Alex, Peter, what do you think about this patch?
>
> On Wed, Aug 9, 2017 at 10:27 PM Philippe Mathieu-Daudé  
> wrote:
> >
> >   $ make info
> > GEN qemu-doc.html
> >   qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.
> > GEN qemu-doc.txt
> >   qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.
> >
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> >  .travis.yml | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/.travis.yml b/.travis.yml
> > index 4bf69b0116..24f62fb7cf 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -74,9 +74,11 @@ git:
> ># we want to do this ourselves
> >submodules: false
> >  before_install:
> > +  # silent texinfo warnings 
> > (https://github.com/xiaohanyu/oh-my-emacs/blob/c664894e2f1c1cb0f95a9f2da88d41b00f190856/core/ome-basic.org#homebrew)
> >- if [ "$TRAVIS_OS_NAME" == "osx" ]; then
> >  brew update ;
> > -brew install libffi gettext glib pixman ;
> > +brew install libffi gettext glib pixman texinfo ;
> > +brew link texinfo --force ;
> >  fi

The commit message is a bit lacking in detail about how it's
fixing things. IIRC this message is caused because the stock
OSX texinfo is too old, so installing a newer version via
brew makes sense. But what does the "brew link" command do?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] ui/cocoa: Include less of the generated modular QAPI headers

[Philippe Mathieu-Daudé]

On 12/19/18 10:12 AM, Markus Armbruster wrote:
> Avoids pointless recompilation.  Missed in commit 112ed241f5d.
> 
> Signed-off-by: Markus Armbruster 
> ---
> Untested; I don't have access to a Mac.

You kinda do, open a GitHub account and enable Travis-CI (free for
public projects), then pushing a branch to your GitHub will trigger a
Travis build:

https://travis-ci.org/philmd/qemu/jobs/470146270#L3047

> 
>  ui/cocoa.m | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index ecf12bfc2e..43e751693c 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -32,7 +32,8 @@
>  #include "ui/input.h"
>  #include "sysemu/sysemu.h"
>  #include "qapi/error.h"
> -#include "qapi/qapi-commands.h"
> +#include "qapi/qapi-commands-block-core.h"
> +#include "qapi/qapi-commands-misc.h"
>  #include "sysemu/blockdev.h"
>  #include "qemu-version.h"
>  #include 
> 

Re: [Qemu-devel] [PATCH 2/4] q35: set split kernel irqchip as default

[Paolo Bonzini]

On 19/12/18 22:24, Eduardo Habkost wrote:
> On Wed, Dec 19, 2018 at 09:12:55PM +0100, Paolo Bonzini wrote:
>> On 19/12/18 09:50, Peter Xu wrote:
>>> Starting from QEMU 4.0, let's specify "split" as the default value for
>>> kernel-irqchip.
>>>
>>> So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
>>>for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N
>>>(omitting all the "kernel_irqchip_" prefix)
>>>
>>> Note that this "split" is optional - we'll first try to enable split
>>> kernel irqchip, and we'll fall back to complete kernel irqchip if we
>>> found that the kernel capability is missing.
>>
>> Please just fail completely and require a new kernel for the 4.0 machine
>> type.  There are subtle differences between kernel and QEMU irqchip, I
>> don't think we want to open that can of worms.
> 
> This would make existing VMs that are runnable with pc-q35-3.1.0
> not runnable by only updating the machine-type.
> 
> The good news is that we can make this a non-issue by clearly
> documenting that QEMU needs a more recent kernel (just like we'll
> do for RDTSCP[1]).

Right, RDTSCP is exactly what came to mind.

Paolo

Re: [Qemu-devel] [PATCH for-2.11 v3 12/16] travis/osx: silent texinfo warnings

[Philippe Mathieu-Daudé]

Hey Alex, Peter, what do you think about this patch?

On Wed, Aug 9, 2017 at 10:27 PM Philippe Mathieu-Daudé  wrote:
>
>   $ make info
> GEN qemu-doc.html
>   qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.
> GEN qemu-doc.txt
>   qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .travis.yml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 4bf69b0116..24f62fb7cf 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -74,9 +74,11 @@ git:
># we want to do this ourselves
>submodules: false
>  before_install:
> +  # silent texinfo warnings 
> (https://github.com/xiaohanyu/oh-my-emacs/blob/c664894e2f1c1cb0f95a9f2da88d41b00f190856/core/ome-basic.org#homebrew)
>- if [ "$TRAVIS_OS_NAME" == "osx" ]; then
>  brew update ;
> -brew install libffi gettext glib pixman ;
> +brew install libffi gettext glib pixman texinfo ;
> +brew link texinfo --force ;
>  fi
>- wget -O - 
> http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar 
> -xvJ
>- travis_retry git submodule update --init --recursive
> --
> 2.13.3
>

Re: [Qemu-devel] [PATCH 1/2] scsi-disk: Convert from DPRINTF() macro to trace events

[Paolo Bonzini]

On 19/12/18 22:30, Paolo Bonzini wrote:
> On 07/12/18 17:37, Laurent Vivier wrote:
>>> TIL about trace_event_get_state_backends(), I'll use it to clean other
>>> hexdump traces, thanks :)
>> I've copied that from gdbstub.c where there is an hexdump() function
>> using it.
>>
>> I'm going to send a v2 of this patch, because I think it is not needed
>> in this case to pass the function in the parameters.
> 
> I think it's better like this, you'd have to free the buffer in the
> caller and that's also ugly.

Nevermind, I diffed v1 and v2 and I see what I mean.  I queued v2.

Paolo

Re: [Qemu-devel] [PATCH 1/2] scsi-disk: Convert from DPRINTF() macro to trace events

[Paolo Bonzini]

On 07/12/18 17:37, Laurent Vivier wrote:
>> TIL about trace_event_get_state_backends(), I'll use it to clean other
>> hexdump traces, thanks :)
> I've copied that from gdbstub.c where there is an hexdump() function
> using it.
> 
> I'm going to send a v2 of this patch, because I think it is not needed
> in this case to pass the function in the parameters.

I think it's better like this, you'd have to free the buffer in the
caller and that's also ugly.

Paolo

Re: [Qemu-devel] [PATCH 1/2] scsi-disk: Convert from DPRINTF() macro to trace events

[Paolo Bonzini]

On 07/12/18 14:17, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier 
> ---
>  hw/scsi/scsi-disk.c  | 105 +--
>  hw/scsi/trace-events |  29 
>  2 files changed, 81 insertions(+), 53 deletions(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 0e9027c8f3..29e541efdb 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -19,15 +19,6 @@
>   * the host adapter emulator.
>   */
>  
> -//#define DEBUG_SCSI
> -
> -#ifdef DEBUG_SCSI
> -#define DPRINTF(fmt, ...) \
> -do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) do {} while(0)
> -#endif
> -
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
>  #include "qapi/error.h"
> @@ -41,6 +32,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
>  #include "hw/block/block.h"
>  #include "sysemu/dma.h"
>  #include "qemu/cutils.h"
> +#include "trace.h"
>  
>  #ifdef __linux
>  #include 
> @@ -128,8 +120,8 @@ static void scsi_free_request(SCSIRequest *req)
>  /* Helper function for command completion with sense.  */
>  static void scsi_check_condition(SCSIDiskReq *r, SCSISense sense)
>  {
> -DPRINTF("Command complete tag=0x%x sense=%d/%d/%d\n",
> -r->req.tag, sense.key, sense.asc, sense.ascq);
> +trace_scsi_disk_check_condition(r->req.tag, sense.key, sense.asc,
> +sense.ascq);
>  scsi_req_build_sense(&r->req, sense);
>  scsi_req_complete(&r->req, CHECK_CONDITION);
>  }
> @@ -317,7 +309,7 @@ static void scsi_read_complete(void * opaque, int ret)
>  }
>  
>  block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
> -DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, r->qiov.size);
> +trace_scsi_disk_read_complete(r->req.tag, r->qiov.size);
>  
>  n = r->qiov.size / 512;
>  r->sector += n;
> @@ -388,7 +380,7 @@ static void scsi_read_data(SCSIRequest *req)
>  SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
>  bool first;
>  
> -DPRINTF("Read sector_count=%d\n", r->sector_count);
> +trace_scsi_disk_read_data_count(r->sector_count);
>  if (r->sector_count == 0) {
>  /* This also clears the sense buffer for REQUEST SENSE.  */
>  scsi_req_complete(&r->req, GOOD);
> @@ -401,7 +393,7 @@ static void scsi_read_data(SCSIRequest *req)
>  /* The request is used as the AIO opaque value, so add a ref.  */
>  scsi_req_ref(&r->req);
>  if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
> -DPRINTF("Data transfer direction invalid\n");
> +trace_scsi_disk_read_data_invalid();
>  scsi_read_complete(r, -EINVAL);
>  return;
>  }
> @@ -502,7 +494,7 @@ static void scsi_write_complete_noio(SCSIDiskReq *r, int 
> ret)
>  return;
>  } else {
>  scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
> -DPRINTF("Write complete tag=0x%x more=%zd\n", r->req.tag, 
> r->qiov.size);
> +trace_scsi_disk_write_complete_noio(r->req.tag, r->qiov.size);
>  scsi_req_data(&r->req, r->qiov.size);
>  }
>  
> @@ -540,7 +532,7 @@ static void scsi_write_data(SCSIRequest *req)
>  /* The request is used as the AIO opaque value, so add a ref.  */
>  scsi_req_ref(&r->req);
>  if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
> -DPRINTF("Data transfer direction invalid\n");
> +trace_scsi_disk_write_data_invalid();
>  scsi_write_complete_noio(r, -EINVAL);
>  return;
>  }
> @@ -605,8 +597,7 @@ static int scsi_disk_emulate_vpd_page(SCSIRequest *req, 
> uint8_t *outbuf)
>  switch (page_code) {
>  case 0x00: /* Supported page codes, mandatory */
>  {
> -DPRINTF("Inquiry EVPD[Supported pages] "
> -"buffer size %zd\n", req->cmd.xfer);
> +trace_scsi_disk_emulate_vpd_page_00(req->cmd.xfer);
>  outbuf[buflen++] = 0x00; /* list of supported pages (this page) */
>  if (s->serial) {
>  outbuf[buflen++] = 0x80; /* unit serial number */
> @@ -624,7 +615,7 @@ static int scsi_disk_emulate_vpd_page(SCSIRequest *req, 
> uint8_t *outbuf)
>  int l;
>  
>  if (!s->serial) {
> -DPRINTF("Inquiry (EVPD[Serial number] not supported\n");
> +trace_scsi_disk_emulate_vpd_page_80_not_supported();
>  return -1;
>  }
>  
> @@ -633,8 +624,7 @@ static int scsi_disk_emulate_vpd_page(SCSIRequest *req, 
> uint8_t *outbuf)
>  l = 36;
>  }
>  
> -DPRINTF("Inquiry EVPD[Serial number] "
> -"buffer size %zd\n", req->cmd.xfer);
> +trace_scsi_disk_emulate_vpd_page_80(req->cmd.xfer);
>  memcpy(outbuf + buflen, s->serial, l);
>  buflen += l;
>  break;
> @@ -649,8 +639,7 @@ static int scsi_disk_emulate_vpd_page(SCSIRequest *req, 
> uint8_t *outbuf)
>  if (id_len > max_len) {
>  id_len = max_len;
>  }
> -DPRINTF("Inquiry EVPD[Device

Re: [Qemu-devel] [PATCH 2/4] q35: set split kernel irqchip as default

[Eduardo Habkost]

On Wed, Dec 19, 2018 at 09:12:55PM +0100, Paolo Bonzini wrote:
> On 19/12/18 09:50, Peter Xu wrote:
> > Starting from QEMU 4.0, let's specify "split" as the default value for
> > kernel-irqchip.
> > 
> > So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
> >for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N
> >(omitting all the "kernel_irqchip_" prefix)
> > 
> > Note that this "split" is optional - we'll first try to enable split
> > kernel irqchip, and we'll fall back to complete kernel irqchip if we
> > found that the kernel capability is missing.
> 
> Please just fail completely and require a new kernel for the 4.0 machine
> type.  There are subtle differences between kernel and QEMU irqchip, I
> don't think we want to open that can of worms.

This would make existing VMs that are runnable with pc-q35-3.1.0
not runnable by only updating the machine-type.

The good news is that we can make this a non-issue by clearly
documenting that QEMU needs a more recent kernel (just like we'll
do for RDTSCP[1]).

[1] https://lore.kernel.org/lkml/20181210181328.ga...@zn.tnic/

-- 
Eduardo

[Qemu-devel] [PATCH v2 0/3] MAINTAINERS: Clean up main target/mips section

[Aleksandar Markovic]

From: Aleksandar Markovic 

Clean up main target/mips section. Add some new files, add email
filter, and reorder items.

v1->v2:
  - fixed typo in a commit message
  - removed unnecessary "?" in a line for filtering email subjects

Aleksandar Markovic (3):
  MAINTAINERS: target/mips: Add MIPS files under default-configs
directory
  MAINTAINERS: target/mips: Add filter for mips in email subjects
  MAINTAINERS: target/mips: Reorder items alphabetically

 MAINTAINERS | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v2 3/3] MAINTAINERS: target/mips: Reorder items alphabetically

[Aleksandar Markovic]

From: Aleksandar Markovic 

Reorder items alphabetically for better visibility.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aleksandar Markovic 
---
 MAINTAINERS | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 05bf140824..7b7ef83d5c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -207,18 +207,18 @@ R: Stefan Markovic 
 S: Maintained
 F: target/mips/
 F: default-configs/*mips*
+F: disas/mips.c
+F: disas/nanomips.cpp
+F: disas/nanomips.h
+F: hw/intc/mips_gic.c
 F: hw/mips/
 F: hw/misc/mips_*
-F: hw/intc/mips_gic.c
 F: hw/timer/mips_gictimer.c
+F: include/hw/intc/mips_gic.h
 F: include/hw/mips/
 F: include/hw/misc/mips_*
-F: include/hw/intc/mips_gic.h
 F: include/hw/timer/mips_gictimer.h
 F: tests/tcg/mips/
-F: disas/mips.c
-F: disas/nanomips.h
-F: disas/nanomips.cpp
 K: ^Subject:.*(?i)mips
 
 Moxie
-- 
2.17.1

[Qemu-devel] [PATCH v2 1/3] MAINTAINERS: target/mips: Add MIPS files under default-configs directory

[Aleksandar Markovic]

From: Aleksandar Markovic 

Add following files as maintained within the main MIPS target
section in MAINTAINERS:

default-configs/mips64el-linux-user.mak
default-configs/mips64-linux-user.mak
default-configs/mipsn32el-linux-user.mak
default-configs/mipsn32-linux-user.mak
default-configs/mipsel-linux-user.mak
default-configs/mips-linux-user.mak
default-configs/mips64el-softmmu.mak
default-configs/mips64-softmmu.mak
default-configs/mipsel-softmmu.mak
default-configs/mips-softmmu.mak
default-configs/mips-softmmu-common.mak

Future nanoMIPS user mode will also have its .mak file, and
because of that "*mips*" was used instead of "mips*" as a
shorthand in the new item in MAINTAINERS.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aleksandar Markovic 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 83c127f0d6..c052523373 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -206,6 +206,7 @@ M: Aleksandar Markovic 
 R: Stefan Markovic 
 S: Maintained
 F: target/mips/
+F: default-configs/*mips*
 F: hw/mips/
 F: hw/misc/mips_*
 F: hw/intc/mips_gic.c
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/3] MAINTAINERS: target/mips: Add filter for mips in email subjects

[Aleksandar Markovic]

From: Aleksandar Markovic 

Add ability to redirect mails (sent to qemu-devel) containing
"mips" in the subject line to MIPS maintainers and reviewers.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Aleksandar Markovic 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c052523373..05bf140824 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -219,6 +219,7 @@ F: tests/tcg/mips/
 F: disas/mips.c
 F: disas/nanomips.h
 F: disas/nanomips.cpp
+K: ^Subject:.*(?i)mips
 
 Moxie
 M: Anthony Green 
-- 
2.17.1

[Qemu-devel] [PATCH v3 7/8] disas: nanoMIPS: Fix an FP-related misnomer 2

[Aleksandar Markovic]

From: Aleksandar Markovic 

-uint64 NMD::extract_fs_15_14_13_12_11(uint64 instruction)
+uint64 NMD::extract_fs_20_19_18_17_16(uint64 instruction)

Signed-off-by: Aleksandar Markovic 
---
 disas/nanomips.cpp | 242 ++---
 disas/nanomips.h   |   2 +-
 2 files changed, 122 insertions(+), 122 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 9682725eef..8f6db93ea4 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -1110,7 +1110,7 @@ uint64 NMD::extract_u_15_to_0(uint64 instruction)
 }
 
 
-uint64 NMD::extract_fs_15_14_13_12_11(uint64 instruction)
+uint64 NMD::extract_fs_20_19_18_17_16(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 16, 5);
@@ -1598,7 +1598,7 @@ bool NMD::SLTU_cond(uint64 instruction)
 std::string NMD::ABS_D(uint64 instruction)
 {
 uint64 fd_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string fs = FPR(copy(fs_value));
 std::string fd = FPR(copy(fd_value));
@@ -1620,7 +1620,7 @@ std::string NMD::ABS_D(uint64 instruction)
 std::string NMD::ABS_S(uint64 instruction)
 {
 uint64 fd_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string fs = FPR(copy(fs_value));
 std::string fd = FPR(copy(fd_value));
@@ -1752,7 +1752,7 @@ std::string NMD::ADD(uint64 instruction)
 std::string NMD::ADD_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string ft = FPR(copy(ft_value));
@@ -1777,7 +1777,7 @@ std::string NMD::ADD_D(uint64 instruction)
 std::string NMD::ADD_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string ft = FPR(copy(ft_value));
@@ -3379,7 +3379,7 @@ std::string NMD::CACHEE(uint64 instruction)
 std::string NMD::CEIL_L_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -3401,7 +3401,7 @@ std::string NMD::CEIL_L_D(uint64 instruction)
 std::string NMD::CEIL_L_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -3423,7 +3423,7 @@ std::string NMD::CEIL_L_S(uint64 instruction)
 std::string NMD::CEIL_W_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -3445,7 +3445,7 @@ std::string NMD::CEIL_W_D(uint64 instruction)
 std::string NMD::CEIL_W_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -3511,7 +3511,7 @@ std::string NMD::CFC2(uint64 instruction)
 std::string NMD::CLASS_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -3533,7 +3533,7 @@ std::string NMD::CLASS_D(uint64 instruction)
 std::string NMD::CLASS_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -3599,7 +3599,7 @@ std::string NMD::CLZ(uint64 instruction)
 std::string NMD::CMP_AF_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
-uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
+uint64 fs_value = extract_fs_20_19_18_17_16(instru

[Qemu-devel] [PATCH v3 5/8] disas: nanoMIPS: Name some function in a more descriptive way

[Aleksandar Markovic]

From: Aleksandar Markovic 

Rename some functions that have names that are hard to understand.

Reviewed-by: Stefan Markovic 
Signed-off-by: Aleksandar Markovic 
---
 disas/nanomips.cpp | 112 ++---
 disas/nanomips.h   |  32 ++---
 2 files changed, 72 insertions(+), 72 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 9e876305f1..477df84d93 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -683,7 +683,7 @@ uint64 NMD::extract_shift3_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil3il3bs9Fmsb11(uint64 instruction)
+uint64 NMD::extract_u_11_10_9_8_7_6_5_4_3__s3(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 3, 9) << 3;
@@ -707,7 +707,7 @@ uint64 NMD::extract_rtz3_9_8_7(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil1il1bs17Fmsb17(uint64 instruction)
+uint64 NMD::extract_u_17_to_1__s1(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 1, 17) << 1;
@@ -767,7 +767,7 @@ uint64 NMD::extract_shift_4_3_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_shiftxil7il1bs4Fmsb4(uint64 instruction)
+uint64 NMD::extract_shiftx_10_9_8_7__s1(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 7, 4) << 1;
@@ -836,7 +836,7 @@ uint64 NMD::extract_rs_20_19_18_17_16(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil1il1bs2Fmsb2(uint64 instruction)
+uint64 NMD::extract_u_2_1__s1(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 1, 2) << 1;
@@ -934,7 +934,7 @@ uint64 NMD::extract_rs_4_3_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil3il3bs18Fmsb20(uint64 instruction)
+uint64 NMD::extract_u_20_to_3__s3(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 3, 18) << 3;
@@ -942,7 +942,7 @@ uint64 NMD::extr_uil3il3bs18Fmsb20(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil0il2bs4Fmsb5(uint64 instruction)
+uint64 NMD::extract_u_3_2_1_0__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 0, 4) << 2;
@@ -958,7 +958,7 @@ uint64 NMD::extract_cofun_25_24_23(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil0il2bs3Fmsb4(uint64 instruction)
+uint64 NMD::extract_u_2_1_0__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 0, 3) << 2;
@@ -1225,7 +1225,7 @@ uint64 NMD::extract_msbt_10_9_8_7_6(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil0il2bs6Fmsb7(uint64 instruction)
+uint64 NMD::extract_u_5_4_3_2_1_0__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 0, 6) << 2;
@@ -1259,7 +1259,7 @@ uint64 NMD::extract_rs3_6_5_4(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil0il32bs32Fmsb63(uint64 instruction)
+uint64 NMD::extract_u_31_to_0__s32(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 0, 32) << 32;
@@ -1307,7 +1307,7 @@ uint64 NMD::extract_op_25_24_23_22_21(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil0il2bs7Fmsb8(uint64 instruction)
+uint64 NMD::extract_u_6_5_4_3_2_1_0__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 0, 7) << 2;
@@ -1339,7 +1339,7 @@ uint64 NMD::extract_eu_3_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil4il4bs4Fmsb7(uint64 instruction)
+uint64 NMD::extract_u_7_6_5_4__s4(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 4, 4) << 4;
@@ -1383,7 +1383,7 @@ uint64 NMD::extract_u_20_19_18_17_16_15_14_13(uint64 
instruction)
 }
 
 
-uint64 NMD::extr_uil2il2bs16Fmsb17(uint64 instruction)
+uint64 NMD::extract_u_17_to_2__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 2, 16) << 2;
@@ -1433,7 +1433,7 @@ uint64 NMD::extract_u_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil3il3bs1_il8il2bs1Fmsb3(uint64 instruction)
+uint64 NMD::extract_u_3_8__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 3, 1) << 3;
@@ -1450,7 +1450,7 @@ uint64 NMD::extract_fd_10_9_8_7_6(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil0il2bs5Fmsb6(uint64 instruction)
+uint64 NMD::extract_u_4_3_2_1_0__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 0, 5) << 2;
@@ -1483,7 +1483,7 @@ uint64 NMD::extract_ct_25_24_23_22_21(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil2il2bs19Fmsb20(uint64 instruction)
+uint64 NMD::extract_u_20_to_2__s2(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 2, 19) << 2;
@@ -1501,7 +1501,7 @@ int64 NMD::extract_s_4_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_uil0il1bs4Fmsb4(uint64 instruction)
+uint64 NMD::extract_u_3_2_1_0__s1(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 0, 4) << 1;
@@ -1535,7 +1535,7 @@ bool NMD::BEQC_16__cond(uint64 instruction)
 {
 uint64 rs3 = extract_rs3_6_5_4(instruction);
 uint64 rt3 = extract_rt3_9_8_7(instruct

[Qemu-devel] [PATCH v3 8/8] disas: nanoMIPS: Fix an FP-related misnomer 3

[Aleksandar Markovic]

From: Aleksandar Markovic 

Rename NMD::extract_ft_20_19_18_17_16(uint64 instruction) to
NMD::extract_ft_25_24_23_22_21(uint64 instruction).

Signed-off-by: Aleksandar Markovic 
---
 disas/nanomips.cpp | 258 ++---
 disas/nanomips.h   |   2 +-
 2 files changed, 130 insertions(+), 130 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 8f6db93ea4..d354a67049 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -1184,7 +1184,7 @@ uint64 NMD::extract_rt3_9_8_7(uint64 instruction)
 }
 
 
-uint64 NMD::extract_ft_20_19_18_17_16(uint64 instruction)
+uint64 NMD::extract_ft_25_24_23_22_21(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 21, 5);
@@ -1597,7 +1597,7 @@ bool NMD::SLTU_cond(uint64 instruction)
  */
 std::string NMD::ABS_D(uint64 instruction)
 {
-uint64 fd_value = extract_ft_20_19_18_17_16(instruction);
+uint64 fd_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string fs = FPR(copy(fs_value));
@@ -1619,7 +1619,7 @@ std::string NMD::ABS_D(uint64 instruction)
  */
 std::string NMD::ABS_S(uint64 instruction)
 {
-uint64 fd_value = extract_ft_20_19_18_17_16(instruction);
+uint64 fd_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string fs = FPR(copy(fs_value));
@@ -1751,7 +1751,7 @@ std::string NMD::ADD(uint64 instruction)
  */
 std::string NMD::ADD_D(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
@@ -1776,7 +1776,7 @@ std::string NMD::ADD_D(uint64 instruction)
  */
 std::string NMD::ADD_S(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
@@ -2783,7 +2783,7 @@ std::string NMD::BC_32_(uint64 instruction)
 std::string NMD::BC1EQZC(uint64 instruction)
 {
 int64 s_value = extr_sil0il14bs1_il1il1bs13Tmsb14(instruction);
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string s = ADDRESS(encode_s_from_address(s_value), 4);
@@ -2805,7 +2805,7 @@ std::string NMD::BC1EQZC(uint64 instruction)
 std::string NMD::BC1NEZC(uint64 instruction)
 {
 int64 s_value = extr_sil0il14bs1_il1il1bs13Tmsb14(instruction);
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string s = ADDRESS(encode_s_from_address(s_value), 4);
@@ -3378,7 +3378,7 @@ std::string NMD::CACHEE(uint64 instruction)
  */
 std::string NMD::CEIL_L_D(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
@@ -3400,7 +3400,7 @@ std::string NMD::CEIL_L_D(uint64 instruction)
  */
 std::string NMD::CEIL_L_S(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
@@ -3422,7 +3422,7 @@ std::string NMD::CEIL_L_S(uint64 instruction)
  */
 std::string NMD::CEIL_W_D(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
@@ -3444,7 +3444,7 @@ std::string NMD::CEIL_W_D(uint64 instruction)
  */
 std::string NMD::CEIL_W_S(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
@@ -3510,7 +3510,7 @@ std::string NMD::CFC2(uint64 instruction)
  */
 std::string NMD::CLASS_D(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint64 ft_value = extract_ft_25_24_23_22_21(instruction);
 uint64 fs_value = extract_fs_20_19_18_17_16(instruction);
 
 std::string ft = FPR(copy(ft_value));
@@ -3532,7 +3532,7 @@ std::string NMD::CLASS_D(uint64 instruction)
  */
 std::string NMD::CLASS_S(uint64 instruction)
 {
-uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
+uint

[Qemu-devel] [PATCH v3 6/8] disas: nanoMIPS: Fix an FP-related misnomer 1

[Aleksandar Markovic]

From: Aleksandar Markovic 

Rename NMD::extract_fd_10_9_8_7_6(uint64 instruction) to
NMD::extract_fd_15_14_13_12_11(uint64 instruction).

Signed-off-by: Aleksandar Markovic 
---
 disas/nanomips.cpp | 142 ++---
 disas/nanomips.h   |   2 +-
 2 files changed, 72 insertions(+), 72 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 477df84d93..9682725eef 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -1442,7 +1442,7 @@ uint64 NMD::extract_u_3_8__s2(uint64 instruction)
 }
 
 
-uint64 NMD::extract_fd_10_9_8_7_6(uint64 instruction)
+uint64 NMD::extract_fd_15_14_13_12_11(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 11, 5);
@@ -1753,7 +1753,7 @@ std::string NMD::ADD_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -1778,7 +1778,7 @@ std::string NMD::ADD_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string ft = FPR(copy(ft_value));
 std::string fs = FPR(copy(fs_value));
@@ -3600,7 +3600,7 @@ std::string NMD::CMP_AF_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3624,7 +3624,7 @@ std::string NMD::CMP_AF_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3648,7 +3648,7 @@ std::string NMD::CMP_EQ_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3694,7 +3694,7 @@ std::string NMD::CMP_EQ_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3718,7 +3718,7 @@ std::string NMD::CMP_LE_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3764,7 +3764,7 @@ std::string NMD::CMP_LE_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3788,7 +3788,7 @@ std::string NMD::CMP_LT_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3834,7 +3834,7 @@ std::string NMD::CMP_LT_S(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);
-uint64 fd_value = extract_fd_10_9_8_7_6(instruction);
+uint64 fd_value = extract_fd_15_14_13_12_11(instruction);
 
 std::string fd = FPR(copy(fd_value));
 std::string fs = FPR(copy(fs_value));
@@ -3858,7 +3858,7 @@ std::string NMD::CMP_NE_D(uint64 instruction)
 {
 uint64 ft_value = extract_ft_20_19_18_17_16(instruction);
 uint64 fs_value = extract_fs_15_14_13_12_11(instruction);

[Qemu-devel] [PATCH v3 2/8] disas: nanoMIPS: Remove functions that are not used

[Aleksandar Markovic]

From: Aleksandar Markovic 

Some functions were not used at all. Compiler doesn't complain
since they are class memebers. Remove them - no future usage is
planned.

Reviewed-by: Stefan Markovic 
Signed-off-by: Aleksandar Markovic 
---
 disas/nanomips.cpp | 208 -
 disas/nanomips.h   |  25 --
 2 files changed, 233 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index f9ef0a25f4..935c2dee3c 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -852,23 +852,6 @@ uint64 NMD::extract_stripe_6(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil17il0bs1Fmsb0(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 17, 1);
-return value;
-}
-
-
-uint64 NMD::extr_xil2il0bs1_il15il0bs1Fmsb0(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 2, 1);
-value |= extract_bits(instruction, 15, 1);
-return value;
-}
-
-
 uint64 NMD::extract_ac_13_12(uint64 instruction)
 {
 uint64 value = 0;
@@ -919,14 +902,6 @@ uint64 NMD::extract_shift_5_4_3_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil10il0bs6Fmsb5(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 10, 6);
-return value;
-}
-
-
 uint64 NMD::extract_count_19_18_17_16(uint64 instruction)
 {
 uint64 value = 0;
@@ -943,15 +918,6 @@ uint64 NMD::extract_code_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil10il0bs4_il22il0bs4Fmsb3(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 10, 4);
-value |= extract_bits(instruction, 22, 4);
-return value;
-}
-
-
 uint64 NMD::extract_u_11_10_9_8_7_6_5_4_3_2_1_0(uint64 instruction)
 {
 uint64 value = 0;
@@ -976,14 +942,6 @@ uint64 NMD::extr_uil3il3bs18Fmsb20(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil12il0bs1Fmsb0(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 12, 1);
-return value;
-}
-
-
 uint64 NMD::extr_uil0il2bs4Fmsb5(uint64 instruction)
 {
 uint64 value = 0;
@@ -1008,14 +966,6 @@ uint64 NMD::extr_uil0il2bs3Fmsb4(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil10il0bs1Fmsb0(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 10, 1);
-return value;
-}
-
-
 uint64 NMD::extract_rd3_3_2_1(uint64 instruction)
 {
 uint64 value = 0;
@@ -1048,22 +998,6 @@ uint64 NMD::extract_ru_7_6_5_4_3(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil21il0bs5Fmsb4(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 21, 5);
-return value;
-}
-
-
-uint64 NMD::extr_xil9il0bs3Fmsb2(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 9, 3);
-return value;
-}
-
-
 uint64 NMD::extract_u_17_to_0(uint64 instruction)
 {
 uint64 value = 0;
@@ -1072,15 +1006,6 @@ uint64 NMD::extract_u_17_to_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil14il0bs1_il15il0bs1Fmsb0(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 14, 1);
-value |= extract_bits(instruction, 15, 1);
-return value;
-}
-
-
 uint64 NMD::extract_rsz4_4_2_1_0(uint64 instruction)
 {
 uint64 value = 0;
@@ -1090,14 +1015,6 @@ uint64 NMD::extract_rsz4_4_2_1_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil24il0bs1Fmsb0(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 24, 1);
-return value;
-}
-
-
 int64 NMD::extr_sil0il21bs1_il1il1bs20Tmsb21(uint64 instruction)
 {
 int64 value = 0;
@@ -1150,15 +1067,6 @@ int64 NMD::extract_shift_21_20_19_18_17_16(uint64 
instruction)
 }
 
 
-uint64 NMD::extr_xil6il0bs3_il10il0bs1Fmsb2(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 6, 3);
-value |= extract_bits(instruction, 10, 1);
-return value;
-}
-
-
 uint64 NMD::extract_rd2_3_8(uint64 instruction)
 {
 uint64 value = 0;
@@ -1168,14 +1076,6 @@ uint64 NMD::extract_rd2_3_8(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil16il0bs5Fmsb4(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 16, 5);
-return value;
-}
-
-
 uint64 NMD::extract_code_17_to_0(uint64 instruction)
 {
 uint64 value = 0;
@@ -1184,14 +1084,6 @@ uint64 NMD::extract_code_17_to_0(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil0il0bs12Fmsb11(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 0, 12);
-return value;
-}
-
-
 uint64 NMD::extract_size_20_19_18_17_16(uint64 instruction)
 {
 uint64 value = 0;
@@ -1260,15 +1152,6 @@ uint64 NMD::extract_hs_20_19_18_17_16(uint64 instruction)
 }
 
 
-uint64 NMD::extr_xil10il0bs1_il14il0bs2Fmsb1(uint64 instruction)
-{
-uint64 value = 0;
-value |= extract_bits(instruction, 10, 1);
-value |= extract_bits(instruction, 14, 2);
-return value;
-}
-
-
 uint64 NMD::extract_sel_13_12_11(uint64 instruction)
 {
 uint64 value = 0;
@@ -1285,14 +1168,6 @@ uint64 NMD::extract_lsb

[Qemu-devel] [PATCH v3 0/8] disas: nanoMIPS: Clean up several issues

[Aleksandar Markovic]

From: Aleksandar Markovic 

Clean up several misc issues in nanoMIPS disassembler. There are
more issues to be cleaned, and this is meant to be just the first
phase. Complete cleanup should happen over the course of next
few months.

All these changes should not and do not affect any functionality.

v2->v3:

  - added three patches that fix function misnomers.
  - minor changes in commit messages.

v1->v2:

  - patch 5 was somehow lost in v1, now should be fine.

Aleksandar Markovic (8):
  disas: nanoMIPS: Fix preamble text in nanomips.* files
  disas: nanoMIPS: Remove functions that are not used
  disas: nanoMIPS: Fix a function misnomer
  disas: nanoMIPS: Fix order of some invocations
  disas: nanoMIPS: Name some function in a more descriptive way
  disas: nanoMIPS: Fix an FP-related misnomer 1
  disas: nanoMIPS: Fix an FP-related misnomer 2
  disas: nanoMIPS: Fix an FP-related misnomer 3

 disas/nanomips.cpp | 1441 +++-
 disas/nanomips.h   |   72 +--
 2 files changed, 641 insertions(+), 872 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v3 3/8] disas: nanoMIPS: Fix a function misnomer

[Aleksandar Markovic]

From: Aleksandar Markovic 

Fix wrong function name. The convention in these files is that names of
extraction functions should reflect bit patterns they are extracting.

Reviewed-by: Stefan Markovic 
Signed-off-by: Aleksandar Markovic 
---
 disas/nanomips.cpp | 264 ++---
 disas/nanomips.h   |   2 +-
 2 files changed, 133 insertions(+), 133 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 935c2dee3c..cfad1ec845 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -1391,7 +1391,7 @@ uint64 NMD::extr_uil2il2bs16Fmsb17(uint64 instruction)
 }
 
 
-uint64 NMD::extract_rd_20_19_18_17_16(uint64 instruction)
+uint64 NMD::extract_rd_15_14_13_12_11(uint64 instruction)
 {
 uint64 value = 0;
 value |= extract_bits(instruction, 11, 5);
@@ -1579,7 +1579,7 @@ bool NMD::PREFE_cond(uint64 instruction)
 
 bool NMD::SLTU_cond(uint64 instruction)
 {
-uint64 rd = extract_rd_20_19_18_17_16(instruction);
+uint64 rd = extract_rd_15_14_13_12_11(instruction);
 return rd != 0;
 }
 
@@ -1727,7 +1727,7 @@ std::string NMD::ACLR(uint64 instruction)
 std::string NMD::ADD(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2039,7 +2039,7 @@ std::string NMD::ADDIUPC_48_(uint64 instruction)
 std::string NMD::ADDQ_PH(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2063,7 +2063,7 @@ std::string NMD::ADDQ_PH(uint64 instruction)
 std::string NMD::ADDQ_S_PH(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2087,7 +2087,7 @@ std::string NMD::ADDQ_S_PH(uint64 instruction)
 std::string NMD::ADDQ_S_W(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2112,7 +2112,7 @@ std::string NMD::ADDQ_S_W(uint64 instruction)
 std::string NMD::ADDQH_PH(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2137,7 +2137,7 @@ std::string NMD::ADDQH_PH(uint64 instruction)
 std::string NMD::ADDQH_R_PH(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2161,7 +2161,7 @@ std::string NMD::ADDQH_R_PH(uint64 instruction)
 std::string NMD::ADDQH_R_W(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2185,7 +2185,7 @@ std::string NMD::ADDQH_R_W(uint64 instruction)
 std::string NMD::ADDQH_W(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2209,7 +2209,7 @@ std::string NMD::ADDQH_W(uint64 instruction)
 std::string NMD::ADDSC(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extract_rd_20_19_18_17_16(instruction);
+uint64 rd_value = extract_rd_15_14_13_12_11(instruction);
 uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
 
 std::string rd = GPR(copy(rd_value));
@@ -2256,7 +2256,7 @@ std::string NMD::ADDU_16_(uint64 instruction)
 std::string NMD::ADDU_32_(uint64 instruction)
 {
 uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
-uint64 rd_value = extr

[Qemu-devel] [PATCH v3 1/8] disas: nanoMIPS: Fix preamble text in nanomips.* files

[Aleksandar Markovic]

From: Aleksandar Markovic 

Fix several mistakes in preambles of nanomips disassembler source
files.

Reviewed-by: Stefan Markovic 
Signed-off-by: Aleksandar Markovic 
---
 disas/nanomips.cpp | 7 ---
 disas/nanomips.h   | 7 ---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 1238c2ff33..f9ef0a25f4 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -1,13 +1,13 @@
 /*
  *  Source file for nanoMIPS disassembler component of QEMU
  *
- *  Copyright (C) 2018  Wave Computing
+ *  Copyright (C) 2018  Wave Computing, Inc.
  *  Copyright (C) 2018  Matthew Fortune 
- *  Copyright (C) 2018  Aleksandar Markovic 
+ *  Copyright (C) 2018  Aleksandar Markovic 
  *
  *  This program is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
+ *  the Free Software Foundation, either version 2 of the License, or
  *  (at your option) any later version.
  *
  *  This program is distributed in the hope that it will be useful,
@@ -17,6 +17,7 @@
  *
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see .
+ *
  */
 
 extern "C" {
diff --git a/disas/nanomips.h b/disas/nanomips.h
index 84cc9a6dfc..3df138d63f 100644
--- a/disas/nanomips.h
+++ b/disas/nanomips.h
@@ -1,13 +1,13 @@
 /*
  *  Header file for nanoMIPS disassembler component of QEMU
  *
- *  Copyright (C) 2018  Wave Computing
+ *  Copyright (C) 2018  Wave Computing, Inc.
  *  Copyright (C) 2018  Matthew Fortune 
- *  Copyright (C) 2018  Aleksandar Markovic 
+ *  Copyright (C) 2018  Aleksandar Markovic 
  *
  *  This program is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
+ *  the Free Software Foundation, either version 2 of the License, or
  *  (at your option) any later version.
  *
  *  This program is distributed in the hope that it will be useful,
@@ -17,6 +17,7 @@
  *
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see .
+ *
  */
 
 #ifndef NANOMIPS_DISASSEMBLER_H
-- 
2.17.1

Re: [Qemu-devel] Question about aio_poll and glib aio_ctx_dispatch

[Paolo Bonzini]

On 19/12/18 11:05, Li Qiang wrote:
> Sent it to qemu-devel.
> 
> Li Qiang mailto:liq...@gmail.com>> 于2018年12月19日周
> 三 下午6:04写道：
> 
> Hello Paolo, Stefan, Fam and all,
> 
> Here I have a question about 'aio_poll'.
> IIUC the 'aio_poll' is (mostly) used for synchronous IO
> as I see a lot of code like this: 
> while(condition)
>  aio_poll();
> 
> However it seems the 'aio_poll' and 'aio_ctx_dispatch' both poll the fd.
> So what happened when the 'fd' has events, which function will be
> wakeup?

Roughly speaking, aio_poll is used for synchronous IO and within I/O
threads; aio_ctx_dispatch is used within the main thread.

Both end up calling aio_dispatch_handlers and timerlistgroup_run_timers.

Paolo

Re: [Qemu-devel] [PATCH 2/4] q35: set split kernel irqchip as default

[Paolo Bonzini]

On 19/12/18 16:52, Michael S. Tsirkin wrote:
> On Wed, Dec 19, 2018 at 04:50:36PM +0800, Peter Xu wrote:
>> Starting from QEMU 4.0, let's specify "split" as the default value for
>> kernel-irqchip.
>>
>> So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
>>for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N
>>(omitting all the "kernel_irqchip_" prefix)
>>
>> Note that this "split" is optional - we'll first try to enable split
>> kernel irqchip, and we'll fall back to complete kernel irqchip if we
>> found that the kernel capability is missing.
>>
>> Signed-off-by: Peter Xu 
> 
> I'm split on this one ;)
> On the one hand why not make pc and q35 are consistent here?
> On the other hand we really should work to leave PC alone
> as much as we can ...
> 
> Paolo, what's your opinion?

The idea was to avoid bumping the minimal kernel version for PC, only
for Q35.  PC can still use split irqchip for security purposes, but only
Q35 needs it for features.

Paolo

Re: [Qemu-devel] [PATCH 1/4] kvm: let split be optional for kvm_arch_irqchip_create

[Paolo Bonzini]

On 19/12/18 16:53, Michael S. Tsirkin wrote:
> On Wed, Dec 19, 2018 at 04:50:35PM +0800, Peter Xu wrote:
>> This patch allows the kvm_arch_irqchip_create() to return 0 if the
>> split irqchip is specified but not forced by the user.  Also, modify
>> kvm_irqchip_create() similiarly.
>>
>> This patch should have no functional change for existing code since
>> currently if split is specified it must be forced by the user so we'll
>> always have machine_kernel_irqchip_required() returns true. However it
>> could potentially be used in follow up patches when we want to turn
>> split kernel irqchip as default for QEMU 4.0 which could trigger the
>> case that kernel_irqchip_required=N while kernel_irqchip_split=Y. When
>> with that, we'll first try with split irqchip, and falls back to
>> normal kernel irqchip when split capability is not provided by the
>> kernel.
>>
>> This brings us benefit that we can even run a default QEMU 4.0 on old
>> kernels that does not support split irqchip (<4.4) but at the same
>> time enable split irqchip for new kernels (>=4.4) as default.
>>
>> Signed-off-by: Peter Xu 
> 
> Paolo, if you can ack this one, I can merge the rest.

If I understand the code well, there is no change needed in the rest of
the code; having the semantics I asked for simply requires dropping this
patch.

However, the commit messages need some adjustment.

Paolo

> 
>> ---
>>  accel/kvm/kvm-all.c | 3 ++-
>>  target/i386/kvm.c   | 6 +++---
>>  2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index 4880a05399..b008364041 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -1468,7 +1468,8 @@ static void kvm_irqchip_create(MachineState *machine, 
>> KVMState *s)
>>   * in-kernel irqchip for us */
>>  ret = kvm_arch_irqchip_create(machine, s);
>>  if (ret == 0) {
>> -if (machine_kernel_irqchip_split(machine)) {
>> +if (machine_kernel_irqchip_required(machine) &&
>> +machine_kernel_irqchip_split(machine)) {
>>  perror("Split IRQ chip mode not supported.");
>>  exit(1);
>>  } else {
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 739cf8c8ea..8f919f8f9f 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -3685,9 +3685,9 @@ int kvm_arch_irqchip_create(MachineState *ms, KVMState 
>> *s)
>>  if (machine_kernel_irqchip_split(ms)) {
>>  ret = kvm_vm_enable_cap(s, KVM_CAP_SPLIT_IRQCHIP, 0, 24);
>>  if (ret) {
>> -error_report("Could not enable split irqchip mode: %s",
>> - strerror(-ret));
>> -exit(1);
>> +assert(ret < 0);
>> +/* If split not required, return 0 instead to retry */
>> +return machine_kernel_irqchip_required(ms) ? ret : 0;
>>  } else {
>>  DPRINTF("Enabled KVM_CAP_SPLIT_IRQCHIP\n");
>>  kvm_split_irqchip = true;
>> -- 
>> 2.17.1

Re: [Qemu-devel] [PATCH 2/4] q35: set split kernel irqchip as default

[Paolo Bonzini]

On 19/12/18 09:50, Peter Xu wrote:
> Starting from QEMU 4.0, let's specify "split" as the default value for
> kernel-irqchip.
> 
> So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
>for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N
>(omitting all the "kernel_irqchip_" prefix)
> 
> Note that this "split" is optional - we'll first try to enable split
> kernel irqchip, and we'll fall back to complete kernel irqchip if we
> found that the kernel capability is missing.

Please just fail completely and require a new kernel for the 4.0 machine
type.  There are subtle differences between kernel and QEMU irqchip, I
don't think we want to open that can of worms.

Paolo

Re: [Qemu-devel] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only

[John Snow]

On 12/19/18 2:01 PM, Eric Blake wrote:
> On 12/19/18 5:27 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
>> But still not sure that it worth it. Isn't it better to just remove
>> fields from dict,
>> which are unpredictable, instead of substituting them..
> 
> For getting the test to pass when we have a key:unpredictable value in
> the dict, you are right that both changing it to key:SUBST or removing
> key work at producing reproducible output. But when it comes to
> debugging test failure, having key:SUBST in the logs gives you a hint at
> what else to look at, whereas omitting key altogether may make the
> reason for the failure completely disappear from the logs.
> 
> Thus, I would argue that even though it is more complex to write a
> filter that can recursively substitute, the resulting output is easier
> to debug if a test starts failing - and that if the work in doing the
> more complex filtering has already been submitted and is not too much of
> a burden to maintain, then we might as well use it rather than going
> with the simpler case of just eliding the problematic keys or using just
> textual filtering.
> 
> However, I'm not in a good position to argue whether there is a
> reasonable maintenance burden with the patches in this series, vs. what
> it would take to rewrite 206 to do just textual filtering instead of QMP
> dict substitution filtering.
> 

My reasoning is this:

(1) It would be good if QMP filters behaved similarly to their plaintext
companions, as this is the least surprising behavior, and

(2) It would be best to log the keys provided in responses in case we
wish to test for their presence (and that their values match something
we were able to predict via a pattern), and

(3) Not arbitrarily changing the nature of the response invisibly in a
way that obscures it from log files to aid debugging, like you say.



I offer some ideas for how to add text filtering for QMP objects in
iotests in some of my replies, but it's not going to happen in 2018,
IMO. I want pretty-printing of QMP commands more than I want text
filtering of serialized QMP objects.

Re: [Qemu-devel] [PATCH v4 1/5] iotests: add qmp recursive sorting function

[John Snow]

On 12/19/18 2:19 PM, Eric Blake wrote:
> On 12/19/18 12:57 PM, John Snow wrote:
>>
>>
>> On 12/19/18 1:52 PM, Eric Blake wrote:
>>> On 12/18/18 7:52 PM, John Snow wrote:
 Python before 3.6 does not sort kwargs by default.
 If we want to print out pretty-printed QMP objects while
 preserving the "exec" > "arguments" ordering, we need a custom sort.
>>>
>>> Naive question - why do we need the sorting? Is it so that the output is
>>> deterministic?  Surely it can't be because the ordering otherwise makes
>>> a difference to execution.
>>>
>>
>> Yeah, it's because dicts are unordered and prior to 3.6 they may shuffle
>> around arbitrarily based on internal hashes.
>>
>> kwargs in particular are unordered- the order we send over the wire may
>> or may not reflect the order the test author wrote in their iotest.
> 
> Which shouldn't matter to what QMP executes, but MIGHT matter in getting
> reproducible log output.
> 
>>
>> Therefore, it's a way to get consistent ordering.
>>
>> Now, we CAN just rely on sort_keys=True to be done with it, however,
>> this puts arguments before execute, and it's less nice to read -- and
>> I'd have to change a LOT of test output.
> 
> Okay, I'm finally seeing it; the existing code has:
> 
>     def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
>     logmsg = '{"execute": "%s", "arguments": %s}' % \
>     (cmd, json.dumps(kwargs, sort_keys=True))
> 
> where our log is outputting a message that resembles a dict where
> "execute": is the first key, and the user's input dict is then sorted
> (the top-most output of {} is unsorted, but the nested ones are sorted,
> and possibly in a different order than the user typed them, but at least
> deterministic). But when you change to the user passing a full QMP
> command (rather than just a dict for the arguments of the QMP command),
> using sort_keys=True will sort everything _including_ putting
> "arguments" before "execute" (which is deterministic but changes log
> output); while using sort_keys=False will not affect output in newer
> python where kwargs remains ordered, but risks nondeterministic output
> on older python.
> 

Yes, and pretty-printing requires the full object, otherwise it's too
hard to manually re-indent the buffered subdict, and you have to indent
the outer dict, and...

...

It's easier to just build the full dictionary and then pretty-print it.

The motivation here is that pretty-printing a call to qmp_log should
indent both outgoing and incoming because that's semantically the most
obvious and right thing to do. Principle of least surprise.

>>
>> This way keeps the order you expect to see while maintaining a strict
>> order for the arguments. I think that's the nicest compromise until we
>> can stipulate 3.6+ in the year 2034 where kwargs *are* strictly ordered.
> 
> But kwargs strict ordering is only at the top-level, not recursive,
> right? That is, even if you can rely on:
> 
> foo(b=1, a=2)
> 
> providing kwargs where 'b' always outputs before 'a', I don't see how
> that would help:
> 
> foo(b={'d':3, 'c':4})
> 
> not reorder the keys within 'b' without your recursive ordering.
> 

Well, kwargs *are* dictionaries, and both become sorted in 3.6. You can
rely on the ordering at any level, but only in 3.6 and after.

We cannot rely on that behavior, though, so in our current reality:

(1) kwargs can be arbitrarily reordered
(2) subdictionaries in kwargs can also be arbitrarily reordered

And so my ordered_kwargs() function recursively orders any dictionaries
it finds, assuming a typical JSON structure -- which will suffice for
QMP objects.

In practice, it appears to correctly re-implement the behavior of
json.dumps(..., sort_keys=True).

--js

Re: [Qemu-devel] [PULL 00/64] slirp updates

[Peter Maydell]

On Tue, 18 Dec 2018 at 23:04, Samuel Thibault
 wrote:
>
> The following changes since commit e85c577158a2e8e252414959da9ef15c12eec63d:
>
>   Merge remote-tracking branch 
> 'remotes/huth-gitlab/tags/pull-request-2018-12-17' into staging (2018-12-18 
> 14:31:06 +)
>
> are available in the Git repository at:
>
>   https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 4c2b5ca1b5dd42bb96e954db7a46ebe39fd24620:
>
>   slirp: Mark debugging calls as unlikely (2018-12-18 23:44:35 +0100)
>
> 
> Abstract away slirp toward a libslirp

This fails to compile (all platforms):

For the windows builds, a compile failure in slirp.c:
/home/petmay01/qemu-for-merges/net/slirp.c: In function 'net_slirp_init':
/home/petmay01/qemu-for-merges/net/slirp.c:302:10: error: implicit
declaration of function 'inet_pton'
[-Werror=implicit-function-declaration]
 if (!inet_pton(AF_INET6, vprefix6, &ip6_prefix)) {
  ^
/home/petmay01/qemu-for-merges/net/slirp.c:302:5: error: nested extern
declaration of 'inet_pton' [-Werror=nested-externs]
 if (!inet_pton(AF_INET6, vprefix6, &ip6_prefix)) {
 ^

On Linux (x86-64 and other host archs) a link failure:

  LINKarm-softmmu/qemu-system-arm
../slirp/ncsi.o: In function `ncsi_input':
/home/petmay01/linaro/qemu-for-merges/slirp/ncsi.c:166: undefined
reference to `slirp_output'
../slirp/slirp.o: In function `arp_input':
/home/petmay01/linaro/qemu-for-merges/slirp/slirp.c:835: undefined
reference to `slirp_output'
../slirp/slirp.o: In function `if_encap4':
/home/petmay01/linaro/qemu-for-merges/slirp/slirp.c:935: undefined
reference to `slirp_output'
../slirp/slirp.o: In function `if_encap':
/home/petmay01/linaro/qemu-for-merges/slirp/slirp.c:1021: undefined
reference to `slirp_output'
collect2: error: ld returned 1 exit status
Makefile:199: recipe for target 'qemu-system-arm' failed

It also manages to provoke an internal linker error on freebsd:

/usr/bin/ld: error in ../slirp/socket.o(.eh_frame); no .eh_frame_hdr
table will be created.
/usr/bin/ld: ../slirp/socket.o: invalid string offset 1416 >= 978 for
section `.strtab'
/usr/bin/ld: ../slirp/socket.o: invalid relocation type 38
/usr/bin/ld: BFD 2.17.50 [FreeBSD] 2007-07-03 assertion fail
/usr/src/gnu/usr.bin/binutils/libbfd/../../../../contrib/binutils/bfd/elf64-x86-
64.c:276
/usr/bin/ld: ../slirp/socket.o: invalid relocation type 51
/usr/bin/ld: BFD 2.17.50 [FreeBSD] 2007-07-03 assertion fail
/usr/src/gnu/usr.bin/binutils/libbfd/../../../../contrib/binutils/bfd/elf64-x86-64.c:276
/usr/bin/ld: ../slirp/socket.o: invalid relocation type 57
[enormous long list of similar messages]
/usr/bin/ld: ../slirp/socket.o: invalid relocation type 8248
/usr/bin/ld: BFD 2.17.50 [FreeBSD] 2007-07-03 assertion fail
/usr/src/gnu/usr.bin/binutils/libbfd/../../../../contrib/binutils/bfd/elf64-x86-
64.c:276
/usr/bin/ld: BFD 2.17.50 [FreeBSD] 2007-07-03 internal error, aborting
at /usr/src/gnu/usr.bin/binutils/libbfd/../../../../contrib/binutils/b
fd/reloc.c line 5288 in bfd_byte
*bfd_generic_get_relocated_section_contents(bfd *, struct
bfd_link_info *, struct bfd_link_order *, bfd_byte
 *, bfd_boolean, asymbol **)

/usr/bin/ld: Please report this bug.

c++: error: linker command failed with exit code 1 (use -v to see invocation)

On NetBSD, a different slirp.c compiler error building the bsd-user
target:
In file included from ../slirp/slirp.c:31:0:
/var/tmp/qemu-test.nLOQbq/include/hw/hw.h:6:2: error: #error Cannot
include hw/hw.h from user emulation
 #error Cannot include hw/hw.h from user emulation
  ^

On OpenBSD, a rather uninformative linker error:
/usr/bin/ld: final link failed: Nonrepresentable section on output
collect2: error: ld returned 1 exit status

On OSX, the clang version of the same issue as Linux:
Undefined symbols for architecture x86_64:
  "_slirp_output", referenced from:
  _ncsi_input in ncsi.o
  _slirp_input in slirp.o
  _if_encap in slirp.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

thanks
-- PMM

Re: [Qemu-devel] Booting Raspbian on RPi emulation

[Ben Hekster via Qemu-devel]

Should have tested a little more:

While the crashing has stopped, the window isn't responsive to 
keystrokes.  This includes the frame buffer emulation itself (so I can't 
actually log in) as well as the QEMU Monitor; nothing I type has any effect in 
either.

Ben

> On Dec 19, 2018, at 11:36, Ben Hekster  wrote:
> 
> Checked out tag patchew/20181201123056.432-1-peter.mayd...@linaro.org 
>  from 
> https://github.com/patchew-project/qemu 
>  and verified the crash no longer 
> occurs.  It does indeed appear that the crash previously happened right 
> around the time where I can now see the window being resized.
> 
> I've attached a screenshot here for you; not sure if this will make it 
> through to the mailing list...
> 
> Ben
> 
>> On Dec 19, 2018, at 10:43, Peter Maydell > > wrote:
>> 
>> On Wed, 19 Dec 2018 at 17:46, Ben Hekster > > wrote:
>>> probably a manifestation of https://bugs.launchpad.net/qemu/+bug/1802684 
>>> 
>> Yep. If you could test the patchset at
>> http://patchew.org/QEMU/20181201123056.432-1-peter.mayd...@linaro.org/ 
>> 
>> and confirm that it fixes the problem that would be great.
>> 
>> thanks
>> -- PMM
> 
> 

Re: [Qemu-devel] [QEMU-devel][PATCH v3] aio-posix: Fix concurrent aio_poll/set_fd_handler.

[Paolo Bonzini]

On 19/12/18 17:29, Remy NOEL wrote:
>>
>>
>>>  atomic_read(&ctx->poll_disable_cnt) +
>>> poll_disable_change);
>>>   -    aio_epoll_update(ctx, node, is_new);
>>> +    if (new_node) {
>>> +    aio_epoll_update(ctx, new_node, is_new);
>>> +    }
>>>   qemu_lockcnt_unlock(&ctx->list_lock);
>>>   aio_notify(ctx);
>> ... so I think this should be "if (node || new_node)"?
> 
> Well, currently, when an AioHandler is removed, we do not change
> node->pdf.events (only revents).
> 
> Therefore a call to aio_epoll_update on node will only result in a call
> to epoll_ctl with EPOLL_CTL_MOD and the same event, which seems kinda
> pointless.
> 
> we may set node->pfd.events to 0 to unregister the file descriptor, but
> this would change the behavior compared to current handling of node
> deletion if i'm not mistaken.

You found another bug then. :)

Paolo

[Qemu-devel] [PATCH v2 20/23] riscv: tcg-target: Add the target init code

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Reviewed-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index bc1a2bc4ab..f718542d63 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -1869,6 +1869,37 @@ static void tcg_target_qemu_prologue(TCGContext *s)
 tcg_out_opc_imm(s, OPC_JALR, TCG_REG_ZERO, TCG_REG_RA, 0);
 }
 
+static void tcg_target_init(TCGContext *s)
+{
+tcg_target_available_regs[TCG_TYPE_I32] = 0x;
+if (TCG_TARGET_REG_BITS == 64) {
+tcg_target_available_regs[TCG_TYPE_I64] = 0x;
+}
+
+tcg_target_call_clobber_regs = -1u;
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S0);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S1);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S2);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S3);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S4);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S5);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S6);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S7);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S8);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S9);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S10);
+tcg_regset_reset_reg(tcg_target_call_clobber_regs, TCG_REG_S11);
+
+s->reserved_regs = 0;
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_ZERO);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_TMP0);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_TMP1);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_TMP2);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_SP);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_GP);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_TP);
+}
+
 typedef struct {
 DebugFrameHeader h;
 uint8_t fde_def_cfa[4];
-- 
2.19.1

[Qemu-devel] [PATCH v2 17/23] riscv: tcg-target: Add direct load and store instructions

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Reviewed-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 158 +
 1 file changed, 158 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 7216bad086..154315787c 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -1151,3 +1151,161 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, 
TCGLabelQemuLdst *l)
 tcg_out_goto(s, l->raddr);
 }
 #endif /* CONFIG_SOFTMMU */
+
+static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg lo, TCGReg hi,
+   TCGReg base, TCGMemOp opc, bool is_64)
+{
+const TCGMemOp bswap = opc & MO_BSWAP;
+
+/* We don't yet handle byteswapping, assert */
+g_assert(!bswap);
+
+switch (opc & (MO_SSIZE)) {
+case MO_UB:
+tcg_out_opc_imm(s, OPC_LBU, lo, base, 0);
+break;
+case MO_SB:
+tcg_out_opc_imm(s, OPC_LB, lo, base, 0);
+break;
+case MO_UW:
+tcg_out_opc_imm(s, OPC_LHU, lo, base, 0);
+break;
+case MO_SW:
+tcg_out_opc_imm(s, OPC_LH, lo, base, 0);
+break;
+case MO_UL:
+if (TCG_TARGET_REG_BITS == 64 && is_64) {
+tcg_out_opc_imm(s, OPC_LWU, lo, base, 0);
+break;
+}
+/* FALLTHRU */
+case MO_SL:
+tcg_out_opc_imm(s, OPC_LW, lo, base, 0);
+break;
+case MO_Q:
+/* Prefer to load from offset 0 first, but allow for overlap.  */
+if (TCG_TARGET_REG_BITS == 64) {
+tcg_out_opc_imm(s, OPC_LD, lo, base, 0);
+} else if (lo != base) {
+tcg_out_opc_imm(s, OPC_LW, lo, base, 0);
+tcg_out_opc_imm(s, OPC_LW, hi, base, 4);
+} else {
+tcg_out_opc_imm(s, OPC_LW, hi, base, 4);
+tcg_out_opc_imm(s, OPC_LW, lo, base, 0);
+}
+break;
+default:
+g_assert_not_reached();
+}
+}
+
+static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64)
+{
+TCGReg addr_regl, addr_regh __attribute__((unused));
+TCGReg data_regl, data_regh;
+TCGMemOpIdx oi;
+TCGMemOp opc;
+#if defined(CONFIG_SOFTMMU)
+tcg_insn_unit *label_ptr[1];
+#endif
+TCGReg base = TCG_REG_TMP0;
+
+data_regl = *args++;
+data_regh = (TCG_TARGET_REG_BITS == 32 && is_64 ? *args++ : 0);
+addr_regl = *args++;
+addr_regh = (TCG_TARGET_REG_BITS < TARGET_LONG_BITS ? *args++ : 0);
+oi = *args++;
+opc = get_memop(oi);
+
+#if defined(CONFIG_SOFTMMU)
+tcg_out_tlb_load(s, addr_regl, addr_regh, oi, label_ptr, 1);
+tcg_out_qemu_ld_direct(s, data_regl, data_regh, base, opc, is_64);
+add_qemu_ldst_label(s, 1, oi,
+(is_64 ? TCG_TYPE_I64 : TCG_TYPE_I32),
+data_regl, data_regh, addr_regl, addr_regh,
+s->code_ptr, label_ptr);
+#else
+if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
+tcg_out_ext32u(s, base, addr_regl);
+addr_regl = base;
+}
+
+if (guest_base == 0) {
+tcg_out_opc_reg(s, OPC_ADD, base, addr_regl, TCG_REG_ZERO);
+} else {
+tcg_out_opc_reg(s, OPC_ADD, base, TCG_GUEST_BASE_REG, addr_regl);
+}
+tcg_out_qemu_ld_direct(s, data_regl, data_regh, base, opc, is_64);
+#endif
+}
+
+static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg lo, TCGReg hi,
+   TCGReg base, TCGMemOp opc)
+{
+const TCGMemOp bswap = opc & MO_BSWAP;
+
+/* We don't yet handle byteswapping, assert */
+g_assert(!bswap);
+
+switch (opc & (MO_SSIZE)) {
+case MO_8:
+tcg_out_opc_store(s, OPC_SB, base, lo, 0);
+break;
+case MO_16:
+tcg_out_opc_store(s, OPC_SH, base, lo, 0);
+break;
+case MO_32:
+tcg_out_opc_store(s, OPC_SW, base, lo, 0);
+break;
+case MO_64:
+if (TCG_TARGET_REG_BITS == 64) {
+tcg_out_opc_store(s, OPC_SD, base, lo, 0);
+} else {
+tcg_out_opc_store(s, OPC_SW, base, lo, 0);
+tcg_out_opc_store(s, OPC_SW, base, hi, 4);
+}
+break;
+default:
+g_assert_not_reached();
+}
+}
+
+static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64)
+{
+TCGReg addr_regl, addr_regh __attribute__((unused));
+TCGReg data_regl, data_regh;
+TCGMemOpIdx oi;
+TCGMemOp opc;
+#if defined(CONFIG_SOFTMMU)
+tcg_insn_unit *label_ptr[1];
+#endif
+TCGReg base = TCG_REG_TMP0;
+
+data_regl = *args++;
+data_regh = (TCG_TARGET_REG_BITS == 32 && is_64 ? *args++ : 0);
+addr_regl = *args++;
+addr_regh = (TCG_TARGET_REG_BITS < TARGET_LONG_BITS ? *args++ : 0);
+oi = *args++;
+opc = get_memop(oi);
+
+#if defined(CONFIG_SOFTMMU)
+tcg_out_tlb_load(s, addr_regl, addr_regh, oi, label_ptr, 0);
+tcg_out_qemu_st_direct(s, data_regl, data_regh, base, opc);
+add_qemu_ldst_label(s, 0, oi,
+   

[Qemu-devel] [PATCH v2 23/23] configure: Add support for building RISC-V host

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Reviewed-by: Richard Henderson 
---
 configure | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 224d3071ac..79375affc1 100755
--- a/configure
+++ b/configure
@@ -710,6 +710,12 @@ elif check_define __s390__ ; then
   else
 cpu="s390"
   fi
+elif check_define __riscv ; then
+  if check_define _LP64 ; then
+cpu="riscv64"
+  else
+cpu="riscv32"
+  fi
 elif check_define __arm__ ; then
   cpu="arm"
 elif check_define __aarch64__ ; then
@@ -722,7 +728,7 @@ ARCH=
 # Normalise host CPU name and set ARCH.
 # Note that this case should only have supported host CPUs, not guests.
 case "$cpu" in
-  ppc|ppc64|s390|s390x|sparc64|x32)
+  ppc|ppc64|s390|s390x|sparc64|x32|riscv32|riscv64)
 cpu="$cpu"
 supported_cpu="yes"
 eval "cross_cc_${cpu}=\$host_cc"
@@ -6937,6 +6943,8 @@ elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
 elif test "$ARCH" = "ppc64" ; then
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
+elif test "$ARCH" = "riscv32" -o "$ARCH" = "riscv64" ; then
+  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/riscv $QEMU_INCLUDES"
 else
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
 fi
@@ -7433,7 +7441,7 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
   ppc*)
 disas_config "PPC"
   ;;
-  riscv)
+  riscv*)
 disas_config "RISCV"
   ;;
   s390*)
-- 
2.19.1

Re: [Qemu-devel] [PATCH v4 5/5] iotests: add iotest 236 for testing bitmap merge

[Eric Blake]

On 12/18/18 7:52 PM, John Snow wrote:

New interface, new smoke test.
---
  tests/qemu-iotests/236 | 131 
  tests/qemu-iotests/236.out | 198 +
  tests/qemu-iotests/group   |   1 +
  3 files changed, 330 insertions(+)
  create mode 100755 tests/qemu-iotests/236
  create mode 100644 tests/qemu-iotests/236.out




+
+log('')
+log('--- Disabling B & Adding C ---\n')
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapB" }},
+{ "type": "block-dirty-bitmap-add",
+  "data": { "node": "drive0", "name": "bitmapC",
+"granularity": granularity }},
+# Purely extraneous, but test that it works:
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapC" }},
+{ "type": "block-dirty-bitmap-enable",
+  "data": { "node": "drive0", "name": "bitmapC" }},
+])


One other possible addition just before this point:

qmp_log("transaction", indent=2, actions=[
{ "type": "block-dirty-bitmap-disable",
  "data": { "node": "drive0", "name": "bitmapB" }},
{ "type": "block-dirty-bitmap-add",
  "data": { "node": "drive0", "name": "bitmapC",
"granularity": granularity }},
{ "type": "block-dirty-bitmap-remove",
  "data": { "node": "drive0", "name": "bitmapA" }},
{ "type": "abort", "data": {}}
  ])

to check that we properly undo things on an aborted transaction (B 
should still be enabled, C should not exist, and A should not be damaged).



+# A and D should be equivalent.
+# Some formats round the size of the disk, so don't print the checksums.
+check_a = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+ node="drive0", name="bitmapA")['return']['sha256']
+check_b = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+ node="drive0", name="bitmapD")['return']['sha256']
+assert(check_a == check_b)


Image agnostic also means that you avoid an 32- vs. 64-bit platform 
discrepancies (we've had issues in the past where the sum is different 
for some image sizes, because the bitmap is always in terms of 'long's, 
but there is one less 'long' in a 32-bit bitmap for the disk size). 
Makes sense.


Also, I don't see any tests of block-dirty-bitmap-remove - this would be 
a good point in the test to insert a final cleanup.




+++ b/tests/qemu-iotests/group
@@ -233,3 +233,4 @@
  233 auto quick
  234 auto quick migration
  235 auto quick
+236 auto quick
\ No newline at end of file


Umm - what's that still doing here?


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

Re: [Qemu-devel] Booting Raspbian on RPi emulation

[Ben Hekster via Qemu-devel]

Checked out tag patchew/20181201123056.432-1-peter.mayd...@linaro.org from 
https://github.com/patchew-project/qemu and verified the crash no longer 
occurs.  It does indeed appear that the crash previously happened right around 
the time where I can now see the window being resized.

I've attached a screenshot here for you; not sure if this will make it through 
to the mailing list...

Ben

> On Dec 19, 2018, at 10:43, Peter Maydell  wrote:
> 
> On Wed, 19 Dec 2018 at 17:46, Ben Hekster  wrote:
>> probably a manifestation of https://bugs.launchpad.net/qemu/+bug/1802684
> 
> Yep. If you could test the patchset at
> http://patchew.org/QEMU/20181201123056.432-1-peter.mayd...@linaro.org/
> and confirm that it fixes the problem that would be great.
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v2 22/23] dias: Add RISC-V support

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Reviewed-by: Richard Henderson 
---
 disas.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/disas.c b/disas.c
index f9c517b358..d9aa713a40 100644
--- a/disas.c
+++ b/disas.c
@@ -522,8 +522,14 @@ void disas(FILE *out, void *code, unsigned long size)
 # ifdef _ARCH_PPC64
 s.info.cap_mode = CS_MODE_64;
 # endif
-#elif defined(__riscv__)
-print_insn = print_insn_riscv;
+#elif defined(__riscv) && defined(CONFIG_RISCV_DIS)
+#if defined(_ILP32) || (__riscv_xlen == 32)
+print_insn = print_insn_riscv32;
+#elif defined(_LP64)
+print_insn = print_insn_riscv64;
+#else
+#error unsupported RISC-V ABI
+#endif
 #elif defined(__aarch64__) && defined(CONFIG_ARM_A64_DIS)
 print_insn = print_insn_arm_a64;
 s.info.cap_arch = CS_ARCH_ARM64;
-- 
2.19.1

[Qemu-devel] [PATCH v2 14/23] riscv: tcg-target: Add the add2 and sub2 instructions

[Alistair Francis]

Signed-off-by: Alistair Francis 
Reviewed-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 55 ++
 1 file changed, 55 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 65718df7ad..5da850b957 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -695,3 +695,58 @@ static bool tcg_out_sti(TCGContext *s, TCGType type, 
TCGArg val,
 }
 return false;
 }
+
+static void tcg_out_addsub2(TCGContext *s,
+TCGReg rl, TCGReg rh,
+TCGReg al, TCGReg ah,
+TCGArg bl, TCGArg bh,
+bool cbl, bool cbh, bool is_sub, bool is32bit)
+{
+const RISCVInsn opc_add = is32bit ? OPC_ADDW : OPC_ADD;
+const RISCVInsn opc_addi = is32bit ? OPC_ADDIW : OPC_ADDI;
+const RISCVInsn opc_sub = is32bit ? OPC_SUBW : OPC_SUB;
+TCGReg th = TCG_REG_TMP1;
+
+/* If we have a negative constant such that negating it would
+   make the high part zero, we can (usually) eliminate one insn.  */
+if (cbl && cbh && bh == -1 && bl != 0) {
+bl = -bl;
+bh = 0;
+is_sub = !is_sub;
+}
+
+/* By operating on the high part first, we get to use the final
+   carry operation to move back from the temporary.  */
+if (!cbh) {
+tcg_out_opc_reg(s, (is_sub ? opc_sub : opc_add), th, ah, bh);
+} else if (bh != 0 || ah == rl) {
+tcg_out_opc_imm(s, opc_addi, th, ah, (is_sub ? -bh : bh));
+} else {
+th = ah;
+}
+
+/* Note that tcg optimization should eliminate the bl == 0 case.  */
+if (is_sub) {
+if (cbl) {
+tcg_out_opc_imm(s, OPC_SLTIU, TCG_REG_TMP0, al, bl);
+tcg_out_opc_imm(s, opc_addi, rl, al, -bl);
+} else {
+tcg_out_opc_reg(s, OPC_SLTU, TCG_REG_TMP0, al, bl);
+tcg_out_opc_reg(s, opc_sub, rl, al, bl);
+}
+tcg_out_opc_reg(s, opc_sub, rh, th, TCG_REG_TMP0);
+} else {
+if (cbl) {
+tcg_out_opc_imm(s, opc_addi, rl, al, bl);
+tcg_out_opc_imm(s, OPC_SLTIU, TCG_REG_TMP0, rl, bl);
+} else if (rl == al && rl == bl) {
+tcg_out_opc_imm(s, OPC_SLTI, TCG_REG_TMP0, al, 0);
+tcg_out_opc_reg(s, opc_addi, rl, al, bl);
+} else {
+tcg_out_opc_reg(s, opc_add, rl, al, bl);
+tcg_out_opc_reg(s, OPC_SLTU, TCG_REG_TMP0,
+rl, (rl == bl ? al : bl));
+}
+tcg_out_opc_reg(s, opc_add, rh, th, TCG_REG_TMP0);
+}
+}
-- 
2.19.1

[Qemu-devel] [PATCH v2 18/23] riscv: tcg-target: Add the out op decoder

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Signed-off-by: Richard Henderson 
Reviewed-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 493 +
 1 file changed, 493 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 154315787c..bab3d1e6d2 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -1309,3 +1309,496 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
*args, bool is_64)
 tcg_out_qemu_st_direct(s, data_regl, data_regh, base, opc);
 #endif
 }
+
+static tcg_insn_unit *tb_ret_addr;
+
+static void tcg_out_op(TCGContext *s, TCGOpcode opc,
+   const TCGArg *args, const int *const_args)
+{
+TCGArg a0 = args[0];
+TCGArg a1 = args[1];
+TCGArg a2 = args[2];
+int c2 = const_args[2];
+
+switch (opc) {
+case INDEX_op_exit_tb:
+/* Reuse the zeroing that exists for goto_ptr.  */
+if (a0 == 0) {
+tcg_out_call_int(s, s->code_gen_epilogue, true);
+} else {
+tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_A0, a0);
+tcg_out_call_int(s, tb_ret_addr, true);
+}
+break;
+
+case INDEX_op_goto_tb:
+assert(s->tb_jmp_insn_offset == 0);
+/* indirect jump method */
+tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
+   (uintptr_t)(s->tb_jmp_target_addr + a0));
+tcg_out_opc_imm(s, OPC_JALR, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+set_jmp_reset_offset(s, a0);
+break;
+
+case INDEX_op_goto_ptr:
+tcg_out_opc_imm(s, OPC_JALR, TCG_REG_ZERO, a0, 0);
+break;
+
+case INDEX_op_br:
+tcg_out_reloc(s, s->code_ptr, R_RISCV_JAL, arg_label(a0), 0);
+tcg_out_opc_jump(s, OPC_JAL, TCG_REG_ZERO, 0);
+break;
+
+case INDEX_op_ld8u_i32:
+case INDEX_op_ld8u_i64:
+tcg_out_ldst(s, OPC_LBU, a0, a1, a2);
+break;
+case INDEX_op_ld8s_i32:
+case INDEX_op_ld8s_i64:
+tcg_out_ldst(s, OPC_LB, a0, a1, a2);
+break;
+case INDEX_op_ld16u_i32:
+case INDEX_op_ld16u_i64:
+tcg_out_ldst(s, OPC_LHU, a0, a1, a2);
+break;
+case INDEX_op_ld16s_i32:
+case INDEX_op_ld16s_i64:
+tcg_out_ldst(s, OPC_LH, a0, a1, a2);
+break;
+case INDEX_op_ld32u_i64:
+tcg_out_ldst(s, OPC_LWU, a0, a1, a2);
+break;
+case INDEX_op_ld_i32:
+case INDEX_op_ld32s_i64:
+tcg_out_ldst(s, OPC_LW, a0, a1, a2);
+break;
+case INDEX_op_ld_i64:
+tcg_out_ldst(s, OPC_LD, a0, a1, a2);
+break;
+
+case INDEX_op_st8_i32:
+case INDEX_op_st8_i64:
+tcg_out_ldst(s, OPC_SB, a0, a1, a2);
+break;
+case INDEX_op_st16_i32:
+case INDEX_op_st16_i64:
+tcg_out_ldst(s, OPC_SH, a0, a1, a2);
+break;
+case INDEX_op_st_i32:
+case INDEX_op_st32_i64:
+tcg_out_ldst(s, OPC_SW, a0, a1, a2);
+break;
+case INDEX_op_st_i64:
+tcg_out_ldst(s, OPC_SD, a0, a1, a2);
+break;
+
+case INDEX_op_add_i32:
+if (c2) {
+tcg_out_opc_imm(s, OPC_ADDIW, a0, a1, a2);
+} else {
+tcg_out_opc_reg(s, OPC_ADDW, a0, a1, a2);
+}
+break;
+case INDEX_op_add_i64:
+if (c2) {
+tcg_out_opc_imm(s, OPC_ADDI, a0, a1, a2);
+} else {
+tcg_out_opc_reg(s, OPC_ADD, a0, a1, a2);
+}
+break;
+
+case INDEX_op_sub_i32:
+if (c2) {
+tcg_out_opc_imm(s, OPC_ADDIW, a0, a1, -a2);
+} else {
+tcg_out_opc_reg(s, OPC_SUBW, a0, a1, a2);
+}
+break;
+case INDEX_op_sub_i64:
+if (c2) {
+tcg_out_opc_imm(s, OPC_ADDI, a0, a1, -a2);
+} else {
+tcg_out_opc_reg(s, OPC_SUB, a0, a1, a2);
+}
+break;
+
+case INDEX_op_and_i32:
+case INDEX_op_and_i64:
+if (c2) {
+tcg_out_opc_imm(s, OPC_ANDI, a0, a1, a2);
+} else {
+tcg_out_opc_reg(s, OPC_AND, a0, a1, a2);
+}
+break;
+
+case INDEX_op_or_i32:
+case INDEX_op_or_i64:
+if (c2) {
+tcg_out_opc_imm(s, OPC_ORI, a0, a1, a2);
+} else {
+tcg_out_opc_reg(s, OPC_OR, a0, a1, a2);
+}
+break;
+
+case INDEX_op_xor_i32:
+case INDEX_op_xor_i64:
+if (c2) {
+tcg_out_opc_imm(s, OPC_XORI, a0, a1, a2);
+} else {
+tcg_out_opc_reg(s, OPC_XOR, a0, a1, a2);
+}
+break;
+
+case INDEX_op_not_i32:
+case INDEX_op_not_i64:
+tcg_out_opc_imm(s, OPC_XORI, a0, a1, -1);
+break;
+
+case INDEX_op_neg_i32:
+tcg_out_opc_reg(s, OPC_SUBW, a0, TCG_REG_ZERO, a1);
+break;
+case INDEX_op_neg_i64:
+tcg_out_opc_reg(s, OPC_SUB, a0, TCG_REG_ZERO, a1);
+break;
+
+case INDEX_op_mul_i32:
+tcg_out_opc_reg(s, OPC_MULW, a0, a1

[Qemu-devel] [PATCH v2 11/23] riscv: tcg-target: Add the mov and movi instruction

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Signed-off-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 86 ++
 1 file changed, 86 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index a26744052f..01b4443d6d 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -510,3 +510,89 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
 tcg_abort();
 }
 }
+
+/*
+ * TCG intrinsics
+ */
+
+static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
+{
+if (ret == arg) {
+return;
+}
+switch (type) {
+case TCG_TYPE_I32:
+case TCG_TYPE_I64:
+tcg_out_opc_imm(s, OPC_ADDI, ret, arg, 0);
+break;
+default:
+g_assert_not_reached();
+}
+}
+
+static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd,
+ tcg_target_long val)
+{
+tcg_target_long lo, hi, tmp;
+int shift, ret;
+
+if (TCG_TARGET_REG_BITS == 64 && type == TCG_TYPE_I32) {
+val = (int32_t)val;
+}
+
+lo = sextreg(val, 0, 12);
+if (val == lo) {
+tcg_out_opc_imm(s, OPC_ADDI, rd, TCG_REG_ZERO, lo);
+return;
+}
+
+hi = val - lo;
+if (TCG_TARGET_REG_BITS == 32 || val == (int32_t)val) {
+tcg_out_opc_upper(s, OPC_LUI, rd, hi);
+if (lo != 0) {
+tcg_out_opc_imm(s, OPC_ADDIW, rd, rd, lo);
+}
+return;
+}
+
+/* We can only be here if TCG_TARGET_REG_BITS != 32 */
+tmp = tcg_pcrel_diff(s, (void *)val);
+if (tmp == (int32_t)tmp) {
+tcg_out_opc_upper(s, OPC_AUIPC, rd, 0);
+tcg_out_opc_imm(s, OPC_ADDI, rd, rd, 0);
+ret = reloc_call(s->code_ptr - 2, (tcg_insn_unit *)val);
+tcg_debug_assert(ret == true);
+return;
+}
+
+/* Look for a single 20-bit section.  */
+shift = ctz64(val);
+tmp = val >> shift;
+if (tmp == sextreg(tmp, 0, 20)) {
+tcg_out_opc_upper(s, OPC_LUI, rd, tmp << 12);
+if (shift > 12) {
+tcg_out_opc_imm(s, OPC_SLLI, rd, rd, shift - 12);
+} else {
+tcg_out_opc_imm(s, OPC_SRAI, rd, rd, 12 - shift);
+}
+return;
+}
+
+/* Look for a few high zero bits, with lots of bits set in the middle.  */
+shift = clz64(val);
+tmp = val << shift;
+if (tmp == sextreg(tmp, 12, 20) << 12) {
+tcg_out_opc_upper(s, OPC_LUI, rd, tmp);
+tcg_out_opc_imm(s, OPC_SRLI, rd, rd, shift);
+return;
+} else if (tmp == sextreg(tmp, 0, 12)) {
+tcg_out_opc_imm(s, OPC_ADDI, rd, TCG_REG_ZERO, tmp);
+tcg_out_opc_imm(s, OPC_SRLI, rd, rd, shift);
+return;
+}
+
+/* Drop into the constant pool.  */
+new_pool_label(s, val, R_RISCV_CALL, s->code_ptr, 0);
+tcg_out_opc_upper(s, OPC_AUIPC, rd, 0);
+tcg_out_opc_imm(s, OPC_LD, rd, rd, 0);
+}
-- 
2.19.1

[Qemu-devel] [PATCH v2 21/23] tcg: Add RISC-V cpu signal handler

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
---
 accel/tcg/user-exec.c | 75 +++
 1 file changed, 75 insertions(+)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index cd75829cf2..941295ea49 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -571,6 +571,81 @@ int cpu_signal_handler(int host_signum, void *pinfo,
 return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
 }
 
+#elif defined(__riscv)
+
+int cpu_signal_handler(int host_signum, void *pinfo,
+   void *puc)
+{
+siginfo_t *info = pinfo;
+ucontext_t *uc = puc;
+greg_t pc = uc->uc_mcontext.__gregs[REG_PC];
+uint32_t insn = *(uint32_t *)pc;
+int is_write = 0;
+
+/* Detect store by reading the instruction at the program
+   counter. Note: we currently only generate 32-bit
+   instructions so we thus only detect 32-bit stores */
+switch (((insn >> 0) & 0b11)) {
+case 3:
+switch (((insn >> 2) & 0b1)) {
+case 8:
+switch (((insn >> 12) & 0b111)) {
+case 0: /* sb */
+case 1: /* sh */
+case 2: /* sw */
+case 3: /* sd */
+case 4: /* sq */
+is_write = 1;
+break;
+default:
+break;
+}
+break;
+case 9:
+switch (((insn >> 12) & 0b111)) {
+case 2: /* fsw */
+case 3: /* fsd */
+case 4: /* fsq */
+is_write = 1;
+break;
+default:
+break;
+}
+break;
+default:
+break;
+}
+}
+
+/* Check for compressed instructions */
+switch (((insn >> 13) & 0b111)) {
+case 7:
+switch (insn & 0b11) {
+case 0: /*c.sd */
+case 2: /* c.sdsp */
+is_write = 1;
+break;
+default:
+break;
+}
+break;
+case 6:
+switch (insn & 0b11) {
+case 0: /* c.sw */
+case 3: /* c.swsp */
+is_write = 1;
+break;
+default:
+break;
+}
+break;
+default:
+break;
+}
+
+return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
+}
+
 #else
 
 #error host CPU specific signal handler needed
-- 
2.19.1

[Qemu-devel] [PATCH v2 10/23] riscv: tcg-target: Add the relocation functions

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Signed-off-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 88 ++
 1 file changed, 88 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index d198cfd5f7..a26744052f 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -422,3 +422,91 @@ static void tcg_out_nop_fill(tcg_insn_unit *p, int count)
 p[i] = encode_i(OPC_ADDI, TCG_REG_ZERO, TCG_REG_ZERO, 0);
 }
 }
+
+/*
+ * Relocations
+ */
+
+static bool reloc_sbimm12(tcg_insn_unit *code_ptr, tcg_insn_unit *target)
+{
+intptr_t offset = (intptr_t)target - (intptr_t)code_ptr;
+
+if (offset == sextreg(offset, 1, 12) << 1) {
+code_ptr[0] |= encode_sbimm12(offset);
+return true;
+}
+
+return false;
+}
+
+static bool reloc_jimm20(tcg_insn_unit *code_ptr, tcg_insn_unit *target)
+{
+intptr_t offset = (intptr_t)target - (intptr_t)code_ptr;
+
+if (offset == sextreg(offset, 1, 20) << 1) {
+code_ptr[0] |= encode_ujimm20(offset);
+return true;
+}
+
+return false;
+}
+
+static bool reloc_call(tcg_insn_unit *code_ptr, tcg_insn_unit *target)
+{
+intptr_t offset = (intptr_t)target - (intptr_t)code_ptr;
+int32_t lo = sextreg(offset, 0, 12);
+int32_t hi = offset - lo;
+
+if (offset == hi + lo) {
+code_ptr[0] |= encode_uimm20(hi);
+code_ptr[1] |= encode_imm12(lo);
+return true;
+}
+
+return false;
+}
+
+static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
+intptr_t value, intptr_t addend)
+{
+uint32_t insn = *code_ptr;
+intptr_t diff;
+bool short_jmp;
+
+tcg_debug_assert(addend == 0);
+
+switch (type) {
+case R_RISCV_BRANCH:
+diff = value - (uintptr_t)code_ptr;
+short_jmp = diff == sextreg(diff, 0, 12);
+if (short_jmp) {
+return reloc_sbimm12(code_ptr, (tcg_insn_unit *)value);
+} else {
+/* Invert the condition */
+insn = insn ^ (1 << 12);
+/* Clear the offset */
+insn &= 0x01fff07f;
+/* Set the offset to the PC + 8 */
+insn |= encode_sbimm12(8);
+
+/* Move forward */
+code_ptr[0] = insn;
+
+/* Overwrite the NOP with jal x0,value */
+diff = value - (uintptr_t)(code_ptr + 1);
+insn = encode_uj(OPC_JAL, TCG_REG_ZERO, diff);
+code_ptr[1] = insn;
+
+return true;
+}
+break;
+case R_RISCV_JAL:
+return reloc_jimm20(code_ptr, (tcg_insn_unit *)value);
+break;
+case R_RISCV_CALL:
+return reloc_call(code_ptr, (tcg_insn_unit *)value);
+break;
+default:
+tcg_abort();
+}
+}
-- 
2.19.1

[Qemu-devel] [PATCH v2 08/23] riscv: tcg-target: Add the immediate encoders

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Signed-off-by: Richard Henderson 
Reviewed-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 90 ++
 1 file changed, 90 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index f853d01803..08838027cd 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -284,3 +284,93 @@ typedef enum {
 
 OPC_FENCE = 0x000f,
 } RISCVInsn;
+
+/*
+ * RISC-V immediate and instruction encoders (excludes 16-bit RVC)
+ */
+
+/* Type-R */
+
+static int32_t encode_r(RISCVInsn opc, TCGReg rd, TCGReg rs1, TCGReg rs2)
+{
+return opc | (rd & 0x1f) << 7 | (rs1 & 0x1f) << 15 | (rs2 & 0x1f) << 20;
+}
+
+/* Type-I */
+
+static int32_t encode_imm12(uint32_t imm)
+{
+return (imm & 0xfff) << 20;
+}
+
+static int32_t encode_i(RISCVInsn opc, TCGReg rd, TCGReg rs1, uint32_t imm)
+{
+return opc | (rd & 0x1f) << 7 | (rs1 & 0x1f) << 15 | encode_imm12(imm);
+}
+
+/* Type-S */
+
+static int32_t encode_simm12(uint32_t imm)
+{
+int32_t ret = 0;
+
+ret |= (imm & 0xFE0) << 20;
+ret |= (imm & 0x1F) << 7;
+
+return ret;
+}
+
+static int32_t encode_s(RISCVInsn opc, TCGReg rs1, TCGReg rs2, uint32_t imm)
+{
+return opc | (rs1 & 0x1f) << 15 | (rs2 & 0x1f) << 20 | encode_simm12(imm);
+}
+
+/* Type-SB */
+
+static int32_t encode_sbimm12(uint32_t imm)
+{
+int32_t ret = 0;
+
+ret |= (imm & 0x1000) << 19;
+ret |= (imm & 0x7e0) << 20;
+ret |= (imm & 0x1e) << 7;
+ret |= (imm & 0x800) >> 4;
+
+return ret;
+}
+
+static int32_t encode_sb(RISCVInsn opc, TCGReg rs1, TCGReg rs2, uint32_t imm)
+{
+return opc | (rs1 & 0x1f) << 15 | (rs2 & 0x1f) << 20 | encode_sbimm12(imm);
+}
+
+/* Type-U */
+
+static int32_t encode_uimm20(uint32_t imm)
+{
+return imm & 0xf000;
+}
+
+static int32_t encode_u(RISCVInsn opc, TCGReg rd, uint32_t imm)
+{
+return opc | (rd & 0x1f) << 7 | encode_uimm20(imm);
+}
+
+/* Type-UJ */
+
+static int32_t encode_ujimm20(uint32_t imm)
+{
+int32_t ret = 0;
+
+ret |= (imm & 0x0007fe) << (21 - 1);
+ret |= (imm & 0x000800) << (20 - 11);
+ret |= (imm & 0x0ff000) << (12 - 12);
+ret |= (imm & 0x10) << (31 - 20);
+
+return ret;
+}
+
+static int32_t encode_uj(RISCVInsn opc, TCGReg rd, uint32_t imm)
+{
+return opc | (rd & 0x1f) << 7 | encode_ujimm20(imm);
+}
-- 
2.19.1

[Qemu-devel] [PATCH v2 16/23] riscv: tcg-target: Add slowpath load and store instructions

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Signed-off-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 256 +
 1 file changed, 256 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index ecc76c9ef8..7216bad086 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -895,3 +895,259 @@ static void tcg_out_call(TCGContext *s, tcg_insn_unit 
*arg)
 {
 tcg_out_call_int(s, arg, false);
 }
+
+static void tcg_out_mb(TCGContext *s, TCGArg a0)
+{
+tcg_insn_unit insn = OPC_FENCE;
+
+if (a0 & TCG_MO_LD_LD) {
+insn |= 0x0220;
+}
+if (a0 & TCG_MO_ST_LD) {
+insn |= 0x0120;
+}
+if (a0 & TCG_MO_LD_ST) {
+insn |= 0x0210;
+}
+if (a0 & TCG_MO_ST_ST) {
+insn |= 0x0220;
+}
+tcg_out32(s, insn);
+}
+
+/*
+ * Load/store and TLB
+ */
+
+#if defined(CONFIG_SOFTMMU)
+#include "tcg-ldst.inc.c"
+
+/* helper signature: helper_ret_ld_mmu(CPUState *env, target_ulong addr,
+ * TCGMemOpIdx oi, uintptr_t ra)
+ */
+static void * const qemu_ld_helpers[16] = {
+[MO_UB]   = helper_ret_ldub_mmu,
+[MO_SB]   = helper_ret_ldsb_mmu,
+[MO_LEUW] = helper_le_lduw_mmu,
+[MO_LESW] = helper_le_ldsw_mmu,
+[MO_LEUL] = helper_le_ldul_mmu,
+#if TCG_TARGET_REG_BITS == 64
+[MO_LESL] = helper_le_ldsl_mmu,
+#endif
+[MO_LEQ]  = helper_le_ldq_mmu,
+[MO_BEUW] = helper_be_lduw_mmu,
+[MO_BESW] = helper_be_ldsw_mmu,
+[MO_BEUL] = helper_be_ldul_mmu,
+#if TCG_TARGET_REG_BITS == 64
+[MO_BESL] = helper_be_ldsl_mmu,
+#endif
+[MO_BEQ]  = helper_be_ldq_mmu,
+};
+
+/* helper signature: helper_ret_st_mmu(CPUState *env, target_ulong addr,
+ * uintxx_t val, TCGMemOpIdx oi,
+ * uintptr_t ra)
+ */
+static void * const qemu_st_helpers[16] = {
+[MO_UB]   = helper_ret_stb_mmu,
+[MO_LEUW] = helper_le_stw_mmu,
+[MO_LEUL] = helper_le_stl_mmu,
+[MO_LEQ]  = helper_le_stq_mmu,
+[MO_BEUW] = helper_be_stw_mmu,
+[MO_BEUL] = helper_be_stl_mmu,
+[MO_BEQ]  = helper_be_stq_mmu,
+};
+
+static void tcg_out_tlb_load(TCGContext *s, TCGReg addrl,
+ TCGReg addrh, TCGMemOpIdx oi,
+ tcg_insn_unit **label_ptr, bool is_load)
+{
+TCGMemOp opc = get_memop(oi);
+unsigned s_bits = opc & MO_SIZE;
+unsigned a_bits = get_alignment_bits(opc);
+target_ulong mask;
+int mem_index = get_mmuidx(oi);
+int cmp_off
+= (is_load
+   ? offsetof(CPUArchState, tlb_table[mem_index][0].addr_read)
+   : offsetof(CPUArchState, tlb_table[mem_index][0].addr_write));
+int add_off = offsetof(CPUArchState, tlb_table[mem_index][0].addend);
+RISCVInsn load_cmp_op = (TARGET_LONG_BITS == 64 ? OPC_LD :
+ TCG_TARGET_REG_BITS == 64 ? OPC_LWU : OPC_LW);
+RISCVInsn load_add_op = TCG_TARGET_REG_BITS == 64 ? OPC_LD : OPC_LW;
+TCGReg base = TCG_AREG0;
+
+/* We don't support oversize guests */
+if (TCG_TARGET_REG_BITS < TARGET_LONG_BITS) {
+g_assert_not_reached();
+}
+
+/* We don't support unaligned accesses. */
+if (a_bits < s_bits) {
+a_bits = s_bits;
+}
+mask = (target_ulong)TARGET_PAGE_MASK | ((1 << a_bits) - 1);
+
+
+/* Compensate for very large offsets.  */
+if (add_off >= 0x1000) {
+int adj;
+base = TCG_REG_TMP2;
+if (cmp_off <= 2 * 0xfff) {
+adj = 0xfff;
+tcg_out_opc_imm(s, OPC_ADDI, base, TCG_AREG0, adj);
+} else {
+adj = cmp_off - sextreg(cmp_off, 0, 12);
+tcg_debug_assert(add_off - adj >= -0x1000
+ && add_off - adj < 0x1000);
+
+tcg_out_opc_upper(s, OPC_LUI, base, adj);
+tcg_out_opc_reg(s, OPC_ADD, base, base, TCG_AREG0);
+}
+add_off -= adj;
+cmp_off -= adj;
+}
+
+/* Extract the page index.  */
+if (CPU_TLB_BITS + CPU_TLB_ENTRY_BITS < 12) {
+tcg_out_opc_imm(s, OPC_SRLI, TCG_REG_TMP0, addrl,
+TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS);
+tcg_out_opc_imm(s, OPC_ANDI, TCG_REG_TMP0, TCG_REG_TMP0,
+MAKE_64BIT_MASK(CPU_TLB_ENTRY_BITS, CPU_TLB_BITS));
+} else if (TARGET_PAGE_BITS >= 12) {
+tcg_out_opc_upper(s, OPC_LUI, TCG_REG_TMP0,
+  MAKE_64BIT_MASK(TARGET_PAGE_BITS, CPU_TLB_BITS));
+tcg_out_opc_reg(s, OPC_AND, TCG_REG_TMP0, TCG_REG_TMP0, addrl);
+tcg_out_opc_imm(s, OPC_SRLI, TCG_REG_TMP0, TCG_REG_TMP0,
+CPU_TLB_BITS - CPU_TLB_ENTRY_BITS);
+} else {
+tcg_out_opc_imm(s, OPC_SRLI, TCG_REG_TMP0, addrl, TARGET_PAGE_BITS);
+tcg_out_opc_imm(s, OPC_ANDI, TCG_REG_TMP0, TCG_REG_TMP0,
+MAKE_64BIT_MASK(0, CPU_TLB_BITS));
+tcg

[Qemu-devel] [PATCH v2 19/23] riscv: tcg-target: Add the prologue generation and register the JIT

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Reviewed-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 111 +
 1 file changed, 111 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index bab3d1e6d2..bc1a2bc4ab 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -1802,3 +1802,114 @@ static const TCGTargetOpDef 
*tcg_target_op_def(TCGOpcode op)
 return NULL;
 }
 }
+
+static const int tcg_target_callee_save_regs[] = {
+TCG_REG_S0,   /* used for the global env (TCG_AREG0) */
+TCG_REG_S1,
+TCG_REG_S2,
+TCG_REG_S3,
+TCG_REG_S4,
+TCG_REG_S5,
+TCG_REG_S6,
+TCG_REG_S7,
+TCG_REG_S8,
+TCG_REG_S9,
+TCG_REG_S10,
+TCG_REG_S11,
+TCG_REG_RA,   /* should be last for ABI compliance */
+};
+
+/* Stack frame parameters.  */
+#define REG_SIZE   (TCG_TARGET_REG_BITS / 8)
+#define SAVE_SIZE  ((int)ARRAY_SIZE(tcg_target_callee_save_regs) * REG_SIZE)
+#define TEMP_SIZE  (CPU_TEMP_BUF_NLONGS * (int)sizeof(long))
+#define FRAME_SIZE ((TCG_STATIC_CALL_ARGS_SIZE + TEMP_SIZE + SAVE_SIZE \
+ + TCG_TARGET_STACK_ALIGN - 1) \
+& -TCG_TARGET_STACK_ALIGN)
+#define SAVE_OFS   (TCG_STATIC_CALL_ARGS_SIZE + TEMP_SIZE)
+
+/* We're expecting to be able to use an immediate for frame allocation.  */
+QEMU_BUILD_BUG_ON(FRAME_SIZE > 0x7ff);
+
+/* Generate global QEMU prologue and epilogue code */
+static void tcg_target_qemu_prologue(TCGContext *s)
+{
+int i;
+
+tcg_set_frame(s, TCG_REG_SP, TCG_STATIC_CALL_ARGS_SIZE, TEMP_SIZE);
+
+/* TB prologue */
+tcg_out_opc_imm(s, OPC_ADDI, TCG_REG_SP, TCG_REG_SP, -FRAME_SIZE);
+for (i = 0; i < ARRAY_SIZE(tcg_target_callee_save_regs); i++) {
+tcg_out_st(s, TCG_TYPE_REG, tcg_target_callee_save_regs[i],
+   TCG_REG_SP, SAVE_OFS + i * REG_SIZE);
+}
+
+#if !defined(CONFIG_SOFTMMU)
+tcg_out_movi(s, TCG_TYPE_PTR, TCG_GUEST_BASE_REG, guest_base);
+tcg_regset_set_reg(s->reserved_regs, TCG_GUEST_BASE_REG);
+#endif
+
+/* Call generated code */
+tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
+tcg_out_opc_imm(s, OPC_JALR, TCG_REG_ZERO, tcg_target_call_iarg_regs[1], 
0);
+
+/* Return path for goto_ptr. Set return value to 0 */
+s->code_gen_epilogue = s->code_ptr;
+tcg_out_mov(s, TCG_TYPE_REG, TCG_REG_A0, TCG_REG_ZERO);
+
+/* TB epilogue */
+tb_ret_addr = s->code_ptr;
+for (i = 0; i < ARRAY_SIZE(tcg_target_callee_save_regs); i++) {
+tcg_out_ld(s, TCG_TYPE_REG, tcg_target_callee_save_regs[i],
+   TCG_REG_SP, SAVE_OFS + i * REG_SIZE);
+}
+
+tcg_out_opc_imm(s, OPC_ADDI, TCG_REG_SP, TCG_REG_SP, FRAME_SIZE);
+tcg_out_opc_imm(s, OPC_JALR, TCG_REG_ZERO, TCG_REG_RA, 0);
+}
+
+typedef struct {
+DebugFrameHeader h;
+uint8_t fde_def_cfa[4];
+uint8_t fde_reg_ofs[ARRAY_SIZE(tcg_target_callee_save_regs) * 2];
+} DebugFrame;
+
+#define ELF_HOST_MACHINE EM_RISCV
+
+static const DebugFrame debug_frame = {
+.h.cie.len = sizeof(DebugFrameCIE) - 4, /* length after .len member */
+.h.cie.id = -1,
+.h.cie.version = 1,
+.h.cie.code_align = 1,
+.h.cie.data_align = -(TCG_TARGET_REG_BITS / 8) & 0x7f, /* sleb128 */
+.h.cie.return_column = TCG_REG_RA,
+
+/* Total FDE size does not include the "len" member.  */
+.h.fde.len = sizeof(DebugFrame) - offsetof(DebugFrame, h.fde.cie_offset),
+
+.fde_def_cfa = {
+12, TCG_REG_SP, /* DW_CFA_def_cfa sp, ... */
+(FRAME_SIZE & 0x7f) | 0x80, /* ... uleb128 FRAME_SIZE */
+(FRAME_SIZE >> 7)
+},
+.fde_reg_ofs = {
+0x80 + 9,  12,  /* DW_CFA_offset, s1,  -96 */
+0x80 + 18, 11,  /* DW_CFA_offset, s2,  -88 */
+0x80 + 19, 10,  /* DW_CFA_offset, s3,  -80 */
+0x80 + 20, 9,   /* DW_CFA_offset, s4,  -72 */
+0x80 + 21, 8,   /* DW_CFA_offset, s5,  -64 */
+0x80 + 22, 7,   /* DW_CFA_offset, s6,  -56 */
+0x80 + 23, 6,   /* DW_CFA_offset, s7,  -48 */
+0x80 + 24, 5,   /* DW_CFA_offset, s8,  -40 */
+0x80 + 25, 4,   /* DW_CFA_offset, s9,  -32 */
+0x80 + 26, 3,   /* DW_CFA_offset, s10, -24 */
+0x80 + 27, 2,   /* DW_CFA_offset, s11, -16 */
+0x80 + 1 , 1,   /* DW_CFA_offset, ra,  -8 */
+}
+};
+
+void tcg_register_jit(void *buf, size_t buf_size)
+{
+tcg_register_jit_int(buf, buf_size, &debug_frame, sizeof(debug_frame));
+}
-- 
2.19.1

[Qemu-devel] [PATCH v2 09/23] riscv: tcg-target: Add the instruction emitters

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Reviewed-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 48 ++
 1 file changed, 48 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 08838027cd..d198cfd5f7 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -374,3 +374,51 @@ static int32_t encode_uj(RISCVInsn opc, TCGReg rd, 
uint32_t imm)
 {
 return opc | (rd & 0x1f) << 7 | encode_ujimm20(imm);
 }
+
+/*
+ * RISC-V instruction emitters
+ */
+
+static void tcg_out_opc_reg(TCGContext *s, RISCVInsn opc,
+TCGReg rd, TCGReg rs1, TCGReg rs2)
+{
+tcg_out32(s, encode_r(opc, rd, rs1, rs2));
+}
+
+static void tcg_out_opc_imm(TCGContext *s, RISCVInsn opc,
+TCGReg rd, TCGReg rs1, TCGArg imm)
+{
+tcg_out32(s, encode_i(opc, rd, rs1, imm));
+}
+
+static void tcg_out_opc_store(TCGContext *s, RISCVInsn opc,
+  TCGReg rs1, TCGReg rs2, uint32_t imm)
+{
+tcg_out32(s, encode_s(opc, rs1, rs2, imm));
+}
+
+static void tcg_out_opc_branch(TCGContext *s, RISCVInsn opc,
+   TCGReg rs1, TCGReg rs2, uint32_t imm)
+{
+tcg_out32(s, encode_sb(opc, rs1, rs2, imm));
+}
+
+static void tcg_out_opc_upper(TCGContext *s, RISCVInsn opc,
+  TCGReg rd, uint32_t imm)
+{
+tcg_out32(s, encode_u(opc, rd, imm));
+}
+
+static void tcg_out_opc_jump(TCGContext *s, RISCVInsn opc,
+ TCGReg rd, uint32_t imm)
+{
+tcg_out32(s, encode_uj(opc, rd, imm));
+}
+
+static void tcg_out_nop_fill(tcg_insn_unit *p, int count)
+{
+int i;
+for (i = 0; i < count; ++i) {
+p[i] = encode_i(OPC_ADDI, TCG_REG_ZERO, TCG_REG_ZERO, 0);
+}
+}
-- 
2.19.1

Re: [Qemu-devel] [PATCH v4 1/5] iotests: add qmp recursive sorting function

[Eric Blake]

On 12/19/18 12:57 PM, John Snow wrote:



On 12/19/18 1:52 PM, Eric Blake wrote:

On 12/18/18 7:52 PM, John Snow wrote:

Python before 3.6 does not sort kwargs by default.
If we want to print out pretty-printed QMP objects while
preserving the "exec" > "arguments" ordering, we need a custom sort.


Naive question - why do we need the sorting? Is it so that the output is
deterministic?  Surely it can't be because the ordering otherwise makes
a difference to execution.



Yeah, it's because dicts are unordered and prior to 3.6 they may shuffle
around arbitrarily based on internal hashes.

kwargs in particular are unordered- the order we send over the wire may
or may not reflect the order the test author wrote in their iotest.


Which shouldn't matter to what QMP executes, but MIGHT matter in getting 
reproducible log output.




Therefore, it's a way to get consistent ordering.

Now, we CAN just rely on sort_keys=True to be done with it, however,
this puts arguments before execute, and it's less nice to read -- and
I'd have to change a LOT of test output.


Okay, I'm finally seeing it; the existing code has:

def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
logmsg = '{"execute": "%s", "arguments": %s}' % \
(cmd, json.dumps(kwargs, sort_keys=True))

where our log is outputting a message that resembles a dict where 
"execute": is the first key, and the user's input dict is then sorted 
(the top-most output of {} is unsorted, but the nested ones are sorted, 
and possibly in a different order than the user typed them, but at least 
deterministic). But when you change to the user passing a full QMP 
command (rather than just a dict for the arguments of the QMP command), 
using sort_keys=True will sort everything _including_ putting 
"arguments" before "execute" (which is deterministic but changes log 
output); while using sort_keys=False will not affect output in newer 
python where kwargs remains ordered, but risks nondeterministic output 
on older python.




This way keeps the order you expect to see while maintaining a strict
order for the arguments. I think that's the nicest compromise until we
can stipulate 3.6+ in the year 2034 where kwargs *are* strictly ordered.


But kwargs strict ordering is only at the top-level, not recursive, 
right? That is, even if you can rely on:


foo(b=1, a=2)

providing kwargs where 'b' always outputs before 'a', I don't see how 
that would help:


foo(b={'d':3, 'c':4})

not reorder the keys within 'b' without your recursive ordering.

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

[Qemu-devel] [PATCH v2 01/23] elf.h: Add the RISCV ELF magic numbers

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Reviewed-by: Richard Henderson 
---
 include/elf.h | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/include/elf.h b/include/elf.h
index c151164b63..0ac7911b7b 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -1338,6 +1338,61 @@ typedef struct {
 #define R_IA64_DTPREL64LSB 0xb7/* @dtprel(sym + add), data8 LSB */
 #define R_IA64_LTOFF_DTPREL22  0xba/* @ltoff(@dtprel(s+a)), imm22 */
 
+/* RISC-V relocations.  */
+#define R_RISCV_NONE  0
+#define R_RISCV_321
+#define R_RISCV_642
+#define R_RISCV_RELATIVE  3
+#define R_RISCV_COPY  4
+#define R_RISCV_JUMP_SLOT 5
+#define R_RISCV_TLS_DTPMOD32  6
+#define R_RISCV_TLS_DTPMOD64  7
+#define R_RISCV_TLS_DTPREL32  8
+#define R_RISCV_TLS_DTPREL64  9
+#define R_RISCV_TLS_TPREL32   10
+#define R_RISCV_TLS_TPREL64   11
+#define R_RISCV_BRANCH16
+#define R_RISCV_JAL   17
+#define R_RISCV_CALL  18
+#define R_RISCV_CALL_PLT  19
+#define R_RISCV_GOT_HI20  20
+#define R_RISCV_TLS_GOT_HI20  21
+#define R_RISCV_TLS_GD_HI20   22
+#define R_RISCV_PCREL_HI2023
+#define R_RISCV_PCREL_LO12_I  24
+#define R_RISCV_PCREL_LO12_S  25
+#define R_RISCV_HI20  26
+#define R_RISCV_LO12_I27
+#define R_RISCV_LO12_S28
+#define R_RISCV_TPREL_HI2029
+#define R_RISCV_TPREL_LO12_I  30
+#define R_RISCV_TPREL_LO12_S  31
+#define R_RISCV_TPREL_ADD 32
+#define R_RISCV_ADD8  33
+#define R_RISCV_ADD16 34
+#define R_RISCV_ADD32 35
+#define R_RISCV_ADD64 36
+#define R_RISCV_SUB8  37
+#define R_RISCV_SUB16 38
+#define R_RISCV_SUB32 39
+#define R_RISCV_SUB64 40
+#define R_RISCV_GNU_VTINHERIT 41
+#define R_RISCV_GNU_VTENTRY   42
+#define R_RISCV_ALIGN 43
+#define R_RISCV_RVC_BRANCH44
+#define R_RISCV_RVC_JUMP  45
+#define R_RISCV_RVC_LUI   46
+#define R_RISCV_GPREL_I   47
+#define R_RISCV_GPREL_S   48
+#define R_RISCV_TPREL_I   49
+#define R_RISCV_TPREL_S   50
+#define R_RISCV_RELAX 51
+#define R_RISCV_SUB6  52
+#define R_RISCV_SET6  53
+#define R_RISCV_SET8  54
+#define R_RISCV_SET16 55
+#define R_RISCV_SET32 56
+
 typedef struct elf32_rel {
   Elf32_Addr   r_offset;
   Elf32_Word   r_info;
-- 
2.19.1

[Qemu-devel] [PATCH v2 15/23] riscv: tcg-target: Add branch and jump instructions

[Alistair Francis]

Signed-off-by: Alistair Francis 
Signed-off-by: Michael Clark 
Signed-off-by: Richard Henderson 
---
 tcg/riscv/tcg-target.inc.c | 145 +
 1 file changed, 145 insertions(+)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 5da850b957..ecc76c9ef8 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -750,3 +750,148 @@ static void tcg_out_addsub2(TCGContext *s,
 tcg_out_opc_reg(s, opc_add, rh, th, TCG_REG_TMP0);
 }
 }
+
+static const struct {
+RISCVInsn op;
+bool swap;
+} tcg_brcond_to_riscv[] = {
+[TCG_COND_EQ] =  { OPC_BEQ,  false },
+[TCG_COND_NE] =  { OPC_BNE,  false },
+[TCG_COND_LT] =  { OPC_BLT,  false },
+[TCG_COND_GE] =  { OPC_BGE,  false },
+[TCG_COND_LE] =  { OPC_BGE,  true  },
+[TCG_COND_GT] =  { OPC_BLT,  true  },
+[TCG_COND_LTU] = { OPC_BLTU, false },
+[TCG_COND_GEU] = { OPC_BGEU, false },
+[TCG_COND_LEU] = { OPC_BGEU, true  },
+[TCG_COND_GTU] = { OPC_BLTU, true  }
+};
+
+static void tcg_out_brcond(TCGContext *s, TCGCond cond, TCGReg arg1,
+   TCGReg arg2, TCGLabel *l)
+{
+RISCVInsn op = tcg_brcond_to_riscv[cond].op;
+
+tcg_debug_assert(op != 0);
+
+if (tcg_brcond_to_riscv[cond].swap) {
+TCGReg t = arg1;
+arg1 = arg2;
+arg2 = t;
+}
+
+if (l->has_value) {
+intptr_t diff = tcg_pcrel_diff(s, l->u.value_ptr);
+if (diff == sextreg(diff, 0, 12)) {
+tcg_out_opc_branch(s, op, arg1, arg2, diff);
+} else {
+/* Invert the conditional branch.  */
+tcg_out_opc_branch(s, op ^ (1 << 12), arg1, arg2, 8);
+tcg_out_opc_jump(s, OPC_JAL, TCG_REG_ZERO, diff - 4);
+}
+} else {
+tcg_out_reloc(s, s->code_ptr, R_RISCV_BRANCH, l, 0);
+tcg_out_opc_branch(s, op, arg1, arg2, 0);
+/* NOP to allow patching later */
+tcg_out_opc_imm(s, OPC_ADDI, TCG_REG_ZERO, TCG_REG_ZERO, 0);
+}
+}
+
+static void tcg_out_setcond(TCGContext *s, TCGCond cond, TCGReg ret,
+TCGReg arg1, TCGReg arg2)
+{
+switch (cond) {
+case TCG_COND_EQ:
+tcg_out_opc_reg(s, OPC_SUB, ret, arg1, arg2);
+tcg_out_opc_imm(s, OPC_SLTIU, ret, ret, 1);
+break;
+case TCG_COND_NE:
+tcg_out_opc_reg(s, OPC_SUB, ret, arg1, arg2);
+tcg_out_opc_reg(s, OPC_SLTU, ret, TCG_REG_ZERO, ret);
+break;
+case TCG_COND_LT:
+tcg_out_opc_reg(s, OPC_SLT, ret, arg1, arg2);
+break;
+case TCG_COND_GE:
+tcg_out_opc_reg(s, OPC_SLT, ret, arg1, arg2);
+tcg_out_opc_imm(s, OPC_XORI, ret, ret, 1);
+break;
+case TCG_COND_LE:
+tcg_out_opc_reg(s, OPC_SLT, ret, arg2, arg1);
+tcg_out_opc_imm(s, OPC_XORI, ret, ret, 1);
+break;
+case TCG_COND_GT:
+tcg_out_opc_reg(s, OPC_SLT, ret, arg2, arg1);
+break;
+case TCG_COND_LTU:
+tcg_out_opc_reg(s, OPC_SLTU, ret, arg1, arg2);
+break;
+case TCG_COND_GEU:
+tcg_out_opc_reg(s, OPC_SLTU, ret, arg1, arg2);
+tcg_out_opc_imm(s, OPC_XORI, ret, ret, 1);
+break;
+case TCG_COND_LEU:
+tcg_out_opc_reg(s, OPC_SLTU, ret, arg2, arg1);
+tcg_out_opc_imm(s, OPC_XORI, ret, ret, 1);
+break;
+case TCG_COND_GTU:
+tcg_out_opc_reg(s, OPC_SLTU, ret, arg2, arg1);
+break;
+default:
+ g_assert_not_reached();
+ break;
+ }
+}
+
+static void tcg_out_brcond2(TCGContext *s, TCGCond cond, TCGReg al, TCGReg ah,
+TCGReg bl, TCGReg bh, TCGLabel *l)
+{
+/* todo */
+g_assert_not_reached();
+}
+
+static void tcg_out_setcond2(TCGContext *s, TCGCond cond, TCGReg ret,
+ TCGReg al, TCGReg ah, TCGReg bl, TCGReg bh)
+{
+/* todo */
+g_assert_not_reached();
+}
+
+static inline void tcg_out_goto(TCGContext *s, tcg_insn_unit *target)
+{
+ptrdiff_t offset = tcg_pcrel_diff(s, target);
+tcg_debug_assert(offset == sextreg(offset, 1, 20) << 1);
+tcg_out_opc_jump(s, OPC_JAL, TCG_REG_ZERO, offset);
+}
+
+static void tcg_out_call_int(TCGContext *s, tcg_insn_unit *arg, bool tail)
+{
+TCGReg link = tail ? TCG_REG_ZERO : TCG_REG_RA;
+ptrdiff_t offset = tcg_pcrel_diff(s, arg);
+int ret;
+
+if (offset == sextreg(offset, 1, 20) << 1) {
+/* short jump: -2097150 to 2097152 */
+tcg_out_opc_jump(s, OPC_JAL, link, offset);
+} else if (TCG_TARGET_REG_BITS == 32 ||
+offset == sextreg(offset, 1, 31) << 1) {
+/* long jump: -2147483646 to 2147483648 */
+tcg_out_opc_upper(s, OPC_AUIPC, TCG_REG_TMP0, 0);
+tcg_out_opc_imm(s, OPC_JALR, link, TCG_REG_TMP0, 0);
+ret = reloc_call(s->code_ptr - 2, arg);\
+tcg_debug_assert(ret == true);
+} else if (TCG_TARGET_REG_BITS == 64) {
+/* far jump: 64-bit */
+tcg_target_long imm = sextreg((tcg_target