[Qemu-devel] [PATCH] vfio/pci-quirks.c: Make stolen memory size adjustable for igd VFIO

[Xiong Zhang]

Currenly linux guest with kernel above 3.19 couldn't boot up on igd
passthrough env. The root case is i915 driver use stolen memory, but
qemu vfio doesn't support it.

This patch set stolen memory size to zero default, so guest i915 won't
use it. But this breaks old windows igd driver which will be unloaded
once it see zero stolen memory size. Then this patch also use x-igd-gms
option to adjust stolen memory size. New windows igd driver fixes this and
could work with zero stolen memory size.

Finally with this patch, Linux guest and windows guest with igd driver
above 15.45.4860 could work successfully, windows guest with igd driver
below 15.45.4860 should add x-igd-gms=* option.

Signed-off-by: Xiong Zhang 
---
 hw/vfio/pci-quirks.c | 95 
 1 file changed, 52 insertions(+), 43 deletions(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 60ad5fb..6e9ed7f 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1372,14 +1372,60 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
*vdev, int nr)
 uint16_t cmd_orig, cmd;
 Error *err = NULL;
 
+/* This must be an Intel VGA device. */
+if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
+!vfio_is_vga(vdev) || nr != 4) {
+return;
+}
+
 /*
- * This must be an Intel VGA device at address 00:02.0 for us to even
- * consider enabling legacy mode.  The vBIOS has dependencies on the
- * PCI bus address.
+ * IGD is not a standard, they like to change their specs often.  We
+ * only attempt to support back to SandBridge and we hope that newer
+ * devices maintain compatibility with generation 8.
  */
-if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
-!vfio_is_vga(vdev) || nr != 4 ||
->pdev != pci_find_device(pci_device_root_bus(>pdev),
+gen = igd_gen(vdev);
+if (gen != 6 && gen != 8) {
+error_report("IGD device %s is unsupported by IGD quirks, "
+ "try SandyBridge or newer", vdev->vbasedev.name);
+return;
+}
+
+/*
+ * Regardless of running in UPT or legacy mode, the guest graphics
+ * driver may attempt to use stolen memory, however only legacy mode
+ * has BIOS support for reserving stolen memory in the guest VM.
+ * Emulate the GMCH register in all cases and zero out the stolen
+ * memory size here.
+ */
+gmch = vfio_pci_read_config(>pdev, IGD_GMCH, 4);
+gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
+
+/*
+ * Assume we have no GMS memory, but allow it to be overrided by device
+ * option (experimental). Old windows igd driver must see non-zero stolen
+ * memory size, otherwise it will be unloaded. New igd windows driver fix
+ * this issue and could load with zero stolen memory size.
+ */
+if (vdev->igd_gms) {
+if (vdev->igd_gms <= 0x10) {
+gms_mb = vdev->igd_gms * 32;
+gmch |= vdev->igd_gms << (gen < 8 ? 3 : 8);
+} else {
+error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);
+vdev->igd_gms = 0;
+}
+}
+
+/* GMCH is read-only, emulated */
+pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
+pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
+pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
+
+/*
+ * This must be at address 00:02.0 for us to even onsider enabling
+ * legacy mode.  The vBIOS has dependencies on the PCI bus address.
+ */
+if (>pdev != pci_find_device(pci_device_root_bus(>pdev),
0, PCI_DEVFN(0x2, 0))) {
 return;
 }
@@ -1399,18 +1445,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
*vdev, int nr)
 }
 
 /*
- * IGD is not a standard, they like to change their specs often.  We
- * only attempt to support back to SandBridge and we hope that newer
- * devices maintain compatibility with generation 8.
- */
-gen = igd_gen(vdev);
-if (gen != 6 && gen != 8) {
-error_report("IGD device %s is unsupported in legacy mode, "
- "try SandyBridge or newer", vdev->vbasedev.name);
-return;
-}
-
-/*
  * Most of what we're doing here is to enable the ROM to run, so if
  * there's no ROM, there's no point in setting up this quirk.
  * NB. We only seem to get BIOS ROMs, so a UEFI VM would need CSM support.
@@ -1465,8 +1499,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
*vdev, int nr)
 goto out;
 }
 
-gmch = vfio_pci_read_config(>pdev, IGD_GMCH, 4);
-
 /*
  * If IGD VGA Disable is clear (expected) and VGA is not already enabled,
  * try to enable it.  Probably shouldn't be using legacy mode without VGA,
@@ -1532,24 +1564,6 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
*vdev, int nr)
 }
 
 /*
- * Assume we have no GMS memory, but 

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v2] spapr: Support ibm, dynamic-memory-v2 property

[David Gibson]

On Wed, Apr 11, 2018 at 10:31:28AM +0530, Bharata B Rao wrote:
> On Wed, Apr 11, 2018 at 02:45:58PM +1000, David Gibson wrote:
> > On Mon, Apr 09, 2018 at 11:55:38AM +0530, Bharata B Rao wrote:
[snip]
> > > @@ -2927,6 +3082,15 @@ static void spapr_instance_init(Object *obj)
> > >  " place of standard EPOW events when 
> > > possible"
> > >  " (required for memory hot-unplug 
> > > support)",
> > >  NULL);
> > > +object_property_add_bool(obj, "drmem-v2",
> > > + spapr_get_drmem_v2,
> > > + spapr_set_drmem_v2,
> > > + NULL);
> > > +object_property_set_description(obj, "ibm-dynamic-memory-v2",
> > > +"Use ibm-dynamic-memory-v2 
> > > representation"
> > > +" in place of ibm-dynamic-memory 
> > > when"
> > > +" possible",
> > > +NULL);
> > 
> > I don't really see any point to making this a user configurable
> > option.  Why not just always enable it if the guest says it can
> > support it.
> 
> Makes sense. While thinking about it, I wonder why other properties like
> modern-hotplug-events remain user configurable.

So, modern-hotplug-events affects the runtime behaviour of the events
subsystem.  That means we have to keep that behaviour consistent
across migration, which means specifically we have to keep the old
behaviour for older machine types.  Generally the easiest way to do
that is to have a compatiblity property whose defaults get set by the
machine type.

dynamic-memory-v2 doesn't affect runtime behaviour, only how stuff is
advertised in the device tree, so there's no need to maintain the old
behaviour even for old machine types: a guest started on an old
version will still have the v1 info from when it booted, and that info
will still work after migrating to a new qemu.  Likewise a guest
started on a new version will still have the v2 info and that will
still work on the DRC objects if migrated to an old version; if it
reboots then it will get v1 info, which is fine.

So, for this case we don't need the backwards compat property.

> Thanks for the review, will post next version with your suggested changes.
> 
> Regards,
> Bharata.
> 
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [qemu-s390x] [PATCH 09/10] target/s390x: avoid integer overflow in next_page PC check

[Thomas Huth]

On 10.04.2018 18:19, Emilio G. Cota wrote:
> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
> 
> Cc: Cornelia Huck 
> Cc: Alexander Graf 
> Cc: David Hildenbrand 
> Cc: qemu-s3...@nongnu.org
> Signed-off-by: Emilio G. Cota 
> ---
>  target/s390x/translate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 7d39ab3..9f1 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -6163,7 +6163,7 @@ void gen_intermediate_code(CPUState *cs, struct 
> TranslationBlock *tb)
>  CPUS390XState *env = cs->env_ptr;
>  DisasContext dc;
>  target_ulong pc_start;
> -uint64_t next_page_start;
> +uint64_t page_start;
>  int num_insns, max_insns;
>  ExitStatus status;
>  bool do_debug;
> @@ -6181,7 +6181,7 @@ void gen_intermediate_code(CPUState *cs, struct 
> TranslationBlock *tb)
>  dc.ex_value = tb->cs_base;
>  do_debug = dc.singlestep_enabled = cs->singlestep_enabled;
>  
> -next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> +page_start = pc_start & TARGET_PAGE_MASK;
>  
>  num_insns = 0;
>  max_insns = tb_cflags(tb) & CF_COUNT_MASK;
> @@ -6218,7 +6218,7 @@ void gen_intermediate_code(CPUState *cs, struct 
> TranslationBlock *tb)
>  /* If we reach a page boundary, are single stepping,
> or exhaust instruction count, stop generation.  */
>  if (status == NO_EXIT
> -&& (dc.pc >= next_page_start
> +&& (dc.pc - page_start >= TARGET_PAGE_SIZE
>  || tcg_op_buf_full()
>  || num_insns >= max_insns
>  || singlestep
> 

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [RFC PATCH v2] spapr: Support ibm, dynamic-memory-v2 property

[Bharata B Rao]

On Wed, Apr 11, 2018 at 02:45:58PM +1000, David Gibson wrote:
> On Mon, Apr 09, 2018 at 11:55:38AM +0530, Bharata B Rao wrote:
> > The new property ibm,dynamic-memory-v2 allows memory to be represented
> > in a more compact manner in device tree.
> > 
> > Signed-off-by: Bharata B Rao 
> > ---
> > v1: https://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg01788.html
> > Changes in v1:
> > - Minor cleanups in the error paths
> > - Rebased on top of ppc-for-2.13
> > 
> >  docs/specs/ppc-spapr-hotplug.txt |  19 +++
> >  hw/ppc/spapr.c   | 257 
> > ---
> >  include/hw/ppc/spapr.h   |   1 +
> >  include/hw/ppc/spapr_ovec.h  |   1 +
> >  4 files changed, 233 insertions(+), 45 deletions(-)
> 
> 
> 
> > diff --git a/docs/specs/ppc-spapr-hotplug.txt 
> > b/docs/specs/ppc-spapr-hotplug.txt
> > index f57e2a09c6..cc7833108e 100644
> > --- a/docs/specs/ppc-spapr-hotplug.txt
> > +++ b/docs/specs/ppc-spapr-hotplug.txt
> > @@ -387,4 +387,23 @@ Each LMB list entry consists of the following elements:
> >  - A 32bit flags word. The bit at bit position 0x0008 defines whether
> >the LMB is assigned to the the partition as of boot time.
> >  
> > +ibm,dynamic-memory-v2
> > +
> > +This property describes the dynamically reconfigurable memory. This is
> > +an alternate and newer way to describe dyanamically reconfigurable memory.
> > +It is a property encoded array that has an integer N (the number of
> > +LMB set entries) followed by N LMB set entries. There is an LMB set entry
> > +for each sequential group of LMBs that share common attributes.
> > +
> > +Each LMB set entry consists of the following elements:
> > +
> > +- Number of sequential LMBs in the entry represented by a 32bit integer.
> > +- Logical address of the first LMB in the set encoded as a 64bit integer.
> > +- DRC index of the first LMB in the set.
> > +- Associativity list index that is used as an index into
> > +  ibm,associativity-lookup-arrays property described earlier. This
> > +  is used to retrieve the right associativity list to be used for all
> > +  the LMBs in this set.
> > +- A 32bit flags word that applies to all the LMBs in the set.
> > +
> >  [1] 
> > http://thread.gmane.org/gmane.linux.ports.ppc.embedded/75350/focus=106867
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 3ffadd6ac7..4a24fac38c 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -669,63 +669,139 @@ static uint32_t 
> > spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
> >  return -1;
> >  }
> >  
> > -/*
> > - * Adds ibm,dynamic-reconfiguration-memory node.
> > - * Refer to docs/specs/ppc-spapr-hotplug.txt for the documentation
> > - * of this device tree node.
> > - */
> > -static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void 
> > *fdt)
> > +struct of_drconf_cell_v2 {
> 
> qemu convention is to use CamelCase for types.

Ok.

> 
> > + uint32_t seq_lmbs;
> > + uint64_t base_addr;
> > + uint32_t drc_index;
> > + uint32_t aa_index;
> > + uint32_t flags;
> > +} __attribute__((packed));
> > +
> > +#define SPAPR_DRCONF_CELL_SIZE 6
> 
> Define this using a sizeof() for safety.

Ok.

> 
> > +/* ibm,dynamic-memory-v2 */
> > +static int spapr_populate_drmem_v2(sPAPRMachineState *spapr, void *fdt,
> > +   int offset, MemoryDeviceInfoList *dimms)
> >  {
> > -MachineState *machine = MACHINE(spapr);
> > -int ret, i, offset;
> > -uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
> > -uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
> > -uint32_t hotplug_lmb_start = spapr->hotplug_memory.base / lmb_size;
> > -uint32_t nr_lmbs = (spapr->hotplug_memory.base +
> > -   memory_region_size(>hotplug_memory.mr)) /
> > -   lmb_size;
> >  uint32_t *int_buf, *cur_index, buf_len;
> > -int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
> > -MemoryDeviceInfoList *dimms = NULL;
> > +int ret;
> > +uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
> > +uint64_t addr, cur_addr, size;
> > +uint32_t nr_boot_lmbs = (spapr->hotplug_memory.base / lmb_size);
> > +uint64_t mem_end = spapr->hotplug_memory.base +
> > +   memory_region_size(>hotplug_memory.mr);
> > +uint32_t node, nr_entries = 0;
> > +sPAPRDRConnector *drc;
> > +typedef struct drconf_cell_queue {
> > +struct of_drconf_cell_v2 cell;
> > +QSIMPLEQ_ENTRY(drconf_cell_queue) entry;
> > +} drconf_cell_queue;
> 
> Likewise CamelCase here.

Ok.

> 
> > +QSIMPLEQ_HEAD(, drconf_cell_queue) drconf_queue
> > += QSIMPLEQ_HEAD_INITIALIZER(drconf_queue);
> > +drconf_cell_queue *elem, *next;
> > +MemoryDeviceInfoList *info;
> >  
> > -/*
> > - * Don't create the node if there is no hotpluggable memory
> > - */
> > -if (machine->ram_size == machine->maxram_size) {
> > -return 0;

Re: [Qemu-devel] [RFC PATCH v2] spapr: Support ibm, dynamic-memory-v2 property

[David Gibson]

On Mon, Apr 09, 2018 at 11:55:38AM +0530, Bharata B Rao wrote:
> The new property ibm,dynamic-memory-v2 allows memory to be represented
> in a more compact manner in device tree.
> 
> Signed-off-by: Bharata B Rao 
> ---
> v1: https://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg01788.html
> Changes in v1:
> - Minor cleanups in the error paths
> - Rebased on top of ppc-for-2.13
> 
>  docs/specs/ppc-spapr-hotplug.txt |  19 +++
>  hw/ppc/spapr.c   | 257 
> ---
>  include/hw/ppc/spapr.h   |   1 +
>  include/hw/ppc/spapr_ovec.h  |   1 +
>  4 files changed, 233 insertions(+), 45 deletions(-)



> diff --git a/docs/specs/ppc-spapr-hotplug.txt 
> b/docs/specs/ppc-spapr-hotplug.txt
> index f57e2a09c6..cc7833108e 100644
> --- a/docs/specs/ppc-spapr-hotplug.txt
> +++ b/docs/specs/ppc-spapr-hotplug.txt
> @@ -387,4 +387,23 @@ Each LMB list entry consists of the following elements:
>  - A 32bit flags word. The bit at bit position 0x0008 defines whether
>the LMB is assigned to the the partition as of boot time.
>  
> +ibm,dynamic-memory-v2
> +
> +This property describes the dynamically reconfigurable memory. This is
> +an alternate and newer way to describe dyanamically reconfigurable memory.
> +It is a property encoded array that has an integer N (the number of
> +LMB set entries) followed by N LMB set entries. There is an LMB set entry
> +for each sequential group of LMBs that share common attributes.
> +
> +Each LMB set entry consists of the following elements:
> +
> +- Number of sequential LMBs in the entry represented by a 32bit integer.
> +- Logical address of the first LMB in the set encoded as a 64bit integer.
> +- DRC index of the first LMB in the set.
> +- Associativity list index that is used as an index into
> +  ibm,associativity-lookup-arrays property described earlier. This
> +  is used to retrieve the right associativity list to be used for all
> +  the LMBs in this set.
> +- A 32bit flags word that applies to all the LMBs in the set.
> +
>  [1] http://thread.gmane.org/gmane.linux.ports.ppc.embedded/75350/focus=106867
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3ffadd6ac7..4a24fac38c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -669,63 +669,139 @@ static uint32_t 
> spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
>  return -1;
>  }
>  
> -/*
> - * Adds ibm,dynamic-reconfiguration-memory node.
> - * Refer to docs/specs/ppc-spapr-hotplug.txt for the documentation
> - * of this device tree node.
> - */
> -static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
> +struct of_drconf_cell_v2 {

qemu convention is to use CamelCase for types.

> + uint32_t seq_lmbs;
> + uint64_t base_addr;
> + uint32_t drc_index;
> + uint32_t aa_index;
> + uint32_t flags;
> +} __attribute__((packed));
> +
> +#define SPAPR_DRCONF_CELL_SIZE 6

Define this using a sizeof() for safety.

> +/* ibm,dynamic-memory-v2 */
> +static int spapr_populate_drmem_v2(sPAPRMachineState *spapr, void *fdt,
> +   int offset, MemoryDeviceInfoList *dimms)
>  {
> -MachineState *machine = MACHINE(spapr);
> -int ret, i, offset;
> -uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
> -uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
> -uint32_t hotplug_lmb_start = spapr->hotplug_memory.base / lmb_size;
> -uint32_t nr_lmbs = (spapr->hotplug_memory.base +
> -   memory_region_size(>hotplug_memory.mr)) /
> -   lmb_size;
>  uint32_t *int_buf, *cur_index, buf_len;
> -int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
> -MemoryDeviceInfoList *dimms = NULL;
> +int ret;
> +uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
> +uint64_t addr, cur_addr, size;
> +uint32_t nr_boot_lmbs = (spapr->hotplug_memory.base / lmb_size);
> +uint64_t mem_end = spapr->hotplug_memory.base +
> +   memory_region_size(>hotplug_memory.mr);
> +uint32_t node, nr_entries = 0;
> +sPAPRDRConnector *drc;
> +typedef struct drconf_cell_queue {
> +struct of_drconf_cell_v2 cell;
> +QSIMPLEQ_ENTRY(drconf_cell_queue) entry;
> +} drconf_cell_queue;

Likewise CamelCase here.

> +QSIMPLEQ_HEAD(, drconf_cell_queue) drconf_queue
> += QSIMPLEQ_HEAD_INITIALIZER(drconf_queue);
> +drconf_cell_queue *elem, *next;
> +MemoryDeviceInfoList *info;
>  
> -/*
> - * Don't create the node if there is no hotpluggable memory
> - */
> -if (machine->ram_size == machine->maxram_size) {
> -return 0;
> -}
> +/* Entry to cover RAM and the gap area */
> +elem = g_malloc0(sizeof(drconf_cell_queue));

Please use sizeof(*elem) - it's more robust in case you need to change
types around.

> +elem->cell.seq_lmbs = cpu_to_be32(nr_boot_lmbs);
> +elem->cell.base_addr = cpu_to_be64(0);
> +elem->cell.drc_index 

Re: [Qemu-devel] [RFC PATCH v2] spapr: Support ibm, dynamic-memory-v2 property

[David Gibson]

On Tue, Apr 10, 2018 at 09:45:21AM +0530, Bharata B Rao wrote:
> On Tue, Apr 10, 2018 at 01:02:45PM +1000, David Gibson wrote:
> > On Mon, Apr 09, 2018 at 11:55:38AM +0530, Bharata B Rao wrote:
> > > The new property ibm,dynamic-memory-v2 allows memory to be represented
> > > in a more compact manner in device tree.
> > 
> > I still need to look at this in more detail, but to start with:
> > what's the rationale for this new format?
> > 
> > It's more compact, but why do we care?  The embedded people always
> > whinge about the size of the deivce tree, but I didn't think that was
> > really a concern with PAPR.
> 
> Here's a real example of how this has affected us earlier:
> 
> SLOF's CAS FDT buffer size was initially 32K, was changed to 64k to
> support 1TB guest memory and again changed to 2MB to support 16TB guest
> memory.

Ah.. I hadn't thought of the CAS buffer, that's a legitimate concern.

> With ibm,dynamic-memory-v2 we are less likely to hit such scenarios.
> 
> Also, theoretically it should be more efficient in the guest kernel
> to handle LMB-sets than individual LMBs.
> 
> We aren't there yet, but I believe grouping of LMBs should eventually
> help us do memory hotplug at set (or DIMM) granularity than at individual
> LMB granularity (Again theoretical possibility)

Ok, sounds like it might be useful.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH v2] spapr: Support ibm, dynamic-memory-v2 property

[David Gibson]

On Wed, Apr 11, 2018 at 01:56:14PM +1000, Alexey Kardashevskiy wrote:
1;5002;0c> On 10/4/18 1:02 pm, David Gibson wrote:
> > On Mon, Apr 09, 2018 at 11:55:38AM +0530, Bharata B Rao wrote:
> >> The new property ibm,dynamic-memory-v2 allows memory to be represented
> >> in a more compact manner in device tree.
> > 
> > I still need to look at this in more detail, but to start with:
> > what's the rationale for this new format?
> > 
> > It's more compact, but why do we care?  The embedded people always
> > whinge about the size of the deivce tree, but I didn't think that was
> > really a concern with PAPR.
> 
> 
> Well, booting a guest with 500 pci devices (let's say emulated e1000 or
> virtio-net) creates >20sec delay in the guest while it is fetching the
> device tree in early setup, property by property, I even have a patch for
> pseries guest to get FDT as a whole blob from SLOF. It is not The Problem
> but still annoying.

Right, but 500 PCI devices is not this case.  Sounds like the slowness
there is mostly from having lots of nodes and properties and therefore
lots of individual OF calls.  The old dynamic memory format is still
just one big property, so it shouldn't have nearly the same impact.

Besides, Bharata pretty much convinced me already with his other reasons.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH v2] spapr: Support ibm, dynamic-memory-v2 property

[Alexey Kardashevskiy]

On 10/4/18 1:02 pm, David Gibson wrote:
> On Mon, Apr 09, 2018 at 11:55:38AM +0530, Bharata B Rao wrote:
>> The new property ibm,dynamic-memory-v2 allows memory to be represented
>> in a more compact manner in device tree.
> 
> I still need to look at this in more detail, but to start with:
> what's the rationale for this new format?
> 
> It's more compact, but why do we care?  The embedded people always
> whinge about the size of the deivce tree, but I didn't think that was
> really a concern with PAPR.


Well, booting a guest with 500 pci devices (let's say emulated e1000 or
virtio-net) creates >20sec delay in the guest while it is fetching the
device tree in early setup, property by property, I even have a patch for
pseries guest to get FDT as a whole blob from SLOF. It is not The Problem
but still annoying.


-- 
Alexey

Re: [Qemu-devel] [PATCH 2/2] qemu-thread: let cur_mon be per-thread

[Peter Xu]

On Wed, Apr 11, 2018 at 09:45:32AM +0800, Stefan Hajnoczi wrote:
> On Tue, Apr 10, 2018 at 08:49:13PM +0800, Peter Xu wrote:
> > cur_mon was only used in main loop so we don't really need that to be
> > per-thread variable.  Now it's possible that we have more than one
> > thread to operate on it.  Let's start to let it be per-thread variable.
> 
> Trying to understand the reason for this patch:
> 
> Are there any users of per-thread cur_mon?

Currently no.  But if considering future OOB-capable commands, they
will modify cur_mon in monitor IOThread at least.

> 
> or
> 
> Does this patch fix a bug?

No; currently we have no bug.  But we have encounter the bug when we
start to add more OOB commands.

Here is the problem (Marc-Andre reported this, and I'll try to
summarize): after we have valid OOB commands,
monitor_qmp_dispatch_one() can be run not only in main thread, but
also in monitor iothread.  When that happens, both of them (main
thread, and monitor iothread) can be modifying the cur_mon variable at
the same time. [1]

Considering that cur_mon is only used "just like" a stack variable, it
should be perfectly fine we just make it as a per thread variable,
hence this patch.

> 
> > In case we'll create threads within a valid cur_mon setup, we'd better
> > let the child threads to inherit the cur_mon from parent thread too.  Do
> > that for both posix and win32 threads.
> 
> Without actual users I don't like this.  It sounds like "let's make it
> global just in case something needs it some day".
> 
> It's ugly for QEMU's thread API to know about the monitor - that's a
> layering violation.

Yes, I'm sorry about it.  Actually I don't like it too.  But that
seems to be an efficient and simple solution to me now.  The ideal
solution should be totally removing cur_mon, which is non-trivial.

And for sure we can try to avoid layer violation.  For example, we can
have something like qemu_thread_register_variable(pthread_key_t), then
monitor code can register the cur_mon pthread_key to the qemu thread
module.  That'll somehow achieve isolation between modules but I'm not
sure whether that would be necessary for now, hence I chose the simple.

> 
> If there's a legitimate need I think this patch might be necessary, but
> I don't see enough justification to do this yet.

The problem was described at [1].  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 2/2] qemu-thread: let cur_mon be per-thread

[Peter Xu]

On Tue, Apr 10, 2018 at 08:54:31AM -0500, Eric Blake wrote:
> On 04/10/2018 07:49 AM, Peter Xu wrote:
> > cur_mon was only used in main loop so we don't really need that to be
> > per-thread variable.  Now it's possible that we have more than one
> > thread to operate on it.  Let's start to let it be per-thread variable.
> > 
> > In case we'll create threads within a valid cur_mon setup, we'd better
> > let the child threads to inherit the cur_mon from parent thread too.  Do
> > that for both posix and win32 threads.
> > 
> > Signed-off-by: Peter Xu 
> > ---
> >  include/monitor/monitor.h   | 2 +-
> >  include/qemu/thread-win32.h | 1 +
> >  monitor.c   | 2 +-
> >  stubs/monitor.c | 2 +-
> >  tests/test-util-sockets.c   | 2 +-
> >  util/qemu-thread-posix.c| 6 ++
> >  util/qemu-thread-win32.c| 6 ++
> >  7 files changed, 17 insertions(+), 4 deletions(-)
> > 
> 
> > @@ -494,6 +496,9 @@ static void *qemu_thread_start(void *args)
> >  void *(*start_routine)(void *) = qemu_thread_args->start_routine;
> >  void *arg = qemu_thread_args->arg;
> >  
> > +/* Inherit the cur_mon pointer from father thread */
> 
> More typical as s/father/parent/

Fixed.

> 
> > +++ b/util/qemu-thread-win32.c
> 
> > @@ -339,6 +341,9 @@ static unsigned __stdcall win32_start_routine(void *arg)
> >  void *(*start_routine)(void *) = data->start_routine;
> >  void *thread_arg = data->arg;
> >  
> > +/* Inherit the cur_mon pointer from father thread */
> > +cur_mon = data->current_monitor;
> 
> Otherwise makes sense to me.
> 
> I agree with your analysis that the set of existing OOB commands (just
> 'x-oob-test') has no direct use of cur_mon.  I'm a little fuzzier on
> whether the OOB changes can cause cur_mon to be modified by two threads
> in parallel (monitor_qmp_dispatch_one() is futzing around with 'cur_mon'
> around the call to qmp_dispatch(), and at least
> qmp_human_monitor_command() is also futzing around with it; is there a
> case where handling qmp_human_monitor_command() in the dispatch thread
> in parallel with more input on the main thread could break?)  Thus I'm
> not sure whether this is needed for 2.12 to avoid a regression.

Could I ask what's the "more input on the main thread"?

AFAIU, if we don't take x-oob-test into account, then still only the
main thread will touch (not only modify, even read) the cur_mon
variable.  And as long as that's true, we are keeping the old behavior
as when we are without Out-Of-Band, then IMHO we'll be fine.

qmp_human_monitor_command() is only one QMP command handler, now it
can only be run within main thread.  So even we can have the stack
like monitor_qmp_dispatch_one (which modifies cur_mon) calls
qmp_human_monitor_command (which modifies cur_mon again), still we'll
be fine too as long as they are in the same thread, just like before.

That's why I think it's not a 2.12 regression.  We just need to be
prepared for what might come. Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 1/2] qemu-thread: always keep the posix wrapper layer

[Peter Xu]

On Tue, Apr 10, 2018 at 08:35:40AM -0500, Eric Blake wrote:
> On 04/10/2018 07:49 AM, Peter Xu wrote:
> > We will conditionally have a wrapper layer depending on whether the host
> > has the PTHREAD_SETNAME capability.  It complicates stuff.  Let's just
> > keep the wrapper there, meanwhile we opt out the pthread_setname_np()
> > call only.  The layer can be helpful in future patches to pass data from
> > the parent thread to the child thread.
> > 
> > Signed-off-by: Peter Xu 
> > ---
> >  util/qemu-thread-posix.c | 33 +
> >  1 file changed, 13 insertions(+), 20 deletions(-)
> > 
> > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> > index b789cf32e9..3ae96210d6 100644
> > --- a/util/qemu-thread-posix.c
> > +++ b/util/qemu-thread-posix.c
> > @@ -482,7 +482,6 @@ static void __attribute__((constructor)) 
> > qemu_thread_atexit_init(void)
> >  }
> >  
> 
> More context:
> 
> static bool name_threads;
> 
> void qemu_thread_naming(bool enable)
> {
> name_threads = enable;
> 
> #ifndef CONFIG_THREAD_SETNAME_BYTHREAD
> /* This is a debugging option, not fatal */
> if (enable) {
> fprintf(stderr, "qemu: thread naming not supported on this host\n");
> }
> #endif
> }
> 
> 
> >  
> > -#ifdef CONFIG_PTHREAD_SETNAME_NP
> 
> Why are we using CONFIG_THREAD_SETNAME_BYTHREAD in one place, and
> CONFIG_PTHREAD_SETNAME_NP in another?
> 
> /me checks configure - oh:
> 
> # Hold two types of flag:
> #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
> # a thread we have a handle to
> #   CONFIG_PTHREAD_SETNAME_NP   - A way of doing it on a particular
> # platform
> 
> even though, right now, we only either set both flags at once or leave
> both clear, since we don't (yet?) have any other platform-specific ways
> to do it.

It seems so.  I'm not sure whether they could be useful in the future
and I'm fine with them, hence I keep it as is.

> 
> >  typedef struct {
> >  void *(*start_routine)(void *);
> >  void *arg;
> > @@ -498,13 +497,15 @@ static void *qemu_thread_start(void *args)
> >  /* Attempt to set the threads name; note that this is for debug, so
> >   * we're not going to fail if we can't set it.
> >   */
> > -pthread_setname_np(pthread_self(), qemu_thread_args->name);
> > +#ifdef CONFIG_PTHREAD_SETNAME_NP
> > +if (qemu_thread_args->name) {
> > +pthread_setname_np(pthread_self(), qemu_thread_args->name);
> 
> Post-patch, this (attempts to) set the thread name if a non-NULL name is
> present...
> 
> 
> >  
> > -#ifdef CONFIG_PTHREAD_SETNAME_NP
> > -if (name_threads) {
> > -QemuThreadArgs *qemu_thread_args;
> > -qemu_thread_args = g_new0(QemuThreadArgs, 1);
> > -qemu_thread_args->name = g_strdup(name);
> 
> ...but pre-patch, qemu_thread_args->name was left NULL unless
> name_threads was true, because someone had called
> qemu_thread_naming(true)...
> 
> > -qemu_thread_args->start_routine = start_routine;
> > -qemu_thread_args->arg = arg;
> > -
> > -err = pthread_create(>thread, ,
> > - qemu_thread_start, qemu_thread_args);
> > -} else
> > -#endif
> > -{
> > -err = pthread_create(>thread, ,
> > - start_routine, arg);
> > -}
> > +qemu_thread_args = g_new0(QemuThreadArgs, 1);
> > +qemu_thread_args->name = g_strdup(name);
> 
> ...so you have changed semantics - you are now unconditionally trying to
> set the thread name, instead of honoring qemu_thread_naming().  Do we
> still need qemu_thread_naming() (tied to opt debug-threads)?
> 
> You need to either fix your code to remain conditional on whether
> name_threads is set, or document the semantic change as intentional in
> the commit message.

Indeed, thanks for catching that. What I really wanted is probably
this:

static void *qemu_thread_start(void *args)
{
...
#ifdef CONFIG_PTHREAD_SETNAME_NP
if (name_threads && qemu_thread_args->name) {
pthread_setname_np(pthread_self(), qemu_thread_args->name);
}
#endif
...
}

I'll fix that up in next version.

> 
> However, the idea for refactoring to always use the shim makes sense.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 7/8] virtio: get avail bytes check for packed ring

[Jason Wang]

On 2018年04月04日 20:54, w...@redhat.com wrote:

From: Wei Xu 

mostly as same as 1.0, copy it separately for
prototype, need a refactoring.

Signed-off-by: Wei Xu 
---
  hw/virtio/virtio.c | 142 +++--
  1 file changed, 139 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index def07c6..cf726f3 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -836,9 +836,9 @@ static int virtqueue_read_next_desc(VirtIODevice *vdev, 
VRingDesc *desc,
  return VIRTQUEUE_READ_DESC_MORE;
  }
  
-void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,

-   unsigned int *out_bytes,
-   unsigned max_in_bytes, unsigned max_out_bytes)
+static void virtqueue_get_avail_bytes_split(VirtQueue *vq,
+unsigned int *in_bytes, unsigned int *out_bytes,
+unsigned max_in_bytes, unsigned max_out_bytes)
  {
  VirtIODevice *vdev = vq->vdev;
  unsigned int max, idx;
@@ -961,6 +961,142 @@ err:
  goto done;
  }
  
+static void virtqueue_get_avail_bytes_packed(VirtQueue *vq,

+unsigned int *in_bytes, unsigned int *out_bytes,
+unsigned max_in_bytes, unsigned max_out_bytes)
+{
+VirtIODevice *vdev = vq->vdev;
+unsigned int max, idx;
+unsigned int total_bufs, in_total, out_total;
+MemoryRegionCache *desc_cache;
+VRingMemoryRegionCaches *caches;
+MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
+int64_t len = 0;
+VRingDescPacked desc;
+
+if (unlikely(!vq->packed.desc)) {
+if (in_bytes) {
+*in_bytes = 0;
+}
+if (out_bytes) {
+*out_bytes = 0;
+}
+return;
+}
+
+rcu_read_lock();
+idx = vq->last_avail_idx;
+total_bufs = in_total = out_total = 0;
+
+max = vq->packed.num;
+caches = vring_get_region_caches(vq);
+if (caches->desc.len < max * sizeof(VRingDescPacked)) {
+virtio_error(vdev, "Cannot map descriptor ring");
+goto err;
+}
+
+desc_cache = >desc;
+vring_desc_read_packed(vdev, , desc_cache, idx);
+while (is_desc_avail()) {
+unsigned int num_bufs;
+unsigned int i;
+
+num_bufs = total_bufs;
+
+if (desc.flags & VRING_DESC_F_INDIRECT) {
+if (desc.len % sizeof(VRingDescPacked)) {
+virtio_error(vdev, "Invalid size for indirect buffer table");
+goto err;
+}
+
+/* If we've got too many, that implies a descriptor loop. */
+if (num_bufs >= max) {
+virtio_error(vdev, "Looped descriptor");
+goto err;
+}
+
+/* loop over the indirect descriptor table */
+len = address_space_cache_init(_desc_cache,
+   vdev->dma_as,
+   desc.addr, desc.len, false);
+desc_cache = _desc_cache;
+if (len < desc.len) {
+virtio_error(vdev, "Cannot map indirect buffer");
+goto err;
+}
+
+max = desc.len / sizeof(VRingDescPacked);
+num_bufs = i = 0;
+vring_desc_read_packed(vdev, , desc_cache, i);
+}
+
+do {
+/* If we've got too many, that implies a descriptor loop. */
+if (++num_bufs > max) {
+virtio_error(vdev, "Looped descriptor");
+goto err;
+}
+
+if (desc.flags & VRING_DESC_F_WRITE) {
+in_total += desc.len;
+} else {
+out_total += desc.len;
+}
+if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
+goto done;
+}
+
+if (desc_cache == _desc_cache) {
+vring_desc_read_packed(vdev, , desc_cache,
+   ++i % vq->packed.num);
+} else {
+vring_desc_read_packed(vdev, , desc_cache,
+   ++idx % vq->packed.num);
+}


Need to make sure desc.flags was read before other fields, otherwise we 
may get stale address.


Thanks


+} while (desc.flags & VRING_DESC_F_NEXT);
+
+if (desc_cache == _desc_cache) {
+address_space_cache_destroy(_desc_cache);
+total_bufs++;
+/* We missed one step on for indirect desc */
+idx++;
+} else {
+total_bufs = num_bufs;
+}
+
+desc_cache = >desc;
+vring_desc_read_packed(vdev, , desc_cache, idx % vq->packed.num);
+}
+
+done:
+address_space_cache_destroy(_desc_cache);
+if (in_bytes) {
+*in_bytes = in_total;
+}
+if (out_bytes) {
+*out_bytes = out_total;
+}

Re: [Qemu-devel] [PATCH 6/8] virtio: flush/push support for packed ring

[Jason Wang]

On 2018年04月04日 20:54, w...@redhat.com wrote:

From: Wei Xu 

Signed-off-by: Wei Xu 
---
  hw/virtio/virtio.c | 104 +
  1 file changed, 90 insertions(+), 14 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 95a4681..def07c6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -26,6 +26,7 @@
  
  #define AVAIL_DESC_PACKED(b) ((b) << 7)

  #define USED_DESC_PACKED(b)  ((b) << 15)
+#define VIRTQ_F_DESC_USED(w)  (AVAIL_DESC_PACKED(w) | USED_DESC_PACKED(w))
  
  /*

   * The alignment to use between consumer and producer parts of vring.
@@ -636,19 +637,11 @@ bool virtqueue_rewind(VirtQueue *vq, unsigned int num)
  }
  
  /* Called within rcu_read_lock().  */

-void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
+static void virtqueue_fill_split(VirtQueue *vq, const VirtQueueElement *elem,
  unsigned int len, unsigned int idx)
  {
  VRingUsedElem uelem;
  
-trace_virtqueue_fill(vq, elem, len, idx);

-
-virtqueue_unmap_sg(vq, elem, len);
-
-if (unlikely(vq->vdev->broken)) {
-return;
-}
-
  if (unlikely(!vq->vring.used)) {
  return;
  }
@@ -660,16 +653,66 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement 
*elem,
  vring_used_write(vq, , idx);
  }
  
-/* Called within rcu_read_lock().  */

-void virtqueue_flush(VirtQueue *vq, unsigned int count)
+static void virtqueue_fill_packed(VirtQueue *vq, const VirtQueueElement *elem)
  {
-uint16_t old, new;
+uint16_t i, w, head;
+VRingMemoryRegionCaches *caches;
+VRingDescPacked desc = {
+.addr = 0,
+.flags = 0,
+};
+
+if (unlikely(!vq->packed.desc)) {
+return;
+}
+
+w = elem->wrap_counter;
+caches = vring_get_region_caches(vq);
+for (i = 0; i < elem->count; i++) {
+head = (elem->index + i) % vq->packed.num;


This looks strange, we should get the location from vq->used_idx.


+/* Don't toggle the first one since it is the originally one */
+if ((i > 0) && (!head)) {
+w ^= 1;
+}
+
+desc.id = elem->index;
+desc.flags = VIRTQ_F_DESC_USED(w);
+desc.len = elem->len[i];


This should be determined by the caller, especially for the WRITE 
descriptor.



+virtio_tswap16s(vq->vdev, );
+virtio_tswap32s(vq->vdev, );
+virtio_tswap16s(vq->vdev, );
+address_space_write_cached(>desc,
+   sizeof(VRingDescPacked) * head, ,
+   sizeof(VRingDescPacked));


We should make sure flags is written after other fileds. So need a wmb() 
between the two writes.



+address_space_cache_invalidate(>desc,
+   sizeof(VRingDescPacked) * head,
+   sizeof(VRingDescPacked));
+}


Have you tested the case when elem->count is greater than one? According 
to the spec device only need to write a single used descriptor for the 
whole list.



+}
+
+void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
+unsigned int len, unsigned int idx)
+{
+trace_virtqueue_fill(vq, elem, len, idx);
+
+virtqueue_unmap_sg(vq, elem, len);
  
  if (unlikely(vq->vdev->broken)) {

-vq->inuse -= count;
  return;
  }
  
+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {

+virtqueue_fill_packed(vq, elem);
+} else {
+virtqueue_fill_split(vq, elem, len, idx);
+}
+}
+
+/* Called within rcu_read_lock().  */
+static void virtqueue_flush_split(VirtQueue *vq, unsigned int count)
+{
+uint16_t old, new;
+
  if (unlikely(!vq->vring.used)) {
  return;
  }
@@ -685,12 +728,45 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count)
  vq->signalled_used_valid = false;
  }
  
+static void virtqueue_flush_packed(VirtQueue *vq, unsigned int count)

+{
+if (unlikely(!vq->packed.desc)) {
+return;
+}
+
+vq->inuse -= count;
+
+/* FIXME: is this correct? */
+if (vq->inuse) {
+return;
+}


I think the semantics here is write flags here.


+}
+
+void virtqueue_flush(VirtQueue *vq, unsigned int count)
+{
+if (unlikely(vq->vdev->broken)) {
+vq->inuse -= count;
+return;
+}
+
+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+virtqueue_flush_packed(vq, count);
+} else {
+virtqueue_flush_split(vq, count);
+}
+}
+
  void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
  unsigned int len)
  {
  rcu_read_lock();
  virtqueue_fill(vq, elem, len, 0);
-virtqueue_flush(vq, 1);
+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+/* FIXME: How to deal with the length field for chained desc */
+virtqueue_flush(vq, elem->count);
+} else {
+

Re: [Qemu-devel] [PATCH 8/8] virtio: queue pop support for packed ring

[Jason Wang]

On 2018年04月04日 20:54, w...@redhat.com wrote:

From: Wei Xu 

cloned from split ring pop, a global static length array
and the inside-element length array are introduced to
easy prototype, this consumes more memory and it is valuable
to move to dynamic allocation as the out/in sg does.


To ease the reviewer, I suggest to reorder this patch to patch 4.



Signed-off-by: Wei Xu 
---
  hw/virtio/virtio.c | 154 -
  1 file changed, 153 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index cf726f3..0eafb38 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1221,7 +1221,7 @@ static void *virtqueue_alloc_element(size_t sz, unsigned 
out_num, unsigned in_nu
  return elem;
  }
  
-void *virtqueue_pop(VirtQueue *vq, size_t sz)

+static void *virtqueue_pop_split(VirtQueue *vq, size_t sz)
  {
  unsigned int i, head, max;
  VRingMemoryRegionCaches *caches;
@@ -1356,6 +1356,158 @@ err_undo_map:
  goto done;
  }
  
+static uint16_t dma_len[VIRTQUEUE_MAX_SIZE];


This looks odd.


+static void *virtqueue_pop_packed(VirtQueue *vq, size_t sz)
+{
+unsigned int i, head, max;
+VRingMemoryRegionCaches *caches;
+MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
+MemoryRegionCache *cache;
+int64_t len;
+VirtIODevice *vdev = vq->vdev;
+VirtQueueElement *elem = NULL;
+unsigned out_num, in_num, elem_entries;
+hwaddr addr[VIRTQUEUE_MAX_SIZE];
+struct iovec iov[VIRTQUEUE_MAX_SIZE];
+VRingDescPacked desc;
+uint8_t wrap_counter;
+
+if (unlikely(vdev->broken)) {
+return NULL;
+}
+
+vq->last_avail_idx %= vq->packed.num;


Queue size could not be a power of 2.


+
+rcu_read_lock();
+if (virtio_queue_empty_packed_rcu(vq)) {
+goto done;
+}
+
+/* When we start there are none of either input nor output. */
+out_num = in_num = elem_entries = 0;
+
+max = vq->vring.num;
+
+if (vq->inuse >= vq->vring.num) {
+virtio_error(vdev, "Virtqueue size exceeded");
+goto done;
+}
+
+if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+/* FIXME: TBD */
+}
+
+head = vq->last_avail_idx;
+i = head;
+
+caches = vring_get_region_caches(vq);
+cache = >desc_packed;
+vring_desc_read_packed(vdev, , cache, i);
+if (desc.flags & VRING_DESC_F_INDIRECT) {
+if (desc.len % sizeof(VRingDescPacked)) {
+virtio_error(vdev, "Invalid size for indirect buffer table");
+goto done;
+}
+
+/* loop over the indirect descriptor table */
+len = address_space_cache_init(_desc_cache, vdev->dma_as,
+   desc.addr, desc.len, false);
+cache = _desc_cache;
+if (len < desc.len) {
+virtio_error(vdev, "Cannot map indirect buffer");
+goto done;
+}
+
+max = desc.len / sizeof(VRingDescPacked);
+i = 0;
+vring_desc_read_packed(vdev, , cache, i);
+}
+
+wrap_counter = vq->wrap_counter;
+/* Collect all the descriptors */
+while (1) {
+bool map_ok;
+
+if (desc.flags & VRING_DESC_F_WRITE) {
+map_ok = virtqueue_map_desc(vdev, _num, addr + out_num,
+iov + out_num,
+VIRTQUEUE_MAX_SIZE - out_num, true,
+desc.addr, desc.len);
+} else {
+if (in_num) {
+virtio_error(vdev, "Incorrect order for descriptors");
+goto err_undo_map;
+}
+map_ok = virtqueue_map_desc(vdev, _num, addr, iov,
+VIRTQUEUE_MAX_SIZE, false,
+desc.addr, desc.len);
+}
+if (!map_ok) {
+goto err_undo_map;
+}
+
+/* If we've got too many, that implies a descriptor loop. */
+if (++elem_entries > max) {
+virtio_error(vdev, "Looped descriptor");
+goto err_undo_map;
+}
+
+dma_len[i++] = desc.len;
+/* Toggle wrap_counter for non indirect desc */
+if ((i == vq->packed.num) && (cache != _desc_cache)) {
+vq->wrap_counter ^= 1;
+}
+
+if (desc.flags & VRING_DESC_F_NEXT) {
+vring_desc_read_packed(vq->vdev, , cache, i % vq->packed.num);
+} else {
+break;
+}
+}
+
+/* Now copy what we have collected and mapped */
+elem = virtqueue_alloc_element(sz, out_num, in_num);
+elem->index = head;


This seems wrong, we could not assume buffer id is last_avail_idx.


+elem->wrap_counter = wrap_counter;
+elem->count = (cache == _desc_cache) ? 1 : out_num + in_num;
+for (i = 0; i < out_num; i++) {
+/* DMA Done by marking the length as 0 */


This seems 

Re: [Qemu-devel] [RFC PATCH 0/8] virtio-net 1.1 userspace backend support

[Wei Xu]

On Tue, Apr 10, 2018 at 11:46:47AM +0800, Jason Wang wrote:
>
> 
> On 2018年04月04日 20:53, w...@redhat.com wrote:
> >From: Wei Xu 
> >
> >This is a prototype for virtio-net 1.1 support in userspace backend,
> >only minimum part are included in this RFC(roughly synced to v8 as
> >Jason and Tiwei's RFC).
> >
> >Test has been done together with Tiwei's RFC guest virtio-net driver
> >patch, ping and a quick iperf test successfully.
> >
> >Issues:
> >1. Rx performance of Iperf is much slower than TX.
> > TX: 13-15Gb
> > RX: 100-300Mb
> 
> This needs to be investigated. What's the pps of TX/RX then? (Maybe you can
> try Jen's dpdk code too).

Yes, I haven't tune any tso/gso on tap so the pps should match the bandwidth,
will try some more debugging and tried Jen's code if I can not resolve it.

Wei

> 
> Thanks
> 
> >
> >Missing:
> >- device and driver
> >- indirect descriptor
> >- migration
> >- vIOMMU support
> >- other revisions since v8
> >- see FIXME
> >
> >Wei Xu (8):
> >   virtio: feature bit, data structure for packed ring
> >   virtio: memory cache for packed ring
> >   virtio: add empty check for packed ring
> >   virtio: add detach element for packed ring(1.1)
> >   virtio: notification tweak for packed ring
> >   virtio: flush/push support for packed ring
> >   virtio: get avail bytes check for packed ring
> >   virtio: queue pop support for packed ring
> >
> >  hw/virtio/virtio.c | 618 
> > +++--
> >  include/hw/virtio/virtio.h |  12 +-
> >  include/standard-headers/linux/virtio_config.h |   2 +
> >  3 files changed, 601 insertions(+), 31 deletions(-)
> >
> 

Re: [Qemu-devel] [PATCH for-2.12] i386: Don't automatically enable FEAT_KVM_HINTS bits

[Wanpeng Li]

2018-04-11 5:15 GMT+08:00 Eduardo Habkost :
> The assumption in the cpu->max_features code is that anything
> enabled on GET_SUPPORTED_CPUID should be enabled on "-cpu host".
> This shouldn't be the case for FEAT_KVM_HINTS.
>
> This adds a new FeatureWordInfo::no_autoenable_flags field, that
> can be used to prevent FEAT_KVM_HINTS bits to be enabled
> automatically.
>
> Reviewed-by: Paolo Bonzini 
> Signed-off-by: Eduardo Habkost 

Thanks for the patch, Eduardo!
Tested-by: Wanpeng Li 

Regards,
Wanpeng Li

> ---
>  target/i386/cpu.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1a6b082b6f..a20fe26573 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -295,6 +295,8 @@ typedef struct FeatureWordInfo {
>  uint32_t tcg_features; /* Feature flags supported by TCG */
>  uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
>  uint32_t migratable_flags; /* Feature flags known to be migratable */
> +/* Features that shouldn't be auto-enabled by "-cpu host" */
> +uint32_t no_autoenable_flags;
>  } FeatureWordInfo;
>
>  static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> @@ -400,6 +402,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
> = {
>  },
>  .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EDX,
>  .tcg_features = TCG_KVM_FEATURES,
> +/*
> + * KVM hints aren't auto-enabled by -cpu host, they need to be
> + * explicitly enabled in the command-line.
> + */
> +.no_autoenable_flags = ~0U,
>  },
>  [FEAT_HYPERV_EAX] = {
>  .feat_names = {
> @@ -4062,7 +4069,8 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error 
> **errp)
>   */
>  env->features[w] |=
>  x86_cpu_get_supported_feature_word(w, cpu->migratable) &
> -~env->user_features[w];
> +~env->user_features[w] & \
> +~feature_word_info[w].no_autoenable_flags;
>  }
>  }
>
> --
> 2.14.3
>

Re: [Qemu-devel] [PATCH for-2.12] commit/stream: Reset delay_ns

[Stefan Hajnoczi]

On Tue, Apr 10, 2018 at 10:58:09AM +0200, Kevin Wolf wrote:
> Streaming and the commit block job only want to apply throttling when
> they actually copied data instead of skipping it, so they made the
> calculation of delay_ns conditional. However, delay_ns isn't reset when
> skipping some sectors, so instead of not waiting, the old delay is
> applied again.
> 
> Properly reset delay_ns where needed.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/commit.c | 2 ++
>  block/stream.c | 2 ++
>  2 files changed, 4 insertions(+)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/2] qemu-thread: let cur_mon be per-thread

[Stefan Hajnoczi]

On Tue, Apr 10, 2018 at 08:49:13PM +0800, Peter Xu wrote:
> cur_mon was only used in main loop so we don't really need that to be
> per-thread variable.  Now it's possible that we have more than one
> thread to operate on it.  Let's start to let it be per-thread variable.

Trying to understand the reason for this patch:

Are there any users of per-thread cur_mon?

or

Does this patch fix a bug?

> In case we'll create threads within a valid cur_mon setup, we'd better
> let the child threads to inherit the cur_mon from parent thread too.  Do
> that for both posix and win32 threads.

Without actual users I don't like this.  It sounds like "let's make it
global just in case something needs it some day".

It's ugly for QEMU's thread API to know about the monitor - that's a
layering violation.

If there's a legitimate need I think this patch might be necessary, but
I don't see enough justification to do this yet.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 01/15] tests: add fp-test, a floating point test suite

[Alex Bennée]

Alex Bennée  writes:

> Emilio G. Cota  writes:
>
>> This will allow us to run correctness tests against our
>> FP implementation. The test can be run in two modes (called
>> "testers"): host and soft. With the former we check the results
>> and FP flags on the host machine against the model.
>> With the latter we check QEMU's fpu primitives against the
>> model. Note that in soft mode we are not instantiating any
>> particular CPU (hence the HW_POISON_H hack to avoid macro poisoning);
>> for that we need to run the test in host mode under QEMU.
> 
>
> So with the attached patch and my proposed cross build we can now get:
>

> --- a/tests/tcg/Makefile
> +++ b/tests/tcg/Makefile
> @@ -24,6 +24,9 @@
>  VPATH = $(SRC_PATH)/tests/tcg/multiarch
>  TEST_SRCS = $(wildcard $(SRC_PATH)/tests/tcg/multiarch/*.c)
>
> +VPATH += $(SRC_PATH)/tests/fp
> +TEST_SRCS += $(wildcard $(SRC_PATH)/tests/fp/*.c)
> +
>  VPATH += $(SRC_PATH)/tests/tcg/$(ARCH)
>  TEST_SRCS += $(wildcard $(SRC_PATH)/tests/tcg/$(ARCH)/*.c)

It also needs:

fp-test: LDFLAGS+=-lm

--
Alex Bennée

Re: [Qemu-devel] [PATCH] migration: calculate expected_downtime with ram_bytes_remaining()

[David Gibson]

On Tue, 10 Apr 2018 11:02:36 +0100
"Dr. David Alan Gilbert"  wrote:

> * David Gibson (dgib...@redhat.com) wrote:
> > On Mon, 9 Apr 2018 19:57:47 +0100
> > "Dr. David Alan Gilbert"  wrote:
> >   
> > > * Balamuruhan S (bal...@linux.vnet.ibm.com) wrote:  
> > > > On 2018-04-04 13:36, Peter Xu wrote:
> > > > > On Wed, Apr 04, 2018 at 11:55:14AM +0530, Balamuruhan S wrote:  
> > [snip]  
> > > > > > > - postcopy: that'll let you start the destination VM even without
> > > > > > >   transferring all the RAMs before hand
> > > > > > 
> > > > > > I am seeing issue in postcopy migration between POWER8(16M) ->
> > > > > > POWER9(1G)
> > > > > > where the hugepage size is different. I am trying to enable it but
> > > > > > host
> > > > > > start
> > > > > > address have to be aligned with 1G page size in
> > > > > > ram_block_discard_range(),
> > > > > > which I am debugging further to fix it.
> > > > > 
> > > > > I thought the huge page size needs to be matched on both side
> > > > > currently for postcopy but I'm not sure.
> > > > 
> > > > you are right! it should be matched, but we need to support
> > > > POWER8(16M) -> POWER9(1G)
> > > > 
> > > > > CC Dave (though I think Dave's still on PTO).
> > > 
> > > There's two problems there:
> > >   a) Postcopy with really big huge pages is a problem, because it takes
> > >  a long time to send the whole 1G page over the network and the vCPU
> > >  is paused during that time;  for example on a 10Gbps link, it takes
> > >  about 1 second to send a 1G page, so that's a silly time to keep
> > >  the vCPU paused.
> > > 
> > >   b) Mismatched pagesizes are a problem on postcopy; we require that the
> > >  whole of a hostpage is sent continuously, so that it can be
> > >  atomically placed in memory, the source knows to do this based on
> > >  the page sizes that it sees.  There are some other cases as well 
> > >  (e.g. discards have to be page aligned.)  
> > 
> > I'm not entirely clear on what mismatched means here.  Mismatched
> > between where and where?  I *think* the relevant thing is a mismatch
> > between host backing page size on source and destination, but I'm not
> > certain.  
> 
> Right.  As I understand it, we make no requirements on (an x86) guest
> as to what page sizes it uses given any particular host page sizes.

Right - AIUI there are basically separate gva->gpa and gpa->hpa page
tables and the pagesizes in each are unrelated.  That's also how it
works on POWER9 radix mode, so it doesn't suffer this restriction
either. In hash mode, though, there's just a single va->hpa hashed page
table which is owned by the host and updated by the guest via hcall.

>  [...]  
> > 
> > Sounds feasible, but like something that will take some thought and
> > time upstream.  
> 
> Yes; it's not too bad.
> 
> > > (a) is a much much harder problem; one *idea* would be a major
> > > reorganisation of the kernels hugepage + userfault code to somehow
> > > allow them to temporarily present as normal pages rather than a
> > > hugepage.  
> > 
> > Yeah... for Power specifically, I think doing that would be really
> > hard, verging on impossible, because of the way the MMU is
> > virtualized.  Well.. it's probably not too bad for a native POWER9
> > guest (using the radix MMU), but the issue here is for POWER8 compat
> > guests which use the hash MMU.  
> 
> My idea was to fill the pagetables for that hugepage using small page
> entries but using the physical hugepages memory; so that once we're
> done we'd flip it back to being a single hugepage entry.
> (But my understanding is that doesn't fit at all into the way the kernel
> hugepage code works).

I think it should be possible with hugepage code, although we might end
up only the physical allocation side of the existing hugepage code, not
the actual putting it in the pagetable parts.  Which is not to say
there couldn't be some curly edge cases.

The bigger problem for us is it really doesn't fit with the way HPT
virtualization works.  The way the hcalls are designed assume a 1-to-1
correspondance between PTEs in the guest view and real hardware PTEs.
It's technically possible, I guess, that we could set up a shadow hash
table beside the guest view of the hash table and populate the former
based on the latter, but it would be a complete PITA.

> > > Does P9 really not have a hugepage that's smaller than 1G?  
> > 
> > It does (2M), but we can't use it in this situation.  As hinted above,
> > POWER9 has two very different MMU modes, hash and radix.  In hash mode
> > (which is similar to POWER8 and earlier CPUs) the hugepage sizes are
> > 16M and 16G, in radix mode (more like x86) they are 2M and 1G.
> > 
> > POWER9 hosts always run in radix mode.  Or at least, we only support
> > running them in radix mode.  We support both radix mode and hash mode
> > guests, the latter including all POWER8 compat mode guests.
> > 
> > The next 

Re: [Qemu-devel] [PATCH v3 01/15] tests: add fp-test, a floating point test suite

[Alex Bennée]

Emilio G. Cota  writes:

> This will allow us to run correctness tests against our
> FP implementation. The test can be run in two modes (called
> "testers"): host and soft. With the former we check the results
> and FP flags on the host machine against the model.
> With the latter we check QEMU's fpu primitives against the
> model. Note that in soft mode we are not instantiating any
> particular CPU (hence the HW_POISON_H hack to avoid macro poisoning);
> for that we need to run the test in host mode under QEMU.


So with the attached patch and my proposed cross build we can now get:

02:15:54 [alex@zen:~/l/q/qemu.git] softfloat-fixes-for-2.12-v1 ± find . -iname 
"fp-test" | xargs file
./ppc64-linux-user/tests/fp-test:  ELF 64-bit LSB executable, 64-bit 
PowerPC or cisco 7500, version 1 (GNU/Linux), statically linked, for GNU/Linux 
3.2.0, not stripped
./armeb-linux-user/tests/fp-test:  ELF 32-bit LSB executable, ARM, EABI5 
version 1 (GNU/Linux), statically linked, for GNU/Linux 3.2.0, not stripped
./aarch64-linux-user/tests/fp-test:ELF 64-bit LSB executable, ARM aarch64, 
version 1 (SYSV), statically linked, for GNU/Linux 3.7.0, not stripped
./i386-linux-user/tests/fp-test:   ELF 32-bit LSB executable, Intel 80386, 
version 1 (GNU/Linux), statically linked, for GNU/Linux 2.6.32, not stripped
./arm-linux-user/tests/fp-test:ELF 32-bit LSB executable, ARM, EABI5 
version 1 (GNU/Linux), statically linked, for GNU/Linux 3.2.0, not stripped
./s390x-linux-user/tests/fp-test:  ELF 64-bit MSB executable, IBM S/390, 
version 1 (SYSV), statically linked, for GNU/Linux 3.2.0, not stripped
./aarch64_be-linux-user/tests/fp-test: ELF 64-bit LSB executable, ARM aarch64, 
version 1 (SYSV), statically linked, for GNU/Linux 3.7.0, not stripped
./tests/fp-test/fp-test:   ELF 64-bit LSB shared object, x86-64, 
version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, 
for GNU/Linux 2.6.32, not stripped

But it did mean having to hack about a little, mainly to get rid of
glib.

--8<---cut here---start->8---
>From 04ed0d9f58f34aa51b9a8284514aab6e36a702b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Benn=C3=A9e?= 
Date: Wed, 11 Apr 2018 01:35:52 +0100
Subject: [PATCH] tests/tcg: add fp-test to per-guest tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The fp-test code was originally designed to be able to include
softfloat. However cross-compiling QEMU based code is harder than it
needs to be so hide softfloat stuff behind USE_SOFTFLOAT. We also need
to tweak:

  - manually include what we need
  - g_assert -> assert()
  - use libc hsearch instead of g_hash_table

Signed-off-by: Alex Bennée 
---
 tests/fp/fp-test.c | 148 ++---
 tests/tcg/Makefile |   3 ++
 2 files changed, 121 insertions(+), 30 deletions(-)

diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
index 27637c4617..4cee2a918c 100644
--- a/tests/fp/fp-test.c
+++ b/tests/fp/fp-test.c
@@ -6,12 +6,72 @@
  * License: GNU GPL, version 2 or later.
  *   See the COPYING file in the top-level directory.
  */
-#ifndef HW_POISON_H
-#error Must define HW_POISON_H to work around TARGET_* poisoning
-#endif
+
+/* If HW_POISON_H isn't defined then we aren't building against qemu's
+ * softfloat */
+#ifdef HW_POISON_H

 #include "qemu/osdep.h"
 #include "fpu/softfloat.h"
+#define USE_SOFTFLOAT 1
+
+#else /* else define what QEMU would have given us */
+
+#define _GNU_SOURCE
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* See include/fpu/softfloat-types.h */
+enum {
+float_tininess_after_rounding  = 0,
+float_tininess_before_rounding = 1
+};
+
+enum {
+float_round_nearest_even = 0,
+float_round_down = 1,
+float_round_up   = 2,
+float_round_to_zero  = 3,
+float_round_ties_away= 4,
+float_round_to_odd   = 5,
+};
+
+enum {
+float_flag_invalid   =  1,
+float_flag_divbyzero =  4,
+float_flag_overflow  =  8,
+float_flag_underflow = 16,
+float_flag_inexact   = 32,
+float_flag_input_denormal = 64,
+float_flag_output_denormal = 128
+};
+
+/* See include/compiler.h */
+#ifndef likely
+#if __GNUC__ < 3
+#define __builtin_expect(x, n) (x)
+#endif
+
+#define likely(x)   __builtin_expect(!!(x), 1)
+#define unlikely(x)   __builtin_expect(!!(x), 0)
+#endif
+
+#endif
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#endif

 #include 
 #include 
@@ -116,16 +176,18 @@ struct tester {
 struct whitelist {
 char **lines;
 size_t n;
-GHashTable *ht;
+struct hsearch_data ht;
 };

 static uint64_t test_stats[ERROR_MAX];
 static struct whitelist whitelist;
 static uint8_t default_exceptions;
 static bool die_on_error = true;
+#ifdef 

[Qemu-devel] [Bug 1762707] Re: VFIO device gets DMA failures when virtio-balloon leak from highmem to lowmem

[Pixel, Ding]

I think we can raise this issue to libvirt. When using virsh or virt-
manager, the memory balloon is still enabled by default even if there's
a device assignment.

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

Title:
  VFIO device gets DMA failures when virtio-balloon leak from highmem to
  lowmem

Status in QEMU:
  Invalid

Bug description:
  Is there any known conflict between VFIO passthrough device and
  virtio-balloon?

  The VM has:
  1. 4GB system memory
  2. one VFIO passthrough device which supports high address memory DMA and 
uses GFP_HIGHUSER pages.
  3. Memory balloon device with 4GB target.

  When setting the memory balloon target to 1GB and 4GB in loop during
  runtime (I used the command "virsh qemu-monitor-command debian --hmp
  --cmd balloon 1024"), the VFIO device DMA randomly gets failure.

  More clues:
  1. configure 2GB system memory (no highmem) VM, no issue with similar 
operations
  2. setting the memory balloon to higher like 8GB, no issue with similar 
operations

  I'm also trying to narrow down this issue. It's appreciated for that
  you guys may share some thoughts.

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

[Qemu-devel] [Bug 1762707] Re: VFIO device gets DMA failures when virtio-balloon leak from highmem to lowmem

[Pixel, Ding]

Hi Alex, Thanks for your confirming. change the status to invalid.

** Changed in: qemu
   Status: New => Invalid

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

Title:
  VFIO device gets DMA failures when virtio-balloon leak from highmem to
  lowmem

Status in QEMU:
  Invalid

Bug description:
  Is there any known conflict between VFIO passthrough device and
  virtio-balloon?

  The VM has:
  1. 4GB system memory
  2. one VFIO passthrough device which supports high address memory DMA and 
uses GFP_HIGHUSER pages.
  3. Memory balloon device with 4GB target.

  When setting the memory balloon target to 1GB and 4GB in loop during
  runtime (I used the command "virsh qemu-monitor-command debian --hmp
  --cmd balloon 1024"), the VFIO device DMA randomly gets failure.

  More clues:
  1. configure 2GB system memory (no highmem) VM, no issue with similar 
operations
  2. setting the memory balloon to higher like 8GB, no issue with similar 
operations

  I'm also trying to narrow down this issue. It's appreciated for that
  you guys may share some thoughts.

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

Re: [Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Richard Henderson]

On 04/11/2018 02:19 AM, Emilio G. Cota wrote:
> Richard pointed out in another thread that when computing
> next_page_start we can break checks for the last page in the
> address space due to integer overflow. This affects several targets;
> the appended fixes them.
> 
> You can fetch the patches from:
>   https://github.com/cota/qemu/tree/next_page_overflow

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH for 2.13 v2 1/2] spapr: Add ibm, max-associativity-domains property

[David Gibson]

On Tue, Apr 10, 2018 at 02:12:25PM -0400, Serhii Popovych wrote:
> Now recent kernels (i.e. since linux-stable commit a346137e9142
> ("powerpc/numa: Use ibm,max-associativity-domains to discover possible nodes")
> support this property to mark initially memory-less NUMA nodes as "possible"
> to allow further memory hot-add to them.
> 
> Advertise this property for pSeries machines to let guest kernels detect
> maximum supported node configuration and benefit from kernel side change
> when hot-add memory to specific, possibly empty before, NUMA node.
> 
> Signed-off-by: Serhii Popovych 
> ---
>  hw/ppc/spapr.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2c0be8c..3f61785 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -909,6 +909,14 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void 
> *fdt)
>  0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
>  cpu_to_be32(max_cpus / smp_threads),
>  };
> +uint32_t maxdomains[] = {
> +cpu_to_be32(5),
> +cpu_to_be32(0),
> +cpu_to_be32(0),
> +cpu_to_be32(0),
> +cpu_to_be32(nb_numa_nodes - 1),
> +cpu_to_be32(0),
> +};

Ah.. close, but not quite right.  This is saying that there's exactly
one node at the bottom (cpu) level, which isn't what we want.  Instead
of setting it to 0, we want to completely drop that layer, keeping it
unspecified.

To do that you need to change the first cell from 5 to 4 (since only 4
levels will be listed) and drop the last cell entirely.

>  _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
>  
> @@ -945,6 +953,9 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void 
> *fdt)
>  _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",
>   refpoints, sizeof(refpoints)));
>  
> +_FDT(fdt_setprop(fdt, rtas, "ibm,max-associativity-domains",
> + maxdomains, sizeof(maxdomains)));
> +
>  _FDT(fdt_setprop_cell(fdt, rtas, "rtas-error-log-max",
>RTAS_ERROR_LOG_MAX));
>  _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate",

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 00/17] Translation loop conversion for sh4/sparc/mips/s390x/openrisc/riscv targets

[Richard Henderson]

On 04/10/2018 11:45 PM, Bastian Koppelmann wrote:
> On 04/10/2018 03:40 PM, Emilio G. Cota wrote:
>> On Tue, Apr 10, 2018 at 15:16:19 +0200, Bastian Koppelmann wrote:
>>> On 04/10/2018 03:03 PM, Emilio G. Cota wrote:
 On Tue, Apr 10, 2018 at 14:24:23 +1000, Richard Henderson wrote:
> On 04/10/2018 02:11 AM, Emilio G. Cota wrote:
>> On Mon, Apr 09, 2018 at 16:01:36 +0200, Bastian Koppelmann wrote:
>>> Thanks for doing this grunt work. Me and a colleague were planning to do
>>> this as well after converting the RISC-V frontend to decodetree. Do you
>>> have any plans to do this for the TriCore frontend as well? I have the
>>> same plan for Tricore: Convert it to decodetree, then to translation 
>>> loop.
>>
>> I won't do any further conversions in the near future, so please go
>> ahead with your plans :-)
>
> In which case I won't review Emilio's final two patches for risc-v.
>>>
>>> I'm confused as well :). I don't understand why Richard is not reviewing
>>> the last two patch. Maybe you can clarify as well, Richard.
>>
>> I think he understood you already had patches (not yet on-list) to do the
>> trloop conversion for riscv, and therefore my patches should be discarded.
> 
> No, I don't have patches.

Ah, right.  Misunderstanding on my part.
I'll get back to the risc-v patches soon.

>> Note that the first patch in this series does change the TranslatorOps
>> interface, that's why I think the whole series should go
>> through the same tree (Richard's TCG tree, I presume).
> 
> Works for me.

That's fine with me too.


r~

[Qemu-devel] [PULL for-2.12 1/1] icount: fix cpu_restore_state_from_tb for non-tb-exit cases

[Richard Henderson]

From: Pavel Dovgalyuk 

In icount mode, instructions that access io memory spaces in the middle
of the translation block invoke TB recompilation.  After recompilation,
such instructions become last in the TB and are allowed to access io
memory spaces.

When the code includes instruction like i386 'xchg eax, 0xd080'
which accesses APIC, QEMU goes into an infinite loop of the recompilation.

This instruction includes two memory accesses - one read and one write.
After the first access, APIC calls cpu_report_tpr_access, which restores
the CPU state to get the current eip.  But cpu_restore_state_from_tb
resets the cpu->can_do_io flag which makes the second memory access invalid.
Therefore the second memory access causes a recompilation of the block.
Then these operations repeat again and again.

This patch moves resetting cpu->can_do_io flag from
cpu_restore_state_from_tb to cpu_loop_exit* functions.

It also adds a parameter for cpu_restore_state which controls restoring
icount.  There is no need to restore icount when we only query CPU state
without breaking the TB.  Restoring it in such cases leads to the
incorrect flow of the virtual time.

In most cases new parameter is true (icount should be recalculated).
But there are two cases in i386 and openrisc when the CPU state is only
queried without the need to break the TB.  This patch fixes both of
these cases.

Signed-off-by: Pavel Dovgalyuk 
Message-Id: <20180409091320.12504.35329.stgit@pasha-VirtualBox>
[rth: Make can_do_io setting unconditional; move from cpu_exec;
make cpu_loop_exit_{noexc,restore} call cpu_loop_exit.]
Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h  |  5 -
 accel/tcg/cpu-exec-common.c  | 10 +-
 accel/tcg/cpu-exec.c |  1 -
 accel/tcg/translate-all.c| 27 ++-
 accel/tcg/user-exec.c|  2 +-
 hw/misc/mips_itu.c   |  3 +--
 target/alpha/helper.c|  2 +-
 target/alpha/mem_helper.c|  6 ++
 target/arm/op_helper.c   |  6 +++---
 target/cris/op_helper.c  |  4 ++--
 target/i386/helper.c |  2 +-
 target/i386/svm_helper.c |  2 +-
 target/m68k/op_helper.c  |  4 ++--
 target/moxie/helper.c|  2 +-
 target/openrisc/sys_helper.c |  8 
 target/tricore/op_helper.c   |  2 +-
 target/xtensa/op_helper.c|  4 ++--
 17 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index e5afd2e6d3..bd68328ed9 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -50,13 +50,16 @@ void cpu_gen_init(void);
  * cpu_restore_state:
  * @cpu: the vCPU state is to be restore to
  * @searched_pc: the host PC the fault occurred at
+ * @will_exit: true if the TB executed will be interrupted after some
+   cpu adjustments. Required for maintaining the correct
+   icount valus
  * @return: true if state was restored, false otherwise
  *
  * Attempt to restore the state for a fault occurring in translated
  * code. If the searched_pc is not in translated code no state is
  * restored and the function returns false.
  */
-bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc);
+bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc, bool will_exit);
 
 void QEMU_NORETURN cpu_loop_exit_noexc(CPUState *cpu);
 void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
index dac5aac477..2988fde650 100644
--- a/accel/tcg/cpu-exec-common.c
+++ b/accel/tcg/cpu-exec-common.c
@@ -27,10 +27,8 @@ bool tcg_allowed;
 /* exit the current TB, but without causing any exception to be raised */
 void cpu_loop_exit_noexc(CPUState *cpu)
 {
-/* XXX: restore cpu registers saved in host registers */
-
 cpu->exception_index = -1;
-siglongjmp(cpu->jmp_env, 1);
+cpu_loop_exit(cpu);
 }
 
 #if defined(CONFIG_SOFTMMU)
@@ -65,15 +63,17 @@ void cpu_reloading_memory_map(void)
 
 void cpu_loop_exit(CPUState *cpu)
 {
+/* Undo the setting in cpu_tb_exec.  */
+cpu->can_do_io = 1;
 siglongjmp(cpu->jmp_env, 1);
 }
 
 void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc)
 {
 if (pc) {
-cpu_restore_state(cpu, pc);
+cpu_restore_state(cpu, pc, true);
 }
-siglongjmp(cpu->jmp_env, 1);
+cpu_loop_exit(cpu);
 }
 
 void cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 9cc697205c..81153e7a13 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -704,7 +704,6 @@ int cpu_exec(CPUState *cpu)
 g_assert(cpu == current_cpu);
 g_assert(cc == CPU_GET_CLASS(cpu));
 #endif /* buggy compiler */
-cpu->can_do_io = 1;
 tb_lock_reset();
 if (qemu_mutex_iothread_locked()) {
 qemu_mutex_unlock_iothread();
diff --git a/accel/tcg/translate-all.c 

[Qemu-devel] [PULL for-2.12 0/1] tcg patch queue

[Richard Henderson]

The following changes since commit 26d6a7c87b05017ffabffb5e16837a0fccf67e90:

  Merge remote-tracking branch 'remotes/ericb/tags/pull-qapi-2018-04-10' into 
staging (2018-04-10 22:16:19 +0100)

are available in the Git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20180411

for you to fetch changes up to afd46fcad2dceffda35c0586f5723c127b6e09d8:

  icount: fix cpu_restore_state_from_tb for non-tb-exit cases (2018-04-11 
09:05:22 +1000)


Handle read-modify-write i/o with icount


Pavel Dovgalyuk (1):
  icount: fix cpu_restore_state_from_tb for non-tb-exit cases

 include/exec/exec-all.h  |  5 -
 accel/tcg/cpu-exec-common.c  | 10 +-
 accel/tcg/cpu-exec.c |  1 -
 accel/tcg/translate-all.c| 27 ++-
 accel/tcg/user-exec.c|  2 +-
 hw/misc/mips_itu.c   |  3 +--
 target/alpha/helper.c|  2 +-
 target/alpha/mem_helper.c|  6 ++
 target/arm/op_helper.c   |  6 +++---
 target/cris/op_helper.c  |  4 ++--
 target/i386/helper.c |  2 +-
 target/i386/svm_helper.c |  2 +-
 target/m68k/op_helper.c  |  4 ++--
 target/moxie/helper.c|  2 +-
 target/openrisc/sys_helper.c |  8 
 target/tricore/op_helper.c   |  2 +-
 target/xtensa/op_helper.c|  4 ++--
 17 files changed, 45 insertions(+), 45 deletions(-)

Re: [Qemu-devel] [PATCH v2 06/17] target/mips: convert to DisasJumpType

[Richard Henderson]

On 04/11/2018 12:23 AM, Emilio G. Cota wrote:
>  case DISAS_STOP:
> -gen_goto_tb(, 0, ctx.pc);
> +tcg_gen_lookup_and_goto_ptr();

You need to write ctx.pc back to the pc first, e.g.

gen_save_pc(ctx.pc);
tcg_gen_lookup_and_goto_ptr();


r~

Re: [Qemu-devel] [Bug 1760262] Re: cmsdk-apb-uart doesn't appear to clear interrupt flags

[Patrick Oppenlander]

On Tue, Apr 10, 2018 at 11:45 PM, Peter Maydell
 wrote:
>
> Thanks for the bug report; I've submitted this patch (which is similar to but 
> not quite the same as your fix):
> https://patchwork.ozlabs.org/patch/896715/
>
> Hopefully this will get into 2.12, but we're quite close to release now
> so it will depend on whether we need to spin an extra release candidate
> for some other reason.

Thanks for looking into it.

Patrick

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

Title:
  cmsdk-apb-uart doesn't appear to clear interrupt flags

Status in QEMU:
  In Progress

Bug description:
  I have been writing a small operating system and using QEMU emulating
  the mps2-an385 board for some of my testing.

  During development of the uart driver I observed some odd behaviour
  with the TX interrupt -- writing a '1' to bit 0 of the INTCLEAR
  register doesn't clear the TX interrupt flag, and the interrupt fires
  continuously.

  It's possible that I have an error somewhere in my code, but after
  inspecting the QEMU source it does appear to be a QEMU bug. I applied
  the following patch and it solved my issue:

  From 9875839c144fa60a3772f16ae44d32685f9328aa Mon Sep 17 00:00:00 2001
  From: Patrick Oppenlander 
  Date: Sat, 31 Mar 2018 15:10:28 +1100
  Subject: [PATCH] hw/char/cmsdk-apb-uart: fix clearing of interrupt flags

  ---
   hw/char/cmsdk-apb-uart.c | 1 +
   1 file changed, 1 insertion(+)

  diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c
  index 1ad1e14295..64991bd9d7 100644
  --- a/hw/char/cmsdk-apb-uart.c
  +++ b/hw/char/cmsdk-apb-uart.c
  @@ -274,6 +274,7 @@ static void uart_write(void *opaque, hwaddr offset, 
uint64_t value,
* is then reflected into the intstatus value by the update 
function).
*/
   s->state &= ~(value & (R_INTSTATUS_TXO_MASK | R_INTSTATUS_RXO_MASK));
  +s->intstatus &= ~(value & ~(R_INTSTATUS_TXO_MASK | 
R_INTSTATUS_RXO_MASK));
   cmsdk_apb_uart_update(s);
   break;
   case A_BAUDDIV:
  -- 
  2.16.2

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

[Qemu-devel] [PATCH v6 9/9] i386: Remove generic SMT thread check

[Babu Moger]

Remove generic non-intel check while validating hyperthreading support.
Certain AMD CPUs can support hyperthreading now.

CPU family with TOPOEXT feature can support hyperthreading now.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index bfe24b9..077364a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4822,17 +4822,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 
 qemu_init_vcpu(cs);
 
-/* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
- * issue by adjusting CPUID__0001_EBX and CPUID_8000_0008_ECX
- * based on inputs (sockets,cores,threads), it is still better to gives
+/* Most Intel and certain AMD CPUs support hyperthreading. Even though QEMU
+ * fixes this issue by adjusting CPUID__0001_EBX and 
CPUID_8000_0008_ECX
+ * based on inputs (sockets,cores,threads), it is still better to give
  * users a warning.
  *
  * NOTE: the following code has to follow qemu_init_vcpu(). Otherwise
  * cs->nr_threads hasn't be populated yet and the checking is incorrect.
  */
-if (!IS_INTEL_CPU(env) && cs->nr_threads > 1 && !ht_warned) {
-error_report("AMD CPU doesn't support hyperthreading. Please configure"
- " -smp options properly.");
+ if (IS_AMD_CPU(env) &&
+ !(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
+ cs->nr_threads > 1 && !ht_warned) {
+error_report("This family of AMD CPU doesn't support "
+ "hyperthreading. Please configure -smp "
+ "options properly.");
 ht_warned = true;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 7/9] i386: Add support for CPUID_8000_001E for AMD

[Babu Moger]

Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
feature is supported. This is required to support hyperthreading feature
on AMD CPUs. This is supported via CPUID_8000_001E extended functions.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 63f2d31..24b39c6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -315,6 +315,12 @@ static uint32_t encode_cache_cpuid8005(CPUCacheInfo 
*cache)
  (((CORES_IN_CMPLX - 1) * 2) + 1)  : \
  (CORES_IN_CMPLX - 1))
 
+/* Definitions used on CPUID Leaf 0x801E */
+#define EXTENDED_APIC_ID(threads, socket_id, core_id, thread_id) \
+((threads) ? \
+ ((socket_id << 6) | (core_id << 1) | thread_id) : \
+ ((socket_id << 6) | core_id))
+
 /*
  * Encode cache info for CPUID[0x8006].ECX and CPUID[0x8006].EDX
  * @l3 can be NULL.
@@ -4098,6 +4104,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 break;
 }
 break;
+case 0x801E:
+assert(cpu->core_id <= 255);
+*eax = EXTENDED_APIC_ID((cs->nr_threads - 1),
+   cpu->socket_id, cpu->core_id, cpu->thread_id);
+*ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
+*ecx = cpu->socket_id;
+*edx = 0;
+break;
 case 0xC000:
 *eax = env->cpuid_xlevel2;
 *ebx = 0;
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 5/9] i386: Use the statically loaded cache definitions

[Babu Moger]

Use the statically loaded cache definitions if available
and legacy-cache parameter is not set.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1659320..76b3bc9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3934,8 +3934,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
(L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
 *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | \
(L1_ITLB_4K_ASSOC <<  8) | (L1_ITLB_4K_ENTRIES);
-*ecx = encode_cache_cpuid8005(_cache_amd);
-*edx = encode_cache_cpuid8005(_cache_amd);
+if (env->cache_info.valid && !cpu->legacy_cache) {
+*ecx = encode_cache_cpuid8005(>cache_info.l1d_cache);
+*edx = encode_cache_cpuid8005(>cache_info.l1i_cache);
+} else {
+*ecx = encode_cache_cpuid8005(_cache_amd);
+*edx = encode_cache_cpuid8005(_cache_amd);
+}
 break;
 case 0x8006:
 /* cache info (L2 cache) */
@@ -3951,9 +3956,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
(L2_DTLB_4K_ENTRIES << 16) | \
(AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) | \
(L2_ITLB_4K_ENTRIES);
-encode_cache_cpuid8006(_cache_amd,
-   cpu->enable_l3_cache ? _cache : NULL,
-   ecx, edx);
+if (env->cache_info.valid && !cpu->legacy_cache) {
+encode_cache_cpuid8006(>cache_info.l2_cache,
+   cpu->enable_l3_cache ?
+   >cache_info.l3_cache : NULL,
+   ecx, edx);
+} else {
+encode_cache_cpuid8006(_cache_amd,
+   cpu->enable_l3_cache ? _cache : NULL,
+   ecx, edx);
+}
 break;
 case 0x8007:
 *eax = 0;
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 3/9] i386: Initialize cache information for EPYC family processors

[Babu Moger]

Initialize pre-determined cache information for EPYC processors.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 96 +++
 1 file changed, 96 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8c84fa2..3b2a19a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2295,6 +2295,54 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_6_EAX_ARAT,
 .xlevel = 0x800A,
 .model_id = "AMD EPYC Processor",
+.cache_info.valid = 1,
+.cache_info.l1d_cache = {
+.type = DCACHE,
+.level = 1,
+.size = 32 * KiB,
+.line_size = 64,
+.associativity = 8,
+.partitions = 1,
+.sets = 64,
+.lines_per_tag = 1,
+.self_init = 1,
+.no_invd_sharing = true,
+},
+.cache_info.l1i_cache = {
+.type = ICACHE,
+.level = 1,
+.size = 64 * KiB,
+.line_size = 64,
+.associativity = 4,
+.partitions = 1,
+.sets = 256,
+.lines_per_tag = 1,
+.self_init = 1,
+.no_invd_sharing = true,
+},
+.cache_info.l2_cache = {
+.type = UNIFIED_CACHE,
+.level = 2,
+.size = 512 * KiB,
+.line_size = 64,
+.associativity = 8,
+.partitions = 1,
+.sets = 1024,
+.lines_per_tag = 1,
+},
+.cache_info.l3_cache = {
+.type = UNIFIED_CACHE,
+.level = 3,
+.size = 8 * MiB,
+.line_size = 64,
+.associativity = 16,
+.partitions = 1,
+.sets = 8192,
+.lines_per_tag = 1,
+.self_init = true,
+.inclusive = true,
+.complex_indexing = true,
+},
 },
 {
 .name = "EPYC-IBPB",
@@ -2341,6 +2389,54 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_6_EAX_ARAT,
 .xlevel = 0x800A,
 .model_id = "AMD EPYC Processor (with IBPB)",
+.cache_info.valid = 1,
+.cache_info.l1d_cache = {
+.type = DCACHE,
+.level = 1,
+.size = 32 * KiB,
+.line_size = 64,
+.associativity = 8,
+.partitions = 1,
+.sets = 64,
+.lines_per_tag = 1,
+.self_init = 1,
+.no_invd_sharing = true,
+},
+.cache_info.l1i_cache = {
+.type = ICACHE,
+.level = 1,
+.size = 64 * KiB,
+.line_size = 64,
+.associativity = 4,
+.partitions = 1,
+.sets = 256,
+.lines_per_tag = 1,
+.self_init = 1,
+.no_invd_sharing = true,
+},
+.cache_info.l2_cache = {
+.type = UNIFIED_CACHE,
+.level = 2,
+.size = 512 * KiB,
+.line_size = 64,
+.associativity = 8,
+.partitions = 1,
+.sets = 1024,
+.lines_per_tag = 1,
+},
+.cache_info.l3_cache = {
+.type = UNIFIED_CACHE,
+.level = 3,
+.size = 8 * MiB,
+.line_size = 64,
+.associativity = 16,
+.partitions = 1,
+.sets = 8192,
+.lines_per_tag = 1,
+.self_init = true,
+.inclusive = true,
+.complex_indexing = true,
+},
 },
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 6/9] i386: Populate AMD Processor Cache Information for cpuid 0x8000001D

[Babu Moger]

Add information for cpuid 0x801D leaf. Populate cache topology information
for different cache types(Data Cache, Instruction Cache, L2 and L3) supported
by 0x801D leaf. Please refer Processor Programming Reference (PPR) for AMD
Family 17h Model for more details.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 92 +++
 target/i386/kvm.c | 29 --
 2 files changed, 118 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 76b3bc9..63f2d31 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -307,6 +307,14 @@ static uint32_t encode_cache_cpuid8005(CPUCacheInfo 
*cache)
   a == ASSOC_FULL ? 0xF : \
   0 /* invalid value */)
 
+/* Definitions used on CPUID Leaf 0x801D */
+/* Number of logical cores in a complex */
+#define CORES_IN_CMPLX  4
+/* Number of logical processors sharing cache */
+#define NUM_SHARING_CACHE(threads)   (threads ? \
+ (((CORES_IN_CMPLX - 1) * 2) + 1)  : \
+ (CORES_IN_CMPLX - 1))
+
 /*
  * Encode cache info for CPUID[0x8006].ECX and CPUID[0x8006].EDX
  * @l3 can be NULL.
@@ -336,6 +344,41 @@ static void encode_cache_cpuid8006(CPUCacheInfo *l2,
 }
 }
 
+/* Encode cache info for CPUID[801D] */
+static void encode_cache_cpuid801d(CPUCacheInfo *cache, int nr_threads,
+uint32_t *eax, uint32_t *ebx,
+uint32_t *ecx, uint32_t *edx)
+{
+assert(cache->size == cache->line_size * cache->associativity *
+  cache->partitions * cache->sets);
+
+*eax = CACHE_TYPE(cache->type) | CACHE_LEVEL(cache->level) |
+   (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0);
+
+/* L3 is shared among multiple cores */
+if (cache->level == 3) {
+*eax |= (NUM_SHARING_CACHE(nr_threads - 1) << 14);
+} else {
+*eax |= ((nr_threads - 1) << 14);
+}
+
+assert(cache->line_size > 0);
+assert(cache->partitions > 0);
+assert(cache->associativity > 0);
+/* We don't implement fully-associative caches */
+assert(cache->associativity < cache->sets);
+*ebx = (cache->line_size - 1) |
+   ((cache->partitions - 1) << 12) |
+   ((cache->associativity - 1) << 22);
+
+assert(cache->sets > 0);
+*ecx = cache->sets - 1;
+
+*edx = (cache->no_invd_sharing ? CACHE_NO_INVD_SHARING : 0) |
+   (cache->inclusive ? CACHE_INCLUSIVE : 0) |
+   (cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
+}
+
 /* Definitions of the hardcoded cache entries we expose: */
 
 /* L1 data cache: */
@@ -4006,6 +4049,55 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *edx = 0;
 }
 break;
+case 0x801D:
+*eax = 0;
+switch (count) {
+case 0: /* L1 dcache info */
+if (env->cache_info.valid && !cpu->legacy_cache) {
+encode_cache_cpuid801d(>cache_info.l1d_cache,
+   cs->nr_threads,
+   eax, ebx, ecx, edx);
+} else {
+encode_cache_cpuid801d(_cache_amd, cs->nr_threads,
+   eax, ebx, ecx, edx);
+}
+break;
+case 1: /* L1 icache info */
+if (env->cache_info.valid && !cpu->legacy_cache) {
+encode_cache_cpuid801d(>cache_info.l1i_cache,
+   cs->nr_threads,
+   eax, ebx, ecx, edx);
+} else {
+encode_cache_cpuid801d(_cache_amd,
+   cs->nr_threads,
+   eax, ebx, ecx, edx);
+}
+break;
+case 2: /* L2 cache info */
+if (env->cache_info.valid && !cpu->legacy_cache) {
+encode_cache_cpuid801d(>cache_info.l2_cache,
+   cs->nr_threads,
+   eax, ebx, ecx, edx);
+} else {
+encode_cache_cpuid801d(_cache_amd, cs->nr_threads,
+   eax, ebx, ecx, edx);
+}
+break;
+case 3: /* L3 cache info */
+if (env->cache_info.valid && !cpu->legacy_cache) {
+encode_cache_cpuid801d(>cache_info.l3_cache,
+   cs->nr_threads,
+   eax, ebx, ecx, edx);
+} else {
+encode_cache_cpuid801d(_cache, cs->nr_threads,
+   eax, ebx, ecx, edx);
+}
+break;
+

[Qemu-devel] [PATCH v6 4/9] i386: Add new property to control cache info

[Babu Moger]

This will be used to control the cache information.
By default new information will be displayed. If user
passes "-cpu legacy-cache" then older information will
be displayed even if the hardware supports new information.
It will be "on" for machine type "pc-q35-2.10" for compatibility.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 include/hw/i386/pc.h | 4 
 target/i386/cpu.c| 1 +
 target/i386/cpu.h| 5 +
 3 files changed, 10 insertions(+)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ffee841..d904a3c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -327,6 +327,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 .driver   = "q35-pcihost",\
 .property = "x-pci-hole64-fix",\
 .value= "off",\
+},{\
+.driver   = TYPE_X86_CPU,\
+.property = "legacy-cache",\
+.value= "on",\
 },
 
 #define PC_COMPAT_2_9 \
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3b2a19a..1659320 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5129,6 +5129,7 @@ static Property x86_cpu_properties[] = {
  false),
 DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true),
 DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
+DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false),
 
 /*
  * From "Requirements for Implementing the Microsoft
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index aff8396..fe8edf6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1394,6 +1394,11 @@ struct X86CPU {
  */
 bool enable_l3_cache;
 
+/* Compatibility bits for old machine types.
+ * If true present the old cache topology information
+ */
+bool legacy_cache;
+
 /* Compatibility bits for old machine types: */
 bool enable_cpuid_0xb;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 8/9] i386: Enable TOPOEXT feature on AMD EPYC CPU

[Babu Moger]

Enable TOPOEXT feature on EPYC CPU. This is required to support
hyperthreading on VM guests. Also extend xlevel to 0x801E.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 24b39c6..bfe24b9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2327,7 +2327,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .features[FEAT_8000_0001_ECX] =
 CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
 CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
-CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+CPUID_EXT3_TOPOEXT,
 .features[FEAT_7_0_EBX] =
 CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 |
 CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
@@ -2419,7 +2420,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .features[FEAT_8000_0001_ECX] =
 CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
 CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
-CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+CPUID_EXT3_TOPOEXT,
 .features[FEAT_8000_0008_EBX] =
 CPUID_8000_0008_EBX_IBPB,
 .features[FEAT_7_0_EBX] =
@@ -4572,6 +4574,11 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error 
**errp)
 x86_cpu_adjust_level(cpu, >cpuid_min_xlevel, 0x800A);
 }
 
+/* TOPOEXT feature requires 0x801E */
+if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
+x86_cpu_adjust_level(cpu, >cpuid_min_xlevel, 0x801E);
+}
+
 /* SEV requires CPUID[0x801F] */
 if (sev_enabled()) {
 x86_cpu_adjust_level(cpu, >cpuid_min_xlevel, 0x801F);
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 2/9] i386: Add cache information in X86CPUDefinition

[Babu Moger]

Add cache information in X86CPUDefinition and CPUX86State.

Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 4 
 target/i386/cpu.h | 8 
 2 files changed, 12 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2d3d7d8..8c84fa2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1098,6 +1098,7 @@ struct X86CPUDefinition {
 int stepping;
 FeatureWordArray features;
 const char *model_id;
+CPUCaches cache_info;
 };
 
 static X86CPUDefinition builtin_x86_defs[] = {
@@ -3235,6 +3236,9 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 env->features[w] = def->features[w];
 }
 
+/* Load Cache information from the X86CPUDefinition */
+memcpy(>cache_info, >cache_info, sizeof(CPUCaches));
+
 /* Special cases not set in the X86CPUDefinition structs: */
 /* TODO: in-kernel irqchip for hvf */
 if (kvm_enabled()) {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index eaed287..aff8396 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1096,6 +1096,13 @@ typedef struct CPUCacheInfo {
 } CPUCacheInfo;
 
 
+typedef struct CPUCaches {
+bool valid;
+CPUCacheInfo l1d_cache;
+CPUCacheInfo l1i_cache;
+CPUCacheInfo l2_cache;
+CPUCacheInfo l3_cache;
+} CPUCaches;
 
 typedef struct CPUX86State {
 /* standard registers */
@@ -1282,6 +1289,7 @@ typedef struct CPUX86State {
 /* Features that were explicitly enabled/disabled */
 FeatureWordArray user_features;
 uint32_t cpuid_model[12];
+CPUCaches cache_info;
 
 /* MTRRs */
 uint64_t mtrr_fixed[11];
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 0/9] i386: Enable TOPOEXT to support hyperthreading on AMD CPU

[Babu Moger]

This series enables the TOPOEXT feature for AMD CPUs. This is required to
support hyperthreading on kvm guests.

This addresses the issues reported in these bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1481253
https://bugs.launchpad.net/qemu/+bug/1703506 

v6:
1.Fixed problem with patch#4(Add new property to control cache info). The 
parameter
 legacy_cache should be "on" by default on machine type "pc-q35-2.10". This was
 found by Alexandr Iarygin.
2.Fixed the l3 cache size for EPYC based machines(patch#3). Also, fixed the 
number of
 logical processors sharing the cache(patch#6). Only L3 cache is shared by 
multiple
 cores but not L1 or L2. This was a bug while decoding. This was found by 
Geoffrey McRae
 and he verified the fix. 

v5:
 In this series I tried to address the feedback from Eduardo Habkost.
 The discussion thread is here.
 https://patchwork.kernel.org/patch/10299745/
 The previous thread is here.
 http://patchwork.ozlabs.org/cover/884885/

Reason for these changes.
 The cache properties for AMD family of processors have changed from
 previous releases. We don't want to display the new information on the
 old family of processors as this might cause compatibility issues.

Changes:
1.Based the patches on top of Eduardo's(patch#1) patch.
  Changed few things.
  Moved the Cache definitions to cpu.h file.
  Changed the CPUID_4 names to generic names.
2.Added a new propery "legacy-cache" in cpu object(patch#2). This can be
  used to display the old property even if the host supports the new cache
  properties.
3.Added cache information in X86CPUDefinition and CPUX86State
4.Patch 6-7 changed quite a bit from previous version does to new approach.
5.Addressed few issues with CPUID_8000_001d and CPUID_8000_001E.

v4:
1.Removed the checks under cpuid 0x801D leaf(patch #2). These check are
  not necessary. Found this during internal review.
2.Added CPUID_EXT3_TOPOEXT feature for all the 17 family(patch #4). This was
  found by Kash Pande during his testing.
3.Removed th hardcoded cpuid xlevel and dynamically extended if 
CPUID_EXT3_TOPOEXT
  is supported(Suggested by Brijesh Singh). 

v3:
1.Removed the patch #1. Radim mentioned that original typo problem is in 
  linux kernel header. qemu is just copying those files.
2.In previous version, I used the cpuid 4 definitions for AMDs cpuid leaf
  0x801D. CPUID 4 is very intel specific and we dont want to expose those
  details under AMD. I have renamed some of these definitions as generic.
  These changes are in patch#1. Radim, let me know if this is what you intended.
3.Added assert to for core_id(Suggested by Radim Kr??m).
4.Changed the if condition under "L3 cache info"(Suggested by Gary Hook).
5.Addressed few more text correction and code cleanup(Suggested by Thomas 
Lendacky).

v2:
  Fixed few more minor issues per Gary Hook's comments. Thank you Gary.
  Removed the patch#1. We need to handle the instruction cache associativity 
  seperately. It varies based on the cpu family. I will comeback to that later.
  Added two more typo corrections in patch#1 and patch#5.

v1:
  Stanislav Lanci posted few patches earlier. 
  https://patchwork.kernel.org/patch/10040903/

Rebased his patches with few changes.
1.Spit the patches into two, separating cpuid functions 
  0x801D and 0x801E (Patch 2 and 3).
2.Removed the generic non-intel check and made a separate patch
  with some changes(Patch 5).
3.Fixed L3_N_SETS_AMD(from 4096 to 8192) based on CPUID_Fn801D_ECX_x03.

Added 2 more patches.
Patch 1. Fixes cache associativity.
Patch 4. Adds TOPOEXT feature on AMD EPYC CPU.


Babu Moger (8):
  i386: Add cache information in X86CPUDefinition
  i386: Initialize cache information for EPYC family processors
  i386: Add new property to control cache info
  i386: Use the statically loaded cache definitions
  i386: Populate AMD Processor Cache Information for cpuid 0x801D
  i386: Add support for CPUID_8000_001E for AMD
  i386: Enable TOPOEXT feature on AMD EPYC CPU
  i386: Remove generic SMT thread check

Eduardo Habkost (1):
  i386: Helpers to encode cache information consistently

 include/hw/i386/pc.h |   4 +
 target/i386/cpu.c| 736 ++-
 target/i386/cpu.h|  66 +
 target/i386/kvm.c|  29 +-
 4 files changed, 702 insertions(+), 133 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v6 1/9] i386: Helpers to encode cache information consistently

[Babu Moger]

From: Eduardo Habkost 

Instead of having a collection of macros that need to be used in
complex expressions to build CPUID data, define a CPUCacheInfo
struct that can hold information about a given cache.  Helper
functions will take a CPUCacheInfo struct as input to encode
CPUID leaves for a cache.

This will help us ensure consistency between cache information
CPUID leaves, and make the existing inconsistencies in CPUID info
more visible.

Signed-off-by: Eduardo Habkost 
Signed-off-by: Babu Moger 
Tested-by: Geoffrey McRae 
---
 target/i386/cpu.c | 495 --
 target/i386/cpu.h |  53 ++
 2 files changed, 424 insertions(+), 124 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 555ae79..2d3d7d8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -56,33 +56,240 @@
 
 #include "disas/capstone.h"
 
+/* Helpers for building CPUID[2] descriptors: */
+
+struct CPUID2CacheDescriptorInfo {
+enum CacheType type;
+int level;
+int size;
+int line_size;
+int associativity;
+};
 
-/* Cache topology CPUID constants: */
+#define KiB 1024
+#define MiB (1024 * 1024)
 
-/* CPUID Leaf 2 Descriptors */
+/*
+ * Known CPUID 2 cache descriptors.
+ * From Intel SDM Volume 2A, CPUID instruction
+ */
+struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = {
+[0x06] = { .level = 1, .type = ICACHE,.size =   8 * KiB,
+   .associativity = 4,  .line_size = 32, },
+[0x08] = { .level = 1, .type = ICACHE,.size =  16 * KiB,
+   .associativity = 4,  .line_size = 32, },
+[0x09] = { .level = 1, .type = ICACHE,.size =  32 * KiB,
+   .associativity = 4,  .line_size = 64, },
+[0x0A] = { .level = 1, .type = DCACHE,.size =   8 * KiB,
+   .associativity = 2,  .line_size = 32, },
+[0x0C] = { .level = 1, .type = DCACHE,.size =  16 * KiB,
+   .associativity = 4,  .line_size = 32, },
+[0x0D] = { .level = 1, .type = DCACHE,.size =  16 * KiB,
+   .associativity = 4,  .line_size = 64, },
+[0x0E] = { .level = 1, .type = DCACHE,.size =  24 * KiB,
+   .associativity = 6,  .line_size = 64, },
+[0x1D] = { .level = 2, .type = UNIFIED_CACHE, .size = 128 * KiB,
+   .associativity = 2,  .line_size = 64, },
+[0x21] = { .level = 2, .type = UNIFIED_CACHE, .size = 256 * KiB,
+   .associativity = 8,  .line_size = 64, },
+/* lines per sector is not supported cpuid2_cache_descriptor(),
+* so descriptors 0x22, 0x23 are not included
+*/
+[0x24] = { .level = 2, .type = UNIFIED_CACHE, .size =   1 * MiB,
+   .associativity = 16, .line_size = 64, },
+/* lines per sector is not supported cpuid2_cache_descriptor(),
+* so descriptors 0x25, 0x20 are not included
+*/
+[0x2C] = { .level = 1, .type = DCACHE,.size =  32 * KiB,
+   .associativity = 8,  .line_size = 64, },
+[0x30] = { .level = 1, .type = ICACHE,.size =  32 * KiB,
+   .associativity = 8,  .line_size = 64, },
+[0x41] = { .level = 2, .type = UNIFIED_CACHE, .size = 128 * KiB,
+   .associativity = 4,  .line_size = 32, },
+[0x42] = { .level = 2, .type = UNIFIED_CACHE, .size = 256 * KiB,
+   .associativity = 4,  .line_size = 32, },
+[0x43] = { .level = 2, .type = UNIFIED_CACHE, .size = 512 * KiB,
+   .associativity = 4,  .line_size = 32, },
+[0x44] = { .level = 2, .type = UNIFIED_CACHE, .size =   1 * MiB,
+   .associativity = 4,  .line_size = 32, },
+[0x45] = { .level = 2, .type = UNIFIED_CACHE, .size =   2 * MiB,
+   .associativity = 4,  .line_size = 32, },
+[0x46] = { .level = 3, .type = UNIFIED_CACHE, .size =   4 * MiB,
+   .associativity = 4,  .line_size = 64, },
+[0x47] = { .level = 3, .type = UNIFIED_CACHE, .size =   8 * MiB,
+   .associativity = 8,  .line_size = 64, },
+[0x48] = { .level = 2, .type = UNIFIED_CACHE, .size =   3 * MiB,
+   .associativity = 12, .line_size = 64, },
+/* Descriptor 0x49 depends on CPU family/model, so it is not included */
+[0x4A] = { .level = 3, .type = UNIFIED_CACHE, .size =   6 * MiB,
+   .associativity = 12, .line_size = 64, },
+[0x4B] = { .level = 3, .type = UNIFIED_CACHE, .size =   8 * MiB,
+   .associativity = 16, .line_size = 64, },
+[0x4C] = { .level = 3, .type = UNIFIED_CACHE, .size =  12 * MiB,
+   .associativity = 12, .line_size = 64, },
+[0x4D] = { .level = 3, .type = UNIFIED_CACHE, .size =  16 * MiB,
+   .associativity = 16, .line_size = 64, },
+[0x4E] = { .level = 2, .type = UNIFIED_CACHE, .size =   6 * MiB,
+   .associativity = 24, .line_size = 64, },
+[0x60] = { .level = 1, .type = DCACHE,  

Re: [Qemu-devel] [PULL 0/3] QAPI patches for 2018-04-10, 2.12-rc3

[Peter Maydell]

On 10 April 2018 at 14:21, Eric Blake  wrote:
> The following changes since commit fb4fe32d5b6290deabe752b51cc1cc2a9e8573db:
>
>   Merge remote-tracking branch 'remotes/xtensa/tags/20180409-xtensa' into 
> staging (2018-04-10 10:22:45 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/ericb.git tags/pull-qapi-2018-04-10
>
> for you to fetch changes up to 951702f39c7e31530ce4ea65b11fe25ea1ea9c37:
>
>   monitor: bind dispatch bh to iohandler context (2018-04-10 07:49:43 -0500)
>
> Hopefully the last of the fixes for regressions caused by OOB which
> could be observed even when x-oob=on is not used.
>
> 
> qapi patches for 2018-04-10
>
> - Peter Xu: iotests: fix wait_until_completed()
> - Peter Xu: iothread: workaround glib bug which hangs qmp-test
> - Peter Xu: monitor: bind dispatch bh to iohandler context
>
> 

Applied, thanks.

-- PMM

[Qemu-devel] [Bug 1036363] Re: Major network performance problems on AMD hardware

[Thomas Huth]

Looking through old bug tickets... can you still reproduce this issue
with the latest version of QEMU? Or could we close this ticket nowadays?

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

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

Title:
  Major network performance problems on AMD hardware

Status in QEMU:
  Incomplete
Status in qemu-kvm:
  New

Bug description:
  Hi,

  I am experiencing some major performance problems with all of our
  beefy AMD Opteron 6274 servers running Fedora 17 (kernel
  3.4.4-5.fc17.x86_64, qemu 1.0-17).  The network performance between
  host and the virtual machine is terrible:

  # iperf -c 10.10.11.22 -r
  
  Server listening on TCP port 5001
  TCP window size: 85.3 KByte (default)
  
  
  Client connecting to 10.10.11.22, TCP port 5001
  TCP window size:  197 KByte (default)
  
  [  5] local 10.10.11.199 port 44192 connected with 10.10.11.22 port 5001
  [ ID] Interval   Transfer Bandwidth
  [  5]  0.0-10.0 sec  2.45 GBytes  2.11 Gbits/sec
  [  4] local 10.10.11.199 port 5001 connected with 10.10.11.22 port 42601
  [  4]  0.0-10.0 sec  8.97 GBytes  7.71 Gbits/sec

  So the VM's receive is super slow.  I would be happy with 7.71 Gbps
  because it's closer to matching the speed of the 10G ethernet adapters
  but the iSCSI drive's write performance is few times faster than read.
  Now running a similar test on the slowest machine I have, Intel core
  i3 I see this:

  # iperf -c 192.168.7.60 -r
  
  Server listening on TCP port 5001
  TCP window size: 85.3 KByte (default)
  
  
  Client connecting to 192.168.7.60, TCP port 5001
  TCP window size:  306 KByte (default)
  
  [  5] local 192.168.7.98 port 53992 connected with 192.168.7.60 port 5001
  [ ID] Interval   Transfer Bandwidth
  [  5]  0.0-10.0 sec  22.5 GBytes  19.3 Gbits/sec
  [  4] local 192.168.7.98 port 5001 connected with 192.168.7.60 port 53339
  [  4]  0.0-10.0 sec  25.1 GBytes  21.5 Gbits/sec

  As you can image this is a huge difference in network IO.  Most setups
  are identical down to the same versions.  Vhost-net is enabled and it
  appears to use MSI-X on the VM.  I've tried all kinds of settings and
  while they improve performance a little I feel it's just masking a
  bigger problem.  All 12 of my AMD servers have this issue and it
  appears I'm not the only one complaining.  Any help would be
  appreciated.  Thanks.

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

[Qemu-devel] [Bug 1192065] Re: qemu release memory to system

[Thomas Huth]

Looking through old bug tickets... I think you should rather use
hotpluggable memory here for the guests instead - or use a big swap
partition on the host. Anyway, this is not a bug, so closing this ticket
now.

** Changed in: qemu
   Status: New => Invalid

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

Title:
  qemu release memory to system

Status in QEMU:
  Invalid

Bug description:
  Qemu pre-allocates the maximum balloon amount which is inconvinient if
  all of the memory is used up and some other system needs to be added
  memory resource

  eg:- I have 4GB RAM with 4 virtual systems to be run.
  I want each of them to start with 1GB RAM with maximum 2GB possible. This is 
not achievable since qemu pre-allocates the maximum balloon amount which is 
2GBx4 systems . So to start all 4 systems the system needs 8GB RAM rather than 
4GB RAM to start with although I have told initial balloon amount to be 1GB.

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

Re: [Qemu-devel] [PATCH] qmp: add pmemload command

[Eric Blake]

On 04/10/2018 04:24 PM, Simon Ruderich wrote:
> Adapted patch from Baojun Wang [1] with the following commit message:
> 
> I found this could be useful to have qemu-softmmu as a cross
> debugger (launch with -s -S command line option), then if we can
> have a command to load guest physical memory, we can use cross gdb
> to do some target debug which gdb cannot do directly.
> 
> pmemload is necessary to directly write physical memory which is not
> possible with gdb alone as it uses only logical addresses.
> 
> [1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00073.html
> 
> Based-on-patch-by: Baojun Wang 
> Signed-off-by: Simon Ruderich 
> ---

> +void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
> +  Error **errp)
> +{
> +FILE *f;
> +size_t l;
> +uint8_t buf[1024];
> +
> +f = fopen(filename, "rb");

Use qemu_fopen() here, so that you can automagically support /dev/fdset/
magic filenames that work on file descriptors passed via SCM_RIGHTS.


> +++ b/qapi-schema.json
> @@ -1136,6 +1136,24 @@
>  { 'command': 'pmemsave',
>'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>  
> +##
> +# @pmemload:
> +#
> +# Load a portion of guest physical memory from a file.
> +#
> +# @val: the physical address of the guest to start from

Is 'val' really the best name for this, or would 'phys-addr' or similar
be a more descriptive name?

> +#
> +# @size: the size of memory region to load
> +#
> +# @filename: the file to load the memory from as binary data
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 2.10

You've missed 2.10 by a long shot.  The earliest this new feature could
appear is 2.13.

Do you additionally need an offset where to start reading from within
the file (that is, since you already have the 'size' parameter to avoid
reading the entire file, and the 'val' parameter to target anywhere in
physical memory, how do I start reading anywhere from the file)?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qmp: add pmemload command

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 
0e59c79ddc01e195ddc59d77d9df2b95bf89b600.1523395243.git.si...@ruderich.org
Subject: [Qemu-devel] [PATCH] qmp: add pmemload command

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/0e59c79ddc01e195ddc59d77d9df2b95bf89b600.1523395243.git.si...@ruderich.org
 -> 
patchew/0e59c79ddc01e195ddc59d77d9df2b95bf89b600.1523395243.git.si...@ruderich.org
Switched to a new branch 'test'
199564756b qmp: add pmemload command

=== OUTPUT BEGIN ===
Checking PATCH 1/1: qmp: add pmemload command...
ERROR: braces {} are necessary for all arms of this statement
#45: FILE: cpus.c:2330:
+if (l > size)
[...]

total: 1 errors, 0 warnings, 104 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH] qmp: add pmemload command

[Simon Ruderich]

Adapted patch from Baojun Wang [1] with the following commit message:

I found this could be useful to have qemu-softmmu as a cross
debugger (launch with -s -S command line option), then if we can
have a command to load guest physical memory, we can use cross gdb
to do some target debug which gdb cannot do directly.

pmemload is necessary to directly write physical memory which is not
possible with gdb alone as it uses only logical addresses.

[1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00073.html

Based-on-patch-by: Baojun Wang 
Signed-off-by: Simon Ruderich 
---

Hello,

I'm using pmemload to manipulate physical memory in Qemu with
this patch; the existing pmemsave can be used to dump the
physical memory.

I've only used pmemload from qemu's monitor and not via qapi.
This part was taken unchanged from the original patch.

Regards
Simon

 cpus.c   | 30 ++
 hmp-commands.hx  | 14 ++
 hmp.c| 11 +++
 hmp.h|  1 +
 qapi-schema.json | 18 ++
 5 files changed, 74 insertions(+)

diff --git a/cpus.c b/cpus.c
index 114c29b6a0..b2325dd7bb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2039,6 +2039,36 @@ exit:
 fclose(f);
 }
 
+void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
+  Error **errp)
+{
+FILE *f;
+size_t l;
+uint8_t buf[1024];
+
+f = fopen(filename, "rb");
+if (!f) {
+error_setg_file_open(errp, errno, filename);
+return;
+}
+
+while (size != 0) {
+l = sizeof(buf);
+if (l > size)
+l = size;
+if (fread(buf, 1, l, f) != l) {
+error_setg(errp, QERR_IO_ERROR);
+goto exit;
+}
+cpu_physical_memory_write(addr, buf, l);
+addr += l;
+size -= l;
+}
+
+exit:
+fclose(f);
+}
+
 void qmp_inject_nmi(Error **errp)
 {
 nmi_monitor_handle(monitor_get_cpu_index(), errp);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 4afd57cf5f..cc1956252e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -853,6 +853,20 @@ STEXI
 @item pmemsave @var{addr} @var{size} @var{file}
 @findex pmemsave
 save to disk physical memory dump starting at @var{addr} of size @var{size}.
+ETEXI
+
+{
+.name   = "pmemload",
+.args_type  = "val:l,size:i,filename:s",
+.params = "addr size file",
+.help   = "load from disk physical memory dump starting at 'addr' 
of size 'size'",
+.cmd= hmp_pmemload,
+},
+
+STEXI
+@item pmemload @var{addr} @var{size} @var{file}
+@findex pmemload
+load from disk physical memory dump starting at @var{addr} of size @var{size}.
 ETEXI
 
 {
diff --git a/hmp.c b/hmp.c
index 35a7041824..bec5eac621 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1107,6 +1107,17 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, );
 }
 
+void hmp_pmemload(Monitor *mon, const QDict *qdict)
+{
+uint32_t size = qdict_get_int(qdict, "size");
+const char *filename = qdict_get_str(qdict, "filename");
+uint64_t addr = qdict_get_int(qdict, "val");
+Error *err = NULL;
+
+qmp_pmemload(addr, size, filename, );
+hmp_handle_error(mon, );
+}
+
 void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
 {
 const char *chardev = qdict_get_str(qdict, "device");
diff --git a/hmp.h b/hmp.h
index a6f56b1f29..b433d919e9 100644
--- a/hmp.h
+++ b/hmp.h
@@ -49,6 +49,7 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
 void hmp_cpu(Monitor *mon, const QDict *qdict);
 void hmp_memsave(Monitor *mon, const QDict *qdict);
 void hmp_pmemsave(Monitor *mon, const QDict *qdict);
+void hmp_pmemload(Monitor *mon, const QDict *qdict);
 void hmp_ringbuf_write(Monitor *mon, const QDict *qdict);
 void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
 void hmp_cont(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index 18457954a8..a013d590c5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1136,6 +1136,24 @@
 { 'command': 'pmemsave',
   'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
 
+##
+# @pmemload:
+#
+# Load a portion of guest physical memory from a file.
+#
+# @val: the physical address of the guest to start from
+#
+# @size: the size of memory region to load
+#
+# @filename: the file to load the memory from as binary data
+#
+# Returns: Nothing on success
+#
+# Since: 2.10
+##
+{ 'command': 'pmemload',
+  'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
+
 ##
 # @cont:
 #
-- 
2.15.0

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: [Qemu-devel] [PULL 0/7] ppc-for-2.12 queue 20180410

[Peter Maydell]

On 10 April 2018 at 13:52, David Gibson <da...@gibson.dropbear.id.au> wrote:
> The following changes since commit 915d34c5f99b0ab91517c69f54272bfdb6ca2b32:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2018-04-09 17:29:10 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.12-20180410
>
> for you to fetch changes up to 6b3913e0858488ef3358c1683605d6894a6cadb0:
>
>   roms/u-boot-sam460ex: Change to qemu git mirror and update (2018-04-10 
> 10:05:38 +1000)
>
> 
> ppc patch queue 2018-04-10
>
> Here's a rather late pull request with a handful of fixes for 2.12.
> These have been blocked for some time, because I wasn't able to
> complete my usual test set due to the SCSI problem fixed in 37c5174
> "scsi-disk: Don't enlarge min_io_size to max_io_size".
>
> Since we're in hard freeze, these are all bugfixes.  Most are also
> regressions, although in one case it's only a "regression" because a
> longstanding bug has been exposed by a new machine type (sam460ex) in
> the testcases.  There are also a couple of sam460ex fixes that aren't
> regressions since the board didn't exist before.  On the flipside
> though, they're low risk because they only touch board specific code
> for a board that doesn't exist in any released version.
>

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH for-2.12] i386: Don't automatically enable FEAT_KVM_HINTS bits

[Eduardo Habkost]

The assumption in the cpu->max_features code is that anything
enabled on GET_SUPPORTED_CPUID should be enabled on "-cpu host".
This shouldn't be the case for FEAT_KVM_HINTS.

This adds a new FeatureWordInfo::no_autoenable_flags field, that
can be used to prevent FEAT_KVM_HINTS bits to be enabled
automatically.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1a6b082b6f..a20fe26573 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -295,6 +295,8 @@ typedef struct FeatureWordInfo {
 uint32_t tcg_features; /* Feature flags supported by TCG */
 uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
 uint32_t migratable_flags; /* Feature flags known to be migratable */
+/* Features that shouldn't be auto-enabled by "-cpu host" */
+uint32_t no_autoenable_flags;
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
@@ -400,6 +402,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 },
 .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EDX,
 .tcg_features = TCG_KVM_FEATURES,
+/*
+ * KVM hints aren't auto-enabled by -cpu host, they need to be
+ * explicitly enabled in the command-line.
+ */
+.no_autoenable_flags = ~0U,
 },
 [FEAT_HYPERV_EAX] = {
 .feat_names = {
@@ -4062,7 +4069,8 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error 
**errp)
  */
 env->features[w] |=
 x86_cpu_get_supported_feature_word(w, cpu->migratable) &
-~env->user_features[w];
+~env->user_features[w] & \
+~feature_word_info[w].no_autoenable_flags;
 }
 }
 
-- 
2.14.3

[Qemu-devel] [PATCH] i386: Don't automatically enable FEAT_KVM_HINTS bits

[Eduardo Habkost]

The assumption in the cpu->max_features code is that anything
enabled on GET_SUPPORTED_CPUID should be enabled on "-cpu host".
This shouldn't be the case for FEAT_KVM_HINTS.

This adds a new FeatureWordInfo::no_autoenable_flags field, that
can be used to prevent FEAT_KVM_HINTS bits to be enabled
automatically.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1a6b082b6f..a20fe26573 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -295,6 +295,8 @@ typedef struct FeatureWordInfo {
 uint32_t tcg_features; /* Feature flags supported by TCG */
 uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
 uint32_t migratable_flags; /* Feature flags known to be migratable */
+/* Features that shouldn't be auto-enabled by "-cpu host" */
+uint32_t no_autoenable_flags;
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
@@ -400,6 +402,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 },
 .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EDX,
 .tcg_features = TCG_KVM_FEATURES,
+/*
+ * KVM hints aren't auto-enabled by -cpu host, they need to be
+ * explicitly enabled in the command-line.
+ */
+.no_autoenable_flags = ~0U,
 },
 [FEAT_HYPERV_EAX] = {
 .feat_names = {
@@ -4062,7 +4069,8 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error 
**errp)
  */
 env->features[w] |=
 x86_cpu_get_supported_feature_word(w, cpu->migratable) &
-~env->user_features[w];
+~env->user_features[w] & \
+~feature_word_info[w].no_autoenable_flags;
 }
 }
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 00/24] fix building of tests/tcg

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180410193919.28026-1-alex.ben...@linaro.org
Subject: [Qemu-devel] [PATCH v1 00/24] fix building of tests/tcg

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/20180410193919.28026-1-alex.ben...@linaro.org -> 
patchew/20180410193919.28026-1-alex.ben...@linaro.org
Switched to a new branch 'test'
90c03a56a7 tests/Makefile.include: add (clean-)check-tcg targets
4d10ecddd9 Makefile.target: add (clean-)guest-tests targets
419d213b73 tests/tcg/Makefile: update to be called from Makefile.target
42375e278e tests/tcg: enable building for ppc64
48844c4a0b tests/tcg: enable building for s390x
30148808c5 tests/tcg: move MIPS specific tests into subdir
2e1ca97890 tests/tcg/arm: fix hello-arm
7e47cb78d1 tests/tcg: move ARM specific tests into subdir
ae14cce45a tests/tcg/i386: fix test-i386-fprem
fff9127a52 tests/tcg/i368: fix hello-i386
32defdca05 tests/tcg/i386: fix test-i386
f29d705818 tests/tcg/i386: move test-i386-sse.c to tests/tcg/x86_64/test-sse.c
420cad3ca6 tests/tcg/i386: Build fix for hello-i386
b9c79e9220 tests/tcg: move i386 specific tests into subdir
d3d8bafea4 tests/tcg/multiarch: Build fix for linux-test
a0e4f21091 tests/tcg: move architecture independent tests into subdir
4b155dc50c docker: Makefile.include introduce DOCKER_SCRIPT
a8974c3037 docker: allow "cc" command to run in user context
2b879dfb5c docker: extend "cc" command to accept compiler
948d434a34 docker: Add "cc" subcommand
6464064827 Makefile: Rename TARGET_DIRS to TARGET_LIST
ca85716ee3 configure: move i386_cc to cross_cc_i386
d0197823de configure: add support for --cross-cc-FOO
ebd12f8f96 configure: add test for docker availability

=== OUTPUT BEGIN ===
Checking PATCH 1/24: configure: add test for docker availability...
Checking PATCH 2/24: configure: add support for --cross-cc-FOO...
Checking PATCH 3/24: configure: move i386_cc to cross_cc_i386...
Checking PATCH 4/24: Makefile: Rename TARGET_DIRS to TARGET_LIST...
Checking PATCH 5/24: docker: Add "cc" subcommand...
Checking PATCH 6/24: docker: extend "cc" command to accept compiler...
Checking PATCH 7/24: docker: allow "cc" command to run in user context...
Checking PATCH 8/24: docker: Makefile.include introduce DOCKER_SCRIPT...
Checking PATCH 9/24: tests/tcg: move architecture independent tests into 
subdir...
Checking PATCH 10/24: tests/tcg/multiarch: Build fix for linux-test...
ERROR: if this code is redundant consider removing it
#152: FILE: tests/tcg/multiarch/linux-test.c:323:
+#if 0

total: 1 errors, 0 warnings, 185 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 11/24: tests/tcg: move i386 specific tests into subdir...
Checking PATCH 12/24: tests/tcg/i386: Build fix for hello-i386...
ERROR: externs should be avoided in .c files
#20: FILE: tests/tcg/i386/hello-i386.c:23:
+void _start(void);

total: 1 errors, 0 warnings, 7 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 13/24: tests/tcg/i386: move test-i386-sse.c to 
tests/tcg/x86_64/test-sse.c...
ERROR: code indent should never use tabs
#36: FILE: tests/tcg/x86_64/test-sse.c:44:
+^I/* SSE4 popcnt r64, r/m64 */$

total: 1 errors, 0 warnings, 21 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 14/24: tests/tcg/i386: fix test-i386...
Checking PATCH 15/24: tests/tcg/i368: fix hello-i386...
Checking PATCH 16/24: tests/tcg/i386: fix test-i386-fprem...
Checking PATCH 17/24: tests/tcg: move ARM specific tests into subdir...
Checking PATCH 18/24: tests/tcg/arm: fix hello-arm...
Checking PATCH 19/24: tests/tcg: move MIPS specific tests into subdir...
Checking PATCH 20/24: tests/tcg: enable building for s390x...
Checking PATCH 21/24: tests/tcg: enable building for ppc64...
Checking PATCH 22/24: tests/tcg/Makefile: update to be called from 
Makefile.target...
Checking PATCH 23/24: Makefile.target: add (clean-)guest-tests targets...
Checking PATCH 24/24: tests/Makefile.include: add (clean-)check-tcg 

[Qemu-devel] [Bug 1759264] Re: fpu/softfloat: round_to_int_and_pack refactor broke TriCore ftoi insns

[Alex Bennée]

Yeah it looks like it was missed, the round_to_uint code does it.

Do you have a test case I can verify?

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

Title:
  fpu/softfloat: round_to_int_and_pack refactor broke TriCore ftoi insns

Status in QEMU:
  New

Bug description:
  After the refactor from ab52f973a504f8de0c5df64631ba4caea70a7d9e the
  bahaviour of int32_to_float32() was altered.

  helper_ftoi() in target/tricore/fpu_helper.c relied on
  int32_to_float32 to raise the invalid flag if the input was NaN to
  properly return 0. Likewise if the input is infinity.

  The obvious fix for softfloat would be to raise this flag in 
round_to_int_and_pack(). However,
  I'm not sure if this breaks other targets and I have no easy way to test it.

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

[Qemu-devel] [PATCH v1 12/24] tests/tcg/i386: Build fix for hello-i386

[Alex Bennée]

From: Fam Zheng 

We have -Werror=missing-prototype, add a dummy prototype to avoid that
warning.

Signed-off-by: Fam Zheng 
---
 tests/tcg/i386/hello-i386.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/tcg/i386/hello-i386.c b/tests/tcg/i386/hello-i386.c
index fa00380de2..cfeb24b2f5 100644
--- a/tests/tcg/i386/hello-i386.c
+++ b/tests/tcg/i386/hello-i386.c
@@ -20,6 +20,7 @@ static inline int write(int fd, const char * buf, int len)
   return status;
 }
 
+void _start(void);
 void _start(void)
 {
 write(1, "Hello World\n", 12);
-- 
2.16.2

[Qemu-devel] [PATCH v1 21/24] tests/tcg: enable building for ppc64

[Alex Bennée]

Currently this just enables building the multiarch tests.

Signed-off-by: Alex Bennée 
---
 tests/tcg/ppc64/Makefile.include | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 tests/tcg/ppc64/Makefile.include

diff --git a/tests/tcg/ppc64/Makefile.include b/tests/tcg/ppc64/Makefile.include
new file mode 100644
index 00..d71cfc9aa7
--- /dev/null
+++ b/tests/tcg/ppc64/Makefile.include
@@ -0,0 +1,2 @@
+DOCKER_IMAGE=debian-ppc64el-cross
+DOCKER_CROSS_COMPILER=powerpc64le-linux-gnu-gcc
-- 
2.16.2

[Qemu-devel] [PATCH v1 11/24] tests/tcg: move i386 specific tests into subdir

[Alex Bennée]

These only need to be built for i386 guests. This includes a stub
tests/tcg/i386/Makfile.target which absorbs some of what was in
tests/tcg/Makefile.

Signed-off-by: Alex Bennée 
---
 tests/tcg/README|  39 
 tests/tcg/i386/Makefile.target  |  10 
 tests/tcg/i386/README   |  38 +++
 tests/tcg/{ => i386}/hello-i386.c   |   0
 tests/tcg/{ => i386}/pi_10.com  | Bin
 tests/tcg/{ => i386}/runcom.c   |   0
 tests/tcg/{ => i386}/test-i386-code16.S |   0
 tests/tcg/{ => i386}/test-i386-fprem.c  |   0
 tests/tcg/{ => i386}/test-i386-muldiv.h |   0
 tests/tcg/{ => i386}/test-i386-shift.h  |   0
 tests/tcg/{ => i386}/test-i386-ssse3.c  |   0
 tests/tcg/{ => i386}/test-i386-vm86.S   |   0
 tests/tcg/{ => i386}/test-i386.c|   0
 tests/tcg/{ => i386}/test-i386.h|   0
 14 files changed, 48 insertions(+), 39 deletions(-)
 create mode 100644 tests/tcg/i386/Makefile.target
 create mode 100644 tests/tcg/i386/README
 rename tests/tcg/{ => i386}/hello-i386.c (100%)
 rename tests/tcg/{ => i386}/pi_10.com (100%)
 rename tests/tcg/{ => i386}/runcom.c (100%)
 rename tests/tcg/{ => i386}/test-i386-code16.S (100%)
 rename tests/tcg/{ => i386}/test-i386-fprem.c (100%)
 rename tests/tcg/{ => i386}/test-i386-muldiv.h (100%)
 rename tests/tcg/{ => i386}/test-i386-shift.h (100%)
 rename tests/tcg/{ => i386}/test-i386-ssse3.c (100%)
 rename tests/tcg/{ => i386}/test-i386-vm86.S (100%)
 rename tests/tcg/{ => i386}/test-i386.c (100%)
 rename tests/tcg/{ => i386}/test-i386.h (100%)

diff --git a/tests/tcg/README b/tests/tcg/README
index 0890044cf0..469504c4cb 100644
--- a/tests/tcg/README
+++ b/tests/tcg/README
@@ -3,45 +3,6 @@ regression testing. Tests are either multi-arch, meaning they 
can be
 built for all guest architectures that support linux-user executable,
 or they are architecture specific.
 
-i386
-
-
-test-i386
--
-
-This program executes most of the 16 bit and 32 bit x86 instructions and
-generates a text output, for comparison with the output obtained with
-a real CPU or another emulator.
-
-The Linux system call modify_ldt() is used to create x86 selectors
-to test some 16 bit addressing and 32 bit with segmentation cases.
-
-The Linux system call vm86() is used to test vm86 emulation.
-
-Various exceptions are raised to test most of the x86 user space
-exception reporting.
-
-linux-test
---
-
-This program tests various Linux system calls. It is used to verify
-that the system call parameters are correctly converted between target
-and host CPUs.
-
-test-i386-fprem

-
-runcom
---
-
-test-mmap
--
-
-sha1
-
-
-hello-i386
---
 
 
 ARM
diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
new file mode 100644
index 00..7dbb7992e7
--- /dev/null
+++ b/tests/tcg/i386/Makefile.target
@@ -0,0 +1,10 @@
+# i386 cross compile notes
+
+# If we are using a cross compiler config-target.mak may have also
+# defined some CFLAGs to use.
+
+ifeq ($(CC), $(CROSS_CC_GUEST))
+ifdef CROSS_CC_GUEST_CFLAGS
+CFLAGS+=$(CROSS_CC_GUEST_CFLAGS)
+endif
+endif
diff --git a/tests/tcg/i386/README b/tests/tcg/i386/README
new file mode 100644
index 00..7a0a47bf27
--- /dev/null
+++ b/tests/tcg/i386/README
@@ -0,0 +1,38 @@
+These are i386 specific guest programs
+
+test-i386
+-
+
+This program executes most of the 16 bit and 32 bit x86 instructions and
+generates a text output, for comparison with the output obtained with
+a real CPU or another emulator.
+
+The Linux system call modify_ldt() is used to create x86 selectors
+to test some 16 bit addressing and 32 bit with segmentation cases.
+
+The Linux system call vm86() is used to test vm86 emulation.
+
+Various exceptions are raised to test most of the x86 user space
+exception reporting.
+
+linux-test
+--
+
+This program tests various Linux system calls. It is used to verify
+that the system call parameters are correctly converted between target
+and host CPUs.
+
+test-i386-fprem
+---
+
+runcom
+--
+
+test-mmap
+-
+
+sha1
+
+
+hello-i386
+--
diff --git a/tests/tcg/hello-i386.c b/tests/tcg/i386/hello-i386.c
similarity index 100%
rename from tests/tcg/hello-i386.c
rename to tests/tcg/i386/hello-i386.c
diff --git a/tests/tcg/pi_10.com b/tests/tcg/i386/pi_10.com
similarity index 100%
rename from tests/tcg/pi_10.com
rename to tests/tcg/i386/pi_10.com
diff --git a/tests/tcg/runcom.c b/tests/tcg/i386/runcom.c
similarity index 100%
rename from tests/tcg/runcom.c
rename to tests/tcg/i386/runcom.c
diff --git a/tests/tcg/test-i386-code16.S b/tests/tcg/i386/test-i386-code16.S
similarity index 100%
rename from tests/tcg/test-i386-code16.S
rename to tests/tcg/i386/test-i386-code16.S
diff --git a/tests/tcg/test-i386-fprem.c b/tests/tcg/i386/test-i386-fprem.c
similarity index 100%
rename from 

[Qemu-devel] [PATCH v1 20/24] tests/tcg: enable building for s390x

[Alex Bennée]

This doesn't add any additional tests but enables building the
multiarch tests for s390x.

Signed-off-by: Alex Bennée 
---
 tests/tcg/s390x/Makefile.include | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 tests/tcg/s390x/Makefile.include

diff --git a/tests/tcg/s390x/Makefile.include b/tests/tcg/s390x/Makefile.include
new file mode 100644
index 00..1f58115d96
--- /dev/null
+++ b/tests/tcg/s390x/Makefile.include
@@ -0,0 +1,2 @@
+DOCKER_IMAGE=debian-s390x-cross
+DOCKER_CROSS_COMPILER=s390x-linux-gnu-gcc
-- 
2.16.2

[Qemu-devel] [PATCH v1 19/24] tests/tcg: move MIPS specific tests into subdir

[Alex Bennée]

These only need to be built for MIPS guests.

Signed-off-by: Alex Bennée 
---
 tests/tcg/README  | 11 ---
 tests/tcg/mips/README |  7 +++
 tests/tcg/{ => mips}/hello-mips.c |  0
 3 files changed, 7 insertions(+), 11 deletions(-)
 create mode 100644 tests/tcg/mips/README
 rename tests/tcg/{ => mips}/hello-mips.c (100%)

diff --git a/tests/tcg/README b/tests/tcg/README
index 625f2326e6..a5643d33e7 100644
--- a/tests/tcg/README
+++ b/tests/tcg/README
@@ -3,17 +3,6 @@ regression testing. Tests are either multi-arch, meaning they 
can be
 built for all guest architectures that support linux-user executable,
 or they are architecture specific.
 
-
-
-MIPS
-
-
-hello-mips
---
-
-hello-mipsel
-
-
 CRIS
 
 The testsuite for CRIS is in tests/tcg/cris.  You can run it
diff --git a/tests/tcg/mips/README b/tests/tcg/mips/README
new file mode 100644
index 00..e5bbc58ec5
--- /dev/null
+++ b/tests/tcg/mips/README
@@ -0,0 +1,7 @@
+MIPS
+
+
+hello-mips
+--
+
+A very simple inline assembly, write syscall based hello world
diff --git a/tests/tcg/hello-mips.c b/tests/tcg/mips/hello-mips.c
similarity index 100%
rename from tests/tcg/hello-mips.c
rename to tests/tcg/mips/hello-mips.c
-- 
2.16.2

[Qemu-devel] [PATCH v1 16/24] tests/tcg/i386: fix test-i386-fprem

[Alex Bennée]

Remove dependencies on QEMU's source tree and build directly.

Signed-off-by: Alex Bennée 
---
 tests/tcg/i386/test-i386-fprem.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/tcg/i386/test-i386-fprem.c b/tests/tcg/i386/test-i386-fprem.c
index 1a71623204..771dc07433 100644
--- a/tests/tcg/i386/test-i386-fprem.c
+++ b/tests/tcg/i386/test-i386-fprem.c
@@ -23,7 +23,10 @@
  *  along with this program; if not, see .
  */
 
-#include "qemu/osdep.h"
+#include 
+#include 
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 /*
  * Inspired by 's union ieee854_long_double, but with single
@@ -39,7 +42,7 @@ union float80u {
 unsigned int exponent:15;
 unsigned int negative:1;
 unsigned int empty:16;
-} QEMU_PACKED ieee;
+} __attribute__((packed)) ieee;
 
 /* This is for NaNs in the IEEE 854 double-extended-precision format.  */
 struct {
@@ -49,7 +52,7 @@ union float80u {
 unsigned int exponent:15;
 unsigned int negative:1;
 unsigned int empty:16;
-} QEMU_PACKED ieee_nan;
+} __attribute__((packed)) ieee_nan;
 };
 
 #define IEEE854_LONG_DOUBLE_BIAS 0x3fff
-- 
2.16.2

[Qemu-devel] [PATCH v1 14/24] tests/tcg/i386: fix test-i386

[Alex Bennée]

The test-i386 test case is a little special as it includes assembler
files. Add the additional compile magic to assemble these bits and
link them to the final binary.

Signed-off-by: Alex Bennée 
---
 tests/tcg/i386/Makefile.target | 19 +++
 tests/tcg/i386/test-i386.c |  1 -
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
index 7dbb7992e7..1df69e0dab 100644
--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -8,3 +8,22 @@ ifdef CROSS_CC_GUEST_CFLAGS
 CFLAGS+=$(CROSS_CC_GUEST_CFLAGS)
 endif
 endif
+
+#
+# test-386 includes a couple of additional objects that need to be linked 
together
+#
+
+TEST_I386_DEPS=test-i386-code16.o test-i386-vm86.o
+
+# override the default compile and link in one go rule
+test-i386.o: test-i386.c
+   $(CC) $(CFLAGS) -c $< -o $@
+
+# and provide a rule to compile .S files
+%.o: %.S
+   $(CC) $(CFLAGS) -c $< -o $@
+
+test-i386: LDFLAGS+=-lm -lc
+test-i386: test-i386.o $(TEST_I386_DEPS)
+   $(LD) -melf_i386 $(LDFLAGS) $< $(TEST_I386_DEPS) -o $@
+
diff --git a/tests/tcg/i386/test-i386.c b/tests/tcg/i386/test-i386.c
index 9599204895..cae6a7773a 100644
--- a/tests/tcg/i386/test-i386.c
+++ b/tests/tcg/i386/test-i386.c
@@ -17,7 +17,6 @@
  *  along with this program; if not, see .
  */
 #define _GNU_SOURCE
-#include "qemu/compiler.h"
 #include 
 #include 
 #include 
-- 
2.16.2

[Qemu-devel] [PATCH v1 17/24] tests/tcg: move ARM specific tests into subdir

[Alex Bennée]

These only need to be built for ARM guests.

Signed-off-by: Alex Bennée 
---
 tests/tcg/README  |  9 -
 tests/tcg/arm/README  | 11 +++
 tests/tcg/{ => arm}/hello-arm.c   |  0
 tests/tcg/{ => arm}/test-arm-iwmmxt.s |  0
 4 files changed, 11 insertions(+), 9 deletions(-)
 create mode 100644 tests/tcg/arm/README
 rename tests/tcg/{ => arm}/hello-arm.c (100%)
 rename tests/tcg/{ => arm}/test-arm-iwmmxt.s (100%)

diff --git a/tests/tcg/README b/tests/tcg/README
index 469504c4cb..625f2326e6 100644
--- a/tests/tcg/README
+++ b/tests/tcg/README
@@ -5,15 +5,6 @@ or they are architecture specific.
 
 
 
-ARM
-===
-
-hello-arm
--
-
-test-arm-iwmmxt

-
 MIPS
 
 
diff --git a/tests/tcg/arm/README b/tests/tcg/arm/README
new file mode 100644
index 00..e6307116e2
--- /dev/null
+++ b/tests/tcg/arm/README
@@ -0,0 +1,11 @@
+These are ARM specific guest programs
+
+hello-arm
+-
+
+A very simple inline assembly, write syscall based hello world
+
+test-arm-iwmmxt
+---
+
+A simple test case for older iwmmxt extended ARMs
diff --git a/tests/tcg/hello-arm.c b/tests/tcg/arm/hello-arm.c
similarity index 100%
rename from tests/tcg/hello-arm.c
rename to tests/tcg/arm/hello-arm.c
diff --git a/tests/tcg/test-arm-iwmmxt.s b/tests/tcg/arm/test-arm-iwmmxt.s
similarity index 100%
rename from tests/tcg/test-arm-iwmmxt.s
rename to tests/tcg/arm/test-arm-iwmmxt.s
-- 
2.16.2

[Qemu-devel] [PATCH v1 09/24] tests/tcg: move architecture independent tests into subdir

[Alex Bennée]

We will want to build these for all supported guest architectures so
lets move them all into one place. We also drop test_path at this
point because it needs qemu utils and glib bits which is hard to
support for cross compiling.

Signed-off-by: Alex Bennée 
---
 tests/tcg/README   |  10 +--
 tests/tcg/multiarch/README |   1 +
 tests/tcg/{ => multiarch}/linux-test.c |   0
 tests/tcg/{ => multiarch}/sha1.c   |   0
 tests/tcg/{ => multiarch}/test-mmap.c  |   0
 tests/tcg/{ => multiarch}/testthread.c |   0
 tests/tcg/test_path.c  | 157 -
 7 files changed, 5 insertions(+), 163 deletions(-)
 create mode 100644 tests/tcg/multiarch/README
 rename tests/tcg/{ => multiarch}/linux-test.c (100%)
 rename tests/tcg/{ => multiarch}/sha1.c (100%)
 rename tests/tcg/{ => multiarch}/test-mmap.c (100%)
 rename tests/tcg/{ => multiarch}/testthread.c (100%)
 delete mode 100644 tests/tcg/test_path.c

diff --git a/tests/tcg/README b/tests/tcg/README
index 5dcfb4852b..0890044cf0 100644
--- a/tests/tcg/README
+++ b/tests/tcg/README
@@ -1,9 +1,7 @@
-This directory contains various interesting programs for
-regression testing.
-
-The target "make test" runs the programs and, if applicable,
-runs "diff" to detect mismatches between output on the host and
-output on QEMU.
+This directory contains various interesting guest programs for
+regression testing. Tests are either multi-arch, meaning they can be
+built for all guest architectures that support linux-user executable,
+or they are architecture specific.
 
 i386
 
diff --git a/tests/tcg/multiarch/README b/tests/tcg/multiarch/README
new file mode 100644
index 00..522c9d2ea3
--- /dev/null
+++ b/tests/tcg/multiarch/README
@@ -0,0 +1 @@
+Multi-architecture linux-user tests
diff --git a/tests/tcg/linux-test.c b/tests/tcg/multiarch/linux-test.c
similarity index 100%
rename from tests/tcg/linux-test.c
rename to tests/tcg/multiarch/linux-test.c
diff --git a/tests/tcg/sha1.c b/tests/tcg/multiarch/sha1.c
similarity index 100%
rename from tests/tcg/sha1.c
rename to tests/tcg/multiarch/sha1.c
diff --git a/tests/tcg/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
similarity index 100%
rename from tests/tcg/test-mmap.c
rename to tests/tcg/multiarch/test-mmap.c
diff --git a/tests/tcg/testthread.c b/tests/tcg/multiarch/testthread.c
similarity index 100%
rename from tests/tcg/testthread.c
rename to tests/tcg/multiarch/testthread.c
diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
deleted file mode 100644
index 1c29bce263..00
--- a/tests/tcg/test_path.c
+++ /dev/null
@@ -1,157 +0,0 @@
-/* Test path override code */
-#include "config-host.h"
-#include "util/cutils.c"
-#include "util/hexdump.c"
-#include "util/iov.c"
-#include "util/path.c"
-#include "util/qemu-timer-common.c"
-#include 
-#include 
-#include 
-
-void qemu_log(const char *fmt, ...);
-
-/* Any log message kills the test. */
-void qemu_log(const char *fmt, ...)
-{
-va_list ap;
-
-fprintf(stderr, "FATAL: ");
-va_start(ap, fmt);
-vfprintf(stderr, fmt, ap);
-va_end(ap);
-exit(1);
-}
-
-#define NO_CHANGE(_path)   \
-   do {\
-   if (strcmp(path(_path), _path) != 0) return __LINE__;   \
-   } while(0)
-
-#define CHANGE_TO(_path, _newpath) \
-   do {\
-   if (strcmp(path(_path), _newpath) != 0) return __LINE__;\
-   } while(0)
-
-static void cleanup(void)
-{
-unlink("/tmp/qemu-test_path/DIR1/DIR2/FILE");
-unlink("/tmp/qemu-test_path/DIR1/DIR2/FILE2");
-unlink("/tmp/qemu-test_path/DIR1/DIR2/FILE3");
-unlink("/tmp/qemu-test_path/DIR1/DIR2/FILE4");
-unlink("/tmp/qemu-test_path/DIR1/DIR2/FILE5");
-rmdir("/tmp/qemu-test_path/DIR1/DIR2");
-rmdir("/tmp/qemu-test_path/DIR1/DIR3");
-rmdir("/tmp/qemu-test_path/DIR1");
-rmdir("/tmp/qemu-test_path");
-}
-
-static unsigned int do_test(void)
-{
-if (mkdir("/tmp/qemu-test_path", 0700) != 0)
-   return __LINE__;
-
-if (mkdir("/tmp/qemu-test_path/DIR1", 0700) != 0)
-   return __LINE__;
-
-if (mkdir("/tmp/qemu-test_path/DIR1/DIR2", 0700) != 0)
-   return __LINE__;
-
-if (mkdir("/tmp/qemu-test_path/DIR1/DIR3", 0700) != 0)
-   return __LINE__;
-
-if (close(creat("/tmp/qemu-test_path/DIR1/DIR2/FILE", 0600)) != 0)
-   return __LINE__;
-
-if (close(creat("/tmp/qemu-test_path/DIR1/DIR2/FILE2", 0600)) != 0)
-   return __LINE__;
-
-if (close(creat("/tmp/qemu-test_path/DIR1/DIR2/FILE3", 0600)) != 0)
-   return __LINE__;
-
-if (close(creat("/tmp/qemu-test_path/DIR1/DIR2/FILE4", 0600)) != 0)
-   return __LINE__;
-
-if (close(creat("/tmp/qemu-test_path/DIR1/DIR2/FILE5", 0600)) != 0)
-   return __LINE__;
-
-

[Qemu-devel] [PATCH v1 22/24] tests/tcg/Makefile: update to be called from Makefile.target

[Alex Bennée]

This make is now invoked from each individual target make with the
appropriate CC and ARCH set for each guest. It includes all the
multiarch tests by default as well as any tests from
tests/tcg/$(ARCH).

As there may be subtle additional requirements for building some of
the tests it also includes tests/tcg/$(ARCH)/Makefile.target if it is
available. This is distinct from tests/tcg/$(ARCH)/Makefile.include
which is used by the parent make machinery to determine potential
docker targets.

Signed-off-by: Alex Bennée 
---
 tests/tcg/Makefile | 185 -
 1 file changed, 42 insertions(+), 143 deletions(-)

diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
index 89e3342f3d..2bba0d2a32 100644
--- a/tests/tcg/Makefile
+++ b/tests/tcg/Makefile
@@ -1,156 +1,55 @@
--include ../../config-host.mak
--include $(SRC_PATH)/rules.mak
+# -*- Mode: makefile -*-
+#
+# TCG tests
+#
+# These are complicated by the fact we want to build them for guest
+# systems. This requires knowing what guests we are building and which
+# ones we have cross-compilers for or docker images with
+# cross-compilers.
+#
+# The tests themselves should be as minimal as possible as
+# cross-compilers don't always have a large amount of libraries
+# available.
+#
+# We only include the host build system for SRC_PATH and we don't
+# bother with the common rules.mk. We expect CC to have been set for
+# us from the parent make. We also expect to be in the tests build dir
+# for the FOO-linux-user.
+#
 
-$(call set-vpath, $(SRC_PATH)/tests/tcg)
+-include ../../config-host.mak
+-include ../config-target.mak
 
-QEMU=../../i386-linux-user/qemu-i386
-QEMU_X86_64=../../x86_64-linux-user/qemu-x86_64
-CC_X86_64=$(CC_I386) -m64
+# Set search path for all sources
+VPATH = $(SRC_PATH)/tests/tcg/multiarch
+TEST_SRCS = $(wildcard $(SRC_PATH)/tests/tcg/multiarch/*.c)
 
-QEMU_INCLUDES += -I../..
-CFLAGS=-Wall -O2 -g -fno-strict-aliasing
-#CFLAGS+=-msse2
-LDFLAGS=
+VPATH += $(SRC_PATH)/tests/tcg/$(ARCH)
+TEST_SRCS += $(wildcard $(SRC_PATH)/tests/tcg/$(ARCH)/*.c)
 
-# TODO: automatically detect ARM and MIPS compilers, and run those too
+SRCS=$(notdir ${TEST_SRCS})
+TESTS=$(SRCS:.c=)
 
-# runcom maps page 0, so it requires root privileges
-# also, pi_10.com runs indefinitely
+# We use what ever CC we have
+CFLAGS=-Wall -O0 -g -fno-strict-aliasing -static
+QEMU_CFLAGS=
+LDFLAGS=
 
-I386_TESTS=hello-i386 \
-  linux-test \
-  testthread \
-  sha1-i386 \
-  test-i386 \
-  test-i386-fprem \
-  test-mmap \
-  # runcom
+# The per ARCH target makefile which might add specific compiler flags
+# for some compilation targets.
 
-# native i386 compilers sometimes are not biarch.  assume cross-compilers are
-ifneq ($(ARCH),i386)
-I386_TESTS+=run-test-x86_64
-endif
+EXTRA_MAKEFILE=$(SRC_PATH)/tests/tcg/$(ARCH)/Makefile.target
+CHECK_INCLUDE=$(wildcard $(EXTRA_MAKEFILE))
 
-TESTS = test_path
-ifneq ($(call find-in-path, $(CC_I386)),)
-TESTS += $(I386_TESTS)
+ifeq ($(EXTRA_MAKEFILE),$(CHECK_INCLUDE))
+include $(EXTRA_MAKEFILE)
 endif
 
-all: $(patsubst %,run-%,$(TESTS))
-test: all
-
-# rules to run tests
-
-.PHONY: $(patsubst %,run-%,$(TESTS))
-
-run-%: %
-   -$(QEMU) ./$*
-
-run-hello-i386: hello-i386
-run-linux-test: linux-test
-run-testthread: testthread
-run-sha1-i386: sha1-i386
-
-run-test-i386: test-i386
-   ./test-i386 > test-i386.ref
-   -$(QEMU) test-i386 > test-i386.out
-   @if diff -u test-i386.ref test-i386.out ; then echo "Auto Test OK"; fi
-
-run-test-i386-fprem: test-i386-fprem
-   ./test-i386-fprem > test-i386-fprem.ref
-   -$(QEMU) test-i386-fprem > test-i386-fprem.out
-   @if diff -u test-i386-fprem.ref test-i386-fprem.out ; then echo "Auto 
Test OK"; fi
-
-run-test-x86_64: test-x86_64
-   ./test-x86_64 > test-x86_64.ref
-   -$(QEMU_X86_64) test-x86_64 > test-x86_64.out
-   @if diff -u test-x86_64.ref test-x86_64.out ; then echo "Auto Test OK"; 
fi
-
-run-test-mmap: test-mmap
-   -$(QEMU) ./test-mmap
-   -$(QEMU) -p 8192 ./test-mmap 8192
-   -$(QEMU) -p 16384 ./test-mmap 16384
-   -$(QEMU) -p 32768 ./test-mmap 32768
-
-run-runcom: runcom
-   -$(QEMU) ./runcom $(SRC_PATH)/tests/pi_10.com
-
-run-test_path: test_path
-   ./test_path
-
-# rules to compile tests
-
-test_path: test_path.o
-
-test_path.o: test_path.c
-
-hello-i386: hello-i386.c
-   $(CC_I386) -nostdlib $(CFLAGS) -static $(LDFLAGS) -o $@ $<
-   strip $@
-
-testthread: testthread.c
-   $(CC_I386) $(CFLAGS) $(LDFLAGS) -o $@ $< -lpthread
-
-# i386/x86_64 emulation test (test various opcodes) */
-test-i386: test-i386.c test-i386-code16.S test-i386-vm86.S \
-   test-i386.h test-i386-shift.h test-i386-muldiv.h
-   $(CC_I386) $(QEMU_INCLUDES) $(CFLAGS) $(LDFLAGS) -o $@ \
-  $(

[Qemu-devel] [PATCH v1 15/24] tests/tcg/i368: fix hello-i386

[Alex Bennée]

This is a direct syscall test so needs additional CFLAGS and LDFLAGS.

Signed-off-by: Alex Bennée 
---
 tests/tcg/i386/Makefile.target | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
index 1df69e0dab..44803c0382 100644
--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -9,6 +9,12 @@ CFLAGS+=$(CROSS_CC_GUEST_CFLAGS)
 endif
 endif
 
+#
+# hello-i386 is a barebones app
+#
+hello-i386: CFLAGS+=-ffreestanding
+hello-i386: LDFLAGS+=-nostdlib
+
 #
 # test-386 includes a couple of additional objects that need to be linked 
together
 #
-- 
2.16.2

[Qemu-devel] [PATCH v1 18/24] tests/tcg/arm: fix hello-arm

[Alex Bennée]

As hello-arm is a bare bones syscall test it needs specific compiler
flags so it doesn't try and link against glibc.

Signed-off-by: Alex Bennée 
---
 tests/tcg/arm/Makefile.target | 6 ++
 1 file changed, 6 insertions(+)
 create mode 100644 tests/tcg/arm/Makefile.target

diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
new file mode 100644
index 00..b7c146c88e
--- /dev/null
+++ b/tests/tcg/arm/Makefile.target
@@ -0,0 +1,6 @@
+# -*- Mode: makefile -*-
+#
+# ARM specific tweaks
+
+hello-arm: CFLAGS+=-marm -ffreestanding
+hello-arm: LDFLAGS+=-nostdlib
-- 
2.16.2

[Qemu-devel] [PATCH v1 10/24] tests/tcg/multiarch: Build fix for linux-test

[Alex Bennée]

From: Fam Zheng 

To keep the compiler happy, and to fit in our buildsys flags:

- Make local functions "static"
- #ifdef out unused functions
- drop cutils/osdep dependencies

Signed-off-by: Fam Zheng 
[AJB: drop cutils/osdep dependencies]
Signed-off-by: Alex Bennée 
---
 tests/tcg/multiarch/linux-test.c | 68 +---
 1 file changed, 21 insertions(+), 47 deletions(-)

diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c
index 5070d31446..4457bd04ba 100644
--- a/tests/tcg/multiarch/linux-test.c
+++ b/tests/tcg/multiarch/linux-test.c
@@ -16,7 +16,6 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, see .
  */
-#define _GNU_SOURCE
 #include 
 #include 
 #include 
@@ -31,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -39,13 +39,12 @@
 #include 
 #include 
 #include 
-#include "qemu/cutils.h"
 
 #define TESTPATH "/tmp/linux-test.tmp"
 #define TESTPORT 7654
 #define STACK_SIZE 16384
 
-void error1(const char *filename, int line, const char *fmt, ...)
+static void error1(const char *filename, int line, const char *fmt, ...)
 {
 va_list ap;
 va_start(ap, fmt);
@@ -56,7 +55,7 @@ void error1(const char *filename, int line, const char *fmt, 
...)
 exit(1);
 }
 
-int __chk_error(const char *filename, int line, int ret)
+static int __chk_error(const char *filename, int line, int ret)
 {
 if (ret < 0) {
 error1(filename, line, "%m (ret=%d, errno=%d)",
@@ -73,7 +72,7 @@ int __chk_error(const char *filename, int line, int ret)
 
 #define FILE_BUF_SIZE 300
 
-void test_file(void)
+static void test_file(void)
 {
 int fd, i, len, ret;
 uint8_t buf[FILE_BUF_SIZE];
@@ -210,7 +209,7 @@ void test_file(void)
 chk_error(rmdir(TESTPATH));
 }
 
-void test_fork(void)
+static void test_fork(void)
 {
 int pid, status;
 
@@ -224,7 +223,7 @@ void test_fork(void)
 error("waitpid status=0x%x", status);
 }
 
-void test_time(void)
+static void test_time(void)
 {
 struct timeval tv, tv2;
 struct timespec ts, rem;
@@ -251,34 +250,7 @@ void test_time(void)
 error("getrusage");
 }
 
-void pstrcpy(char *buf, int buf_size, const char *str)
-{
-int c;
-char *q = buf;
-
-if (buf_size <= 0)
-return;
-
-for(;;) {
-c = *str++;
-if (c == 0 || q >= buf + buf_size - 1)
-break;
-*q++ = c;
-}
-*q = '\0';
-}
-
-/* strcat and truncate. */
-char *pstrcat(char *buf, int buf_size, const char *s)
-{
-int len;
-len = strlen(buf);
-if (len < buf_size)
-pstrcpy(buf + len, buf_size - len, s);
-return buf;
-}
-
-int server_socket(void)
+static int server_socket(void)
 {
 int val, fd;
 struct sockaddr_in sockaddr;
@@ -298,7 +270,7 @@ int server_socket(void)
 
 }
 
-int client_socket(void)
+static int client_socket(void)
 {
 int fd;
 struct sockaddr_in sockaddr;
@@ -312,9 +284,9 @@ int client_socket(void)
 return fd;
 }
 
-const char socket_msg[] = "hello socket\n";
+static const char socket_msg[] = "hello socket\n";
 
-void test_socket(void)
+static void test_socket(void)
 {
 int server_fd, client_fd, fd, pid, ret, val;
 struct sockaddr_in sockaddr;
@@ -348,9 +320,10 @@ void test_socket(void)
 chk_error(close(server_fd));
 }
 
+#if 0
 #define WCOUNT_MAX 512
 
-void test_pipe(void)
+static void test_pipe(void)
 {
 fd_set rfds, wfds;
 int fds[2], fd_max, ret;
@@ -391,10 +364,10 @@ void test_pipe(void)
 chk_error(close(fds[1]));
 }
 
-int thread1_res;
-int thread2_res;
+static int thread1_res;
+static int thread2_res;
 
-int thread1_func(void *arg)
+static int thread1_func(void *arg)
 {
 int i;
 for(i=0;i<5;i++) {
@@ -404,7 +377,7 @@ int thread1_func(void *arg)
 return 0;
 }
 
-int thread2_func(void *arg)
+static int thread2_func(void *arg)
 {
 int i;
 for(i=0;i<6;i++) {
@@ -435,27 +408,28 @@ void test_clone(void)
 thread2_res != 6)
 error("clone");
 }
+#endif
 
 /***/
 
 volatile int alarm_count;
 jmp_buf jmp_env;
 
-void sig_alarm(int sig)
+static void sig_alarm(int sig)
 {
 if (sig != SIGALRM)
 error("signal");
 alarm_count++;
 }
 
-void sig_segv(int sig, siginfo_t *info, void *puc)
+static void sig_segv(int sig, siginfo_t *info, void *puc)
 {
 if (sig != SIGSEGV)
 error("signal");
 longjmp(jmp_env, 1);
 }
 
-void test_signal(void)
+static void test_signal(void)
 {
 struct sigaction act;
 struct itimerval it, oit;
@@ -510,7 +484,7 @@ void test_signal(void)
 
 #define SHM_SIZE 32768
 
-void test_shm(void)
+static void test_shm(void)
 {
 void *ptr;
 int shmid;
-- 
2.16.2

[Qemu-devel] [PATCH v1 23/24] Makefile.target: add (clean-)guest-tests targets

[Alex Bennée]

Now all the build infrastructure is in place we can build tests for
each guest that we support. That support mainly depends on having
cross compilers installed or docker setup. To keep all the logic for
that together we put the rules in tests/tcg/Makefile.include and
include it from the main Makefile.target.

Signed-off-by: Alex Bennée 
---
 Makefile.target|  5 +++
 tests/tcg/Makefile.include | 79 ++
 2 files changed, 84 insertions(+)
 create mode 100644 tests/tcg/Makefile.include

diff --git a/Makefile.target b/Makefile.target
index d0ec77a307..a30fd40257 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -36,6 +36,11 @@ endif
 PROGS=$(QEMU_PROG) $(QEMU_PROGW)
 STPFILES=
 
+# Makefile Tests
+ifdef CONFIG_USER_ONLY
+include $(SRC_PATH)/tests/tcg/Makefile.include
+endif
+
 config-target.h: config-target.h-timestamp
 config-target.h-timestamp: config-target.mak
 
diff --git a/tests/tcg/Makefile.include b/tests/tcg/Makefile.include
new file mode 100644
index 00..cb8bb36026
--- /dev/null
+++ b/tests/tcg/Makefile.include
@@ -0,0 +1,79 @@
+# -*- Mode: makefile -*-
+#
+# TCG tests (per-target rules)
+#
+# This Makefile fragement is included from the per-target
+# Makefile.target so will be invoked for each linux-user program we
+# build. We have two options for compiling, either using a configured
+# guest compiler or calling one of our docker images to do it for us.
+#
+
+# The per ARCH makefile, if it exists holds extra information about
+# useful docker images or alternative compiler flags. Include it if it
+# exists
+
+ARCH_MAKEFILE=$(SRC_PATH)/tests/tcg/$(TARGET_NAME)/Makefile.include
+CHECK_INCLUDE=$(wildcard $(ARCH_MAKEFILE))
+
+ifeq ($(ARCH_MAKEFILE),$(CHECK_INCLUDE))
+include $(ARCH_MAKEFILE)
+endif
+
+GUEST_BUILD=
+
+# Support installed Cross Compilers
+
+ifdef CROSS_CC_GUEST
+
+.PHONY: cross-build-guest-tests
+cross-build-guest-tests:
+   $(call quiet-command, \
+  (mkdir -p tests && cd tests && \
+  make -f $(SRC_PATH)/tests/tcg/Makefile ARCH=$(TARGET_NAME) 
CC=$(CROSS_CC_GUEST)), \
+ "CROSS-BUILD","$(TARGET_NAME) guest-tests with $(CROSS_CC_GUEST)")
+
+
+GUEST_BUILD=cross-build-guest-tests
+
+endif
+
+# Support building with Docker
+
+ifeq ($(HAVE_USER_DOCKER)$(GUEST_BUILD),y)
+ifneq ($(DOCKER_IMAGE),)
+
+# We also need the Docker make rules to depend on
+include $(SRC_PATH)/tests/docker/Makefile.include
+
+DOCKER_COMPILE_CMD="$(DOCKER_SCRIPT) cc --user $(shell id -u) \
+   --cc $(DOCKER_CROSS_COMPILER) \
+   -i qemu:$(DOCKER_IMAGE) \
+   -s $(SRC_PATH) -- "
+DOCKER_PREREQ=docker-image-$(DOCKER_IMAGE)
+
+.PHONY: docker-build-guest-tests
+docker-build-guest-tests: $(DOCKER_PREREQ)
+   $(call quiet-command, \
+  (mkdir -p tests && cd tests && \
+  make -f $(SRC_PATH)/tests/tcg/Makefile ARCH=$(TARGET_NAME) 
CC=$(DOCKER_COMPILE_CMD)), \
+ "CROSS-BUILD","$(TARGET_NAME) guest-tests with docker 
qemu:$(DOCKER_IMAGE)")
+
+GUEST_BUILD=docker-build-guest-tests
+
+endif
+endif
+
+# Final targets
+.PHONY: guest-tests
+
+ifneq ($(GUEST_BUILD),)
+guest-tests: $(GUEST_BUILD)
+else
+guest-tests:
+   $(call quiet-command, /bin/true, "CROSS-BUILD", "$(TARGET_NAME) 
guest-tests SKIPPED")
+endif
+
+# It doesn't mater if these don't exits
+.PHONY: clean-guest-tests
+clean-guest-tests:
+   rm -rf tests || echo "no $(TARGET_NAME) tests to remove"
-- 
2.16.2

[Qemu-devel] [PATCH v1 13/24] tests/tcg/i386: move test-i386-sse.c to tests/tcg/x86_64/test-sse.c

[Alex Bennée]

The test mixes up 32bit and 64 bit code. It should probably be split
into two distinct test cases. However for now just move it out of the
way of the i386 build.

Signed-off-by: Alex Bennée 
---
 tests/tcg/{i386/test-i386-ssse3.c => x86_64/test-sse.c} | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)
 rename tests/tcg/{i386/test-i386-ssse3.c => x86_64/test-sse.c} (93%)

diff --git a/tests/tcg/i386/test-i386-ssse3.c b/tests/tcg/x86_64/test-sse.c
similarity index 93%
rename from tests/tcg/i386/test-i386-ssse3.c
rename to tests/tcg/x86_64/test-sse.c
index 0a42bd03e2..196ec7f32f 100644
--- a/tests/tcg/i386/test-i386-ssse3.c
+++ b/tests/tcg/x86_64/test-sse.c
@@ -1,4 +1,4 @@
-/* See if various MMX/SSE SSSE3 instructions give expected results */
+/* See if various MMX/SSE SSSE3/4 instructions give expected results */
 #include 
 #include 
 #include 
@@ -41,8 +41,7 @@ int main(int argc, char *argv[]) {
asm volatile ("movdqa  %%xmm0, (%0)" : : "r" (hello));
printf("%5.5s\n", hello);
 
-#if 1 /* SSE4 */
-   /* popcnt r64, r/m64 */
+   /* SSE4 popcnt r64, r/m64 */
asm volatile ("movq$0x842110009c63, %%rax" : : : "rax");
asm volatile ("popcnt  %%ax, %%dx" : : : "dx");
asm volatile ("popcnt  %%eax, %%ecx" : : : "ecx");
@@ -51,7 +50,6 @@ int main(int argc, char *argv[]) {
asm volatile ("movl%%ecx, %0" : "=m" (c));
asm volatile ("movw%%dx, %0" : "=m" (d));
printf("%i = %i\n%i = %i = %i\n", 13, (int) a, 9, c, d + 1);
-#endif
 
return 0;
 }
-- 
2.16.2

[Qemu-devel] [PATCH v1 08/24] docker: Makefile.include introduce DOCKER_SCRIPT

[Alex Bennée]

Define this in one place to make it easy to re-use.

Signed-off-by: Alex Bennée 
---
 tests/docker/Makefile.include | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index de87341528..6a5aa9ec71 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -14,6 +14,8 @@ DOCKER_TESTS := $(notdir $(shell \
 
 DOCKER_TOOLS := travis
 
+DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py
+
 TESTS ?= %
 IMAGES ?= %
 
@@ -37,7 +39,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
echo WARNING: EXECUTABLE is not set, debootstrap may fail. 2>&1 
; \
fi
$(call quiet-command,\
-   $(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \
+   $(DOCKER_SCRIPT) build qemu:$* $< \
$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
$(if $(NOUSER),,--add-current-user) \
$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
@@ -129,11 +131,11 @@ docker-run: docker-qemu-src
fi
$(if $(EXECUTABLE), \
$(call quiet-command,   \
-   $(SRC_PATH)/tests/docker/docker.py update   \
+   $(DOCKER_SCRIPT) update \
$(IMAGE) $(EXECUTABLE), \
"  COPYING $(EXECUTABLE) to $(IMAGE)"))
$(call quiet-command,   \
-   $(SRC_PATH)/tests/docker/docker.py run  \
+   $(DOCKER_SCRIPT) run\
$(if $(NOUSER),,-u $(shell id -u))  \
--security-opt seccomp=unconfined   \
$(if $V,,--rm)  \
@@ -163,4 +165,4 @@ docker-run-%:
@$(MAKE) docker-run TEST=$(CMD) IMAGE=qemu:$(IMAGE)
 
 docker-clean:
-   $(call quiet-command, $(SRC_PATH)/tests/docker/docker.py clean)
+   $(call quiet-command, $(DOCKER_SCRIPT) clean)
-- 
2.16.2

[Qemu-devel] [PATCH v1 07/24] docker: allow "cc" command to run in user context

[Alex Bennée]

Signed-off-by: Alex Bennée 
---
 tests/docker/docker.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 9444f4bea4..f79213044d 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -399,6 +399,8 @@ class CcCommand(SubCommand):
 help="The docker image in which to run cc")
 parser.add_argument("--cc",
 help="The compiler executable to call")
+parser.add_argument("--user",
+help="The user-id to run under")
 parser.add_argument("--source-path", "-s", nargs="*", dest="paths",
 help="""Extra paths to (ro) mount into container 
for
 reading sources""")
@@ -414,6 +416,9 @@ class CcCommand(SubCommand):
 for p in args.paths:
 cmd += ["-v", "%s:%s:ro,z" % (p, p)]
 
+if args.user:
+cmd += ["-u", args.user]
+
 cmd += [args.image]
 
 # The compile command we are running
-- 
2.16.2

[Qemu-devel] [PATCH v1 05/24] docker: Add "cc" subcommand

[Alex Bennée]

From: Fam Zheng 

Signed-off-by: Fam Zheng 
---
 tests/docker/docker.py | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 1246ba9578..8733266153 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -390,6 +390,29 @@ class ImagesCommand(SubCommand):
 def run(self, args, argv):
 return Docker().command("images", argv, args.quiet)
 
+class CcCommand(SubCommand):
+"""Compile sources with cc in images"""
+name = "cc"
+
+def args(self, parser):
+parser.add_argument("--image", "-i", required=True,
+help="The docker image in which to run cc")
+parser.add_argument("--source-path", "-s", nargs="*", dest="paths",
+help="""Extra paths to (ro) mount into container 
for
+reading sources""")
+
+def run(self, args, argv):
+if argv and argv[0] == "--":
+argv = argv[1:]
+cwd = os.getcwd()
+cmd = ["--rm", "-w", cwd,
+   "-v", "%s:%s:rw" % (cwd, cwd)]
+for p in args.paths:
+   cmd += ["-v", "%s:%s:ro,z" % (p, p)]
+cmd += [args.image, "cc"]
+cmd += argv
+return Docker().command("run", cmd, True)
+
 def main():
 parser = argparse.ArgumentParser(description="A Docker helper",
 usage="%s  ..." % os.path.basename(sys.argv[0]))
-- 
2.16.2

[Qemu-devel] [PATCH v1 06/24] docker: extend "cc" command to accept compiler

[Alex Bennée]

When calling our cross-compilation images we want to call something
other than the default cc.

Signed-off-by: Alex Bennée 
---
 tests/docker/docker.py | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 8733266153..9444f4bea4 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -397,6 +397,8 @@ class CcCommand(SubCommand):
 def args(self, parser):
 parser.add_argument("--image", "-i", required=True,
 help="The docker image in which to run cc")
+parser.add_argument("--cc",
+help="The compiler executable to call")
 parser.add_argument("--source-path", "-s", nargs="*", dest="paths",
 help="""Extra paths to (ro) mount into container 
for
 reading sources""")
@@ -407,9 +409,19 @@ class CcCommand(SubCommand):
 cwd = os.getcwd()
 cmd = ["--rm", "-w", cwd,
"-v", "%s:%s:rw" % (cwd, cwd)]
-for p in args.paths:
-   cmd += ["-v", "%s:%s:ro,z" % (p, p)]
-cmd += [args.image, "cc"]
+
+if args.paths:
+for p in args.paths:
+cmd += ["-v", "%s:%s:ro,z" % (p, p)]
+
+cmd += [args.image]
+
+# The compile command we are running
+if args.cc:
+cmd += [args.cc]
+else:
+cmd += ["cc"]
+
 cmd += argv
 return Docker().command("run", cmd, True)
 
-- 
2.16.2

[Qemu-devel] [PATCH v1 24/24] tests/Makefile.include: add (clean-)check-tcg targets

[Alex Bennée]

This will ensure all linux-user targets build their guest test
programs.

Signed-off-by: Alex Bennée 
---
 tests/Makefile.include | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3d2f0458ab..c402de901e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -10,6 +10,7 @@ check-help:
@echo " $(MAKE) check-speed  Run qobject speed tests"
@echo " $(MAKE) check-qapi-schemaRun QAPI schema tests"
@echo " $(MAKE) check-block  Run block tests"
+   @echo " $(MAKE) check-tcgRun TCG tests"
@echo " $(MAKE) check-report.htmlGenerates an HTML test report"
@echo " $(MAKE) check-clean  Clean the tests"
@echo
@@ -916,6 +917,23 @@ check-report.xml: $(patsubst %,check-report-qtest-%.xml, 
$(QTEST_TARGETS)) check
 check-report.html: check-report.xml
$(call quiet-command,gtester-report $< > $@,"GEN","$@")
 
+# Per guest TCG tests
+
+LINUX_USER_TARGETS=$(filter %-linux-user,$(TARGET_LIST))
+BUILD_TCG_TARGET_RULES=$(patsubst %,tcg-tests-%, $(LINUX_USER_TARGETS))
+CLEAN_TCG_TARGET_RULES=$(patsubst %,clean-tcg-tests-%, $(LINUX_USER_TARGETS))
+
+tcg-tests-%:
+   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" 
TARGET_DIR="$*/" guest-tests,)
+
+clean-tcg-tests-%:
+   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" 
TARGET_DIR="$*/" clean-guest-tests,)
+
+.PHONY: check-tcg
+check-tcg: $(BUILD_TCG_TARGET_RULES)
+
+.PHONY: clean-tcg
+clean-tcg: $(CLEAN_TCG_TARGET_RULES)
 
 # Other tests
 
@@ -958,7 +976,6 @@ check-speed: $(patsubst %,check-%, $(check-speed-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-qapi-schema check-unit check-qtest check-decodetree
 check-clean:
-   $(MAKE) -C tests/tcg clean
rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), 
$(check-qtest-$(target)-y)) $(check-qtest-generic-y))
rm -f tests/test-qapi-gen-timestamp
-- 
2.16.2

[Qemu-devel] [PATCH v1 03/24] configure: move i386_cc to cross_cc_i386

[Alex Bennée]

We should still be able to use the system cross compiler with the
appropriate flags on x86_64 hosts.

Signed-off-by: Alex Bennée 
---
 configure | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index b5f3b3fe29..add87ff4d4 100755
--- a/configure
+++ b/configure
@@ -284,7 +284,6 @@ libs_softmmu=""
 libs_tools=""
 audio_pt_int=""
 audio_win_int=""
-cc_i386=i386-pc-linux-gnu-gcc
 libs_qga=""
 debug_info="yes"
 stack_protector=""
@@ -457,6 +456,8 @@ docker="no"
 cross_cc_aarch64="aarch64-linux-gnu-gcc"
 cross_cc_arm="arm-linux-gnueabihf-gcc"
 cross_cc_powerpc="powerpc-linux-gnu-gcc"
+cross_cc_i386="i386-pc-linux-gnu-gcc"
+cross_cc_i386_cflags=""
 
 enabled_cross_compilers=""
 
@@ -687,12 +688,10 @@ case "$cpu" in
   i386|i486|i586|i686|i86pc|BePC)
 cpu="i386"
 supported_cpu="yes"
-cross_cc_i386=gcc
   ;;
   x86_64|amd64)
 cpu="x86_64"
 supported_cpu="yes"
-cross_cc_x86_64=gcc
   ;;
   armv*b|armv*l|arm)
 cpu="arm"
@@ -1435,7 +1434,6 @@ case "$cpu" in
 i386)
CPU_CFLAGS="-m32"
LDFLAGS="-m32 $LDFLAGS"
-   cc_i386='$(CC) -m32'
;;
 x86_64)
# ??? Only extremely old AMD cpus do not have cmpxchg16b.
@@ -1443,12 +1441,14 @@ case "$cpu" in
# runtime and generate the fallback to serial emulation.
CPU_CFLAGS="-m64 -mcx16"
LDFLAGS="-m64 $LDFLAGS"
-   cc_i386='$(CC) -m32'
+   cross_cc_i386=$cc
+   cross_cc_i386_cflags="-m32"
;;
 x32)
CPU_CFLAGS="-mx32"
LDFLAGS="-mx32 $LDFLAGS"
-   cc_i386='$(CC) -m32'
+   cross_cc_i386=$cc
+   cross_cc_i386_cflags="-m32"
;;
 # No special flags required for other host CPUs
 esac
@@ -6664,7 +6664,6 @@ echo "CC=$cc" >> $config_host_mak
 if $iasl -h > /dev/null 2>&1; then
   echo "IASL=$iasl" >> $config_host_mak
 fi
-echo "CC_I386=$cc_i386" >> $config_host_mak
 echo "HOST_CC=$host_cc" >> $config_host_mak
 echo "CXX=$cxx" >> $config_host_mak
 echo "OBJCC=$objcc" >> $config_host_mak
@@ -6783,6 +6782,7 @@ case "$target" in
 esac
 
 target_compiler=""
+target_compiler_cflags=""
 
 mkdir -p $target_dir
 echo "# Automatically generated by configure - do not modify" > 
$config_target_mak
@@ -6799,10 +6799,13 @@ TARGET_ABI_DIR=""
 case "$target_name" in
   i386)
 gdb_xml_files="i386-32bit.xml i386-32bit-core.xml i386-32bit-sse.xml"
+target_compiler=$cross_cc_i386
+target_compiler_cflags=$cross_cc_i386_cflags
   ;;
   x86_64)
 TARGET_BASE_ARCH=i386
 gdb_xml_files="i386-64bit.xml i386-64bit-core.xml i386-64bit-sse.xml"
+target_compiler=$cross_cc_x86_64
   ;;
   alpha)
 mttcg="yes"
@@ -6947,7 +6950,7 @@ int main(void) {
 }
 EOF
 
-if ! do_compiler $target_compiler -o $TMPE $TMPC -static ; then
+if ! do_compiler $target_compiler $target_compiler_cflags -o $TMPE $TMPC 
-static ; then
 target_compiler=""
 else
 enabled_cross_compilers="${enabled_cross_compilers} ${target_compiler}"
@@ -7033,6 +7036,10 @@ if test -n "$target_compiler"; then
   echo "CROSS_CC_GUEST=$target_compiler" >> $config_target_mak
 fi
 
+if test -n "$target_compiler_cflags"; then
+  echo "CROSS_CC_GUEST_CFLAGS=$target_compiler_cflags" >> $config_target_mak
+fi
+
 # generate QEMU_CFLAGS/LDFLAGS for targets
 
 cflags=""
-- 
2.16.2

[Qemu-devel] [PATCH v1 04/24] Makefile: Rename TARGET_DIRS to TARGET_LIST

[Alex Bennée]

From: Fam Zheng 

To be more accurate on its purpose and make code that looks for a certain
target out of this variable more readable.

Signed-off-by: Fam Zheng 
---
 Makefile   | 20 ++--
 configure  |  2 +-
 scripts/create_config  |  2 +-
 tests/Makefile.include |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 727ef118f3..2c54cd8345 100644
--- a/Makefile
+++ b/Makefile
@@ -62,8 +62,8 @@ seems to have been used for an in-tree build. You can fix 
this by running \
 endif
 endif
 
-CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
-CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
+CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_LIST)),y)
+CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_LIST)),y)
 CONFIG_XEN := $(CONFIG_XEN_BACKEND)
 CONFIG_ALL=y
 -include config-all-devices.mak
@@ -362,8 +362,8 @@ DOCS=
 endif
 
 SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory --quiet) 
BUILD_DIR=$(BUILD_DIR)
-SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS))
-SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_DIRS))
+SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_LIST))
+SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_LIST))
 
 ifeq ($(SUBDIR_DEVICES_MAK),)
 config-all-devices.mak:
@@ -466,7 +466,7 @@ config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > 
$@,"GEN","$@")
 
-SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
+SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_LIST))
 SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
@@ -510,7 +510,7 @@ ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 romsubdir-%:
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" 
TARGET_DIR="$*/" CFLAGS="$(filter -O% -g%,$(CFLAGS))",)
 
-ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
+ALL_SUBDIRS=$(TARGET_LIST) $(patsubst %,pc-bios/%, $(ROMS))
 
 recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
 
@@ -763,7 +763,7 @@ distclean: clean
rm -f docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf
rm -f docs/interop/qemu-qmp-ref.html docs/interop/qemu-ga-ref.html
rm -f docs/qemu-block-drivers.7
-   for d in $(TARGET_DIRS); do \
+   for d in $(TARGET_LIST); do \
rm -rf $$d || exit 1 ; \
 done
rm -Rf .sdk
@@ -864,7 +864,7 @@ endif
$(INSTALL_DATA) $(SRC_PATH)/pc-bios/keymaps/$$x 
"$(DESTDIR)$(qemu_datadir)/keymaps"; \
done
$(INSTALL_DATA) $(BUILD_DIR)/trace-events-all 
"$(DESTDIR)$(qemu_datadir)/trace-events-all"
-   for d in $(TARGET_DIRS); do \
+   for d in $(TARGET_LIST); do \
$(MAKE) $(SUBDIR_MAKEFLAGS) TARGET_DIR=$$d/ -C $$d $@ || exit 1 ; \
 done
 
@@ -1062,9 +1062,9 @@ endif
@echo  '  ctags/TAGS  - Generate tags file for editors'
@echo  '  cscope  - Generate cscope index'
@echo  ''
-   @$(if $(TARGET_DIRS), \
+   @$(if $(TARGET_LIST), \
echo 'Architecture specific targets:'; \
-   $(foreach t, $(TARGET_DIRS), \
+   $(foreach t, $(TARGET_LIST), \
printf "  %-30s - Build for %s\\n" $(patsubst %,subdir-%,$(t)) 
$(t);) \
echo '')
@echo  'Cleaning targets:'
diff --git a/configure b/configure
index add87ff4d4..5a41c87cc3 100755
--- a/configure
+++ b/configure
@@ -6094,7 +6094,7 @@ qemu_version=$(head $source_path/VERSION)
 echo "VERSION=$qemu_version" >>$config_host_mak
 echo "PKGVERSION=$pkgversion" >>$config_host_mak
 echo "SRC_PATH=$source_path" >> $config_host_mak
-echo "TARGET_DIRS=$target_list" >> $config_host_mak
+echo "TARGET_LIST=$target_list" >> $config_host_mak
 if [ "$docs" = "yes" ] ; then
   echo "BUILD_DOCS=yes" >> $config_host_mak
 fi
diff --git a/scripts/create_config b/scripts/create_config
index d727e5e36e..58948a67a4 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -107,7 +107,7 @@ case $line in
 target_name=${line#*=}
 echo "#define TARGET_NAME \"$target_name\""
 ;;
- TARGET_DIRS=*)
+ TARGET_LIST=*)
 # do nothing
 ;;
  TARGET_*=y) # configuration
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3b9a5e31a2..3d2f0458ab 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -854,7 +854,7 @@ endif
 
 # QTest rules
 
-TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
+TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_LIST)))
 ifeq ($(CONFIG_POSIX),y)
 QTEST_TARGETS = $(TARGETS)
 check-qtest-y=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y))
-- 
2.16.2

[Qemu-devel] [PATCH v1 00/24] fix building of tests/tcg

[Alex Bennée]

Hi,

We've talked about fixing this up for a long time and there have been
several RFC patches sent attempting to do that. This is yet another
RFC but hopefully shows a way forward which can build on our docker
support but also allow users to specify their own guest cross
compilers.

So far I've built arm, aarch64, ppc64 and s390x but adding support for
additional guests is simple, especially if we already have a docker
cross compile setup.

I've weeded out any tests that linked with QEMU (testpath) and reduced
the expectations of any tests/tcg test to just having access to libc.
This is the lowest common denominator for anything that can reasonably
be called a cross compiler.

The tests are all aimed at linux-user targets. We could probably come
up with something for building bare-metal softmmu tests but I suspect
that might be better served by using a different mechanism to import
existing tests into our build (e.g. kvm-unit-tests).

Let's breakdown the series:

configure:

As we need make magic we move the detection of docker and cross
compilers into configure. If any cross compiler is detected for a
given target we set CROSS_CC_GUEST in each config-target.mak. A
functioning cross compiler takes precedence over the docker fallback.
I think this makes sense for individual target sub-maintainers as they
likely already have a cross compile setup.

For docker we only set HAVE_USER_DOCKER in config-host.mak if the user
can run docker without sudo. We still need match up a docker image and
compiler once we build. This is handled by
tests/tcg/$(ARCH)/Makefile.include.

docker:

I've extended Fam's original patches to allow a bit more flexibility
for the "cc" command. I'm not overly wedded to using the docker.py
wrapper, we could just construct the command directly in Make if we
wanted to.

tests/tcg:

The top of the tree has been cleared out and everything moved into
sub-directories. There is a new multiarch sub-directory which is built
for every linux-user guest that has compiler support. Additional tests
are then included from tests/tcg/$(ARCH)/*.c or manually
added/modified by tests/tcg/$(ARCH)/Makefile.target.

Makefile:

As we are building tests/tcg for each target so Makefile.target now
invokes a sub-make with tests/tcg/Makefile while in the appropriate
build directory $(ARCH)-linux-user/tests/. The check-tcg and clean-tcg
targets unroll into all the configured FOO-linux-user targets and will
build the tests if cross compilation is available, otherwise the build
is skipped without failing the make.

Running tests:

Currently this is done manually from the build directory:

  ./qemu-arm tests/hello-arm

However once we have everything converted it shouldn't be too hard to
plumb into the normal make check sequence.

So what do people think? Is this a viable way to go forward?

Alex Bennée (20):
  configure: add test for docker availability
  configure: add support for --cross-cc-FOO
  configure: move i386_cc to cross_cc_i386
  docker: extend "cc" command to accept compiler
  docker: allow "cc" command to run in user context
  docker: Makefile.include introduce DOCKER_SCRIPT
  tests/tcg: move architecture independent tests into subdir
  tests/tcg: move i386 specific tests into subdir
  tests/tcg/i386: move test-i386-sse.c to tests/tcg/x86_64/test-sse.c
  tests/tcg/i386: fix test-i386
  tests/tcg/i368: fix hello-i386
  tests/tcg/i386: fix test-i386-fprem
  tests/tcg: move ARM specific tests into subdir
  tests/tcg/arm: fix hello-arm
  tests/tcg: move MIPS specific tests into subdir
  tests/tcg: enable building for s390x
  tests/tcg: enable building for ppc64
  tests/tcg/Makefile: update to be called from Makefile.target
  Makefile.target: add (clean-)guest-tests targets
  tests/Makefile.include: add (clean-)check-tcg targets

Fam Zheng (4):
  Makefile: Rename TARGET_DIRS to TARGET_LIST
  docker: Add "cc" subcommand
  tests/tcg/multiarch: Build fix for linux-test
  tests/tcg/i386: Build fix for hello-i386

 Makefile   |  20 +--
 Makefile.target|   5 +
 configure  |  92 +-
 scripts/create_config  |   2 +-
 tests/Makefile.include |  21 ++-
 tests/docker/Makefile.include  |  10 +-
 tests/docker/docker.py |  40 +
 tests/tcg/Makefile | 185 +
 tests/tcg/Makefile.include |  79 +
 tests/tcg/README   |  69 +---
 tests/tcg/arm/Makefile.target  |   6 +
 tests/tcg/arm/README   |  11 ++
 tests/tcg/{ => arm}/hello-arm.c|   0
 tests/tcg/{ => arm}/test-arm-iwmmxt.s  |   0
 tests/tcg/i386/Makefile.target |  35 
 tests/tcg/i386/README  |  38 +
 

[Qemu-devel] [PATCH v1 01/24] configure: add test for docker availability

[Alex Bennée]

This tests for a working docker installation without sudo and sets up
config-host.mak accordingly. This will be useful from cross compiling
things in the future.

Signed-off-by: Alex Bennée 
---
 configure | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/configure b/configure
index 4d0e92c96c..b402befe94 100755
--- a/configure
+++ b/configure
@@ -451,6 +451,7 @@ jemalloc="no"
 replication="yes"
 vxhs=""
 libxml2=""
+docker="no"
 
 supported_cpu="no"
 supported_os="no"
@@ -5396,6 +5397,23 @@ EOF
   fi
 fi
 
+##
+# Docker and cross-compiler support
+#
+# This is specifically for building test
+# cases for foreign architectures, not
+# cross-compiling QEMU itself.
+
+if has "docker"; then
+if docker images  >/dev/null 2>&1 ; then
+docker="yes"
+else
+# docker may be available but using sudo
+# so we won't use it for cross-building
+docker="maybe"
+fi
+fi
+
 ##
 # End of CC checks
 # After here, no more $cc or $ld runs
@@ -5857,6 +5875,7 @@ echo "avx2 optimization $avx2_opt"
 echo "replication support $replication"
 echo "VxHS block device $vxhs"
 echo "capstone  $capstone"
+echo "docker$docker"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -6680,6 +6699,10 @@ if test "$gcov" = "yes" ; then
   echo "GCOV=$gcov_tool" >> $config_host_mak
 fi
 
+if test "$docker" = "yes"; then
+echo "HAVE_USER_DOCKER=y" >> $config_host_mak
+fi
+
 # use included Linux headers
 if test "$linux" = "yes" ; then
   mkdir -p linux-headers
-- 
2.16.2

[Qemu-devel] [PATCH v1 02/24] configure: add support for --cross-cc-FOO

[Alex Bennée]

This allows us to specify cross compilers for our guests. This is
useful for building test images/programs. Currently we re-run the
compile test for each target. I couldn't think of a way to cache the
value for a given arch without getting messier configure code.

The cross compiler for the guest is visible to each target as
CROSS_CC_GUEST in config-target.mak.

Signed-off-by: Alex Bennée 
---
 configure | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/configure b/configure
index b402befe94..b5f3b3fe29 100755
--- a/configure
+++ b/configure
@@ -453,6 +453,13 @@ vxhs=""
 libxml2=""
 docker="no"
 
+# cross compilers defaults, can be overridden with --cross-cc-ARCH
+cross_cc_aarch64="aarch64-linux-gnu-gcc"
+cross_cc_arm="arm-linux-gnueabihf-gcc"
+cross_cc_powerpc="powerpc-linux-gnu-gcc"
+
+enabled_cross_compilers=""
+
 supported_cpu="no"
 supported_os="no"
 bogus_os="no"
@@ -483,6 +490,11 @@ for opt do
   ;;
   --disable-debug-info) debug_info="no"
   ;;
+  --cross-cc-*[!a-zA-Z0-9_0]=*) error_exit "Passed bad --cross-cc-FOO option"
+  ;;
+  --cross-cc-*) cc_arch=${opt#--cross-cc-}
+eval "cross_cc_${cc_arch}=\$optarg"
+  ;;
   esac
 done
 # OS specific
@@ -675,10 +687,12 @@ case "$cpu" in
   i386|i486|i586|i686|i86pc|BePC)
 cpu="i386"
 supported_cpu="yes"
+cross_cc_i386=gcc
   ;;
   x86_64|amd64)
 cpu="x86_64"
 supported_cpu="yes"
+cross_cc_x86_64=gcc
   ;;
   armv*b|armv*l|arm)
 cpu="arm"
@@ -912,6 +926,8 @@ for opt do
   ;;
   --disable-debug-info)
   ;;
+  --cross-cc-*)
+  ;;
   --enable-modules)
   modules="yes"
   ;;
@@ -6766,6 +6782,8 @@ case "$target" in
 ;;
 esac
 
+target_compiler=""
+
 mkdir -p $target_dir
 echo "# Automatically generated by configure - do not modify" > 
$config_target_mak
 
@@ -6794,6 +6812,7 @@ case "$target_name" in
 bflt="yes"
 mttcg="yes"
 gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
+target_compiler=$cross_cc_arm
   ;;
   aarch64|aarch64_be)
 TARGET_ARCH=aarch64
@@ -6801,6 +6820,7 @@ case "$target_name" in
 bflt="yes"
 mttcg="yes"
 gdb_xml_files="aarch64-core.xml aarch64-fpu.xml arm-core.xml arm-vfp.xml 
arm-vfp3.xml arm-neon.xml"
+target_compiler=$cross_cc_aarch64
   ;;
   cris)
   ;;
@@ -6842,6 +6862,7 @@ case "$target_name" in
   ;;
   ppc)
 gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml"
+target_compiler=$cross_cc_powerpc
   ;;
   ppcemb)
 TARGET_BASE_ARCH=ppc
@@ -6916,6 +6937,25 @@ if [ "$TARGET_BASE_ARCH" = "" ]; then
   TARGET_BASE_ARCH=$TARGET_ARCH
 fi
 
+# Do we have a cross compiler for this target?
+if has $target_compiler; then
+
+cat > $TMPC << EOF
+#include 
+int main(void) {
+printf("Hello World!\n");
+}
+EOF
+
+if ! do_compiler $target_compiler -o $TMPE $TMPC -static ; then
+target_compiler=""
+else
+enabled_cross_compilers="${enabled_cross_compilers} ${target_compiler}"
+fi
+else
+target_compiler=""
+fi
+
 symlink "$source_path/Makefile.target" "$target_dir/Makefile"
 
 upper() {
@@ -6989,6 +7029,10 @@ if test "$target_bsd_user" = "yes" ; then
   echo "CONFIG_BSD_USER=y" >> $config_target_mak
 fi
 
+if test -n "$target_compiler"; then
+  echo "CROSS_CC_GUEST=$target_compiler" >> $config_target_mak
+fi
+
 # generate QEMU_CFLAGS/LDFLAGS for targets
 
 cflags=""
@@ -7111,6 +7155,12 @@ echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
 
 done # for target in $targets
 
+if test -n "$enabled_cross_compilers"; then
+echo
+echo "NOTE: cross-compilers enabled:"
+printf '%s\n' $enabled_cross_compilers | sort -u
+fi
+
 if [ "$dtc_internal" = "yes" ]; then
   echo "config-host.h: subdir-dtc" >> $config_host_mak
 fi
-- 
2.16.2

[Qemu-devel] [PATCH for 2.13 v2 2/2] Revert "spapr: Don't allow memory hotplug to memory less nodes"

[Serhii Popovych]

This reverts commit b556854bd8524c26b8be98ab1bfdf0826831e793.

Leave change @node type from uint32_t to to int from reverted commit
because node < 0 is always false.

Note that implementing capability or some trick to detect if guest
kernel does not support hot-add to memory: this returns previous
behavour where memory added to first non-empty node.

Signed-off-by: Serhii Popovych 
---
 hw/ppc/spapr.c | 22 --
 1 file changed, 22 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3f61785..cd7a347 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3488,28 +3488,6 @@ static void spapr_machine_device_plug(HotplugHandler 
*hotplug_dev,
 return;
 }
 
-/*
- * Currently PowerPC kernel doesn't allow hot-adding memory to
- * memory-less node, but instead will silently add the memory
- * to the first node that has some memory. This causes two
- * unexpected behaviours for the user.
- *
- * - Memory gets hotplugged to a different node than what the user
- *   specified.
- * - Since pc-dimm subsystem in QEMU still thinks that memory belongs
- *   to memory-less node, a reboot will set things accordingly
- *   and the previously hotplugged memory now ends in the right node.
- *   This appears as if some memory moved from one node to another.
- *
- * So until kernel starts supporting memory hotplug to memory-less
- * nodes, just prevent such attempts upfront in QEMU.
- */
-if (nb_numa_nodes && !numa_info[node].node_mem) {
-error_setg(errp, "Can't hotplug memory to memory-less node %d",
-   node);
-return;
-}
-
 spapr_memory_plug(hotplug_dev, dev, node, errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
 spapr_core_plug(hotplug_dev, dev, errp);
-- 
1.8.3.1

[Qemu-devel] [PATCH for 2.13 v2 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes

[Serhii Popovych]

Now PowerPC Linux kernel supports hot-add to NUMA nodes not populated
initially with memory we can enable such support in qemu. This requires
two changes:

  o Add device tree property "ibm,max-associativity-domains" to let
guest kernel chance to find max possible NUMA node

  o Revert  commit b556854bd852 ("spapr: Don't allow memory hotplug to
memory less nodes") to remove check for hot-add to memory-less node.

See description messges for individual changes for more details.

v2:
  - Reorder patches in series according to description above.
  - Add extra coment to revert noticing return to previous behaviour for
guests without support for hot-add to empty node.
  - Drop max_cpus from topology in property due to vcpu id discontiguous
allocations. Thanks to David Gibson for extra explanation.
  - Rebase to current state of master branch.

Serhii Popovych (2):
  spapr: Add ibm,max-associativity-domains property
  Revert "spapr: Don't allow memory hotplug to memory less nodes"

 hw/ppc/spapr.c | 33 +++--
 1 file changed, 11 insertions(+), 22 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH for 2.13 v2 1/2] spapr: Add ibm, max-associativity-domains property

[Serhii Popovych]

Now recent kernels (i.e. since linux-stable commit a346137e9142
("powerpc/numa: Use ibm,max-associativity-domains to discover possible nodes")
support this property to mark initially memory-less NUMA nodes as "possible"
to allow further memory hot-add to them.

Advertise this property for pSeries machines to let guest kernels detect
maximum supported node configuration and benefit from kernel side change
when hot-add memory to specific, possibly empty before, NUMA node.

Signed-off-by: Serhii Popovych 
---
 hw/ppc/spapr.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2c0be8c..3f61785 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -909,6 +909,14 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void 
*fdt)
 0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
 cpu_to_be32(max_cpus / smp_threads),
 };
+uint32_t maxdomains[] = {
+cpu_to_be32(5),
+cpu_to_be32(0),
+cpu_to_be32(0),
+cpu_to_be32(0),
+cpu_to_be32(nb_numa_nodes - 1),
+cpu_to_be32(0),
+};
 
 _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
 
@@ -945,6 +953,9 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void 
*fdt)
 _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",
  refpoints, sizeof(refpoints)));
 
+_FDT(fdt_setprop(fdt, rtas, "ibm,max-associativity-domains",
+ maxdomains, sizeof(maxdomains)));
+
 _FDT(fdt_setprop_cell(fdt, rtas, "rtas-error-log-max",
   RTAS_ERROR_LOG_MAX));
 _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate",
-- 
1.8.3.1

[Qemu-devel] [Bug 1762558] Re: Many crashes with "memslot_get_virt: slot_id 170 too big"-type errors in 2.12.0 rc2

[Adam Williamson]

...on the other hand, I was clearly not thinking straight in associating
this with the qemu version bump in Rawhide, because we don't *run* that
qemu. We use the qemu from the worker host, not from the image under
test, and the worker hosts are not running Rawhide, and their qemu
hasn't changed during the time this problem appeared.

It still seems like the change that triggered this problem is something
that changed in Rawhide between 2018-04-02 and 2018-04-07, but whatever
that is, it almost certainly isn't qemu. You can close this issue for
now, then. Sorry for the thinko.

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

Title:
  Many crashes with "memslot_get_virt: slot_id 170 too big"-type errors
  in 2.12.0 rc2

Status in QEMU:
  New

Bug description:
  Since qemu 2.12.0 rc2 - qemu-2.12.0-0.6.rc2.fc29 - landed in Fedora
  Rawhide, just about all of our openQA-automated tests of Rawhide
  guests which run with qxl / SPICE graphics in the guest have died
  partway in, always shortly after the test switches from the installer
  (an X environment) to a console on a tty. qemu is, I think, hanging.
  There are always some errors like this right around the time of the
  hang:

  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: id 0, group 0, virt start 0, 
virt end , generation 0, delta 0
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: id 1, group 1, virt start 
7f42dbc0, virt end 7f42dfbfe000, generation 0, delta 7f42dbc0
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: id 2, group 1, virt start 
7f42d7a0, virt end 7f42dba0, generation 0, delta 7f42d7a0
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: 
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: (process:45812): Spice-CRITICAL 
**: memslot.c:111:memslot_get_virt: slot_id 218 too big, addr=da8e21fbda8e21fb

  or occasionally like this:

  [2018-04-09T20:13:58.0717 UTC] [debug] QEMU: id 0, group 0, virt start 0, 
virt end , generation 0, delta 0
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: id 1, group 1, virt start 
7ff093c0, virt end 7ff097bfe000, generation 0, delta 7ff093c0
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: id 2, group 1, virt start 
7ff08fa0, virt end 7ff093a0, generation 0, delta 7ff08fa0
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: 
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: (process:25622): Spice-WARNING 
**: memslot.c:68:memslot_validate_virt: virtual address out of range
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: virt=0x0+0x18 slot_id=0 
group_id=1
  [2018-04-09T20:13:58.0721 UTC] [debug] QEMU: slot=0x0-0x0 delta=0x0
  [2018-04-09T20:13:58.0721 UTC] [debug] QEMU: 
  [2018-04-09T20:13:58.0721 UTC] [debug] QEMU: (process:25622): Spice-WARNING 
**: display-channel.c:2426:display_channel_validate_surface: invalid surface_id 
1048576
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: id 0, group 0, virt start 0, 
virt end , generation 0, delta 0
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: id 1, group 1, virt start 
7ff093c0, virt end 7ff097bfe000, generation 0, delta 7ff093c0
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: id 2, group 1, virt start 
7ff08fa0, virt end 7ff093a0, generation 0, delta 7ff08fa0
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: 
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: (process:25622): Spice-CRITICAL 
**: memslot.c:122:memslot_get_virt: address generation is not valid, group_id 
1, slot_id 0, gen 110, slot_gen 0

  The same tests running on Fedora 28 guests on the same hosts are not
  hanging, and the same tests were not hanging right before the qemu
  package got updated, so this seems very strongly tied to the new qemu.

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

Re: [Qemu-devel] [RfC PATCH] Add udmabuf misc device

[Dongwon Kim]

On Tue, Apr 10, 2018 at 09:37:53AM +0300, Oleksandr Andrushchenko wrote:
> On 04/06/2018 09:57 PM, Dongwon Kim wrote:
> >On Fri, Apr 06, 2018 at 03:36:03PM +0300, Oleksandr Andrushchenko wrote:
> >>On 04/06/2018 02:57 PM, Gerd Hoffmann wrote:
> >>>   Hi,
> >>>
> >I fail to see any common ground for xen-zcopy and udmabuf ...
> Does the above mean you can assume that xen-zcopy and udmabuf
> can co-exist as two different solutions?
> >>>Well, udmabuf route isn't fully clear yet, but yes.
> >>>
> >>>See also gvt (intel vgpu), where the hypervisor interface is abstracted
> >>>away into a separate kernel modules even though most of the actual vgpu
> >>>emulation code is common.
> >>Thank you for your input, I'm just trying to figure out
> >>which of the three z-copy solutions intersect and how much
> And what about hyper-dmabuf?
> >xen z-copy solution is pretty similar fundamentally to hyper_dmabuf
> >in terms of these core sharing feature:
> >
> >1. the sharing process - import prime/dmabuf from the producer -> extract
> >underlying pages and get those shared -> return references for shared pages

Another thing is danvet was kind of against to the idea of importing existing
dmabuf/prime buffer and forward it to the other domain due to synchronization
issues. He proposed to make hyper_dmabuf only work as an exporter so that it
can have a full control over the buffer. I think we need to talk about this
further as well.

danvet, can you comment on this topic?

> >
> >2. the page sharing mechanism - it uses Xen-grant-table.
> >
> >And to give you a quick summary of differences as far as I understand
> >between two implementations (please correct me if I am wrong, Oleksandr.)
> >
> >1. xen-zcopy is DRM specific - can import only DRM prime buffer
> >while hyper_dmabuf can export any dmabuf regardless of originator
> Well, this is true. And at the same time this is just a matter
> of extending the API: xen-zcopy is a helper driver designed for
> xen-front/back use-case, so this is why it only has DRM PRIME API
> >
> >2. xen-zcopy doesn't seem to have dma-buf synchronization between two VMs
> >while (as danvet called it as remote dmabuf api sharing) hyper_dmabuf sends
> >out synchronization message to the exporting VM for synchronization.
> This is true. Again, this is because of the use-cases it covers.
> But having synchronization for a generic solution seems to be a good idea.

Yeah, understood xen-zcopy works ok with your use case. But I am just curious
if it is ok not to have any inter-domain synchronization in this sharing model.
The buffer being shared is technically dma-buf and originator needs to be able
to keep track of it.

> >
> >3. 1-level references - when using grant-table for sharing pages, there will
> >be same # of refs (each 8 byte)
> To be precise, grant ref is 4 bytes
You are right. Thanks for correction.;)

> >as # of shared pages, which is passed to
> >the userspace to be shared with importing VM in case of xen-zcopy.
> The reason for that is that xen-zcopy is a helper driver, e.g.
> the grant references come from the display backend [1], which implements
> Xen display protocol [2]. So, effectively the backend extracts references
> from frontend's requests and passes those to xen-zcopy as an array
> of refs.
> >  Compared
> >to this, hyper_dmabuf does multiple level addressing to generate only one
> >reference id that represents all shared pages.
> In the protocol [2] only one reference to the gref directory is passed
> between VMs
> (and the gref directory is a single-linked list of shared pages containing
> all
> of the grefs of the buffer).

ok, good to know. I will look into its implementation in more details but is
this gref directory (chained grefs) something that can be used for any general
memory sharing use case or is it jsut for xen-display (in current code base)?

> 
> >
> >4. inter VM messaging (hype_dmabuf only) - hyper_dmabuf has inter-vm msg
> >communication defined for dmabuf synchronization and private data (meta
> >info that Matt Roper mentioned) exchange.
> This is true, xen-zcopy has no means for inter VM sync and meta-data,
> simply because it doesn't have any code for inter VM exchange in it,
> e.g. the inter VM protocol is handled by the backend [1].
> >
> >5. driver-to-driver notification (hyper_dmabuf only) - importing VM gets
> >notified when newdmabuf is exported from other VM - uevent can be optionally
> >generated when this happens.
> >
> >6. structure - hyper_dmabuf is targetting to provide a generic solution for
> >inter-domain dmabuf sharing for most hypervisors, which is why it has two
> >layers as mattrope mentioned, front-end that contains standard API and 
> >backend
> >that is specific to hypervisor.
> Again, xen-zcopy is decoupled from inter VM communication
> >>>No idea, didn't look at it in detail.
> >>>
> >>>Looks pretty complex from a distant view.  Maybe because it tries to
> >>>build a communication framework using dma-bufs instead of a 

Re: [Qemu-devel] [PATCH 1/2] Fix error message about compressed clusters with OFLAG_COPIED

[Alberto Garcia]

On Tue 10 Apr 2018 06:18:28 PM CEST, Eric Blake wrote:
> On 04/10/2018 11:05 AM, Alberto Garcia wrote:
>> Compressed clusters are not supposed to have the COPIED bit set.
>> "qemu-img check" detects that and prints an error message reporting
>> the number of the affected host cluster. This doesn't make much sense
>> because compressed clusters are not aligned to host clusters, so it
>> would be better to report the offset instead. Plus, the calculation is
>> wrong and it uses the raw L2 entry as if it was simply an offset.
>> 
>> This patch fixes the error message and reports the offset of the
>> compressed cluster.
>> 
>> Signed-off-by: Alberto Garcia 
>> ---
>>  block/qcow2-refcount.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>
> Do we have iotests coverage of this?

I have one half-written, but I was thinking to put it in 214, so I'll
wait until Max's patch is merged.

Berto

Re: [Qemu-devel] [PATCH 04/10] target/xtensa: avoid integer overflow in next_page PC check

[Max Filippov]

On Tue, Apr 10, 2018 at 9:19 AM, Emilio G. Cota  wrote:
> If the PC is in the last page of the address space, next_page_start
> overflows to 0. Fix it.
>
> Cc: Max Filippov 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/xtensa/translate.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)

Acked-by: Max Filippov 

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 2/2] specs/qcow2: Clarify that compressed clusters have the COPIED bit reset

[Eric Blake]

On 04/10/2018 11:05 AM, Alberto Garcia wrote:
> Compressed clusters are not supposed to have the COPIED bit set, but
> this is not made explicit in the specs, so let's document it.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  docs/interop/qcow2.txt | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index feb711fb6a..8e1547ded2 100644
> --- a/docs/interop/qcow2.txt
> +++ b/docs/interop/qcow2.txt
> @@ -400,10 +400,10 @@ L2 table entry:
>62:   0 for standard clusters
>  1 for compressed clusters
>  
> -  63:   0 for a cluster that is unused or requires COW, 1 if its
> -refcount is exactly one. This information is only 
> accurate
> -in L2 tables that are reachable from the active L1
> -table.
> +  63:   0 for clusters that are unused, compressed or require 
> COW.
> +1 for standard clusters whose refcount is exactly one.
> +This information is only accurate in L2 tables
> +that are reachable from the active L1 table.

This matches what qemu outputs, so the question becomes whether it is
technically necessary to make this requirement mandatory for 3rd-party
implementations.  But I'm in favor of the tighter wording, as it gets
rather hairy to check whether exactly one compressed cluster is
occupying a host cluster, plus I don't want to think about what happens
if a compressed cluster with the bit set crosses a host cluster boundary
(does it mean that compressed cluster is the only [remaining] source of
data for BOTH host clusters at once, where both the head of the first
host cluster and tail of the second host cluster is unused?)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1761535] Re: qemu-aarch64-static docker arm64v8/openjdk coredump

[Richard Henwood]

Many thanks!

I've just compiled master, and docker/aarch64/openjdk image now works as
expected on my x86 machine.

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

Title:
  qemu-aarch64-static docker arm64v8/openjdk coredump

Status in QEMU:
  Fix Committed

Bug description:
  I am using qemu-aarch64-static to run the arm64v8/openjdk official
  image on my x86 machine. Using QEMU master, I immediately hit a bug
  which hangs the container. With Ubuntu default version qemu-aarch64
  version 2.5.0 (Debian 1:2.5+dfsg-5ubuntu10.24) and qemu-aarch64
  version 2.11.1 (v2.11.1-dirty) the hang does not take place.

  To reproduce (and get to the core dump):

  $ /tmp/tmptgyg3nvh/qemu-aarch64-static/qemu-aarch64-static -version
  qemu-aarch64 version 2.11.91 (v2.12.0-rc1-5-g47d3b60-dirty)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  $ docker run -it -v 
/tmp/tmptgyg3nvh/qemu-aarch64-static:/usr/bin/qemu-aarch64-static 
arm64v8/openjdk /bin/bash
  root@bf75cf45d311:/# javac
  Usage: javac  
  where possible options include:
-g Generate all debugging info
  <...snip...>
@Read options and filenames from file

  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  ...TERMINAL HANGS...

  
  To get the core dump, In a separate terminal:

  # snapshot the file system of the hung image
  $ docker commit $(docker ps -aqf "name=latest_qemu") qemu_coredump

  # connect with known working qemu
  $ docker run -t -v /usr/bin/qemu-aarch64-static:/usr/bin/qemu-aarch64-static  
-i qemu_coredump /bin/bash

  $$ ls -lat
  total 10608
  
  -rw-r--r--   1 root root 10792960 Mar 29 18:02 
qemu_bash_20180329-180251_1.core
  drwxrwxrwt   5 root root 4096 Mar 29 18:02 tmp
  

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

[Qemu-devel] [PATCH 10/10] target/mips: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Signed-off-by: Emilio G. Cota 
---
 target/mips/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index d05ee67..d8e717d 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20202,14 +20202,14 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 CPUMIPSState *env = cs->env_ptr;
 DisasContext ctx;
 target_ulong pc_start;
-target_ulong next_page_start;
+target_ulong page_start;
 int num_insns;
 int max_insns;
 int insn_bytes;
 int is_slot;
 
 pc_start = tb->pc;
-next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+page_start = pc_start & TARGET_PAGE_MASK;
 ctx.pc = pc_start;
 ctx.saved_pc = -1;
 ctx.singlestep_enabled = cs->singlestep_enabled;
@@ -20320,7 +20320,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 break;
 }
 
-if (ctx.pc >= next_page_start) {
+if (ctx.pc - page_start >= TARGET_PAGE_SIZE) {
 break;
 }
 
-- 
2.7.4

[Qemu-devel] [Bug 1762558] Re: Many crashes with "memslot_get_virt: slot_id 170 too big"-type errors in 2.12.0 rc2

[Adam Williamson]

Nothing about SPICE changed in the affected time frame. This started
happening between 2018-04-02 and 2018-04-07. The last time SPICE was
changed in Rawhide was on 2018-02-09. However, qemu was bumped from rc1
to rc2 on 2018-04-05.

It's possible that https://bugzilla.redhat.com/show_bug.cgi?id=1564210
is involved; the offending mesa for that bug was built on 2018-04-03, so
it also fits the time frame. However, the bug is not happening in Fedora
28 tests, and that same mesa change was sent to Fedora 28, so this seems
less likely. The only related thing I've yet found that changed in
Rawhide but not Fedora 28 during the time in which this bug started
happening on Rawhide but not Fedora 28 is qemu.

** Bug watch added: Red Hat Bugzilla #1564210
   https://bugzilla.redhat.com/show_bug.cgi?id=1564210

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

Title:
  Many crashes with "memslot_get_virt: slot_id 170 too big"-type errors
  in 2.12.0 rc2

Status in QEMU:
  New

Bug description:
  Since qemu 2.12.0 rc2 - qemu-2.12.0-0.6.rc2.fc29 - landed in Fedora
  Rawhide, just about all of our openQA-automated tests of Rawhide
  guests which run with qxl / SPICE graphics in the guest have died
  partway in, always shortly after the test switches from the installer
  (an X environment) to a console on a tty. qemu is, I think, hanging.
  There are always some errors like this right around the time of the
  hang:

  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: id 0, group 0, virt start 0, 
virt end , generation 0, delta 0
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: id 1, group 1, virt start 
7f42dbc0, virt end 7f42dfbfe000, generation 0, delta 7f42dbc0
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: id 2, group 1, virt start 
7f42d7a0, virt end 7f42dba0, generation 0, delta 7f42d7a0
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: 
  [2018-04-09T20:13:42.0736 UTC] [debug] QEMU: (process:45812): Spice-CRITICAL 
**: memslot.c:111:memslot_get_virt: slot_id 218 too big, addr=da8e21fbda8e21fb

  or occasionally like this:

  [2018-04-09T20:13:58.0717 UTC] [debug] QEMU: id 0, group 0, virt start 0, 
virt end , generation 0, delta 0
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: id 1, group 1, virt start 
7ff093c0, virt end 7ff097bfe000, generation 0, delta 7ff093c0
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: id 2, group 1, virt start 
7ff08fa0, virt end 7ff093a0, generation 0, delta 7ff08fa0
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: 
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: (process:25622): Spice-WARNING 
**: memslot.c:68:memslot_validate_virt: virtual address out of range
  [2018-04-09T20:13:58.0720 UTC] [debug] QEMU: virt=0x0+0x18 slot_id=0 
group_id=1
  [2018-04-09T20:13:58.0721 UTC] [debug] QEMU: slot=0x0-0x0 delta=0x0
  [2018-04-09T20:13:58.0721 UTC] [debug] QEMU: 
  [2018-04-09T20:13:58.0721 UTC] [debug] QEMU: (process:25622): Spice-WARNING 
**: display-channel.c:2426:display_channel_validate_surface: invalid surface_id 
1048576
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: id 0, group 0, virt start 0, 
virt end , generation 0, delta 0
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: id 1, group 1, virt start 
7ff093c0, virt end 7ff097bfe000, generation 0, delta 7ff093c0
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: id 2, group 1, virt start 
7ff08fa0, virt end 7ff093a0, generation 0, delta 7ff08fa0
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: 
  [2018-04-09T20:14:14.0728 UTC] [debug] QEMU: (process:25622): Spice-CRITICAL 
**: memslot.c:122:memslot_get_virt: address generation is not valid, group_id 
1, slot_id 0, gen 110, slot_gen 0

  The same tests running on Fedora 28 guests on the same hosts are not
  hanging, and the same tests were not hanging right before the qemu
  package got updated, so this seems very strongly tied to the new qemu.

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

[Qemu-devel] [PATCH 08/10] target/arm: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Peter Maydell 
Signed-off-by: Emilio G. Cota 
---
 target/arm/translate.h |  2 +-
 target/arm/translate.c | 11 +--
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/target/arm/translate.h b/target/arm/translate.h
index c47febf..2287894 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -9,7 +9,7 @@ typedef struct DisasContext {
 DisasContextBase base;
 
 target_ulong pc;
-target_ulong next_page_start;
+target_ulong page_start;
 uint32_t insn;
 /* Nonzero if this instruction has been conditionally skipped.  */
 int condjmp;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index fc03b5b..ade8d2d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9913,7 +9913,7 @@ static bool thumb_insn_is_16bit(DisasContext *s, uint32_t 
insn)
 return false;
 }
 
-if ((insn >> 11) == 0x1e && (s->pc < s->next_page_start - 3)) {
+if ((insn >> 11) == 0x1e && s->pc - s->page_start < TARGET_PAGE_SIZE - 3) {
 /* 0b_0xxx__ : BL/BLX prefix, and the suffix
  * is not on the next page; we merge this into a 32-bit
  * insn.
@@ -12269,8 +12269,7 @@ static int arm_tr_init_disas_context(DisasContextBase 
*dcbase,
 dc->is_ldex = false;
 dc->ss_same_el = false; /* Can't be true since EL_d must be AArch64 */
 
-dc->next_page_start =
-(dc->base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+dc->page_start = dc->base.pc_first & TARGET_PAGE_MASK;
 
 /* If architectural single step active, limit to 1.  */
 if (is_singlestepping(dc)) {
@@ -12280,7 +12279,7 @@ static int arm_tr_init_disas_context(DisasContextBase 
*dcbase,
 /* ARM is a fixed-length ISA.  Bound the number of insns to execute
to those left on the page.  */
 if (!dc->thumb) {
-int bound = (dc->next_page_start - dc->base.pc_first) / 4;
+int bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4;
 max_insns = MIN(max_insns, bound);
 }
 
@@ -12552,8 +12551,8 @@ static void thumb_tr_translate_insn(DisasContextBase 
*dcbase, CPUState *cpu)
  * but isn't very efficient).
  */
 if (dc->base.is_jmp == DISAS_NEXT
-&& (dc->pc >= dc->next_page_start
-|| (dc->pc >= dc->next_page_start - 3
+&& (dc->pc - dc->page_start >= TARGET_PAGE_SIZE
+|| (dc->pc - dc->page_start >= TARGET_PAGE_SIZE - 3
 && insn_crosses_page(env, dc {
 dc->base.is_jmp = DISAS_TOO_MANY;
 }
-- 
2.7.4

[Qemu-devel] [PATCH 05/10] target/unicore32: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Guan Xuetao 
Signed-off-by: Emilio G. Cota 
---
 target/unicore32/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c
index 5b51f21..abe2ea8 100644
--- a/target/unicore32/translate.c
+++ b/target/unicore32/translate.c
@@ -1875,7 +1875,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock 
*tb)
 CPUUniCore32State *env = cs->env_ptr;
 DisasContext dc1, *dc = 
 target_ulong pc_start;
-uint32_t next_page_start;
+uint32_t page_start;
 int num_insns;
 int max_insns;
 
@@ -1894,7 +1894,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock 
*tb)
 cpu_F1s = tcg_temp_new_i32();
 cpu_F0d = tcg_temp_new_i64();
 cpu_F1d = tcg_temp_new_i64();
-next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+page_start = pc_start & TARGET_PAGE_MASK;
 num_insns = 0;
 max_insns = tb_cflags(tb) & CF_COUNT_MASK;
 if (max_insns == 0) {
@@ -1951,7 +1951,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock 
*tb)
 } while (!dc->is_jmp && !tcg_op_buf_full() &&
  !cs->singlestep_enabled &&
  !singlestep &&
- dc->pc < next_page_start &&
+ dc->pc - page_start < TARGET_PAGE_SIZE &&
  num_insns < max_insns);
 
 if (tb_cflags(tb) & CF_LAST_IO) {
-- 
2.7.4

[Qemu-devel] [PATCH 07/10] target/microblaze: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: "Edgar E. Iglesias" 
Signed-off-by: Emilio G. Cota 
---
 target/microblaze/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 7628b0e..401dbe6 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1637,7 +1637,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 uint32_t pc_start;
 struct DisasContext ctx;
 struct DisasContext *dc = 
-uint32_t next_page_start, org_flags;
+uint32_t page_start, org_flags;
 target_ulong npc;
 int num_insns;
 int max_insns;
@@ -1663,7 +1663,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 cpu_abort(cs, "Microblaze: unaligned PC=%x\n", pc_start);
 }
 
-next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+page_start = pc_start & TARGET_PAGE_MASK;
 num_insns = 0;
 max_insns = tb_cflags(tb) & CF_COUNT_MASK;
 if (max_insns == 0) {
@@ -1749,7 +1749,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 } while (!dc->is_jmp && !dc->cpustate_changed
  && !tcg_op_buf_full()
  && !singlestep
- && (dc->pc < next_page_start)
+ && (dc->pc - page_start < TARGET_PAGE_SIZE)
  && num_insns < max_insns);
 
 npc = dc->pc;
-- 
2.7.4

[Qemu-devel] [PATCH 09/10] target/s390x: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Cornelia Huck 
Cc: Alexander Graf 
Cc: David Hildenbrand 
Cc: qemu-s3...@nongnu.org
Signed-off-by: Emilio G. Cota 
---
 target/s390x/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 7d39ab3..9f1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6163,7 +6163,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 CPUS390XState *env = cs->env_ptr;
 DisasContext dc;
 target_ulong pc_start;
-uint64_t next_page_start;
+uint64_t page_start;
 int num_insns, max_insns;
 ExitStatus status;
 bool do_debug;
@@ -6181,7 +6181,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 dc.ex_value = tb->cs_base;
 do_debug = dc.singlestep_enabled = cs->singlestep_enabled;
 
-next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+page_start = pc_start & TARGET_PAGE_MASK;
 
 num_insns = 0;
 max_insns = tb_cflags(tb) & CF_COUNT_MASK;
@@ -6218,7 +6218,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 /* If we reach a page boundary, are single stepping,
or exhaust instruction count, stop generation.  */
 if (status == NO_EXIT
-&& (dc.pc >= next_page_start
+&& (dc.pc - page_start >= TARGET_PAGE_SIZE
 || tcg_op_buf_full()
 || num_insns >= max_insns
 || singlestep
-- 
2.7.4

[Qemu-devel] [PATCH 01/10] target/riscv: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Reported-by: Richard Henderson 
Suggested-by: Richard Henderson 
Cc: Michael Clark 
Cc: Palmer Dabbelt 
Cc: Sagar Karandikar 
Cc: Bastian Koppelmann 
Signed-off-by: Emilio G. Cota 
---
 target/riscv/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 808eab7..d2d2e5e 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1849,11 +1849,11 @@ void gen_intermediate_code(CPUState *cs, 
TranslationBlock *tb)
 CPURISCVState *env = cs->env_ptr;
 DisasContext ctx;
 target_ulong pc_start;
-target_ulong next_page_start;
+target_ulong page_start;
 int num_insns;
 int max_insns;
 pc_start = tb->pc;
-next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+page_start = pc_start & TARGET_PAGE_MASK;
 ctx.pc = pc_start;
 
 /* once we have GDB, the rest of the translate.c implementation should be
@@ -1903,7 +1903,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock 
*tb)
 if (cs->singlestep_enabled) {
 break;
 }
-if (ctx.pc >= next_page_start) {
+if (ctx.pc - page_start >= TARGET_PAGE_SIZE) {
 break;
 }
 if (tcg_op_buf_full()) {
-- 
2.7.4

[Qemu-devel] [PATCH 02/10] target/cris: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: "Edgar E. Iglesias" 
Signed-off-by: Emilio G. Cota 
---
 target/cris/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/cris/translate.c b/target/cris/translate.c
index f51a731..64b9ec6 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3091,7 +3091,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 unsigned int insn_len;
 struct DisasContext ctx;
 struct DisasContext *dc = 
-uint32_t next_page_start;
+uint32_t page_start;
 target_ulong npc;
 int num_insns;
 int max_insns;
@@ -3138,7 +3138,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 
 dc->cpustate_changed = 0;
 
-next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+page_start = pc_start & TARGET_PAGE_MASK;
 num_insns = 0;
 max_insns = tb_cflags(tb) & CF_COUNT_MASK;
 if (max_insns == 0) {
@@ -3234,7 +3234,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 } while (!dc->is_jmp && !dc->cpustate_changed
 && !tcg_op_buf_full()
 && !singlestep
-&& (dc->pc < next_page_start)
+&& (dc->pc - page_start < TARGET_PAGE_SIZE)
 && num_insns < max_insns);
 
 if (dc->clear_locked_irq) {
-- 
2.7.4

[Qemu-devel] [PATCH 00/10] Avoid integer overflow in next_page_start

[Emilio G. Cota]

Richard pointed out in another thread that when computing
next_page_start we can break checks for the last page in the
address space due to integer overflow. This affects several targets;
the appended fixes them.

You can fetch the patches from:
  https://github.com/cota/qemu/tree/next_page_overflow

Thanks,

Emilio
---
 target/arm/translate.c| 11 +--
 target/arm/translate.h|  2 +-
 target/cris/translate.c   |  6 +++---
 target/lm32/translate.c   |  6 +++---
 target/microblaze/translate.c |  6 +++---
 target/mips/translate.c   |  6 +++---
 target/riscv/translate.c  |  6 +++---
 target/s390x/translate.c  |  6 +++---
 target/tilegx/translate.c |  4 ++--
 target/unicore32/translate.c  |  6 +++---
 target/xtensa/translate.c |  9 -
 11 files changed, 33 insertions(+), 35 deletions(-)

[Qemu-devel] [PATCH 03/10] target/lm32: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Michael Walle 
Signed-off-by: Emilio G. Cota 
---
 target/lm32/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/lm32/translate.c b/target/lm32/translate.c
index 2e1c5e6..fdd206a 100644
--- a/target/lm32/translate.c
+++ b/target/lm32/translate.c
@@ -1055,7 +1055,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 LM32CPU *cpu = lm32_env_get_cpu(env);
 struct DisasContext ctx, *dc = 
 uint32_t pc_start;
-uint32_t next_page_start;
+uint32_t page_start;
 int num_insns;
 int max_insns;
 
@@ -1075,7 +1075,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 pc_start &= ~3;
 }
 
-next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+page_start = pc_start & TARGET_PAGE_MASK;
 num_insns = 0;
 max_insns = tb_cflags(tb) & CF_COUNT_MASK;
 if (max_insns == 0) {
@@ -1115,7 +1115,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
  && !tcg_op_buf_full()
  && !cs->singlestep_enabled
  && !singlestep
- && (dc->pc < next_page_start)
+ && (dc->pc - page_start < TARGET_PAGE_SIZE)
  && num_insns < max_insns);
 
 if (tb_cflags(tb) & CF_LAST_IO) {
-- 
2.7.4

[Qemu-devel] [PATCH 04/10] target/xtensa: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Cc: Max Filippov 
Signed-off-by: Emilio G. Cota 
---
 target/xtensa/translate.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 4f6d030..aad4963 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -1061,8 +1061,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock 
*tb)
 int insn_count = 0;
 int max_insns = tb_cflags(tb) & CF_COUNT_MASK;
 uint32_t pc_start = tb->pc;
-uint32_t next_page_start =
-(pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+uint32_t page_start = pc_start & TARGET_PAGE_MASK;
 
 if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
@@ -1162,9 +1161,9 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock 
*tb)
 }
 } while (dc.is_jmp == DISAS_NEXT &&
 insn_count < max_insns &&
-dc.pc < next_page_start &&
-dc.pc + xtensa_insn_len(env, ) <= next_page_start &&
-!tcg_op_buf_full());
+dc.pc - page_start < TARGET_PAGE_SIZE &&
+dc.pc - page_start + xtensa_insn_len(env, ) <= TARGET_PAGE_SIZE
+&& !tcg_op_buf_full());
 done:
 reset_sar_tracker();
 if (dc.icount) {
-- 
2.7.4

[Qemu-devel] [PATCH 06/10] target/tilegx: avoid integer overflow in next_page PC check

[Emilio G. Cota]

If the PC is in the last page of the address space, next_page_start
overflows to 0. Fix it.

Signed-off-by: Emilio G. Cota 
---
 target/tilegx/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/tilegx/translate.c b/target/tilegx/translate.c
index d63bf5b..6c53c5e 100644
--- a/target/tilegx/translate.c
+++ b/target/tilegx/translate.c
@@ -2375,7 +2375,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 DisasContext ctx;
 DisasContext *dc = 
 uint64_t pc_start = tb->pc;
-uint64_t next_page_start = (pc_start & TARGET_PAGE_MASK) + 
TARGET_PAGE_SIZE;
+uint64_t page_start = pc_start & TARGET_PAGE_MASK;
 int num_insns = 0;
 int max_insns = tb_cflags(tb) & CF_COUNT_MASK;
 
@@ -2415,7 +2415,7 @@ void gen_intermediate_code(CPUState *cs, struct 
TranslationBlock *tb)
 }
 dc->pc += TILEGX_BUNDLE_SIZE_IN_BYTES;
 if (num_insns >= max_insns
-|| dc->pc >= next_page_start
+|| (dc->pc - page_start >= TARGET_PAGE_SIZE)
 || tcg_op_buf_full()) {
 /* Ending the TB due to TB size or page boundary.  Set PC.  */
 tcg_gen_movi_tl(cpu_pc, dc->pc);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/2] Fix error message about compressed clusters with OFLAG_COPIED

[Eric Blake]

On 04/10/2018 11:05 AM, Alberto Garcia wrote:
> Compressed clusters are not supposed to have the COPIED bit set.
> "qemu-img check" detects that and prints an error message reporting
> the number of the affected host cluster. This doesn't make much sense
> because compressed clusters are not aligned to host clusters, so it
> would be better to report the offset instead. Plus, the calculation is
> wrong and it uses the raw L2 entry as if it was simply an offset.
> 
> This patch fixes the error message and reports the offset of the
> compressed cluster.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2-refcount.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Do we have iotests coverage of this?

Should the qcow2 spec be explicit that OFLAG_COPIED should never be set
on a compressed cluster? The current documentation for L2 table entry
mentions that bit 63 is '1' for a cluster with a refcount of exactly 1
if it is an L2 table reachable from the active L1 table, with no mention
of a restriction that it must not be set when bit 62 is set.  Or is it
feasible that although qemu itself (should) never set OFLAG_COPIED on a
compressed cluster, that some third-party tool could still validly set
the bit for a compressed cluster that happens to occupy a host cluster
with a refcount of exactly 1 (and where writing to that cluster could be
done by replacing the cluster in-place with uncompressed data, or by
again writing compressed data - compression is rather wasteful in that
sense as the tail of the host cluster is unused, and the only way to use
the tail of the cluster is with another compressed cluster at which
point the refcount is no longer 1).

> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 6b8b63514a..2dc23005b7 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1577,9 +1577,9 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>  case QCOW2_CLUSTER_COMPRESSED:
>  /* Compressed clusters don't have QCOW_OFLAG_COPIED */
>  if (l2_entry & QCOW_OFLAG_COPIED) {
> -fprintf(stderr, "ERROR: cluster %" PRId64 ": "
> +fprintf(stderr, "ERROR: coffset=0x%" PRIx64 ": "
>  "copied flag must never be set for compressed "
> -"clusters\n", l2_entry >> s->cluster_bits);
> +"clusters\n", l2_entry & s->cluster_offset_mask);
>  l2_entry &= ~QCOW_OFLAG_COPIED;

At any rate, regardless of the answers to my questions, the error
message cleanup makes sense.
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL v2 0/3] linux-user fixes for -rc3

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180410160142.21096-1-laur...@vivier.eu
Subject: [Qemu-devel] [PULL v2 0/3] linux-user fixes for -rc3

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/20180410160142.21096-1-laur...@vivier.eu -> 
patchew/20180410160142.21096-1-laur...@vivier.eu
Switched to a new branch 'test'
3d63c01ac9 linux-user: implement HWCAP bits on MIPS
53058bad56 linux-user: add microblaze/microblazeel magic numbers in 
qemu-binfmt-conf.sh
871304edc3 linux-user: fix microblaze get_sp_from_cpustate()

=== OUTPUT BEGIN ===
Checking PATCH 1/3: linux-user: fix microblaze get_sp_from_cpustate()...
Checking PATCH 2/3: linux-user: add microblaze/microblazeel magic numbers in 
qemu-binfmt-conf.sh...
ERROR: line over 90 characters
#24: FILE: scripts/qemu-binfmt-conf.sh:7:
+sh4 sh4eb s390x aarch64 aarch64_be hppa riscv32 riscv64 xtensa xtensaeb 
microblaze microblazeel"

WARNING: line over 80 characters
#32: FILE: scripts/qemu-binfmt-conf.sh:119:
+microblaze_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\xba\xab'

ERROR: line over 90 characters
#33: FILE: scripts/qemu-binfmt-conf.sh:120:
+microblaze_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'

ERROR: line over 90 characters
#36: FILE: scripts/qemu-binfmt-conf.sh:123:
+microblazeel_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\xab\xba'

ERROR: line over 90 characters
#37: FILE: scripts/qemu-binfmt-conf.sh:124:
+microblazeel_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'

total: 4 errors, 1 warnings, 26 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/3: linux-user: implement HWCAP bits on MIPS...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH 2/2] specs/qcow2: Clarify that compressed clusters have the COPIED bit reset

[Alberto Garcia]

Compressed clusters are not supposed to have the COPIED bit set, but
this is not made explicit in the specs, so let's document it.

Signed-off-by: Alberto Garcia 
---
 docs/interop/qcow2.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index feb711fb6a..8e1547ded2 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -400,10 +400,10 @@ L2 table entry:
   62:   0 for standard clusters
 1 for compressed clusters
 
-  63:   0 for a cluster that is unused or requires COW, 1 if its
-refcount is exactly one. This information is only accurate
-in L2 tables that are reachable from the active L1
-table.
+  63:   0 for clusters that are unused, compressed or require COW.
+1 for standard clusters whose refcount is exactly one.
+This information is only accurate in L2 tables
+that are reachable from the active L1 table.
 
 Standard Cluster Descriptor:
 
-- 
2.11.0